Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add max-depth flag to describe #521

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Add max-depth flag to describe #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions pkg/imgpkg/bundle/bundle_images_lock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,26 @@ 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 {
return o.cachedImageRefs.AllImagesWithErrors()
}

// 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/imgpkg/bundle/bundle_images_lock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/imgpkg/bundle/bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions pkg/imgpkg/bundle/fetch_images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/imgpkg/cmd/copy_repo_src.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/imgpkg/cmd/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type DescribeOptions struct {
RegistryFlags RegistryFlags

Concurrency int
MaxDepth int
OutputType string
IncludeCosignArtifacts bool
}
Expand All @@ -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
Expand All @@ -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())
Expand Down
5 changes: 3 additions & 2 deletions pkg/imgpkg/v1/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type Description struct {
type DescribeOpts struct {
Logger bundle.Logger
Concurrency int
MaxDepth int
IncludeCosignArtifacts bool
}

Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we call FetchAllImagesRefs do you mind checking that the depth is >= 0? Just to ensure people do not try to get a negative number in

if err != nil {
return Description{}, fmt.Errorf("Retrieving Images from bundle: %s", err)
}
Expand Down Expand Up @@ -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()
Expand Down
115 changes: 112 additions & 3 deletions pkg/imgpkg/v1/describe_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type testDescribe struct {
description string
subject testBundle
includeCosignArtifacts bool
maxDepth int
}

func TestDescribeBundle(t *testing.T) {
Expand Down Expand Up @@ -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{
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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 {
Expand Down