fix pod volume passing and alter infra inheritance

the infra Inherit function was not properly passing pod volume information to new containers alter the inherit function and struct to use the new `ConfigToSpec` function used in clone pick and choose the proper entities from a temp spec and validate them on the spegen side rather than passing directly to a config resolves containers#13548 Signed-off-by: cdoern <cbdoer23@g.holycross.edu>
cdoern · Mar 22, 2022 · 7106bcb · 7106bcb
1 parent b4b8b8b
commit 7106bcb
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 30 deletions.
diff --git a/libpod/container_config.go b/libpod/container_config.go
@@ -8,6 +8,7 @@ import (
 	"github.com/containers/common/pkg/secrets"
 	"github.com/containers/image/v5/manifest"
 	"github.com/containers/podman/v4/pkg/namespaces"
+	"github.com/containers/podman/v4/pkg/specgen"
 	"github.com/containers/storage"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 )
@@ -406,12 +407,7 @@ type ContainerMiscConfig struct {
 }
 
 type InfraInherit struct {
-	InfraSecurity     ContainerSecurityConfig
-	InfraLabels       []string                  `json:"labelopts,omitempty"`
-	InfraVolumes      []*ContainerNamedVolume   `json:"namedVolumes,omitempty"`
-	InfraOverlay      []*ContainerOverlayVolume `json:"overlayVolumes,omitempty"`
-	InfraImageVolumes []*ContainerImageVolume   `json:"ctrImageVolumes,omitempty"`
-	InfraUserVolumes  []string                  `json:"userVolumes,omitempty"`
-	InfraResources    *spec.LinuxResources      `json:"resources,omitempty"`
-	InfraDevices      []spec.LinuxDevice        `json:"device_host_src,omitempty"`
+	InfraSecurity  specgen.ContainerSecurityConfig `json:"container_security_config,omitempty"`
+	InfraResources *spec.LinuxResources            `json:"resource_limits,omitempty"`
+	InfraStorage   specgen.ContainerStorageConfig  `json:"container_storage_config,omitempty"`
 }
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
@@ -351,6 +351,10 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID s
 	conf.Systemd = nil
 	conf.Mounts = []string{}
 
+	if specg == nil {
+		specg = &specgen.SpecGenerator{}
+	}
+
 	specg.Pod = conf.Pod
 
 	matching, err := json.Marshal(conf)
@@ -483,5 +487,17 @@ func ConfigToSpec(rt *libpod.Runtime, specg *specgen.SpecGenerator, contaierID s
 	specg.OverlayVolumes = overlay
 	specg.Mounts = conf.Spec.Mounts
 	specg.HostDeviceList = conf.DeviceHostSrc
+	mapSecurityConfig(conf, specg)
 	return c, nil
 }
+
+// mapSecurityConfig takes a libpod.ContainerSecurityConfig and converts it to a specgen.ContinerSecurityConfig
+func mapSecurityConfig(c *libpod.ContainerConfig, s *specgen.SpecGenerator) {
+	s.Privileged = c.Privileged
+	if len(c.LabelOpts) > 0 {
+		s.SelinuxOpts = append(s.SelinuxOpts, c.LabelOpts...)
+	}
+	s.User = c.User
+	s.Groups = c.Groups
+	s.HostUsers = c.HostUsers
+}
diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
@@ -49,7 +49,7 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 	compatibleOptions := &libpod.InfraInherit{}
 	var infraSpec *spec.Spec
 	if infra != nil {
-		options, infraSpec, compatibleOptions, err = Inherit(*infra)
+		options, infraSpec, compatibleOptions, err = Inherit(*infra, s, rt)
 		if err != nil {
 			return nil, nil, nil, err
 		}
@@ -152,8 +152,8 @@ func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGener
 		return nil, nil, nil, err
 	}
 
