Refactor:

- Remove stuttering of IncludeNonDistributableFlag.IncludeNonDistributableFlag
- Add test to assert building an image with IncludeNonDistributableFlag

Authored-by: Dennis Leon <[email protected]>

pkg/imgpkg/cmd/build.go

@@ -23,15 +23,15 @@ import (
 type BuildOptions struct {
 	ui ui.UI
 
-	ImageFlags                  ImageFlags
-	BundleFlags                 BundleFlags
-	FileFlags                   FileFlags
-	RegistryFlags               RegistryFlags
-	SignatureFlags              SignatureFlags
-	IncludeNonDistributableFlag IncludeNonDistributableFlag
-
-	TarDst      string
-	Concurrency int
+	ImageFlags     ImageFlags
+	BundleFlags    BundleFlags
+	FileFlags      FileFlags
+	RegistryFlags  RegistryFlags
+	SignatureFlags SignatureFlags
+
+	TarDst                  string
+	Concurrency             int
+	IncludeNonDistributable bool
 }
 
 // NewBuildOptions constructor to BuildOptions
@@ -57,10 +57,11 @@ func NewBuildCmd(o *BuildOptions) *cobra.Command {
 	o.FileFlags.Set(cmd)
 	o.RegistryFlags.Set(cmd)
 	o.SignatureFlags.Set(cmd)
-	o.IncludeNonDistributableFlag.Set(cmd)
 
 	cmd.Flags().StringVar(&o.TarDst, "to-tar", "", "Location to write a tar file containing assets")
 	cmd.Flags().IntVar(&o.Concurrency, "concurrency", 5, "Concurrency")
+	cmd.Flags().BoolVar(&o.IncludeNonDistributable, "include-non-distributable-layers", false,
+		"Include non-distributable layers when copying an image/bundle")
 	return cmd
 }
 
@@ -150,7 +151,7 @@ func (bo *BuildOptions) buildBundle(registry registry.Registry, signatureRetriev
 	})
 
 	tarImageSet := ctlimgset.NewTarImageSet(ctlimgset.NewImageSet(bo.Concurrency, prefixedLogger), bo.Concurrency, prefixedLogger)
-	includeNonDistributable := bo.IncludeNonDistributableFlag.IncludeNonDistributable
+	includeNonDistributable := bo.IncludeNonDistributable
 
 	_, err = tarImageSet.Export(rootBundleArtifactRefs, processedImages, bo.TarDst, registry, imagetar.NewImageLayerWriterCheck(includeNonDistributable))
 	if err != nil {
@@ -192,7 +193,7 @@ func (bo *BuildOptions) buildImage(registry registry.Registry, prefixedLogger ui
 		ImageIndex: nil,
 	})
 
