Refactor push command to lower the cyclomatic complexity

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>

Author: silvin-lubecki

internal/commands/push.go

@@ -72,36 +72,46 @@ func pushCmd(dockerCli command.Cli) *cobra.Command {
 
 func runPush(dockerCli command.Cli, name string, opts pushOptions) error {
 	defer muteDockerCli(dockerCli)()
-
-	bundleStore, err := prepareBundleStore()
+	// Get the bundle
+	bndl, ref, err := resolveReferenceAndBundle(dockerCli, name)
 	if err != nil {
 		return err
 	}
-
-	bndl, ref, err := resolveBundle(dockerCli, bundleStore, name, false, nil)
+	// Retag invocation image if needed
+	retag, err := shouldRetagInvocationImage(metadata.FromBundle(bndl), bndl, opts.tag, ref)
 	if err != nil {
 		return err
 	}
-	// Use the bundle reference as a tag
-	if ref != "" && opts.tag == "" {
-		opts.tag = ref
+	if retag.shouldRetag {
+		if err := retagInvocationImage(dockerCli, bndl, retag.invocationImageRef.String()); err != nil {
+			return err
+		}
 	}
-	if err := bndl.Validate(); err != nil {
+	// Push the invocation image
+	if err := pushInvocationImage(dockerCli, retag); err != nil {
 		return err
 	}
+	// Push the bundle
+	return pushBundle(dockerCli, opts, bndl, retag)
+}
 
-	retag, err := shouldRetagInvocationImage(metadata.FromBundle(bndl), bndl, opts.tag)
+func resolveReferenceAndBundle(dockerCli command.Cli, name string) (*bundle.Bundle, string, error) {
+	bundleStore, err := prepareBundleStore()
 	if err != nil {
-		return err
+		return nil, "", err
 	}
-	if retag.shouldRetag {
-		err := retagInvocationImage(dockerCli, bndl, retag.invocationImageRef.String())
-		if err != nil {
-			return err
-		}
+
+	bndl, ref, err := resolveBundle(dockerCli, bundleStore, name, false, nil)
+	if err != nil {
+		return nil, "", err
+	}
+	if err := bndl.Validate(); err != nil {
+		return nil, "", err
 	}
+	return bndl, ref, err
+}
 
-	// pushing invocation image
+func pushInvocationImage(dockerCli command.Cli, retag retagResult) error {
 	repoInfo, err := registry.ParseRepositoryInfo(retag.invocationImageRef)
 	if err != nil {
 		return err
@@ -117,10 +127,13 @@ func runPush(dockerCli command.Cli, name string, opts pushOptions) error {
 		return errors.Wrapf(err, "starting push of %q", retag.invocationImageRef.String())
 	}
 	defer reader.Close()
-	if err = jsonmessage.DisplayJSONMessagesStream(reader, ioutil.Discard, 0, false, nil); err != nil {
+	if err := jsonmessage.DisplayJSONMessagesStream(reader, ioutil.Discard, 0, false, nil); err != nil {
 		return errors.Wrapf(err, "pushing to %q", retag.invocationImageRef.String())
 	}
+	return nil
+}
 
+func pushBundle(dockerCli command.Cli, opts pushOptions, bndl *bundle.Bundle, retag retagResult) error {
 	resolver := remotes.CreateResolver(dockerCli.ConfigFile(), opts.registry.insecureRegistries...)
 	var display fixupDisplay = &plainDisplay{out: os.Stdout}
 	if term.IsTerminal(os.Stdout.Fd()) {
@@ -133,9 +146,7 @@ func runPush(dockerCli command.Cli, name string, opts pushOptions) error {
 		fixupOptions = append(fixupOptions, remotes.WithComponentImagePlatforms(platforms))
 	}
 	// bundle fixup
-	err = remotes.FixupBundle(context.Background(), bndl, retag.cnabRef, resolver, fixupOptions...)
-
-	if err != nil {
+	if err := remotes.FixupBundle(context.Background(), bndl, retag.cnabRef, resolver, fixupOptions...); err != nil {
 		return errors.Wrapf(err, "fixing up %q for push", retag.cnabRef)
 	}
 	// push bundle manifest
@@ -185,7 +196,11 @@ type retagResult struct {
 	invocationImageRef reference.Named
 }
 
-func shouldRetagInvocationImage(meta metadata.AppMetadata, bndl *bundle.Bundle, tagOverride string) (retagResult, error) {
+func shouldRetagInvocationImage(meta metadata.AppMetadata, bndl *bundle.Bundle, tagOverride, bundleRef string) (retagResult, error) {
+	// Use the bundle reference as a tag override
+	if tagOverride == "" && bundleRef != "" {
+		tagOverride = bundleRef
+	}
 	imgName := tagOverride
 	var err error
 	if imgName == "" {

internal/commands/push_test.go

@@ -15,6 +15,7 @@ type retagTestCase struct {
 	metaVersion         string
 	invocationImageName string
 	tag                 string
+	bundleRef           string
 	expectedImageRef    reference.Named
 	expectedCnabRef     reference.Named
 	shouldRetag         bool
@@ -40,7 +41,7 @@ func parseRefOrDie(t *testing.T, name string) reference.Named {
 func TestInvocationImageRetag(t *testing.T) {
 	cases := []retagTestCase{
 		{
-			name:                "no-tag-aligned-names",
+			name:                "no-tag-override-should-not-retag",
 			metaName:            "app",
 			metaVersion:         "0.1.0",
 			invocationImageName: "app:0.1.0-invoc",
@@ -50,7 +51,7 @@ func TestInvocationImageRetag(t *testing.T) {
 			shouldRetag:         false,
 		},
 		{
-			name:                "tag-aligned-names-untagged-ref",
+			name:                "name-override-should-retag",
 			metaName:            "app",
 			metaVersion:         "0.1.0",
 			invocationImageName: "app:0.1.0-invoc",
@@ -60,7 +61,7 @@ func TestInvocationImageRetag(t *testing.T) {
 			shouldRetag:         true,
 		},
 		{
-			name:                "tag-aligned-names-tagged-ref",
+			name:                "tag-override-should-retag",
 			metaName:            "app",
 			metaVersion:         "0.1.0",
 			invocationImageName: "app:0.1.0-invoc",
@@ -69,6 +70,27 @@ func TestInvocationImageRetag(t *testing.T) {
 			expectedCnabRef:     parseRefOrDie(t, "some-app:test"),
 			shouldRetag:         true,
 		},
+		{
+			name:                "bundle-ref-should-retag",
+			metaName:            "app",
+			metaVersion:         "0.1.0",
+			invocationImageName: "app:0.1.0-invoc",
+			bundleRef:           "some-app:test",
+			expectedImageRef:    parseRefOrDie(t, "some-app:test-invoc"),
+			expectedCnabRef:     parseRefOrDie(t, "some-app:test"),
+			shouldRetag:         true,
+		},
+		{
+			name:                "tag-overrides-bundle-ref",
+			metaName:            "app",
+			metaVersion:         "0.1.0",
+			invocationImageName: "app:0.1.0-invoc",
+			tag:                 "some-app:test",
+			bundleRef:           "other-app:other-test",
+			expectedImageRef:    parseRefOrDie(t, "some-app:test-invoc"),
+			expectedCnabRef:     parseRefOrDie(t, "some-app:test"),
+			shouldRetag:         true,
+		},
 		{
 			name:                "parsing-error",
 			metaName:            "app",
@@ -78,7 +100,7 @@ func TestInvocationImageRetag(t *testing.T) {
 			errorMessage:        "some-App:test: invalid reference format: repository name must be lowercase",
 		},
 		{
-			name:                "no-digest",
+			name:                "error-no-digest",
 			metaName:            "app",
 			metaVersion:         "0.1.0",
 			invocationImageName: "app:0.1.0-invoc",
@@ -90,7 +112,7 @@ func TestInvocationImageRetag(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
 			p, b := c.buildPackageMetaAndBundle()
-			retag, err := shouldRetagInvocationImage(p, b, c.tag)
+			retag, err := shouldRetagInvocationImage(p, b, c.tag, c.bundleRef)
 			if c.errorMessage == "" {
 				assert.NilError(t, err)
 				assert.Equal(t, retag.shouldRetag, c.shouldRetag)