github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor based on comments

Browse Source 
Signed-off-by: Mike Brown <brownwm@us.ibm.com>
This commit is contained in:
Mike Brown 2020-12-03 16:25:12 -06:00
parent b4727eafbe
commit 6467c3374d
6 changed files with 147 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
go.mod
View File

@ -61,7 +61,7 @@ require (
k8s.io/apiserver v0.19.4
k8s.io/client-go v0.19.4
k8s.io/component-base v0.19.4
k8s.io/cri-api 3990421b69a024ba452db5f6e4ae7d45d93a019a
k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0
k8s.io/klog/v2 v2.2.0
k8s.io/utils v0.0.0-20200729134348-d5654de09c73
)

4
go.sum
View File

@ -714,8 +714,8 @@ k8s.io/client-go v0.19.4/go.mod h1:ZrEy7+wj9PjH5VMBCuu/BDlvtUAku0oVFk4MmnW9mWA=
k8s.io/component-base v0.19.4 h1:HobPRToQ8KJ9ubRju6PUAk9I5V1GNMJZ4PyWbiWA0uI=
k8s.io/component-base v0.19.4/go.mod h1:ZzuSLlsWhajIDEkKF73j64Gz/5o0AgON08FgRbEPI70=
k8s.io/cri-api v0.17.3/go.mod h1:X1sbHmuXhwaHs9xxYffLqJogVsnI+f6cPRcgPel7ywM=
k8s.io/cri-api v0.19.4 h1:Vc00x5LSSbLBgvj7UAi4kjsv276n4SGX0XlI/pWhG2E=
k8s.io/cri-api v0.19.4/go.mod h1:UN/iU9Ua0iYdDREBXNE9vqCJ7MIh/FW3VIL0d8pw7Fw=
k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0 h1:/AO/xlTAHFP+ZLhfYMjSUzMsZe9of1fwh1TupXAKzKE=
k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0/go.mod h1:UN/iU9Ua0iYdDREBXNE9vqCJ7MIh/FW3VIL0d8pw7Fw=
k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A=

177
pkg/cri/server/container_create_linux.go
View File

