Add parsing for UID, GID in volume "o" option

Everything else is a flag to mount, but "uid" and "gid" are not. We need to parse them out of "o" and handle them separately. Signed-off-by: Matthew Heon <[email protected]>
containers · Oct 22, 2019 · f60a814 · f60a814
1 parent d358840
commit f60a814
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 3 deletions.
diff --git a/cmd/podman/shared/volumes_shared.go b/cmd/podman/shared/volumes_shared.go
@@ -2,8 +2,13 @@ package shared
 
 import (
 	"context"
+	"strconv"
+	"strings"
 
 	"github.com/containers/libpod/libpod"
+	"github.com/containers/libpod/libpod/define"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // Remove given set of volumes
@@ -45,3 +50,60 @@ func SharedRemoveVolumes(ctx context.Context, runtime *libpod.Runtime, vols []st
 
 	return success, failed, nil
 }
+
+// Handle volume options from CLI.
+// Parse "o" option to find UID, GID.
+func ParseVolumeOptions(opts map[string]string) ([]libpod.VolumeCreateOption, error) {
+	libpodOptions := []libpod.VolumeCreateOption{}
+	volumeOptions := make(map[string]string)
+
+	for key, value := range opts {
+		switch key {
+		case "o":
+			// o has special handling to parse out UID, GID.
+			// These are separate Libpod options.
+			splitVal := strings.Split(value, ",")
+			finalVal := []string{}
+			for _, o := range splitVal {
+				// Options will be formatted as either "opt" or
+				// "opt=value"
+				splitO := strings.SplitN(o, "=", 2)
+				switch strings.ToLower(splitO[0]) {
+				case "uid":
+					if len(splitO) != 2 {
+						return nil, errors.Wrapf(define.ErrInvalidArg, "uid option must provide a UID")
+					}
+					intUID, err := strconv.Atoi(splitO[1])
+					if err != nil {
+						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))
+				case "gid":
+					if len(splitO) != 2 {
+						return nil, errors.Wrapf(define.ErrInvalidArg, "gid option must provide a GID")
+					}
+					intGID, err := strconv.Atoi(splitO[1])
+					if err != nil {
+						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))
+				default:
+					finalVal = append(finalVal, o)
+				}
+			}
+			if len(finalVal) > 0 {
+				volumeOptions[key] = strings.Join(finalVal, ",")
+			}
+		default:
+			volumeOptions[key] = value
+		}
+	}
+
+	if len(volumeOptions) > 0 {
+		libpodOptions = append(libpodOptions, libpod.WithVolumeOptions(volumeOptions))
+	}
+
+	return libpodOptions, nil
+}
diff --git a/cmd/podman/volume_create.go b/cmd/podman/volume_create.go
@@ -37,7 +37,7 @@ func init() {
 	flags := volumeCreateCommand.Flags()
 	flags.StringVar(&volumeCreateCommand.Driver, "driver", "", "Specify volume driver name (default local)")
 	flags.StringSliceVarP(&volumeCreateCommand.Label, "label", "l", []string{}, "Set metadata for a volume (default [])")
-	flags.StringSliceVarP(&volumeCreateCommand.Opt, "opt", "o", []string{}, "Set driver specific options (default [])")
+	flags.StringArrayVarP(&volumeCreateCommand.Opt, "opt", "o", []string{}, "Set driver specific options (default [])")
 }
 
 func volumeCreateCmd(c *cliconfig.VolumeCreateValues) error {

diff --git a/libpod/options.go b/libpod/options.go
@@ -1487,6 +1487,8 @@ func WithVolumeLabels(labels map[string]string) VolumeCreateOption {
 }
 
 // WithVolumeOptions sets the options of the volume.
+// If the "local" driver has been selected, options will be validated. There are
+// currently 3 valid options for the "local" driver - o, type, and device.
 func WithVolumeOptions(options map[string]string) VolumeCreateOption {
 	return func(volume *Volume) error {
 		if volume.valid {
@@ -1495,6 +1497,13 @@ func WithVolumeOptions(options map[string]string) VolumeCreateOption {
 
 		volume.config.Options = make(map[string]string)
 		for key, value := range options {
+			switch key {
+			case "type", "device", "o":
+				volume.config.Options[key] = value
+			default:
+				return errors.Wrapf(define.ErrInvalidArg, "unrecognized volume option %q is not supported with local driver", key)
+			}
+
 			volume.config.Options[key] = value
 		}
 

diff --git a/pkg/adapter/runtime.go b/pkg/adapter/runtime.go
@@ -186,7 +186,12 @@ func (r *LocalRuntime) CreateVolume(ctx context.Context, c *cliconfig.VolumeCrea
 	}
 
 	if len(opts) != 0 {
-		options = append(options, libpod.WithVolumeOptions(opts))
+		// We need to process -o for uid, gid
+		parsedOptions, err := shared.ParseVolumeOptions(opts)
+		if err != nil {
+			return "", err
+		}
+		options = append(options, parsedOptions...)
 	}
 	newVolume, err := r.NewVolume(ctx, options...)
 	if err != nil {

diff --git a/pkg/varlinkapi/volumes.go b/pkg/varlinkapi/volumes.go
@@ -24,7 +24,11 @@ func (i *LibpodAPI) VolumeCreate(call iopodman.VarlinkCall, options iopodman.Vol
 		volumeOptions = append(volumeOptions, libpod.WithVolumeLabels(options.Labels))
 	}
 	if len(options.Options) > 0 {
-		volumeOptions = append(volumeOptions, libpod.WithVolumeOptions(options.Options))
+		parsedOptions, err := shared.ParseVolumeOptions(options.Options)
+		if err != nil {
+			return call.ReplyErrorOccurred(err.Error())
+		}
+		volumeOptions = append(volumeOptions, parsedOptions...)
 	}
 	newVolume, err := i.Runtime.NewVolume(getContext(), volumeOptions...)
 	if err != nil {

diff --git a/test/e2e/volume_create_test.go b/test/e2e/volume_create_test.go
@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/containers/libpod/test/utils"
@@ -63,4 +64,23 @@ var _ = Describe("Podman volume create", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session).To(ExitWithError())
 	})
+
+	It("podman create volume with o=uid,gid", func() {
+		volName := "testVol"
+		uid := "3000"
+		gid := "4000"
+		session := podmanTest.Podman([]string{"volume", "create", "--opt", fmt.Sprintf("o=uid=%s,gid=%s", uid, gid), volName})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		inspectUID := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{ .UID }}", volName})
+		inspectUID.WaitWithDefaultTimeout()
+		Expect(inspectUID.ExitCode()).To(Equal(0))
+		Expect(inspectUID.OutputToString()).To(Equal(uid))
+
+		inspectGID := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{ .GID }}", volName})
+		inspectGID.WaitWithDefaultTimeout()
+		Expect(inspectGID.ExitCode()).To(Equal(0))
+		Expect(inspectGID.OutputToString()).To(Equal(gid))
+	})
 })