Repository: vmware-tanzu/carvel-imgpkg (go)

Issue:
Describe command no longer outputs yaml when tty is set to false
**What steps did you take:**
When executing `imgpkg describe -b bundle.io/bundle -o yaml --tty=false` imgpkg is no longer providing any output.

**What happened:**
Nothing is shown in the console

**What did you expect:**
Should see the yaml output

**Anything else you would like to add:**
This behavior should be defined for computer readable output, so when doing `-o text` or not specifying `-o` flag the output with `--tty=false` should not provide any output


**Environment:**

- imgpkg version (use `imgpkg --version`): 0.31.0
- Docker registry used (e.g. `Docker HUB`): any
- OS (e.g. from `/etc/os-release`): any

---
Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help work on this issue.


Pull request:
Fix output issue on describe command
This PR contains 2 changes:
- Move the API calls that previously were in the bundle pkg to the API package in order to signal they are public and people can use them directly
- Fix the issue where `-o yaml` didn't output anything when `--tty=false`


Fixes #434 

Patch:
diff --git a/pkg/imgpkg/bundle/fetch_images.go b/pkg/imgpkg/bundle/fetch_images.go
index 254fad3a..ad2705c5 100644
--- a/pkg/imgpkg/bundle/fetch_images.go
+++ b/pkg/imgpkg/bundle/fetch_images.go
@@ -10,6 +10,11 @@ import (
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 )
 
+// SignatureFetcher Interface to retrieve signatures associated with Images
+type SignatureFetcher interface {
+	FetchForImageRefs(images []lockconfig.ImageRef) ([]lockconfig.ImageRef, error)
+}
+
 // FetchAllImagesRefs returns a flat list of nested bundles and every image reference for a specific bundle
 func (o *Bundle) FetchAllImagesRefs(concurrency int, ui Logger, sigFetcher SignatureFetcher) ([]*Bundle, error) {
 	bundles, _, err := o.AllImagesLockRefs(concurrency, ui)
diff --git a/pkg/imgpkg/cmd/describe.go b/pkg/imgpkg/cmd/describe.go
index bd318f2b..fb988c2d 100644
--- a/pkg/imgpkg/cmd/describe.go
+++ b/pkg/imgpkg/cmd/describe.go
@@ -12,6 +12,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/internal/util"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/v1"
 	"sigs.k8s.io/yaml"
 )
 
