Add --pull-secret flag (#617)

* Add --pull-secrets flag

 Fixes #616

 - Add --pull-secrets flag for service create/update operations
 - Setting empty string to flag clears the pull secrets
 - List ImagePullSecrets for service in `service describe` default output
 - Run e2e tests against latets serving v0.12.0 (ImagePullSecrets introduced in v0.11.1 release)

* Space separated field name for describe and update flag help msg

 - Update the key name in service describe:
  - ImagePullSecrets --> Image Pull Secrets
  - ServiceAccount --> Service Account
 - Update the help message for --service-account and --pull-secrets

docs/cmd/kn_service_create.md

@@ -62,10 +62,11 @@ kn service create NAME --image IMAGE [flags]
   -n, --namespace string          Specify the namespace to operate in.
       --no-lock-to-digest         do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
   -p, --port int32                The port where application listens on.
+      --pull-secret string        Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
       --requests-cpu string       The requested CPU (e.g., 250m).
       --requests-memory string    The requested memory (e.g., 64Mi).
       --revision-name string      The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}")
-      --service-account string    Service account name to set. Empty service account name will result to clear the service account.
+      --service-account string    Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --volume stringArray        Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. You can use this flag multiple times. To unset a ConfigMap/Secret reference, append "-" to the name, e.g. --volume myvolume-.
       --wait-timeout int          Seconds to wait before giving up on waiting for service to be ready. (default 600)
 ```

docs/cmd/kn_service_update.md

@@ -57,10 +57,11 @@ kn service update NAME [flags]
   -n, --namespace string          Specify the namespace to operate in.
       --no-lock-to-digest         do not keep the running image for the service constant when not explicitly specifying the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)
   -p, --port int32                The port where application listens on.
+      --pull-secret string        Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
       --requests-cpu string       The requested CPU (e.g., 250m).
       --requests-memory string    The requested memory (e.g., 64Mi).
       --revision-name string      The revision name to set. Must start with the service name and a dash as a prefix. Empty revision name will result in the server generating a name for the revision. Accepts golang templates, allowing {{.Service}} for the service name, {{.Generation}} for the generation, and {{.Random [n]}} for n random consonants. (default "{{.Service}}-{{.Random 5}}-{{.Generation}}")
-      --service-account string    Service account name to set. Empty service account name will result to clear the service account.
+      --service-account string    Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --tag strings               Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
       --traffic strings           Set traffic distribution (format: --traffic revisionRef=percent) where revisionRef can be a revision or a tag or '@latest' string representing latest ready revision. This flag can be given multiple times with percent summing up to 100%.
       --untag strings             Untag revision (format: --untag tagName). This flag can be specified multiple times.

pkg/kn/commands/service/configuration_edit_flags.go

@@ -47,6 +47,7 @@ type ConfigurationEditFlags struct {
 	NamePrefix                 string
 	RevisionName               string
 	ServiceAccountName         string
+	ImagePullSecrets           string
 	Annotations                []string
 
 	// Preferences about how to do the action.
@@ -141,13 +142,21 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 		"keep the running image for the service constant when not explicitly specifying "+
 			"the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)")
 	// Don't mark as changing the revision.
-	command.Flags().StringVar(&p.ServiceAccountName, "service-account", "", "Service account name to set. Empty service account name will result to clear the service account.")
+	command.Flags().StringVar(&p.ServiceAccountName,
+		"service-account",
+		"",
+		"Service account name to set. An empty argument (\"\") clears the service account. The referenced service account must exist in the service's namespace.")
 	p.markFlagMakesRevision("service-account")
 	command.Flags().StringArrayVar(&p.Annotations, "annotation", []string{},
 		"Service annotation to set. name=value; you may provide this flag "+
 			"any number of times to set multiple annotations. "+
 			"To unset, specify the annotation name followed by a \"-\" (e.g., name-).")
 	p.markFlagMakesRevision("annotation")
+	command.Flags().StringVar(&p.ImagePullSecrets,
+		"pull-secret",
+		"",
+		"Image pull secret to set. An empty argument (\"\") clears the pull secret. The referenced secret must exist in the service's namespace.")
+	p.markFlagMakesRevision("pull-secret")
 }
 
 // AddUpdateFlags adds the flags specific to update.
@@ -347,6 +356,10 @@ func (p *ConfigurationEditFlags) Apply(
 		}
 	}
 
+	if cmd.Flags().Changed("pull-secret") {
+		servinglib.UpdateImagePullSecrets(template, p.ImagePullSecrets)
+	}
+
 	return nil
 }
 

pkg/kn/commands/service/describe.go

@@ -169,7 +169,10 @@ func writeService(dw printers.PrefixWriter, service *v1alpha1.Service) {
 		}
 	}
 	if (service.Spec.Template != nil) && (service.Spec.Template.Spec.ServiceAccountName != "") {
-		dw.WriteAttribute("ServiceAccount", service.Spec.Template.Spec.ServiceAccountName)
+		dw.WriteAttribute("Service Account", service.Spec.Template.Spec.ServiceAccountName)
+	}
+	if service.Spec.Template != nil && service.Spec.Template.Spec.ImagePullSecrets != nil {
+		dw.WriteAttribute("Image Pull Secret", service.Spec.Template.Spec.ImagePullSecrets[0].Name)
 	}
 }
 

pkg/kn/commands/service/describe_test.go

@@ -69,7 +69,7 @@ func TestServiceDescribeBasic(t *testing.T) {
 	assert.Assert(t, cmp.Regexp(`(?m)\s*Annotations:.*\.\.\.$`, output))
 	assert.Assert(t, util.ContainsAll(output, "Labels:", "label1=lval1, label2=lval2\n"))
 	assert.Assert(t, util.ContainsAll(output, "[1]"))
-	assert.Assert(t, cmp.Regexp("ServiceAccount: \\s+default-sa", output))
+	assert.Assert(t, cmp.Regexp("Service Account: \\s+default-sa", output))
 
 	assert.Equal(t, strings.Count(output, "rev1"), 1)
 

pkg/serving/config_changes.go

@@ -407,6 +407,18 @@ func UpdateServiceAccountName(template *servingv1alpha1.RevisionTemplateSpec, se
 	return nil
 }
 
+// UpdateImagePullSecrets updates the image pull secrets used for the corresponding knative service
+func UpdateImagePullSecrets(template *servingv1alpha1.RevisionTemplateSpec, pullsecrets string) {
+	pullsecrets = strings.TrimSpace(pullsecrets)
+	if pullsecrets == "" {
+		template.Spec.ImagePullSecrets = nil
+	} else {
+		template.Spec.ImagePullSecrets = []corev1.LocalObjectReference{{
+			Name: pullsecrets,
+		}}
+	}
+}
+
 // GenerateVolumeName generates a volume name with respect to a given path string.
 // Current implementation basically sanitizes the path string by changing "/" into "."
 // To reduce any chance of duplication, a checksum part generated from the path string is appended to the sanitized string.

pkg/serving/config_changes_test.go

@@ -562,6 +562,17 @@ func TestUpdateServiceAccountName(t *testing.T) {
 	assert.Equal(t, template.Spec.ServiceAccountName, "")
 }
 
+func TestUpdateImagePullSecrets(t *testing.T) {
+	template, _ := getV1alpha1RevisionTemplateWithOldFields()
+	template.Spec.ImagePullSecrets = nil
+
+	UpdateImagePullSecrets(template, "quay")
+	assert.Equal(t, template.Spec.ImagePullSecrets[0].Name, "quay")
+
+	UpdateImagePullSecrets(template, " ")
+	assert.Check(t, template.Spec.ImagePullSecrets == nil)
+}
+
 func TestUpdateAnnotationsNew(t *testing.T) {
 	service, template, _ := getV1alpha1Service()