cri-o · openshift-merge-bot · Aug 28, 2024 · Aug 26, 2024
@@ -188,7 +188,7 @@ jobs:
             defaultRuntime: runc
             runtimeType: pod
             critest: 0
-            userns: 1
+            userns: 0
             jobs: 2
             timeout: 120
 
@@ -198,7 +198,7 @@ jobs:
             defaultRuntime: runc
             runtimeType: pod
             critest: 0
-            userns: 1
+            userns: 0
             jobs: 2
             timeout: 120
 

@@ -61,6 +61,7 @@ kata_skip_files:
   - "metrics.bats"
   - "network_ping.bats"
   - "nri.bats"
+  - "oci_volumes.bats"
   - "policy.bats"
   - "restore.bats" # restore operations are failing with Kata
   - "seccomp_notifier.bats"

@@ -3,6 +3,7 @@ package server
 import (
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -1069,18 +1070,24 @@ func (s *Server) addOCIBindMounts(ctx context.Context, ctr ctrfactory.Container,
 	if err != nil {
 		return nil, nil, err
 	}
+
+	imageVolumesPath, err := s.ensureImageVolumesPath(ctx, mounts)
+	if err != nil {
+		return nil, nil, fmt.Errorf("ensure image volumes path: %w", err)
+	}
+
 	for _, m := range mounts {
 		dest := m.ContainerPath
 		if dest == "" {
 			return nil, nil, errors.New("mount.ContainerPath is empty")
 		}
 		if m.Image != nil && m.Image.Image != "" {
-			mountPoint, imageID, err := s.mountImage(ctx, m.Image.Image, mountLabel)
+			volume, err := s.mountImage(ctx, specgen, imageVolumesPath, m)
 			if err != nil {
 				return nil, nil, fmt.Errorf("mount image: %w", err)
 			}
-			m.Image.Image = imageID // Set the ID for container status later on
-			m.HostPath = mountPoint // Adjust the host path to use the mount point
+			volumes = append(volumes, *volume)
+			continue
 		}
 		if m.HostPath == "" {
 			return nil, nil, errors.New("mount.HostPath is empty")
@@ -1237,25 +1244,92 @@ func (s *Server) addOCIBindMounts(ctx context.Context, ctr ctrfactory.Container,
 	return volumes, ociMounts, nil
 }
 
-// mountImage mounts the provided imageRef using the mountLabel and returns the hostPath as well as imageID on success.
-func (s *Server) mountImage(ctx context.Context, imageRef, mountLabel string) (hostPath, imageID string, err error) {
-	log.Debugf(ctx, "Image ref to mount: %s", imageRef)
-	status, err := s.storageImageStatus(ctx, types.ImageSpec{Image: imageRef})
+// mountImage adds required image mounts to the provided spec generator and returns a corresponding ContainerVolume.
+func (s *Server) mountImage(ctx context.Context, specgen *generate.Generator, imageVolumesPath string, m *types.Mount) (*oci.ContainerVolume, error) {
+	if m == nil || m.Image == nil || m.Image.Image == "" || m.ContainerPath == "" {
+		return nil, fmt.Errorf("invalid mount specified: %+v", m)
+	}
+
+	log.Debugf(ctx, "Image ref to mount: %s", m.Image.Image)
+	status, err := s.storageImageStatus(ctx, types.ImageSpec{Image: m.Image.Image})
 	if err != nil {
-		return "", "", fmt.Errorf("get storage image status: %w", err)
+		return nil, fmt.Errorf("get storage image status: %w", err)
 	}
 
-	id := status.ID.IDStringForOutOfProcessConsumptionOnly()
-	log.Debugf(ctx, "Image ID to mount: %v", id)
+	if status == nil {
+		// Should not happen because the kubelet ensures the image.
+		return nil, fmt.Errorf("image %q does not exist locally", m.Image.Image)
+	}
+
+	imageID := status.ID.IDStringForOutOfProcessConsumptionOnly()
+	log.Debugf(ctx, "Image ID to mount: %v", imageID)
 
 	options := []string{"ro", "noexec", "nosuid", "nodev"}
-	mountPoint, err := s.Store().MountImage(id, options, mountLabel)
+	mountPoint, err := s.Store().MountImage(imageID, options, "")
 	if err != nil {
-		return "", "", fmt.Errorf("mount storage: %w", err)
+		return nil, fmt.Errorf("mount storage: %w", err)
 	}
-
 	log.Infof(ctx, "Image mounted to: %s", mountPoint)
-	return mountPoint, id, nil
+
+	const overlay = "overlay"
+	specgen.AddMount(rspec.Mount{
+		Type:        overlay,
+		Source:      overlay,
+		Destination: m.ContainerPath,
+		Options: []string{
+			"lowerdir=" + mountPoint + ":" + imageVolumesPath,
+		},
+		UIDMappings: getOCIMappings(m.UidMappings),
+		GIDMappings: getOCIMappings(m.GidMappings),
+	})
+	log.Debugf(ctx, "Added overlay mount from %s to %s", mountPoint, imageVolumesPath)
+
+	return &oci.ContainerVolume{
+		ContainerPath:     m.ContainerPath,
+		HostPath:          mountPoint,
+		Readonly:          m.Readonly,
+		RecursiveReadOnly: m.RecursiveReadOnly,
+		Propagation:       m.Propagation,
+		SelinuxRelabel:    m.SelinuxRelabel,
+		Image:             &types.ImageSpec{Image: imageID},
+	}, nil
+}
+
+func (s *Server) ensureImageVolumesPath(ctx context.Context, mounts []*types.Mount) (string, error) {
+	// Check if we need to anything at all
+	noop := true
+	for _, m := range mounts {
+		if m.Image != nil && m.Image.Image != "" {
+			noop = false
+			break
+		}
+	}
+
+	if noop {
+		return "", nil
+	}
+
+	imageVolumesPath := filepath.Join(filepath.Dir(s.Config().ContainerExitsDir), "image-volumes")
+	log.Debugf(ctx, "Using image volumes path: %s", imageVolumesPath)
+
+	if err := os.MkdirAll(imageVolumesPath, 0o700); err != nil {
+		return "", fmt.Errorf("create image volumes path: %w", err)
+	}
+
+	f, err := os.Open(imageVolumesPath)
+	if err != nil {
+		return "", fmt.Errorf("open image volumes path %s: %w", imageVolumesPath, err)
+	}
+
+	_, readErr := f.ReadDir(1)
+	if readErr != nil && !errors.Is(readErr, io.EOF) {
+		return "", fmt.Errorf("unable to read dir names of image volumes path %s: %w", imageVolumesPath, err)
+	}
+	if readErr == nil {
+		return "", fmt.Errorf("image volumes path %s is not empty", imageVolumesPath)
+	}
+
+	return imageVolumesPath, nil
 }
 
 func getOCIMappings(m []*types.IDMapping) []rspec.LinuxIDMapping {

@@ -12,11 +12,15 @@ function teardown() {
 	cleanup_test
 }
 
+CONTAINER_PATH=/volume
+IMAGE=quay.io/crio/artifact:v1
+
 @test "OCI image volume mount lifecycle" {
-	start_crio
+	if [[ "$TEST_USERNS" == "1" ]]; then
+		skip "test fails in a user namespace"
+	fi
 
-	CONTAINER_PATH=/volume
-	IMAGE=quay.io/crio/artifact:v1
+	start_crio
 
 	# Prepull the artifact
 	crictl pull "$IMAGE"
@@ -51,3 +55,59 @@ function teardown() {
 	# Image removal should work now
 	crictl rmi $IMAGE
 }
+
+@test "OCI image volume SELinux" {
+	if ! is_selinux_enforcing; then
+		skip "not enforcing"
+	fi
+
+	# RHEL/CentOS 7's container-selinux package replaces container_file_t with svirt_sandbox_file_t
+	# under the hood. This causes the annotation to not work correctly.
+	if is_rhel_7; then
+		skip "fails on RHEL 7 or earlier"
+	fi
+
+	start_crio
+
+	# Prepull the artifact
+	crictl pull "$IMAGE"
+
+	# Build a second sandbox using a different level
+	jq '.metadata.name = "sb-1" |
+		.metadata.uid = "new-uid" |
+		.linux.security_context.selinux_options.level = "s0:c200,c100"' \
+		"$TESTDATA"/sandbox_config.json > "$TESTDIR"/sandbox.json
+
+	# Set mounts in the same way as the kubelet would do
+	jq --arg IMAGE "$IMAGE" --arg CONTAINER_PATH "$CONTAINER_PATH" \
+		'.mounts = [{
+			host_path: "",
+			container_path: $CONTAINER_PATH,
+			image: { image: $IMAGE },
+			readonly: true
+		}]' \
+		"$TESTDATA"/container_sleep.json > "$TESTDIR/container.json"
+
+	CTR_ID1=$(crictl run "$TESTDIR/container.json" "$TESTDATA/sandbox_config.json")
+	CTR_ID2=$(crictl run "$TESTDIR/container.json" "$TESTDIR/sandbox.json")
+
+	# Assert the right labels
+	crictl exec -s "$CTR_ID1" ls -Z "$CONTAINER_PATH" | grep -q "s0:c4,c5"
+	crictl exec -s "$CTR_ID2" ls -Z "$CONTAINER_PATH" | grep -q "s0:c100,c200"
+}
+
+@test "OCI image volume does not exist locally" {
+	start_crio
+
+	# Set mounts in the same way as the kubelet would do
+	jq --arg IMAGE "$IMAGE" --arg CONTAINER_PATH "$CONTAINER_PATH" \
+		'.mounts = [{
+			host_path: "",
+			container_path: $CONTAINER_PATH,
+			image: { image: $IMAGE },
+			readonly: true
+		}]' \
+		"$TESTDATA"/container_sleep.json > "$TESTDIR/container.json"
+
+	run ! crictl run "$TESTDIR/container.json" "$TESTDATA/sandbox_config.json"
+}