Merge pull request #26936 from yifan-gu/fix_selinux

Automatic merge from submit-queue

rkt: Fix incomplete selinux context string when the option is partial.

Fix "EmptyDir" e2e tests failures caused by #https://github.com/kubernetes/kubernetes/pull/24901

As mentioned in https://github.com/kubernetes/kubernetes/pull/24901#discussion_r61372312
We should apply the selinux context of the rkt data directory (/var/lib/rkt) when users do not specify all the selinux options.

Due to my fault, the change was missed during rebase, thus caused the regression.

After applying this PR, the e2e tests passed.
```
$ go run hack/e2e.go -v -test --test_args="--ginkgo.dryRun=false --ginkgo.focus=EmptyDir"
...
Ran 19 of 313 Specs in 199.319 seconds
SUCCESS! -- 19 Passed | 0 Failed | 0 Pending | 294 Skipped PASS
```

BTW, the test is removed because the `--no-overlay=true` flag will only be there on non-coreos distro.

cc @euank @kubernetes/sig-node

pkg/kubelet/kubelet.go

@@ -1209,7 +1209,7 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap)
 		return err
 	}
 
-	chconRunner := selinux.NewChconRunner()
+	selinuxRunner := selinux.NewSelinuxContextRunner()
 	// Apply the pod's Level to the rootDirContext
 	rootDirSELinuxOptions, err := securitycontext.ParseSELinuxOptions(rootDirContext)
 	if err != nil {
@@ -1226,7 +1226,7 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap)
 				if err != nil {
 					return err
 				}
-				return chconRunner.SetContext(path, volumeContext)
+				return selinuxRunner.SetContext(path, volumeContext)
 			})
 			if err != nil {
 				return err

pkg/kubelet/rkt/rkt.go

@@ -58,6 +58,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/errors"
 	utilexec "k8s.io/kubernetes/pkg/util/exec"
 	"k8s.io/kubernetes/pkg/util/flowcontrol"
+	"k8s.io/kubernetes/pkg/util/selinux"
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	utilwait "k8s.io/kubernetes/pkg/util/wait"
 )
@@ -1002,6 +1003,34 @@ func (r *Runtime) preparePodArgs(manifest *appcschema.PodManifest, manifestFileN
 	return cmds
 }
 
+func (r *Runtime) getSelinuxContext(opt *api.SELinuxOptions) (string, error) {
+	selinuxRunner := selinux.NewSelinuxContextRunner()
+	str, err := selinuxRunner.Getfilecon(r.config.Dir)
+	if err != nil {
+		return "", err
+	}
+
+	ctx := strings.SplitN(str, ":", 4)
+	if len(ctx) != 4 {
+		return "", fmt.Errorf("malformated selinux context")
+	}
+
+	if opt.User != "" {
+		ctx[0] = opt.User
+	}
+	if opt.Role != "" {
+		ctx[1] = opt.Role
+	}
+	if opt.Type != "" {
+		ctx[2] = opt.Type
+	}
+	if opt.Level != "" {
+		ctx[3] = opt.Level
+	}
+
+	return strings.Join(ctx, ":"), nil
+}
+
 // preparePod will:
 //
 // 1. Invoke 'rkt prepare' to prepare the pod, and get the rkt pod uuid.
@@ -1073,7 +1102,11 @@ func (r *Runtime) preparePod(pod *api.Pod, podIP string, pullSecrets []api.Secre
 
 	if pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SELinuxOptions != nil {
 		opt := pod.Spec.SecurityContext.SELinuxOptions
-		selinuxContext := fmt.Sprintf("%s:%s:%s:%s", opt.User, opt.Role, opt.Type, opt.Level)
+		selinuxContext, err := r.getSelinuxContext(opt)
+		if err != nil {
+			glog.Errorf("rkt: Failed to construct selinux context with selinux option %q: %v", opt, err)
+			return "", nil, err
+		}
 		units = append(units, newUnitOption("Service", "SELinuxContext", selinuxContext))
 	}
 

pkg/kubelet/rkt/rkt_test.go

@@ -1172,27 +1172,6 @@ func TestGenerateRunCommand(t *testing.T) {
 			nil,
 			fmt.Sprintf("/bin/rkt/rkt --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --net=host --hostname=%s rkt-uuid-foo", hostName),
 		},
-		// Case #5, returns --net=host --no-overlay
-		{
-			&api.Pod{
-				ObjectMeta: api.ObjectMeta{
-					Name: "pod-name-foo",
-				},
-				Spec: api.PodSpec{
-					SecurityContext: &api.PodSecurityContext{
-						HostNetwork:    true,
-						SELinuxOptions: &api.SELinuxOptions{},
-					},
-				},
-			},
-			"rkt-uuid-foo",
-			"",
-			[]string{""},
-			[]string{""},
-			"pod-hostname-foo",
-			nil,
-			fmt.Sprintf("/bin/rkt/rkt --insecure-options=image,ondisk --local-config=/var/rkt/local/data --dir=/var/data run-prepared --no-overlay=true --net=host --hostname=%s rkt-uuid-foo", hostName),
-		},
 	}
 
 	rkt := &Runtime{

pkg/util/selinux/selinux.go

@@ -16,12 +16,14 @@ limitations under the License.
 
 package selinux
 
-// chconRunner knows how to chcon a directory.
+// SelinuxContextRunner knows how to chcon of a directory and
-type ChconRunner interface {
+// how to get the selinux context of a file.
+type SelinuxContextRunner interface {
 	SetContext(dir, context string) error
+	Getfilecon(path string) (string, error)
 }
 
-// newChconRunner returns a new chconRunner.
+// NewSelinuxContextRunner returns a new chconRunner.
-func NewChconRunner() ChconRunner {
+func NewSelinuxContextRunner() SelinuxContextRunner {
-	return &realChconRunner{}
+	return &realSelinuxContextRunner{}
 }

pkg/util/selinux/selinux_linux.go

@@ -19,12 +19,14 @@ limitations under the License.
 package selinux
 
 import (
+	"fmt"
+
 	"github.com/opencontainers/runc/libcontainer/selinux"
 )
 
-type realChconRunner struct{}
+type realSelinuxContextRunner struct{}
 
-func (_ *realChconRunner) SetContext(dir, context string) error {
+func (_ *realSelinuxContextRunner) SetContext(dir, context string) error {
 	// If SELinux is not enabled, return an empty string
 	if !selinux.SelinuxEnabled() {
 		return nil
@@ -32,3 +34,10 @@ func (_ *realChconRunner) SetContext(dir, context string) error {
 
 	return selinux.Setfilecon(dir, context)
 }
+
+func (_ *realSelinuxContextRunner) Getfilecon(path string) (string, error) {
+	if !selinux.SelinuxEnabled() {
+		return "", fmt.Errorf("SELinux is not enabled")
+	}
+	return selinux.Getfilecon(path)
+}

pkg/util/selinux/selinux_unsupported.go

@@ -18,9 +18,14 @@ limitations under the License.
 
 package selinux
 
-type realChconRunner struct{}
+type realSelinuxContextRunner struct{}
 
-func (_ *realChconRunner) SetContext(dir, context string) error {
+func (_ *realSelinuxContextRunner) SetContext(dir, context string) error {
 	// NOP
 	return nil
 }
+
+func (_ *realSelinuxContextRunner) Getfilecon(path string) (string, error) {
+	// NOP
+	return "", nil
+}