Skip to content

Commit

Permalink
Fix volumes with uid and gid options
Browse files Browse the repository at this point in the history
Podman uses the volume option map to check if it has to mount the volume
or not when the container is started. Commit 28138da added to uid
and gid options to this map, however when only uid/gid is set we cannot
mount this volume because there is no filesystem or device specified.
Make sure we do not try to mount the volume when only the uid/gid option
is set since this is a simple chown operation.

Also when a uid/gid is explicity set, do not chown the volume based on
the container user when the volume is used for the first time.

Fixes containers#10620

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 authored and mheon committed Jun 25, 2021
1 parent c2dcb3e commit 647c202
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 4 deletions.
13 changes: 13 additions & 0 deletions libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,19 @@ func WithVolumeGID(gid int) VolumeCreateOption {
}
}

// WithVolumeNoChown prevents the volume from being chowned to the process uid at first use.
func WithVolumeNoChown() VolumeCreateOption {
return func(volume *Volume) error {
if volume.valid {
return define.ErrVolumeFinalized
}

volume.state.NeedsChown = false

return nil
}
}

// withSetAnon sets a bool notifying libpod that this volume is anonymous and
// should be removed when containers using it are removed and volumes are
// specified for removal.
Expand Down
19 changes: 17 additions & 2 deletions libpod/volume_internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,23 @@ func (v *Volume) needsMount() bool {
return true
}

// Local driver with options needs mount
return len(v.config.Options) > 0
// Commit 28138dafcc added the UID and GID options to this map
// However we should only mount when options other than uid and gid are set.
// see https://github.com/containers/podman/issues/10620
index := 0
if _, ok := v.config.Options["UID"]; ok {
index++
}
if _, ok := v.config.Options["GID"]; ok {
index++
}
// when uid or gid is set there is also the "o" option
// set so we have to ignore this one as well
if index > 0 {
index++
}
// Local driver with options other than uid,gid needs mount
return len(v.config.Options) > index
}

// update() updates the volume state from the DB.
Expand Down
4 changes: 2 additions & 2 deletions pkg/domain/infra/abi/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error)
return nil, errors.Wrapf(err, "cannot convert UID %s to integer", splitO[1])
}
logrus.Debugf("Removing uid= from options and adding WithVolumeUID for UID %d", intUID)
libpodOptions = append(libpodOptions, libpod.WithVolumeUID(intUID))
libpodOptions = append(libpodOptions, libpod.WithVolumeUID(intUID), libpod.WithVolumeNoChown())
finalVal = append(finalVal, o)
// set option "UID": "$uid"
volumeOptions["UID"] = splitO[1]
Expand All @@ -50,7 +50,7 @@ func VolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error)
return nil, errors.Wrapf(err, "cannot convert GID %s to integer", splitO[1])
}
logrus.Debugf("Removing gid= from options and adding WithVolumeGID for GID %d", intGID)
libpodOptions = append(libpodOptions, libpod.WithVolumeGID(intGID))
libpodOptions = append(libpodOptions, libpod.WithVolumeGID(intGID), libpod.WithVolumeNoChown())
finalVal = append(finalVal, o)
// set option "GID": "$gid"
volumeOptions["GID"] = splitO[1]
Expand Down
32 changes: 32 additions & 0 deletions test/e2e/run_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,4 +668,36 @@ USER testuser`, fedoraMinimal)
Expect(strings.Contains(test2.OutputToString(), testString)).To(BeTrue())

})

It("podman volume with uid and gid works", func() {
volName := "testVol"
volCreate := podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000", volName})
volCreate.WaitWithDefaultTimeout()
Expect(volCreate.ExitCode()).To(Equal(0))

volMount := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%u", "/test"})
volMount.WaitWithDefaultTimeout()
Expect(volMount.ExitCode()).To(Equal(0))
Expect(volMount.OutputToString()).To(Equal("1000"))

volName = "testVol2"
volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=gid=1000", volName})
volCreate.WaitWithDefaultTimeout()
Expect(volCreate.ExitCode()).To(Equal(0))

volMount = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%g", "/test"})
volMount.WaitWithDefaultTimeout()
Expect(volMount.ExitCode()).To(Equal(0))
Expect(volMount.OutputToString()).To(Equal("1000"))

volName = "testVol3"
volCreate = podmanTest.Podman([]string{"volume", "create", "--opt", "o=uid=1000,gid=1000", volName})
volCreate.WaitWithDefaultTimeout()
Expect(volCreate.ExitCode()).To(Equal(0))

volMount = podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "stat", "-c", "%u:%g", "/test"})
volMount.WaitWithDefaultTimeout()
Expect(volMount.ExitCode()).To(Equal(0))
Expect(volMount.OutputToString()).To(Equal("1000:1000"))
})
})

0 comments on commit 647c202

Please sign in to comment.