From ac1d5fdf3753cbe2c78a6b93e842b8aa094411a6 Mon Sep 17 00:00:00 2001 From: Shingo Omura Date: Thu, 13 Oct 2022 17:04:30 +0900 Subject: [PATCH] Improve the description of PodSecurityContext.SupplementalGroups (including cri-api) so that it explicitly describe group information defined in the container image will be kept. This also adds e2e test case of SupplementalGroups with pre-defined groups in the container image to make the behaivier clearer. --- api/openapi-spec/swagger.json | 2 +- api/openapi-spec/v3/api__v1_openapi.json | 2 +- .../v3/apis__apps__v1_openapi.json | 2 +- .../v3/apis__batch__v1_openapi.json | 2 +- pkg/apis/core/types.go | 7 +++-- pkg/generated/openapi/zz_generated.openapi.go | 2 +- .../src/k8s.io/api/core/v1/generated.proto | 7 +++-- staging/src/k8s.io/api/core/v1/types.go | 7 +++-- .../core/v1/types_swagger_doc_generated.go | 2 +- .../cri-api/pkg/apis/runtime/v1/api.pb.go | 12 +++++-- .../cri-api/pkg/apis/runtime/v1/api.proto | 12 +++++-- test/e2e/node/security_context.go | 31 +++++++++++++++++++ 12 files changed, 72 insertions(+), 16 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index c219ecc41c8f6..1bf14995722be 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -7283,7 +7283,7 @@ "description": "The seccomp options to use by the containers in this pod. Note that this field cannot be set when spec.os.name is windows." }, "supplementalGroups": { - "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", "items": { "format": "int64", "type": "integer" diff --git a/api/openapi-spec/v3/api__v1_openapi.json b/api/openapi-spec/v3/api__v1_openapi.json index 854e3d77d95dd..ba123a508ffe9 100644 --- a/api/openapi-spec/v3/api__v1_openapi.json +++ b/api/openapi-spec/v3/api__v1_openapi.json @@ -4912,7 +4912,7 @@ "description": "The seccomp options to use by the containers in this pod. Note that this field cannot be set when spec.os.name is windows." }, "supplementalGroups": { - "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", "items": { "default": 0, "format": "int64", diff --git a/api/openapi-spec/v3/apis__apps__v1_openapi.json b/api/openapi-spec/v3/apis__apps__v1_openapi.json index c840a58845153..f6ab1d28d7cd1 100644 --- a/api/openapi-spec/v3/apis__apps__v1_openapi.json +++ b/api/openapi-spec/v3/apis__apps__v1_openapi.json @@ -3334,7 +3334,7 @@ "description": "The seccomp options to use by the containers in this pod. Note that this field cannot be set when spec.os.name is windows." }, "supplementalGroups": { - "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", "items": { "default": 0, "format": "int64", diff --git a/api/openapi-spec/v3/apis__batch__v1_openapi.json b/api/openapi-spec/v3/apis__batch__v1_openapi.json index 37c0c2cd5a94d..711302e911f7d 100644 --- a/api/openapi-spec/v3/apis__batch__v1_openapi.json +++ b/api/openapi-spec/v3/apis__batch__v1_openapi.json @@ -2528,7 +2528,7 @@ "description": "The seccomp options to use by the containers in this pod. Note that this field cannot be set when spec.os.name is windows." }, "supplementalGroups": { - "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + "description": "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", "items": { "default": 0, "format": "int64", diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index ec105a41f44a1..9452aba82db49 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3131,8 +3131,11 @@ type PodSecurityContext struct { // +optional RunAsNonRoot *bool // A list of groups applied to the first process run in each container, in addition - // to the container's primary GID. If unspecified, no groups will be added to - // any container. + // to the container's primary GID, the fsGroup (if specified), and group memberships + // defined in the container image for the uid of the container process. If unspecified, + // no additional groups are added to any container. Note that group memberships + // defined in the container image for the uid of the container process are still effective, + // even if they are not included in this list. // Note that this field cannot be set when spec.os.name is windows. // +optional SupplementalGroups []int64 diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index fcba2fb375c87..55e1785ad3df9 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -22388,7 +22388,7 @@ func schema_k8sio_api_core_v1_PodSecurityContext(ref common.ReferenceCallback) c }, "supplementalGroups": { SchemaProps: spec.SchemaProps{ - Description: "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + Description: "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", Type: []string{"array"}, Items: &spec.SchemaOrArray{ Schema: &spec.Schema{ diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index c43dbc3bacebe..0794a698e9baa 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -3401,8 +3401,11 @@ message PodSecurityContext { optional bool runAsNonRoot = 3; // A list of groups applied to the first process run in each container, in addition - // to the container's primary GID. If unspecified, no groups will be added to - // any container. + // to the container's primary GID, the fsGroup (if specified), and group memberships + // defined in the container image for the uid of the container process. If unspecified, + // no additional groups are added to any container. Note that group memberships + // defined in the container image for the uid of the container process are still effective, + // even if they are not included in this list. // Note that this field cannot be set when spec.os.name is windows. // +optional repeated int64 supplementalGroups = 4; diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index adfad114e4a47..fa6d0213660d9 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3561,8 +3561,11 @@ type PodSecurityContext struct { // +optional RunAsNonRoot *bool `json:"runAsNonRoot,omitempty" protobuf:"varint,3,opt,name=runAsNonRoot"` // A list of groups applied to the first process run in each container, in addition - // to the container's primary GID. If unspecified, no groups will be added to - // any container. + // to the container's primary GID, the fsGroup (if specified), and group memberships + // defined in the container image for the uid of the container process. If unspecified, + // no additional groups are added to any container. Note that group memberships + // defined in the container image for the uid of the container process are still effective, + // even if they are not included in this list. // Note that this field cannot be set when spec.os.name is windows. // +optional SupplementalGroups []int64 `json:"supplementalGroups,omitempty" protobuf:"varint,4,rep,name=supplementalGroups"` diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index 4a2394a524656..90f2d74ea4b2e 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -1613,7 +1613,7 @@ var map_PodSecurityContext = map[string]string{ "runAsUser": "The UID to run the entrypoint of the container process. Defaults to user specified in image metadata if unspecified. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence for that container. Note that this field cannot be set when spec.os.name is windows.", "runAsGroup": "The GID to run the entrypoint of the container process. Uses runtime default if unset. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence for that container. Note that this field cannot be set when spec.os.name is windows.", "runAsNonRoot": "Indicates that the container must run as a non-root user. If true, the Kubelet will validate the image at runtime to ensure that it does not run as UID 0 (root) and fail to start the container if it does. If unset or false, no such validation will be performed. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence.", - "supplementalGroups": "A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container. Note that this field cannot be set when spec.os.name is windows.", + "supplementalGroups": "A list of groups applied to the first process run in each container, in addition to the container's primary GID, the fsGroup (if specified), and group memberships defined in the container image for the uid of the container process. If unspecified, no additional groups are added to any container. Note that group memberships defined in the container image for the uid of the container process are still effective, even if they are not included in this list. Note that this field cannot be set when spec.os.name is windows.", "fsGroup": "A special supplemental group that applies to all containers in a pod. Some volume types allow the Kubelet to change the ownership of that volume to be owned by the pod:\n\n1. The owning GID will be the FSGroup 2. The setgid bit is set (new files created in the volume will be owned by FSGroup) 3. The permission bits are OR'd with rw-rw ", "sysctls": "Sysctls hold a list of namespaced sysctls used for the pod. Pods with unsupported sysctls (by the container runtime) might fail to launch. Note that this field cannot be set when spec.os.name is windows.", "fsGroupChangePolicy": "fsGroupChangePolicy defines behavior of changing ownership and permission of the volume before being exposed inside Pod. This field will only apply to volume types which support fsGroup based ownership(and permissions). It will have no effect on ephemeral volume types such as: secret, configmaps and emptydir. Valid values are \"OnRootMismatch\" and \"Always\". If not specified, \"Always\" is used. Note that this field cannot be set when spec.os.name is windows.", diff --git a/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.pb.go b/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.pb.go index ca7f142290b2c..388bcaf09cb2f 100644 --- a/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.pb.go +++ b/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.pb.go @@ -917,7 +917,11 @@ type LinuxSandboxSecurityContext struct { // If set, the root filesystem of the sandbox is read-only. ReadonlyRootfs bool `protobuf:"varint,4,opt,name=readonly_rootfs,json=readonlyRootfs,proto3" json:"readonly_rootfs,omitempty"` // List of groups applied to the first process run in the sandbox, in - // addition to the sandbox's primary GID. + // addition to the sandbox's primary GID, and group memberships defined + // in the container image for the sandbox's primary UID of the container process. + // If the list is empty, no additional groups are added to any container. + // Note that group memberships defined in the container image for the sandbox's primary UID + // of the container process are still effective, even if they are not included in this list. SupplementalGroups []int64 `protobuf:"varint,5,rep,packed,name=supplemental_groups,json=supplementalGroups,proto3" json:"supplemental_groups,omitempty"` // Indicates whether the sandbox will be asked to run a privileged // container. If a privileged container is to be executed within it, this @@ -3624,7 +3628,11 @@ type LinuxContainerSecurityContext struct { // If set, the root filesystem of the container is read-only. ReadonlyRootfs bool `protobuf:"varint,7,opt,name=readonly_rootfs,json=readonlyRootfs,proto3" json:"readonly_rootfs,omitempty"` // List of groups applied to the first process run in the container, in - // addition to the container's primary GID. + // addition to the container's primary GID, and group memberships defined + // in the container image for the container's primary UID of the container process. + // If the list is empty, no additional groups are added to any container. + // Note that group memberships defined in the container image for the container's primary UID + // of the container process are still effective, even if they are not included in this list. SupplementalGroups []int64 `protobuf:"varint,8,rep,packed,name=supplemental_groups,json=supplementalGroups,proto3" json:"supplemental_groups,omitempty"` // no_new_privs defines if the flag for no_new_privs should be set on the // container. diff --git a/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto b/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto index 9feb4b245f420..50a03f5ee1178 100644 --- a/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto +++ b/staging/src/k8s.io/cri-api/pkg/apis/runtime/v1/api.proto @@ -315,7 +315,11 @@ message LinuxSandboxSecurityContext { // If set, the root filesystem of the sandbox is read-only. bool readonly_rootfs = 4; // List of groups applied to the first process run in the sandbox, in - // addition to the sandbox's primary GID. + // addition to the sandbox's primary GID, and group memberships defined + // in the container image for the sandbox's primary UID of the container process. + // If the list is empty, no additional groups are added to any container. + // Note that group memberships defined in the container image for the sandbox's primary UID + // of the container process are still effective, even if they are not included in this list. repeated int64 supplemental_groups = 5; // Indicates whether the sandbox will be asked to run a privileged // container. If a privileged container is to be executed within it, this @@ -819,7 +823,11 @@ message LinuxContainerSecurityContext { // If set, the root filesystem of the container is read-only. bool readonly_rootfs = 7; // List of groups applied to the first process run in the container, in - // addition to the container's primary GID. + // addition to the container's primary GID, and group memberships defined + // in the container image for the container's primary UID of the container process. + // If the list is empty, no additional groups are added to any container. + // Note that group memberships defined in the container image for the container's primary UID + // of the container process are still effective, even if they are not included in this list. repeated int64 supplemental_groups = 8; // no_new_privs defines if the flag for no_new_privs should be set on the // container. diff --git a/test/e2e/node/security_context.go b/test/e2e/node/security_context.go index b18b003ab4586..f92950224864f 100644 --- a/test/e2e/node/security_context.go +++ b/test/e2e/node/security_context.go @@ -77,6 +77,37 @@ var _ = SIGDescribe("Security Context", func() { e2eoutput.TestContainerOutput(f, "pod.Spec.SecurityContext.SupplementalGroups", pod, 0, groups) }) + ginkgo.When("if the container's primary UID belongs to some groups in the image [LinuxOnly]", func() { + ginkgo.It("should add pod.Spec.SecurityContext.SupplementalGroups to them [LinuxOnly] in resultant supplementary groups for the container processes", func() { + uidInImage := int64(1000) + gidDefinedInImage := int64(50000) + supplementalGroup := int64(60000) + agnhost := imageutils.GetConfig(imageutils.Agnhost) + (&agnhost).SetVersion("2.43") + pod := scTestPod(false, false) + pod.Spec.Containers[0].Image = agnhost.GetE2EImage() + pod.Spec.Containers[0].Command = []string{"id", "-G"} + pod.Spec.SecurityContext.SupplementalGroups = []int64{int64(supplementalGroup)} + pod.Spec.SecurityContext.RunAsUser = &uidInImage + + // In specified image(agnhost E2E image), + // - user-defined-in-image(uid=1000) is defined + // - user-defined-in-image belongs to group-defined-in-image(gid=50000) + // thus, resultant supplementary group of the container processes should be + // - 1000: self + // - 50000: pre-defined groups define in the container image of self(uid=1000) + // - 60000: SupplementalGroups + // $ id -G + // 1000 50000 60000 + e2eoutput.TestContainerOutput( + f, + "pod.Spec.SecurityContext.SupplementalGroups with pre-defined-group in the image", + pod, 0, + []string{fmt.Sprintf("%d %d %d", uidInImage, gidDefinedInImage, supplementalGroup)}, + ) + }) + }) + ginkgo.It("should support pod.Spec.SecurityContext.RunAsUser [LinuxOnly]", func() { pod := scTestPod(false, false) userID := int64(1001)