Separate feature-gate for AppArmor fields
This commit is contained in:
		| @@ -539,12 +539,14 @@ func dropDisabledFields( | ||||
| 		podSpec = &api.PodSpec{} | ||||
| 	} | ||||
|  | ||||
| 	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmor) && !appArmorInUse(oldPodAnnotations, oldPodSpec) { | ||||
| 	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 | ||||
| 		} | ||||
| @@ -947,17 +949,21 @@ func procMountInUse(podSpec *api.PodSpec) bool { | ||||
| 	return inUse | ||||
| } | ||||
|  | ||||
| // appArmorInUse returns true if the pod has apparmor related information | ||||
| func appArmorInUse(podAnnotations map[string]string, podSpec *api.PodSpec) bool { | ||||
| 	if podSpec == nil { | ||||
| 		return false | ||||
| 	} | ||||
|  | ||||
| // appArmorAnnotationsInUse returns true if the pod has apparmor annotations | ||||
| func appArmorAnnotationsInUse(podAnnotations map[string]string) bool { | ||||
| 	for k := range podAnnotations { | ||||
| 		if strings.HasPrefix(k, api.DeprecatedAppArmorAnnotationKeyPrefix) { | ||||
| 			return true | ||||
| 		} | ||||
| 	} | ||||
| 	return false | ||||
| } | ||||
|  | ||||
| // appArmorFieldsInUse returns true if the pod has apparmor fields set | ||||
| func appArmorFieldsInUse(podSpec *api.PodSpec) bool { | ||||
| 	if podSpec == nil { | ||||
| 		return false | ||||
| 	} | ||||
| 	if podSpec.SecurityContext != nil && podSpec.SecurityContext.AppArmorProfile != nil { | ||||
| 		return true | ||||
| 	} | ||||
|   | ||||
| @@ -707,19 +707,34 @@ func TestDropProcMount(t *testing.T) { | ||||
|  | ||||
| func TestDropAppArmor(t *testing.T) { | ||||
| 	tests := []struct { | ||||
| 		description string | ||||
| 		hasAppArmor bool | ||||
| 		pod         api.Pod | ||||
| 		description    string | ||||
| 		hasAnnotations bool | ||||
| 		hasFields      bool | ||||
| 		pod            api.Pod | ||||
| 	}{{ | ||||
| 		description: "with AppArmor Annotations", | ||||
| 		hasAppArmor: true, | ||||
| 		description:    "with AppArmor Annotations", | ||||
| 		hasAnnotations: true, | ||||
| 		pod: api.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"a": "1", v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "foo": "default"}}, | ||||
| 			Spec:       api.PodSpec{}, | ||||
| 		}, | ||||
| 	}, { | ||||
| 		description:    "with AppArmor Annotations & fields", | ||||
| 		hasAnnotations: true, | ||||
| 		hasFields:      true, | ||||
| 		pod: api.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"a": "1", v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "foo": "default"}}, | ||||
| 			Spec: api.PodSpec{ | ||||
| 				SecurityContext: &api.PodSecurityContext{ | ||||
| 					AppArmorProfile: &api.AppArmorProfile{ | ||||
| 						Type: api.AppArmorProfileTypeRuntimeDefault, | ||||
| 					}, | ||||
| 				}, | ||||
| 			}, | ||||
| 		}, | ||||
| 	}, { | ||||
| 		description: "with pod AppArmor profile", | ||||
| 		hasAppArmor: true, | ||||
| 		hasFields:   true, | ||||
| 		pod: api.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"a": "1"}}, | ||||
| 			Spec: api.PodSpec{ | ||||
| @@ -732,7 +747,7 @@ func TestDropAppArmor(t *testing.T) { | ||||
| 		}, | ||||
| 	}, { | ||||
| 		description: "with container AppArmor profile", | ||||
| 		hasAppArmor: true, | ||||
| 		hasFields:   true, | ||||
| 		pod: api.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"a": "1"}}, | ||||
| 			Spec: api.PodSpec{ | ||||
| @@ -747,7 +762,6 @@ func TestDropAppArmor(t *testing.T) { | ||||
| 		}, | ||||
| 	}, { | ||||
| 		description: "without AppArmor", | ||||
| 		hasAppArmor: false, | ||||
| 		pod: api.Pod{ | ||||
| 			ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"a": "1"}}, | ||||
| 			Spec:       api.PodSpec{}, | ||||
| @@ -756,34 +770,43 @@ func TestDropAppArmor(t *testing.T) { | ||||
|  | ||||
| 	for _, test := range tests { | ||||
| 		for _, enabled := range []bool{true, false} { | ||||
| 			t.Run(fmt.Sprintf("%v/enabled=%v", test.description, enabled), func(t *testing.T) { | ||||
| 				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmor, enabled)() | ||||
| 			for _, fieldsEnabled := range []bool{true, false} { | ||||
| 				t.Run(fmt.Sprintf("%v/enabled=%v/fields=%v", test.description, enabled, fieldsEnabled), func(t *testing.T) { | ||||
| 					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmor, enabled)() | ||||
| 					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AppArmorFields, fieldsEnabled)() | ||||
|  | ||||
| 				newPod := test.pod.DeepCopy() | ||||
| 					newPod := test.pod.DeepCopy() | ||||
|  | ||||
| 				if actual := appArmorInUse(newPod.Annotations, &newPod.Spec); actual != test.hasAppArmor { | ||||
| 					t.Errorf("appArmorInUse does not match expectation: %t != %t", actual, test.hasAppArmor) | ||||
| 				} | ||||
|  | ||||
| 				DropDisabledPodFields(newPod, newPod) | ||||
| 				require.Equal(t, &test.pod, newPod, "unchanged pod should never be mutated") | ||||
|  | ||||
| 				DropDisabledPodFields(newPod, nil) | ||||
|  | ||||
| 				if enabled { | ||||
| 					assert.Equal(t, &test.pod, newPod, "pod should not be mutated when AppArmor is enabled") | ||||
| 				} else { | ||||
| 					if appArmorInUse(newPod.Annotations, &newPod.Spec) { | ||||
| 						t.Errorf("newPod should not be using appArmor after dropping disabled fields") | ||||
| 					if hasAnnotations := appArmorAnnotationsInUse(newPod.Annotations); 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) | ||||
| 					} | ||||
|  | ||||
| 					if test.hasAppArmor { | ||||
| 						assert.NotEqual(t, &test.pod, newPod, "pod should be mutated to drop AppArmor") | ||||
| 					} else { | ||||
| 						assert.Equal(t, &test.pod, newPod, "pod without AppArmor should not be mutated") | ||||
| 					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") | ||||
| 					} | ||||
| 				}) | ||||
| 			} | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
|   | ||||
| @@ -4736,6 +4736,9 @@ 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 | ||||
|   | ||||
| @@ -63,6 +63,10 @@ const ( | ||||
| 	// beta: v1.4 | ||||
| 	AppArmor featuregate.Feature = "AppArmor" | ||||
|  | ||||
| 	// owner: @tallclair | ||||
| 	// beta: v1.30 | ||||
| 	AppArmorFields featuregate.Feature = "AppArmorFields" | ||||
|  | ||||
| 	// owner: @danwinship | ||||
| 	// alpha: v1.27 | ||||
| 	// beta: v1.29 | ||||
| @@ -975,6 +979,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS | ||||
|  | ||||
| 	AppArmor: {Default: true, PreRelease: featuregate.Beta}, | ||||
|  | ||||
| 	AppArmorFields: {Default: true, PreRelease: featuregate.Beta}, | ||||
|  | ||||
| 	CloudDualStackNodeIPs: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32 | ||||
|  | ||||
| 	ClusterTrustBundle: {Default: false, PreRelease: featuregate.Alpha}, | ||||
|   | ||||
| @@ -764,6 +764,10 @@ 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(pod *api.Pod) { | ||||
| 	if !utilfeature.DefaultFeatureGate.Enabled(features.AppArmorFields) { | ||||
| 		return | ||||
| 	} | ||||
|  | ||||
| 	if pod.Spec.OS != nil && pod.Spec.OS.Name == api.Windows { | ||||
| 		return | ||||
| 	} | ||||
|   | ||||
| @@ -20,7 +20,9 @@ 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 | ||||
| @@ -52,6 +54,10 @@ 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 | ||||
| 	} | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Tim Allclair
					Tim Allclair