Replace random generated "inventory ID" by Jaisheesh-2006 · Pull Request #4396 · kptdev/kpt

Jaisheesh-2006 · 2026-02-17T12:50:40Z

Description

This PR replaces the generation of random UUIDs for inventory IDs with a deterministic SHA-1 hashing mechanism. By deriving the inventory ID from the package namespace and --name, we ensure that the same package configuration always maps to the same inventory object in the cluster.

Key Changes:

Deterministic ID Generation: Implemented generateHash(namespace, name) using length-prefixed SHA-1 to replace google/uuid.
Mandatory Flag: The --name flag is now mandatory for kpt live init to ensure the hash can be consistently generated.
Validation: Added DNS-1123 label validation for the --name input to ensure compatibility with Kubernetes resource naming conventions.
Legacy Support: Hidden the --inventory-id flag from the help menu to encourage deterministic usage while maintaining backward compatibility for existing workflows.
Robustness: Added guards in kpt live migrate to prevent operations on empty or malformed inventory names.

Motivation

Previously, re-fetching or re-initializing a package would generate a new random UUID. This led to "lost inventory" bugs where kpt could no longer track or prune resources previously applied to the cluster because the association (the ID) had changed. Deterministic hashing ensures that as long as the namespace and name remain constant, the inventory remains trackable across different environments and local clones.

Fixes

Fixes #4387

Replaces random UUIDs with SHA-1 hashes derived from namespace and name to prevent lost inventory bugs. Makes --name mandatory and adds DNS-1123 validation. Hides --inventory-id to favor deterministic generation. Fixes kptdev#4387 Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

netlify · 2026-02-17T12:50:45Z

✅ Deploy Preview for kptdocs ready!

Name	Link
🔨 Latest commit	`51f18a4`
🔍 Latest deploy log	https://app.netlify.com/projects/kptdocs/deploys/699c57a14693880008e75af3
😎 Deploy Preview	https://deploy-preview-4396--kptdocs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR addresses lost-inventory behavior by making kpt live init inventory IDs deterministic (derived from namespace + --name) instead of random UUID-based values, and updates related CLI guidance/tests/docs.

Changes:

Make --name mandatory for kpt live init and validate it against Kubernetes DNS-1123 naming rules.
Replace auto-generated inventory IDs with a deterministic, length-prefixed SHA-1 hash.
Update migrate/init flows, generated docs, and e2e coverage to reflect the new required --name behavior.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/lib/errors/resolver/live.go	Updates user-facing resolver messages to instruct using `kpt live init --name=...`.
internal/docs/generated/livedocs/docs.go	Updates generated CLI docs to reflect required `--name` and “advanced” inventory-id override semantics.
e2e/live/end-to-end-test.sh	Updates e2e invocations/assertions to pass `--name` and expect stable RG names.
commands/live/migrate/migratecmd.go	Adds guardrails in migration; however, introduces/retains error-handling issues in RG-file existence checks and malformed inventory handling.
commands/live/init/cmdliveinit_test.go	Reworks tests for name validation and deterministic hash expectations.
commands/live/init/cmdliveinit.go	Implements deterministic hash ID generation, mandatory `--name`, hides `--inventory-id`, and updates init behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T12:56:27Z

commands/live/migrate/migratecmd.go

+		if kf.Inventory.Name == "" {
+			return errors.E(op, types.UniquePath(dir),
+				fmt.Errorf("Kptfile inventory has empty name; re-run: kpt live init --name=<name>"))
+		}


This empty-name guard is likely unreachable: kptfileutil.ValidateInventory(kf.Inventory) earlier returns an error when Inventory.Name is empty, and the current code treats any validation error as “no inventory” and returns nil. To actually guard malformed inventories, handle the validation error explicitly (e.g., distinguish nil inventory vs missing fields) or move the check before the early return.

Copilot · 2026-02-17T12:56:27Z

commands/live/init/cmdliveinit.go

-	timeStr := strconv.FormatInt(t.UTC().UnixNano(), 10)
-	return fmt.Sprintf("%s-%s", hashStr, timeStr), nil
+	h := sha1.New()
+	fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name)


fmt.Fprintf's return values are ignored here. Since this repo enables the errcheck linter, this will fail CI. Capture and handle the returned error (or explicitly assign to _ and document why it can’t fail for a hash.Hash) so errcheck passes.

Suggested change

fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name)

if _, err := fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name); err != nil {

return "", fmt.Errorf("failed to write hash input: %w", err)

}

Copilot · 2026-02-17T12:56:28Z

commands/live/init/cmdliveinit.go

