Merge pull request #564 from silvin-lubecki/fix-push-bundle

Fix pushing from bundle store

Author: jcsirot

e2e/pushpull_test.go

@@ -269,6 +269,27 @@ func TestPushInstallBundle(t *testing.T) {
 			cmd.Command = dockerCli.Command("service", "ls")
 			assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref2))
 		})
+
+		// push it again using an app pre-bundled and tagged in the bundle store and install it to check it is also available
+		t.Run("push-bundleref", func(t *testing.T) {
+			name := strings.Replace(t.Name(), "/", "_", 1)
+			ref2 := ref + ":v0.42"
+			// Create a new command so the bundle store can be trashed before installing the app
+			cmd2, cleanup2 := dockerCli.createTestCmd()
+			// bundle the app again but this time with a tag to store it into the bundle store
+			cmd2.Command = dockerCli.Command("app", "bundle", "--tag", ref2, "-o", bundleFile, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
+			icmd.RunCmd(cmd2).Assert(t, icmd.Success)
+			// Push the app without tagging it explicitly
+			cmd2.Command = dockerCli.Command("app", "push", "--insecure-registries="+info.registryAddress, ref2)
+			icmd.RunCmd(cmd2).Assert(t, icmd.Success)
+			// remove the bundle from the bundle store to be sure it won't be used instead of registry
+			cleanup2()
+			// install from the registry
+			cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref2, "--name", name)
+			icmd.RunCmd(cmd).Assert(t, icmd.Success)
+			cmd.Command = dockerCli.Command("service", "ls")
+			assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
+		})
 	})
 }
 

internal/commands/cnab.go

@@ -251,6 +251,9 @@ func extractAndLoadAppBasedBundle(dockerCli command.Cli, name string) (*bundle.B
 	return bndl, "", err
 }
 
+//resolveBundle looks for a CNAB bundle which can be in a Docker App Package format or
+// a bundle stored locally or in the bundle store. It returns a built or found bundle,
+// a reference to the bundle if it is found in the bundlestore, and an error.
 func resolveBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name string, pullRef bool, insecureRegistries []string) (*bundle.Bundle, string, error) {
 	// resolution logic:
 	// - if there is a docker-app package in working directory, or an http:// / https:// prefix, use packager.Extract result

internal/commands/push.go

@@ -72,32 +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, _, 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
 	}
-	if err := bndl.Validate(); err != nil {
+	if retag.shouldRetag {
+		if err := retagInvocationImage(dockerCli, bndl, retag.invocationImageRef.String()); err != nil {
+			return err
+		}
+	}
+	// 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
@@ -113,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()) {
@@ -129,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
@@ -181,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)