-	isNonDistributable := bo.IncludeNonDistributableFlag.IncludeNonDistributable
+	isNonDistributable := bo.IncludeNonDistributable
 	tarImageSet := ctlimgset.NewTarImageSet(ctlimgset.NewImageSet(bo.Concurrency, prefixedLogger), bo.Concurrency, prefixedLogger)
 	_, err = tarImageSet.Export(ctlimgset.NewUnprocessedImageRefs(), processedImages, bo.TarDst, registry, imagetar.NewImageLayerWriterCheck(isNonDistributable))
 	if err != nil {

pkg/imgpkg/cmd/copy.go

@@ -24,18 +24,18 @@ const rootBundleLabelKey string = "dev.carvel.imgpkg.copy.root-bundle"
 type CopyOptions struct {
 	ui ui.UI
 
-	ImageFlags                  ImageFlags
-	BundleFlags                 BundleFlags
-	LockInputFlags              LockInputFlags
-	LockOutputFlags             LockOutputFlags
-	TarFlags                    TarFlags
-	RegistryFlags               RegistryFlags
-	SignatureFlags              SignatureFlags
-	IncludeNonDistributableFlag IncludeNonDistributableFlag
+	ImageFlags      ImageFlags
+	BundleFlags     BundleFlags
+	LockInputFlags  LockInputFlags
+	LockOutputFlags LockOutputFlags
+	TarFlags        TarFlags
+	RegistryFlags   RegistryFlags
+	SignatureFlags  SignatureFlags
 
 	RepoDst string
 
-	Concurrency int
+	Concurrency             int
+	IncludeNonDistributable bool
 }
 
 // NewCopyOptions constructor for building a CopyOptions, holding values derived via flags
@@ -66,10 +66,11 @@ func NewCopyCmd(o *CopyOptions) *cobra.Command {
 	o.TarFlags.Set(cmd)
 	o.RegistryFlags.Set(cmd)
 	o.SignatureFlags.Set(cmd)
-	o.IncludeNonDistributableFlag.Set(cmd)
 
 	cmd.Flags().StringVar(&o.RepoDst, "to-repo", "", "Location to upload assets")
 	cmd.Flags().IntVar(&o.Concurrency, "concurrency", 5, "Concurrency")
+	cmd.Flags().BoolVar(&o.IncludeNonDistributable, "include-non-distributable-layers", false,
+		"Include non-distributable layers when copying an image/bundle")
 	return cmd
 }
 
@@ -82,7 +83,7 @@ func (c *CopyOptions) Run() error {
 	}
 
 	registryOpts := c.RegistryFlags.AsRegistryOpts()
-	registryOpts.IncludeNonDistributableLayers = c.IncludeNonDistributableFlag.IncludeNonDistributable
+	registryOpts.IncludeNonDistributableLayers = c.IncludeNonDistributable
 
 	reg, err := registry.NewRegistry(registryOpts)
 	if err != nil {
@@ -108,7 +109,7 @@ func (c *CopyOptions) Run() error {
 		BundleFlags:             c.BundleFlags,
 		LockInputFlags:          c.LockInputFlags,
 		TarFlags:                c.TarFlags,
-		IncludeNonDistributable: c.IncludeNonDistributableFlag.IncludeNonDistributable,
+		IncludeNonDistributable: c.IncludeNonDistributable,
 		Concurrency:             c.Concurrency,
 
 		ui:                 levelLogger,

pkg/imgpkg/cmd/copy_repo_src_test.go

@@ -128,7 +128,7 @@ func TestToTarBundleContainingNonDistributableLayers(t *testing.T) {
 	fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
 	defer fakeRegistry.CleanUp()
 	randomImage := fakeRegistry.WithRandomImage("library/image_with_config")
-	randomImageWithNonDistributableLayer := fakeRegistry.
+	randomImageWithNonDistributableLayer, _ := fakeRegistry.
 		WithRandomImage("library/image_with_non_dist_layer").WithNonDistributableLayer()
 
 	fakeRegistry.WithBundleFromPath(bundleName, "test_assets/bundle_with_mult_images").

test/e2e/build_test.go

@@ -6,6 +6,7 @@ package e2e
 import (
 	"fmt"
 	"io/fs"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -14,6 +15,7 @@ import (
 
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
+	regv1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/k14s/imgpkg/pkg/imgpkg/imagedesc"
 	"github.com/k14s/imgpkg/pkg/imgpkg/imagetar"
@@ -96,6 +98,36 @@ func TestBuildBundle(t *testing.T) {
 		require.NoError(t, err)
 	})
 
+	t.Run("Bundle contains an image with a non-distributable layer", func(t *testing.T) {
+		env := helpers.BuildEnv(t)
+		imgpkg := helpers.Imgpkg{T: t, L: helpers.Logger{}, ImgpkgPath: env.ImgpkgPath}
+		defer env.Cleanup()
+
+		fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
+		defer fakeRegistry.CleanUp()
+
+		imageWithNonDistributableLayer, nonDistributableLayer := fakeRegistry.WithRandomImage("repo/image_belonging_to_image_index").WithNonDistributableLayer()
+
+		bundleDir := env.BundleFactory.CreateBundleDir(helpers.BundleYAML, helpers.ImagesYAML)
+		imagesLockYAML := fmt.Sprintf(`---
+apiVersion: imgpkg.carvel.dev/v1alpha1
+kind: ImagesLock
+images:
+- image: %s
+`, imageWithNonDistributableLayer.RefDigest)
+		env.BundleFactory.AddFileToBundle(filepath.Join(".imgpkg", "images.yml"), imagesLockYAML)
+
+		fakeRegistry.Build()
+
+		tempBundleTarDir := env.Assets.CreateTempFolder("bundle-tar")
+		tempBundleTarFile := filepath.Join(tempBundleTarDir, "bundle-tar.tgz")
+		imgpkg.Run([]string{"build", "--include-non-distributable-layers", "--tty", "-f", bundleDir, "-b", "repo/bundle", "--to-tar", tempBundleTarFile})
+		nonDistributableLayerDigest, err := nonDistributableLayer.Digest()
+		assert.NoError(t, err)
+
+		assertTarballContainsImageWithNDL(tempBundleTarFile, imageWithNonDistributableLayer.RefDigest, nonDistributableLayerDigest, t)
+	})
+
 	t.Run("Bundle with signed images are preserved", func(t *testing.T) {
 		logger := &helpers.Logger{}
 
@@ -267,7 +299,6 @@ images:
 	})
 }
 
-
 func TestBuildImage(t *testing.T) {
 	env := helpers.BuildEnv(t)
 	imgpkg := helpers.Imgpkg{T: t, L: helpers.Logger{}, ImgpkgPath: env.ImgpkgPath}
@@ -320,4 +351,26 @@ func assertTarballContainsImage(imageTarPath string, imageRef string, t *testing
 	assert.NotNil(t, imageReferencesFound)
 	assert.Len(t, imageReferencesFound, 1)
 	assert.Equal(t, imageRef, imageReferencesFound[0].Ref())
-}
+}
+
+func assertTarballContainsImageWithNDL(imageTarPath string, imageRef string, digest regv1.Hash, t *testing.T) {
+	tarReader := imagetar.NewTarReader(imageTarPath)
+	imageOrIndices, err := tarReader.Read()
+	assert.NoError(t, err)
+	var imageReferencesFound []imagedesc.ImageOrIndex
+	for _, imageOrIndex := range imageOrIndices {
+		if imageOrIndex.Ref() == imageRef {
+			imageReferencesFound = append(imageReferencesFound, imageOrIndex)
+		}
+	}
+
+	assert.NotNil(t, imageReferencesFound)
+	assert.Len(t, imageReferencesFound, 1)
+	assert.Equal(t, imageRef, imageReferencesFound[0].Ref())
+	layers, err := (*imageReferencesFound[0].Image).LayerByDigest(digest)
+	assert.NoError(t, err)
+	compressed, err := layers.Compressed()
+	assert.NoError(t, err)
+	_, err = ioutil.ReadAll(compressed)
+	assert.NoError(t, err)
+}

test/e2e/copy_from_tar_test.go

@@ -67,7 +67,7 @@ func TestCopyTarSrc(t *testing.T) {
 		fakeRegistry := helpers.NewFakeRegistry(t, &helpers.Logger{LogLevel: helpers.LogDebug})
 		defer fakeRegistry.CleanUp()
 
-		imageWithNonDistributableLayer := fakeRegistry.WithRandomImage("repo/image_belonging_to_image_index").WithNonDistributableLayer()
+		imageWithNonDistributableLayer, _ := fakeRegistry.WithRandomImage("repo/image_belonging_to_image_index").WithNonDistributableLayer()
 		nestedImageIndex := fakeRegistry.WithImageIndex("repo/nestedimageindex", imageWithNonDistributableLayer.Image)
 
 		randomImage := fakeRegistry.WithRandomImage("repo/randomimage")

test/helpers/fake_registry.go

@@ -323,13 +323,13 @@ func (r *FakeTestRegistryBuilder) WithNonDistributableLayerInImage(imageNames ..
 	}
 }
 
-func (r *ImageOrImageIndexWithTarPath) WithNonDistributableLayer() *ImageOrImageIndexWithTarPath {
+func (r *ImageOrImageIndexWithTarPath) WithNonDistributableLayer() (*ImageOrImageIndexWithTarPath, v1.Layer) {
 	layer, err := random.Layer(1024, types.OCIUncompressedRestrictedLayer)
 	require.NoError(r.t, err)
 
 	r.Image, err = mutate.AppendLayers(r.Image, layer)
 	require.NoError(r.t, err)
-	return r.fakeRegistry.updateState(r.RefDigest, r.Image, r.ImageIndex, r.path, "")
+	return r.fakeRegistry.updateState(r.RefDigest, r.Image, r.ImageIndex, r.path, ""), layer
 }
 
 func (r *FakeTestRegistryBuilder) CleanUp() {