diff --git a/commands/live/init/cmdliveinit.go b/commands/live/init/cmdliveinit.go index 8d3ff2c943..021e3570ae 100644 --- a/commands/live/init/cmdliveinit.go +++ b/commands/live/init/cmdliveinit.go @@ -17,13 +17,12 @@ package init import ( "context" "crypto/sha1" + "encoding/hex" goerrors "errors" "fmt" "os" "path/filepath" - "strconv" "strings" - "time" "github.com/kptdev/kpt/internal/docs/generated/livedocs" "github.com/kptdev/kpt/internal/pkg" @@ -36,15 +35,17 @@ import ( "github.com/kptdev/kpt/pkg/lib/errors" "github.com/kptdev/kpt/pkg/printer" "github.com/spf13/cobra" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/cli-runtime/pkg/genericclioptions" k8scmdutil "k8s.io/kubectl/pkg/cmd/util" - "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/config" "sigs.k8s.io/kustomize/kyaml/filesys" "sigs.k8s.io/kustomize/kyaml/yaml" ) -const defaultInventoryName = "inventory" +// errNameRequired is returned when --name is not provided or blank. +var errNameRequired = fmt.Errorf( + "--name is required: provide a stable deployment name (e.g. --name=my-app-staging) that remains consistent across re-initializations") // InvExistsError defines new error when the inventory // values have already been set on the Kptfile. @@ -90,11 +91,12 @@ func NewRunner(ctx context.Context, factory k8scmdutil.Factory, } r.Command = cmd - cmd.Flags().StringVar(&r.Name, "name", "", "Inventory object name") + cmd.Flags().StringVar(&r.Name, "name", "", "Stable deployment name for this package (required)") cmd.Flags().BoolVar(&r.Force, "force", false, "Set inventory values even if already set in Kptfile or ResourceGroup file") cmd.Flags().BoolVar(&r.Quiet, "quiet", false, "If true, do not print output message for initialization") - cmd.Flags().StringVar(&r.InventoryID, "inventory-id", "", "Inventory id for the package") - cmd.Flags().StringVar(&r.RGFileName, "rg-file", rgfilev1alpha1.RGFileName, "Name of the file holding the ResourceGroup resource.") + cmd.Flags().StringVar(&r.InventoryID, "inventory-id", "", "Override the auto-derived inventory ID (advanced)") + _ = cmd.Flags().MarkHidden("inventory-id") + cmd.Flags().StringVar(&r.RGFileName, "rg-file", rgfilev1alpha1.RGFileName, "Filename for the ResourceGroup CR") return r } @@ -126,6 +128,11 @@ func (r *Runner) preRunE(_ *cobra.Command, _ []string) error { func (r *Runner) runE(_ *cobra.Command, args []string) error { const op errors.Op = "cmdliveinit.runE" + name, err := validateName(r.Name) + if err != nil { + return errors.E(op, err) + } + r.Name = name if len(args) == 0 { // default to the current working directory cwd, err := os.Getwd() @@ -193,15 +200,25 @@ func (c *ConfigureInventoryInfo) Run(ctx context.Context) error { pr.Printf("initializing %q data (namespace: %s)...", c.RGFileName, namespace) } - // Autogenerate the name if it is not provided through the flag. + // 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 != "" { + dirName := filepath.Base(c.Pkg.UniquePath.String()) + if errs := validation.IsDNS1123Subdomain(dirName); len(errs) > 0 { + return errors.E(op, c.Pkg.UniquePath, + fmt.Errorf("directory name %q is not a valid Kubernetes resource name and --name was not provided: %s", + dirName, strings.Join(errs, "; "))) + } + c.Name = dirName + } else { + return errors.E(op, c.Pkg.UniquePath, errNameRequired) + } } - // Autogenerate the inventory ID if not provided through the flag. + // Derive inventory ID from namespace+name unless explicitly overridden. if c.InventoryID == "" { - c.InventoryID, err = generateID(namespace, c.Name, time.Now()) + c.InventoryID, err = generateHash(namespace, c.Name) if err != nil { return errors.E(op, c.Pkg.UniquePath, err) } @@ -262,9 +279,8 @@ func createRGFile(p *pkg.Pkg, inv *kptfilev1.Inventory, filename string, force b if rg != nil && !force { return errors.E(op, p.UniquePath, &InvInRGExistsError{}) } - // Initialize new resourcegroup object, as rg should have been nil. + // Initialize new ResourceGroup and populate inventory fields. rg = &rgfilev1alpha1.ResourceGroup{ResourceMeta: rgfilev1alpha1.DefaultMeta} - // // Finally, set the inventory parameters in the ResourceGroup object and write it. rg.Name = inv.Name rg.Namespace = inv.Namespace rg.Labels = map[string]string{rgfilev1alpha1.RGInventoryIDLabel: inv.InventoryID} @@ -302,33 +318,37 @@ func writeRGFile(dir string, rg *rgfilev1alpha1.ResourceGroup, filename string) return nil } -// generateID returns the string which is a SHA1 hash of the passed namespace -// and name, with the unix timestamp string concatenated. Returns an error -// if either the namespace or name are empty. -func generateID(namespace string, name string, t time.Time) (string, error) { - const op errors.Op = "cmdliveinit.generateID" - hashStr, err := generateHash(namespace, name) - if err != nil { - return "", errors.E(op, err) +// generateHash returns a deterministic 40-char hex inventory ID from namespace +// and name using SHA-1. Both fields are length-prefixed to prevent collisions +// (e.g. ns="ab", name="cd" vs ns="a", name="bcd"). +func generateHash(namespace, name string) (string, error) { + if namespace == "" || name == "" { + return "", fmt.Errorf("cannot generate inventory ID: namespace and name must be non-empty") + } + // 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. + h := sha1.New() + 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) } - timeStr := strconv.FormatInt(t.UTC().UnixNano(), 10) - return fmt.Sprintf("%s-%s", hashStr, timeStr), nil + return hex.EncodeToString(h.Sum(nil)), nil } -// generateHash returns the SHA1 hash of the concatenated "namespace:name" string, -// or an error if either namespace or name is empty. -func generateHash(namespace string, name string) (string, error) { - const op errors.Op = "cmdliveinit.generateHash" - if len(namespace) == 0 || len(name) == 0 { - return "", errors.E(op, - fmt.Errorf("can not generate hash with empty namespace or name")) +// validateName rejects empty, whitespace-only, and invalid RFC 1123 subdomain names. +// Returns the trimmed name on success. +func validateName(name string) (string, error) { + trimmed := strings.TrimSpace(name) + if trimmed == "" { + return "", errNameRequired } - str := fmt.Sprintf("%s:%s", namespace, name) - h := sha1.New() - if _, err := h.Write([]byte(str)); err != nil { - return "", errors.E(op, err) + 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, "; ")) } - return fmt.Sprintf("%x", (h.Sum(nil))), nil + return trimmed, nil } // kptfileInventoryEmpty returns true if the Inventory structure diff --git a/commands/live/init/cmdliveinit_test.go b/commands/live/init/cmdliveinit_test.go index 49963969f4..cd95583d9f 100644 --- a/commands/live/init/cmdliveinit_test.go +++ b/commands/live/init/cmdliveinit_test.go @@ -17,9 +17,8 @@ package init import ( "os" "path/filepath" - "regexp" + "strings" "testing" - "time" "github.com/kptdev/kpt/internal/pkg" "github.com/kptdev/kpt/internal/testutil" @@ -28,6 +27,7 @@ import ( "github.com/kptdev/kpt/pkg/kptfile/kptfileutil" "github.com/kptdev/kpt/pkg/printer/fake" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/cli-runtime/pkg/genericclioptions" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" "sigs.k8s.io/kustomize/kyaml/filesys" @@ -70,8 +70,6 @@ inventory: namespace: test-namespace inventoryID: ` + testInventoryID + "\n" -var testTime = time.Unix(5555555, 66666666) - var resourceGroupInventory = ` apiVersion: kpt.dev/v1alpha1 kind: ResourceGroup @@ -80,49 +78,111 @@ metadata: namespace: test-namespace ` -func TestCmd_generateID(t *testing.T) { +func TestValidateName(t *testing.T) { + testCases := map[string]struct { + name string + expectError bool + errContains string + }{ + "valid lowercase name": { + name: "my-app-staging", + }, + "dots are accepted in subdomains": { + name: "my.app.v1", + }, + "empty string is rejected": { + name: "", + expectError: true, + errContains: "--name is required", + }, + "whitespace-only is rejected": { + name: " ", + expectError: true, + errContains: "--name is required", + }, + "uppercase is rejected": { + name: "MyApp", + expectError: true, + errContains: "not a valid Kubernetes resource name", + }, + "underscore is rejected": { + name: "my_app", + expectError: true, + errContains: "not a valid Kubernetes resource name", + }, + "special chars are rejected": { + name: "my-app!", + expectError: true, + errContains: "not a valid Kubernetes resource name", + }, + "starts with dash is rejected": { + name: "-my-app", + expectError: true, + errContains: "not a valid Kubernetes resource name", + }, + "exceeds 253 chars is rejected": { + name: strings.Repeat("a", 254), + expectError: true, + errContains: "not a valid Kubernetes resource name", + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + result, err := validateName(tc.name) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + assert.Empty(t, result) + } else { + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(tc.name), result) + } + }) + } +} + +func TestCmd_generateHash(t *testing.T) { testCases := map[string]struct { namespace string name string - t time.Time expected string isError bool }{ "Empty inventory namespace is an error": { name: inventoryName, namespace: "", - t: testTime, isError: true, }, "Empty inventory name is an error": { name: "", namespace: inventoryNamespace, - t: testTime, isError: true, }, - "Namespace/name hash is valid": { + "Namespace/name hash is deterministic": { name: inventoryName, namespace: inventoryNamespace, - t: testTime, - expected: "fa6dc0d39b0465b90f101c2ad50d50e9b4022f23-5555555066666666", + expected: "b71156e872dad0b8efe1ce0303da20ef583453d6", + isError: false, + }, + "Same inputs produce same hash": { + name: inventoryName, + namespace: inventoryNamespace, + expected: "b71156e872dad0b8efe1ce0303da20ef583453d6", isError: false, }, } for tn, tc := range testCases { t.Run(tn, func(t *testing.T) { - actual, err := generateID(tc.namespace, tc.name, tc.t) - // Check if there should be an error + actual, err := generateHash(tc.namespace, tc.name) if tc.isError { - if err == nil { - t.Fatalf("expected error but received none") - } + require.Error(t, err) return } - assert.NoError(t, err) - if tc.expected != actual { - t.Errorf("expecting generated id (%s), got (%s)", tc.expected, actual) - } + require.NoError(t, err) + assert.Len(t, actual, 40, "SHA-1 hex must be 40 chars (valid label value)") + assert.Equal(t, tc.expected, actual) }) } } @@ -137,19 +197,53 @@ func TestCmd_Run(t *testing.T) { inventoryID string force bool expectedErrorMsg string - expectAutoGenID bool expectedInventory kptfilev1.Inventory }{ - "Fields are defaulted if not provided": { - kptfile: kptFile, - name: "", - rgfilename: "resourcegroup.yaml", - namespace: "testns", - inventoryID: "", - expectAutoGenID: true, + "Missing name is an error": { + kptfile: kptFile, + name: "", + rgfilename: "resourcegroup.yaml", + namespace: "testns", + inventoryID: "", + expectedErrorMsg: "--name is required", + }, + "Whitespace-only name is rejected": { + kptfile: kptFile, + name: " ", + rgfilename: "resourcegroup.yaml", + namespace: "testns", + inventoryID: "", + expectedErrorMsg: "--name is required", + }, + "Invalid DNS name is rejected": { + kptfile: kptFile, + name: "My_App!", + rgfilename: "resourcegroup.yaml", + namespace: "testns", + inventoryID: "", + expectedErrorMsg: "not a valid Kubernetes resource name", + }, + "Explicit inventory-id is preserved when both flags are set": { + kptfile: kptFile, + rgfilename: "resourcegroup.yaml", + name: "my-pkg", + namespace: "my-ns", + inventoryID: "custom-legacy-id-123", + expectedInventory: kptfilev1.Inventory{ + Namespace: "my-ns", + Name: "my-pkg", + InventoryID: "custom-legacy-id-123", + }, + }, + "Provided name derives deterministic inventory ID": { + kptfile: kptFile, + rgfilename: "resourcegroup.yaml", + name: "my-pkg", + namespace: "my-ns", + inventoryID: "", expectedInventory: kptfilev1.Inventory{ - Namespace: "testns", - Name: "inventory-*", + Namespace: "my-ns", + Name: "my-pkg", }, }, "Provided values are used": { @@ -313,42 +407,193 @@ func TestCmd_Run(t *testing.T) { } expectedInv := tc.expectedInventory - assertInventoryName(t, expectedInv.Name, actualInv.Name) + assert.Equal(t, expectedInv.Name, actualInv.Name) assert.Equal(t, expectedInv.Namespace, actualInv.Namespace) - if tc.expectAutoGenID { - assertGenInvID(t, actualInv.Name, actualInv.Namespace, actualInv.InventoryID) - } else { + if expectedInv.InventoryID != "" { assert.Equal(t, expectedInv.InventoryID, actualInv.InventoryID) + } else { + // Verify deterministic derivation: same name+namespace always yields same hash. + expectedHash, err := generateHash(actualInv.Namespace, actualInv.Name) + assert.NoError(t, err) + assert.Equal(t, expectedHash, actualInv.InventoryID) } }) } } -func assertInventoryName(t *testing.T, expected, actual string) bool { - re := regexp.MustCompile(`^inventory-[0-9]+$`) - if expected == "inventory-*" { - if re.MatchString(actual) { - return true - } - t.Errorf("expected value on the format 'inventory-[0-9]+', but found %q", actual) +func TestGenerateHash_DifferentInputs(t *testing.T) { + testCases := []struct { + desc string + ns string + name string + expected string + }{ + {"short pair ab:cd", "ab", "cd", "6d6a43180d720d2526a9c90829cde33f9b36dbdb"}, + {"my-ns:my-pkg", "my-ns", "my-pkg", "6ebf2b6944e9fc957759dd2405ff3879d06197f7"}, + {"ns-a:name-a", "ns-a", "name-a", "01a2429bd398b1d880a145b9f6a40c091119ca7a"}, + {"ns-a:name-b", "ns-a", "name-b", "a7a0df7b43e5aafeb3161a480aa7e68fdc8f3201"}, + {"ns-b:name-a", "ns-b", "name-a", "5ba3e66f5bcd5729b91904f0a7fcc78141644db6"}, } - return assert.Equal(t, expected, actual) + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + got, err := generateHash(tc.ns, tc.name) + require.NoError(t, err) + assert.Len(t, got, 40) + assert.Equal(t, tc.expected, got) + }) + } + + // Changing either namespace or name must change the hash. + h1, _ := generateHash("ns-a", "name-a") + h2, _ := generateHash("ns-a", "name-b") + h3, _ := generateHash("ns-b", "name-a") + assert.NotEqual(t, h1, h2, "different name should produce different hash") + assert.NotEqual(t, h1, h3, "different namespace should produce different hash") + assert.NotEqual(t, h2, h3, "both differ should produce different hash") +} + +func TestGenerateHash_NoSeparatorAmbiguity(t *testing.T) { + // These inputs would collide without length-prefixed encoding: + // "a" + "bcd" vs "abc" + "d" + // With the format "%d:%s:%d:%s": + // "1:a:3:bcd" vs "3:abc:1:d" + h1, err := generateHash("a", "bcd") + require.NoError(t, err) + h2, err := generateHash("abc", "d") + require.NoError(t, err) + assert.NotEqual(t, h1, h2, "length-prefixed encoding must prevent separator ambiguity") + + // Also verify the exact expected values. + assert.Equal(t, "a1724ac2a61ec038d055881eb4403c74ab4256e9", h1) + assert.Equal(t, "f99cca29ebcfd3bca8c3605d253e4fec27b917ae", h2) } -func assertGenInvID(t *testing.T, name, namespace, actual string) bool { - re := regexp.MustCompile(`^([a-z0-9]+)-[0-9]+$`) - match := re.FindStringSubmatch(actual) - if len(match) != 2 { - t.Errorf("unexpected format for autogenerated inventoryID") - return false +func TestConfigureInventoryInfo_InvalidDirectoryNameFallback(t *testing.T) { + // Internal callers (e.g. migrate) pass Name="" with InventoryID set. + // ConfigureInventoryInfo.Run() derives the name from the package directory. + // If the directory name fails IsDNS1123Subdomain, Run must return an error. + tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") + defer tf.Cleanup() + + parentDir, err := os.MkdirTemp("", "kpt-init-test-*") + require.NoError(t, err) + defer os.RemoveAll(parentDir) + + invalidDir := filepath.Join(parentDir, "UPPER_invalid") + require.NoError(t, os.MkdirAll(invalidDir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(invalidDir, kptfilev1.KptFileName), + []byte(kptFile), 0600)) + + p, err := pkg.New(filesys.FileSystemOrOnDisk{}, invalidDir) + require.NoError(t, err) + + c := &ConfigureInventoryInfo{ + Pkg: p, + Factory: tf, + Quiet: true, + Name: "", + InventoryID: "explicit-inv-id-123", + RGFileName: "resourcegroup.yaml", } - prefix, err := generateHash(namespace, name) - if err != nil { - panic(err) + err = c.Run(fake.CtxWithDefaultPrinter()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid Kubernetes resource name") + assert.Contains(t, err.Error(), "--name was not provided") +} + +func TestConfigureInventoryInfo_ValidDirectoryNameFallback(t *testing.T) { + // Happy path for internal callers: Name="" with InventoryID set, + // and the directory name IS a valid DNS-1123 subdomain. + tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") + defer tf.Cleanup() + + parentDir, err := os.MkdirTemp("", "kpt-init-test-*") + require.NoError(t, err) + defer os.RemoveAll(parentDir) + + validDir := filepath.Join(parentDir, "my-valid-pkg") + require.NoError(t, os.MkdirAll(validDir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(validDir, kptfilev1.KptFileName), + []byte(kptFile), 0600)) + + p, err := pkg.New(filesys.FileSystemOrOnDisk{}, validDir) + require.NoError(t, err) + + c := &ConfigureInventoryInfo{ + Pkg: p, + Factory: tf, + Quiet: true, + Name: "", + InventoryID: "explicit-inv-id-456", + RGFileName: "resourcegroup.yaml", } - if got, want := match[1], prefix; got != want { - t.Errorf("expected prefix %q, but found %q", want, got) - return false + err = c.Run(fake.CtxWithDefaultPrinter()) + + require.NoError(t, err) + + rg, err := pkg.ReadRGFile(validDir, "resourcegroup.yaml") + require.NoError(t, err) + require.NotNil(t, rg) + assert.Equal(t, "my-valid-pkg", rg.Name) + assert.Equal(t, "explicit-inv-id-456", rg.Labels[rgfilev1alpha1.RGInventoryIDLabel]) +} + +func TestConfigureInventoryInfo_DottedDirectoryNameFallback(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") + defer tf.Cleanup() + + parentDir, err := os.MkdirTemp("", "kpt-init-test-*") + require.NoError(t, err) + defer os.RemoveAll(parentDir) + + validDir := filepath.Join(parentDir, "my.valid.pkg") + require.NoError(t, os.MkdirAll(validDir, 0700)) + require.NoError(t, os.WriteFile(filepath.Join(validDir, kptfilev1.KptFileName), + []byte(kptFile), 0600)) + + p, err := pkg.New(filesys.FileSystemOrOnDisk{}, validDir) + require.NoError(t, err) + + c := &ConfigureInventoryInfo{ + Pkg: p, + Factory: tf, + Quiet: true, + Name: "", + InventoryID: "explicit-inv-id-789", + RGFileName: "resourcegroup.yaml", } - return true + err = c.Run(fake.CtxWithDefaultPrinter()) + + require.NoError(t, err) + + rg, err := pkg.ReadRGFile(validDir, "resourcegroup.yaml") + require.NoError(t, err) + require.NotNil(t, rg) + assert.Equal(t, "my.valid.pkg", rg.Name) + assert.Equal(t, "explicit-inv-id-789", rg.Labels[rgfilev1alpha1.RGInventoryIDLabel]) +} + +func TestCmd_MissingNameFlagReturnsError(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") + defer tf.Cleanup() + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() //nolint:dogsled + + w, clean := testutil.SetupWorkspace(t) + defer clean() + err := os.WriteFile(filepath.Join(w.WorkspaceDirectory, "Kptfile"), + []byte(kptFile), 0600) + require.NoError(t, err) + + revert := testutil.Chdir(t, w.WorkspaceDirectory) + defer revert() + + runner := NewRunner(fake.CtxWithDefaultPrinter(), tf, ioStreams) + runner.RGFileName = "resourcegroup.yaml" + runner.Command.SetArgs([]string{}) + + err = runner.Command.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "--name is required") } diff --git a/commands/live/migrate/migratecmd.go b/commands/live/migrate/migratecmd.go index 616e228898..f9eea6fb9c 100644 --- a/commands/live/migrate/migratecmd.go +++ b/commands/live/migrate/migratecmd.go @@ -377,8 +377,11 @@ func (mr *Runner) migrateKptfileToRG(args []string) error { } if _, err := kptfileutil.ValidateInventory(kf.Inventory); err != nil { - // Kptfile does not contain inventory: migration is not needed. - return nil + if kf.Inventory == nil { + return nil + } + return errors.E(op, types.UniquePath(dir), + fmt.Errorf("inventory in Kptfile is incomplete and cannot be migrated: %w", err)) } // Make sure resourcegroup file does not exist. @@ -386,8 +389,8 @@ func (mr *Runner) migrateKptfileToRG(args []string) error { switch { case rgFileErr == nil: return errors.E(op, errors.IO, types.UniquePath(dir), "the resourcegroup file already exists and inventory information cannot be migrated") - case err != nil && !goerrors.Is(err, os.ErrNotExist): - return errors.E(op, errors.IO, types.UniquePath(dir), err) + case rgFileErr != nil && !goerrors.Is(rgFileErr, os.ErrNotExist): + return errors.E(op, errors.IO, types.UniquePath(dir), rgFileErr) } err = (&initialization.ConfigureInventoryInfo{ diff --git a/e2e/live/end-to-end-test.sh b/e2e/live/end-to-end-test.sh index c939a8b887..87bf967289 100755 --- a/e2e/live/end-to-end-test.sh +++ b/e2e/live/end-to-end-test.sh @@ -473,25 +473,26 @@ function assertCMInventory { fi } -# assertRGInventory checks that a ResourceGroup inventory object exists -# in the passed namespace. Assumes the inventory object name begins -# with "inventory-". +# assertRGInventory checks that exactly one ResourceGroup inventory object +# exists in the passed namespace (selected by the inventory-id label). function assertRGInventory { local ns=$1 - echo "kubectl get resourcegroups.kpt.dev -n $ns --selector='cli-utils.sigs.k8s.io/inventory-id' --no-headers | awk '{print $1}'" - kubectl get resourcegroups.kpt.dev -n $ns --selector='cli-utils.sigs.k8s.io/inventory-id' --no-headers | awk '{print $1}' > $OUTPUT_DIR/invname + echo "kubectl get resourcegroups.kpt.dev -n $ns --selector='cli-utils.sigs.k8s.io/inventory-id' --no-headers" + kubectl get resourcegroups.kpt.dev -n $ns --selector='cli-utils.sigs.k8s.io/inventory-id' --no-headers > $OUTPUT_DIR/invname - test 1 == $(grep "inventory-" $OUTPUT_DIR/invname | wc -l); - if [ $? == 0 ]; then + local count + count=$(wc -l < $OUTPUT_DIR/invname | tr -d ' ') + if [ "$count" -eq 1 ]; then echo -n '.' else echo -n 'E' if [ ! -f $OUTPUT_DIR/errors ]; then touch $OUTPUT_DIR/errors fi - echo "error: expected missing ResourceGroup inventory in ${ns} namespace" >> $OUTPUT_DIR/errors - HAS_TEST_FAILURE=1 + echo "error: expected exactly 1 ResourceGroup inventory in ${ns} namespace, found ${count}:" >> $OUTPUT_DIR/errors + cat $OUTPUT_DIR/invname >> $OUTPUT_DIR/errors + HAS_TEST_FAILURE=1 fi } @@ -617,8 +618,8 @@ waitForDefaultServiceAccount # Test: Apply dry-run without ResourceGroup CRD installation fails echo "[ResourceGroup] Testing initial apply dry-run without ResourceGroup inventory CRD" cp -f e2e/live/testdata/Kptfile e2e/live/testdata/rg-test-case-1a/ -echo "kpt live init --quiet e2e/live/testdata/rg-test-case-1a" -${BIN_DIR}/kpt live init --quiet e2e/live/testdata/rg-test-case-1a +echo "kpt live init --quiet --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a" +${BIN_DIR}/kpt live init --quiet --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a echo "kpt live apply --dry-run e2e/live/testdata/rg-test-case-1a" ${BIN_DIR}/kpt live apply --dry-run e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status assertContains "Error: The ResourceGroup CRD was not found in the cluster. Please install it either by using the '--install-resource-group' flag or the 'kpt live install-resource-group' command." @@ -731,9 +732,9 @@ printResult # Test: Basic Kptfile/ResourceGroup inititalizing inventory info echo "Testing init for Kptfile/ResourceGroup" cp -f e2e/live/testdata/Kptfile e2e/live/testdata/rg-test-case-1a/ -echo "kpt live init e2e/live/testdata/rg-test-case-1a" +echo "kpt live init --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a" cp -f e2e/live/testdata/Kptfile e2e/live/testdata/rg-test-case-1a -${BIN_DIR}/kpt live init e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status +${BIN_DIR}/kpt live init --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status assertContains "initializing \"resourcegroup.yaml\" data (namespace: rg-test-namespace)...success" # Difference in Kptfile should have inventory data diff e2e/live/testdata/Kptfile e2e/live/testdata/rg-test-case-1a/Kptfile 2>&1 | tee $OUTPUT_DIR/status @@ -745,12 +746,12 @@ assertNotContains "inventoryID:" cat e2e/live/testdata/rg-test-case-1a/resourcegroup.yaml 2>&1 | tee $OUTPUT_DIR/status assertContains "kind: ResourceGroup" assertContains "namespace: rg-test-namespace" -assertContains "name: inventory-" +assertContains "name: rg-test-case-1a" printResult echo "Testing init Kptfile/ResourceGroup already initialized" -echo "kpt live init e2e/live/testdata/rg-test-case-1a" -${BIN_DIR}/kpt live init e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status +echo "kpt live init --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a" +${BIN_DIR}/kpt live init --name=rg-test-case-1a e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status assertContains "initializing \"resourcegroup.yaml\" data (namespace: rg-test-namespace)...failed" assertContains "Error: Inventory information has already been added to the package ResourceGroup object." printResult @@ -764,8 +765,8 @@ assertContains "name: inventory-18030002" printResult echo "Testing init quiet Kptfile/ResourceGroup" -echo "kpt live init --quiet e2e/live/testdata/rg-test-case-1a" -${BIN_DIR}/kpt live init --quiet e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status +echo "kpt live init --quiet --force --name=inventory-18030002 e2e/live/testdata/rg-test-case-1a" +${BIN_DIR}/kpt live init --quiet --force --name=inventory-18030002 e2e/live/testdata/rg-test-case-1a 2>&1 | tee $OUTPUT_DIR/status assertNotContains "initializing resourcegroup" printResult @@ -1113,7 +1114,7 @@ printResult echo "[ResourceGroup] Testing continue-on-error" echo "kpt live apply e2e/live/testdata/continue-on-error" cp -f e2e/live/testdata/Kptfile e2e/live/testdata/continue-on-error -${BIN_DIR}/kpt live init e2e/live/testdata/continue-on-error 2>&1 | tee $OUTPUT_DIR/status +${BIN_DIR}/kpt live init --name=continue-on-error e2e/live/testdata/continue-on-error 2>&1 | tee $OUTPUT_DIR/status diff e2e/live/testdata/Kptfile e2e/live/testdata/continue-on-error/resourcegroup.yaml 2>&1 | tee $OUTPUT_DIR/status assertContains "namespace: continue-err-namespace" printResult @@ -1214,12 +1215,12 @@ waitForDefaultServiceAccount # Setup: kpt live init with custom resourcegroup file # Applies resources in "test-case-1c" directory echo "Testing kpt live init with custom ResourceGroup file" -echo "kpt live init --rg-file=custom-rg.yaml e2e/live/testdata/test-case-1c" -${BIN_DIR}/kpt live init --rg-file=custom-rg.yaml e2e/live/testdata/test-case-1c 2>&1 | tee $OUTPUT_DIR/status +echo "kpt live init --rg-file=custom-rg.yaml --name=test-case-1c e2e/live/testdata/test-case-1c" +${BIN_DIR}/kpt live init --rg-file=custom-rg.yaml --name=test-case-1c e2e/live/testdata/test-case-1c 2>&1 | tee $OUTPUT_DIR/status assertContains "initializing \"custom-rg.yaml\" data (namespace: test-namespace)...success" printResult # Re-running live init should fail as ResourceGroup file already exists -${BIN_DIR}/kpt live init --rg-file=custom-rg.yaml e2e/live/testdata/test-case-1c 2>&1 | tee $OUTPUT_DIR/status +${BIN_DIR}/kpt live init --rg-file=custom-rg.yaml --name=test-case-1c e2e/live/testdata/test-case-1c 2>&1 | tee $OUTPUT_DIR/status assertContains "initializing \"custom-rg.yaml\" data (namespace: test-namespace)...failed" printResult diff --git a/e2e/testdata/live-apply/crd-and-cr/resources/cr.yaml b/e2e/testdata/live-apply/crd-and-cr/resources/cr.yaml index 4469a1e516..31f229523d 100644 --- a/e2e/testdata/live-apply/crd-and-cr/resources/cr.yaml +++ b/e2e/testdata/live-apply/crd-and-cr/resources/cr.yaml @@ -16,3 +16,5 @@ apiVersion: kpt.dev/v1 kind: Custom metadata: name: cr + annotations: + config.kubernetes.io/depends-on: apiextensions.k8s.io/CustomResourceDefinition/customs.kpt.dev diff --git a/internal/docs/generated/livedocs/docs.go b/internal/docs/generated/livedocs/docs.go index 81eed54988..62700b449e 100644 --- a/internal/docs/generated/livedocs/docs.go +++ b/internal/docs/generated/livedocs/docs.go @@ -171,13 +171,15 @@ Flags: Defaults to false. --inventory-id: - Inventory identifier for the package. This is used to detect overlap between - packages that might use the same name and namespace for the inventory object. - Defaults to an auto-generated value. + (Advanced) Override the auto-derived inventory ID. Only needed to adopt a + legacy ResourceGroup with a non-standard ID. Normally this is automatically + derived from --name and the namespace. --name: - The name for the ResourceGroup resource that contains the inventory - for the package. Defaults to the name of the package. + A stable deployment name for this package instance (required). This + identifies the set of resources managed on the cluster and must remain + consistent across re-initializations. For example, if you delete and + re-fetch the package, use the same --name to preserve resource ownership. --namespace: The namespace for the ResourceGroup resource that contains the inventory @@ -191,10 +193,10 @@ Flags: ` var InitExamples = ` # initialize a package in the current directory. - $ kpt live init + $ kpt live init --name=my-app # initialize a package with explicit namespace for the ResourceGroup. - $ kpt live init --namespace=test my-dir + $ kpt live init --name=my-app --namespace=test my-dir ` var InstallResourceGroupShort = `Install the ResourceGroup CRD in the cluster.` diff --git a/pkg/lib/errors/resolver/live.go b/pkg/lib/errors/resolver/live.go index 41dcd12b85..f63c3d932c 100644 --- a/pkg/lib/errors/resolver/live.go +++ b/pkg/lib/errors/resolver/live.go @@ -35,7 +35,7 @@ func init() { const ( noInventoryObjErrorMsg = ` -Error: Package uninitialized. Please run "kpt live init" command. +Error: Package uninitialized. Please run "kpt live init --name=DEPLOYMENT_NAME" command. The package needs to be initialized to generate the template which will store state for resource sets. This state is diff --git a/pkg/lib/errors/resolver/pkg.go b/pkg/lib/errors/resolver/pkg.go index 6f2ab4ca9b..be7e569f95 100644 --- a/pkg/lib/errors/resolver/pkg.go +++ b/pkg/lib/errors/resolver/pkg.go @@ -82,7 +82,7 @@ func resolveNestedErr(err error, path string) (ResolvedResult, bool) { if errors.As(err, &deprecatedv1alpha2KptfileError) && deprecatedv1alpha2KptfileError.Version == "v1alpha2" { msg := fmt.Sprintf("Error: Kptfile at %q has an old version (%q) of the Kptfile schema.\n", path, deprecatedv1alpha2KptfileError.Version) - msg += "Please run \"kpt fn eval -i ghcr.io/kptdev/krm-functions-catalog/fix:latest --include-meta-resources\" to upgrade the package and retry." + msg += fmt.Sprintf("Please run \"kpt fn eval %q -i ghcr.io/kptdev/krm-functions-catalog/fix:latest --include-meta-resources\" to upgrade the package and retry.", path) return ResolvedResult{ Message: msg, diff --git a/pkg/lib/errors/resolver/update.go b/pkg/lib/errors/resolver/update.go index a94c8f3e96..8f3046014d 100644 --- a/pkg/lib/errors/resolver/update.go +++ b/pkg/lib/errors/resolver/update.go @@ -37,13 +37,13 @@ func (*updateErrorResolver) Resolve(err error) (ResolvedResult, bool) { if errors.As(err, &pkgNotGitRepoError) { //nolint:lll msg = fmt.Sprintf("Package %q is not within a git repository.", pkgNotGitRepoError.Path) - msg += " Please initialize a repository using 'git init' and then commit the changes using 'git commit -m \"\"'." + msg += " Please initialize a repository using 'git init' and then commit the changes using 'git commit -m \"\"'." } var pkgRepoDirtyError *update.PkgRepoDirtyError if errors.As(err, &pkgRepoDirtyError) { msg = fmt.Sprintf("Package %q contains uncommitted changes.", pkgRepoDirtyError.Path) - msg += " Please commit the changes using 'git commit -m \"\"'." + msg += " Please commit the changes using 'git commit -m \"\"'." } if msg != "" {