Remove apparmor dependency on pkg/kubelet/lifecycle

pkg/kubelet/kubelet.go

@@ -73,7 +73,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/util/queue"
 	"k8s.io/kubernetes/pkg/kubelet/volumemanager"
 	"k8s.io/kubernetes/pkg/runtime"
-	"k8s.io/kubernetes/pkg/security/apparmor"
 	"k8s.io/kubernetes/pkg/securitycontext"
 	"k8s.io/kubernetes/pkg/types"
 	"k8s.io/kubernetes/pkg/util/bandwidth"
@@ -601,7 +600,7 @@ func NewMainKubelet(
 	klet.AddPodSyncLoopHandler(activeDeadlineHandler)
 	klet.AddPodSyncHandler(activeDeadlineHandler)
 
-	klet.AddPodAdmitHandler(apparmor.NewValidator(containerRuntime))
+	klet.AddPodAdmitHandler(lifecycle.NewAppArmorAdmitHandler(containerRuntime))
 
 	// apply functional Option's
 	for _, opt := range kubeOptions {

pkg/kubelet/lifecycle/handlers.go

@@ -30,6 +30,7 @@ import (
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/kubelet/util/format"
 	"k8s.io/kubernetes/pkg/kubelet/util/ioutils"
+	"k8s.io/kubernetes/pkg/security/apparmor"
 	"k8s.io/kubernetes/pkg/types"
 	"k8s.io/kubernetes/pkg/util/intstr"
 )
@@ -142,3 +143,25 @@ func getHttpRespBody(resp *http.Response) string {
 	}
 	return ""
 }
+
+func NewAppArmorAdmitHandler(runtime string) PodAdmitHandler {
+	return &appArmorAdmitHandler{
+		Validator: apparmor.NewValidator(runtime),
+	}
+}
+
+type appArmorAdmitHandler struct {
+	apparmor.Validator
+}
+
+func (a *appArmorAdmitHandler) Admit(attrs *PodAdmitAttributes) PodAdmitResult {
+	err := a.Validate(attrs.Pod)
+	if err == nil {
+		return PodAdmitResult{Admit: true}
+	}
+	return PodAdmitResult{
+		Admit:   false,
+		Reason:  "AppArmor",
+		Message: fmt.Sprintf("Cannot enforce AppArmor: %v", err),
+	}
+}

pkg/security/apparmor/validate.go

@@ -26,7 +26,6 @@ import (
 	"strings"
 
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 	"k8s.io/kubernetes/pkg/util"
 )
 
@@ -34,11 +33,12 @@ import (
 // Set to true if the wrong build tags are set (see validate_disabled.go).
 var isDisabledBuild bool
 
-const (
-	rejectReason = "AppArmor"
-)
+// Interface for validating that a pod with with an AppArmor profile can be run by a Node.
+type Validator interface {
+	Validate(pod *api.Pod) error
+}
 
-func NewValidator(runtime string) lifecycle.PodAdmitHandler {
+func NewValidator(runtime string) Validator {
 	if err := validateHost(runtime); err != nil {
 		return &validator{validateHostErr: err}
 	}
@@ -58,21 +58,7 @@ type validator struct {
 	appArmorFS      string
 }
 
-// TODO(timstclair): Refactor the PodAdmitInterface to return a (Admit, Reason Message) rather than
-// the PodAdmitResult struct so that the interface can be implemented without importing lifecycle.
-func (v *validator) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitResult {
-	err := v.validate(attrs.Pod)
-	if err == nil {
-		return lifecycle.PodAdmitResult{Admit: true}
-	}
-	return lifecycle.PodAdmitResult{
-		Admit:   false,
-		Reason:  rejectReason,
-		Message: fmt.Sprintf("Cannot enforce AppArmor: %v", err),
-	}
-}
-
-func (v *validator) validate(pod *api.Pod) error {
+func (v *validator) Validate(pod *api.Pod) error {
 	if !isRequired(pod) {
 		return nil
 	}
@@ -122,18 +108,27 @@ func validateHost(runtime string) error {
 
 // Verify that the profile is valid and loaded.
 func validateProfile(profile string, loadedProfiles map[string]bool) error {
+	if err := ValidateProfileFormat(profile); err != nil {
+		return err
+	}
+
+	if strings.HasPrefix(profile, ProfileNamePrefix) {
+		profileName := strings.TrimPrefix(profile, ProfileNamePrefix)
+		if !loadedProfiles[profileName] {
+			return fmt.Errorf("profile %q is not loaded", profileName)
+		}
+	}
+
+	return nil
+}
+
+func ValidateProfileFormat(profile string) error {
 	if profile == "" || profile == ProfileRuntimeDefault {
 		return nil
 	}
 	if !strings.HasPrefix(profile, ProfileNamePrefix) {
 		return fmt.Errorf("invalid AppArmor profile name: %q", profile)
 	}
-
-	profileName := strings.TrimPrefix(profile, ProfileNamePrefix)
-	if !loadedProfiles[profileName] {
-		return fmt.Errorf("profile %q is not loaded", profileName)
-	}
-
 	return nil
 }
 

pkg/security/apparmor/validate_test.go

@@ -21,7 +21,6 @@ import (
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -37,7 +36,7 @@ func TestGetAppArmorFS(t *testing.T) {
 	assert.Equal(t, expectedPath, actualPath)
 }
 
-func TestAdmitHost(t *testing.T) {
+func TestValidateHost(t *testing.T) {
 	// This test only passes on systems running AppArmor with the default configuration.
 	// The test should be manually run if modifying the getAppArmorFS function.
 	t.Skip()
@@ -46,7 +45,7 @@ func TestAdmitHost(t *testing.T) {
 	assert.Error(t, validateHost("rkt"))
 }
 
-func TestAdmitProfile(t *testing.T) {
+func TestValidateProfile(t *testing.T) {
 	loadedProfiles := map[string]bool{
 		"docker-default":                                true,
 		"foo-bar":                                       true,
@@ -79,7 +78,7 @@ func TestAdmitProfile(t *testing.T) {
 	}
 }
 
-func TestAdmitBadHost(t *testing.T) {
+func TestValidateBadHost(t *testing.T) {
 	hostErr := errors.New("expected host error")
 	v := &validator{
 		validateHostErr: hostErr,
@@ -95,20 +94,16 @@ func TestAdmitBadHost(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result := v.Admit(&lifecycle.PodAdmitAttributes{
-			Pod: getPodWithProfile(test.profile),
-		})
+		err := v.Validate(getPodWithProfile(test.profile))
 		if test.expectValid {
-			assert.True(t, result.Admit, "Pod with profile %q should be admitted", test.profile)
+			assert.NoError(t, err, "Pod with profile %q should be valid", test.profile)
 		} else {
-			assert.False(t, result.Admit, "Pod with profile %q should be rejected", test.profile)
-			assert.Equal(t, rejectReason, result.Reason, "Pod with profile %q", test.profile)
-			assert.Contains(t, result.Message, hostErr.Error(), "Pod with profile %q", test.profile)
+			assert.Equal(t, hostErr, err, "Pod with profile %q should trigger a host validation error", test.profile)
 		}
 	}
 }
 
-func TestAdmitValidHost(t *testing.T) {
+func TestValidateValidHost(t *testing.T) {
 	v := &validator{
 		appArmorFS: "./testdata/",
 	}
@@ -128,15 +123,11 @@ func TestAdmitValidHost(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result := v.Admit(&lifecycle.PodAdmitAttributes{
-			Pod: getPodWithProfile(test.profile),
-		})
+		err := v.Validate(getPodWithProfile(test.profile))
 		if test.expectValid {
-			assert.True(t, result.Admit, "Pod with profile %q should be admitted", test.profile)
+			assert.NoError(t, err, "Pod with profile %q should be valid", test.profile)
 		} else {
-			assert.False(t, result.Admit, "Pod with profile %q should be rejected", test.profile)
-			assert.Equal(t, rejectReason, result.Reason, "Pod with profile %q", test.profile)
-			assert.NotEmpty(t, result.Message, "Pod with profile %q", test.profile)
+			assert.Error(t, err, "Pod with profile %q should trigger a validation error", test.profile)
 		}
 	}
 
@@ -160,16 +151,10 @@ func TestAdmitValidHost(t *testing.T) {
 			},
 		},
 	}
-	assert.True(t, v.Admit(&lifecycle.PodAdmitAttributes{Pod: pod}).Admit,
-		"Multi-container pod should be admitted")
+	assert.NoError(t, v.Validate(pod), "Multi-container pod should validate")
 	for k, val := range pod.Annotations {
 		pod.Annotations[k] = val + "-bad"
-
-		result := v.Admit(&lifecycle.PodAdmitAttributes{Pod: pod})
-		assert.False(t, result.Admit, "Multi-container pod with invalid profile should be rejected")
-		assert.Equal(t, rejectReason, result.Reason, "Multi-container pod with invalid profile")
-		assert.NotEmpty(t, result.Message, "Multi-container pod with invalid profile")
-
+		assert.Error(t, v.Validate(pod), "Multi-container pod with invalid profile %s:%s", k, pod.Annotations[k])
 		pod.Annotations[k] = val // Restore.
 	}
 }