From ec0d89e91d8d05e26aa9d937d17c2e29b1a7953d Mon Sep 17 00:00:00 2001
From: JeromeJu <jeromeju@google.com>
Date: Thu, 17 Aug 2023 01:34:08 +0000
Subject: [PATCH] Change condition for the behavior when --no-push=true without
 --destinations

This commit changes the condition check for the behavior when no-push is
set to true while destinations are needed. Prior this change, users would
have to set destinations even when noPush option is set to true. More
specifically, a workaround for tar files to be generated when --no-push is
true and destinations is empty is provided where a dummy destination would be
set.
---
 pkg/executor/push.go      | 29 ++++++++++++++----
 pkg/executor/push_test.go | 62 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/pkg/executor/push.go b/pkg/executor/push.go
index faf59ff94d..483bf2b2fd 100644
--- a/pkg/executor/push.go
+++ b/pkg/executor/push.go
@@ -53,11 +53,13 @@ type withUserAgent struct {
 
 // for testing
 var (
-	newRetry = transport.NewRetry
+	newRetry          = transport.NewRetry
+	DummyDestinations = []string{DummyDestination}
 )
 
 const (
 	UpstreamClientUaKey = "UPSTREAM_CLIENT_TYPE"
+	DummyDestination    = "docker.io/unset-repo/unset-image-name"
 )
 
 func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) {
@@ -143,11 +145,18 @@ func writeDigestFile(path string, digestByteArray []byte) error {
 	return ioutil.WriteFile(path, digestByteArray, 0644)
 }
 
-// DoPush is responsible for pushing image to the destinations specified in opts
+// DoPush is responsible for pushing image to the destinations specified in opts.
+// A dummy destination would be set when --no-push is set to true and --tar-path
+// is not empty with empty --destinations.
 func DoPush(image v1.Image, opts *config.KanikoOptions) error {
 	t := timing.Start("Total Push Time")
 	var digestByteArray []byte
 	var builder strings.Builder
+
+	if !opts.NoPush && len(opts.Destinations) == 0 {
+		return errors.New("must provide at least one destination to push")
+	}
+
 	if opts.DigestFile != "" || opts.ImageNameDigestFile != "" || opts.ImageNameTagDigestFile != "" {
 		var err error
 		digestByteArray, err = getDigest(image)
@@ -173,6 +182,12 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
 		}
 	}
 
+	if opts.NoPush && len(opts.Destinations) == 0 {
+		if opts.TarPath != "" {
+			setDummyDestinations(opts)
+		}
+	}
+
 	destRefs := []name.Tag{}
 	for _, destination := range opts.Destinations {
 		destRef, err := name.NewTag(destination, name.WeakValidation)
@@ -208,10 +223,6 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error {
 	if opts.TarPath != "" {
 		tagToImage := map[name.Tag]v1.Image{}
 
-		if len(destRefs) == 0 {
-			return errors.New("must provide at least one destination when tarPath is specified")
-		}
-
 		for _, destRef := range destRefs {
 			tagToImage[destRef] = image
 		}
@@ -362,3 +373,9 @@ func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, tarPath strin
 	}
 	return DoPush(empty, &cacheOpts)
 }
+
+// setDummyDestinations sets the dummy destinations required to generate new
+// tag names for tarPath in DoPush.
+func setDummyDestinations(opts *config.KanikoOptions) {
+	opts.Destinations = DummyDestinations
+}
diff --git a/pkg/executor/push_test.go b/pkg/executor/push_test.go
index 09dba837e3..84338e095e 100644
--- a/pkg/executor/push_test.go
+++ b/pkg/executor/push_test.go
@@ -222,6 +222,68 @@ func TestImageNameDigestFile(t *testing.T) {
 
 }
 
+func TestDoPushWithOpts(t *testing.T) {
+	tarPath := "image.tar"
+
+	for _, tc := range []struct {
+		name        string
+		opts        config.KanikoOptions
+		expectedErr bool
+	}{
+		{
+			name: "no push with tarPath without destinations",
+			opts: config.KanikoOptions{
+				NoPush:  true,
+				TarPath: tarPath,
+			},
+			expectedErr: false,
+		}, {
+			name: "no push with tarPath with destinations",
+			opts: config.KanikoOptions{
+				NoPush:       true,
+				TarPath:      tarPath,
+				Destinations: []string{"image"},
+			},
+			expectedErr: false,
+		}, {
+			name: "no push with tarPath with destinations empty",
+			opts: config.KanikoOptions{
+				NoPush:       true,
+				TarPath:      tarPath,
+				Destinations: []string{},
+			},
+			expectedErr: false,
+		}, {
+			name: "tarPath with destinations empty",
+			opts: config.KanikoOptions{
+				NoPush:       false,
+				TarPath:      tarPath,
+				Destinations: []string{},
+			},
+			expectedErr: true,
+		}} {
+		t.Run(tc.name, func(t *testing.T) {
+			image, err := random.Image(1024, 4)
+			if err != nil {
+				t.Fatalf("could not create image: %s", err)
+			}
+			defer os.Remove("image.tar")
+
+			err = DoPush(image, &tc.opts)
+			if err != nil {
+				if !tc.expectedErr {
+					t.Errorf("unexpected error with opts: could not push image: %s", err)
+				}
+			} else {
+				if tc.expectedErr {
+					t.Error("expected error with opts not found")
+				}
+			}
+
+		})
+	}
+}
+
 func TestImageNameTagDigestFile(t *testing.T) {
 	image, err := random.Image(1024, 4)
 	if err != nil {