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 c219ecc41c8..1bf14995722 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 854e3d77d95..ba123a508ff 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 c840a588451..f6ab1d28d7c 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 37c0c2cd5a9..711302e911f 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 ec105a41f44..9452aba82db 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 fcba2fb375c..55e1785ad3d 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 c43dbc3bace..0794a698e9b 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 adfad114e4a..fa6d0213660 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 4a2394a5246..90f2d74ea4b 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 ca7f142290b..388bcaf09cb 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 9feb4b245f4..50a03f5ee11 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 b18b003ab45..f9295022486 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)