Skip to content

Commit

Permalink
Add privileged indicator to container status
Browse files Browse the repository at this point in the history
We now add an additional field to the `info` of the container status
indicating if a container is running in privileged mode or not.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>
  • Loading branch information
saschagrunert committed May 18, 2020
1 parent 79532ed commit 247d465
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 41 deletions.
18 changes: 10 additions & 8 deletions internal/storage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ type RuntimeServer interface {
// both its pod's ID and its container ID.
// Pointer arguments can be nil. Either the image name or ID can be
// omitted, but not both. All other arguments are required.
CreatePodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string) (ContainerInfo, error)
CreatePodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, privileged bool) (ContainerInfo, error)
// RemovePodSandbox deletes a pod sandbox's infrastructure container.
// The CRI expects that a sandbox can't be removed unless its only
// container is its infrastructure container, but we don't enforce that
Expand All @@ -87,7 +87,7 @@ type RuntimeServer interface {
// CreateContainer creates a container with the specified ID.
// Pointer arguments can be nil. Either the image name or ID can be
// omitted, but not both. All other arguments are required.
CreateContainer(systemContext *types.SystemContext, podName, podID, imageName, imageID, containerName, containerID, metadataName string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string) (ContainerInfo, error)
CreateContainer(systemContext *types.SystemContext, podName, podID, imageName, imageID, containerName, containerID, metadataName string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, privileged bool) (ContainerInfo, error)
// DeleteContainer deletes a container, unmounting it first if need be.
DeleteContainer(idOrName string) error

Expand Down Expand Up @@ -138,7 +138,8 @@ type RuntimeContainerMetadata struct {
CreatedAt int64 `json:"created-at"` // Applicable to both PodSandboxes and Containers
Attempt uint32 `json:"attempt,omitempty"` // Applicable to both PodSandboxes and Containers
// Pod is true if this is the pod's infrastructure container.
Pod bool `json:"pod,omitempty"` // Applicable to both PodSandboxes and Containers
Pod bool `json:"pod,omitempty"` // Applicable to both PodSandboxes and Containers
Privileged bool `json:"privileged,omitempty"` // Applicable to both PodSandboxes and Containers
}

// SetMountLabel updates the mount label held by a RuntimeContainerMetadata
Expand All @@ -147,7 +148,7 @@ func (metadata *RuntimeContainerMetadata) SetMountLabel(mountLabel string) {
metadata.MountLabel = mountLabel
}

func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, containerID, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, isPauseImage bool) (ci ContainerInfo, retErr error) {
func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, containerID, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, isPauseImage, privileged bool) (ci ContainerInfo, retErr error) {
var ref types.ImageReference
if podName == "" || podID == "" {
return ContainerInfo{}, ErrInvalidPodName
Expand Down Expand Up @@ -249,6 +250,7 @@ func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.System
Namespace: namespace,
Attempt: attempt,
CreatedAt: time.Now().Unix(),
Privileged: privileged,
}
mdata, err := json.Marshal(&metadata)
if err != nil {
Expand Down Expand Up @@ -344,12 +346,12 @@ func (r *runtimeService) createContainerOrPodSandbox(systemContext *types.System
}, nil
}

func (r *runtimeService) CreatePodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string) (ContainerInfo, error) {
return r.createContainerOrPodSandbox(systemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, podID, metadataName, uid, namespace, attempt, idMappings, labelOptions, true)
func (r *runtimeService) CreatePodSandbox(systemContext *types.SystemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, metadataName, uid, namespace string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, privileged bool) (ContainerInfo, error) {
return r.createContainerOrPodSandbox(systemContext, podName, podID, imageName, imageAuthFile, imageID, containerName, podID, metadataName, uid, namespace, attempt, idMappings, labelOptions, true, privileged)
}

func (r *runtimeService) CreateContainer(systemContext *types.SystemContext, podName, podID, imageName, imageID, containerName, containerID, metadataName string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string) (ContainerInfo, error) {
return r.createContainerOrPodSandbox(systemContext, podName, podID, imageName, "", imageID, containerName, containerID, metadataName, "", "", attempt, idMappings, labelOptions, false)
func (r *runtimeService) CreateContainer(systemContext *types.SystemContext, podName, podID, imageName, imageID, containerName, containerID, metadataName string, attempt uint32, idMappings *idtools.IDMappings, labelOptions []string, privileged bool) (ContainerInfo, error) {
return r.createContainerOrPodSandbox(systemContext, podName, podID, imageName, "", imageID, containerName, containerID, metadataName, "", "", attempt, idMappings, labelOptions, false, privileged)
}

func (r *runtimeService) RemovePodSandbox(idOrName string) error {
Expand Down
45 changes: 30 additions & 15 deletions internal/storage/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)
})