@ -306,9 +306,15 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
}
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr))
asp := securityContext.GetApparmor()
if asp == nil {
asp, err = generateApparmorSecurityProfile(securityContext.GetApparmorProfile()) // nolint:staticcheck deprecated but we don't want to remove yet
if err != nil {
return nil, errors.Wrap(err, "failed to generate apparmor spec opts")
}
}
apparmorSpecOpts, err := generateApparmorSpecOpts(
securityContext.GetApparmor(),
securityContext.GetApparmorProfile(), // nolint:staticcheck deprecated but we don't want to remove yet
asp,
securityContext.GetPrivileged(),
c.apparmorEnabled())
if err != nil {
@ -318,9 +324,17 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
specOpts = append(specOpts, apparmorSpecOpts)
}
ssp := securityContext.GetSeccomp()
if ssp == nil {
ssp, err = generateSeccompSecurityProfile(
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
c.config.UnsetSeccompProfile)
if err != nil {
return nil, errors.Wrap(err, "failed to generate seccomp spec opts")
}
}
seccompSpecOpts, err := c.generateSeccompSpecOpts(
securityContext.GetSeccomp(),
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
ssp,
securityContext.GetPrivileged(),
c.seccompEnabled())
if err != nil {
@ -332,24 +346,51 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
return specOpts, nil
}
func generateSeccompSecurityProfile(profilePath string, unsetProfilePath string) (*runtime.SecurityProfile, error) {
if profilePath != "" {
return generateSecurityProfile(profilePath)
}
if unsetProfilePath != "" {
return generateSecurityProfile(unsetProfilePath)
}
return nil, nil
}
func generateApparmorSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) {
if profilePath != "" {
return generateSecurityProfile(profilePath)
}
return nil, nil
}
func generateSecurityProfile(profilePath string) (*runtime.SecurityProfile, error) {
switch profilePath {
case runtimeDefault, dockerDefault, "":
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_RuntimeDefault,
}, nil
case unconfinedProfile:
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_Unconfined,
}, nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(profilePath, profileNamePrefix) {
return nil, errors.Errorf("invalid profile %q", profilePath)
}
return &runtime.SecurityProfile{
ProfileType: runtime.SecurityProfile_Localhost,
LocalhostRef: strings.TrimPrefix(profilePath, profileNamePrefix),
}, nil
}
}
// generateSeccompSpecOpts generates containerd SpecOpts for seccomp.
func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, seccompProf string, privileged, seccompEnabled bool) (oci.SpecOpts, error) {
func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, privileged, seccompEnabled bool) (oci.SpecOpts, error) {
if privileged {
// Do not set seccomp profile when container is privileged
return nil, nil
}
if seccompProf == "" && sp == nil {
seccompProf = c.config.UnsetSeccompProfile
}
// Set seccomp profile
if seccompProf == runtimeDefault || seccompProf == dockerDefault {
// use correct default profile (Eg. if not configured otherwise, the default is docker/default)
seccompProf = seccompDefaultProfile
}
if !seccompEnabled {
if seccompProf != "" && seccompProf != unconfinedProfile {
return nil, errors.New("seccomp is not supported")
}
if sp != nil {
if sp.ProfileType != runtime.SecurityProfile_Unconfined {
return nil, errors.New("seccomp is not supported")
@ -358,49 +399,33 @@ func (c *criService) generateSeccompSpecOpts(sp *runtime.SecurityProfile, seccom
return nil, nil
}
if sp != nil {
if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" {
return nil, errors.New("seccomp config invalid LocalhostRef must only be set if ProfileType is Localhost")
}
switch sp.ProfileType {
case runtime.SecurityProfile_Unconfined:
// Do not set seccomp profile.
return nil, nil
case runtime.SecurityProfile_RuntimeDefault:
return seccomp.WithDefaultProfile(), nil
case runtime.SecurityProfile_Localhost:
// trimming the localhost/ prefix just in case even through it should not
// be necessary with the new SecurityProfile struct
return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil
default:
return nil, errors.New("seccomp unknown ProfileType")
}
if sp == nil {
return nil, nil
}
switch seccompProf {
case "", unconfinedProfile:
if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" {
return nil, errors.New("seccomp config invalid LocalhostRef must only be set if ProfileType is Localhost")
}
switch sp.ProfileType {
case runtime.SecurityProfile_Unconfined:
// Do not set seccomp profile.
return nil, nil
case dockerDefault:
// Note: WithDefaultProfile specOpts must be added after capabilities
case runtime.SecurityProfile_RuntimeDefault:
return seccomp.WithDefaultProfile(), nil
case runtime.SecurityProfile_Localhost:
// trimming the localhost/ prefix just in case even though it should not
// be necessary with the new SecurityProfile struct
return seccomp.WithProfile(strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)), nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(seccompProf, profileNamePrefix) {
return nil, errors.Errorf("invalid seccomp profile %q", seccompProf)
}
return seccomp.WithProfile(strings.TrimPrefix(seccompProf, profileNamePrefix)), nil
return nil, errors.New("seccomp unknown ProfileType")
}
}
// generateApparmorSpecOpts generates containerd SpecOpts for apparmor.
func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string, privileged, apparmorEnabled bool) (oci.SpecOpts, error) {
func generateApparmorSpecOpts(sp *runtime.SecurityProfile, privileged, apparmorEnabled bool) (oci.SpecOpts, error) {
if !apparmorEnabled {
// Should fail loudly if user try to specify apparmor profile
// but we don't support it.
if apparmorProf != "" && apparmorProf != unconfinedProfile {
return nil, errors.New("apparmor is not supported")
}
if sp != nil {
if sp.ProfileType != runtime.SecurityProfile_Unconfined {
return nil, errors.New("apparmor is not supported")
@ -409,55 +434,31 @@ func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string,
return nil, nil
}
if sp != nil {
if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" {
return nil, errors.New("apparmor config invalid LocalhostRef must only be set if ProfileType is Localhost")
}
switch sp.ProfileType {
case runtime.SecurityProfile_Unconfined:
// Do not set apparmor profile.
return nil, nil
case runtime.SecurityProfile_RuntimeDefault:
if privileged {
// Do not set apparmor profile when container is privileged
return nil, nil
}
return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil
case runtime.SecurityProfile_Localhost:
// trimming the localhost/ prefix just in case even through it should not
// be necessary with the new SecurityProfile struct
appArmorProfile := strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)
if profileExists, err := appArmorProfileExists(appArmorProfile); !profileExists {
if err != nil {
return nil, errors.Wrap(err, "failed to generate apparmor spec opts")
}
return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile)
}
return apparmor.WithProfile(appArmorProfile), nil
default:
return nil, errors.New("apparmor unknown ProfileType")
}
if sp == nil {
// Based on kubernetes#51746, default apparmor profile should be applied
// for when apparmor is not specified.
sp, _ = generateSecurityProfile("")
}
switch apparmorProf {
// Based on kubernetes#51746, default apparmor profile should be applied
// for when apparmor is not specified.
case runtimeDefault, "":
if sp.ProfileType != runtime.SecurityProfile_Localhost && sp.LocalhostRef != "" {
return nil, errors.New("apparmor config invalid LocalhostRef must only be set if ProfileType is Localhost")
}
switch sp.ProfileType {
case runtime.SecurityProfile_Unconfined:
// Do not set apparmor profile.
return nil, nil
case runtime.SecurityProfile_RuntimeDefault:
if privileged {
// Do not set apparmor profile when container is privileged
return nil, nil
}
// TODO (mikebrow): delete created apparmor default profile
return apparmor.WithDefaultProfile(appArmorDefaultProfileName), nil
case unconfinedProfile:
return nil, nil
default:
// Require and Trim default profile name prefix
if !strings.HasPrefix(apparmorProf, profileNamePrefix) {
return nil, errors.Errorf("invalid apparmor profile %q", apparmorProf)
}
appArmorProfile := strings.TrimPrefix(apparmorProf, profileNamePrefix)
case runtime.SecurityProfile_Localhost:
// trimming the localhost/ prefix just in case even through it should not
// be necessary with the new SecurityProfile struct
appArmorProfile := strings.TrimPrefix(sp.LocalhostRef, profileNamePrefix)
if profileExists, err := appArmorProfileExists(appArmorProfile); !profileExists {
if err != nil {
return nil, errors.Wrap(err, "failed to generate apparmor spec opts")
@ -465,6 +466,8 @@ func generateApparmorSpecOpts(sp *runtime.SecurityProfile, apparmorProf string,
return nil, errors.Errorf("apparmor profile not found %s", appArmorProfile)
}
return apparmor.WithProfile(appArmorProfile), nil
default:
return nil, errors.New("apparmor unknown ProfileType")
}
}

60
pkg/cri/server/container_create_linux_test.go
View File

@ -832,10 +832,6 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) {
profile: profileNamePrefix + "test-profile",
specOpts: seccomp.WithProfile("test-profile"),
},
"should return error if specified profile is invalid": {
profile: "test-profile",
expectErr: true,
},
"should use default profile when seccomp is empty": {
defaultProfile: profileNamePrefix + "test-profile",
specOpts: seccomp.WithProfile("test-profile"),
@ -903,14 +899,29 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) {
t.Run(fmt.Sprintf("TestCase %q", desc), func(t *testing.T) {
cri := &criService{}
cri.config.UnsetSeccompProfile = test.defaultProfile
specOpts, err := cri.generateSeccompSpecOpts(test.sp, test.profile, test.privileged, !test.disable)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
ssp := test.sp
csp, err := generateSeccompSecurityProfile(
test.profile,
test.defaultProfile)
if err != nil {
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
} else {
assert.NoError(t, err)
if ssp == nil {
ssp = csp
}
specOpts, err := cri.generateSeccompSpecOpts(ssp, test.privileged, !test.disable)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
}
})
}
@ -1039,14 +1050,27 @@ func TestGenerateApparmorSpecOpts(t *testing.T) {
},
} {
t.Logf("TestCase %q", desc)
specOpts, err := generateApparmorSpecOpts(test.sp, test.profile, test.privileged, !test.disable)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
asp := test.sp
csp, err := generateApparmorSecurityProfile(test.profile)
if err != nil {
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
} else {
assert.NoError(t, err)
if asp == nil {
asp = csp
}
specOpts, err := generateApparmorSpecOpts(asp, test.privileged, !test.disable)
assert.Equal(t,
reflect.ValueOf(test.specOpts).Pointer(),
reflect.ValueOf(specOpts).Pointer())
if test.expectErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
}
}
}

13
pkg/cri/server/sandbox_run_linux.go
View File

@ -163,10 +163,19 @@ func (c *criService) sandboxContainerSpecOpts(config *runtime.PodSandboxConfig,
var (
securityContext = config.GetLinux().GetSecurityContext()
specOpts []oci.SpecOpts
err error
)
ssp := securityContext.GetSeccomp()
if ssp == nil {
ssp, err = generateSeccompSecurityProfile(
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
c.config.UnsetSeccompProfile)
if err != nil {
return nil, errors.Wrap(err, "failed to generate seccomp spec opts")
}
}
seccompSpecOpts, err := c.generateSeccompSpecOpts(
securityContext.GetSeccomp(),
securityContext.GetSeccompProfilePath(), // nolint:staticcheck deprecated but we don't want to remove yet
ssp,
securityContext.GetPrivileged(),
c.seccompEnabled())
if err != nil {

2
vendor/modules.txt vendored
View File

@ -501,7 +501,7 @@ k8s.io/client-go/util/workqueue
# k8s.io/component-base v0.19.4
## explicit
k8s.io/component-base/logs/logreduction
# k8s.io/cri-api v0.19.4
# k8s.io/cri-api v0.20.0-beta.1.0.20201105173512-3990421b69a0
## explicit
k8s.io/cri-api/pkg/apis
k8s.io/cri-api/pkg/apis/runtime/v1alpha2