Implement Kubelet AppArmor field handling

pkg/kubelet/kuberuntime/helpers.go

@@ -18,6 +18,7 @@ package kuberuntime
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"strconv"
@@ -28,6 +29,7 @@ import (
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
 	"k8s.io/klog/v2"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
+	"k8s.io/kubernetes/pkg/security/apparmor"
 )
 
 type podsByID []*kubecontainer.Pod
@@ -285,3 +287,35 @@ func (m *kubeGenericRuntimeManager) getSeccompProfile(annotations map[string]str
 		ProfileType: runtimeapi.SecurityProfile_Unconfined,
 	}, nil
 }
+
+func getAppArmorProfile(pod *v1.Pod, container *v1.Container) (*runtimeapi.SecurityProfile, error) {
+	profile := apparmor.GetProfile(pod, container)
+	if profile == nil {
+		return nil, nil
+	}
+
+	switch profile.Type {
+	case v1.AppArmorProfileTypeRuntimeDefault:
+		return &runtimeapi.SecurityProfile{
+			ProfileType: runtimeapi.SecurityProfile_RuntimeDefault,
+		}, nil
+
+	case v1.AppArmorProfileTypeUnconfined:
+		return &runtimeapi.SecurityProfile{
+			ProfileType: runtimeapi.SecurityProfile_Unconfined,
+		}, nil
+
+	case v1.AppArmorProfileTypeLocalhost:
+		if profile.LocalhostProfile == nil {
+			return nil, errors.New("missing localhost apparmor profile name")
+		}
+		return &runtimeapi.SecurityProfile{
+			ProfileType:  runtimeapi.SecurityProfile_Localhost,
+			LocalhostRef: *profile.LocalhostProfile,
+		}, nil
+
+	default:
+		// Shouldn't happen.
+		return nil, fmt.Errorf("unknown apparmor profile type: %q", profile.Type)
+	}
+}

pkg/kubelet/kuberuntime/helpers_test.go

@@ -31,6 +31,7 @@ import (
 	runtimetesting "k8s.io/cri-api/pkg/apis/testing"
 	"k8s.io/kubernetes/pkg/features"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
+	"k8s.io/utils/ptr"
 )
 
 type podStatusProviderFunc func(uid types.UID, name, namespace string) (*kubecontainer.PodStatus, error)
@@ -363,3 +364,75 @@ func TestToKubeContainerState(t *testing.T) {
 		})
 	}
 }
+
+func TestGetAppArmorProfile(t *testing.T) {
+	tests := []struct {
+		name            string
+		podProfile      *v1.AppArmorProfile
+		expectedProfile *runtimeapi.SecurityProfile
+		expectError     bool
+	}{{
+		name:            "no appArmor",
+		expectedProfile: nil,
+	}, {
+		name:       "runtime default",
+		podProfile: &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeRuntimeDefault},
+		expectedProfile: &runtimeapi.SecurityProfile{
+			ProfileType: runtimeapi.SecurityProfile_RuntimeDefault,
+		},
+	}, {
+		name:       "unconfined",
+		podProfile: &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeUnconfined},
+		expectedProfile: &runtimeapi.SecurityProfile{
+			ProfileType: runtimeapi.SecurityProfile_Unconfined,
+		},
+	}, {
+		name: "localhost",
+		podProfile: &v1.AppArmorProfile{
+			Type:             v1.AppArmorProfileTypeLocalhost,
+			LocalhostProfile: ptr.To("test"),
+		},
+		expectedProfile: &runtimeapi.SecurityProfile{
+			ProfileType:  runtimeapi.SecurityProfile_Localhost,
+			LocalhostRef: "test",
+		},
+	}, {
+		name: "invalid localhost",
+		podProfile: &v1.AppArmorProfile{
+			Type: v1.AppArmorProfileTypeLocalhost,
+		},
+		expectError: true,
+	}, {
+		name: "invalid type",
+		podProfile: &v1.AppArmorProfile{
+			Type: "foo",
+		},
+		expectError: true,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			pod := v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "bar",
+				},
+				Spec: v1.PodSpec{
+					SecurityContext: &v1.PodSecurityContext{
+						AppArmorProfile: test.podProfile,
+					},
+					Containers: []v1.Container{{Name: "foo"}},
+				},
+			}
+
+			actual, err := getAppArmorProfile(&pod, &pod.Spec.Containers[0])
+
+			if test.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+
+			assert.Equal(t, test.expectedProfile, actual)
+		})
+	}
+}

pkg/kubelet/kuberuntime/security_context.go

@@ -20,7 +20,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
 	runtimeutil "k8s.io/kubernetes/pkg/kubelet/kuberuntime/util"
-	"k8s.io/kubernetes/pkg/security/apparmor"
 	"k8s.io/kubernetes/pkg/securitycontext"
 )
 
