Fix bug where unwanted v is prepended to tag name (#2915)

check for latest and empty tag

Signed-off-by: Ludvig Liljenberg <lliljenberg@microsoft.com>
Co-authored-by: Ludvig Liljenberg <lliljenberg@microsoft.com>

pkg/porter/lifecycle.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"strings"
 
 	"get.porter.sh/porter/pkg/cache"
@@ -198,7 +199,7 @@ func (p *Porter) resolveBundleReference(ctx context.Context, opts *BundleReferen
 		pullOpts := *opts // make a copy just to do the pull
 		pullOpts.Reference = ref.String()
 
-		err := ensureVPrefix(&pullOpts)
+		err := ensureVPrefix(&pullOpts, p.Out)
 		if err != nil {
 			return err
 		}
@@ -309,7 +310,7 @@ func (p *Porter) BuildActionArgs(ctx context.Context, installation storage.Insta
 // Version tag should always be prefixed with a "v", see https://github.com/getporter/porter/issues/2886.
 // This is safe because "porter publish" adds a "v", see
 // https://github.com/getporter/porter/blob/17bd7816ef6bde856793f6122e32274aa9d01d1b/pkg/storage/installation.go#L350
-func ensureVPrefix(opts *BundleReferenceOptions) error {
+func ensureVPrefix(opts *BundleReferenceOptions, out io.Writer) error {
 	var ociRef *cnab.OCIReference
 	if opts._ref != nil {
 		ociRef = opts._ref
@@ -321,8 +322,8 @@ func ensureVPrefix(opts *BundleReferenceOptions) error {
 		ociRef = &ref
 	}
 
-	if strings.HasPrefix(ociRef.Tag(), "v") {
-		// don't do anything if "v" is already there
+	if ociRef.Tag() == "" || ociRef.Tag() == "latest" || strings.HasPrefix(ociRef.Tag(), "v") {
+		// don't do anything if missing tag, if tag is "latest", or if "v" is already there
 		return nil
 	}
 
@@ -332,6 +333,7 @@ func ensureVPrefix(opts *BundleReferenceOptions) error {
 	}
 
 	// always update the .Reference string, but don't add the _ref field unless it was already there (non-nil)
+	fmt.Fprintf(out, "WARNING: using reference %q instead of %q because missing v-prefix on tag\n", vRef.String(), ociRef.String())
 	opts.Reference = vRef.String()
 	if opts._ref != nil {
 		opts._ref = &vRef

pkg/porter/lifecycle_test.go

@@ -3,6 +3,7 @@ package porter
 import (
 	"context"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 
@@ -585,7 +586,7 @@ func Test_ensureVPrefix(t *testing.T) {
 				assert.False(t, tt.args.opts._ref.Tag()[0] == 'v')
 			}
 
-			err := ensureVPrefix(tt.args.opts)
+			err := ensureVPrefix(tt.args.opts, io.Discard)
 
 			// after
 			tt.wantErr(t, err, fmt.Sprintf("ensureVPrefix(%v)", tt.args.opts))
@@ -653,7 +654,7 @@ func Test_ensureVPrefix_idempotent(t *testing.T) {
 				assert.True(t, tt.args.opts._ref.Tag()[1] != 'v')
 			}
 
-			err := ensureVPrefix(tt.args.opts)
+			err := ensureVPrefix(tt.args.opts, io.Discard)
 
 			// after
 			tt.wantErr(t, err, fmt.Sprintf("ensureVPrefix(%v)", tt.args.opts))
@@ -668,3 +669,32 @@ func Test_ensureVPrefix_idempotent(t *testing.T) {
 		})
 	}
 }
+
+// ensure no v is added if specifying :latest tag or no tag at all
+func Test_ensureVPrefix_latest(t *testing.T) {
+	testcases := map[string]struct {
+		latestRef string
+	}{
+		"latest":     {latestRef: "example/porter-hello:latest"},
+		"not-latest": {latestRef: "example/porter-hello"},
+	}
+	for name, test := range testcases {
+		t.Run(name, func(t *testing.T) {
+			opts := BundleReferenceOptions{
+				installationOptions: installationOptions{},
+				BundlePullOptions: BundlePullOptions{
+					Reference:        test.latestRef,
+					_ref:             nil,
+					InsecureRegistry: false,
+					Force:            false,
+				},
+				bundleRef: nil,
+			}
+			err := ensureVPrefix(&opts, io.Discard)
+			assert.NoError(t, err)
+
+			// shouldn't change
+			assert.Equal(t, test.latestRef, opts.BundlePullOptions.Reference)
+		})
+	}
+}