From 66173059fc77bcb17c737d30415628ee33518aa2 Mon Sep 17 00:00:00 2001 From: Murugappan Chetty Date: Fri, 28 Feb 2020 03:55:51 -0800 Subject: [PATCH] Add --user flag for service create and update (#679) * add run as user flag #678 * add run as user flag #678 * add changelog for pr 679 * review comments for pr 679 * review comments for pr 679 * add test for config changes * add user flag --- CHANGELOG.adoc | 4 ++ docs/cmd/kn_service_create.md | 1 + docs/cmd/kn_service_update.md | 1 + .../service/configuration_edit_flags.go | 7 +++ pkg/kn/commands/service/create_mock_test.go | 24 ++++++++++ .../service/service_update_mock_test.go | 46 +++++++++++++++++++ pkg/serving/config_changes.go | 12 +++++ pkg/serving/config_changes_test.go | 18 ++++++++ 8 files changed, 113 insertions(+) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 01540f740..d97c1ba27 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -59,6 +59,10 @@ | 🧽 | Add `--wait` and `--no-wait` to service delete operation. Change service delete to wait by default. | https://github.com/knative/client/pull/682[#682] + +| 🎁 +| Add `--user` flag for specifying the user id to run the container +| https://github.com/knative/client/pull/679[#679] |=== ## v0.12.0 (2020-01-29) diff --git a/docs/cmd/kn_service_create.md b/docs/cmd/kn_service_create.md index ce4f4a99f..041a0767e 100644 --- a/docs/cmd/kn_service_create.md +++ b/docs/cmd/kn_service_create.md @@ -70,6 +70,7 @@ kn service create NAME --image IMAGE [flags] --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. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace. + --user int The user ID to run the container (e.g., 1001). --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 0792fa970..2898a2d7a 100644 --- a/docs/cmd/kn_service_update.md +++ b/docs/cmd/kn_service_update.md @@ -68,6 +68,7 @@ kn service update NAME [flags] --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. + --user int The user ID to run the container (e.g., 1001). --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/pkg/kn/commands/service/configuration_edit_flags.go b/pkg/kn/commands/service/configuration_edit_flags.go index 8731eec00..127a342a3 100644 --- a/pkg/kn/commands/service/configuration_edit_flags.go +++ b/pkg/kn/commands/service/configuration_edit_flags.go @@ -52,6 +52,7 @@ type ConfigurationEditFlags struct { ServiceAccountName string ImagePullSecrets string Annotations []string + User int64 // Preferences about how to do the action. LockToDigest bool @@ -189,6 +190,8 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) { "", "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") + command.Flags().Int64VarP(&p.User, "user", "", 0, "The user ID to run the container (e.g., 1001).") + p.markFlagMakesRevision("user") } // AddUpdateFlags adds the flags specific to update. @@ -396,6 +399,10 @@ func (p *ConfigurationEditFlags) Apply( servinglib.UpdateImagePullSecrets(template, p.ImagePullSecrets) } + if cmd.Flags().Changed("user") { + servinglib.UpdateUser(template, p.User) + } + return nil } diff --git a/pkg/kn/commands/service/create_mock_test.go b/pkg/kn/commands/service/create_mock_test.go index 395f654f0..37500b55c 100644 --- a/pkg/kn/commands/service/create_mock_test.go +++ b/pkg/kn/commands/service/create_mock_test.go @@ -32,6 +32,7 @@ import ( "knative.dev/client/pkg/wait" "knative.dev/client/pkg/util" + "knative.dev/pkg/ptr" ) func TestServiceCreateImageMock(t *testing.T) { @@ -385,6 +386,29 @@ func TestServiceCreateWithMountSecret(t *testing.T) { r.Validate() } +func TestServiceCreateWithUser(t *testing.T) { + client := knclient.NewMockKnServiceClient(t) + + r := client.Recorder() + r.GetService("foo", nil, errors.NewNotFound(servingv1.Resource("service"), "foo")) + + service := getService("foo") + + template := &service.Spec.Template + template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + RunAsUser: ptr.Int64(int64(1001)), + } + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Annotations = map[string]string{servinglib.UserImageAnnotationKey: "gcr.io/foo/bar:baz"} + r.CreateService(service, nil) + + output, err := executeServiceCommand(client, "create", "foo", "--image", "gcr.io/foo/bar:baz", "--user", "1001", "--no-wait", "--revision-name=") + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", "foo", "default")) + + r.Validate() +} + func getService(name string) *servingv1.Service { service := &servingv1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/kn/commands/service/service_update_mock_test.go b/pkg/kn/commands/service/service_update_mock_test.go index 1f9688e84..1c216a710 100644 --- a/pkg/kn/commands/service/service_update_mock_test.go +++ b/pkg/kn/commands/service/service_update_mock_test.go @@ -26,6 +26,7 @@ import ( clientserving "knative.dev/client/pkg/serving" clientservingv1 "knative.dev/client/pkg/serving/v1" "knative.dev/client/pkg/util" + "knative.dev/pkg/ptr" ) func TestServiceUpdateEnvMock(t *testing.T) { @@ -1427,3 +1428,48 @@ func TestServiceUpdateWithRemovingMount(t *testing.T) { r.Validate() } + +func TestServiceUpdateUser(t *testing.T) { + client := clientservingv1.NewMockKnServiceClient(t) + svcName := "svc1" + newService := getService(svcName) + template := &newService.Spec.Template + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + RunAsUser: ptr.Int64(int64(1001)), + } + template.ObjectMeta.Annotations = map[string]string{ + clientserving.UserImageAnnotationKey: "gcr.io/foo/bar:baz", + } + + updatedService := getService(svcName) + template = &updatedService.Spec.Template + template.Spec.Containers[0].Image = "gcr.io/foo/bar:baz" + template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ + RunAsUser: ptr.Int64(int64(1002)), + } + template.ObjectMeta.Annotations = map[string]string{ + clientserving.UserImageAnnotationKey: "gcr.io/foo/bar:baz", + } + + r := client.Recorder() + recordServiceUpdateWithSuccess(r, svcName, newService, updatedService) + + output, err := executeServiceCommand(client, + "create", svcName, "--image", "gcr.io/foo/bar:baz", + "--user", "1001", + "--no-wait", "--revision-name=", + ) + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "created", svcName, "default")) + + output, err = executeServiceCommand(client, + "update", svcName, + "--user", "1002", + "--no-wait", "--revision-name=", + ) + assert.NilError(t, err) + assert.Assert(t, util.ContainsAll(output, "updated", svcName, "default")) + + r.Validate() +} diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go index a265243b1..7afad5c21 100644 --- a/pkg/serving/config_changes.go +++ b/pkg/serving/config_changes.go @@ -337,6 +337,18 @@ func UpdateContainerPort(template *servingv1.RevisionTemplateSpec, port int32) e return nil } +// UpdateRunAsUser updates container with a given user id +func UpdateUser(template *servingv1.RevisionTemplateSpec, user int64) error { + container, err := ContainerOfRevisionTemplate(template) + if err != nil { + return err + } + container.SecurityContext = &corev1.SecurityContext{ + RunAsUser: &user, + } + return nil +} + // UpdateResources updates resources as requested func UpdateResources(template *servingv1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error { container, err := ContainerOfRevisionTemplate(template) diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go index e43c27d0a..f17fd170f 100644 --- a/pkg/serving/config_changes_test.go +++ b/pkg/serving/config_changes_test.go @@ -300,6 +300,10 @@ func checkPortUpdate(t *testing.T, template *servingv1.RevisionTemplateSpec, por } } +func checkUserUpdate(t *testing.T, template *servingv1.RevisionTemplateSpec, user *int64) { + assert.DeepEqual(t, template.Spec.Containers[0].SecurityContext.RunAsUser, user) +} + func TestUpdateEnvVarsBoth(t *testing.T) { template, container := getRevisionTemplate() container.Env = []corev1.EnvVar{ @@ -649,6 +653,20 @@ func TestGenerateVolumeName(t *testing.T) { } } +func TestUpdateUser(t *testing.T) { + template, _ := getRevisionTemplate() + err := UpdateUser(template, int64(1001)) + assert.NilError(t, err) + + checkUserUpdate(t, template, ptr.Int64(int64(1001))) + + template.Spec.Containers[0].SecurityContext.RunAsUser = ptr.Int64(int64(1002)) + err = UpdateUser(template, int64(1002)) + assert.NilError(t, err) + + checkUserUpdate(t, template, ptr.Int64(int64(1002))) +} + // // =========================================================================================================