diff --git a/docs/source/markdown/podman-volume-inspect.1.md b/docs/source/markdown/podman-volume-inspect.1.md index 7e7e831feb..ed1a109d05 100644 --- a/docs/source/markdown/podman-volume-inspect.1.md +++ b/docs/source/markdown/podman-volume-inspect.1.md @@ -26,25 +26,25 @@ Format volume output using Go template Valid placeholders for the Go template are listed below: -| **Placeholder** | **Description** | -| ------------------- | ------------------------------------------------------ | -| .Anonymous | Indicates whether volume is anonymous | -| .CreatedAt ... | Volume creation time | -| .Driver | Volume driver | -| .GID | GID the volume was created with | -| .Labels ... | Label information associated with the volume | -| .LockNumber | Number of the volume's Libpod lock | -| .MountCount | Number of times the volume is mounted | -| .Mountpoint | Source of volume mount point | -| .Name | Volume name | -| .NeedsChown | Indicates volume needs to be chowned on first use | -| .NeedsCopyUp | Indicates volume needs dest data copied up on first use| -| .Options ... | Volume options | -| .Scope | Volume scope | -| .Status ... | Status of the volume | -| .StorageID | StorageID of the volume | -| .Timeout | Timeout of the volume | -| .UID | UID the volume was created with | +| **Placeholder** | **Description** | +| ------------------- | --------------------------------------------------------------------------- | +| .Anonymous | Indicates whether volume is anonymous | +| .CreatedAt ... | Volume creation time | +| .Driver | Volume driver | +| .GID | GID the volume was created with | +| .Labels ... | Label information associated with the volume | +| .LockNumber | Number of the volume's Libpod lock | +| .MountCount | Number of times the volume is mounted | +| .Mountpoint | Source of volume mount point | +| .Name | Volume name | +| .NeedsChown | Indicates volume will be chowned on next use | +| .NeedsCopyUp | Indicates data at the destination will be copied into the volume on next use| +| .Options ... | Volume options | +| .Scope | Volume scope | +| .Status ... | Status of the volume | +| .StorageID | StorageID of the volume | +| .Timeout | Timeout of the volume | +| .UID | UID the volume was created with | #### **--help** diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 62eb460634..d3302a5978 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1901,6 +1901,7 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) // Set NeedsCopyUp to false since we are about to do first copy // Do not copy second time. vol.state.NeedsCopyUp = false + vol.state.CopiedUp = true if err := vol.save(); err != nil { return nil, err } diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index d76dea2995..70f6f741f5 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2868,11 +2868,26 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { return err } + // If the volume is not empty, and it is not the first copy-up event - + // we should not do a chown. + if vol.state.NeedsChown && !vol.state.CopiedUp { + contents, err := os.ReadDir(vol.mountPoint()) + if err != nil { + return fmt.Errorf("reading contents of volume %q: %w", vol.Name(), err) + } + // Not empty, do nothing and unset NeedsChown. + if len(contents) > 0 { + vol.state.NeedsChown = false + if err := vol.save(); err != nil { + return fmt.Errorf("saving volume %q state: %w", vol.Name(), err) + } + return nil + } + } + // Volumes owned by a volume driver are not chowned - we don't want to // mess with a mount not managed by us. if vol.state.NeedsChown && (!vol.UsesVolumeDriver() && vol.config.Driver != "image") { - vol.state.NeedsChown = false - uid := int(c.config.Spec.Process.User.UID) gid := int(c.config.Spec.Process.User.GID) @@ -2891,6 +2906,10 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { gid = newPair.GID } + if vol.state.CopiedUp { + vol.state.NeedsChown = false + } + vol.state.CopiedUp = false vol.state.UIDChowned = uid vol.state.GIDChowned = gid @@ -2935,6 +2954,16 @@ func (c *Container) fixVolumePermissions(v *ContainerNamedVolume) error { if err := idtools.SafeLchown(mountPoint, uid, gid); err != nil { return err } + + // UID/GID 0 are sticky - if we chown to root, + // we stop chowning thereafter. + if uid == 0 && gid == 0 && vol.state.NeedsChown { + vol.state.NeedsChown = false + + if err := vol.save(); err != nil { + return fmt.Errorf("saving volume %q state to database: %w", vol.Name(), err) + } + } } if err := os.Chmod(mountPoint, st.Mode()); err != nil { return err diff --git a/libpod/volume.go b/libpod/volume.go index 4b5a224f80..05acb2a6e5 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -98,6 +98,10 @@ type VolumeState struct { // a container, the container will chown the volume to the container process // UID/GID. NeedsChown bool `json:"notYetChowned,omitempty"` + // Indicates that a copy-up event occurred during the current mount of + // the volume into a container. + // We use this to determine if a chown is appropriate. + CopiedUp bool `json:"copiedUp,omitempty"` // UIDChowned is the UID the volume was chowned to. UIDChowned int `json:"uidChowned,omitempty"` // GIDChowned is the GID the volume was chowned to. diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index 3d308c8620..e512582305 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -110,4 +110,5 @@ func (v *Volume) refresh() error { func resetVolumeState(state *VolumeState) { state.MountCount = 0 state.MountPoint = "" + state.CopiedUp = false } diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index df6fd368f8..c9cf658163 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -30,6 +30,14 @@ var _ = Describe("Podman run with volumes", func() { return strings.Fields(session.OutputToString()) } + //nolint:unparam + mountVolumeAndCheckDirectory := func(volName, volPath, expectedOwner, imgName string) { + check := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:%s", volName, volPath), imgName, "stat", "-c", "%U:%G", volPath}) + check.WaitWithDefaultTimeout() + Expect(check).Should(ExitCleanly()) + Expect(check.OutputToString()).Should(ContainSubstring(fmt.Sprintf("%s:%s", expectedOwner, expectedOwner))) + } + It("podman run with volume flag", func() { mountPath := filepath.Join(podmanTest.TempDir, "secrets") err = os.Mkdir(mountPath, 0755) @@ -970,4 +978,89 @@ USER testuser`, CITEST_IMAGE) Expect(run1.OutputToString()).Should(Equal(run2.OutputToString())) }) + + It("podman run -v chowns multiple times on empty volume", func() { + imgName := "testimg" + dockerfile := fmt.Sprintf(`FROM %s +RUN addgroup -g 1234 test1 +RUN addgroup -g 4567 test2 +RUN addgroup -g 7890 test3 +RUN adduser -D -u 1234 -G test1 test1 +RUN adduser -D -u 4567 -G test2 test2 +RUN adduser -D -u 7890 -G test3 test3 +RUN mkdir /test1 /test2 /test3 /test4 +RUN chown test1:test1 /test1 +RUN chown test2:test2 /test2 +RUN chown test3:test3 /test4 +RUN chmod 755 /test1 /test2 /test3 /test4`, ALPINE) + podmanTest.BuildImage(dockerfile, imgName, "false") + + volName := "testVol" + volCreate := podmanTest.Podman([]string{"volume", "create", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate).Should(ExitCleanly()) + + mountVolumeAndCheckDirectory(volName, "/test1", "test1", imgName) + mountVolumeAndCheckDirectory(volName, "/test2", "test2", imgName) + mountVolumeAndCheckDirectory(volName, "/test3", "root", imgName) + mountVolumeAndCheckDirectory(volName, "/test4", "root", imgName) + }) + + It("podman run -v chowns until copy-up on volume", func() { + imgName := "testimg" + dockerfile := fmt.Sprintf(`FROM %s +RUN addgroup -g 1234 test1 +RUN addgroup -g 4567 test2 +RUN addgroup -g 7890 test3 +RUN adduser -D -u 1234 -G test1 test1 +RUN adduser -D -u 4567 -G test2 test2 +RUN adduser -D -u 7890 -G test3 test3 +RUN mkdir /test1 /test2 /test3 +RUN touch /test2/file1 +RUN chown test1:test1 /test1 +RUN chown -R test2:test2 /test2 +RUN chown test3:test3 /test3 +RUN chmod 755 /test1 /test2 /test3`, ALPINE) + podmanTest.BuildImage(dockerfile, imgName, "false") + + volName := "testVol" + volCreate := podmanTest.Podman([]string{"volume", "create", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate).Should(ExitCleanly()) + + mountVolumeAndCheckDirectory(volName, "/test1", "test1", imgName) + mountVolumeAndCheckDirectory(volName, "/test2", "test2", imgName) + mountVolumeAndCheckDirectory(volName, "/test3", "test2", imgName) + }) + + It("podman run -v chowns until volume has contents", func() { + imgName := "testimg" + dockerfile := fmt.Sprintf(`FROM %s +RUN addgroup -g 1234 test1 +RUN addgroup -g 4567 test2 +RUN addgroup -g 7890 test3 +RUN adduser -D -u 1234 -G test1 test1 +RUN adduser -D -u 4567 -G test2 test2 +RUN adduser -D -u 7890 -G test3 test3 +RUN mkdir /test1 /test2 /test3 +RUN chown test1:test1 /test1 +RUN chown test2:test2 /test2 +RUN chown test3:test3 /test3 +RUN chmod 755 /test1 /test2 /test3`, ALPINE) + podmanTest.BuildImage(dockerfile, imgName, "false") + + volName := "testVol" + volCreate := podmanTest.Podman([]string{"volume", "create", volName}) + volCreate.WaitWithDefaultTimeout() + Expect(volCreate).Should(ExitCleanly()) + + mountVolumeAndCheckDirectory(volName, "/test1", "test1", imgName) + mountVolumeAndCheckDirectory(volName, "/test2", "test2", imgName) + + session := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:/test2", volName), imgName, "touch", "/test2/file1"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitCleanly()) + + mountVolumeAndCheckDirectory(volName, "/test3", "test2", imgName) + }) }) diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index ad64d71936..2323c33a7d 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -467,11 +467,13 @@ NeedsChown | true run_podman volume inspect --format '{{ .NeedsCopyUp }}' $myvolume is "${output}" "true" "If content in dest '/vol' empty NeedsCopyUP should still be true" run_podman volume inspect --format '{{ .NeedsChown }}' $myvolume - is "${output}" "false" "After first use within a container NeedsChown should still be false" + is "${output}" "true" "No copy up occurred so the NeedsChown will still be true" run_podman run --rm --volume $myvolume:/etc $IMAGE ls /etc/passwd run_podman volume inspect --format '{{ .NeedsCopyUp }}' $myvolume is "${output}" "false" "If content in dest '/etc' non-empty NeedsCopyUP should still have happened and be false" + run_podman volume inspect --format '{{ .NeedsChown }}' $myvolume + is "${output}" "false" "Content has been copied up into volume, needschown will be false" run_podman volume inspect --format '{{.Mountpoint}}' $myvolume mountpoint="$output"