diff --git a/pkg/porter/lifecycle.go b/pkg/porter/lifecycle.go index 7b5cc0cce..33cae07e3 100644 --- a/pkg/porter/lifecycle.go +++ b/pkg/porter/lifecycle.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "get.porter.sh/porter/pkg/cache" "get.porter.sh/porter/pkg/cnab" @@ -196,6 +197,12 @@ func (p *Porter) resolveBundleReference(ctx context.Context, opts *BundleReferen useReference := func(ref cnab.OCIReference) error { pullOpts := *opts // make a copy just to do the pull pullOpts.Reference = ref.String() + + err := ensureVPrefix(&pullOpts) + if err != nil { + return err + } + cachedBundle, err := p.prepullBundleByReference(ctx, &pullOpts) if err != nil { return err @@ -298,6 +305,40 @@ func (p *Porter) BuildActionArgs(ctx context.Context, installation storage.Insta return args, nil } +// ensureVPrefix adds a "v" prefix to the version tag if it's not already there. +// 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 { + var ociRef *cnab.OCIReference + if opts._ref != nil { + ociRef = opts._ref + } else { + ref, err := cnab.ParseOCIReference(opts.Reference) + if err != nil { + return fmt.Errorf("unable to parse OCI reference from '%s': %w", opts.Reference, err) + } + ociRef = &ref + } + + if strings.HasPrefix(ociRef.Tag(), "v") { + // don't do anything if "v" is already there + return nil + } + + vRef, err := ociRef.WithTag("v" + ociRef.Tag()) + if err != nil { + return fmt.Errorf("unable to prefix reference tag '%s' with 'v': %w", ociRef.Tag(), err) + } + + // always update the .Reference string, but don't add the _ref field unless it was already there (non-nil) + opts.Reference = vRef.String() + if opts._ref != nil { + opts._ref = &vRef + } + return nil +} + // prepullBundleByReference handles calling the bundle pull operation and updating // the shared options like name and bundle file path. This is used by install, upgrade // and uninstall diff --git a/pkg/porter/lifecycle_test.go b/pkg/porter/lifecycle_test.go index 8f0f35d73..8d7926611 100644 --- a/pkg/porter/lifecycle_test.go +++ b/pkg/porter/lifecycle_test.go @@ -2,6 +2,8 @@ package porter import ( "context" + "fmt" + "strings" "testing" "get.porter.sh/porter/pkg" @@ -530,3 +532,139 @@ func TestPorter_applyActionOptionsToInstallation_PreservesExistingParams(t *test "my-second-param": "2", // Should have kept the existing value from the last run }, params, "Incorrect parameter values were persisted on the installationß") } + +// make sure ensureVPrefix correctly adds a 'v' to the version tag of a reference +func Test_ensureVPrefix(t *testing.T) { + type args struct { + opts *BundleReferenceOptions + } + + ref, err := cnab.ParseOCIReference("registry/bundle:1.2.3") + assert.NoError(t, err) + + testCases := []struct { + name string + args args + wantErr assert.ErrorAssertionFunc + }{ + { + name: "add v to Reference (nil _ref)", + args: struct{ opts *BundleReferenceOptions }{opts: &BundleReferenceOptions{ + installationOptions: installationOptions{}, + BundlePullOptions: BundlePullOptions{ + Reference: "registry/bundle:1.2.3", + _ref: nil, + InsecureRegistry: false, + Force: false, + }, + bundleRef: nil, + }, + }, wantErr: assert.NoError, + }, + { + name: "add v to Reference (non-nil _ref)", + args: struct{ opts *BundleReferenceOptions }{opts: &BundleReferenceOptions{ + installationOptions: installationOptions{}, + BundlePullOptions: BundlePullOptions{ + Reference: "registry/bundle:1.2.3", + _ref: &ref, + InsecureRegistry: false, + Force: false, + }, + bundleRef: nil, + }, + }, wantErr: assert.NoError, + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + // before + idx := strings.LastIndex(tt.args.opts.Reference, ":") + assert.False(t, tt.args.opts.Reference[idx+1] == 'v') + if tt.args.opts._ref != nil { + assert.False(t, tt.args.opts._ref.Tag()[0] == 'v') + } + + err := ensureVPrefix(tt.args.opts) + + // after + tt.wantErr(t, err, fmt.Sprintf("ensureVPrefix(%v)", tt.args.opts)) + + idx = strings.LastIndex(tt.args.opts.Reference, ":") + assert.True(t, tt.args.opts.Reference[idx+1] == 'v') + if tt.args.opts._ref != nil { + assert.True(t, tt.args.opts._ref.Tag()[0] == 'v') + } + }) + } +} + +// make sure ensureVPrefix doesn't add an extra 'v' to the version tag of a reference if it's already there +func Test_ensureVPrefix_idempotent(t *testing.T) { + type args struct { + opts *BundleReferenceOptions + } + + vRef, err := cnab.ParseOCIReference("registry/bundle:v1.2.3") + assert.NoError(t, err) + + testcasesIdempotent := []struct { + name string + args args + wantErr assert.ErrorAssertionFunc + }{ + { + name: "don't add v to Reference with existing v (nil _ref)", + args: struct{ opts *BundleReferenceOptions }{opts: &BundleReferenceOptions{ + installationOptions: installationOptions{}, + BundlePullOptions: BundlePullOptions{ + Reference: "registry/bundle:v1.2.3", + _ref: nil, + InsecureRegistry: false, + Force: false, + }, + bundleRef: nil, + }, + }, wantErr: assert.NoError, + }, + { + name: "don't add v to Reference with existing v (non-nil _ref)", + args: struct{ opts *BundleReferenceOptions }{opts: &BundleReferenceOptions{ + installationOptions: installationOptions{}, + BundlePullOptions: BundlePullOptions{ + Reference: "registry/bundle:v1.2.3", + _ref: &vRef, + InsecureRegistry: false, + Force: false, + }, + bundleRef: nil, + }, + }, wantErr: assert.NoError, + }, + } + for _, tt := range testcasesIdempotent { + t.Run(tt.name, func(t *testing.T) { + // before + idx := strings.LastIndex(tt.args.opts.Reference, ":") + assert.True(t, tt.args.opts.Reference[idx+1] == 'v') + assert.True(t, tt.args.opts.Reference[idx+2] != 'v') + if tt.args.opts._ref != nil { + assert.True(t, tt.args.opts._ref.Tag()[0] == 'v') + assert.True(t, tt.args.opts._ref.Tag()[1] != 'v') + } + + err := ensureVPrefix(tt.args.opts) + + // after + tt.wantErr(t, err, fmt.Sprintf("ensureVPrefix(%v)", tt.args.opts)) + + idx = strings.LastIndex(tt.args.opts.Reference, ":") + assert.True(t, tt.args.opts.Reference[idx+1] == 'v') + assert.True(t, tt.args.opts.Reference[idx+2] != 'v') + if tt.args.opts._ref != nil { + assert.True(t, tt.args.opts._ref.Tag()[0] == 'v') + assert.True(t, tt.args.opts._ref.Tag()[1] != 'v') + } + }) + } +}