From 3c0d2cd5659cbe55002d4ccf1276610f3d8241d1 Mon Sep 17 00:00:00 2001 From: Fraser Waters Date: Tue, 16 Aug 2022 15:13:51 +0100 Subject: [PATCH] Use NewUniqueName (#2137) * Use NewUniqueName * Change comments * Fix test * Add to CHANGELOG --- CHANGELOG.md | 4 +++- provider/pkg/metadata/naming.go | 4 ++-- provider/pkg/metadata/naming_test.go | 6 +++--- provider/pkg/provider/provider.go | 21 ++++++++++++--------- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3a06c95a8..e421d43240 100644 --- 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 index e59623cb49..07bd7d3fd2 100644 --- 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 index b08da3e4ba..d1bdee69a4 100644 --- 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 index 35ce2fb0d1..f82082c9bf 100644 --- 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