Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use NewUniqueName #2137

Merged
merged 5 commits into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions provider/pkg/metadata/naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions provider/pkg/metadata/naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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-"))
Expand All @@ -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())
Expand All @@ -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())
Expand Down
21 changes: 12 additions & 9 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down