Use NewUniqueName (#2137)

* Use NewUniqueName * Change comments * Fix test * Add to CHANGELOG
pulumi · Aug 16, 2022 · 3c0d2cd · 3c0d2cd
1 parent fd27374
commit 3c0d2cd
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## Unreleased
 
+- Update autonaming to use NewUniqueName for deterministic update plans. (https://github.com/pulumi/pulumi-kubernetes/pull/2137)
+
 ## 3.20.4 (August 15, 2022)
 
 - Fix Helm charts being ignored by policy packs. (https://github.com/pulumi/pulumi-kubernetes/pull/2133)
@@ -138,7 +140,7 @@ Note: The `kubernetes:storage.k8s.io/v1alpha1:CSIStorageCapacity` API was remove
 
 - Helm Release: Helm Release imports support (https://github.com/pulumi/pulumi-kubernetes/pull/1818)
 - Helm Release: fix username fetch option (https://github.com/pulumi/pulumi-kubernetes/pull/1824)
-- Helm Release: Use URN name as base for autonaming, Drop warning, fix default value for 
+- Helm Release: Use URN name as base for autonaming, Drop warning, fix default value for
   keyring (https://github.com/pulumi/pulumi-kubernetes/pull/1826)
 - Helm Release: Add support for loading values from yaml files (https://github.com/pulumi/pulumi-kubernetes/pull/1828)
 

diff --git a/provider/pkg/metadata/naming.go b/provider/pkg/metadata/naming.go
@@ -27,7 +27,7 @@ var dns1123Alphabet = []rune("abcdefghijklmnopqrstuvwxyz0123456789")
 
 // AssignNameIfAutonamable generates a name for an object. Uses DNS-1123-compliant characters.
 // All auto-named resources get the annotation `pulumi.com/autonamed` for tooling purposes.
-func AssignNameIfAutonamable(obj *unstructured.Unstructured, propMap resource.PropertyMap, urn resource.URN) {
+func AssignNameIfAutonamable(randomSeed []byte, obj *unstructured.Unstructured, propMap resource.PropertyMap, urn resource.URN) {
 	contract.Assert(urn.Name().String() != "")
 	// Check if the .metadata.name is set and is a computed value. If so, do not auto-name.
 	if md, ok := propMap["metadata"].V.(resource.PropertyMap); ok {
@@ -38,7 +38,7 @@ func AssignNameIfAutonamable(obj *unstructured.Unstructured, propMap resource.Pr
 
 	if obj.GetName() == "" {
 		prefix := urn.Name().String() + "-"
-		autoname, err := resource.NewUniqueHex(prefix, 0, 0)
+		autoname, err := resource.NewUniqueName(randomSeed, prefix, 0, 0, nil)
 		contract.AssertNoError(err)
 		obj.SetName(autoname)
 		SetAnnotationTrue(obj, AnnotationAutonamed)

diff --git a/provider/pkg/metadata/naming_test.go b/provider/pkg/metadata/naming_test.go
@@ -30,7 +30,7 @@ func TestAssignNameIfAutonamable(t *testing.T) {
 	// o1 has no name, so autonaming succeeds.
 	o1 := &unstructured.Unstructured{}
 	pm1 := resource.NewPropertyMap(struct{}{})
-	AssignNameIfAutonamable(o1, pm1, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
+	AssignNameIfAutonamable(nil, o1, pm1, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
 		tokens.Type(""), tokens.Type("bang:boom/fizzle:MajorResource"), "foo"))
 	assert.True(t, IsAutonamed(o1))
 	assert.True(t, strings.HasPrefix(o1.GetName(), "foo-"))
@@ -45,7 +45,7 @@ func TestAssignNameIfAutonamable(t *testing.T) {
 			"name": resource.NewStringProperty("bar"),
 		}),
 	}
-	AssignNameIfAutonamable(o2, pm2, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
+	AssignNameIfAutonamable(nil, o2, pm2, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
 		tokens.Type(""), tokens.Type("bang:boom/fizzle:AnotherResource"), "bar"))
 	assert.False(t, IsAutonamed(o2))
 	assert.Equal(t, "bar", o2.GetName())
@@ -59,7 +59,7 @@ func TestAssignNameIfAutonamable(t *testing.T) {
 			"name": resource.MakeComputed(resource.NewStringProperty("bar")),
 		}),
 	}
-	AssignNameIfAutonamable(o3, pm3, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
+	AssignNameIfAutonamable(nil, o3, pm3, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
 		tokens.Type(""), tokens.Type("bang:boom/fizzle:MajorResource"), "foo"))
 	assert.False(t, IsAutonamed(o3))
 	assert.Equal(t, "[Computed]", o3.GetName())

diff --git a/provider/pkg/provider/provider.go b/provider/pkg/provider/provider.go
@@ -1306,7 +1306,7 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
 			}
 		}
 	} else {
-		metadata.AssignNameIfAutonamable(newInputs, news, urn)
+		metadata.AssignNameIfAutonamable(req.RandomSeed, newInputs, news, urn)
 
 		// Set a "managed-by: pulumi" label on resources created with Client-Side Apply. To avoid churn on previously
 		// created resources, keep the label in SSA mode if it's already present on the resource.
@@ -1544,7 +1544,7 @@ func (k *kubeProvider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*p
 			newInputs.GetNamespace(), newInputs.GetName())
 	}
 
-	fieldManager := fieldManagerName(newResInputs, newInputs)
+	fieldManager := fieldManagerName(nil, newResInputs, newInputs)
 
 	// Try to compute a server-side patch.
 	ssPatch, ssPatchBase, ssPatchOk, err := k.tryServerSidePatch(oldInputs, newInputs, gvk, fieldManager)
@@ -1737,7 +1737,7 @@ func (k *kubeProvider) Create(
 	}
 
 	initialAPIVersion := newInputs.GetAPIVersion()
-	fieldManager := fieldManagerName(newResInputs, newInputs)
+	fieldManager := fieldManagerName(nil, newResInputs, newInputs)
 
 	if k.yamlRenderMode {
 		if newResInputs.ContainsSecrets() {
@@ -1961,7 +1961,7 @@ func (k *kubeProvider) Read(ctx context.Context, req *pulumirpc.ReadRequest) (*p
 	if err != nil {
 		return nil, err
 	}
-	fieldManager := fieldManagerName(oldState, oldInputs)
+	fieldManager := fieldManagerName(nil, oldState, oldInputs)
 
 	if k.yamlRenderMode {
 		// Return a new "checkpoint object".
@@ -2230,8 +2230,8 @@ func (k *kubeProvider) Update(
 		return nil, err
 	}
 
-	fieldManagerOld := fieldManagerName(oldState, oldInputs)
-	fieldManager := fieldManagerName(oldState, newInputs)
+	fieldManagerOld := fieldManagerName(nil, oldState, oldInputs)
+	fieldManager := fieldManagerName(nil, oldState, newInputs)
 
 	if k.yamlRenderMode {
 		if newResInputs.ContainsSecrets() {
@@ -2408,7 +2408,7 @@ func (k *kubeProvider) Delete(ctx context.Context, req *pulumirpc.DeleteRequest)
 	if err != nil {
 		return nil, err
 	}
-	fieldManager := fieldManagerName(oldState, oldInputs)
+	fieldManager := fieldManagerName(nil, oldState, oldInputs)
 	resources, err := k.getResources()
 	if err != nil {
 		return nil, pkgerrors.Wrapf(err, "Failed to fetch OpenAPI schema from the API server")
@@ -2879,7 +2879,7 @@ func initialAPIVersion(state resource.PropertyMap, oldConfig *unstructured.Unstr
 // 1. Resource annotation (this will likely change to a typed option field in the next major release)
 // 2. Value from the Pulumi state
 // 3. Randomly generated name
-func fieldManagerName(state resource.PropertyMap, inputs *unstructured.Unstructured) string {
+func fieldManagerName(randomSeed []byte, state resource.PropertyMap, inputs *unstructured.Unstructured) string {
 	if v := metadata.GetAnnotationValue(inputs, metadata.AnnotationPatchFieldManager); len(v) > 0 {
 		return v
 	}
@@ -2888,7 +2888,10 @@ func fieldManagerName(state resource.PropertyMap, inputs *unstructured.Unstructu
 	}
 
 	prefix := "pulumi-kubernetes-"
-	fieldManager, err := resource.NewUniqueHex(prefix, 0, 0)
+	// This function is called from other provider function apart from Check and so doesn't have a randomSeed
+	// for those calls, but the field manager name should have already been filled in via Check so this case
+	// shouldn't actually get hit.
+	fieldManager, err := resource.NewUniqueName(randomSeed, prefix, 0, 0, nil)
 	contract.AssertNoError(err)
 
 	return fieldManager