diff --git a/pkg/kubelet/dockershim/BUILD b/pkg/kubelet/dockershim/BUILD index 431c24e1c77..748a74c79e8 100644 --- a/pkg/kubelet/dockershim/BUILD +++ b/pkg/kubelet/dockershim/BUILD @@ -101,8 +101,6 @@ go_test( "//conditions:default": [], }), data = [ - "fixtures/seccomp/sub/subtest", - "fixtures/seccomp/test", ], importpath = "k8s.io/kubernetes/pkg/kubelet/dockershim", library = ":go_default_library", diff --git a/pkg/kubelet/dockershim/fixtures/seccomp/sub/subtest b/pkg/kubelet/dockershim/fixtures/seccomp/sub/subtest deleted file mode 100644 index 5dc0574216c..00000000000 --- a/pkg/kubelet/dockershim/fixtures/seccomp/sub/subtest +++ /dev/null @@ -1,3 +0,0 @@ -{ - "abc": "def" -} diff --git a/pkg/kubelet/dockershim/fixtures/seccomp/test b/pkg/kubelet/dockershim/fixtures/seccomp/test deleted file mode 100644 index e63d37b65a8..00000000000 --- a/pkg/kubelet/dockershim/fixtures/seccomp/test +++ /dev/null @@ -1,3 +0,0 @@ -{ - "foo": "bar" -} diff --git a/pkg/kubelet/dockershim/helpers_linux.go b/pkg/kubelet/dockershim/helpers_linux.go index a53ecce1777..f8a9a1fbbe0 100644 --- a/pkg/kubelet/dockershim/helpers_linux.go +++ b/pkg/kubelet/dockershim/helpers_linux.go @@ -62,7 +62,11 @@ func getSeccompDockerOpts(seccompProfile string) ([]dockerOpt, error) { return nil, fmt.Errorf("unknown seccomp profile option: %s", seccompProfile) } - fname := strings.TrimPrefix(seccompProfile, "localhost/") // by pod annotation validation, name is a valid subpath + // get the full path of seccomp profile when prefixed with 'localhost/'. + fname := strings.TrimPrefix(seccompProfile, "localhost/") + if !filepath.IsAbs(fname) { + return nil, fmt.Errorf("seccomp profile path must be absolute, but got relative path %q", fname) + } file, err := ioutil.ReadFile(filepath.FromSlash(fname)) if err != nil { return nil, fmt.Errorf("cannot load seccomp profile %q: %v", fname, err) diff --git a/pkg/kubelet/dockershim/helpers_linux_test.go b/pkg/kubelet/dockershim/helpers_linux_test.go index 7dc8dbe0bde..c7aa670831e 100644 --- a/pkg/kubelet/dockershim/helpers_linux_test.go +++ b/pkg/kubelet/dockershim/helpers_linux_test.go @@ -20,9 +20,13 @@ package dockershim import ( "fmt" + "io/ioutil" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetSeccompSecurityOpts(t *testing.T) { @@ -55,26 +59,31 @@ func TestGetSeccompSecurityOpts(t *testing.T) { } func TestLoadSeccompLocalhostProfiles(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "seccomp-local-profile-test") + require.NoError(t, err) + defer os.RemoveAll(tmpdir) + testProfile := `{"foo": "bar"}` + err = ioutil.WriteFile(filepath.Join(tmpdir, "test"), []byte(testProfile), 0644) + require.NoError(t, err) + tests := []struct { msg string seccompProfile string expectedOpts []string expectErr bool }{{ - msg: "Seccomp localhost/test profile", - // We are abusing localhost for loading test seccomp profiles. - // The profile should be an absolute path while we are using a relative one. - seccompProfile: "localhost/fixtures/seccomp/test", + msg: "Seccomp localhost/test profile should return correct seccomp profiles", + seccompProfile: "localhost/" + filepath.Join(tmpdir, "test"), expectedOpts: []string{`seccomp={"foo":"bar"}`}, expectErr: false, }, { - msg: "Seccomp localhost/sub/subtest profile", - seccompProfile: "localhost/fixtures/seccomp/sub/subtest", - expectedOpts: []string{`seccomp={"abc":"def"}`}, - expectErr: false, + msg: "Non-existent profile should return error", + seccompProfile: "localhost/" + filepath.Join(tmpdir, "fixtures/non-existent"), + expectedOpts: nil, + expectErr: true, }, { - msg: "Seccomp non-existent", - seccompProfile: "localhost/fixtures/seccomp/non-existent", + msg: "Relative profile path should return error", + seccompProfile: "localhost/fixtures/test", expectedOpts: nil, expectErr: true, }} diff --git a/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go b/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go index 0dd2ddeafb4..a630588e2e9 100644 --- a/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go +++ b/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go @@ -33,6 +33,10 @@ import ( proberesults "k8s.io/kubernetes/pkg/kubelet/prober/results" ) +const ( + fakeSeccompProfileRoot = "/fakeSeccompProfileRoot" +) + type fakeHTTP struct { url string err error @@ -78,6 +82,7 @@ func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageS runtimeService: runtimeService, imageService: imageService, keyring: keyring, + seccompProfileRoot: fakeSeccompProfileRoot, internalLifecycle: cm.NewFakeInternalContainerLifecycle(), } diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index 89664a27437..d1ee8de3f98 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -273,7 +273,7 @@ func (m *kubeGenericRuntimeManager) getSeccompProfileFromAnnotations(annotations if strings.HasPrefix(profile, "localhost/") { name := strings.TrimPrefix(profile, "localhost/") fname := filepath.Join(m.seccompProfileRoot, filepath.FromSlash(name)) - return fname + return "localhost/" + fname } return profile diff --git a/pkg/kubelet/kuberuntime/helpers_test.go b/pkg/kubelet/kuberuntime/helpers_test.go index 9de0544825f..8690749fa1c 100644 --- a/pkg/kubelet/kuberuntime/helpers_test.go +++ b/pkg/kubelet/kuberuntime/helpers_test.go @@ -17,9 +17,12 @@ limitations under the License. package kuberuntime import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimetesting "k8s.io/kubernetes/pkg/kubelet/apis/cri/testing" @@ -205,3 +208,100 @@ func TestGetImageUser(t *testing.T) { assert.Equal(t, test.expectedImageUserValues.username, username, "TestCase[%d]", j) } } + +func TestGetSeccompProfileFromAnnotations(t *testing.T) { + _, _, m, err := createTestRuntimeManager() + require.NoError(t, err) + + tests := []struct { + description string + annotation map[string]string + containerName string + expectedProfile string + }{ + { + description: "no seccomp should return empty string", + expectedProfile: "", + }, + { + description: "no seccomp with containerName should return exmpty string", + containerName: "container1", + expectedProfile: "", + }, + { + description: "pod docker/default seccomp profile should return docker/default", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "docker/default", + }, + expectedProfile: "docker/default", + }, + { + description: "pod docker/default seccomp profile with containerName should return docker/default", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "docker/default", + }, + containerName: "container1", + expectedProfile: "docker/default", + }, + { + description: "pod unconfined seccomp profile should return unconfined", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "unconfined", + }, + expectedProfile: "unconfined", + }, + { + description: "pod unconfined seccomp profile with containerName should return unconfined", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "unconfined", + }, + containerName: "container1", + expectedProfile: "unconfined", + }, + { + description: "pod localhost seccomp profile should return local profile path", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "localhost/chmod.json", + }, + expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), + }, + { + description: "pod localhost seccomp profile with containerName should return local profile path", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "localhost/chmod.json", + }, + containerName: "container1", + expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), + }, + { + description: "container localhost seccomp profile with containerName should return local profile path", + annotation: map[string]string{ + v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", + }, + containerName: "container1", + expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), + }, + { + description: "container localhost seccomp profile should override pod profile", + annotation: map[string]string{ + v1.SeccompPodAnnotationKey: "unconfined", + v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", + }, + containerName: "container1", + expectedProfile: "localhost/" + filepath.Join(fakeSeccompProfileRoot, "chmod.json"), + }, + { + description: "container localhost seccomp profile with unmatched containerName should return empty string", + annotation: map[string]string{ + v1.SeccompContainerAnnotationKeyPrefix + "container1": "localhost/chmod.json", + }, + containerName: "container2", + expectedProfile: "", + }, + } + + for i, test := range tests { + seccompProfile := m.getSeccompProfileFromAnnotations(test.annotation, test.containerName) + assert.Equal(t, test.expectedProfile, seccompProfile, "TestCase[%d]", i) + } +}