Merge pull request #113047 from everpeace/improve-supplemental-groups-description

Improve the API  description of `PodSecurityContext.SupplementalGroups` to clarify its unfamiliar behavior

api/openapi-spec/swagger.json

@@ -7296,7 +7296,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"

api/openapi-spec/v3/api__v1_openapi.json

@@ -4926,7 +4926,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",

api/openapi-spec/v3/apis__apps__v1_openapi.json

@@ -3348,7 +3348,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",

api/openapi-spec/v3/apis__batch__v1_openapi.json

@@ -2542,7 +2542,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",

pkg/apis/core/types.go

@@ -3147,8 +3147,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

pkg/generated/openapi/zz_generated.openapi.go

@@ -22411,7 +22411,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{

staging/src/k8s.io/api/core/v1/generated.proto

@@ -3408,8 +3408,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;

staging/src/k8s.io/api/core/v1/types.go

@@ -3586,8 +3586,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"`

staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go

@@ -1622,7 +1622,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.",

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
@@ -3873,7 +3877,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.

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
@@ -860,7 +864,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.

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)