-	infraVolumes := (len(compatibleOptions.InfraVolumes) > 0 || len(compatibleOptions.InfraUserVolumes) > 0 || len(compatibleOptions.InfraImageVolumes) > 0)
-	opts, err := createContainerOptions(ctx, rt, s, pod, finalVolumes, finalOverlays, imageData, command, infraVolumes, *compatibleOptions)
+	infraVol := (len(compatibleOptions.InfraStorage.Mounts) > 0 || len(compatibleOptions.InfraStorage.Volumes) > 0 || len(compatibleOptions.InfraStorage.ImageVolumes) > 0 || len(compatibleOptions.InfraStorage.OverlayVolumes) > 0)
+	opts, err := createContainerOptions(ctx, rt, s, pod, finalVolumes, finalOverlays, imageData, command, infraVol, *compatibleOptions)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -437,7 +437,7 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
 	if len(s.SelinuxOpts) > 0 {
 		options = append(options, libpod.WithSecLabels(s.SelinuxOpts))
 	} else {
-		if pod != nil && len(compatibleOptions.InfraLabels) == 0 {
+		if pod != nil && len(compatibleOptions.InfraSecurity.SelinuxOpts) == 0 {
 			// duplicate the security options from the pod
 			processLabel, err := pod.ProcessLabel()
 			if err != nil {
@@ -535,32 +535,38 @@ func createContainerOptions(ctx context.Context, rt *libpod.Runtime, s *specgen.
 	return options, nil
 }
 
-func Inherit(infra libpod.Container) (opts []libpod.CtrCreateOption, infraS *spec.Spec, compat *libpod.InfraInherit, err error) {
+func Inherit(infra libpod.Container, s *specgen.SpecGenerator, rt *libpod.Runtime) (opts []libpod.CtrCreateOption, infraS *spec.Spec, compat *libpod.InfraInherit, err error) {
+	inheritSpec := &specgen.SpecGenerator{}
+	_, err = ConfigToSpec(rt, inheritSpec, infra.ID())
+	if err != nil {
+		return nil, nil, nil, err
+	}
 	options := []libpod.CtrCreateOption{}
 	compatibleOptions := &libpod.InfraInherit{}
 	infraConf := infra.Config()
 	infraSpec := infraConf.Spec
 
-	config, err := json.Marshal(infraConf)
+	// double unmarshal, once into compat options and then again into the spec itself
+	// this is to avoid taking everything from infra's spec
+	specG, err := json.Marshal(inheritSpec)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	err = json.Unmarshal(config, compatibleOptions)
+
+	err = json.Unmarshal(specG, compatibleOptions)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	if infraSpec.Linux != nil && infraSpec.Linux.Resources != nil {
-		resources, err := json.Marshal(infraSpec.Linux.Resources)
-		if err != nil {
-			return nil, nil, nil, err
-		}
-		err = json.Unmarshal(resources, &compatibleOptions.InfraResources)
-		if err != nil {
-			return nil, nil, nil, err
-		}
+
+	var compatByte []byte
+	compatByte, err = json.Marshal(compatibleOptions)
+	if err != nil {
+		return nil, nil, nil, err
 	}
-	if compatibleOptions != nil {
-		options = append(options, libpod.WithInfraConfig(*compatibleOptions))
+
+	err = json.Unmarshal(compatByte, s)
+	if err != nil {
+		return nil, nil, nil, err
 	}
 	return options, infraSpec, compatibleOptions, nil
 }
diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go
@@ -352,8 +352,8 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 				return nil, err
 			}
 		}
-		if len(compatibleOptions.InfraDevices) > 0 && len(s.Devices) == 0 {
-			userDevices = compatibleOptions.InfraDevices
+		if len(compatibleOptions.InfraStorage.HostDeviceList) > 0 && len(s.Devices) == 0 {
+			userDevices = compatibleOptions.InfraStorage.HostDeviceList
 		} else {
 			userDevices = s.Devices
 		}

diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go
@@ -523,8 +523,8 @@ type ContainerHealthCheckConfig struct {
 // swagger:model SpecGenerator
 type SpecGenerator struct {
 	ContainerBasicConfig
-	ContainerStorageConfig
-	ContainerSecurityConfig
+	ContainerStorageConfig  `json:"container_storage_config,omitempty"`
+	ContainerSecurityConfig `json:"container_security_config,omitempty"`
 	ContainerCgroupConfig
 	ContainerNetworkConfig
 	ContainerResourceConfig

diff --git a/test/e2e/pod_create_test.go b/test/e2e/pod_create_test.go
@@ -874,6 +874,10 @@ ENTRYPOINT ["sleep","99999"]
 		ctr3 := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "cat", "/tmp1/test"})
 		ctr3.WaitWithDefaultTimeout()
 		Expect(ctr3.OutputToString()).To(ContainSubstring("hello"))
+
+		ctr4 := podmanTest.Podman([]string{"run", "--pod", podName, ALPINE, "touch", "/tmp1/testing.txt"})
+		ctr4.WaitWithDefaultTimeout()
+		Expect(ctr4).Should(Exit(0))
 	})
 
 	It("podman pod create --device", func() {