@@ -42,7 +41,10 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po
 	}
 
 	// set ApparmorProfile.
-	synthesized.ApparmorProfile = apparmor.GetProfileNameFromPodAnnotations(pod.Annotations, container.Name)
+	synthesized.Apparmor, err = getAppArmorProfile(pod, container)
+	if err != nil {
+		return nil, err
+	}
 
 	// set RunAsUser.
 	if synthesized.RunAsUser == nil {

pkg/security/apparmor/helpers.go

@@ -19,11 +19,29 @@ package apparmor
 import (
 	"strings"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
+	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 )
 
-// Checks whether app armor is required for pod to be run.
+// Checks whether app armor is required for the pod to run. AppArmor is considered required if any
+// non-unconfined profiles are specified.
 func isRequired(pod *v1.Pod) bool {
+	if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.AppArmorProfile != nil &&
+		pod.Spec.SecurityContext.AppArmorProfile.Type != v1.AppArmorProfileTypeUnconfined {
+		return true
+	}
+
+	inUse := !podutil.VisitContainers(&pod.Spec, podutil.AllContainers, func(c *v1.Container, _ podutil.ContainerType) bool {
+		if c.SecurityContext != nil && c.SecurityContext.AppArmorProfile != nil &&
+			c.SecurityContext.AppArmorProfile.Type != v1.AppArmorProfileTypeUnconfined {
+			return false // is in use; short-circuit
+		}
+		return true
+	})
+	if inUse {
+		return true
+	}
+
 	for key, value := range pod.Annotations {
 		if strings.HasPrefix(key, v1.AppArmorBetaContainerAnnotationKeyPrefix) {
 			return value != v1.AppArmorBetaProfileNameUnconfined
@@ -33,12 +51,49 @@ func isRequired(pod *v1.Pod) bool {
 }
 
 // GetProfileName returns the name of the profile to use with the container.
-func GetProfileName(pod *v1.Pod, containerName string) string {
-	return GetProfileNameFromPodAnnotations(pod.Annotations, containerName)
+func GetProfile(pod *v1.Pod, container *v1.Container) *v1.AppArmorProfile {
+	if container.SecurityContext != nil && container.SecurityContext.AppArmorProfile != nil {
+		return container.SecurityContext.AppArmorProfile
+	}
+
+	// Static pods may not have had annotations synced to fields, so fallback to annotations before
+	// the pod profile.
+	if profile := getProfileFromPodAnnotations(pod.Annotations, container.Name); profile != nil {
+		return profile
+	}
+
+	if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.AppArmorProfile != nil {
+		return pod.Spec.SecurityContext.AppArmorProfile
+	}
+
+	return nil
 }
 
-// GetProfileNameFromPodAnnotations gets the name of the profile to use with container from
-// pod annotations
-func GetProfileNameFromPodAnnotations(annotations map[string]string, containerName string) string {
-	return annotations[v1.AppArmorBetaContainerAnnotationKeyPrefix+containerName]
+// getProfileFromPodAnnotations gets the AppArmor profile to use with container from
+// (deprecated) pod annotations.
+func getProfileFromPodAnnotations(annotations map[string]string, containerName string) *v1.AppArmorProfile {
+	val, ok := annotations[v1.AppArmorBetaContainerAnnotationKeyPrefix+containerName]
+	if !ok {
+		return nil
+	}
+
+	switch {
+	case val == v1.AppArmorBetaProfileRuntimeDefault:
+		return &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeRuntimeDefault}
+
+	case val == v1.AppArmorBetaProfileNameUnconfined:
+		return &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeUnconfined}
+
+	case strings.HasPrefix(val, v1.AppArmorBetaProfileNamePrefix):
+		// Note: an invalid empty localhost profile will be rejected by kubelet admission.
+		profileName := strings.TrimPrefix(val, v1.AppArmorBetaProfileNamePrefix)
+		return &v1.AppArmorProfile{
+			Type:             v1.AppArmorProfileTypeLocalhost,
+			LocalhostProfile: &profileName,
+		}
+
+	default:
+		// Invalid annotation.
+		return nil
+	}
 }

pkg/security/apparmor/helpers_test.go

@@ -0,0 +1,124 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package apparmor
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
+)
+
+func TestGetProfile(t *testing.T) {
+	runtimeDefault := &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeRuntimeDefault}
+	unconfined := &v1.AppArmorProfile{Type: v1.AppArmorProfileTypeUnconfined}
+	localhost := &v1.AppArmorProfile{
+		Type:             v1.AppArmorProfileTypeLocalhost,
+		LocalhostProfile: ptr.To("test"),
+	}
+
+	tests := []struct {
+		name              string
+		annotationProfile string
+		containerProfile  *v1.AppArmorProfile
+		podProfile        *v1.AppArmorProfile
+		expectedProfile   *v1.AppArmorProfile
+	}{{
+		name:            "no appArmor",
+		expectedProfile: nil,
+	}, {
+		name:            "pod profile",
+		podProfile:      runtimeDefault,
+		expectedProfile: runtimeDefault,
+	}, {
+		name:             "container profile",
+		containerProfile: unconfined,
+		expectedProfile:  unconfined,
+	}, {
+		name:              "annotation profile",
+		annotationProfile: v1.AppArmorBetaProfileNamePrefix + "test",
+		expectedProfile:   localhost,
+	}, {
+		name:              "invalid annotation",
+		annotationProfile: "invalid",
+		expectedProfile:   nil,
+	}, {
+		name:              "invalid annotation with pod field",
+		annotationProfile: "invalid",
+		podProfile:        runtimeDefault,
+		expectedProfile:   runtimeDefault,
+	}, {
+		name:              "container field before annotation",
+		annotationProfile: v1.AppArmorBetaProfileNameUnconfined,
+		containerProfile:  runtimeDefault,
+		expectedProfile:   runtimeDefault,
+	}, {
+		name:             "container field before pod field",
+		containerProfile: runtimeDefault,
+		podProfile:       unconfined,
+		expectedProfile:  runtimeDefault,
+	}, {
+		name:              "annotation before pod field",
+		annotationProfile: v1.AppArmorBetaProfileNameUnconfined,
+		podProfile:        runtimeDefault,
+		expectedProfile:   unconfined,
+	}, {
+		name:              "all profiles",
+		annotationProfile: v1.AppArmorBetaProfileRuntimeDefault,
+		containerProfile:  localhost,
+		podProfile:        unconfined,
+		expectedProfile:   localhost,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			container := v1.Container{
+				Name: "foo",
+			}
+			if test.containerProfile != nil {
+				container.SecurityContext = &v1.SecurityContext{
+					AppArmorProfile: test.containerProfile.DeepCopy(),
+				}
+			}
+			pod := v1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "bar",
+					Annotations: map[string]string{
+						"unrelated": "baz",
+						v1.AppArmorBetaContainerAnnotationKeyPrefix + "other": v1.AppArmorBetaProfileRuntimeDefault,
+					},
+				},
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{container},
+				},
+			}
+			if test.annotationProfile != "" {
+				pod.Annotations[v1.AppArmorBetaContainerAnnotationKeyPrefix+container.Name] = test.annotationProfile
+			}
+			if test.podProfile != nil {
+				pod.Spec.SecurityContext = &v1.PodSecurityContext{
+					AppArmorProfile: test.podProfile.DeepCopy(),
+				}
+			}
+
+			actual := GetProfile(&pod, &container)
+			assert.Equal(t, test.expectedProfile, actual)
+		})
+	}
+}