It("should succeed to create a pod sandbox", func() {
Expand All @@ -576,7 +577,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename", "",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

})

Expand All @@ -597,7 +599,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -611,7 +614,8 @@ var _ = t.Describe("Runtime", func() {
"", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -624,7 +628,8 @@ var _ = t.Describe("Runtime", func() {
_, err := sut.CreateContainer(&types.SystemContext{},
"podName", "podID", "", "",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -637,7 +642,8 @@ var _ = t.Describe("Runtime", func() {
_, err := sut.CreateContainer(&types.SystemContext{},
"podName", "podID", "imagename", "imageID",
"", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand Down Expand Up @@ -670,7 +676,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand Down Expand Up @@ -699,7 +706,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -726,7 +734,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename", "",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -751,7 +760,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename", "",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -771,7 +781,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename", "",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand All @@ -791,7 +802,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand Down Expand Up @@ -819,7 +831,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "imagename",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "containerID", "metadataName",
0, &idtools.IDMappings{}, []string{"mountLabel"})
0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)

// Then
Expect(err).NotTo(BeNil())
Expand Down Expand Up @@ -877,7 +890,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "pauseimagename", "",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)
})

It("should pull pauseImage if not available locally, using provided credential file", func() {
Expand All @@ -896,7 +910,8 @@ var _ = t.Describe("Runtime", func() {
"podName", "podID", "pauseimagename", "/var/non-default/credentials.json",
"8a788232037eaf17794408ff3df6b922a1aedf9ef8de36afdae3ed0b0381907b",
"containerName", "metadataName",
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"})
"uid", "namespace", 0, &idtools.IDMappings{}, []string{"mountLabel"}, false,
)
})

AfterEach(func() {
Expand Down
7 changes: 4 additions & 3 deletions server/container_create_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
metadata.Name,
metadata.Attempt,
containerIDMappings,
labelOptions)
labelOptions,
privileged,
)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -602,8 +604,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID, contai
}
specgen.SetProcessNoNewPrivileges(linux.GetSecurityContext().GetNoNewPrivs())

if containerConfig.GetLinux().GetSecurityContext() != nil &&
!containerConfig.GetLinux().GetSecurityContext().Privileged {
if !privileged {
// TODO(runcom): have just one of this var at the top of the function
securityContext := containerConfig.GetLinux().GetSecurityContext()
for _, mp := range []string{
Expand Down
11 changes: 9 additions & 2 deletions server/container_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq
resp.Status.LogPath = c.LogPath()

if req.Verbose {
info, err := createContainerInfo(c)
info, err := s.createContainerInfo(c)
if err != nil {
return nil, errors.Wrap(err, "creating container info")
}
Expand All @@ -110,15 +110,22 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq
return resp, nil
}

func createContainerInfo(container *oci.Container) (map[string]string, error) {
func (s *Server) createContainerInfo(container *oci.Container) (map[string]string, error) {
metadata, err := s.StorageRuntimeServer().GetContainerMetadata(container.ID())
if err != nil {
return nil, errors.Wrap(err, "getting container metadata")
}

info := struct {
SandboxID string `json:"sandboxID"`
Pid int `json:"pid"`
RuntimeSpec spec.Spec `json:"runtimeSpec"`
Privileged bool `json:"privileged"`
}{
container.Sandbox(),
container.State().Pid,
container.Spec(),
metadata.Privileged,
}
bytes, err := json.Marshal(info)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions server/container_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"context"

"github.com/cri-o/cri-o/internal/oci"
"github.com/cri-o/cri-o/internal/storage"
"github.com/cri-o/cri-o/utils"
"github.com/golang/mock/gomock"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -33,6 +35,11 @@ var _ = t.Describe("ContainerStatus", func() {
testContainer.SetState(givenState)
testContainer.SetSpec(&specs.Spec{Version: "1.0.0"})

gomock.InOrder(
runtimeServerMock.EXPECT().GetContainerMetadata(gomock.Any()).
Return(storage.RuntimeContainerMetadata{}, nil),
)

// When
response, err := sut.ContainerStatus(context.Background(),
&pb.ContainerStatusRequest{
Expand Down
9 changes: 6 additions & 3 deletions server/sandbox_run_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ func (s *Server) runPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
if selinuxConfig != nil {
labelOptions = getLabelOptions(selinuxConfig)
}

privileged := s.privilegedSandbox(req)

podContainer, err := s.StorageRuntimeServer().CreatePodSandbox(s.config.SystemContext,
sbox.Name(), sbox.ID(),
s.config.PauseImage,
Expand All @@ -101,7 +104,9 @@ func (s *Server) runPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
namespace,
attempt,
s.defaultIDMappings,
labelOptions)
labelOptions,
privileged,
)

mountLabel := podContainer.MountLabel
processLabel := podContainer.ProcessLabel
Expand Down Expand Up @@ -220,8 +225,6 @@ func (s *Server) runPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
return nil, fmt.Errorf("requested logDir for sbox id %s is a relative path: %s", sbox.ID(), logDir)
}

privileged := s.privilegedSandbox(req)

// Add capabilities from crio.conf if default_capabilities is defined
capabilities := &pb.Capability{}
if s.config.DefaultCapabilities != nil {
Expand Down
6 changes: 4 additions & 2 deletions server/sandbox_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ var _ = t.Describe("RunPodSandbox", func() {
runtimeServerMock.EXPECT().CreatePodSandbox(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any()).
Return(storage.ContainerInfo{
RunDir: "/tmp",
Config: &v1.Image{Config: v1.ImageConfig{}},
Expand Down Expand Up @@ -120,7 +121,8 @@ var _ = t.Describe("RunPodSandbox", func() {
runtimeServerMock.EXPECT().CreatePodSandbox(gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(),
gomock.Any()).
Return(storage.ContainerInfo{}, nil),
runtimeServerMock.EXPECT().RemovePodSandbox(gomock.Any()).
Return(nil),
Expand Down
7 changes: 7 additions & 0 deletions test/ctr.bats
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ function wait_until_exit() {
run crictl inspect "$ctr_id"
echo "$output"
[ "$status" -eq 0 ]
run crictl inspect "$ctr_id" | jq -e ".info.privileged == false"
echo "$output"
[ "$status" -eq 0 ]
run crictl start "$ctr_id"
echo "$output"
[ "$status" -eq 0 ]
Expand Down Expand Up @@ -1566,6 +1569,10 @@ function wait_until_exit() {
run crictl start "$ctr_id"
[ "$status" -eq 0 ]

run crictl inspect "$ctr_id" | jq -e ".info.privileged == true"
echo "$output"
[ "$status" -eq 0 ]

# TODO there seems to be a difference in behavior between runc and crun
# where crun has this mounted ro, and now runc has it mounted rw
run crictl exec "$ctr_id" cat /proc/mounts
Expand Down

0 comments on commit 247d465

Please sign in to comment.