Merge pull request #125257 from vinayakankugoyal/armor

KEP-24: Update AppArmor feature gates to GA stage.

pkg/api/pod/util.go

@@ -628,25 +628,6 @@ func dropDisabledFields(
 		podSpec = &api.PodSpec{}
 	}
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmor) && !appArmorAnnotationsInUse(oldPodAnnotations) {
-		for k := range podAnnotations {
-			if strings.HasPrefix(k, api.DeprecatedAppArmorAnnotationKeyPrefix) {
-				delete(podAnnotations, k)
-			}
-		}
-	}
-	if (!utilfeature.DefaultFeatureGate.Enabled(features.AppArmor) || !utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields)) && !appArmorFieldsInUse(oldPodSpec) {
-		if podSpec.SecurityContext != nil {
-			podSpec.SecurityContext.AppArmorProfile = nil
-		}
-		VisitContainers(podSpec, AllContainers, func(c *api.Container, _ ContainerType) bool {
-			if c.SecurityContext != nil {
-				c.SecurityContext.AppArmorProfile = nil
-			}
-			return true
-		})
-	}
-
 	// If the feature is disabled and not in use, drop the hostUsers field.
 	if !utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport) && !hostUsersInUse(oldPodSpec) {
 		// Drop the field in podSpec only if SecurityContext is not nil.

pkg/api/pod/util_test.go

@@ -769,45 +769,36 @@ func TestDropAppArmor(t *testing.T) {
 	}}
 
 	for _, test := range tests {
-		for _, enabled := range []bool{true, false} {
-			for _, fieldsEnabled := range []bool{true, false} {
-				t.Run(fmt.Sprintf("%v/enabled=%v/fields=%v", test.description, enabled, fieldsEnabled), func(t *testing.T) {
-					featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmor, enabled)
-					featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmorFields, fieldsEnabled)
 
-					newPod := test.pod.DeepCopy()
+		t.Run(fmt.Sprintf("%v", test.description), func(t *testing.T) {
+			newPod := test.pod.DeepCopy()
 
-					if hasAnnotations := appArmorAnnotationsInUse(newPod.Annotations); hasAnnotations != test.hasAnnotations {
+			if hasAnnotations := appArmorAnnotationsInUse(newPod.Annotations); hasAnnotations != test.hasAnnotations {
-						t.Errorf("appArmorAnnotationsInUse does not match expectation: %t != %t", hasAnnotations, test.hasAnnotations)
+				t.Errorf("appArmorAnnotationsInUse does not match expectation: %t != %t", hasAnnotations, test.hasAnnotations)
-					}
-					if hasFields := appArmorFieldsInUse(&newPod.Spec); hasFields != test.hasFields {
-						t.Errorf("appArmorFieldsInUse does not match expectation: %t != %t", hasFields, test.hasFields)
-					}
-
-					DropDisabledPodFields(newPod, newPod)
-					require.Equal(t, &test.pod, newPod, "unchanged pod should never be mutated")
-
-					DropDisabledPodFields(newPod, nil)
-
-					if enabled && fieldsEnabled {
-						assert.Equal(t, &test.pod, newPod, "pod should not be mutated when both feature gates are enabled")
-						return
-					}
-
-					expectAnnotations := test.hasAnnotations && enabled
-					assert.Equal(t, expectAnnotations, appArmorAnnotationsInUse(newPod.Annotations), "AppArmor annotations expectation")
-					if expectAnnotations == test.hasAnnotations {
-						assert.Equal(t, test.pod.Annotations, newPod.Annotations, "annotations should not be mutated")
-					}
-
-					expectFields := test.hasFields && enabled && fieldsEnabled
-					assert.Equal(t, expectFields, appArmorFieldsInUse(&newPod.Spec), "AppArmor fields expectation")
-					if expectFields == test.hasFields {
-						assert.Equal(t, &test.pod.Spec, &newPod.Spec, "PodSpec should not be mutated")
-					}
-				})
 			}
-		}
+			if hasFields := appArmorFieldsInUse(&newPod.Spec); hasFields != test.hasFields {
+				t.Errorf("appArmorFieldsInUse does not match expectation: %t != %t", hasFields, test.hasFields)
+			}
+
+			DropDisabledPodFields(newPod, newPod)
+			require.Equal(t, &test.pod, newPod, "unchanged pod should never be mutated")
+
+			DropDisabledPodFields(newPod, nil)
+			assert.Equal(t, &test.pod, newPod, "pod should not be mutated when both feature gates are enabled")
+
+			expectAnnotations := test.hasAnnotations
+			assert.Equal(t, expectAnnotations, appArmorAnnotationsInUse(newPod.Annotations), "AppArmor annotations expectation")
+			if expectAnnotations == test.hasAnnotations {
+				assert.Equal(t, test.pod.Annotations, newPod.Annotations, "annotations should not be mutated")
+			}
+
+			expectFields := test.hasFields
+			assert.Equal(t, expectFields, appArmorFieldsInUse(&newPod.Spec), "AppArmor fields expectation")
+			if expectFields == test.hasFields {
+				assert.Equal(t, &test.pod.Spec, &newPod.Spec, "PodSpec should not be mutated")
+			}
+		})
+
 	}
 }
 