pkg/security/apparmor/validate.go

@@ -25,7 +25,6 @@ import (
 	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/apis/core/validation"
 	"k8s.io/kubernetes/pkg/features"
 )
 
@@ -62,15 +61,15 @@ func (v *validator) Validate(pod *v1.Pod) error {
 
 	var retErr error
 	podutil.VisitContainers(&pod.Spec, podutil.AllContainers, func(container *v1.Container, containerType podutil.ContainerType) bool {
-		profile := GetProfileName(pod, container.Name)
-		retErr = validation.ValidateAppArmorProfileFormat(profile)
-		if retErr != nil {
-			return false
+		profile := GetProfile(pod, container)
+		if profile == nil {
+			return true
 		}
+
 		// TODO(#64841): This would ideally be part of validation.ValidateAppArmorProfileFormat, but
 		// that is called for API validation, and this is tightening validation.
-		if strings.HasPrefix(profile, v1.AppArmorBetaProfileNamePrefix) {
-			if strings.TrimSpace(strings.TrimPrefix(profile, v1.AppArmorBetaProfileNamePrefix)) == "" {
+		if profile.Type == v1.AppArmorProfileTypeLocalhost {
+			if profile.LocalhostProfile == nil || strings.TrimSpace(*profile.LocalhostProfile) == "" {
 				retErr = fmt.Errorf("invalid empty AppArmor profile name: %q", profile)
 				return false
 			}

pkg/security/apparmor/validate_test.go

@@ -64,7 +64,6 @@ func TestValidateValidHost(t *testing.T) {
 		{v1.AppArmorBetaProfileNamePrefix + "docker-default", true},
 		{v1.AppArmorBetaProfileNamePrefix + "foo-container", true},
 		{v1.AppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true},
-		{"docker-default", false},
 		{v1.AppArmorBetaProfileNamePrefix + "", false}, // Empty profile explicitly forbidden.
 		{v1.AppArmorBetaProfileNamePrefix + " ", false},
 	}