From ff7dffc0a354d909eaed13aaf6edba29b08a92c0 Mon Sep 17 00:00:00 2001 From: Navid Shaikh Date: Wed, 22 Jan 2020 20:27:25 +0530 Subject: [PATCH] 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 | 3 ++- docs/cmd/kn_service_update.md | 3 ++- .../commands/service/configuration_edit_flags.go | 15 ++++++++++++++- pkg/kn/commands/service/describe.go | 5 ++++- pkg/kn/commands/service/describe_test.go | 2 +- pkg/serving/config_changes.go | 12 ++++++++++++ pkg/serving/config_changes_test.go | 11 +++++++++++ 7 files changed, 46 insertions(+), 5 deletions(-) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index 44ab350a2..ed8502d4d 100644 --- a/docs/cmd/kn_service_create.md +++ b/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) ``` diff --git a/docs/cmd/kn_service_update.md b/docs/cmd/kn_service_update.md index 35f40c0d5..154d2dfc4 100644 --- a/docs/cmd/kn_service_update.md +++ b/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. diff --git a/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 3aa6ea439..d3aa169d8 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/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 } diff --git a/pkg/kn/commands/service/describe.go b/pkg/kn/commands/service/describe.go index c942f1463..7db5bdd7b 100644 --- a/pkg/kn/commands/service/describe.go +++ b/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) } } diff --git a/pkg/kn/commands/service/describe_test.go b/pkg/kn/commands/service/describe_test.go index 572f947a5..b54e62e59 100644 --- a/pkg/kn/commands/service/describe_test.go +++ b/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) diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index 53f064b94..86b9a3a8d 100644 --- a/pkg/serving/config_changes.go +++ b/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. diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index a431bf3b1..46fa666a7 100644 --- a/pkg/serving/config_changes_test.go +++ b/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()