pkg/api/pod/warnings.go

@@ -24,12 +24,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	nodeapi "k8s.io/kubernetes/pkg/api/node"
 	pvcutil "k8s.io/kubernetes/pkg/api/persistentvolumeclaim"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/pods"
-	"k8s.io/kubernetes/pkg/features"
 )
 
 func GetWarningsForPod(ctx context.Context, pod, oldPod *api.Pod) []string {
@@ -225,14 +223,13 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
 		}
 
 		// use of container AppArmor annotation without accompanying field
-		if utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields) {
+
-			isPodTemplate := fieldPath != nil // Pod warnings are emitted through applyAppArmorVersionSkew instead.
+		isPodTemplate := fieldPath != nil // Pod warnings are emitted through applyAppArmorVersionSkew instead.
-			hasAppArmorField := hasPodAppArmorProfile || (c.SecurityContext != nil && c.SecurityContext.AppArmorProfile != nil)
+		hasAppArmorField := hasPodAppArmorProfile || (c.SecurityContext != nil && c.SecurityContext.AppArmorProfile != nil)
-			if isPodTemplate && !hasAppArmorField {
+		if isPodTemplate && !hasAppArmorField {
-				key := api.DeprecatedAppArmorAnnotationKeyPrefix + c.Name
+			key := api.DeprecatedAppArmorAnnotationKeyPrefix + c.Name
-				if _, exists := meta.Annotations[key]; exists {
+			if _, exists := meta.Annotations[key]; exists {
-					warnings = append(warnings, fmt.Sprintf(`%s: deprecated since v1.30; use the "appArmorProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(key)))
+				warnings = append(warnings, fmt.Sprintf(`%s: deprecated since v1.30; use the "appArmorProfile" field instead`, fieldPath.Child("metadata", "annotations").Key(key)))
-				}
 			}
 		}
 

pkg/apis/core/validation/validation.go

@@ -4793,9 +4793,6 @@ func ValidateAppArmorProfileFormat(profile string) error {
 
 // validateAppArmorAnnotationsAndFieldsMatchOnCreate validates that AppArmor fields and annotations are consistent.
 func validateAppArmorAnnotationsAndFieldsMatchOnCreate(objectMeta metav1.ObjectMeta, podSpec *core.PodSpec, specPath *field.Path) field.ErrorList {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields) {
-		return nil
-	}
 	if podSpec.OS != nil && podSpec.OS.Name == core.Windows {
 		// Skip consistency check for windows pods.
 		return nil

pkg/features/kube_features.go

@@ -68,10 +68,12 @@ const (
 
 	// owner: @tallclair
 	// beta: v1.4
+	// GA: v1.31
 	AppArmor featuregate.Feature = "AppArmor"
 
 	// owner: @tallclair
 	// beta: v1.30
+	// GA: v1.31
 	AppArmorFields featuregate.Feature = "AppArmorFields"
 
 	// owner: @liggitt
@@ -995,9 +997,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	AnyVolumeDataSource: {Default: true, PreRelease: featuregate.Beta}, // on by default in 1.24
 
-	AppArmor: {Default: true, PreRelease: featuregate.Beta},
+	AppArmor: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.33
 
-	AppArmorFields: {Default: true, PreRelease: featuregate.Beta},
+	AppArmorFields: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.33
 
 	AuthorizeNodeWithSelectors: {Default: false, PreRelease: featuregate.Alpha},
 

pkg/registry/core/pod/strategy.go

@@ -765,10 +765,6 @@ func applySchedulingGatedCondition(pod *api.Pod) {
 // applyAppArmorVersionSkew implements the version skew behavior described in:
 // https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/24-apparmor#version-skew-strategy
 func applyAppArmorVersionSkew(ctx context.Context, pod *api.Pod) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields) {
-		return
-	}
-
 	if pod.Spec.OS != nil && pod.Spec.OS.Name == api.Windows {
 		return
 	}

pkg/security/apparmor/helpers.go

@@ -20,9 +20,7 @@ import (
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
-	"k8s.io/kubernetes/pkg/features"
 )
 
 // Checks whether app armor is required for the pod to run. AppArmor is considered required if any
@@ -54,10 +52,6 @@ func isRequired(pod *v1.Pod) bool {
 
 // GetProfileName returns the name of the profile to use with the container.
 func GetProfile(pod *v1.Pod, container *v1.Container) *v1.AppArmorProfile {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields) {
-		return getProfileFromPodAnnotations(pod.Annotations, container.Name)
-	}
-
 	if container.SecurityContext != nil && container.SecurityContext.AppArmorProfile != nil {
 		return container.SecurityContext.AppArmorProfile
 	}

pkg/security/apparmor/validate.go

@@ -23,9 +23,7 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer/apparmor"
 	v1 "k8s.io/api/core/v1"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
-	"k8s.io/kubernetes/pkg/features"
 )
 
 // Whether AppArmor should be disabled by default.
@@ -89,11 +87,6 @@ func (v *validator) ValidateHost() error {
 
 // validateHost verifies that the host and runtime is capable of enforcing AppArmor profiles.
 func validateHost() error {
-	// Check feature-gates
-	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmor) {
-		return errors.New("AppArmor disabled by feature-gate")
-	}
-
 	// Check build support.
 	if isDisabledBuild {
 		return errors.New("binary not compiled for linux")

test/e2e/feature/feature.go

@@ -28,9 +28,6 @@ var (
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	APIServerIdentity = framework.WithFeature(framework.ValidFeatures.Add("APIServerIdentity"))
 
-	// TODO: document the feature (owning SIG, when to use this feature for a test)
-	AppArmor = framework.WithFeature(framework.ValidFeatures.Add("AppArmor"))
-
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	BootstrapTokens = framework.WithFeature(framework.ValidFeatures.Add("BootstrapTokens"))
 

test/e2e/nodefeature/nodefeature.go

@@ -25,9 +25,6 @@ import (
 var (
 	// Please keep the list in alphabetical order.
 
-	// TODO: document the feature (owning SIG, when to use this feature for a test)
-	AppArmor = framework.WithNodeFeature(framework.ValidNodeFeatures.Add("AppArmor"))
-
 	// TODO: document the feature (owning SIG, when to use this feature for a test)
 	CheckpointContainer = framework.WithNodeFeature(framework.ValidNodeFeatures.Add("CheckpointContainer"))
 

test/e2e_node/apparmor_test.go

@@ -38,10 +38,8 @@ import (
 	watchtools "k8s.io/client-go/tools/watch"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/kubelet/kuberuntime"
-	"k8s.io/kubernetes/test/e2e/feature"
 	"k8s.io/kubernetes/test/e2e/framework"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
-	"k8s.io/kubernetes/test/e2e/nodefeature"
 	admissionapi "k8s.io/pod-security-admission/api"
 
 	"github.com/onsi/ginkgo/v2"
@@ -49,7 +47,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/apparmor"
 )
 
-var _ = SIGDescribe("AppArmor", feature.AppArmor, nodefeature.AppArmor, func() {
+var _ = SIGDescribe("AppArmor", framework.WithNodeConformance(), func() {
 	if isAppArmorEnabled() {
 		ginkgo.BeforeEach(func() {
 			ginkgo.By("Loading AppArmor profiles for testing")

test/integration/auth/podsecurity_test.go

@@ -54,7 +54,6 @@ func TestPodSecurity(t *testing.T) {
 	// Enable all feature gates needed to allow all fields to be exercised
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ProcMountType, true)
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.UserNamespacesSupport, true)
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmor, true)
 	// Start server
 	server := startPodSecurityServer(t)
 	opts := podsecuritytest.Options{
@@ -101,7 +100,6 @@ func TestPodSecurityWebhook(t *testing.T) {
 	// Enable all feature gates needed to allow all fields to be exercised
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ProcMountType, true)
 	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.UserNamespacesSupport, true)
-	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmor, true)
 
 	// Start test API server.
 	capabilities.SetForTests(capabilities.Capabilities{AllowPrivileged: true})