Merge pull request #22727 from mheon/chown_all_the_time

Always chown volumes when mounting into a container
This commit is contained in:
openshift-merge-bot[bot]
2024-05-23 12:34:07 +00:00
committed by GitHub
7 changed files with 152 additions and 22 deletions

View File

@ -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**

View File

@ -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
}

View File

@ -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

View File

@ -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.

View File

@ -110,4 +110,5 @@ func (v *Volume) refresh() error {
func resetVolumeState(state *VolumeState) {
state.MountCount = 0
state.MountPoint = ""
state.CopiedUp = false
}

View File

@ -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)
})
})

View File

@ -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"