From 4924f1f297559193d62e669c1b13948e684b1183 Mon Sep 17 00:00:00 2001 From: Per Hermansson Date: Thu, 18 May 2023 11:27:20 +0200 Subject: [PATCH] Add max-depth flag to describe Signed-off-by: Per Hermansson --- pkg/imgpkg/bundle/bundle_images_lock.go | 29 ++++- pkg/imgpkg/bundle/bundle_images_lock_test.go | 8 +- pkg/imgpkg/bundle/bundle_test.go | 2 +- pkg/imgpkg/bundle/fetch_images.go | 4 +- pkg/imgpkg/cmd/copy_repo_src.go | 4 +- pkg/imgpkg/cmd/describe.go | 3 + pkg/imgpkg/v1/describe.go | 5 +- pkg/imgpkg/v1/describe_test.go | 115 ++++++++++++++++++- 8 files changed, 150 insertions(+), 20 deletions(-) diff --git a/pkg/imgpkg/bundle/bundle_images_lock.go b/pkg/imgpkg/bundle/bundle_images_lock.go index f93f7be31..93cab431a 100644 --- a/pkg/imgpkg/bundle/bundle_images_lock.go +++ b/pkg/imgpkg/bundle/bundle_images_lock.go @@ -18,6 +18,11 @@ import ( "github.com/vmware-tanzu/carvel-imgpkg/pkg/imgpkg/lockconfig" ) +const ( + // NoDepthLimit results in unlimited max recursion depth + NoDepthLimit = 0 +) + // ImagesRefsWithErrors Retrieve the references for the Images of this particular bundle including images that imgpkg // was not able to retrieve information for func (o *Bundle) ImagesRefsWithErrors() []ImageRef { @@ -25,14 +30,14 @@ func (o *Bundle) ImagesRefsWithErrors() []ImageRef { } // AllImagesLockRefs returns a flat list of nested bundles and every image reference for a specific bundle -func (o *Bundle) AllImagesLockRefs(concurrency int, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, error) { +func (o *Bundle) AllImagesLockRefs(concurrency int, maxDepth int, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, error) { throttleReq := util.NewThrottle(concurrency) - return o.buildAllImagesLock(&throttleReq, logger) + return o.buildAllImagesLock(&throttleReq, maxDepth, logger) } // buildAllImagesLock recursive function that will iterate over the Bundle graph and collect all the bundles and images -func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, error) { +func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, maxDepth int, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, error) { img, err := o.checkedImage() if err != nil { return nil, ImageRefs{}, err @@ -71,9 +76,17 @@ func (o *Bundle) buildAllImagesLock(throttleReq *util.Throttle, logger util.Logg continue } + if maxDepth == 1 { + typedImageRef := NewBundleImageRef(image.ImageRef).DeepCopy() + processedImageRefs.AddImagesRef(typedImageRef) + o.cachedImageRefs.StoreImageRef(typedImageRef) + errChan <- nil + continue + } + image := image.DeepCopy() go func() { - nestedBundles, nestedBundlesProcessedImageRefs, imgRef, err := o.imagesLockIfIsBundle(throttleReq, image, logger) + nestedBundles, nestedBundlesProcessedImageRefs, imgRef, err := o.imagesLockIfIsBundle(throttleReq, maxDepth, image, logger) if err != nil { errChan <- err return @@ -129,7 +142,7 @@ func (o *Bundle) fetchImagesRef(img regv1.Image, locationsConfig ImageRefLocatio } // imagesLockIfIsBundle retrieve all the images associated with Bundle imgRef. if it is not a bundle will return no new images -func (o *Bundle) imagesLockIfIsBundle(throttleReq *util.Throttle, imgRef ImageRef, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, lockconfig.ImageRef, error) { +func (o *Bundle) imagesLockIfIsBundle(throttleReq *util.Throttle, maxDepth int, imgRef ImageRef, logger util.LoggerWithLevels) ([]*Bundle, ImageRefs, lockconfig.ImageRef, error) { newImgRef, bundle, err := o.bundleFetcher.Bundle(throttleReq, imgRef) if err != nil { return nil, ImageRefs{}, lockconfig.ImageRef{}, err @@ -138,7 +151,11 @@ func (o *Bundle) imagesLockIfIsBundle(throttleReq *util.Throttle, imgRef ImageRe var processedImageRefs ImageRefs var nestedBundles []*Bundle if bundle != nil { - nestedBundles, processedImageRefs, err = bundle.buildAllImagesLock(throttleReq, logger) + var newDepth = maxDepth + if maxDepth > 1 { + newDepth = maxDepth - 1 + } + nestedBundles, processedImageRefs, err = bundle.buildAllImagesLock(throttleReq, newDepth, logger) if err != nil { return nil, ImageRefs{}, lockconfig.ImageRef{}, fmt.Errorf("Retrieving images for bundle '%s': %s", imgRef.Image, err) } diff --git a/pkg/imgpkg/bundle/bundle_images_lock_test.go b/pkg/imgpkg/bundle/bundle_images_lock_test.go index aacba61b0..637f7eaa8 100644 --- a/pkg/imgpkg/bundle/bundle_images_lock_test.go +++ b/pkg/imgpkg/bundle/bundle_images_lock_test.go @@ -452,7 +452,7 @@ func TestBundle_AllImagesLock_NoLocations_AllImagesCollocated(t *testing.T) { fmt.Println("============") subject, metrics := subTest.subjectCreator.BuildSubject(t, registryFakeBuilder, topBundleInfo, fakeImagesLockReader) - bundles, imagesRefs, err := subject.AllImagesLockRefs(6, uiLogger) + bundles, imagesRefs, err := subject.AllImagesLockRefs(6, bundle.NoDepthLimit, uiLogger) require.NoError(t, err) runAssertions(t, test.assertions, imagesRefs, imagesTree) @@ -729,7 +729,7 @@ func TestBundle_AllImagesLock_NoLocations_ImagesNotCollocated(t *testing.T) { metrics.AddMetricsHandler(registryFakeBuilder) subject := bundle.NewBundleFromRef(topBundleInfo, reg, fakeImagesLockReader, bundle.NewRegistryFetcher(reg, fakeImagesLockReader)) - bundles, imagesRefs, err := subject.AllImagesLockRefs(1, uiLogger) + bundles, imagesRefs, err := subject.AllImagesLockRefs(1, bundle.NoDepthLimit, uiLogger) require.NoError(t, err) runAssertions(t, test.assertions, imagesRefs, imagesTree) @@ -1168,7 +1168,7 @@ func TestBundle_AllImagesLock_Locations_AllImagesCollocated(t *testing.T) { fmt.Println("============") subject, metrics := subTest.subjectCreator.BuildSubject(t, registryFakeBuilder, topBundleInfo, fakeImagesLockReader) - bundles, imagesRefs, err := subject.AllImagesLockRefs(6, uiLogger) + bundles, imagesRefs, err := subject.AllImagesLockRefs(6, bundle.NoDepthLimit, uiLogger) require.NoError(t, err) runAssertions(t, test.assertions, imagesRefs, imagesTree) @@ -1221,7 +1221,7 @@ func TestBundle_AllImagesLock_Locations_AllImagesCollocated(t *testing.T) { metrics.AddMetricsHandler(registryFakeBuilder) subject := bundle.NewBundleFromRef(topBundleInfo, reg, fakeImagesLockReader, bundle.NewRegistryFetcher(reg, fakeImagesLockReader)) - bundles, _, err := subject.AllImagesLockRefs(6, uiLogger) + bundles, _, err := subject.AllImagesLockRefs(6, bundle.NoDepthLimit, uiLogger) require.NoError(t, err) checkBundlesPresence(t, bundles, imagesTree) diff --git a/pkg/imgpkg/bundle/bundle_test.go b/pkg/imgpkg/bundle/bundle_test.go index 02fe82e32..c87a6d9aa 100644 --- a/pkg/imgpkg/bundle/bundle_test.go +++ b/pkg/imgpkg/bundle/bundle_test.go @@ -857,7 +857,7 @@ func TestNoteCopy(t *testing.T) { uiLogger := util.NewUILevelLogger(util.LogDebug, util.NewNoopLogger()) subject := bundle.NewBundleFromPlainImage(plainimage.NewFetchedPlainImageWithTag(rootBundle.RefDigest, "", rootBundle.Image), reg) - _, _, err = subject.AllImagesLockRefs(1, uiLogger) + _, _, err = subject.AllImagesLockRefs(1, bundle.NoDepthLimit, uiLogger) require.NoError(t, err) processedImages := imageset.NewProcessedImages() diff --git a/pkg/imgpkg/bundle/fetch_images.go b/pkg/imgpkg/bundle/fetch_images.go index 59c760949..f9c990058 100644 --- a/pkg/imgpkg/bundle/fetch_images.go +++ b/pkg/imgpkg/bundle/fetch_images.go @@ -17,8 +17,8 @@ type SignatureFetcher interface { } // 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) +func (o *Bundle) FetchAllImagesRefs(concurrency int, maxDepth int, ui Logger, sigFetcher SignatureFetcher) ([]*Bundle, error) { + bundles, _, err := o.AllImagesLockRefs(concurrency, maxDepth, ui) if err != nil { return nil, err } diff --git a/pkg/imgpkg/cmd/copy_repo_src.go b/pkg/imgpkg/cmd/copy_repo_src.go index 940162fa9..ef9f80a3f 100644 --- a/pkg/imgpkg/cmd/copy_repo_src.go +++ b/pkg/imgpkg/cmd/copy_repo_src.go @@ -95,7 +95,7 @@ func (c CopyRepoSrc) CopyToRepo(repo string) (*ctlimgset.ProcessedImages, error) } if foundRootBundle { - bundles, _, err := parentBundle.AllImagesLockRefs(c.Concurrency, c.logger) + bundles, _, err := parentBundle.AllImagesLockRefs(c.Concurrency, ctlbundle.NoDepthLimit, c.logger) if err != nil { return nil, err } @@ -257,7 +257,7 @@ func (c CopyRepoSrc) getBundleImageRefs(bundleRef string) (*ctlbundle.Bundle, [] return nil, nil, ctlbundle.ImageRefs{}, fmt.Errorf("Expected bundle image but found plain image (hint: Did you use -i instead of -b?)") } - nestedBundles, imageRefs, err := bundle.AllImagesLockRefs(c.Concurrency, c.logger) + nestedBundles, imageRefs, err := bundle.AllImagesLockRefs(c.Concurrency, ctlbundle.NoDepthLimit, c.logger) if err != nil { return nil, nil, ctlbundle.ImageRefs{}, fmt.Errorf("Reading Images from Bundle: %s", err) } diff --git a/pkg/imgpkg/cmd/describe.go b/pkg/imgpkg/cmd/describe.go index fb988c2d4..0ee0804a3 100644 --- a/pkg/imgpkg/cmd/describe.go +++ b/pkg/imgpkg/cmd/describe.go @@ -29,6 +29,7 @@ type DescribeOptions struct { RegistryFlags RegistryFlags Concurrency int + MaxDepth int OutputType string IncludeCosignArtifacts bool } @@ -52,6 +53,7 @@ func NewDescribeCmd(o *DescribeOptions) *cobra.Command { o.BundleFlags.SetCopy(cmd) o.RegistryFlags.Set(cmd) cmd.Flags().IntVar(&o.Concurrency, "concurrency", 5, "Concurrency") + cmd.Flags().IntVar(&o.MaxDepth, "max-depth", 0, "Maximum recursion depth (0 is no limit)") cmd.Flags().StringVarP(&o.OutputType, "output-type", "o", "text", "Type of output possible values: [text, yaml]") cmd.Flags().BoolVar(&o.IncludeCosignArtifacts, "cosign-artifacts", true, "Retrieve cosign artifact information (Default: true)") return cmd @@ -71,6 +73,7 @@ func (d *DescribeOptions) Run() error { v1.DescribeOpts{ Logger: levelLogger, Concurrency: d.Concurrency, + MaxDepth: d.MaxDepth, IncludeCosignArtifacts: d.IncludeCosignArtifacts, }, d.RegistryFlags.AsRegistryOpts()) diff --git a/pkg/imgpkg/v1/describe.go b/pkg/imgpkg/v1/describe.go index ebe071d3f..2d2fcca4e 100644 --- a/pkg/imgpkg/v1/describe.go +++ b/pkg/imgpkg/v1/describe.go @@ -61,6 +61,7 @@ type Description struct { type DescribeOpts struct { Logger bundle.Logger Concurrency int + MaxDepth int IncludeCosignArtifacts bool } @@ -98,7 +99,7 @@ func DescribeWithRegistryAndSignatureFetcher(bundleImage string, opts DescribeOp 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) + allBundles, err := newBundle.FetchAllImagesRefs(opts.Concurrency, opts.MaxDepth, opts.Logger, sigFetcher) if err != nil { return Description{}, fmt.Errorf("Retrieving Images from bundle: %s", err) } @@ -146,7 +147,7 @@ func (r *refWithDescription) describeBundleRec(visitedImgs map[string]refWithDes } } if newBundle == nil { - panic(fmt.Sprintf("Internal consistency: bundle with ref '%s' could not be found in list of bundles", currentBundle.PrimaryLocation())) + return desc.bundle } imagesRefs := newBundle.ImagesRefsWithErrors() diff --git a/pkg/imgpkg/v1/describe_test.go b/pkg/imgpkg/v1/describe_test.go index 1df47252c..b180526b3 100644 --- a/pkg/imgpkg/v1/describe_test.go +++ b/pkg/imgpkg/v1/describe_test.go @@ -26,6 +26,7 @@ type testDescribe struct { description string subject testBundle includeCosignArtifacts bool + maxDepth int } func TestDescribeBundle(t *testing.T) { @@ -303,6 +304,7 @@ func TestDescribeBundle(t *testing.T) { bundleDescription, err := v1.Describe(topBundle.refDigest, v1.DescribeOpts{ Logger: logger, Concurrency: 1, + MaxDepth: test.maxDepth, IncludeCosignArtifacts: test.includeCosignArtifacts, }, registry.Opts{ @@ -318,7 +320,101 @@ func TestDescribeBundle(t *testing.T) { require.Equal(t, topBundle.refDigest, bundleDescription.Image) - assertBundleResult(t, topBundle, bundleDescription) + assertBundleResult(t, topBundle, bundleDescription, test.maxDepth) + }) + } +} + +func TestDescribeBundleWithMaxDepth(t *testing.T) { + logger := &helpers.Logger{LogLevel: helpers.LogDebug} + + allTests := []testDescribe{ + { + description: "Bundle with no images", + subject: testBundle{ + name: "simple/no-images-bundle", + }, + maxDepth: 1, + }, + { + description: "Bundle with only images are resolved", + subject: testBundle{ + name: "simple/only-images-bundle", + images: []testImage{ + { + testBundle{ + name: "app/img1", + }, + }, + }, + }, + maxDepth: 1, + }, + { + description: "Bundle with inner bundles", + subject: testBundle{ + name: "simple/outer-bundle", + images: []testImage{ + { + testBundle{ + name: "app/bundle1", + images: []testImage{ + { + testBundle{ + name: "app/img1", + }, + }, + { + testBundle{ + name: "app1/inner-bundle-not-fetched", + images: []testImage{ + { + testBundle{ + name: "random/img1", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + maxDepth: 2, + }, + } + + for _, test := range allTests { + t.Run(test.description, func(t *testing.T) { + fakeRegBuilder := helpers.NewFakeRegistry(t, logger) + 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 := v1.Describe(topBundle.refDigest, v1.DescribeOpts{ + Logger: logger, + Concurrency: 1, + MaxDepth: test.maxDepth, + IncludeCosignArtifacts: test.includeCosignArtifacts, + }, + registry.Opts{ + EnvironFunc: os.Environ, + RetryCount: 3, + }, + ) + require.NoError(t, err) + + fmt.Printf("Result:\n\n") + printDescribedBundle("", bundleDescription) + fmt.Printf("----------------\n\n") + + require.Equal(t, topBundle.refDigest, bundleDescription.Image) + + assertBundleResult(t, topBundle, bundleDescription, test.maxDepth) }) } @@ -434,12 +530,25 @@ func printDescribedBundle(prefix string, bundle v1.Description) { } } -func assertBundleResult(t *testing.T, expectedBundle createdBundle, result v1.Description) { +func assertBundleResult(t *testing.T, expectedBundle createdBundle, result v1.Description, depth int) { for _, image := range expectedBundle.images { + if depth == 1 { + _, _, ok := findImageWithRef(result, image.refDigest) + assert.False(t, ok, fmt.Sprintf("expected to not find image %s in the bundle %s", image.refDigest, result.Image)) + for _, innerImage := range image.images { + _, _, ok := findImageWithRef(result, innerImage.refDigest) + assert.False(t, ok, fmt.Sprintf("expected to not find inner image %s in the bundle %s", innerImage.refDigest, result.Image)) + } + continue + } if len(image.images) > 0 { bundleDesc, imgInfo, ok := findImageWithRef(result, image.refDigest) if assert.True(t, ok, fmt.Sprintf("unable to find bundle %s in the bundle %s", image.refDigest, result.Image)) { - assertBundleResult(t, image.createdBundle, bundleDesc) + var newDepth = depth + if newDepth > 1 { + newDepth = depth - 1 + } + assertBundleResult(t, image.createdBundle, bundleDesc, newDepth) if len(image.annotations) > 0 { assert.Equal(t, image.annotations, imgInfo.Annotations) } else {