Adds random suffix to legacy default inventory name

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)
+}

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: <DATETIME>
@@ -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())
+	randomSuffix := common.RandomStr(now.UTC().UnixNano())
-	randomInt := rand.Intn(maxRandInt)
-	randomSuffix := fmt.Sprintf("%08d", randomInt)
 	manifestStr := configMapTemplate
 	manifestStr = strings.ReplaceAll(manifestStr, "<DATETIME>", nowStr)
 	manifestStr = strings.ReplaceAll(manifestStr, "<NAMESPACE>", i.Namespace)

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

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
+}

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 {