resolves review

knative · Dec 5, 2024 · 55e26ad · 55e26ad
1 parent 4809dca
commit 55e26ad
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 11 deletions.
diff --git a/config/core/300-resources/configuration.yaml b/config/core/300-resources/configuration.yaml
@@ -124,9 +124,6 @@ spec:
                         automountServiceAccountToken:
                           description: AutomountServiceAccountToken indicates whether a service account token should be automatically mounted.
                           type: boolean
-                        Privileged:
-                          description: Indicates whether to run container in privileged mode.
-                          type: boolean
                         containerConcurrency:
                           description: |-
                             ContainerConcurrency specifies the maximum allowed in-flight (concurrent)
@@ -737,6 +734,13 @@ spec:
                                           description: Capability represent POSIX capabilities type
                                           type: string
                                         x-kubernetes-list-type: atomic
+                                  privileged:
+                                    description: |-
+                                      Run container in privileged mode.
+                                      Processes in privileged containers are essentially equivalent to root on the host.
+                                      Defaults to false.
+                                      Note that this field cannot be set when spec.os.name is windows.
+                                    type: boolean
                                   readOnlyRootFilesystem:
                                     description: |-
                                       Whether this container has a read-only root filesystem.

diff --git a/config/core/300-resources/revision.yaml b/config/core/300-resources/revision.yaml
@@ -101,9 +101,6 @@ spec:
                 automountServiceAccountToken:
                   description: AutomountServiceAccountToken indicates whether a service account token should be automatically mounted.
                   type: boolean
-                Privileged:
-                  description: Indicates whether to run container in privileged mode.
-                  type: boolean
                 containerConcurrency:
                   description: |-
                     ContainerConcurrency specifies the maximum allowed in-flight (concurrent)
@@ -714,6 +711,13 @@ spec:
                                   description: Capability represent POSIX capabilities type
                                   type: string
                                 x-kubernetes-list-type: atomic
+                          privileged:
+                            description: |-
+                              Run container in privileged mode.
+                              Processes in privileged containers are essentially equivalent to root on the host.
+                              Defaults to false.
+                              Note that this field cannot be set when spec.os.name is windows.
+                            type: boolean
                           readOnlyRootFilesystem:
                             description: |-
                               Whether this container has a read-only root filesystem.

diff --git a/config/core/300-resources/service.yaml b/config/core/300-resources/service.yaml
@@ -144,9 +144,6 @@ spec:
                         automountServiceAccountToken:
                           description: AutomountServiceAccountToken indicates whether a service account token should be automatically mounted.
                           type: boolean
-                        Privileged:
-                          description: Indicates whether to run container in privileged mode.
-                          type: boolean          
                         containerConcurrency:
                           description: |-
                             ContainerConcurrency specifies the maximum allowed in-flight (concurrent)
@@ -757,6 +754,13 @@ spec:
                                           description: Capability represent POSIX capabilities type
                                           type: string
                                         x-kubernetes-list-type: atomic
+                                  privileged:
+                                    description: |-
+                                      Run container in privileged mode.
+                                      Processes in privileged containers are essentially equivalent to root on the host.
+                                      Defaults to false.
+                                      Note that this field cannot be set when spec.os.name is windows.
+                                    type: boolean
                                   readOnlyRootFilesystem:
                                     description: |-
                                       Whether this container has a read-only root filesystem.

diff --git a/hack/schemapatch-config.yaml b/hack/schemapatch-config.yaml
@@ -58,7 +58,6 @@ k8s.io/api/core/v1.PodSpec:
     - ImagePullSecrets
     - EnableServiceLinks
     - AutomountServiceAccountToken
-    - Privileged
     # Properties behind feature flags
     - Affinity
     - DNSConfig
@@ -330,6 +329,7 @@ k8s.io/api/core/v1.SecurityContext:
     - RunAsNonRoot
     - RunAsUser
     - SeccompProfile
+    - Privileged
 k8s.io/api/core/v1.Capabilities:
   fieldMask:
     - Add

diff --git a/pkg/apis/serving/fieldmask.go b/pkg/apis/serving/fieldmask.go
@@ -710,7 +710,7 @@ func SecurityContextMask(ctx context.Context, in *corev1.SecurityContext) *corev
 	// SeccompProfile defaults to "unconstrained", but the safe values are
 	// "RuntimeDefault" or "Localhost" (with localhost path set)
 	out.SeccompProfile = in.SeccompProfile
-	// Allow setting Privileged to only false
+	// Only allow setting Privileged to false
 	if in.Privileged != nil && !*in.Privileged {
 		out.Privileged = in.Privileged
 	}

diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go
@@ -2703,6 +2703,15 @@ func getCommonContainerValidationTestCases() []containerValidationTestCase {
 				},
 			},
 			want: apis.ErrDisallowedFields("securityContext.privileged"),
+			}, {
+				name: "allowed setting security context field Privileged to false",
+				c: corev1.Container{
+					Image: "foo",
+					SecurityContext: &corev1.SecurityContext{
+						Privileged: ptr.Bool(false),
+					},
+				},
+				want:    nil,
 		}, {
 			name: "too large uid",
 			c: corev1.Container{