Merge pull request #1524 from carolynvs/v-prefix

Improve semver defaulting when building a bundle
getporter · Mar 30, 2021 · 9f608ff · 9f608ff
2 parents 8db0121 + b5a1eb7
commit 9f608ff
Show file tree

Hide file tree

Showing 12 changed files with 37 additions and 29 deletions.
diff --git a/docs/content/author-bundles.md b/docs/content/author-bundles.md
@@ -38,14 +38,16 @@ dockerfile: dockerfile.tmpl
 
 * `name`: The name of the bundle
 * `description`: A description of the bundle
-* `version`: The version of the bundle, uses [semver](https://semver.org). Should not have a 'v' prefix.
+* `version`: The version of the bundle, uses [semver](https://semver.org). A leading v prefix may optionally be used.
 * `registry`: The registry to use for publishing the bundle. The format is `REGISTRY_HOST/ORG`.
     Both the final bundle reference and invocation image name will be based on this value.
     For example, if the bundle name is `porter-hello`, registry is `getporter` and the version is `0.1.0`,
     the bundle reference will be `getporter/porter-hello:v0.1.0` and the invocation image name will be `getporter/porter-hello-installer:v0.1.0`
 * `reference`: OPTIONAL. The bundle reference, taking precedence over any values set for the `registry`, `name` fields. The format is `REGISTRY_HOST/ORG/NAME`.  The recommended pattern is to let the Docker tag be auto-derived from the `version` field.  However, a full reference with a Docker tag included may also be specified.
    The invocation image name will also be based on this value when set. For example, if the `reference` is
-   `getporter/porter-hello`, then the final invocation image name will be `getporter/porter-hello-installer:v0.1.0`
+   `getporter/porter-hello`, then the final invocation image name will be `getporter/porter-hello-installer:v0.1.0`. 
+
+   When the version is used to default the tag, and it contains a plus sign (+), the plus sign is replaced with an underscore because while + is a valid semver delimiter for the build metadata, it is not an allowed character in a tag.
 * `dockerfile`: OPTIONAL. The relative path to a Dockerfile to use as a template during `porter build`. 
     See [Custom Dockerfile](/custom-dockerfile/) for details on how to use a custom Dockerfile.
 * `custom`: OPTIONAL. A map of [custom bundle metadata](https://github.com/cnabio/cnab-spec/blob/master/101-bundle-json.md#custom-extensions).

diff --git a/go.mod b/go.mod
@@ -25,7 +25,7 @@ replace (
 )
 
 require (
-	github.com/Masterminds/semver v1.5.0
+	github.com/Masterminds/semver/v3 v3.1.1
 	github.com/PaesslerAG/jsonpath v0.1.1
 	github.com/PuerkitoBio/goquery v1.5.0 // indirect
 	github.com/carolynvs/aferox v0.2.1

diff --git a/go.sum b/go.sum
@@ -33,6 +33,8 @@ github.com/GeertJohan/go.incremental v1.0.0/go.mod h1:6fAjUhbVuX1KcMD3c8TEgVUqmo
 github.com/GeertJohan/go.rice v1.0.0/go.mod h1:eH6gbSOAUv07dQuZVnBmoDP8mgsM1rtixis4Tib9if0=
 github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww=
 github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y=
+github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc=
+github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
 github.com/Microsoft/go-winio v0.4.14 h1:+hMXMk01us9KgxGb7ftKQt2Xpf5hH/yky+TDA+qxleU=
 github.com/Microsoft/go-winio v0.4.14/go.mod h1:qXqCSQ3Xa7+6tgxaGTIe4Kpcdsi+P8jBhyzoq1bpyYA=
 github.com/Microsoft/hcsshim v0.8.6 h1:ZfF0+zZeYdzMIVMZHKtDKJvLHj76XCuVae/jNkjj0IA=

diff --git a/pkg/cnab/config-adapter/stamp_test.go b/pkg/cnab/config-adapter/stamp_test.go
@@ -12,7 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var simpleManifestDigest = "62686a974a7bce589c981cb16549feb58ef308fbe98b9763e9151eaf30b27562"
+var simpleManifestDigest = "cbdf7c0f95e525d4ecbf2b56c852ec3acdc984a6c6d3392b05e31c28e70280bd"
 
 func TestConfig_GenerateStamp(t *testing.T) {
 	// Do not run this test in parallel

diff --git a/pkg/cnab/extensions/solver.go b/pkg/cnab/extensions/solver.go
@@ -3,7 +3,7 @@ package extensions
 import (
 	"sort"
 
-	"github.com/Masterminds/semver"
+	"github.com/Masterminds/semver/v3"
 	"github.com/cnabio/cnab-go/bundle"
 	"github.com/docker/distribution/reference"
 	"github.com/google/go-containerregistry/pkg/crane"

diff --git a/pkg/manifest/manifest.go b/pkg/manifest/manifest.go
@@ -12,7 +12,7 @@ import (
 
 	"get.porter.sh/porter/pkg/context"
 	"get.porter.sh/porter/pkg/yaml"
-	"github.com/Masterminds/semver"
+	"github.com/Masterminds/semver/v3"
 	"github.com/cbroglie/mustache"
 	"github.com/cnabio/cnab-go/bundle/definition"
 	"github.com/cnabio/cnab-go/claim"
@@ -182,6 +182,15 @@ func (m *Manifest) validateMetadata(cxt *context.Context) error {
 		}
 	}
 
+	// Allow for the user to have specified the version with a leading v prefix but save it as
+	// proper semver
+	if m.Version != "" {
+		v, err := semver.NewVersion(m.Version)
+		if err != nil {
+			return errors.Wrapf(err, "version %q is not a valid semver value", m.Version)
+		}
+		m.Version = v.String()
+	}
 	return nil
 }
 
@@ -912,14 +921,11 @@ func (m *Manifest) getDockerTagFromBundleRef(bundleRef reference.Named) (string,
 	case reference.Tagged:
 		dockerTag = v.Tag()
 	case reference.Named:
-		ver, err := semver.NewVersion(m.Version)
-		if err != nil {
-			return "", errors.Wrapf(err, "could not parse the bundle version %q as a semantic version", m.Version)
-		}
 		// Docker tag is missing from the provided bundle tag, so default it
 		// to use the manifest version prefixed with v
 		// Example: bundle version is 1.0.0, so the bundle tag is v1.0.0
-		dockerTag = fmt.Sprintf("v%s", ver.String())
+		cleanTag := strings.ReplaceAll(m.Version, "+", "_") // Semver may include a + which is not allowed in a docker tag, e.g. v1.0.0-alpha.1+buildmetadata, change that to v1.0.0-alpha.1_buildmetadata
+		dockerTag = fmt.Sprintf("v%s", cleanTag)
 	case reference.Digested:
 		return "", errors.New("invalid bundle tag format, must be an OCI image tag")
 	}

diff --git a/pkg/manifest/manifest_test.go b/pkg/manifest/manifest_test.go
@@ -243,16 +243,16 @@ func TestSetDefaults(t *testing.T) {
 		cxt := context.NewTestContext(t)
 		m := Manifest{
 			Name:      "mybun",
-			Version:   "1.2.3-beta.1",
+			Version:   "1.2.3-beta.1+15",
 			Reference: "getporter/mybun",
 		}
 		err := m.validateMetadata(cxt.Context)
 		require.NoError(t, err)
 
 		err = m.SetDefaults()
 		require.NoError(t, err)
-		assert.Equal(t, "getporter/mybun:v1.2.3-beta.1", m.Reference)
-		assert.Equal(t, "getporter/mybun-installer:v1.2.3-beta.1", m.Image)
+		assert.Equal(t, "getporter/mybun:v1.2.3-beta.1_15", m.Reference)
+		assert.Equal(t, "getporter/mybun-installer:v1.2.3-beta.1_15", m.Image)
 	})
 
 	t.Run("bundle reference includes registry with port", func(t *testing.T) {

diff --git a/pkg/manifest/testdata/simple.porter.yaml b/pkg/manifest/testdata/simple.porter.yaml
@@ -3,7 +3,7 @@ mixins:
 
 name: hello
 description: "An example Porter configuration"
-version: 0.1.0
+version: v0.1.0
 registry: getporter
 
 install:

diff --git a/pkg/pkgmgmt/feed/feed.go b/pkg/pkgmgmt/feed/feed.go
@@ -7,7 +7,7 @@ import (
 	"time"
 
 	"get.porter.sh/porter/pkg/context"
-	"github.com/Masterminds/semver"
+	"github.com/Masterminds/semver/v3"
 )
 
 type MixinFeed struct {

diff --git a/pkg/pkgmgmt/feed/generate.go b/pkg/pkgmgmt/feed/generate.go
@@ -8,7 +8,7 @@ import (
 	"time"
 
 	"get.porter.sh/porter/pkg/context"
-	"github.com/Masterminds/semver"
+	"github.com/Masterminds/semver/v3"
 	"github.com/cbroglie/mustache"
 	"github.com/pkg/errors"
 )

diff --git a/pkg/porter/build.go b/pkg/porter/build.go
@@ -3,14 +3,14 @@ package porter
 import (
 	"fmt"
 	"os"
-	"regexp"
 
 	"get.porter.sh/porter/pkg/build"
 	configadapter "get.porter.sh/porter/pkg/cnab/config-adapter"
 	"get.porter.sh/porter/pkg/context"
 	"get.porter.sh/porter/pkg/manifest"
 	"get.porter.sh/porter/pkg/mixin"
 	"get.porter.sh/porter/pkg/printer"
+	"github.com/Masterminds/semver/v3"
 	"github.com/cnabio/cnab-go/bundle"
 	"github.com/pkg/errors"
 )
@@ -30,19 +30,13 @@ type BuildOptions struct {
 	NoLint bool
 }
 
-// semVerRegex is a regex for ensuring bundle versions adhere to
-// semantic versioning per https://semver.org/#is-v123-a-semantic-version
-// Regex adapted from github.com/Masterminds/semver
-const semVerRegex string = `([0-9]+)(\.[0-9]+)?(\.[0-9]+)?` +
-	`(-([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?` +
-	`(\+([0-9A-Za-z\-]+(\.[0-9A-Za-z\-]+)*))?`
-
 func (o *BuildOptions) Validate(cxt *context.Context) error {
 	if o.Version != "" {
-		versionRegex := regexp.MustCompile("^" + semVerRegex + "$")
-		if m := versionRegex.FindStringSubmatch(o.Version); m == nil {
+		v, err := semver.NewVersion(o.Version)
+		if err != nil {
 			return fmt.Errorf("invalid bundle version: %q is not a valid semantic version", o.Version)
 		}
+		o.Version = v.String()
 	}
 
 	return o.bundleFileOptions.Validate(cxt)

diff --git a/pkg/porter/build_test.go b/pkg/porter/build_test.go
@@ -140,9 +140,13 @@ func TestValidateBuildOpts(t *testing.T) {
 		opts:      BuildOptions{metadataOpts: metadataOpts{Version: "latest"}},
 		wantError: `invalid bundle version: "latest" is not a valid semantic version`,
 	}, {
-		name:      "invalid version set - v prefix",
+		name:      "valid version - v prefix",
 		opts:      BuildOptions{metadataOpts: metadataOpts{Version: "v1.0.0"}},
-		wantError: `invalid bundle version: "v1.0.0" is not a valid semantic version`,
+		wantError: "",
+	}, {
+		name:      "valid version - with hash",
+		opts:      BuildOptions{metadataOpts: metadataOpts{Version: "v0.1.7+58d98af56c3a4c40c69535654216bd4a1fa701e7"}},
+		wantError: "",
 	}, {
 		name:      "valid name and value set",
 		opts:      BuildOptions{metadataOpts: metadataOpts{Name: "newname", Version: "1.0.0"}},