From 4809dcad4430346ae3aff3092f7df5e58b7ca8af Mon Sep 17 00:00:00 2001 From: KapilSareen Date: Mon, 2 Dec 2024 16:32:10 +0530 Subject: [PATCH 1/3] makes securityContext.Privileged configurable --- config/core/300-resources/configuration.yaml | 3 +++ config/core/300-resources/revision.yaml | 3 +++ config/core/300-resources/service.yaml | 3 +++ hack/schemapatch-config.yaml | 1 + pkg/apis/serving/fieldmask.go | 6 ++++-- 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/config/core/300-resources/configuration.yaml b/config/core/300-resources/configuration.yaml index 2c43aaa5753f..a42c2d29dfc1 100644 --- a/config/core/300-resources/configuration.yaml +++ b/config/core/300-resources/configuration.yaml @@ -124,6 +124,9 @@ 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) diff --git a/config/core/300-resources/revision.yaml b/config/core/300-resources/revision.yaml index b4651d9bfe53..746e8fa7dac4 100644 --- a/config/core/300-resources/revision.yaml +++ b/config/core/300-resources/revision.yaml @@ -101,6 +101,9 @@ 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) diff --git a/config/core/300-resources/service.yaml b/config/core/300-resources/service.yaml index bd322152cee9..4371cf4c53e5 100644 --- a/config/core/300-resources/service.yaml +++ b/config/core/300-resources/service.yaml @@ -144,6 +144,9 @@ 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) diff --git a/hack/schemapatch-config.yaml b/hack/schemapatch-config.yaml index 7656efc9eb0b..39fbc371a56f 100644 --- a/hack/schemapatch-config.yaml +++ b/hack/schemapatch-config.yaml @@ -58,6 +58,7 @@ k8s.io/api/core/v1.PodSpec: - ImagePullSecrets - EnableServiceLinks - AutomountServiceAccountToken + - Privileged # Properties behind feature flags - Affinity - DNSConfig diff --git a/pkg/apis/serving/fieldmask.go b/pkg/apis/serving/fieldmask.go index faff5dba9690..c34af98497aa 100644 --- a/pkg/apis/serving/fieldmask.go +++ b/pkg/apis/serving/fieldmask.go @@ -710,10 +710,12 @@ 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 + if in.Privileged != nil && !*in.Privileged { + out.Privileged = in.Privileged + } // Disallowed // This list is unnecessary, but added here for clarity - out.Privileged = nil out.SELinuxOptions = nil out.ProcMount = nil From 55e26ade54d058433f27b8fcb33bea5d47d8ab8c Mon Sep 17 00:00:00 2001 From: KapilSareen Date: Thu, 5 Dec 2024 12:58:56 +0530 Subject: [PATCH 2/3] resolves review --- config/core/300-resources/configuration.yaml | 10 +++++++--- config/core/300-resources/revision.yaml | 10 +++++++--- config/core/300-resources/service.yaml | 10 +++++++--- hack/schemapatch-config.yaml | 2 +- pkg/apis/serving/fieldmask.go | 2 +- pkg/apis/serving/k8s_validation_test.go | 9 +++++++++ 6 files changed, 32 insertions(+), 11 deletions(-) diff --git a/config/core/300-resources/configuration.yaml b/config/core/300-resources/configuration.yaml index a42c2d29dfc1..65bf809eb612 100644 --- 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 index 746e8fa7dac4..71bee879eb96 100644 --- 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 index 4371cf4c53e5..69dbe087012d 100644 --- 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 index 39fbc371a56f..abd6846a04e4 100644 --- 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 index c34af98497aa..ab859740d913 100644 --- 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 index 892e381f8901..cc45756275c8 100644 --- 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{ From 0e8ca1806e949e2fd1b2d98817113f974e48b2fa Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Mon, 30 Dec 2024 10:49:43 -0500 Subject: [PATCH 3/3] fix go formatting --- pkg/apis/serving/k8s_validation_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index cc45756275c8..cc12b8457ab6 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -2703,15 +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), - }, + }, { + name: "allowed setting security context field Privileged to false", + c: corev1.Container{ + Image: "foo", + SecurityContext: &corev1.SecurityContext{ + Privileged: ptr.Bool(false), }, - want: nil, + }, + want: nil, }, { name: "too large uid", c: corev1.Container{