diff --git a/pkg/common/common.go b/pkg/common/common.go index ac196a4..0140646 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -3,6 +3,11 @@ package common +import ( + "fmt" + "math/rand" +) + const ( // InventoryLabel is the label stored on the ConfigMap // inventory object. The value of the label is a unique @@ -22,4 +27,14 @@ const ( OnRemoveAnnotation = "cli-utils.sigs.k8s.io/on-remove" // Resource lifecycle annotation value to prevent deletion. OnRemoveKeep = "keep" + // Maximum random number, non-inclusive, eight digits. + maxRandInt = 100000000 ) + +// RandomStr returns an eight-digit (with leading zeros) string of a +// random number seeded with the parameter. +func RandomStr(seed int64) string { + rand.Seed(seed) + randomInt := rand.Intn(maxRandInt) + return fmt.Sprintf("%08d", randomInt) +} diff --git a/pkg/config/initoptions.go b/pkg/config/initoptions.go index 0f85301..07aaa5a 100644 --- a/pkg/config/initoptions.go +++ b/pkg/config/initoptions.go @@ -5,7 +5,6 @@ package config import ( "fmt" - "math/rand" "os" "path/filepath" "regexp" @@ -16,12 +15,12 @@ import ( "github.com/google/uuid" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/cli-runtime/pkg/genericclioptions" + "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/kustomize/kyaml/kio" ) const ( manifestFilename = "inventory-template.yaml" - maxRandInt = 100000000 ) const configMapTemplate = `# NOTE: auto-generated. Some fields should NOT be modified. # Date: @@ -211,9 +210,7 @@ func fileExists(path string) bool { func (i *InitOptions) fillInValues() string { now := time.Now() nowStr := now.Format("2006-01-02 15:04:05 MST") - rand.Seed(time.Now().UTC().UnixNano()) - randomInt := rand.Intn(maxRandInt) - randomSuffix := fmt.Sprintf("%08d", randomInt) + randomSuffix := common.RandomStr(now.UTC().UnixNano()) manifestStr := configMapTemplate manifestStr = strings.ReplaceAll(manifestStr, "", nowStr) manifestStr = strings.ReplaceAll(manifestStr, "", i.Namespace) diff --git a/pkg/inventory/inventory-client.go b/pkg/inventory/inventory-client.go index c1b0784..c21864f 100644 --- a/pkg/inventory/inventory-client.go +++ b/pkg/inventory/inventory-client.go @@ -325,6 +325,12 @@ func (cic *ClusterInventoryClient) createInventoryObj(info *resource.Info) error if info == nil { return fmt.Errorf("attempting create a nil inventory object") } + // Default inventory name gets random suffix. Fixes problem where legacy + // inventory templates within same namespace will collide on name. + err := fixLegacyInventoryName(info) + if err != nil { + return err + } obj, err := object.InfoToObjMeta(info) if err != nil { return err diff --git a/pkg/inventory/inventory.go b/pkg/inventory/inventory.go index 10dc4c3..0e7d067 100644 --- a/pkg/inventory/inventory.go +++ b/pkg/inventory/inventory.go @@ -14,13 +14,18 @@ package inventory import ( "fmt" "strings" + "time" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/cli-runtime/pkg/resource" + "k8s.io/klog" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) +// The default inventory name stored in the inventory template. +const legacyInvName = "inventory" + // Inventory describes methods necessary for an object which // can persist the object metadata for pruning and other group // operations. @@ -148,3 +153,26 @@ func addSuffixToName(info *resource.Info, suffix string) error { return nil } + +// fixLegacyInventoryName modifies the inventory name if it is +// the legacy default name (i.e. inventory) by adding a random suffix. +// This fixes a problem where inventory object names collide if +// they are created in the same namespace. +func fixLegacyInventoryName(info *resource.Info) error { + if info == nil || info.Object == nil { + return fmt.Errorf("invalid inventory object is nil or info.Object is nil") + } + obj := info.Object + accessor, err := meta.Accessor(obj) + if err != nil { + return err + } + name := accessor.GetName() + if info.Name == legacyInvName || name == legacyInvName { + klog.V(4).Infof("renaming legacy inventory name") + seed := time.Now().UTC().UnixNano() + randomSuffix := common.RandomStr(seed) + return addSuffixToName(info, randomSuffix) + } + return nil +} diff --git a/pkg/inventory/inventory_test.go b/pkg/inventory/inventory_test.go index 6059d8b..ed33c03 100644 --- a/pkg/inventory/inventory_test.go +++ b/pkg/inventory/inventory_test.go @@ -5,6 +5,7 @@ package inventory import ( "fmt" + "regexp" "testing" "k8s.io/apimachinery/pkg/api/meta" @@ -37,6 +38,20 @@ var inventoryObj = unstructured.Unstructured{ }, } +var legacyInvObj = unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": legacyInvName, + "namespace": testNamespace, + "labels": map[string]interface{}{ + common.InventoryLabel: testInventoryLabel, + }, + }, + }, +} + var invInfo = &resource.Info{ Namespace: testNamespace, Name: inventoryObjName, @@ -455,6 +470,96 @@ func TestAddSuffixToName(t *testing.T) { } } +func TestLegacyInventoryName(t *testing.T) { + tests := map[string]struct { + info *resource.Info + invName string // Expected inventory name (if not modified) + isModified bool // Should inventory name be changed + isError bool // Should an error be thrown + }{ + "Nil info is an error": { + info: nil, + isModified: false, + isError: true, + }, + "Nil info.Object is an error": { + info: &resource.Info{ + Namespace: testNamespace, + Name: inventoryObjName, + Object: nil, + }, + invName: inventoryObjName, + isModified: false, + isError: true, + }, + "Info name differs from object name should return error": { + info: &resource.Info{ + Namespace: testNamespace, + Name: inventoryObjName, + Object: &legacyInvObj, + }, + invName: inventoryObjName, + isModified: false, + isError: true, + }, + "Legacy inventory name gets random suffix": { + info: &resource.Info{ + Namespace: testNamespace, + Name: legacyInvName, + Object: &legacyInvObj, + }, + invName: legacyInvName, + isModified: true, + isError: false, + }, + "Non-legacy inventory name does not get modified": { + info: &resource.Info{ + Namespace: testNamespace, + Name: inventoryObjName, + Object: &inventoryObj, + }, + invName: inventoryObjName, + isModified: false, + isError: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := fixLegacyInventoryName(tc.info) + if tc.isError { + if err == nil { + t.Fatalf("Should have produced an error, but returned none.") + } + return + } + if !tc.isError && err != nil { + t.Fatalf("Received error when expecting none (%s)\n", err) + } + actualName, err := getObjectName(tc.info.Object) + if err != nil { + t.Fatalf("Error getting object name: %s", err) + } + if actualName != tc.info.Name { + t.Errorf("Object name and info name differ: %s/%s", actualName, tc.info.Name) + } + if !tc.isModified { + if tc.invName != tc.info.Name { + t.Fatalf("expected non-modified name (%s), got (%s)", tc.invName, tc.info.Name) + } + return + } + matched, err := regexp.MatchString(`inventory-\d{8}`, actualName) + if err != nil { + t.Errorf("unexpected error parsing inventory name: %s", err) + } + if !matched { + t.Errorf("expected inventory name with random suffix, got (%s)", actualName) + } + }) + } +} + func getObjectName(obj runtime.Object) (string, error) { u, ok := obj.(*unstructured.Unstructured) if !ok {