+	// Internal callers (e.g. migrate) may pass empty Name with an explicit
+	// InventoryID; derive a stable name from the package directory.
 	if c.Name == "" {
-		randomSuffix := common.RandomStr()
-		c.Name = fmt.Sprintf("%s-%s", defaultInventoryName, randomSuffix)
+		if c.InventoryID != "" {
+			c.Name = filepath.Base(c.Pkg.UniquePath.String())
+		} else {
+			return errors.E(op, c.Pkg.UniquePath, errNameRequired)
+		}


When Name is empty but InventoryID is provided, this derives the ResourceGroup name from the package directory base without validating it. Directory names can be invalid for Kubernetes metadata.name (uppercase, underscore, etc.), which would create an invalid ResourceGroup YAML and break later operations. Consider validating/sanitizing the derived name (or returning an error that requires an explicit --name in the internal caller).

Copilot · 2026-02-17T12:56:28Z

commands/live/migrate/migratecmd.go

@@ -390,6 +390,11 @@ func (mr *Runner) migrateKptfileToRG(args []string) error {
 			return errors.E(op, errors.IO, types.UniquePath(dir), err)


In the os.Stat error handling switch, the second case checks err (from earlier operations) instead of rgFileErr. This will silently ignore filesystem errors from os.Stat (e.g., permission errors) and can lead to migrating into an invalid state. Update the condition to use rgFileErr and return that error when it's not os.ErrNotExist.

Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

The quiet init test was changing the inventory name from inventory-18030002 back to rg-test-case-1a, causing the downstream 'status on symlink' assertion to fail. Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-17T13:27:03Z

e2e/live/end-to-end-test.sh

+    local count
+    count=$(wc -l < $OUTPUT_DIR/invname | tr -d ' ')
+    if [ "$count" -ge 1 ]; then


assertRGInventory claims to verify exactly one ResourceGroup inventory object, but the check currently passes when any count >= 1. This can hide regressions where multiple inventories exist in the same namespace. Update the condition to assert the count is exactly 1 (and ideally include the matching rows in the error output for debugging).

Copilot · 2026-02-17T13:27:03Z

commands/live/migrate/migratecmd.go

+		if kf.Inventory.Name == "" {
+			return errors.E(op, types.UniquePath(dir),
+				fmt.Errorf("kptfile inventory has empty name; re-run: kpt live init --name=<name>"))
+		}
+


This kf.Inventory.Name == "" guard is effectively unreachable because kptfileutil.ValidateInventory(kf.Inventory) above already rejects empty/whitespace names and returns early. If the intent is to surface a clearer error for malformed inventory, handle the specific ValidateInventory error (or move the check before the validation/early-return) so malformed inventory doesn't get silently treated as "migration not needed".

Suggested change

if kf.Inventory.Name == "" {

return errors.E(op, types.UniquePath(dir),

fmt.Errorf("kptfile inventory has empty name; re-run: kpt live init --name=<name>"))

}

Copilot · 2026-02-17T13:27:04Z

commands/live/init/cmdliveinit.go

+	if errs := validation.IsDNS1123Subdomain(trimmed); len(errs) > 0 {
+		return "", fmt.Errorf("--name %q is not a valid Kubernetes resource name: %s",
+			trimmed, strings.Join(errs, "; "))


PR description says --name is validated as a DNS-1123 label, but the implementation uses validation.IsDNS1123Subdomain (which permits dots and longer names). Either update the PR description/docs to say “DNS-1123 subdomain” (which matches Kubernetes metadata.name rules) or switch to label validation if dots should be rejected.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T13:04:36Z

commands/live/init/cmdliveinit.go

+	h := sha1.New()
+	fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name)
+	return hex.EncodeToString(h.Sum(nil)), nil


Using SHA-1 for non-cryptographic purposes (generating deterministic IDs) is acceptable. However, consider adding a code comment explicitly noting that SHA-1 is used here for deterministic hashing, not for security, to prevent future confusion and potential linter warnings from security scanners like gosec.

Copilot · 2026-02-20T13:04:36Z

e2e/live/end-to-end-test.sh

+# assertRGInventory checks that exactly one ResourceGroup inventory object
+# exists in the passed namespace (selected by the inventory-id label).


The comment says the function "checks that exactly one ResourceGroup inventory object exists," but the implementation uses ">= 1" which allows multiple ResourceGroup inventories. Consider either updating the comment to match the implementation ("at least one") or making the check stricter to truly verify exactly one inventory object exists.

- Replace IsDNS1123Subdomain with IsDNS1123Label for stricter name validation (63-char limit, no dots). - Capture fmt.Fprintf error in generateHash. - Validate directory-name fallback with IsDNS1123Label when --name is omitted by internal callers (e.g., migrate). - Fix wrong error variable (err to rgFileErr) in os.Stat switch in migratecmd.go. - Remove unreachable kf.Inventory.Name == '' guard in migratecmd.go. - Tighten assertRGInventory bash check from -ge 1 to -eq 1. - Remove dead generateID function and unused imports. - Add tests for directory-name fallback validation path. Fixes kptdev#4387 Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

- Add explanatory comment for SHA-1 usage to clarify it is not for cryptographic security. - Align assertRGInventory bash script comment with the strict -eq 1 implementation. Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T13:40:25Z

commands/live/init/cmdliveinit.go

+var errNameRequired = fmt.Errorf(
+	"--name is required: provide a stable deployment name " +
+		"(e.g. --name=my-app-staging) that remains consistent across re-initializations")


Consider using fmt.Errorf with formatting on a single line or breaking the string at natural word boundaries. The current concatenation with '+' operators makes the error message harder to read and modify. For example: fmt.Errorf(\"--name is required: provide a stable deployment name (e.g. --name=my-app-staging) that remains consistent across re-initializations\")

Copilot · 2026-02-23T13:40:26Z

commands/live/init/cmdliveinit.go

+	// Note: SHA-1 is used here strictly for deterministic ID generation,
+	// not for cryptographic security.


While the comment correctly clarifies that SHA-1 is not used for cryptographic purposes, it would be helpful to add a brief explanation of why SHA-1 was chosen over alternatives (e.g., simplicity, sufficient collision resistance for this use case, or compatibility with existing systems).

Suggested change

// Note: SHA-1 is used here strictly for deterministic ID generation,

// not for cryptographic security.

// Note: SHA-1 is used here strictly for deterministic ID generation, not for

// cryptographic security. It is chosen for its simplicity, stable 40-character

// hex output, and compatibility with existing inventory IDs; collision resistance

// is sufficient for this constrained input space (namespace + name), and no

// secrets or security-sensitive data are being protected.

Copilot AI review requested due to automatic review settings February 17, 2026 12:50

Jaisheesh-2006 requested review from efiacor, kispaljr, liamfallon and mozesl-nokia as code owners February 17, 2026 12:50

Copilot started reviewing on behalf of Jaisheesh-2006 February 17, 2026 12:51 View session

dosubot bot added the area/live label Feb 17, 2026

Copilot AI reviewed Feb 17, 2026

View reviewed changes

Jaisheesh-2006 added 2 commits February 17, 2026 18:34

fix: lowercase error string and fix e2e assertRGInventory name matching

0c363ec

Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

fix(e2e): use consistent inventory name in quiet init test

1e0c7f5

The quiet init test was changing the inventory name from inventory-18030002 back to rg-test-case-1a, causing the downstream 'status on symlink' assertion to fail. Signed-off-by: Jaisheesh-2006 <jaicodes2006@gmail.com>

Copilot AI review requested due to automatic review settings February 17, 2026 13:19

Copilot started reviewing on behalf of Jaisheesh-2006 February 17, 2026 13:20 View session

Copilot AI reviewed Feb 17, 2026

View reviewed changes

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Feb 20, 2026

Copilot AI review requested due to automatic review settings February 20, 2026 12:56

Jaisheesh-2006 force-pushed the fix/4387-loose-inventory-id branch from 07896b7 to 1e0c7f5 Compare February 20, 2026 12:56

Copilot started reviewing on behalf of Jaisheesh-2006 February 20, 2026 12:56 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

Jaisheesh-2006 added 2 commits February 20, 2026 19:14

Copilot AI review requested due to automatic review settings February 23, 2026 13:35

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Copilot started reviewing on behalf of Jaisheesh-2006 February 23, 2026 13:43 View session

efiacor changed the title ~~Fix #4387~~ Replace random generated "inventory ID" Feb 23, 2026

-	fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name)
+	if _, err := fmt.Fprintf(h, "%d:%s:%d:%s", len(namespace), namespace, len(name), name); err != nil {
+		return "", fmt.Errorf("failed to write hash input: %w", err)
+	}

		@@ -390,6 +390,11 @@ func (mr *Runner) migrateKptfileToRG(args []string) error {
		return errors.E(op, errors.IO, types.UniquePath(dir), err)

		# assertRGInventory checks that exactly one ResourceGroup inventory object
		# exists in the passed namespace (selected by the inventory-id label).

		// Note: SHA-1 is used here strictly for deterministic ID generation,
		// not for cryptographic security.

-	// Note: SHA-1 is used here strictly for deterministic ID generation,
-	// not for cryptographic security.
+	// Note: SHA-1 is used here strictly for deterministic ID generation, not for
+	// cryptographic security. It is chosen for its simplicity, stable 40-character
+	// hex output, and compatibility with existing inventory IDs; collision resistance
+	// is sufficient for this constrained input space (namespace + name), and no
+	// secrets or security-sensitive data are being protected.

Conversation

Jaisheesh-2006 commented Feb 17, 2026

Description

Key Changes:

Motivation

Fixes

Uh oh!

netlify bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kptdocs ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

netlify bot commented Feb 17, 2026 •

edited

Loading