@@ -62,11 +63,12 @@ func (d *DescribeOptions) Run() error {
 	if err != nil {
 		return err
 	}
+	logLevel := util.LogWarn
 
-	levelLogger := util.NewUILevelLogger(util.LogWarn, util.NewLogger(d.ui))
-	description, err := bundle.Describe(
+	levelLogger := util.NewUILevelLogger(logLevel, util.NewLogger(d.ui))
+	description, err := v1.Describe(
 		d.BundleFlags.Bundle,
-		bundle.DescribeOpts{
+		v1.DescribeOpts{
 			Logger:                 levelLogger,
 			Concurrency:            d.Concurrency,
 			IncludeCosignArtifacts: d.IncludeCosignArtifacts,
@@ -80,7 +82,7 @@ func (d *DescribeOptions) Run() error {
 		p := bundleTextPrinter{logger: levelLogger}
 		p.Print(description)
 	} else if d.OutputType == "yaml" {
-		p := bundleYAMLPrinter{logger: levelLogger}
+		p := bundleYAMLPrinter{logger: util.NewUILevelLogger(logLevel, util.NewLoggerNoTTY(d.ui))}
 		return p.Print(description)
 	}
 	return nil
@@ -104,7 +106,7 @@ type bundleTextPrinter struct {
 	logger Logger
 }
 
-func (p bundleTextPrinter) Print(description bundle.Description) {
+func (p bundleTextPrinter) Print(description v1.Description) {
 	bundleRef, err := regname.ParseReference(description.Image)
 	if err != nil {
 		panic(fmt.Sprintf("Internal consistency: expected %s to be a digest reference", description.Image))
@@ -115,7 +117,7 @@ func (p bundleTextPrinter) Print(description bundle.Description) {
 	p.printerRec(description, p.logger, p.logger)
 }
 
-func (p bundleTextPrinter) printerRec(description bundle.Description, originalLogger Logger, logger Logger) {
+func (p bundleTextPrinter) printerRec(description v1.Description, originalLogger Logger, logger Logger) {
 	indentLogger := util.NewIndentedLogger(logger)
 	if len(description.Content.Bundles) == 0 && len(description.Content.Images) == 0 {
 		return
@@ -182,7 +184,7 @@ type bundleYAMLPrinter struct {
 	logger Logger
 }
 
-func (p bundleYAMLPrinter) Print(description bundle.Description) error {
+func (p bundleYAMLPrinter) Print(description v1.Description) error {
 	bundleRef, err := regname.ParseReference(description.Image)
 	if err != nil {
 		panic(fmt.Sprintf("Internal consistency: expected %s to be a digest reference", description.Image))
diff --git a/pkg/imgpkg/bundle/describe.go b/pkg/imgpkg/v1/describe.go
similarity index 81%
rename from pkg/imgpkg/bundle/describe.go
rename to pkg/imgpkg/v1/describe.go
index 9d947823..cc7b1983 100644
--- a/pkg/imgpkg/bundle/describe.go
+++ b/pkg/imgpkg/v1/describe.go
@@ -1,13 +1,14 @@
 // Copyright 2022 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-package bundle
+package v1
 
 import (
 	"fmt"
 	"sort"
 
 	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/bundle"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/registry"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/signature"
@@ -36,7 +37,7 @@ type ImageInfo struct {
 	Image       string            `json:"image"`
 	Origin      string            `json:"origin"`
 	Annotations map[string]string `json:"annotations,omitempty"`
-	ImageType   ImageType         `json:"imageType"`
+	ImageType   bundle.ImageType  `json:"imageType"`
 }
 
 // Content Contents present in a Bundle
@@ -56,7 +57,7 @@ type Description struct {
 
 // DescribeOpts Options used when calling the Describe function
 type DescribeOpts struct {
-	Logger                 Logger
+	Logger                 bundle.Logger
 	Concurrency            int
 	IncludeCosignArtifacts bool
 }
@@ -84,30 +85,38 @@ func Describe(bundleImage string, opts DescribeOpts, registryOpts registry.Opts)
 }
 
 // DescribeWithRegistryAndSignatureFetcher Given a Bundle URL fetch the information about the contents of the Bundle and Nested Bundles
-func DescribeWithRegistryAndSignatureFetcher(bundleImage string, opts DescribeOpts, reg ImagesMetadata, sigFetcher SignatureFetcher) (Description, error) {
-	bundle := NewBundle(bundleImage, reg)
-	allBundles, err := bundle.FetchAllImagesRefs(opts.Concurrency, opts.Logger, sigFetcher)
+func DescribeWithRegistryAndSignatureFetcher(bundleImage string, opts DescribeOpts, reg bundle.ImagesMetadata, sigFetcher SignatureFetcher) (Description, error) {
+	newBundle := bundle.NewBundle(bundleImage, reg)
+	isBundle, err := newBundle.IsBundle()
+	if err != nil {
+		return Description{}, fmt.Errorf("Unable to check if %s is a bundle: %s", bundleImage, err)
+	}
+	if !isBundle {
+		return Description{}, fmt.Errorf("Only bundles can be described, and %s is not a bundle", bundleImage)
+	}
+
+	allBundles, err := newBundle.FetchAllImagesRefs(opts.Concurrency, opts.Logger, sigFetcher)
 	if err != nil {
 		return Description{}, fmt.Errorf("Retrieving Images from bundle: %s", err)
 	}
 
 	topBundle := refWithDescription{
-		imgRef: NewBundleImageRef(lockconfig.ImageRef{Image: bundle.DigestRef()}),
+		imgRef: bundle.NewBundleImageRef(lockconfig.ImageRef{Image: newBundle.DigestRef()}),
 	}
 	return topBundle.DescribeBundle(allBundles), nil
 }
 
 type refWithDescription struct {
-	imgRef ImageRef
+	imgRef bundle.ImageRef
 	bundle Description
 }
 
-func (r *refWithDescription) DescribeBundle(bundles []*Bundle) Description {
+func (r *refWithDescription) DescribeBundle(bundles []*bundle.Bundle) Description {
 	var visitedImgs map[string]refWithDescription
 	return r.describeBundleRec(visitedImgs, r.imgRef, bundles)
 }
 
-func (r *refWithDescription) describeBundleRec(visitedImgs map[string]refWithDescription, currentBundle ImageRef, bundles []*Bundle) Description {
+func (r *refWithDescription) describeBundleRec(visitedImgs map[string]refWithDescription, currentBundle bundle.ImageRef, bundles []*bundle.Bundle) Description {
 	desc, wasVisited := visitedImgs[currentBundle.Image]
 	if wasVisited {
 		return desc.bundle
@@ -126,18 +135,18 @@ func (r *refWithDescription) describeBundleRec(visitedImgs map[string]refWithDes
 			},
 		},
 	}
-	var bundle *Bundle
+	var newBundle *bundle.Bundle
 	for _, b := range bundles {
 		if b.DigestRef() == currentBundle.PrimaryLocation() {
-			bundle = b
+			newBundle = b
 			break
 		}
 	}
-	if bundle == nil {
+	if newBundle == nil {
 		panic("Internal consistency: bundle could not be found in list of bundles")
 	}
 
-	imagesRefs := bundle.ImagesRefs()
+	imagesRefs := newBundle.ImagesRefs()
 	sort.Slice(imagesRefs, func(i, j int) bool {
 		return imagesRefs[i].Image < imagesRefs[j].Image
 	})


Test patch:
diff --git a/pkg/imgpkg/bundle/describe_test.go b/pkg/imgpkg/v1/describe_test.go
similarity index 92%
rename from pkg/imgpkg/bundle/describe_test.go
rename to pkg/imgpkg/v1/describe_test.go
index eb56173d..2163dba7 100644
--- a/pkg/imgpkg/bundle/describe_test.go
+++ b/pkg/imgpkg/v1/describe_test.go
@@ -1,7 +1,7 @@
 // Copyright 2022 VMware, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-package bundle_test
+package v1_test
 
 import (
 	"fmt"
@@ -17,6 +17,7 @@ import (
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/registry"
 	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/signature/cosign"
+	"github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/v1"
 	"github.com/vmware-tanzu/carvel-imgpkg/test/helpers"
 )
 
@@ -291,14 +292,14 @@ func TestDescribeBundle(t *testing.T) {
 	for _, test := range allTests {
 		t.Run(test.description, func(t *testing.T) {
 			fakeRegBuilder := helpers.NewFakeRegistry(t, logger)
-			topBundle := createBundle(t, fakeRegBuilder, test.subject, map[string]*createdBundle{}, map[string]*helpers.ImageOrImageIndexWithTarPath{}, test.includeCosignArtifacts)
+			topBundle := createBundleRec(t, fakeRegBuilder, test.subject, map[string]*createdBundle{}, map[string]*helpers.ImageOrImageIndexWithTarPath{}, test.includeCosignArtifacts)
 			fakeRegBuilder.Build()
 
 			fmt.Printf("Expected structure:\n\n")
 			topBundle.Print("")
 			fmt.Printf("++++++++++++++++\n\n")
 
-			bundleDescription, err := ctlbundle.Describe(topBundle.refDigest, ctlbundle.DescribeOpts{
+			bundleDescription, err := v1.Describe(topBundle.refDigest, v1.DescribeOpts{
 				Logger:                 logger,
 				Concurrency:            1,
 				IncludeCosignArtifacts: test.includeCosignArtifacts,
@@ -367,7 +368,7 @@ func (c createdBundle) Print(prefix string) {
 	}
 }
 
-func printDescribedBundle(prefix string, bundle ctlbundle.Description) {
+func printDescribedBundle(prefix string, bundle v1.Description) {
 	fmt.Printf("%sBundle: %s\n", prefix, bundle.Image)
 	fmt.Printf("%s%sAnnotations: %v\n", prefix, prefix, bundle.Annotations)
 	for _, b := range bundle.Content.Bundles {
@@ -386,7 +387,7 @@ func printDescribedBundle(prefix string, bundle ctlbundle.Description) {
 	}
 }
 
-func assertBundleResult(t *testing.T, expectedBundle createdBundle, result ctlbundle.Description) {
+func assertBundleResult(t *testing.T, expectedBundle createdBundle, result v1.Description) {
 	for _, image := range expectedBundle.images {
 		if len(image.images) > 0 {
 			bundleDesc, imgInfo, ok := findImageWithRef(result, image.refDigest)
@@ -413,10 +414,10 @@ func assertBundleResult(t *testing.T, expectedBundle createdBundle, result ctlbu
 		}
 	}
 }
-func findImageWithRef(bundle ctlbundle.Description, refDigest string) (ctlbundle.Description, ctlbundle.ImageInfo, bool) {
+func findImageWithRef(bundle v1.Description, refDigest string) (v1.Description, v1.ImageInfo, bool) {
 	for _, bundleDesc := range bundle.Content.Bundles {
 		if bundleDesc.Image == refDigest {
-			return bundleDesc, ctlbundle.ImageInfo{
+			return bundleDesc, v1.ImageInfo{
 				Image:       bundle.Image,
 				Origin:      bundle.Origin,
 				Annotations: bundle.Annotations,
@@ -425,13 +426,13 @@ func findImageWithRef(bundle ctlbundle.Description, refDigest string) (ctlbundle
 	}
 	for _, img := range bundle.Content.Images {
 		if img.Image == refDigest {
-			return ctlbundle.Description{}, img, true
+			return v1.Description{}, img, true
 		}
 	}
-	return ctlbundle.Description{}, ctlbundle.ImageInfo{}, false
+	return v1.Description{}, v1.ImageInfo{}, false
 }
 
-func createBundle(t *testing.T, reg *helpers.FakeTestRegistryBuilder, bToCreate testBundle, allBundlesCreated map[string]*createdBundle, createdImages map[string]*helpers.ImageOrImageIndexWithTarPath, includeCosignArtifacts bool) createdBundle {
+func createBundleRec(t *testing.T, reg *helpers.FakeTestRegistryBuilder, bToCreate testBundle, allBundlesCreated map[string]*createdBundle, createdImages map[string]*helpers.ImageOrImageIndexWithTarPath, includeCosignArtifacts bool) createdBundle {
 	if cb, ok := allBundlesCreated[bToCreate.name]; ok {
 		return *cb
 	}
@@ -444,7 +445,7 @@ func createBundle(t *testing.T, reg *helpers.FakeTestRegistryBuilder, bToCreate
 	for _, image := range bToCreate.images {
 		imgDigestRef := ""
 		if len(image.images) > 0 {
-			innerBundle := createBundle(t, reg, image.testBundle, allBundlesCreated, createdImages, includeCosignArtifacts)
+			innerBundle := createBundleRec(t, reg, image.testBundle, allBundlesCreated, createdImages, includeCosignArtifacts)
 			imgs = append(imgs, lockconfig.ImageRef{Image: innerBundle.refDigest})
 			result.images = append(result.images, createdImage{createdBundle: innerBundle})
 			imgDigestRef = innerBundle.refDigest
diff --git a/test/e2e/describe_test.go b/test/e2e/describe_test.go
index 6c7d7efa..c2a686f1 100644
--- a/test/e2e/describe_test.go
+++ b/test/e2e/describe_test.go
@@ -397,7 +397,105 @@ origin: %s%s
 		})
 	})
 
-	t.Run("bundle with bundle collocated with text output", func(t *testing.T) {
+	t.Run("bundle prints output even when tty=false", func(t *testing.T) {
+		env := helpers.BuildEnv(t)
+		imgpkg := helpers.Imgpkg{T: t, L: helpers.Logger{}, ImgpkgPath: env.ImgpkgPath}
+		defer env.Cleanup()
+
+		bundleTag := fmt.Sprintf(":%d", time.Now().UnixNano())
+		var bundleDigest, imageDigest, imgSigTag, bundleSigTag, imgSigDigest, bundleSigDigest string
+		logger.Section("create bundle with image", func() {
+			imageDigest = env.ImageFactory.PushSimpleAppImageWithRandomFile(imgpkg, env.Image)
+
+			imageLockYAML := fmt.Sprintf(`---
+apiVersion: imgpkg.carvel.dev/v1alpha1
+kind: ImagesLock
+images:
+- image: %s%s
+  annotations:
+    some.other.annotation: some other value
+    some.annotation: some value
+`, env.Image, imageDigest)
+			bundleDir := env.BundleFactory.CreateBundleDir(helpers.BundleYAML, imageLockYAML)
+
+			out := imgpkg.Run([]string{"push", "--tty", "-b", fmt.Sprintf("%s%s", env.Image, bundleTag), "-f", bundleDir})
+			bundleDigest = fmt.Sprintf("@%s", helpers.ExtractDigest(t, out))
+
+			logger.Section("sign image and Bundle", func() {
+				imgSigTag = env.ImageFactory.SignImage(fmt.Sprintf("%s%s", env.Image, imageDigest))
+				imgSigDigest = env.ImageFactory.ImageDigest(fmt.Sprintf("%s:%s", env.Image, imgSigTag))
+				bundleSigTag = env.ImageFactory.SignImage(fmt.Sprintf("%s%s", env.Image, bundleTag))
+				bundleSigDigest = env.ImageFactory.ImageDigest(fmt.Sprintf("%s:%s", env.Image, bundleSigTag))
+			})
+		})
+
+		logger.Section("copy bundle to repository", func() {
+			imgpkg.Run([]string{"copy",
+				"--bundle", fmt.Sprintf("%s%s", env.Image, bundleDigest),
+				"--to-repo", env.RelocationRepo,
+				"--cosign-signatures",
+			},
+			)
+		})
+
+		logger.Section("executes describe command", func() {
+			stdout := imgpkg.Run(
+				[]string{"describe",
+					"--tty=false", "--bundle", fmt.Sprintf("%s%s", env.RelocationRepo, bundleDigest),
+					"-o", "yaml",
+				},
+			)
+			locationsImgDigest := env.ImageFactory.ImageDigest(fmt.Sprintf("%s:%s.image-locations.imgpkg", env.RelocationRepo, strings.ReplaceAll(bundleDigest[1:], ":", "-")))
+
+			require.YAMLEq(t, fmt.Sprintf(`content:
+  images:
+    "%s":
+      annotations:
+        some.annotation: some value
+        some.other.annotation: some other value
+      image: %s%s
+      imageType: Image
+      origin: %s%s
+    "%s":
+      annotations:
+        tag: %s
+      image: %s@%s
+      imageType: Signature
+      origin: %s@%s
+    "%s":
+      annotations:
+        tag: %s
+      image: %s@%s
+      imageType: Signature
+      origin: %s@%s
+    "%s":
+      image: %s@%s
+      imageType: Internal
+      origin: %s@%s
+metadata: {}
+image: %s%s
+origin: %s%s
+sha: %s
+`,
+				imageDigest[1:],
+				env.RelocationRepo, imageDigest,
+				env.Image, imageDigest,
+				bundleSigDigest,
+				bundleSigTag,
+				env.RelocationRepo, bundleSigDigest,
+				env.RelocationRepo, bundleSigDigest,
+				imgSigDigest,
+				imgSigTag,
+				env.RelocationRepo, imgSigDigest,
+				env.RelocationRepo, imgSigDigest,
+				locationsImgDigest,
+				env.RelocationRepo, locationsImgDigest,
+				env.RelocationRepo, locationsImgDigest,
+				env.RelocationRepo, bundleDigest, env.RelocationRepo, bundleDigest, bundleDigest[1:]), stdout)
+		})
+	})
+
+	t.Run("bundle with bundle collocated with yaml output", func(t *testing.T) {
 		env := helpers.BuildEnv(t)
 		imgpkg := helpers.Imgpkg{T: t, L: helpers.Logger{}, ImgpkgPath: env.ImgpkgPath}
 		defer env.Cleanup()