Merge pull request #49178 from feiskyer/seccomp-impl

Automatic merge from submit-queue Support seccomp profile from container's security context **What this PR does / why we need it**: Support seccomp profile from container's security context, followup of #46332. **Which issue this PR fixes** fixes #46332. **Special notes for your reviewer**: ~~Depends on #49179. (already merged)~~ **Release note**: ```release-note NONE ```
kubernetes · Aug 14, 2017 · b32639f · b32639f
2 parents 2820b45 + ea4a341
commit b32639f
Show file tree

Hide file tree

Showing 15 changed files with 110 additions and 118 deletions.
diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go
@@ -819,9 +819,8 @@ func RunDockershim(c *componentconfig.KubeletConfiguration, r *options.Container
 		SupportedPortForwardProtocols:   streaming.DefaultConfig.SupportedPortForwardProtocols,
 	}
 
-	ds, err := dockershim.NewDockerService(dockerClient, c.SeccompProfileRoot, r.PodSandboxImage,
-		streamingConfig, &pluginSettings, c.RuntimeCgroups, c.CgroupDriver, r.DockerExecHandlerName, r.DockershimRootDirectory,
-		r.DockerDisableSharedPID)
+	ds, err := dockershim.NewDockerService(dockerClient, r.PodSandboxImage, streamingConfig, &pluginSettings,
+		c.RuntimeCgroups, c.CgroupDriver, r.DockerExecHandlerName, r.DockershimRootDirectory, r.DockerDisableSharedPID)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/kubelet/apis/cri/v1alpha1/runtime/api.pb.go b/pkg/kubelet/apis/cri/v1alpha1/runtime/api.pb.go
diff --git a/pkg/kubelet/apis/cri/v1alpha1/runtime/api.proto b/pkg/kubelet/apis/cri/v1alpha1/runtime/api.proto
@@ -203,10 +203,11 @@ message LinuxSandboxSecurityContext {
     // privileged containers are expected to be run.
     bool privileged = 6;
     // Seccomp profile for the sandbox, candidate values are:
-    // * runtime/default: the default profile for the container runtime
+    // * docker/default: the default profile for the docker container runtime
     // * unconfined: unconfined profile, ie, no seccomp sandboxing
     // * localhost/<full-path-to-profile>: the profile installed on the node.
     //   <full-path-to-profile> is the full path of the profile.
+    // Default: "", which is identical with unconfined.
     string seccomp_profile_path = 7;
 }
 
@@ -515,10 +516,11 @@ message LinuxContainerSecurityContext {
     //    http://wiki.apparmor.net/index.php/AppArmor_Core_Policy_Reference
     string apparmor_profile = 9;
     // Seccomp profile for the container, candidate values are:
-    // * runtime/default: the default profile for the container runtime
+    // * docker/default: the default profile for the docker container runtime
     // * unconfined: unconfined profile, ie, no seccomp sandboxing
     // * localhost/<full-path-to-profile>: the profile installed on the node.
     //   <full-path-to-profile> is the full path of the profile.
+    // Default: "", which is identical with unconfined.
     string seccomp_profile_path = 10;
     // no_new_privs defines if the flag for no_new_privs should be set on the
     // container.

diff --git a/pkg/kubelet/dockershim/BUILD b/pkg/kubelet/dockershim/BUILD
@@ -123,12 +123,7 @@ go_test(
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/github.com/stretchr/testify/require:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/clock:go_default_library",
-    ] + select({
-        "@io_bazel_rules_go//go/platform:linux_amd64": [
-            "//vendor/k8s.io/api/core/v1:go_default_library",
-        ],
-        "//conditions:default": [],
-    }),
+    ],
 )
 
 filegroup(

diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go
@@ -150,7 +150,7 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeapi
 	}
 	hc.Resources.Devices = devices
 
-	securityOpts, err := ds.getSecurityOpts(config.Metadata.Name, sandboxConfig, securityOptSep)
+	securityOpts, err := ds.getSecurityOpts(config.GetLinux().GetSecurityContext().GetSeccompProfilePath(), securityOptSep)
 	if err != nil {
 		return "", fmt.Errorf("failed to generate security options for container %q: %v", config.Metadata.Name, err)
 	}

diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go
@@ -588,7 +588,7 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 	}
 
 	// Set security options.
-	securityOpts, err := ds.getSecurityOpts(sandboxContainerName, c, securityOptSep)
+	securityOpts, err := ds.getSecurityOpts(c.GetLinux().GetSecurityContext().GetSeccompProfilePath(), securityOptSep)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate sandbox security options for sandbox %q: %v", c.Metadata.Name, err)
 	}

diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go
@@ -146,7 +146,7 @@ type dockerNetworkHost struct {
 var internalLabelKeys []string = []string{containerTypeLabelKey, containerLogPathLabelKey, sandboxIDLabelKey}
 
 // NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process.
-func NewDockerService(client libdocker.Interface, seccompProfileRoot string, podSandboxImage string, streamingConfig *streaming.Config,
+func NewDockerService(client libdocker.Interface, podSandboxImage string, streamingConfig *streaming.Config,
 	pluginSettings *NetworkPluginSettings, cgroupsName string, kubeCgroupDriver string, execHandlerName, dockershimRootDir string, disableSharedPID bool) (DockerService, error) {
 	c := libdocker.NewInstrumentedInterface(client)
 	checkpointHandler, err := NewPersistentCheckpointHandler(dockershimRootDir)
@@ -165,10 +165,9 @@ func NewDockerService(client libdocker.Interface, seccompProfileRoot string, pod
 	}
 
 	ds := &dockerService{
-		seccompProfileRoot: seccompProfileRoot,
-		client:             c,
-		os:                 kubecontainer.RealOS{},
-		podSandboxImage:    podSandboxImage,
+		client:          c,
+		os:              kubecontainer.RealOS{},
+		podSandboxImage: podSandboxImage,
 		streamingRuntime: &streamingRuntime{
 			client:      client,
 			execHandler: execHandler,
@@ -244,12 +243,11 @@ type DockerService interface {
 }
 
 type dockerService struct {
-	seccompProfileRoot string
-	client             libdocker.Interface
-	os                 kubecontainer.OSInterface
-	podSandboxImage    string
-	streamingRuntime   *streamingRuntime
-	streamingServer    streaming.Server
+	client           libdocker.Interface
+	os               kubecontainer.OSInterface
+	podSandboxImage  string
+	streamingRuntime *streamingRuntime
+	streamingServer  streaming.Server
 
 	network *network.PluginManager
 	// Map of podSandboxID :: network-is-ready

diff --git a/pkg/kubelet/dockershim/helpers_linux.go b/pkg/kubelet/dockershim/helpers_linux.go
@@ -30,71 +30,58 @@ import (
 	"github.com/blang/semver"
 	dockertypes "github.com/docker/docker/api/types"
 	dockercontainer "github.com/docker/docker/api/types/container"
-	"k8s.io/api/core/v1"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
 )
 
 func DefaultMemorySwap() int64 {
 	return 0
 }
 
-func (ds *dockerService) getSecurityOpts(containerName string, sandboxConfig *runtimeapi.PodSandboxConfig, separator rune) ([]string, error) {
+func (ds *dockerService) getSecurityOpts(seccompProfile string, separator rune) ([]string, error) {
 	// Apply seccomp options.
-	seccompSecurityOpts, err := getSeccompSecurityOpts(containerName, sandboxConfig, ds.seccompProfileRoot, separator)
+	seccompSecurityOpts, err := getSeccompSecurityOpts(seccompProfile, separator)
 	if err != nil {
-		return nil, fmt.Errorf("failed to generate seccomp security options for container %q: %v", containerName, err)
+		return nil, fmt.Errorf("failed to generate seccomp security options for container: %v", err)
 	}
 
 	return seccompSecurityOpts, nil
 }
 
-func getSeccompDockerOpts(annotations map[string]string, ctrName, profileRoot string) ([]dockerOpt, error) {
-	profile, profileOK := annotations[v1.SeccompContainerAnnotationKeyPrefix+ctrName]
-	if !profileOK {
-		// try the pod profile
-		profile, profileOK = annotations[v1.SeccompPodAnnotationKey]
-		if !profileOK {
-			// return early the default
-			return defaultSeccompOpt, nil
-		}
-	}
-
-	if profile == "unconfined" {
+func getSeccompDockerOpts(seccompProfile string) ([]dockerOpt, error) {
+	if seccompProfile == "" || seccompProfile == "unconfined" {
 		// return early the default
 		return defaultSeccompOpt, nil
 	}
 
-	if profile == "docker/default" {
+	if seccompProfile == "docker/default" {
 		// return nil so docker will load the default seccomp profile
 		return nil, nil
 	}
 
-	if !strings.HasPrefix(profile, "localhost/") {
-		return nil, fmt.Errorf("unknown seccomp profile option: %s", profile)
+	if !strings.HasPrefix(seccompProfile, "localhost/") {
+		return nil, fmt.Errorf("unknown seccomp profile option: %s", seccompProfile)
 	}
 
-	name := strings.TrimPrefix(profile, "localhost/") // by pod annotation validation, name is a valid subpath
-	fname := filepath.Join(profileRoot, filepath.FromSlash(name))
-	file, err := ioutil.ReadFile(fname)
+	fname := strings.TrimPrefix(seccompProfile, "localhost/") // by pod annotation validation, name is a valid subpath
+	file, err := ioutil.ReadFile(filepath.FromSlash(fname))
 	if err != nil {
-		return nil, fmt.Errorf("cannot load seccomp profile %q: %v", name, err)
+		return nil, fmt.Errorf("cannot load seccomp profile %q: %v", fname, err)
 	}
 
 	b := bytes.NewBuffer(nil)
 	if err := json.Compact(b, file); err != nil {
 		return nil, err
 	}
 	// Rather than the full profile, just put the filename & md5sum in the event log.
-	msg := fmt.Sprintf("%s(md5:%x)", name, md5.Sum(file))
+	msg := fmt.Sprintf("%s(md5:%x)", fname, md5.Sum(file))
 
 	return []dockerOpt{{"seccomp", b.String(), msg}}, nil
 }
 
-// getSeccompSecurityOpts gets container seccomp options from container and sandbox
-// config, currently from sandbox annotations.
+// getSeccompSecurityOpts gets container seccomp options from container seccomp profile.
 // It is an experimental feature and may be promoted to official runtime api in the future.
-func getSeccompSecurityOpts(containerName string, sandboxConfig *runtimeapi.PodSandboxConfig, seccompProfileRoot string, separator rune) ([]string, error) {
-	seccompOpts, err := getSeccompDockerOpts(sandboxConfig.GetAnnotations(), containerName, seccompProfileRoot)
+func getSeccompSecurityOpts(seccompProfile string, separator rune) ([]string, error) {
+	seccompOpts, err := getSeccompDockerOpts(seccompProfile)
 	if err != nil {
 		return nil, err
 	}

diff --git a/pkg/kubelet/dockershim/helpers_linux_test.go b/pkg/kubelet/dockershim/helpers_linux_test.go
@@ -20,51 +20,32 @@ package dockershim
 
 import (
 	"fmt"
-	"path"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/api/core/v1"
-
-	runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
 )
 
 func TestGetSeccompSecurityOpts(t *testing.T) {
-	containerName := "bar"
-	makeConfig := func(annotations map[string]string) *runtimeapi.PodSandboxConfig {
-		return makeSandboxConfigWithLabelsAndAnnotations("pod", "ns", "1234", 1, nil, annotations)
-	}
-
 	tests := []struct {
-		msg          string
-		config       *runtimeapi.PodSandboxConfig
-		expectedOpts []string
+		msg            string
+		seccompProfile string
+		expectedOpts   []string
 	}{{
-		msg:          "No security annotations",
-		config:       makeConfig(nil),
-		expectedOpts: []string{"seccomp=unconfined"},
+		msg:            "No security annotations",
+		seccompProfile: "",
+		expectedOpts:   []string{"seccomp=unconfined"},
 	}, {
-		msg: "Seccomp unconfined",
-		config: makeConfig(map[string]string{
-			v1.SeccompContainerAnnotationKeyPrefix + containerName: "unconfined",
-		}),
-		expectedOpts: []string{"seccomp=unconfined"},
+		msg:            "Seccomp unconfined",
+		seccompProfile: "unconfined",
+		expectedOpts:   []string{"seccomp=unconfined"},
 	}, {
-		msg: "Seccomp default",
-		config: makeConfig(map[string]string{
-			v1.SeccompContainerAnnotationKeyPrefix + containerName: "docker/default",
-		}),
-		expectedOpts: nil,
-	}, {
-		msg: "Seccomp pod default",
-		config: makeConfig(map[string]string{
-			v1.SeccompPodAnnotationKey: "docker/default",
-		}),
-		expectedOpts: nil,
+		msg:            "Seccomp default",
+		seccompProfile: "docker/default",
+		expectedOpts:   nil,
 	}}
 
 	for i, test := range tests {
-		opts, err := getSeccompSecurityOpts(containerName, test.config, "test/seccomp/profile/root", '=')
+		opts, err := getSeccompSecurityOpts(test.seccompProfile, '=')
 		assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg)
 		assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg)
 		for _, opt := range test.expectedOpts {
@@ -74,42 +55,32 @@ func TestGetSeccompSecurityOpts(t *testing.T) {
 }
 
 func TestLoadSeccompLocalhostProfiles(t *testing.T) {
-	containerName := "bar"
-	makeConfig := func(annotations map[string]string) *runtimeapi.PodSandboxConfig {
-		return makeSandboxConfigWithLabelsAndAnnotations("pod", "ns", "1234", 1, nil, annotations)
-	}
-
 	tests := []struct {
-		msg          string
-		config       *runtimeapi.PodSandboxConfig
-		expectedOpts []string
-		expectErr    bool
+		msg            string
+		seccompProfile string
+		expectedOpts   []string
+		expectErr      bool
 	}{{
 		msg: "Seccomp localhost/test profile",
-		config: makeConfig(map[string]string{
-			v1.SeccompPodAnnotationKey: "localhost/test",
-		}),
-		expectedOpts: []string{`seccomp={"foo":"bar"}`},
-		expectErr:    false,
+		// 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",
+		expectedOpts:   []string{`seccomp={"foo":"bar"}`},
+		expectErr:      false,
 	}, {
-		msg: "Seccomp localhost/sub/subtest profile",
-		config: makeConfig(map[string]string{
-			v1.SeccompPodAnnotationKey: "localhost/sub/subtest",
-		}),
-		expectedOpts: []string{`seccomp={"abc":"def"}`},
-		expectErr:    false,
+		msg:            "Seccomp localhost/sub/subtest profile",
+		seccompProfile: "localhost/fixtures/seccomp/sub/subtest",
+		expectedOpts:   []string{`seccomp={"abc":"def"}`},
+		expectErr:      false,
 	}, {
-		msg: "Seccomp non-existent",
-		config: makeConfig(map[string]string{
-			v1.SeccompPodAnnotationKey: "localhost/non-existent",
-		}),
-		expectedOpts: nil,
-		expectErr:    true,
+		msg:            "Seccomp non-existent",
+		seccompProfile: "localhost/fixtures/seccomp/non-existent",
+		expectedOpts:   nil,
+		expectErr:      true,
 	}}
 
-	profileRoot := path.Join("fixtures", "seccomp")
 	for i, test := range tests {
-		opts, err := getSeccompSecurityOpts(containerName, test.config, profileRoot, '=')
+		opts, err := getSeccompSecurityOpts(test.seccompProfile, '=')
 		if test.expectErr {
 			assert.Error(t, err, fmt.Sprintf("TestCase[%d]: %s", i, test.msg))
 			continue

diff --git a/pkg/kubelet/dockershim/helpers_unsupported.go b/pkg/kubelet/dockershim/helpers_unsupported.go
@@ -29,7 +29,7 @@ func DefaultMemorySwap() int64 {
 	return -1
 }
 
-func (ds *dockerService) getSecurityOpts(containerName string, sandboxConfig *runtimeapi.PodSandboxConfig, separator rune) ([]string, error) {
+func (ds *dockerService) getSecurityOpts(seccompProfile string, separator rune) ([]string, error) {
 	glog.Warningf("getSecurityOpts is unsupported in this build")
 	return nil, nil
 }

diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go
@@ -577,8 +577,8 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration,
 		case kubetypes.DockerContainerRuntime:
 			// Create and start the CRI shim running as a grpc server.
 			streamingConfig := getStreamingConfig(kubeCfg, kubeDeps)
-			ds, err := dockershim.NewDockerService(kubeDeps.DockerClient, kubeCfg.SeccompProfileRoot, crOptions.PodSandboxImage,
-				streamingConfig, &pluginSettings, kubeCfg.RuntimeCgroups, kubeCfg.CgroupDriver, crOptions.DockerExecHandlerName,
+			ds, err := dockershim.NewDockerService(kubeDeps.DockerClient, crOptions.PodSandboxImage, streamingConfig,
+				&pluginSettings, kubeCfg.RuntimeCgroups, kubeCfg.CgroupDriver, crOptions.DockerExecHandlerName,
 				crOptions.DockershimRootDirectory, crOptions.DockerDisableSharedPID)
 			if err != nil {
 				return nil, err
@@ -621,6 +621,7 @@ func NewMainKubelet(kubeCfg *componentconfig.KubeletConfiguration,
 		runtime, err := kuberuntime.NewKubeGenericRuntimeManager(
 			kubecontainer.FilterEventRecorder(kubeDeps.Recorder),
 			klet.livenessManager,
+			kubeCfg.SeccompProfileRoot,
 			containerRefManager,
 			machineInfo,
 			klet.podManager,