From dd3421c3c7392978ee06ad1047b8371626b30d1b Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Wed, 20 Sep 2017 23:26:22 +0000 Subject: [PATCH] Fix apparmor empty case. Signed-off-by: Lantao Liu --- .travis.yml | 4 ++++ cmd/cri-containerd/options/options.go | 9 +++++++-- hack/test-utils.sh | 5 ++++- pkg/server/container_create.go | 20 ++++++++++++-------- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index aea15291c..ee3a108a4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,6 +42,10 @@ before_script: gcloud auth activate-service-account --key-file gcp-secret.json --project=k8s-cri-containerd; fi +env: + # Travis trusty has disabled apparmor, so disable enable in our test. + - CRI_CONTAINERD_FLAGS="--enable-apparmor=false" + jobs: include: - stage: Build diff --git a/cmd/cri-containerd/options/options.go b/cmd/cri-containerd/options/options.go index a6a1ee40c..d43475090 100644 --- a/cmd/cri-containerd/options/options.go +++ b/cmd/cri-containerd/options/options.go @@ -62,8 +62,11 @@ type Config struct { StreamServerPort string `toml:"stream_server_port"` // CgroupPath is the path for the cgroup that cri-containerd is placed in. CgroupPath string `toml:"cgroup_path"` - // EnableSelinux indicates to enable the selinux support + // EnableSelinux indicates to enable the selinux support. EnableSelinux bool `toml:"enable_selinux"` + // EnableAppArmor indicates to enable apparmor support. cri-containerd will + // apply default apparmor profile if apparmor is enabled. + EnableAppArmor bool `toml:"enable_apparmor"` // SandboxImage is the image used by sandbox container. SandboxImage string `toml:"sandbox_image"` } @@ -109,8 +112,10 @@ func (c *CRIContainerdOptions) AddFlags(fs *pflag.FlagSet) { "10010", "The port streaming server is listening on.") fs.StringVar(&c.CgroupPath, "cgroup-path", "", "The cgroup that cri-containerd is part of. By default cri-containerd is not placed in a cgroup.") - fs.BoolVar(&c.EnableSelinux, "selinux-enabled", + fs.BoolVar(&c.EnableSelinux, "enable-selinux", false, "Enable selinux support.") + fs.BoolVar(&c.EnableAppArmor, "enable-apparmor", + true, "Enable apparmor support. cri-containerd will apply default apparmor profile when apparmor is enabled.") fs.StringVar(&c.SandboxImage, "sandbox-image", "gcr.io/google_containers/pause:3.0", "The image used by sandbox container.") fs.BoolVar(&c.PrintDefaultConfig, "default-config", diff --git a/hack/test-utils.sh b/hack/test-utils.sh index a3a58cb33..231699273 100644 --- a/hack/test-utils.sh +++ b/hack/test-utils.sh @@ -16,6 +16,8 @@ ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"/.. . ${ROOT}/hack/versions +# CRI_CONTAINERD_FLAGS are the extra flags to use when start cri-containerd. +CRI_CONTAINERD_FLAGS=${CRI_CONTAINERD_FLAGS:-""} # start_cri_containerd starts containerd and cri-containerd. start_cri_containerd() { @@ -44,7 +46,8 @@ start_cri_containerd() { done # Start cri-containerd - sudo ${ROOT}/_output/cri-containerd --alsologtostderr --v 4 &> ${report_dir}/cri-containerd.log & + sudo ${ROOT}/_output/cri-containerd --alsologtostderr --v 4 ${CRI_CONTAINERD_FLAGS} \ + &> ${report_dir}/cri-containerd.log & } # kill_cri_containerd kills containerd and cri-containerd. diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 346014127..8a4760d47 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -51,8 +51,6 @@ const ( runtimeDefault = "runtime/default" // appArmorDefaultProfileName is name to use when creating a default apparmor profile. appArmorDefaultProfileName = "cri-containerd.apparmor.d" - // appArmorEnabled is a flag for globally enabling/disabling apparmor profiles for containers. - appArmorEnabled = true // TODO (mikebrow): make these apparmor defaults configurable ) // CreateContainer creates a new container in the given PodSandbox. @@ -178,24 +176,30 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C } var specOpts []containerd.SpecOpts + securityContext := config.GetLinux().GetSecurityContext() // Set container username. This could only be done by containerd, because it needs // access to the container rootfs. Pass user name to containerd, and let it overwrite // the spec for us. - if uid := config.GetLinux().GetSecurityContext().GetRunAsUser(); uid != nil { + if uid := securityContext.GetRunAsUser(); uid != nil { specOpts = append(specOpts, containerd.WithUserID(uint32(uid.GetValue()))) } - if username := config.GetLinux().GetSecurityContext().GetRunAsUsername(); username != "" { + if username := securityContext.GetRunAsUsername(); username != "" { specOpts = append(specOpts, containerd.WithUsername(username)) } // Set apparmor profile, (privileged or not) if apparmor is enabled - if appArmorEnabled { - appArmorProf := config.GetLinux().GetSecurityContext().GetApparmorProfile() + if c.config.EnableAppArmor { + appArmorProf := securityContext.GetApparmorProfile() switch appArmorProf { case runtimeDefault: // TODO (mikebrow): delete created apparmor default profile specOpts = append(specOpts, apparmor.WithDefaultProfile(appArmorDefaultProfileName)) + // TODO(random-liu): Should support "unconfined" after kubernetes#52395 lands. case "": - // TODO (mikebrow): handle no apparmor profile case see kubernetes/kubernetes#51746 + // Based on kubernetes#51746, default apparmor profile should be applied + // for non-privileged container when apparmor is not specified. + if !securityContext.GetPrivileged() { + specOpts = append(specOpts, apparmor.WithDefaultProfile(appArmorDefaultProfileName)) + } default: // Require and Trim default profile name prefix if !strings.HasPrefix(appArmorProf, profileNamePrefix) { @@ -298,7 +302,7 @@ func (c *criContainerdService) generateContainerSpec(id string, sandboxPid uint3 } if securityContext.GetPrivileged() { - if !sandboxConfig.GetLinux().GetSecurityContext().GetPrivileged() { + if !securityContext.GetPrivileged() { return nil, fmt.Errorf("no privileged container allowed in sandbox") } if err := setOCIPrivileged(&g, config); err != nil {