Improve the description of PodSecurityContext.SupplementalGroups (inc…

…luding 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.
kubernetes · Nov 6, 2022 · ac1d5fd · ac1d5fd
1 parent c98aef4
commit ac1d5fd
Show file tree

Hide file tree

Showing 12 changed files with 72 additions and 16 deletions.
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto
diff --git 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
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
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
@@ -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
@@ -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)