diff --git a/pkg/kubelet/dockershim/security_context.go b/pkg/kubelet/dockershim/security_context.go index 9e52131d84f..76000c76bf3 100644 --- a/pkg/kubelet/dockershim/security_context.go +++ b/pkg/kubelet/dockershim/security_context.go @@ -24,6 +24,7 @@ import ( "k8s.io/kubernetes/pkg/api/v1" runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + "k8s.io/kubernetes/pkg/kubelet/dockertools" "k8s.io/kubernetes/pkg/kubelet/dockertools/securitycontext" "k8s.io/kubernetes/pkg/kubelet/network" ) @@ -103,6 +104,7 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig * Type: sc.SelinuxOptions.Type, Level: sc.SelinuxOptions.Level, }, + dockertools.SecurityOptSeparatorNew, ) } } diff --git a/pkg/kubelet/dockershim/security_context_test.go b/pkg/kubelet/dockershim/security_context_test.go index 386810859d8..fe0120b5b30 100644 --- a/pkg/kubelet/dockershim/security_context_test.go +++ b/pkg/kubelet/dockershim/security_context_test.go @@ -82,10 +82,10 @@ func TestModifyHostConfig(t *testing.T) { } setSELinuxHC := &dockercontainer.HostConfig{ SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser, "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole, "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType, "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel, "level"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), }, } @@ -181,10 +181,10 @@ func TestModifyHostConfigAndNamespaceOptionsForContainer(t *testing.T) { } setSELinuxHC := &dockercontainer.HostConfig{ SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser, "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole, "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType, "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel, "level"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), }, IpcMode: dockercontainer.IpcMode(sandboxNSMode), NetworkMode: dockercontainer.NetworkMode(sandboxNSMode), @@ -351,10 +351,10 @@ func fullValidHostConfig() *dockercontainer.HostConfig { CapAdd: []string{"addCapA", "addCapB"}, CapDrop: []string{"dropCapA", "dropCapB"}, SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser, "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole, "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType, "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel, "level"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), + fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), }, } } diff --git a/pkg/kubelet/dockertools/BUILD b/pkg/kubelet/dockertools/BUILD index ec211366a81..3bf5402ad5c 100644 --- a/pkg/kubelet/dockertools/BUILD +++ b/pkg/kubelet/dockertools/BUILD @@ -129,6 +129,7 @@ go_test( "//vendor:github.com/golang/mock/gomock", "//vendor:github.com/google/cadvisor/info/v1", "//vendor:github.com/stretchr/testify/assert", + "//vendor:github.com/stretchr/testify/require", "//vendor:k8s.io/apimachinery/pkg/api/equality", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/runtime", diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index adce8f9af13..7780fc88715 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -108,6 +108,11 @@ const ( // The expiration time of version cache. versionCacheTTL = 60 * time.Second + + // Docker changed the API for specifying options in v1.11 + SecurityOptSeparatorChangeVersion = "1.23" // Corresponds to docker 1.11.x + SecurityOptSeparatorOld = ':' + SecurityOptSeparatorNew = '=' ) var ( @@ -631,10 +636,11 @@ func (dm *DockerManager) runContainer( if err != nil { return kubecontainer.ContainerID{}, err } - fmtSecurityOpts, err := dm.fmtDockerOpts(securityOpts) + optSeparator, err := dm.getDockerOptSeparator() if err != nil { return kubecontainer.ContainerID{}, err } + fmtSecurityOpts := FmtDockerOpts(securityOpts, optSeparator) // Pod information is recorded on the container as labels to preserve it in the event the pod is deleted // while the Kubelet is down and there is no information available to recover the pod. @@ -813,7 +819,7 @@ func (dm *DockerManager) runContainer( glog.V(3).Infof("Container %v/%v/%v: setting entrypoint \"%v\" and command \"%v\"", pod.Namespace, pod.Name, container.Name, dockerOpts.Config.Entrypoint, dockerOpts.Config.Cmd) supplementalGids := dm.runtimeHelper.GetExtraSupplementalGroupsForPod(pod) - securityContextProvider := dockersecurity.NewSimpleSecurityContextProvider() + securityContextProvider := dockersecurity.NewSimpleSecurityContextProvider(optSeparator) securityContextProvider.ModifyContainerConfig(pod, container, dockerOpts.Config) securityContextProvider.ModifyHostConfig(pod, container, dockerOpts.HostConfig, supplementalGids) createResp, err := dm.client.CreateContainer(dockerOpts) @@ -1157,31 +1163,23 @@ func (dm *DockerManager) checkVersionCompatibility() error { return nil } -func (dm *DockerManager) fmtDockerOpts(opts []dockerOpt) ([]string, error) { - version, err := dm.APIVersion() - if err != nil { - return nil, err - } - - const ( - // Docker changed the API for specifying options in v1.11 - optSeparatorChangeVersion = "1.23" // Corresponds to docker 1.11.x - optSeparatorOld = ':' - optSeparatorNew = '=' - ) - - sep := optSeparatorNew - if result, err := version.Compare(optSeparatorChangeVersion); err != nil { - return nil, fmt.Errorf("error parsing docker API version: %v", err) +func (dm *DockerManager) getDockerOptSeparator() (rune, error) { + sep := SecurityOptSeparatorNew + if result, err := dm.checkDockerAPIVersion(SecurityOptSeparatorChangeVersion); err != nil { + return sep, err } else if result < 0 { - sep = optSeparatorOld + sep = SecurityOptSeparatorOld } + return sep, nil +} +// FmtDockerOpts formats the docker security options using the given separator. +func FmtDockerOpts(opts []dockerOpt, sep rune) []string { fmtOpts := make([]string, len(opts)) for i, opt := range opts { fmtOpts[i] = fmt.Sprintf("%s%c%s", opt.key, sep, opt.value) } - return fmtOpts, nil + return fmtOpts } type dockerOpt struct { diff --git a/pkg/kubelet/dockertools/docker_manager_linux_test.go b/pkg/kubelet/dockertools/docker_manager_linux_test.go index cb975afb36d..0c61496281c 100644 --- a/pkg/kubelet/dockertools/docker_manager_linux_test.go +++ b/pkg/kubelet/dockertools/docker_manager_linux_test.go @@ -89,8 +89,7 @@ func TestGetSecurityOpts(t *testing.T) { for i, test := range tests { securityOpts, err := dm.getSecurityOpts(test.pod, containerName) assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg) - opts, err := dm.fmtDockerOpts(securityOpts) - assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg) + opts := FmtDockerOpts(securityOpts, '=') assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg) for _, opt := range test.expectedOpts { assert.Contains(t, opts, opt, "TestCase[%d]: %s", i, test.msg) diff --git a/pkg/kubelet/dockertools/docker_manager_test.go b/pkg/kubelet/dockertools/docker_manager_test.go index 958da658f4a..f3f484bf291 100644 --- a/pkg/kubelet/dockertools/docker_manager_test.go +++ b/pkg/kubelet/dockertools/docker_manager_test.go @@ -37,6 +37,7 @@ import ( "github.com/golang/mock/gomock" cadvisorapi "github.com/google/cadvisor/info/v1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -1644,27 +1645,29 @@ func verifySyncResults(t *testing.T, expectedResults []*kubecontainer.SyncResult } } } - -func TestSecurityOptsOperator(t *testing.T) { +func TestGetDockerOptSeparator(t *testing.T) { dm110, _ := newTestDockerManagerWithVersion("1.10.1", "1.22") dm111, _ := newTestDockerManagerWithVersion("1.11.0", "1.23") - secOpts := []dockerOpt{{"seccomp", "unconfined", ""}} - opts, err := dm110.fmtDockerOpts(secOpts) - if err != nil { - t.Fatalf("error getting security opts for Docker 1.10: %v", err) - } - if expected := []string{"seccomp:unconfined"}; len(opts) != 1 || opts[0] != expected[0] { - t.Fatalf("security opts for Docker 1.10: expected %v, got: %v", expected, opts) - } + sep, err := dm110.getDockerOptSeparator() + require.NoError(t, err, "error getting docker opt separator for 1.10.1") + assert.Equal(t, SecurityOptSeparatorOld, sep, "security opt separator for docker 1.10") - opts, err = dm111.fmtDockerOpts(secOpts) - if err != nil { - t.Fatalf("error getting security opts for Docker 1.11: %v", err) - } - if expected := []string{"seccomp=unconfined"}; len(opts) != 1 || opts[0] != expected[0] { - t.Fatalf("security opts for Docker 1.11: expected %v, got: %v", expected, opts) - } + sep, err = dm111.getDockerOptSeparator() + require.NoError(t, err, "error getting docker opt separator for 1.11.1") + assert.Equal(t, SecurityOptSeparatorNew, sep, "security opt separator for docker 1.11") +} + +func TestFmtDockerOpts(t *testing.T) { + secOpts := []dockerOpt{{"seccomp", "unconfined", ""}} + + opts := FmtDockerOpts(secOpts, ':') + assert.Len(t, opts, 1) + assert.Contains(t, opts, "seccomp:unconfined", "Docker 1.10") + + opts = FmtDockerOpts(secOpts, '=') + assert.Len(t, opts, 1) + assert.Contains(t, opts, "seccomp=unconfined", "Docker 1.11") } func TestCheckVersionCompatibility(t *testing.T) { diff --git a/pkg/kubelet/dockertools/securitycontext/BUILD b/pkg/kubelet/dockertools/securitycontext/BUILD index cf85d15d6c4..dae0d55e915 100644 --- a/pkg/kubelet/dockertools/securitycontext/BUILD +++ b/pkg/kubelet/dockertools/securitycontext/BUILD @@ -15,6 +15,7 @@ go_library( "fake.go", "provider.go", "types.go", + "util.go", ], tags = ["automanaged"], deps = [ diff --git a/pkg/kubelet/dockertools/securitycontext/provider.go b/pkg/kubelet/dockertools/securitycontext/provider.go index c7a9201e982..66493a615c1 100644 --- a/pkg/kubelet/dockertools/securitycontext/provider.go +++ b/pkg/kubelet/dockertools/securitycontext/provider.go @@ -29,12 +29,14 @@ import ( ) // NewSimpleSecurityContextProvider creates a new SimpleSecurityContextProvider. -func NewSimpleSecurityContextProvider() SecurityContextProvider { - return SimpleSecurityContextProvider{} +func NewSimpleSecurityContextProvider(securityOptSeparator rune) SecurityContextProvider { + return SimpleSecurityContextProvider{securityOptSeparator} } // SimpleSecurityContextProvider is the default implementation of a SecurityContextProvider. -type SimpleSecurityContextProvider struct{} +type SimpleSecurityContextProvider struct { + securityOptSeparator rune +} // ModifyContainerConfig is called before the Docker createContainer call. // The security context provider can make changes to the Config with which @@ -93,25 +95,37 @@ func (p SimpleSecurityContextProvider) ModifyHostConfig(pod *v1.Pod, container * } if effectiveSC.SELinuxOptions != nil { - hostConfig.SecurityOpt = ModifySecurityOptions(hostConfig.SecurityOpt, effectiveSC.SELinuxOptions) + hostConfig.SecurityOpt = ModifySecurityOptions(hostConfig.SecurityOpt, effectiveSC.SELinuxOptions, p.securityOptSeparator) } } -// ModifySecurityOptions adds SELinux options to config. -func ModifySecurityOptions(config []string, selinuxOpts *v1.SELinuxOptions) []string { - config = modifySecurityOption(config, DockerLabelUser, selinuxOpts.User) - config = modifySecurityOption(config, DockerLabelRole, selinuxOpts.Role) - config = modifySecurityOption(config, DockerLabelType, selinuxOpts.Type) - config = modifySecurityOption(config, DockerLabelLevel, selinuxOpts.Level) +// ModifySecurityOptions adds SELinux options to config using the given +// separator. +func ModifySecurityOptions(config []string, selinuxOpts *v1.SELinuxOptions, separator rune) []string { + // Note, strictly speaking, we are actually mutating the values of these + // keys, rather than formatting name and value into a string. Docker re- + // uses the same option name multiple times (it's just 'label') with + // different values which are themselves key-value pairs. For example, + // the SELinux type is represented by the security opt: + // + // labeltype: + // + // In Docker API versions before 1.23, the separator was the `:` rune; in + // API version 1.23 it changed to the `=` rune. + config = modifySecurityOption(config, DockerLabelUser(separator), selinuxOpts.User) + config = modifySecurityOption(config, DockerLabelRole(separator), selinuxOpts.Role) + config = modifySecurityOption(config, DockerLabelType(separator), selinuxOpts.Type) + config = modifySecurityOption(config, DockerLabelLevel(separator), selinuxOpts.Level) return config } -// modifySecurityOption adds the security option of name to the config array with value in the form -// of name:value +// modifySecurityOption adds the security option of name to the config array +// with value in the form of name:value. func modifySecurityOption(config []string, name, value string) []string { if len(value) > 0 { config = append(config, fmt.Sprintf("%s:%s", name, value)) } + return config } diff --git a/pkg/kubelet/dockertools/securitycontext/provider_test.go b/pkg/kubelet/dockertools/securitycontext/provider_test.go index 2f848ee42a4..3e271d90b17 100644 --- a/pkg/kubelet/dockertools/securitycontext/provider_test.go +++ b/pkg/kubelet/dockertools/securitycontext/provider_test.go @@ -74,7 +74,7 @@ func TestModifyContainerConfig(t *testing.T) { }, } - provider := NewSimpleSecurityContextProvider() + provider := NewSimpleSecurityContextProvider('=') dummyContainer := &v1.Container{} for _, tc := range cases { pod := &v1.Pod{Spec: v1.PodSpec{SecurityContext: tc.podSc}} @@ -104,10 +104,10 @@ func TestModifyHostConfig(t *testing.T) { setSELinuxHC := &dockercontainer.HostConfig{} setSELinuxHC.SecurityOpt = []string{ - fmt.Sprintf("%s:%s", DockerLabelUser, "user"), - fmt.Sprintf("%s:%s", DockerLabelRole, "role"), - fmt.Sprintf("%s:%s", DockerLabelType, "type"), - fmt.Sprintf("%s:%s", DockerLabelLevel, "level"), + fmt.Sprintf("%s:%s", DockerLabelUser(':'), "user"), + fmt.Sprintf("%s:%s", DockerLabelRole(':'), "role"), + fmt.Sprintf("%s:%s", DockerLabelType(':'), "type"), + fmt.Sprintf("%s:%s", DockerLabelLevel(':'), "level"), } // seLinuxLabelsSC := fullValidSecurityContext() @@ -158,7 +158,7 @@ func TestModifyHostConfig(t *testing.T) { }, } - provider := NewSimpleSecurityContextProvider() + provider := NewSimpleSecurityContextProvider(':') dummyContainer := &v1.Container{} for _, tc := range cases { @@ -228,7 +228,7 @@ func TestModifyHostConfigPodSecurityContext(t *testing.T) { }, } - provider := NewSimpleSecurityContextProvider() + provider := NewSimpleSecurityContextProvider(':') dummyContainer := &v1.Container{} dummyContainer.SecurityContext = fullValidSecurityContext() dummyPod := &v1.Pod{ @@ -325,10 +325,10 @@ func fullValidHostConfig() *dockercontainer.HostConfig { CapAdd: []string{"addCapA", "addCapB"}, CapDrop: []string{"dropCapA", "dropCapB"}, SecurityOpt: []string{ - fmt.Sprintf("%s:%s", DockerLabelUser, "user"), - fmt.Sprintf("%s:%s", DockerLabelRole, "role"), - fmt.Sprintf("%s:%s", DockerLabelType, "type"), - fmt.Sprintf("%s:%s", DockerLabelLevel, "level"), + fmt.Sprintf("%s:%s", DockerLabelUser(':'), "user"), + fmt.Sprintf("%s:%s", DockerLabelRole(':'), "role"), + fmt.Sprintf("%s:%s", DockerLabelType(':'), "type"), + fmt.Sprintf("%s:%s", DockerLabelLevel(':'), "level"), }, } } diff --git a/pkg/kubelet/dockertools/securitycontext/types.go b/pkg/kubelet/dockertools/securitycontext/types.go index fba98398a52..4d31e4f9ae9 100644 --- a/pkg/kubelet/dockertools/securitycontext/types.go +++ b/pkg/kubelet/dockertools/securitycontext/types.go @@ -39,11 +39,3 @@ type SecurityContextProvider interface { // - supplementalGids: additional supplemental GIDs associated with the pod's volumes ModifyHostConfig(pod *v1.Pod, container *v1.Container, hostConfig *dockercontainer.HostConfig, supplementalGids []int64) } - -const ( - DockerLabelUser string = "label:user" - DockerLabelRole string = "label:role" - DockerLabelType string = "label:type" - DockerLabelLevel string = "label:level" - DockerLabelDisable string = "label:disable" -) diff --git a/pkg/kubelet/dockertools/securitycontext/util.go b/pkg/kubelet/dockertools/securitycontext/util.go new file mode 100644 index 00000000000..faf6ed53925 --- /dev/null +++ b/pkg/kubelet/dockertools/securitycontext/util.go @@ -0,0 +1,59 @@ +/* +Copyright 2017 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 securitycontext + +import ( + "fmt" +) + +// DockerLabelUser returns the fragment of a Docker security opt that +// describes the SELinux user. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func DockerLabelUser(separator rune) string { + return fmt.Sprintf("label%cuser", separator) +} + +// DockerLabelRole returns the fragment of a Docker security opt that +// describes the SELinux role. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func DockerLabelRole(separator rune) string { + return fmt.Sprintf("label%crole", separator) +} + +// DockerLabelType returns the fragment of a Docker security opt that +// describes the SELinux type. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func DockerLabelType(separator rune) string { + return fmt.Sprintf("label%ctype", separator) +} + +// DockerLabelLevel returns the fragment of a Docker security opt that +// describes the SELinux level. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func DockerLabelLevel(separator rune) string { + return fmt.Sprintf("label%clevel", separator) +} + +// DockerLaelDisable returns the Docker security opt that disables SELinux for +// the container. +func DockerLabelDisable(separator rune) string { + return fmt.Sprintf("label%cdisable", separator) +}