Skip to content

Commit

Permalink
Revert "Do not make read-only mounts recursively read-only by default…
Browse files Browse the repository at this point in the history
… (also updates Docker client module to v25) (Mirantis#311)"

This reverts commit c2e3805.
  • Loading branch information
brandond committed Apr 10, 2024
1 parent 6cb4f77 commit 6a85da4
Show file tree
Hide file tree
Showing 137 changed files with 1,259 additions and 3,409 deletions.
30 changes: 3 additions & 27 deletions core/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ import (
"path/filepath"

"github.com/Mirantis/cri-dockerd/libdocker"
dockerbackend "github.com/docker/docker/api/types/backend"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/strslice"
"github.com/opencontainers/selinux/go-selinux/label"
v1 "k8s.io/cri-api/pkg/apis/runtime/v1"
)

Expand Down Expand Up @@ -65,9 +64,8 @@ func (ds *dockerService) CreateContainer(
image = iSpec.Image
}
containerName := makeContainerName(sandboxConfig, config)
mounts := config.GetMounts()
terminationMessagePath, _ := config.Annotations["io.kubernetes.container.terminationMessagePath"]
createConfig := dockerbackend.ContainerCreateConfig{
createConfig := types.ContainerCreateConfig{
Name: containerName,
Config: &container.Config{
Entrypoint: strslice.StrSlice(config.Command),
Expand All @@ -87,35 +85,13 @@ func (ds *dockerService) CreateContainer(
},
},
HostConfig: &container.HostConfig{
Mounts: libdocker.GenerateMountBindings(mounts, terminationMessagePath),
Binds: libdocker.GenerateMountBindings(config.GetMounts(), terminationMessagePath),
RestartPolicy: container.RestartPolicy{
Name: "no",
},
},
}

// Only request relabeling if the pod provides an SELinux context. If the pod
// does not provide an SELinux context relabeling will label the volume with
// the container's randomly allocated MCS label. This would restrict access
// to the volume to the container which mounts it first.
if selinuxOpts := config.GetLinux().GetSecurityContext().GetSelinuxOptions(); selinuxOpts != nil {
mountLabel, err := selinuxMountLabel(selinuxOpts)
if err != nil {
return nil, fmt.Errorf("unable to generate SELinux mount label: %v", err)
}
if mountLabel != "" {
// Equates to "Z" in the old bind API
const shared = false
for _, m := range mounts {
if m.SelinuxRelabel {
if err := label.Relabel(m.HostPath, mountLabel, shared); err != nil {
return nil, fmt.Errorf("unable to relabel %q with %q: %v", m.HostPath, mountLabel, err)
}
}
}
}
}

hc := createConfig.HostConfig
err = ds.updateCreateConfig(
&createConfig,
Expand Down
8 changes: 3 additions & 5 deletions core/helpers_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ package core

import (
"fmt"

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
Expand All @@ -36,7 +34,7 @@ func DefaultMemorySwap() int64 {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand Down Expand Up @@ -99,12 +97,12 @@ func getNetworkNamespace(c *dockertypes.ContainerJSON) (string, error) {

type containerCleanupInfo struct{}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
*runtimeapi.CreateContainerRequest,
*dockerbackend.ContainerCreateConfig,
*dockertypes.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
return nil, nil
}
Expand Down
22 changes: 10 additions & 12 deletions core/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (
"github.com/Mirantis/cri-dockerd/libdocker"

dockertypes "github.com/docker/docker/api/types"
dockermount "github.com/docker/docker/api/types/mount"
dockerregistry "github.com/docker/docker/api/types/registry"
dockernat "github.com/docker/go-connections/nat"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -142,7 +140,7 @@ func TestParsingCreationConflictError(t *testing.T) {

func TestEnsureSandboxImageExists(t *testing.T) {
sandboxImage := "gcr.io/test/image"
authConfig := dockerregistry.AuthConfig{Username: "user", Password: "pass"}
authConfig := dockertypes.AuthConfig{Username: "user", Password: "pass"}
for desc, test := range map[string]struct {
injectImage bool
imgNeedsAuth bool
Expand Down Expand Up @@ -328,15 +326,15 @@ func TestGenerateMountBindings(t *testing.T) {
Propagation: runtimeapi.MountPropagation_PROPAGATION_BIDIRECTIONAL,
},
}
expectedResult := []dockermount.Mount{
{Type: dockermount.TypeBind, Source: "/mnt/1", Target: "/var/lib/mysql/1", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/2", Target: "/var/lib/mysql/2", ReadOnly: true, BindOptions: &dockermount.BindOptions{CreateMountpoint: true, ReadOnlyNonRecursive: true}},
{Type: dockermount.TypeBind, Source: "/mnt/3", Target: "/var/lib/mysql/3", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}}, // Relabeling is not handled here
{Type: dockermount.TypeBind, Source: "/mnt/4", Target: "/var/lib/mysql/4", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/5", Target: "/var/lib/mysql/5", BindOptions: &dockermount.BindOptions{CreateMountpoint: true, Propagation: dockermount.PropagationRSlave}},
{Type: dockermount.TypeBind, Source: "/mnt/6", Target: "/var/lib/mysql/6", BindOptions: &dockermount.BindOptions{CreateMountpoint: true, Propagation: dockermount.PropagationRShared}},
{Type: dockermount.TypeBind, Source: "/mnt/7", Target: "/var/lib/mysql/7", BindOptions: &dockermount.BindOptions{CreateMountpoint: true}},
{Type: dockermount.TypeBind, Source: "/mnt/8", Target: "/var/lib/mysql/8", ReadOnly: true, BindOptions: &dockermount.BindOptions{CreateMountpoint: true, ReadOnlyNonRecursive: true, Propagation: dockermount.PropagationRShared}}, // Relabeling is not handled here
expectedResult := []string{
"/mnt/1:/var/lib/mysql/1",
"/mnt/2:/var/lib/mysql/2:ro",
"/mnt/3:/var/lib/mysql/3:Z",
"/mnt/4:/var/lib/mysql/4",
"/mnt/5:/var/lib/mysql/5:rslave",
"/mnt/6:/var/lib/mysql/6:rshared",
"/mnt/7:/var/lib/mysql/7",
"/mnt/8:/var/lib/mysql/8:ro,Z,rshared",
}
result := libdocker.GenerateMountBindings(mounts, "")

Expand Down
7 changes: 3 additions & 4 deletions core/helpers_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)
Expand All @@ -48,7 +47,7 @@ func (ds *dockerService) getSandBoxSecurityOpts(separator rune) []string {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand All @@ -67,12 +66,12 @@ func getNetworkNamespace(c *dockertypes.ContainerJSON) (string, error) {

type containerCleanupInfo struct{}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
*runtimeapi.CreateContainerRequest,
*dockerbackend.ContainerCreateConfig,
*dockertypes.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
return nil, nil
}
Expand Down
12 changes: 5 additions & 7 deletions core/helpers_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"golang.org/x/sys/windows/registry"
"os"
"regexp"
"runtime"

"golang.org/x/sys/windows/registry"

"github.com/blang/semver"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
dockerfilters "github.com/docker/docker/api/types/filters"
"github.com/sirupsen/logrus"
Expand All @@ -44,7 +42,7 @@ func DefaultMemorySwap() int64 {
}

func (ds *dockerService) updateCreateConfig(
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
config *runtimeapi.ContainerConfig,
sandboxConfig *runtimeapi.PodSandboxConfig,
podSandboxID string, securityOptSep rune, apiVersion *semver.Version) error {
Expand Down Expand Up @@ -149,12 +147,12 @@ type containerCleanupInfo struct {
gMSARegistryValueName string
}

// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockerbackend.ContainerCreateConfig struct.
// applyPlatformSpecificDockerConfig applies platform-specific configurations to a dockertypes.ContainerCreateConfig struct.
// The containerCleanupInfo struct it returns will be passed as is to performPlatformSpecificContainerCleanup
// after either the container creation has failed or the container has been removed.
func (ds *dockerService) applyPlatformSpecificDockerConfig(
request *runtimeapi.CreateContainerRequest,
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
) (*containerCleanupInfo, error) {
cleanupInfo := &containerCleanupInfo{}

Expand All @@ -175,7 +173,7 @@ func (ds *dockerService) applyPlatformSpecificDockerConfig(
// https://github.com/moby/moby/pull/38777
func applyGMSAConfig(
config *runtimeapi.ContainerConfig,
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
cleanupInfo *containerCleanupInfo,
) error {
var credSpec string
Expand Down
3 changes: 1 addition & 2 deletions core/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

dockertypes "github.com/docker/docker/api/types"
dockerfilters "github.com/docker/docker/api/types/filters"
dockerregistry "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/pkg/jsonmessage"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -138,7 +137,7 @@ func (ds *dockerService) PullImage(
) (*runtimeapi.PullImageResponse, error) {
image := r.GetImage()
auth := r.GetAuth()
authConfig := dockerregistry.AuthConfig{}
authConfig := dockertypes.AuthConfig{}

if auth != nil {
authConfig.Username = auth.Username
Expand Down
23 changes: 10 additions & 13 deletions core/sandbox_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,17 @@ package core

import (
"fmt"
"os"
"strings"
"time"

"github.com/Mirantis/cri-dockerd/libdocker"
"github.com/Mirantis/cri-dockerd/utils"
"github.com/Mirantis/cri-dockerd/utils/errors"
"k8s.io/kubernetes/pkg/credentialprovider"
"os"
"strings"
"time"

"github.com/Mirantis/cri-dockerd/config"
dockertypes "github.com/docker/docker/api/types"
dockerbackend "github.com/docker/docker/api/types/backend"
dockercontainer "github.com/docker/docker/api/types/container"
dockerregistry "github.com/docker/docker/api/types/registry"
"github.com/sirupsen/logrus"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)
Expand Down Expand Up @@ -172,7 +169,7 @@ func (ds *dockerService) getPodSandboxDetails(
func (ds *dockerService) applySandboxLinuxOptions(
hc *dockercontainer.HostConfig,
lc *runtimeapi.LinuxPodSandboxConfig,
createConfig *dockerbackend.ContainerCreateConfig,
createConfig *dockertypes.ContainerCreateConfig,
image string,
separator rune,
) error {
Expand Down Expand Up @@ -210,11 +207,11 @@ func (ds *dockerService) applySandboxResources(
return nil
}

// makeSandboxDockerConfig returns dockerbackend.ContainerCreateConfig based on runtimeapi.PodSandboxConfig.
// makeSandboxDockerConfig returns dockertypes.ContainerCreateConfig based on runtimeapi.PodSandboxConfig.
func (ds *dockerService) makeSandboxDockerConfig(
c *runtimeapi.PodSandboxConfig,
image string,
) (*dockerbackend.ContainerCreateConfig, error) {
) (*dockertypes.ContainerCreateConfig, error) {
// Merge annotations and labels because docker supports only labels.
labels := makeLabels(c.GetLabels(), c.GetAnnotations())
// Apply a label to distinguish sandboxes from regular containers.
Expand All @@ -225,7 +222,7 @@ func (ds *dockerService) makeSandboxDockerConfig(
hc := &dockercontainer.HostConfig{
IpcMode: dockercontainer.IpcMode("shareable"),
}
createConfig := &dockerbackend.ContainerCreateConfig{
createConfig := &dockertypes.ContainerCreateConfig{
Name: makeSandboxName(c),
Config: &dockercontainer.Config{
Hostname: c.Hostname,
Expand Down Expand Up @@ -377,7 +374,7 @@ func rewriteFile(filePath, stringToWrite string) error {

func recoverFromCreationConflictIfNeeded(
client libdocker.DockerClientInterface,
createConfig dockerbackend.ContainerCreateConfig,
createConfig dockertypes.ContainerCreateConfig,
err error,
) (*dockercontainer.CreateResponse, error) {
matches := conflictRE.FindStringSubmatch(err.Error())
Expand Down Expand Up @@ -424,7 +421,7 @@ func ensureSandboxImageExists(client libdocker.DockerClientInterface, image stri
if !withCredentials {
logrus.Infof("Pulling the image without credentials. Image: %v", image)

err := client.PullImage(image, dockerregistry.AuthConfig{}, dockertypes.ImagePullOptions{})
err := client.PullImage(image, dockertypes.AuthConfig{}, dockertypes.ImagePullOptions{})
if err != nil {
return fmt.Errorf("failed pulling image %q: %v", image, err)
}
Expand All @@ -434,7 +431,7 @@ func ensureSandboxImageExists(client libdocker.DockerClientInterface, image stri

var pullErrs []error
for _, currentCreds := range creds {
authConfig := dockerregistry.AuthConfig(currentCreds)
authConfig := dockertypes.AuthConfig(currentCreds)
err := client.PullImage(image, authConfig, dockertypes.ImagePullOptions{})
// If there was no error, return success
if err == nil {
Expand Down
24 changes: 0 additions & 24 deletions core/selinux_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package core
import (
"fmt"

"github.com/opencontainers/selinux/go-selinux/label"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
)

Expand Down Expand Up @@ -89,26 +88,3 @@ func modifySecurityOption(config []string, name, value string) []string {

return config
}

func selinuxMountLabel(selinuxOpts *runtimeapi.SELinuxOption) (string, error) {
var opts []string
if selinuxOpts != nil {
if selinuxOpts.User != "" {
opts = append(opts, "user:"+selinuxOpts.User)
}
if selinuxOpts.Role != "" {
opts = append(opts, "role:"+selinuxOpts.Role)
}
if selinuxOpts.Type != "" {
opts = append(opts, "type:"+selinuxOpts.Type)
}
if selinuxOpts.Level != "" {
opts = append(opts, "level:"+selinuxOpts.Level)
}
}
if len(opts) == 0 {
opts = []string{"disable"}
}
_, s, err := label.InitLabels(opts)
return s, err
}
15 changes: 0 additions & 15 deletions core/selinux_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ package core
import (
"reflect"
"testing"

"github.com/opencontainers/selinux/go-selinux"
"github.com/stretchr/testify/assert"
)

func TestModifySecurityOptions(t *testing.T) {
Expand Down Expand Up @@ -60,15 +57,3 @@ func TestModifySecurityOptions(t *testing.T) {
}
}
}

func TestSELinuxMountLabel(t *testing.T) {
selinuxOpts := inputSELinuxOptions()
s, err := selinuxMountLabel(selinuxOpts)
assert.NoError(t, err)
t.Logf("mount label for %+v=%q", selinuxOpts, s)
if selinux.GetEnabled() {
assert.NotEmpty(t, s)
} else {
assert.Empty(t, s)
}
}
Loading

0 comments on commit 6a85da4

Please sign in to comment.