From db01bbd2945a6e1505b13befff9312dc8f23acca Mon Sep 17 00:00:00 2001
From: Gangadhar Chalapaka <gangadhar@resolve.ai>
Date: Thu, 13 Aug 2026 03:37:17 -0700
Subject: [PATCH] Error when explicit destination of pkg get already contains a
 package

A repeated `kpt pkg get REPO_URI DEST` silently fetched the package
into a nested subdirectory (DEST/<pkg-name>): getDest always defaulted
to a repo-named subdirectory when the destination existed, even when
the user explicitly named the directory that the first run created.

Reject an explicitly named destination only when it already contains a
Kptfile, which is exactly the repeated-fetch case. Existing directories
without a Kptfile keep working as containers to fetch into
(DEST/<pkg-name>), the '.' special case is unchanged, and omitted
destinations keep the defaulting behavior, so no existing tests change.

Fixes #2656

Signed-off-by: Gangadhar Chalapaka <gangadhar@resolve.ai>
---
 pkg/lib/util/parse/parse.go      | 11 ++++
 pkg/lib/util/parse/parse_test.go | 87 ++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/pkg/lib/util/parse/parse.go b/pkg/lib/util/parse/parse.go
index 7ef16eeecd..f45a10229f 100644
--- a/pkg/lib/util/parse/parse.go
+++ b/pkg/lib/util/parse/parse.go
@@ -276,6 +276,17 @@ func getDest(v, repo, subdir string, explicitDest bool) (string, error) {
 		return v, nil
 	}
 
+	// The user explicitly named a destination that already holds a fetched
+	// package: error out instead of silently nesting another copy inside it,
+	// so a repeated `kpt pkg get REPO_URI DEST` fails the same way the
+	// defaulted destination does below. An existing directory without a
+	// Kptfile keeps working as a container to fetch the package into.
+	if explicitDest {
+		if _, err := os.Stat(filepath.Join(v, kptfilev1.KptFileName)); err == nil {
+			return "", errors.Errorf("destination directory %q already exists and contains a package", v)
+		}
+	}
+
 	// default the location to a new subdirectory matching the pkg URI base
 	repo = strings.TrimSuffix(repo, "/")
 	repo = strings.TrimSuffix(repo, ".git")
diff --git a/pkg/lib/util/parse/parse_test.go b/pkg/lib/util/parse/parse_test.go
index f360d206a8..b5657893ce 100644
--- a/pkg/lib/util/parse/parse_test.go
+++ b/pkg/lib/util/parse/parse_test.go
@@ -17,6 +17,8 @@ package parse
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
 	"testing"
 
 	kptfilev1 "github.com/kptdev/kpt/api/kptfile/v1"
@@ -264,3 +266,88 @@ func Test_GitParseArgs(t *testing.T) {
 		})
 	}
 }
+
+func Test_getDest(t *testing.T) {
+	repo := "https://github.com/kptdev/kpt"
+	subdir := "package-examples/nginx"
+
+	tests := map[string]struct {
+		dest         func(t *testing.T) string
+		explicitDest bool
+		want         func(dest string) string
+		errS         string
+	}{
+		"explicit destination that does not exist yet": {
+			dest: func(t *testing.T) string {
+				return filepath.Join(t.TempDir(), "newDirName")
+			},
+			explicitDest: true,
+			want:         func(dest string) string { return dest },
+		},
+		"explicit destination that already contains a package": {
+			dest: func(t *testing.T) string {
+				dir := t.TempDir()
+				require.NoError(t, os.WriteFile(filepath.Join(dir, "Kptfile"), []byte("apiVersion: kpt.dev/v1"), 0o600))
+				return dir
+			},
+			explicitDest: true,
+			errS:         "already exists and contains a package",
+		},
+		"explicit destination that exists without a package defaults to a subdirectory": {
+			dest: func(t *testing.T) string {
+				return t.TempDir()
+			},
+			explicitDest: true,
+			want:         func(dest string) string { return filepath.Join(dest, "nginx") },
+		},
+		"explicit current directory": {
+			dest:         func(t *testing.T) string { return "." },
+			explicitDest: true,
+			want:         func(string) string { return "." },
+		},
+		"defaulted destination inside an existing directory": {
+			dest: func(t *testing.T) string {
+				return t.TempDir()
+			},
+			explicitDest: false,
+			want:         func(dest string) string { return filepath.Join(dest, "nginx") },
+		},
+		"defaulted destination whose subdirectory already exists": {
+			dest: func(t *testing.T) string {
+				dir := t.TempDir()
+				require.NoError(t, os.Mkdir(filepath.Join(dir, "nginx"), 0o700))
+				return dir
+			},
+			explicitDest: false,
+			errS:         "already exists",
+		},
+		"destination parent does not exist": {
+			dest: func(t *testing.T) string {
+				return filepath.Join(t.TempDir(), "missing-parent", "dest")
+			},
+			explicitDest: true,
+			errS:         "does not exist",
+		},
+		"destination is a file": {
+			dest: func(t *testing.T) string {
+				f := filepath.Join(t.TempDir(), "file")
+				require.NoError(t, os.WriteFile(f, []byte("x"), 0o600))
+				return f
+			},
+			explicitDest: true,
+			errS:         "must be a directory",
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			dest := test.dest(t)
+			actual, err := getDest(dest, repo, subdir, test.explicitDest)
+			if test.errS != "" {
+				require.ErrorContains(t, err, test.errS)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, test.want(dest), actual)
+		})
+	}
+}