diff --git a/cmd/podman/pod_create.go b/cmd/podman/pod_create.go index cee6476eaa..0f72780f94 100644 --- a/cmd/podman/pod_create.go +++ b/cmd/podman/pod_create.go @@ -6,6 +6,7 @@ import ( "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/cmd/podman/shared" + "github.com/containers/libpod/cmd/podman/shared/parse" "github.com/containers/libpod/libpod/define" "github.com/containers/libpod/pkg/adapter" "github.com/containers/libpod/pkg/errorhandling" @@ -103,7 +104,7 @@ func podCreateCmd(c *cliconfig.PodCreateValues) error { defer errorhandling.SyncQuiet(podIdFile) } - labels, err := shared.GetAllLabels(c.LabelFile, c.Labels) + labels, err := parse.GetAllLabels(c.LabelFile, c.Labels) if err != nil { return errors.Wrapf(err, "unable to process labels") } diff --git a/cmd/podman/shared/create.go b/cmd/podman/shared/create.go index be5adcccbb..99538b3dc8 100644 --- a/cmd/podman/shared/create.go +++ b/cmd/podman/shared/create.go @@ -488,7 +488,7 @@ func ParseCreateOpts(ctx context.Context, c *GenericCLIResults, runtime *libpod. } // LABEL VARIABLES - labels, err := GetAllLabels(c.StringSlice("label-file"), c.StringArray("label")) + labels, err := parse.GetAllLabels(c.StringSlice("label-file"), c.StringArray("label")) if err != nil { return nil, errors.Wrapf(err, "unable to process labels") } diff --git a/cmd/podman/shared/create_cli.go b/cmd/podman/shared/create_cli.go index 00b83906dd..10e27350b8 100644 --- a/cmd/podman/shared/create_cli.go +++ b/cmd/podman/shared/create_cli.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/containers/libpod/cmd/podman/shared/parse" "github.com/containers/libpod/pkg/cgroups" cc "github.com/containers/libpod/pkg/spec" "github.com/containers/libpod/pkg/sysinfo" @@ -12,16 +11,6 @@ import ( "github.com/sirupsen/logrus" ) -// GetAllLabels ... -func GetAllLabels(labelFile, inputLabels []string) (map[string]string, error) { - labels := make(map[string]string) - labelErr := parse.ReadKVStrings(labels, labelFile, inputLabels) - if labelErr != nil { - return labels, errors.Wrapf(labelErr, "unable to process labels from --label and label-file") - } - return labels, nil -} - // validateSysctl validates a sysctl and returns it. func validateSysctl(strSlice []string) (map[string]string, error) { sysctl := make(map[string]string) diff --git a/cmd/podman/shared/create_cli_test.go b/cmd/podman/shared/create_cli_test.go index fea1a2390d..a045962cba 100644 --- a/cmd/podman/shared/create_cli_test.go +++ b/cmd/podman/shared/create_cli_test.go @@ -1,33 +1,11 @@ package shared import ( - "io/ioutil" - "os" "testing" "github.com/stretchr/testify/assert" ) -var ( - Var1 = []string{"ONE=1", "TWO=2"} -) - -func createTmpFile(content []byte) (string, error) { - tmpfile, err := ioutil.TempFile(os.TempDir(), "unittest") - if err != nil { - return "", err - } - - if _, err := tmpfile.Write(content); err != nil { - return "", err - - } - if err := tmpfile.Close(); err != nil { - return "", err - } - return tmpfile.Name(), nil -} - func TestValidateSysctl(t *testing.T) { strSlice := []string{"net.core.test1=4", "kernel.msgmax=2"} result, _ := validateSysctl(strSlice) @@ -39,32 +17,3 @@ func TestValidateSysctlBadSysctl(t *testing.T) { _, err := validateSysctl(strSlice) assert.Error(t, err) } - -func TestGetAllLabels(t *testing.T) { - fileLabels := []string{} - labels, _ := GetAllLabels(fileLabels, Var1) - assert.Equal(t, len(labels), 2) -} - -func TestGetAllLabelsBadKeyValue(t *testing.T) { - inLabels := []string{"=badValue", "="} - fileLabels := []string{} - _, err := GetAllLabels(fileLabels, inLabels) - assert.Error(t, err, assert.AnError) -} - -func TestGetAllLabelsBadLabelFile(t *testing.T) { - fileLabels := []string{"/foobar5001/be"} - _, err := GetAllLabels(fileLabels, Var1) - assert.Error(t, err, assert.AnError) -} - -func TestGetAllLabelsFile(t *testing.T) { - content := []byte("THREE=3") - tFile, err := createTmpFile(content) - defer os.Remove(tFile) - assert.NoError(t, err) - fileLabels := []string{tFile} - result, _ := GetAllLabels(fileLabels, Var1) - assert.Equal(t, len(result), 3) -} diff --git a/cmd/podman/shared/parse/parse.go b/cmd/podman/shared/parse/parse.go index 3a75ff7a88..79449029df 100644 --- a/cmd/podman/shared/parse/parse.go +++ b/cmd/podman/shared/parse/parse.go @@ -79,6 +79,34 @@ func ValidateDomain(val string) (string, error) { return "", fmt.Errorf("%s is not a valid domain", val) } +// GetAllLabels retrieves all labels given a potential label file and a number +// of labels provided from the command line. +func GetAllLabels(labelFile, inputLabels []string) (map[string]string, error) { + labels := make(map[string]string) + for _, file := range labelFile { + // Use of parseEnvFile still seems safe, as it's missing the + // extra parsing logic of parseEnv. + // There's an argument that we SHOULD be doing that parsing for + // all environment variables, even those sourced from files, but + // that would require a substantial rework. + if err := parseEnvFile(labels, file); err != nil { + return nil, err + } + } + for _, label := range inputLabels { + split := strings.SplitN(label, "=", 2) + if split[0] == "" { + return nil, errors.Errorf("invalid label format: %q", label) + } + value := "" + if len(split) > 1 { + value = split[1] + } + labels[split[0]] = value + } + return labels, nil +} + // reads a file of line terminated key=value pairs, and overrides any keys // present in the file with additional pairs specified in the override parameter // for env-file and labels-file flags diff --git a/cmd/podman/shared/parse/parse_test.go b/cmd/podman/shared/parse/parse_test.go index 1359076a02..a6ddc2be9c 100644 --- a/cmd/podman/shared/parse/parse_test.go +++ b/cmd/podman/shared/parse/parse_test.go @@ -4,9 +4,33 @@ package parse import ( + "io/ioutil" + "os" "testing" + + "github.com/stretchr/testify/assert" +) + +var ( + Var1 = []string{"ONE=1", "TWO=2"} ) +func createTmpFile(content []byte) (string, error) { + tmpfile, err := ioutil.TempFile(os.TempDir(), "unittest") + if err != nil { + return "", err + } + + if _, err := tmpfile.Write(content); err != nil { + return "", err + + } + if err := tmpfile.Close(); err != nil { + return "", err + } + return tmpfile.Name(), nil +} + func TestValidateExtraHost(t *testing.T) { type args struct { val string @@ -97,3 +121,32 @@ func TestValidateFileName(t *testing.T) { }) } } + +func TestGetAllLabels(t *testing.T) { + fileLabels := []string{} + labels, _ := GetAllLabels(fileLabels, Var1) + assert.Equal(t, len(labels), 2) +} + +func TestGetAllLabelsBadKeyValue(t *testing.T) { + inLabels := []string{"=badValue", "="} + fileLabels := []string{} + _, err := GetAllLabels(fileLabels, inLabels) + assert.Error(t, err, assert.AnError) +} + +func TestGetAllLabelsBadLabelFile(t *testing.T) { + fileLabels := []string{"/foobar5001/be"} + _, err := GetAllLabels(fileLabels, Var1) + assert.Error(t, err, assert.AnError) +} + +func TestGetAllLabelsFile(t *testing.T) { + content := []byte("THREE=3") + tFile, err := createTmpFile(content) + defer os.Remove(tFile) + assert.NoError(t, err) + fileLabels := []string{tFile} + result, _ := GetAllLabels(fileLabels, Var1) + assert.Equal(t, len(result), 3) +} diff --git a/cmd/podman/volume_create.go b/cmd/podman/volume_create.go index e5a5767495..52189657bc 100644 --- a/cmd/podman/volume_create.go +++ b/cmd/podman/volume_create.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/containers/libpod/cmd/podman/cliconfig" - "github.com/containers/libpod/cmd/podman/shared" + "github.com/containers/libpod/cmd/podman/shared/parse" "github.com/containers/libpod/pkg/adapter" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -51,12 +51,12 @@ func volumeCreateCmd(c *cliconfig.VolumeCreateValues) error { return errors.Errorf("too many arguments, create takes at most 1 argument") } - labels, err := shared.GetAllLabels([]string{}, c.Label) + labels, err := parse.GetAllLabels([]string{}, c.Label) if err != nil { return errors.Wrapf(err, "unable to process labels") } - opts, err := shared.GetAllLabels([]string{}, c.Opt) + opts, err := parse.GetAllLabels([]string{}, c.Opt) if err != nil { return errors.Wrapf(err, "unable to process options") } diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go index 07d1cdba7a..7a0bfe1171 100644 --- a/pkg/api/handlers/libpod/pods.go +++ b/pkg/api/handlers/libpod/pods.go @@ -42,7 +42,8 @@ func PodCreate(w http.ResponseWriter, r *http.Request) { } if len(input.Labels) > 0 { - if err := parse.ReadKVStrings(labels, []string{}, input.Labels); err != nil { + labels, err = parse.GetAllLabels([]string{}, input.Labels) + if err != nil { utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err) return } diff --git a/test/e2e/create_test.go b/test/e2e/create_test.go index 134b7b1622..10742a0e84 100644 --- a/test/e2e/create_test.go +++ b/test/e2e/create_test.go @@ -304,4 +304,42 @@ var _ = Describe("Podman create", func() { session.WaitWithDefaultTimeout() Expect(session).To(Not(Equal(0))) }) + + It("podman create with unset label", func() { + // Alpine is assumed to have no labels here, which seems safe + ctrName := "testctr" + session := podmanTest.Podman([]string{"create", "--label", "TESTKEY1=", "--label", "TESTKEY2", "--name", ctrName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + data := inspect.InspectContainerToJSON() + Expect(len(data)).To(Equal(1)) + Expect(len(data[0].Config.Labels)).To(Equal(2)) + _, ok1 := data[0].Config.Labels["TESTKEY1"] + Expect(ok1).To(BeTrue()) + _, ok2 := data[0].Config.Labels["TESTKEY2"] + Expect(ok2).To(BeTrue()) + }) + + It("podman create with set label", func() { + // Alpine is assumed to have no labels here, which seems safe + ctrName := "testctr" + session := podmanTest.Podman([]string{"create", "--label", "TESTKEY1=value1", "--label", "TESTKEY2=bar", "--name", ctrName, ALPINE, "top"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + inspect := podmanTest.Podman([]string{"inspect", ctrName}) + inspect.WaitWithDefaultTimeout() + data := inspect.InspectContainerToJSON() + Expect(len(data)).To(Equal(1)) + Expect(len(data[0].Config.Labels)).To(Equal(2)) + val1, ok1 := data[0].Config.Labels["TESTKEY1"] + Expect(ok1).To(BeTrue()) + Expect(val1).To(Equal("value1")) + val2, ok2 := data[0].Config.Labels["TESTKEY2"] + Expect(ok2).To(BeTrue()) + Expect(val2).To(Equal("bar")) + }) })