From af1f6a230b37013610495ea4b4ae11409b3ceac9 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Tue, 10 Jan 2023 10:53:50 +0100 Subject: [PATCH] Make seccomp annotations non-functional This cleanup has been planned to finish the corresponding KEP: https://github.com/kubernetes/kubernetes/issues/91286 As follow-up on the partly removal of the seccomp annotations in https://github.com/kubernetes/kubernetes/pull/109819, we now drop the version skew handling completely, but still warn as well as keep the validation in place if both (annotation and field) are set. The Pod Security Admission code has been already changed in https://github.com/kubernetes/kubernetes/pull/114846. Signed-off-by: Sascha Grunert --- pkg/api/pod/util.go | 25 -- pkg/api/pod/warnings.go | 4 +- pkg/api/pod/warnings_test.go | 4 +- pkg/registry/core/pod/strategy.go | 85 ------- pkg/registry/core/pod/strategy_test.go | 312 ------------------------- 5 files changed, 4 insertions(+), 426 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index dfd5229146f..9cc7aeb1156 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -785,31 +785,6 @@ func schedulingGatesInUse(podSpec *api.PodSpec) bool { return len(podSpec.SchedulingGates) != 0 } -// SeccompAnnotationForField takes a pod seccomp profile field and returns the -// converted annotation value -func SeccompAnnotationForField(field *api.SeccompProfile) string { - // If only seccomp fields are specified, add the corresponding annotations. - // This ensures that the fields are enforced even if the node version - // trails the API version - switch field.Type { - case api.SeccompProfileTypeUnconfined: - return v1.SeccompProfileNameUnconfined - - case api.SeccompProfileTypeRuntimeDefault: - return v1.SeccompProfileRuntimeDefault - - case api.SeccompProfileTypeLocalhost: - if field.LocalhostProfile != nil { - return v1.SeccompLocalhostProfileNamePrefix + *field.LocalhostProfile - } - } - - // we can only reach this code path if the LocalhostProfile is nil but the - // provided field type is SeccompProfileTypeLocalhost or if an unrecognized - // type is specified - return "" -} - func hasInvalidLabelValueInAffinitySelector(spec *api.PodSpec) bool { if spec.Affinity != nil { if spec.Affinity.PodAffinity != nil { diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index f2e5d8fa01d..8a07caee072 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -210,7 +210,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta // use of pod seccomp annotation without accompanying field if podSpec.SecurityContext == nil || podSpec.SecurityContext.SeccompProfile == nil { if _, exists := meta.Annotations[api.SeccompPodAnnotationKey]; exists { - warnings = append(warnings, fmt.Sprintf(`%s: deprecated since v1.19, non-functional in a future release; use the "seccompProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(api.SeccompPodAnnotationKey))) + warnings = append(warnings, fmt.Sprintf(`%s: non-functional in v1.27+; use the "seccompProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(api.SeccompPodAnnotationKey))) } } @@ -218,7 +218,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta // use of container seccomp annotation without accompanying field if c.SecurityContext == nil || c.SecurityContext.SeccompProfile == nil { if _, exists := meta.Annotations[api.SeccompContainerAnnotationKeyPrefix+c.Name]; exists { - warnings = append(warnings, fmt.Sprintf(`%s: deprecated since v1.19, non-functional in a future release; use the "seccompProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(api.SeccompContainerAnnotationKeyPrefix+c.Name))) + warnings = append(warnings, fmt.Sprintf(`%s: non-functional in v1.27+; use the "seccompProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(api.SeccompContainerAnnotationKeyPrefix+c.Name))) } } diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 6bab0367c69..71b1e06eda8 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -432,8 +432,8 @@ func TestWarnings(t *testing.T) { }, expected: []string{ `metadata.annotations[scheduler.alpha.kubernetes.io/critical-pod]: non-functional in v1.16+; use the "priorityClassName" field instead`, - `metadata.annotations[seccomp.security.alpha.kubernetes.io/pod]: deprecated since v1.19, non-functional in a future release; use the "seccompProfile" field instead`, - `metadata.annotations[container.seccomp.security.alpha.kubernetes.io/foo]: deprecated since v1.19, non-functional in a future release; use the "seccompProfile" field instead`, + `metadata.annotations[seccomp.security.alpha.kubernetes.io/pod]: non-functional in v1.27+; use the "seccompProfile" field instead`, + `metadata.annotations[container.seccomp.security.alpha.kubernetes.io/foo]: non-functional in v1.27+; use the "seccompProfile" field instead`, `metadata.annotations[security.alpha.kubernetes.io/sysctls]: non-functional in v1.11+; use the "sysctls" field instead`, `metadata.annotations[security.alpha.kubernetes.io/unsafe-sysctls]: non-functional in v1.11+; use the "sysctls" field instead`, }, diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 4f48457530f..200189994f1 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -26,7 +26,6 @@ import ( "strings" "time" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -89,7 +88,6 @@ func (podStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { podutil.DropDisabledPodFields(pod, nil) - applySeccompVersionSkew(pod) applyWaitingForSchedulingGatesCondition(pod) } @@ -674,86 +672,3 @@ func applyWaitingForSchedulingGatesCondition(pod *api.Pod) { Message: "Scheduling is blocked due to non-empty scheduling gates", }) } - -// applySeccompVersionSkew implements the version skew behavior described in: -// https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/135-seccomp#version-skew-strategy -// Note that we dropped copying the field to annotation synchronization in -// v1.25 with the functional removal of the annotations. -func applySeccompVersionSkew(pod *api.Pod) { - // get possible annotation and field - annotation, hasAnnotation := pod.Annotations[v1.SeccompPodAnnotationKey] - hasField := false - - if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SeccompProfile != nil { - hasField = true - } - - // sync field and annotation - if hasAnnotation && !hasField { - newField := seccompFieldForAnnotation(annotation) - - if newField != nil { - if pod.Spec.SecurityContext == nil { - pod.Spec.SecurityContext = &api.PodSecurityContext{} - } - pod.Spec.SecurityContext.SeccompProfile = newField - } - } - - // Handle the containers of the pod - podutil.VisitContainers(&pod.Spec, podutil.AllFeatureEnabledContainers(), - func(ctr *api.Container, _ podutil.ContainerType) bool { - // get possible annotation and field - key := api.SeccompContainerAnnotationKeyPrefix + ctr.Name - annotation, hasAnnotation := pod.Annotations[key] - - hasField := false - if ctr.SecurityContext != nil && ctr.SecurityContext.SeccompProfile != nil { - hasField = true - } - - // sync field and annotation - if hasAnnotation && !hasField { - newField := seccompFieldForAnnotation(annotation) - - if newField != nil { - if ctr.SecurityContext == nil { - ctr.SecurityContext = &api.SecurityContext{} - } - ctr.SecurityContext.SeccompProfile = newField - } - } - - return true - }) -} - -// seccompFieldForAnnotation takes a pod annotation and returns the converted -// seccomp profile field. -func seccompFieldForAnnotation(annotation string) *api.SeccompProfile { - // If only seccomp annotations are specified, copy the values into the - // corresponding fields. This ensures that existing applications continue - // to enforce seccomp, and prevents the kubelet from needing to resolve - // annotations & fields. - if annotation == v1.SeccompProfileNameUnconfined { - return &api.SeccompProfile{Type: api.SeccompProfileTypeUnconfined} - } - - if annotation == api.SeccompProfileRuntimeDefault || annotation == api.DeprecatedSeccompProfileDockerDefault { - return &api.SeccompProfile{Type: api.SeccompProfileTypeRuntimeDefault} - } - - if strings.HasPrefix(annotation, v1.SeccompLocalhostProfileNamePrefix) { - localhostProfile := strings.TrimPrefix(annotation, v1.SeccompLocalhostProfileNamePrefix) - if localhostProfile != "" { - return &api.SeccompProfile{ - Type: api.SeccompProfileTypeLocalhost, - LocalhostProfile: &localhostProfile, - } - } - } - - // we can only reach this code path if the localhostProfile name has a zero - // length or if the annotation has an unrecognized value - return nil -} diff --git a/pkg/registry/core/pod/strategy_test.go b/pkg/registry/core/pod/strategy_test.go index dc1fc9a594b..3d471baea66 100644 --- a/pkg/registry/core/pod/strategy_test.go +++ b/pkg/registry/core/pod/strategy_test.go @@ -26,8 +26,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - "github.com/stretchr/testify/require" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -794,316 +792,6 @@ func TestPodIndexFunc(t *testing.T) { } } -func TestApplySeccompVersionSkew(t *testing.T) { - const containerName = "container" - testProfile := "test" - - for _, test := range []struct { - description string - pod *api.Pod - validation func(*testing.T, *api.Pod) - }{ - { - description: "Security context nil", - pod: &api.Pod{}, - validation: func(t *testing.T, pod *api.Pod) { - require.NotNil(t, pod) - }, - }, - { - description: "Security context not nil", - pod: &api.Pod{ - Spec: api.PodSpec{SecurityContext: &api.PodSecurityContext{}}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.NotNil(t, pod) - }, - }, - { - description: "Field set and no annotation present", - pod: &api.Pod{ - Spec: api.PodSpec{ - SecurityContext: &api.PodSecurityContext{ - SeccompProfile: &api.SeccompProfile{ - Type: api.SeccompProfileTypeUnconfined, - }, - }, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Len(t, pod.Annotations, 0) - }, - }, - { - description: "Annotation 'unconfined' and no field present", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompPodAnnotationKey: v1.SeccompProfileNameUnconfined, - }, - }, - Spec: api.PodSpec{}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeUnconfined, pod.Spec.SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'runtime/default' and no field present", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompPodAnnotationKey: v1.SeccompProfileRuntimeDefault, - }, - }, - Spec: api.PodSpec{SecurityContext: &api.PodSecurityContext{}}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeRuntimeDefault, pod.Spec.SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'docker/default' and no field present", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompPodAnnotationKey: v1.DeprecatedSeccompProfileDockerDefault, - }, - }, - Spec: api.PodSpec{SecurityContext: &api.PodSecurityContext{}}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeRuntimeDefault, pod.Spec.SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'localhost/test' and no field present", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompPodAnnotationKey: v1.SeccompLocalhostProfileNamePrefix + testProfile, - }, - }, - Spec: api.PodSpec{SecurityContext: &api.PodSecurityContext{}}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeLocalhost, pod.Spec.SecurityContext.SeccompProfile.Type) - require.Equal(t, testProfile, *pod.Spec.SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'localhost/' has zero length", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompPodAnnotationKey: v1.SeccompLocalhostProfileNamePrefix, - }, - }, - Spec: api.PodSpec{SecurityContext: &api.PodSecurityContext{}}, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Nil(t, pod.Spec.SecurityContext.SeccompProfile) - }, - }, - { - description: "Security context nil (container)", - pod: &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{{}}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.NotNil(t, pod) - }, - }, - { - description: "Security context not nil (container)", - pod: &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{{ - SecurityContext: &api.SecurityContext{}, - }}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.NotNil(t, pod) - }, - }, - { - description: "Field set and no annotation present (container)", - pod: &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: containerName, - SecurityContext: &api.SecurityContext{ - SeccompProfile: &api.SeccompProfile{ - Type: api.SeccompProfileTypeUnconfined, - }, - }, - }, - }, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Len(t, pod.Annotations, 0) - }, - }, - { - description: "Multiple containers with fields (container)", - pod: &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: containerName + "1", - SecurityContext: &api.SecurityContext{ - SeccompProfile: &api.SeccompProfile{ - Type: api.SeccompProfileTypeUnconfined, - }, - }, - }, - { - Name: containerName + "2", - }, - { - Name: containerName + "3", - SecurityContext: &api.SecurityContext{ - SeccompProfile: &api.SeccompProfile{ - Type: api.SeccompProfileTypeRuntimeDefault, - }, - }, - }, - }, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Len(t, pod.Annotations, 0) - }, - }, - { - description: "Annotation 'unconfined' and no field present (container)", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompContainerAnnotationKeyPrefix + containerName: v1.SeccompProfileNameUnconfined, - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{{ - Name: containerName, - }}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeUnconfined, pod.Spec.Containers[0].SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.Containers[0].SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'runtime/default' and no field present (container)", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompContainerAnnotationKeyPrefix + containerName: v1.SeccompProfileRuntimeDefault, - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{{ - Name: containerName, - SecurityContext: &api.SecurityContext{}, - }}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeRuntimeDefault, pod.Spec.Containers[0].SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.Containers[0].SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Annotation 'docker/default' and no field present (container)", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompContainerAnnotationKeyPrefix + containerName: v1.DeprecatedSeccompProfileDockerDefault, - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{{ - Name: containerName, - SecurityContext: &api.SecurityContext{}, - }}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeRuntimeDefault, pod.Spec.Containers[0].SecurityContext.SeccompProfile.Type) - require.Nil(t, pod.Spec.Containers[0].SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - { - description: "Multiple containers by annotations (container)", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompContainerAnnotationKeyPrefix + containerName + "1": v1.SeccompLocalhostProfileNamePrefix + testProfile, - v1.SeccompContainerAnnotationKeyPrefix + containerName + "3": v1.SeccompProfileRuntimeDefault, - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - {Name: containerName + "1"}, - {Name: containerName + "2"}, - {Name: containerName + "3"}, - }, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeLocalhost, pod.Spec.Containers[0].SecurityContext.SeccompProfile.Type) - require.Equal(t, testProfile, *pod.Spec.Containers[0].SecurityContext.SeccompProfile.LocalhostProfile) - require.Equal(t, api.SeccompProfileTypeRuntimeDefault, pod.Spec.Containers[2].SecurityContext.SeccompProfile.Type) - }, - }, - { - description: "Annotation 'localhost/test' and no field present (container)", - pod: &api.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1.SeccompContainerAnnotationKeyPrefix + containerName: v1.SeccompLocalhostProfileNamePrefix + testProfile, - }, - }, - Spec: api.PodSpec{ - Containers: []api.Container{{ - Name: containerName, - SecurityContext: &api.SecurityContext{}, - }}, - }, - }, - validation: func(t *testing.T, pod *api.Pod) { - require.Equal(t, api.SeccompProfileTypeLocalhost, pod.Spec.Containers[0].SecurityContext.SeccompProfile.Type) - require.Equal(t, testProfile, *pod.Spec.Containers[0].SecurityContext.SeccompProfile.LocalhostProfile) - }, - }, - } { - output := &api.Pod{ - ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}, - } - for i, ctr := range test.pod.Spec.Containers { - output.Spec.Containers = append(output.Spec.Containers, api.Container{}) - if ctr.SecurityContext != nil && ctr.SecurityContext.SeccompProfile != nil { - output.Spec.Containers[i].SecurityContext = &api.SecurityContext{ - SeccompProfile: &api.SeccompProfile{ - Type: api.SeccompProfileType(ctr.SecurityContext.SeccompProfile.Type), - LocalhostProfile: ctr.SecurityContext.SeccompProfile.LocalhostProfile, - }, - } - } - } - applySeccompVersionSkew(test.pod) - test.validation(t, test.pod) - } -} func newPodWithHugePageValue(resourceName api.ResourceName, value resource.Quantity) *api.Pod { return &api.Pod{