Simplify bundle docker value as image string (#168)

* Simplify bundle docker value as image string

* Extra line

README.md

@@ -83,9 +83,7 @@ long_description: |-
 permissions:
   - can_echo
 
-docker:
-  image: ubuntu
-  tag: 20.04
+image: ubuntu:20.04
 
 commands:
   foo:

bundles/bundle_test.go

@@ -35,8 +35,7 @@ func TestLoadBundleFromFile(t *testing.T) {
 	assert.Equal(t, "A test bundle.", b.Description)
 	assert.Equal(t, "This is test bundle.\nThere are many like it, but this one is mine.", b.LongDescription)
 	assert.Len(t, b.Permissions, 1)
-	assert.Equal(t, "ubuntu", b.Docker.Image)
-	assert.Equal(t, "20.04", b.Docker.Tag)
+	assert.Equal(t, "ubuntu:20.04", b.Image)
 	assert.Len(t, b.Commands, 4)
 
 	// Bundle templates

bundles/default.yml

@@ -3,6 +3,7 @@ gort_bundle_version: 1
 
 name: gort
 version: 0.0.1
+
 author: Matt Titmus <[email protected]>
 homepage: https://guide.getgort.io
 description: The default command bundle.
@@ -17,9 +18,7 @@ permissions:
   - manage_roles
   - manage_users
 
-docker:
-  image: getgort/gort
-  tag: {{.Version}}
+image: getgort/gort:{{.Version}}
 
 commands:
   bundle:

data/bundle.go

@@ -34,41 +34,61 @@ type BundleInfo struct {
 // Bundle represents a bundle as defined in the "bundles" section of the
 // config.
 type Bundle struct {
-	GortBundleVersion int                       `yaml:"gort_bundle_version,omitempty" json:"gort_bundle_version,omitempty"`
-	Name              string                    `yaml:",omitempty" json:"name,omitempty"`
-	Version           string                    `yaml:",omitempty" json:"version,omitempty"`
-	Enabled           bool                      `yaml:",omitempty" json:"enabled"`
-	Author            string                    `yaml:",omitempty" json:"author,omitempty"`
-	Homepage          string                    `yaml:",omitempty" json:"homepage,omitempty"`
-	Description       string                    `yaml:",omitempty" json:"description,omitempty"`
-	InstalledOn       time.Time                 `yaml:"-" json:"installed_on,omitempty"`
-	InstalledBy       string                    `yaml:",omitempty" json:"installed_by,omitempty"`
-	LongDescription   string                    `yaml:"long_description,omitempty" json:"long_description,omitempty"`
-	Docker            BundleDocker              `yaml:",omitempty" json:"docker,omitempty"`
-	Kubernetes        BundleKubernetes          `yaml:",omitempty" json:"kubernetes,omitempty"`
-	Permissions       []string                  `yaml:",omitempty" json:"permissions,omitempty"`
-	Commands          map[string]*BundleCommand `yaml:",omitempty" json:"commands,omitempty"`
-	Default           bool                      `yaml:"-" json:"default,omitempty"`
-	Templates         Templates                 `yaml:",omitempty" json:"templates,omitempty"`
+	GortBundleVersion int                       `yaml:"gort_bundle_version,omitempty" json:",omitempty"`
+	Name              string                    `yaml:",omitempty" json:",omitempty"`
+	Version           string                    `yaml:",omitempty" json:",omitempty"`
+	Enabled           bool                      `yaml:",omitempty" json:",omitempty"`
+	Author            string                    `yaml:",omitempty" json:",omitempty"`
+	Homepage          string                    `yaml:",omitempty" json:",omitempty"`
+	Description       string                    `yaml:",omitempty" json:",omitempty"`
+	Image             string                    `yaml:",omitempty" json:",omitempty"`
+	InstalledOn       time.Time                 `yaml:"-" json:",omitempty"`
+	InstalledBy       string                    `yaml:",omitempty" json:",omitempty"`
+	LongDescription   string                    `yaml:"long_description,omitempty" json:",omitempty"`
+	Kubernetes        BundleKubernetes          `yaml:",omitempty" json:",omitempty"`
+	Permissions       []string                  `yaml:",omitempty" json:",omitempty"`
+	Commands          map[string]*BundleCommand `yaml:",omitempty" json:",omitempty"`
+	Default           bool                      `yaml:"-" json:",omitempty"`
+	Templates         Templates                 `yaml:",omitempty" json:",omitempty"`
 }
 
-// BundleKubernetes represents the "bundles/kubernetes" subsection of the config doc
-type BundleKubernetes struct {
-	ServiceAccountName string `yaml:"serviceAccountName,omitempty" json:"serviceAccountName,omitempty"`
-}
+// ImageFull returns the full image name, consisting of a repository and tag.
+func (b Bundle) ImageFull() string {
+	if repo, tag := b.ImageFullParts(); repo != "" {
+		return repo + ":" + tag
+	}
 
-func (b Bundle) Semver() semver.Version {
-	var version = b.Version
+	return ""
+}
 
-	if version == "" {
-		return semver.Version{}
+// ImageFullParts returns the image repository and tag. If the tag isn't
+// specified in b.Image, the returned tag will be "latest".
+func (b Bundle) ImageFullParts() (repository, tag string) {
+	if b.Image == "" {
+		return
 	}
 
-	if strings.ToLower(version)[0] == 'v' {
-		version = version[1:]
+	ss := strings.SplitN(b.Image, ":", 2)
+
+	repository = ss[0]
+
+	if len(ss) > 1 {
+		tag = ss[1]
+	} else {
+		tag = "latest"
 	}
 
-	if v, err := semver.NewVersion(version); err != nil {
+	return
+}
+
+// Semver returns b.Version as a semver.Version value, which makes it easier
+// to compare and sort version numbers. If b.Version == "", a zero-value
+// Version{} is returned. If b.Version isn't valid per Semantic Versioning
+// 2.0.0 (https://semver.org), it will attempt to coerce it into a correct
+// semantic version (since users be crazy). If it fails, a zero-value
+// Version{} is returned.
+func (b Bundle) Semver() semver.Version {
+	if v, err := semver.NewVersion(CoerceVersionToSemver(b.Version)); err != nil {
 		return semver.Version{}
 	} else {
 		return *v
@@ -86,8 +106,63 @@ type BundleCommand struct {
 	Templates       Templates `yaml:",omitempty" json:"templates,omitempty"`
 }
 
-// BundleDocker represents the "bundles/docker" subsection of the config doc
-type BundleDocker struct {
-	Image string `yaml:",omitempty" json:"image,omitempty"`
-	Tag   string `yaml:",omitempty" json:"tag,omitempty"`
+// BundleKubernetes represents the "bundles/kubernetes" subsection of the config doc
+type BundleKubernetes struct {
+	ServiceAccountName string `yaml:"serviceAccountName,omitempty" json:"serviceAccountName,omitempty"`
+}
+
+// CoerceVersionToSemver takes a version number and attempts to coerce it
+// into a semver-compliant dotted-tri format. It also understands semver
+// pre-release and metadata decorations.
+func CoerceVersionToSemver(version string) string {
+	version = strings.TrimSpace(version)
+
+	if version == "" {
+		return "0.0.0"
+	}
+
+	if strings.ToLower(version)[0] == 'v' {
+		version = version[1:]
+	}
+
+	v := version
+
+	var metadata, preRelease string
+	var ss []string
+	var dotParts = make([]string, 3)
+
+	ss = strings.SplitN(v, "+", 2)
+	if len(ss) > 1 {
+		v = ss[0]
+		metadata = ss[1]
+	}
+
+	ss = strings.SplitN(v, "-", 2)
+	if len(ss) > 1 {
+		v = ss[0]
+		preRelease = ss[1]
+	}
+
+	// If it turns out to be in dotted-tri format, return the original
+	ss = strings.SplitN(v, ".", 4)
+	for i := 0; i < len(ss) && i < 3; i++ {
+		dotParts[i] = ss[i]
+	}
+	for i := 0; i < len(dotParts); i++ {
+		if dotParts[i] == "" {
+			dotParts[i] = "0"
+		}
+	}
+
+	v = strings.Join(dotParts, ".")
+
+	if preRelease != "" {
+		v += "-" + preRelease
+	}
+
+	if metadata != "" {
+		v += "+" + metadata
+	}
+
+	return v
 }

data/bundle_test.go

@@ -0,0 +1,139 @@
+/*
+ * Copyright 2021 The Gort Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package data
+
+import (
+	"testing"
+
+	"github.com/coreos/go-semver/semver"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBundleImageFullParts(t *testing.T) {
+	tests := []struct {
+		Image        string
+		ExpectedRepo string
+		ExpectedTag  string
+	}{
+		{"", "", ""},
+		{"ubuntu", "ubuntu", "latest"},
+		{"ubuntu:20.04", "ubuntu", "20.04"},
+		{"linux:ubuntu:20.04", "linux", "ubuntu:20.04"},
+	}
+
+	for _, test := range tests {
+		b := Bundle{Image: test.Image}
+		repo, tag := b.ImageFullParts()
+
+		assert.Equal(t, test.ExpectedRepo, repo)
+		assert.Equal(t, test.ExpectedTag, tag)
+	}
+}
+
+func TestCoerceVersionToSemver(t *testing.T) {
+	tests := []struct {
+		Version  string
+		Expected string
+	}{
+		// Zero value
+		{"", "0.0.0"},
+
+		// Malicious? edge cases
+		{"v", "0.0.0"},
+		{"  v1  ", "1.0.0"},
+
+		// Examples from semver.org
+		{"1.0.0-alpha", "1.0.0-alpha"},
+		{"v1.0.0-alpha", "1.0.0-alpha"},
+		{"1.0.0-alpha.1", "1.0.0-alpha.1"},
+		{"v1.0.0-alpha.1", "1.0.0-alpha.1"},
+		{"1.0.0-0.3.7", "1.0.0-0.3.7"},
+		{"v1.0.0-0.3.7", "1.0.0-0.3.7"},
+		{"1.0.0-x.7.z.92", "1.0.0-x.7.z.92"},
+		{"v1.0.0-x.7.z.92", "1.0.0-x.7.z.92"},
+		{"1.0.0-alpha+001", "1.0.0-alpha+001"},
+		{"v1.0.0-alpha+001", "1.0.0-alpha+001"},
+		{"1.0.0+20130313144700", "1.0.0+20130313144700"},
+		{"v1.0.0+20130313144700", "1.0.0+20130313144700"},
+		{"1.0.0-beta+exp.sha.5114f85", "1.0.0-beta+exp.sha.5114f85"},
+		{"v1.0.0-beta+exp.sha.5114f85", "1.0.0-beta+exp.sha.5114f85"},
+
+		// Version coercion
+		{"1", "1.0.0"},
+		{"v1", "1.0.0"},
+		{"1.2", "1.2.0"},
+		{"v1.2", "1.2.0"},
+		{"1.2.3", "1.2.3"},
+		{"v1.2.3", "1.2.3"},
+		{"1.2.3.4", "1.2.3"}, // Truncate to 3 parts.
+		{"v1.2.3.4", "1.2.3"},
+	}
+
+	for _, test := range tests {
+		result := CoerceVersionToSemver(test.Version)
+		assert.Equal(t, test.Expected, result, "Test case: %q", test.Version)
+
+		_, err := semver.NewVersion(result)
+		assert.NoError(t, err, "Test case: %q", test.Version)
+	}
+}
+
+func TestSemver(t *testing.T) {
+	tests := []struct {
+		Version  string
+		Expected semver.Version
+	}{
+		// Zero value
+		{"", semver.Version{}},
+
+		// Malicious? edge cases
+		{"v", semver.Version{}},
+		{"  v1  ", *semver.New("1.0.0")},
+
+		// Examples from *semver.org
+		{"1.0.0-alpha", *semver.New("1.0.0-alpha")},
+		{"v1.0.0-alpha", *semver.New("1.0.0-alpha")},
+		{"1.0.0-alpha.1", *semver.New("1.0.0-alpha.1")},
+		{"v1.0.0-alpha.1", *semver.New("1.0.0-alpha.1")},
+		{"1.0.0-0.3.7", *semver.New("1.0.0-0.3.7")},
+		{"v1.0.0-0.3.7", *semver.New("1.0.0-0.3.7")},
+		{"1.0.0-x.7.z.92", *semver.New("1.0.0-x.7.z.92")},
+		{"v1.0.0-x.7.z.92", *semver.New("1.0.0-x.7.z.92")},
+		{"1.0.0-alpha+001", *semver.New("1.0.0-alpha+001")},
+		{"v1.0.0-alpha+001", *semver.New("1.0.0-alpha+001")},
+		{"1.0.0+20130313144700", *semver.New("1.0.0+20130313144700")},
+		{"v1.0.0+20130313144700", *semver.New("1.0.0+20130313144700")},
+		{"1.0.0-beta+exp.sha.5114f85", *semver.New("1.0.0-beta+exp.sha.5114f85")},
+		{"v1.0.0-beta+exp.sha.5114f85", *semver.New("1.0.0-beta+exp.sha.5114f85")},
+
+		// Version coercion
+		{"1", *semver.New("1.0.0")},
+		{"v1", *semver.New("1.0.0")},
+		{"1.2", *semver.New("1.2.0")},
+		{"v1.2", *semver.New("1.2.0")},
+		{"1.2.3", *semver.New("1.2.3")},
+		{"v1.2.3", *semver.New("1.2.3")},
+		{"1.2.3.4", *semver.New("1.2.3")},
+		{"v1.2.3.4", *semver.New("1.2.3")},
+	}
+
+	for _, test := range tests {
+		b := Bundle{Version: test.Version}
+		result := b.Semver()
+		assert.Equal(t, test.Expected, result, "Test case: %q", test.Version)
+	}
+}

dataaccess/memory/bundle-access.go

@@ -42,6 +42,8 @@ func (da *InMemoryDataAccess) BundleCreate(ctx context.Context, bundle data.Bund
 		return errs.ErrBundleExists
 	}
 
+	bundle.Image = bundle.ImageFull()
+
 	da.bundles[bundleKey(bundle.Name, bundle.Version)] = &bundle
 
 	return nil

dataaccess/postgres/bundle-access.go

@@ -647,21 +647,31 @@ func (da PostgresDataAccess) doFindCommandEntry(ctx context.Context, tx *sql.Tx,
 
 func (da PostgresDataAccess) doBundleGet(ctx context.Context, tx *sql.Tx, name string, version string) (data.Bundle, error) {
 	query := `SELECT gort_bundle_version, name, version, author, homepage,
-			description, long_description, docker_image, docker_tag,
+			description, long_description, image_repository, image_tag,
 			install_timestamp, install_user
 		FROM bundles
 		WHERE name=$1 AND version=$2`
 
+	var repository, tag string
+
 	bundle := data.Bundle{}
 	row := tx.QueryRowContext(ctx, query, name, version)
 	err := row.Scan(&bundle.GortBundleVersion, &bundle.Name, &bundle.Version,
 		&bundle.Author, &bundle.Homepage, &bundle.Description,
-		&bundle.LongDescription, &bundle.Docker.Image, &bundle.Docker.Tag,
+		&bundle.LongDescription, &repository, &tag,
 		&bundle.InstalledOn, &bundle.InstalledBy)
 	if err != nil {
 		return bundle, gerr.Wrap(errs.ErrNoSuchBundle, err)
 	}
 
+	if repository != "" {
+		if tag == "" {
+			tag = "latest"
+		}
+
+		bundle.Image = repository + ":" + tag
+	}
+
 	enabledVersion, err := da.doBundleEnabledVersion(ctx, tx, name)
 	if err != nil {
 		return bundle, gerr.Wrap(fmt.Errorf("failed to get bundle enabled version"), err)
@@ -893,13 +903,15 @@ func (da PostgresDataAccess) doBundleGetTemplates(ctx context.Context, tx *sql.T
 
 func (da PostgresDataAccess) doBundleInsert(ctx context.Context, tx *sql.Tx, bundle data.Bundle) error {
 	query := `INSERT INTO bundles (gort_bundle_version, name, version, author,
-		homepage, description, long_description, docker_image,
-		docker_tag, install_user)
+		homepage, description, long_description, image_repository, image_tag,
+		install_user)
 		VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10);`
 
+	repository, tag := bundle.ImageFullParts()
+
 	_, err := tx.ExecContext(ctx, query, bundle.GortBundleVersion, bundle.Name, bundle.Version,
 		bundle.Author, bundle.Homepage, bundle.Description, bundle.LongDescription,
-		bundle.Docker.Image, bundle.Docker.Tag, bundle.InstalledBy)
+		repository, tag, bundle.InstalledBy)
 
 	if err != nil {
 		if strings.Contains(err.Error(), "violates") {