diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index d6294ce2837..3d0ddf0d423 100644 --- a/pkg/kubelet/kubelet.go +++ b/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 { diff --git a/pkg/kubelet/lifecycle/handlers.go b/pkg/kubelet/lifecycle/handlers.go index f43305bb2b2..74b9b7331d6 100644 --- a/pkg/kubelet/lifecycle/handlers.go +++ b/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), + } +} diff --git a/pkg/security/apparmor/validate.go b/pkg/security/apparmor/validate.go index 4466fd76b30..a34636caea2 100644 --- a/pkg/security/apparmor/validate.go +++ b/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 } diff --git a/pkg/security/apparmor/validate_test.go b/pkg/security/apparmor/validate_test.go index d71212d8c75..b200f5b02d0 100644 --- a/pkg/security/apparmor/validate_test.go +++ b/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. } }