Merge pull request #16682 from rhatdan/ro

Fix handling of readonly containers when defined in kube.yaml

libpod/kube.go

@@ -435,7 +435,7 @@ func (p *Pod) podWithContainers(ctx context.Context, containers []*Container, po
 			}
 		}
 	}
-	podVolumes := make([]v1.Volume, 0, len(deDupPodVolumes))
+	podVolumes := []v1.Volume{}
 	for _, vol := range deDupPodVolumes {
 		podVolumes = append(podVolumes, *vol)
 	}
@@ -471,18 +471,12 @@ func newPodObject(podName string, annotations map[string]string, initCtrs, conta
 		CreationTimestamp: v12.Now(),
 		Annotations:       annotations,
 	}
-	// Set enableServiceLinks to false as podman doesn't use the service port environment variables
-	enableServiceLinks := false
-	// Set automountServiceAccountToken to false as podman doesn't use service account tokens
-	automountServiceAccountToken := false
 	ps := v1.PodSpec{
-		Containers:                   containers,
-		Hostname:                     hostname,
-		HostNetwork:                  hostNetwork,
-		InitContainers:               initCtrs,
-		Volumes:                      volumes,
-		EnableServiceLinks:           &enableServiceLinks,
-		AutomountServiceAccountToken: &automountServiceAccountToken,
+		Containers:     containers,
+		Hostname:       hostname,
+		HostNetwork:    hostNetwork,
+		InitContainers: initCtrs,
+		Volumes:        volumes,
 	}
 	if !hostUsers {
 		ps.HostUsers = &hostUsers
@@ -894,35 +888,46 @@ func generateKubeVolumeMount(m specs.Mount) (v1.VolumeMount, v1.Volume, error) {
 	vm := v1.VolumeMount{}
 	vo := v1.Volume{}
 
-	name, err := convertVolumePathToName(m.Source)
-	if err != nil {
-		return vm, vo, err
+	var (
+		name string
+		err  error
+	)
+	if m.Type == define.TypeTmpfs {
+		name = "tmp"
+		vo.EmptyDir = &v1.EmptyDirVolumeSource{
+			Medium: v1.StorageMediumMemory,
+		}
+		vo.Name = name
+	} else {
+		name, err = convertVolumePathToName(m.Source)
+		if err != nil {
+			return vm, vo, err
+		}
+		// To avoid naming conflicts with any persistent volume mounts, add a unique suffix to the volume's name.
+		name += "-host"
+		vo.Name = name
+		vo.HostPath = &v1.HostPathVolumeSource{}
+		vo.HostPath.Path = m.Source
+		isDir, err := isHostPathDirectory(m.Source)
+		// neither a directory or a file lives here, default to creating a directory
+		// TODO should this be an error instead?
+		var hostPathType v1.HostPathType
+		switch {
+		case err != nil:
+			hostPathType = v1.HostPathDirectoryOrCreate
+		case isDir:
+			hostPathType = v1.HostPathDirectory
+		default:
+			hostPathType = v1.HostPathFile
+		}
+		vo.HostPath.Type = &hostPathType
 	}
-	// To avoid naming conflicts with any persistent volume mounts, add a unique suffix to the volume's name.
-	name += "-host"
 	vm.Name = name
 	vm.MountPath = m.Destination
 	if cutil.StringInSlice("ro", m.Options) {
 		vm.ReadOnly = true
 	}
 
-	vo.Name = name
-	vo.HostPath = &v1.HostPathVolumeSource{}
-	vo.HostPath.Path = m.Source
-	isDir, err := isHostPathDirectory(m.Source)
-	// neither a directory or a file lives here, default to creating a directory
-	// TODO should this be an error instead?
-	var hostPathType v1.HostPathType
-	switch {
-	case err != nil:
-		hostPathType = v1.HostPathDirectoryOrCreate
-	case isDir:
-		hostPathType = v1.HostPathDirectory
-	default:
-		hostPathType = v1.HostPathFile
-	}
-	vo.HostPath.Type = &hostPathType
-
 	return vm, vo, nil
 }
 

pkg/specgen/generate/kube/kube.go

@@ -20,6 +20,7 @@ import (
 	"github.com/containers/common/pkg/secrets"
 	cutil "github.com/containers/common/pkg/util"
 	"github.com/containers/image/v5/manifest"
+	itypes "github.com/containers/image/v5/types"
 	"github.com/containers/podman/v4/libpod/define"
 	ann "github.com/containers/podman/v4/pkg/annotations"
 	"github.com/containers/podman/v4/pkg/domain/entities"
@@ -119,6 +120,8 @@ type CtrSpecGenOptions struct {
 	ConfigMaps []v1.ConfigMap
 	// SeccompPaths for finding the seccomp profile path
 	SeccompPaths *KubeSeccompPaths
+	// ReadOnly make all containers root file system readonly
+	ReadOnly itypes.OptionalBool
 	// RestartPolicy defines the restart policy of the container
 	RestartPolicy string
 	// NetNSIsHost tells the container to use the host netns
@@ -444,6 +447,10 @@ func ToSpecGen(ctx context.Context, opts *CtrSpecGenOptions) (*specgen.SpecGener
 		}
 	}
 
+	if ro := opts.ReadOnly; ro != itypes.OptionalBoolUndefined {
+		s.ReadOnlyFilesystem = (ro == itypes.OptionalBoolTrue)
+	}
+
 	// Make sure the container runs in a systemd unit which is
 	// stored as a label at container creation.
 	if unit := os.Getenv(systemdDefine.EnvVariable); unit != "" {

pkg/specgenutil/specgen.go

@@ -676,7 +676,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 
 	// Only add read-only tmpfs mounts in case that we are read-only and the
 	// read-only tmpfs flag has been set.
-	mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS, c.ReadOnlyTmpFS && c.ReadOnly)
+	mounts, volumes, overlayVolumes, imageVolumes, err := parseVolumes(c.Volume, c.Mount, c.TmpFS)
 	if err != nil {
 		return err
 	}
@@ -854,6 +854,10 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
 		s.PasswdEntry = c.PasswdEntry
 	}
 
+	if c.ReadOnly && c.ReadOnlyTmpFS {
+		s.Mounts = addReadOnlyMounts(s.Mounts)
+	}
+
 	return nil
 }
 

pkg/specgenutil/volumes.go

@@ -26,7 +26,7 @@ var (
 // Does not handle image volumes, init, and --volumes-from flags.
 // Can also add tmpfs mounts from read-only tmpfs.
 // TODO: handle options parsing/processing via containers/storage/pkg/mount
-func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bool) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
+func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, []*specgen.ImageVolume, error) {
 	// Get mounts from the --mounts flag.
 	unifiedMounts, unifiedVolumes, unifiedImageVolumes, err := Mounts(mountFlag)
 	if err != nil {
@@ -78,26 +78,6 @@ func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bo
 		unifiedMounts[dest] = tmpfs
 	}
 
-	// If requested, add tmpfs filesystems for read-only containers.
-	if addReadOnlyTmpfs {
-		readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
-		options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
-		for _, dest := range readonlyTmpfs {
-			if _, ok := unifiedMounts[dest]; ok {
-				continue
-			}
-			if _, ok := unifiedVolumes[dest]; ok {
-				continue
-			}
-			unifiedMounts[dest] = spec.Mount{
-				Destination: dest,
-				Type:        define.TypeTmpfs,
-				Source:      "tmpfs",
-				Options:     options,
-			}
-		}
-	}
-
 	// Check for conflicts between named volumes, overlay & image volumes,
 	// and mounts
 	allMounts := make(map[string]bool)
@@ -723,3 +703,26 @@ func validChownFlag(flag string) (bool, error) {
 func unixPathClean(p string) string {
 	return path.Clean(p)
 }
+
+func addReadOnlyMounts(mounts []spec.Mount) []spec.Mount {
+	readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
+	options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
+	for _, dest := range readonlyTmpfs {
+		for _, m := range mounts {
+			if m.Destination == dest {
+				continue
+			}
+		}
+		mnt := spec.Mount{
+			Destination: dest,
+			Type:        define.TypeTmpfs,
+			Source:      define.TypeTmpfs,
+			Options:     options,
+		}
+		if dest != "/run" {
+			mnt.Options = append(mnt.Options, "noexec")
+		}
+		mounts = append(mounts, mnt)
+	}
+	return mounts
+}

test/e2e/generate_kube_test.go

@@ -71,10 +71,6 @@ var _ = Describe("Podman generate kube", func() {
 		Expect(pod.Spec.Containers[0]).To(HaveField("WorkingDir", ""))
 		Expect(pod.Spec.Containers[0].Env).To(BeNil())
 		Expect(pod).To(HaveField("Name", "top-pod"))
-		enableServiceLinks := false
-		Expect(pod.Spec).To(HaveField("EnableServiceLinks", &enableServiceLinks))
-		automountServiceAccountToken := false
-		Expect(pod.Spec).To(HaveField("AutomountServiceAccountToken", &automountServiceAccountToken))
 
 		numContainers := 0
 		for range pod.Spec.Containers {
@@ -169,10 +165,6 @@ var _ = Describe("Podman generate kube", func() {
 		err := yaml.Unmarshal(kube.Out.Contents(), pod)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(pod.Spec).To(HaveField("HostNetwork", false))
-		enableServiceLinks := false
-		Expect(pod.Spec).To(HaveField("EnableServiceLinks", &enableServiceLinks))
-		automountServiceAccountToken := false
-		Expect(pod.Spec).To(HaveField("AutomountServiceAccountToken", &automountServiceAccountToken))
 
 		numContainers := 0
 		for range pod.Spec.Containers {

test/system/700-play.bats

@@ -196,6 +196,30 @@ EOF
     run_podman rm -a
 }
 
+@test "podman kube play read-only" {
+    YAML=$PODMAN_TMPDIR/test.yml
+    run_podman create --pod new:pod1 --name test1 $IMAGE touch /testrw
+    run_podman create --pod pod1 --read-only --name test2 $IMAGE touch /testro
+    run_podman create --pod pod1 --read-only --name test3 $IMAGE touch /tmp/testtmp
+    run_podman kube generate pod1 -f $YAML
+
+    run_podman kube play --replace $YAML
+    run_podman container inspect --format '{{.HostConfig.ReadonlyRootfs}}' pod1-test1 pod1-test2 pod1-test3
+    is "$output" "false.*true.*true" "Rootfs should be read/only"
+
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test1
+    is "$output" "0" "Container / should be read/write"
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test2
+    is "$output" "1" "Container / should be read/only"
+    run_podman inspect --format "{{.State.ExitCode}}" pod1-test3
+    is "$output" "0" "/tmp in a read-only container should be read/write"
+
+    run_podman kube down - < $YAML
+    run_podman 1 container exists pod1-test1
+    run_podman 1 container exists pod1-test2
+    run_podman 1 container exists pod1-test3
+}
+
 @test "podman play with user from image" {
     TESTDIR=$PODMAN_TMPDIR/testdir
     mkdir -p $TESTDIR
@@ -416,4 +440,7 @@ spec:
     run_podman pod inspect test_pod --format "{{.InfraConfig.PortBindings}}"
     assert "$output" = "map[$HOST_PORT/tcp:[{ $HOST_PORT}]]"
     run_podman kube down $PODMAN_TMPDIR/testpod.yaml
+
+    run_podman pod rm -a -f
+    run_podman rm -a -f
 }