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

Merge pull request #9737 from everpeace/kep-3169-SupplementalGroupsPolicy

Browse Source 
KEP-3619: Fine-grained SupplementalGroups control
This commit is contained in:
Mike Brown
2024-06-13 16:59:20 +00:00
committed by GitHub
parent c75f6d404d 8bcffa9446
commit 98ebb3ecff
18 changed files with 1762 additions and 635 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
internal/cri/server/container_create_linux.go
View File

@@ -65,8 +65,23 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
} else if imageConfig.User != "" {
userstr, _, _ = strings.Cut(imageConfig.User, ":")
}
specOpts = append(specOpts, customopts.WithAdditionalGIDs(userstr),
customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()))
switch securityContext.GetSupplementalGroupsPolicy() {
case runtime.SupplementalGroupsPolicy_Merge:
// merging group defined in /etc/passwd
// and SupplementalGroups defined in security context
specOpts = append(specOpts,
customopts.WithAdditionalGIDs(userstr),
customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()),
)
case runtime.SupplementalGroupsPolicy_Strict:
// no merging group defined in /etc/passwd
specOpts = append(specOpts,
customopts.WithSupplementalGroups(securityContext.GetSupplementalGroups()),
)
default:
return nil, fmt.Errorf("not implemented in this containerd release: SupplementalGroupsPolicy=%d", securityContext.GetSupplementalGroupsPolicy())
}
asp := securityContext.GetApparmor()
if asp == nil {

37
internal/cri/server/container_create_linux_test.go
View File

@@ -1468,7 +1468,7 @@ additional-group-for-root:x:22222:root
expected runtimespec.User
}{
{
desc: "Only SecurityContext was set, SecurityContext defines User",
desc: "[SupplementalGroupsPolicy=Merge(default)] Only SecurityContext was set, SecurityContext defines User",
securityContext: &runtime.LinuxContainerSecurityContext{
RunAsUser: &runtime.Int64Value{Value: 1000},
RunAsGroup: &runtime.Int64Value{Value: 2000},
@@ -1477,13 +1477,13 @@ additional-group-for-root:x:22222:root
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
},
{
desc: "Only imageConfig.User was set, imageConfig.User defines User",
desc: "[SupplementalGroupsPolicy=Merge(default)] Only imageConfig.User was set, imageConfig.User defines User",
imageConfigUser: "1000",
securityContext: nil,
expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}},
},
{
desc: "Both SecurityContext and ImageConfig.User was set, SecurityContext defines User",
desc: "[SupplementalGroupsPolicy=Merge(default)] Both SecurityContext and ImageConfig.User were set, SecurityContext defines User",
imageConfigUser: "0",
securityContext: &runtime.LinuxContainerSecurityContext{
RunAsUser: &runtime.Int64Value{Value: 1000},
@@ -1493,9 +1493,38 @@ additional-group-for-root:x:22222:root
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}},
},
{
desc: "No SecurityContext nor ImageConfig.User were set, runtime default defines User",
desc: "[SupplementalGroupsPolicy=Merge(default)] No SecurityContext nor ImageConfig.User were set, runtime default defines User",
expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}},
},
{
desc: "[SupplementalGroupsPolicy=Strict] Only SecurityContext was set, SecurityContext defines User",
securityContext: &runtime.LinuxContainerSecurityContext{
RunAsUser: &runtime.Int64Value{Value: 1000},
RunAsGroup: &runtime.Int64Value{Value: 2000},
SupplementalGroups: []int64{3333},
SupplementalGroupsPolicy: runtime.SupplementalGroupsPolicy_Strict,
},
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333}},
},
{
desc: "[SupplementalGroupsPolicy=Strict] Only imageConfig.User was set, imageConfig.User defines User",
imageConfigUser: "1000",
securityContext: &runtime.LinuxContainerSecurityContext{
SupplementalGroupsPolicy: runtime.SupplementalGroupsPolicy_Strict,
},
expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000}},
},
{
desc: "[SupplementalGroupsPolicy=Strict] Both SecurityContext and ImageConfig.User were set, SecurityContext defines User",
imageConfigUser: "0",
securityContext: &runtime.LinuxContainerSecurityContext{
RunAsUser: &runtime.Int64Value{Value: 1000},
RunAsGroup: &runtime.Int64Value{Value: 2000},
SupplementalGroups: []int64{3333},
SupplementalGroupsPolicy: runtime.SupplementalGroupsPolicy_Strict,
},
expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333}},
},
} {
test := test
t.Run(test.desc, func(t *testing.T) {

17
internal/cri/server/container_status.go
View File

@@ -24,6 +24,7 @@ import (
containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
"github.com/containerd/containerd/v2/internal/cri/util"
"github.com/containerd/errdefs"
"github.com/containerd/log"
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
@@ -60,7 +61,10 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
imageRef = repoDigests[0]
}
}
status := toCRIContainerStatus(container, spec, imageRef)
status, err := toCRIContainerStatus(ctx, container, spec, imageRef)
if err != nil {
return nil, fmt.Errorf("failed to get ContainerStatus: %w", err)
}
if status.GetCreatedAt() == 0 {
// CRI doesn't allow CreatedAt == 0.
info, err := container.Container.Info(ctx)
@@ -82,7 +86,7 @@ func (c *criService) ContainerStatus(ctx context.Context, r *runtime.ContainerSt
}
// toCRIContainerStatus converts internal container object to CRI container status.
func toCRIContainerStatus(container containerstore.Container, spec *runtime.ImageSpec, imageRef string) *runtime.ContainerStatus {
func toCRIContainerStatus(ctx context.Context, container containerstore.Container, spec *runtime.ImageSpec, imageRef string) (*runtime.ContainerStatus, error) {
meta := container.Metadata
status := container.Status.Get()
reason := status.Reason
@@ -104,6 +108,12 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
st, ft = status.StartedAt, status.FinishedAt
}
runtimeUser, err := toCRIContainerUser(ctx, container)
if err != nil {
log.G(ctx).WithField("Id", meta.ID).WithError(err).Debug("failed to get ContainerUser. returning an empty ContainerUser")
runtimeUser = &runtime.ContainerUser{}
}
return &runtime.ContainerStatus{
Id: meta.ID,
Metadata: meta.Config.GetMetadata(),
@@ -121,7 +131,8 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
Mounts: meta.Config.GetMounts(),
LogPath: meta.LogPath,
Resources: status.Resources,
}
User: runtimeUser,
}, nil
}
// ContainerInfo is extra information for a container.

54
internal/cri/server/container_status_linux.go Normal file
View File

@@ -0,0 +1,54 @@
/*
Copyright The containerd 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 server
import (
"context"
"errors"
"fmt"
containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)
func toCRIContainerUser(ctx context.Context, container containerstore.Container) (*runtime.ContainerUser, error) {
if container.Container == nil {
return nil, errors.New("container must not be nil")
}
runtimeSpec, err := container.Container.Spec(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get container runtime spec: %w", err)
}
if runtimeSpec.Process == nil {
return &runtime.ContainerUser{}, nil
}
user := runtimeSpec.Process.User
var supplementalGroups []int64
for _, gid := range user.AdditionalGids {
supplementalGroups = append(supplementalGroups, int64(gid))
}
return &runtime.ContainerUser{
Linux: &runtime.LinuxContainerUser{
Uid: int64(user.UID),
Gid: int64(user.GID),
SupplementalGroups: supplementalGroups,
},
}, nil
}

125
internal/cri/server/container_status_linux_test.go Normal file
View File

@@ -0,0 +1,125 @@
/*
Copyright The containerd 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 server
import (
"context"
"errors"
"fmt"
"testing"
containerd "github.com/containerd/containerd/v2/client"
"github.com/containerd/containerd/v2/internal/cri/store/container"
specs "github.com/opencontainers/runtime-spec/specs-go"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestToCRIContainerUser(t *testing.T) {
fakeErrorOnSpec := errors.New("error")
testCases := []struct {
name string
container containerd.Container
expected *runtime.ContainerUser
expectErr bool
expectedErr error
}{
{
name: "container is nil",
container: nil,
expectErr: true,
expectedErr: errors.New("container must not be nil"),
},
{
name: "Spec() returns error",
container: &fakeSpecOnlyContainer{
t: t,
errOnSpec: fakeErrorOnSpec,
},
expectErr: true,
expectedErr: fmt.Errorf("failed to get container runtime spec: %w", fakeErrorOnSpec),
},
{
name: "no Process",
container: &fakeSpecOnlyContainer{
t: t,
spec: &specs.Spec{},
},
expected: &runtime.ContainerUser{},
},
{
name: "no additionalGids",
container: &fakeSpecOnlyContainer{
t: t,
spec: &specs.Spec{
Process: &specs.Process{
User: specs.User{
UID: 0,
GID: 0,
},
},
},
},
expected: &runtime.ContainerUser{
Linux: &runtime.LinuxContainerUser{
Uid: 0,
Gid: 0,
},
},
},
{
name: "with additionalGids",
container: &fakeSpecOnlyContainer{
t: t,
spec: &specs.Spec{
Process: &specs.Process{
User: specs.User{
UID: 0,
GID: 0,
AdditionalGids: []uint32{0, 1234},
},
},
},
},
expected: &runtime.ContainerUser{
Linux: &runtime.LinuxContainerUser{
Uid: 0,
Gid: 0,
SupplementalGroups: []int64{0, 1234},
},
},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
got, err := toCRIContainerUser(context.Background(), container.Container{
Container: testCase.container,
})
if testCase.expectErr {
require.Nil(t, got)
require.Error(t, err)
assert.Equal(t, testCase.expectedErr, err)
} else {
require.NoError(t, err)
assert.Equal(t, testCase.expected, got)
}
})
}
}

30
internal/cri/server/container_status_other.go Normal file
View File

@@ -0,0 +1,30 @@
//go:build !windows && !linux
/*
Copyright The containerd 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 server
import (
"context"
containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)
func toCRIContainerUser(ctx context.Context, container containerstore.Container) (*runtime.ContainerUser, error) {
return &runtime.ContainerUser{}, nil
}

107
internal/cri/server/container_status_test.go
View File

@@ -22,8 +22,13 @@ import (
"testing"
"time"
containerd "github.com/containerd/containerd/v2/client"
"github.com/containerd/containerd/v2/core/containers"
criconfig "github.com/containerd/containerd/v2/internal/cri/config"
snapshotstore "github.com/containerd/containerd/v2/internal/cri/store/snapshot"
"github.com/containerd/containerd/v2/pkg/cio"
"github.com/containerd/typeurl/v2"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
@@ -31,7 +36,7 @@ import (
imagestore "github.com/containerd/containerd/v2/internal/cri/store/image"
)
func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Status,
func getContainerStatusTestData(t *testing.T) (*containerstore.Metadata, containerd.Container, *containerstore.Status,
*imagestore.Image, *runtime.ContainerStatus) {
imageID := "sha256:1123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"
testID := "test-id"
@@ -70,6 +75,9 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta
"gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582",
},
}
container := &fakeSpecOnlyContainer{t: t, spec: &specs.Spec{}}
expected := &runtime.ContainerStatus{
Id: testID,
Metadata: config.GetMetadata(),
@@ -82,9 +90,10 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta
Annotations: config.GetAnnotations(),
Mounts: config.GetMounts(),
LogPath: "test-log-path",
User: &runtime.ContainerUser{},
}
return metadata, status, image, expected
return metadata, container, status, image, expected
}
func TestToCRIContainerStatus(t *testing.T) {
@@ -139,7 +148,7 @@ func TestToCRIContainerStatus(t *testing.T) {
test := test
t.Run(test.desc, func(t *testing.T) {
metadata, status, _, expected := getContainerStatusTestData()
metadata, ctnr, status, _, expected := getContainerStatusTestData(t)
// Update status with test case.
status.StartedAt = test.startedAt
status.FinishedAt = test.finishedAt
@@ -149,6 +158,7 @@ func TestToCRIContainerStatus(t *testing.T) {
container, err := containerstore.NewContainer(
*metadata,
containerstore.WithFakeStatus(*status),
containerstore.WithContainer(ctnr),
)
assert.NoError(t, err)
// Set expectation based on test case.
@@ -158,9 +168,11 @@ func TestToCRIContainerStatus(t *testing.T) {
expected.ExitCode = test.exitCode
expected.Message = test.message
patchExceptedWithState(expected, test.expectedState)
containerStatus := toCRIContainerStatus(container,
containerStatus, err := toCRIContainerStatus(context.Background(),
container,
expected.Image,
expected.ImageRef)
assert.Nil(t, err)
assert.Equal(t, expected, containerStatus, test.desc)
})
}
@@ -168,7 +180,7 @@ func TestToCRIContainerStatus(t *testing.T) {
// TODO(mikebrow): add a fake containerd container.Container.Spec client api so we can test verbose is true option
func TestToCRIContainerInfo(t *testing.T) {
metadata, status, _, _ := getContainerStatusTestData()
metadata, _, status, _, _ := getContainerStatusTestData(t)
container, err := containerstore.NewContainer(
*metadata,
containerstore.WithFakeStatus(*status),
@@ -231,7 +243,7 @@ func TestContainerStatus(t *testing.T) {
test := test
t.Run(test.desc, func(t *testing.T) {
c := newTestCRIService()
metadata, status, image, expected := getContainerStatusTestData()
metadata, ctnr, status, image, expected := getContainerStatusTestData(t)
// Update status with test case.
status.StartedAt = test.startedAt
status.FinishedAt = test.finishedAt
@@ -239,6 +251,7 @@ func TestContainerStatus(t *testing.T) {
container, err := containerstore.NewContainer(
*metadata,
containerstore.WithFakeStatus(*status),
containerstore.WithContainer(ctnr),
)
assert.NoError(t, err)
if test.exist {
@@ -302,3 +315,85 @@ func patchExceptedWithState(expected *runtime.ContainerStatus, state runtime.Con
expected.FinishedAt = 0
}
}
var _ containerd.Container = &fakeSpecOnlyContainer{}
type fakeSpecOnlyContainer struct {
t *testing.T
spec *specs.Spec
errOnSpec error
}
// Spec implements client.Container.
func (c *fakeSpecOnlyContainer) Spec(context.Context) (*specs.Spec, error) {
if c.errOnSpec != nil {
return nil, c.errOnSpec
}
return c.spec, nil
}
// Checkpoint implements client.Container.
func (c *fakeSpecOnlyContainer) Checkpoint(context.Context, string, ...containerd.CheckpointOpts) (containerd.Image, error) {
c.t.Error("fakeSpecOnlyContainer.Checkpoint: not implemented")
return nil, errors.New("not implemented")
}
// Delete implements client.Container.
func (c *fakeSpecOnlyContainer) Delete(context.Context, ...containerd.DeleteOpts) error {
c.t.Error("fakeSpecOnlyContainer.Delete: not implemented")
return errors.New("not implemented")
}
// Extensions implements client.Container.
func (c *fakeSpecOnlyContainer) Extensions(context.Context) (map[string]typeurl.Any, error) {
c.t.Error("fakeSpecOnlyContainer.Extensions: not implemented")
return nil, errors.New("not implemented")
}
// ID implements client.Container.
func (c *fakeSpecOnlyContainer) ID() string {
c.t.Error("fakeSpecOnlyContainer.ID: not implemented")
return "" // not implemented
}
// Image implements client.Container.
func (c *fakeSpecOnlyContainer) Image(context.Context) (containerd.Image, error) {
c.t.Error("fakeSpecOnlyContainer.Image: not implemented")
return nil, errors.New("not implemented")
}
// Info implements client.Container.
func (c *fakeSpecOnlyContainer) Info(context.Context, ...containerd.InfoOpts) (containers.Container, error) {
c.t.Error("fakeSpecOnlyContainer.Info: not implemented")
return containers.Container{}, errors.New("not implemented")
}
// Labels implements client.Container.
func (c *fakeSpecOnlyContainer) Labels(context.Context) (map[string]string, error) {
c.t.Error("fakeSpecOnlyContainer.Labels: not implemented")
return nil, errors.New("not implemented")
}
// NewTask implements client.Container.
func (c *fakeSpecOnlyContainer) NewTask(context.Context, cio.Creator, ...containerd.NewTaskOpts) (containerd.Task, error) {
c.t.Error("fakeSpecOnlyContainer.NewTask: not implemented")
return nil, errors.New("not implemented")
}
// SetLabels implements client.Container.
func (c *fakeSpecOnlyContainer) SetLabels(context.Context, map[string]string) (map[string]string, error) {
c.t.Error("fakeSpecOnlyContainer.SetLabels: not implemented")
return nil, errors.New("not implemented")
}
// Task implements client.Container.
func (c *fakeSpecOnlyContainer) Task(context.Context, cio.Attach) (containerd.Task, error) {
c.t.Error("fakeSpecOnlyContainer.Task: not implemented")
return nil, errors.New("not implemented")
}
// Update implements client.Container.
func (c *fakeSpecOnlyContainer) Update(context.Context, ...containerd.UpdateContainerOpts) error {
c.t.Error("fakeSpecOnlyContainer.Update: not implemented")
return errors.New("not implemented")
}

28
internal/cri/server/container_status_windows.go Normal file
View File

@@ -0,0 +1,28 @@
/*
Copyright The containerd 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 server
import (
"context"
containerstore "github.com/containerd/containerd/v2/internal/cri/store/container"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)
func toCRIContainerUser(ctx context.Context, container containerstore.Container) (*runtime.ContainerUser, error) {
return &runtime.ContainerUser{}, nil
}