Use semi-colon as the field separator for internal volumes-from inspect annotation

The current field separator comma of the inspect annotation conflicts with the mount options of --volumes-from as the mount options itself can be comma separated.

Signed-off-by: Vikas Goel <[email protected]>

Author: vikas-goel

libpod/container_inspect.go

@@ -54,7 +54,7 @@ func (c *Container) volumesFrom() ([]string, error) {
 		return nil, err
 	}
 	if ctrs, ok := ctrSpec.Annotations[define.InspectAnnotationVolumesFrom]; ok {
-		return strings.Split(ctrs, ","), nil
+		return strings.Split(ctrs, ";"), nil
 	}
 	return nil, nil
 }
@@ -511,7 +511,7 @@ func (c *Container) generateInspectContainerHostConfig(ctrSpec *spec.Spec, named
 			hostConfig.AutoRemove = true
 		}
 		if ctrs, ok := ctrSpec.Annotations[define.InspectAnnotationVolumesFrom]; ok {
-			hostConfig.VolumesFrom = strings.Split(ctrs, ",")
+			hostConfig.VolumesFrom = strings.Split(ctrs, ";")
 		}
 		if ctrSpec.Annotations[define.InspectAnnotationPrivileged] == define.InspectResponseTrue {
 			hostConfig.Privileged = true

libpod/pod.go

@@ -278,7 +278,7 @@ func (p *Pod) VolumesFrom() []string {
 		return nil
 	}
 	if ctrs, ok := infra.config.Spec.Annotations[define.InspectAnnotationVolumesFrom]; ok {
-		return strings.Split(ctrs, ",")
+		return strings.Split(ctrs, ";")
 	}
 	return nil
 }

pkg/specgen/generate/oci_freebsd.go

@@ -152,7 +152,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 
 	if len(s.VolumesFrom) > 0 {
-		configSpec.Annotations[define.InspectAnnotationVolumesFrom] = strings.Join(s.VolumesFrom, ",")
+		configSpec.Annotations[define.InspectAnnotationVolumesFrom] = strings.Join(s.VolumesFrom, ";")
 	}
 
 	if s.IsPrivileged() {

pkg/specgen/generate/oci_linux.go

@@ -322,7 +322,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 
 	if len(s.VolumesFrom) > 0 {
-		configSpec.Annotations[define.InspectAnnotationVolumesFrom] = strings.Join(s.VolumesFrom, ",")
+		configSpec.Annotations[define.InspectAnnotationVolumesFrom] = strings.Join(s.VolumesFrom, ";")
 	}
 
 	if s.IsPrivileged() {

test/e2e/container_inspect_test.go

@@ -1,6 +1,9 @@
 package integration
 
 import (
+	"os"
+	"path/filepath"
+
 	"github.com/containers/podman/v4/libpod/define"
 	"github.com/containers/podman/v4/pkg/annotations"
 	. "github.com/containers/podman/v4/test/utils"
@@ -44,4 +47,27 @@ var _ = Describe("Podman container inspect", func() {
 		Expect(data[0].NetworkSettings.Ports).
 			To(Equal(map[string][]define.InspectHostPort{"80/tcp": nil, "8989/tcp": nil}))
 	})
+
+	It("podman inspect shows volumes-from with mount options", func() {
+		ctr1 := "volfctr"
+		ctr2 := "voltctr"
+		vol1 := filepath.Join(podmanTest.TempDir, "vol-test1")
+		volsctr := ctr1 + ":z,ro"
+
+		err := os.MkdirAll(vol1, 0755)
+		Expect(err).ToNot(HaveOccurred())
+
+		session := podmanTest.Podman([]string{"create", "--name", ctr1, "-v", vol1, CITEST_IMAGE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		session = podmanTest.Podman([]string{"create", "--volumes-from", volsctr, "--name", ctr2, CITEST_IMAGE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		data := podmanTest.InspectContainer(ctr2)
+		Expect(data).To(HaveLen(1))
+		Expect(data[0].HostConfig.VolumesFrom).To(Equal([]string{volsctr}))
+		Expect(data[0].Config.Annotations[define.InspectAnnotationVolumesFrom]).To(Equal(volsctr))
+	})
 })

test/e2e/generate_kube_test.go

@@ -1699,6 +1699,44 @@ USER test1`
 		Expect(pod.Annotations).To(HaveKeyWithValue(define.InspectAnnotationVolumesFrom+"/"+ctr2, ctr1))
 	})
 
+	It("pod volumes-from annotation with semicolon as field separator", func() {
+		// Assert that volumes-from annotation for multiple source
+		// containers along with their mount options are getting
+		// generated with semicolon as the field separator.
+
+		srcctr1, srcctr2, tgtctr := "srcctr1", "srcctr2", "tgtctr"
+		frmopt1, frmopt2 := srcctr1+":ro", srcctr2+":ro"
+		vol1 := filepath.Join(podmanTest.TempDir, "vol-test1")
+		vol2 := filepath.Join(podmanTest.TempDir, "vol-test2")
+
+		err1 := os.MkdirAll(vol1, 0755)
+		Expect(err1).ToNot(HaveOccurred())
+
+		err2 := os.MkdirAll(vol2, 0755)
+		Expect(err2).ToNot(HaveOccurred())
+
+		session := podmanTest.Podman([]string{"create", "--name", srcctr1, "-v", vol1, CITEST_IMAGE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		session = podmanTest.Podman([]string{"create", "--name", srcctr2, "-v", vol2, CITEST_IMAGE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		session = podmanTest.Podman([]string{"create", "--volumes-from", frmopt1, "--volumes-from", frmopt2, "--name", tgtctr, CITEST_IMAGE})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(ExitCleanly())
+
+		kube := podmanTest.Podman([]string{"kube", "generate", "--podman-only", tgtctr})
+		kube.WaitWithDefaultTimeout()
+		Expect(kube).Should(ExitCleanly())
+
+		pod := new(v1.Pod)
+		err3 := yaml.Unmarshal(kube.Out.Contents(), pod)
+		Expect(err3).ToNot(HaveOccurred())
+		Expect(pod.Annotations).To(HaveKeyWithValue(define.InspectAnnotationVolumesFrom+"/"+tgtctr, frmopt1+";"+frmopt2))
+	})
+
 	It("--podman-only on container with --rm", func() {
 		ctr := "ctr"