Skip to content

Commit

Permalink
Support for metadata.generateName (CSA) (#2808)
Browse files Browse the repository at this point in the history
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

This PR implements support for `.metadata.generateName` in CSA mode,
based on #2790.
1. Adjust the auto-naming logic to consider `.metadata.generateName` to
be a variant of auto-naming. Pulumi will not assign an auto-name to a
new resource that has `generateName`, and upon delete will use the
replace-then-delete technique.
2. Use outputs rather than inputs to fetch the live object or to obtain
the live object's name.
3. Fix an autonaming bug where the old auto-name would erroneously be
adopted, overwriting a new computed name.

### Tests
1. A new integration test is provided to test the use of
`metadata.generateName`. It tests creation, update, replacement, and
promotion from `.generateName` to `.name`.
([ref](https://github.com/pulumi/pulumi-kubernetes/pull/2808/files#diff-7e85ce707283b1d281d971a8e5b0c49b959b0803ba3437dc9dbd422552326835R269))
2. New test cases for the await logic.
3. The existing autonaming test is enhanced to test how autonaming takes
precedence over `generateName`, in the update case. This is to ensure
backwards compatibility, e.g. in the edge case that an existing object
has a `generateName` field (which Pulumi ignored until now).

### Related issues (optional)
Closes #2539
  • Loading branch information
EronWright authored Feb 3, 2024
1 parent 33c84f9 commit cc3c25d
Show file tree
Hide file tree
Showing 22 changed files with 677 additions and 67 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
- Fix DiffConfig issue when when provider's kubeconfig is set to file path (https://github.com/pulumi/pulumi-kubernetes/pull/2771)
- Fix for replacement having incorrect status messages (https://github.com/pulumi/pulumi-kubernetes/pull/2810)
- Use output properties for await logic (https://github.com/pulumi/pulumi-kubernetes/pull/2790)

- Support for metadata.generateName (CSA) (https://github.com/pulumi/pulumi-kubernetes/pull/2808)

## 4.7.1 (January 17, 2024)

- Fix deployment await logic for accurate rollout detection
Expand Down
17 changes: 17 additions & 0 deletions provider/pkg/await/await_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func Test_Creation(t *testing.T) {
require.NotNil(t, actual)
require.Equal(t, ns, actual.GetNamespace(), "Object should have the expected namespace")
require.Equal(t, name, actual.GetName(), "Object should have the expected name")

gvr, err := clients.GVRForGVK(ctx.mapper, ctx.config.Inputs.GroupVersionKind())
require.NoError(t, err)
_, err = ctx.client.Tracker().Get(gvr, ns, name)
Expand Down Expand Up @@ -241,6 +242,15 @@ func Test_Creation(t *testing.T) {
awaiter: touch,
expect: []expectF{created("", "foo"), touched()},
},
{
name: "GenerateName",
args: args{
resType: tokens.Type("kubernetes:core/v1:Pod"),
inputs: withGenerateName(validPodUnstructured),
},
awaiter: touch,
expect: []expectF{created("default", "foo-generated"), touched()},
},
{
name: "SkipAwait",
args: args{
Expand Down Expand Up @@ -374,6 +384,13 @@ func withSkipAwait(obj *unstructured.Unstructured) *unstructured.Unstructured {
return copy
}

func withGenerateName(obj *unstructured.Unstructured) *unstructured.Unstructured {
copy := obj.DeepCopy()
copy.SetGenerateName(fmt.Sprintf("%s-", obj.GetName()))
copy.SetName("")
return copy
}

// --------------------------------------------------------------------------

// Mock implementations of Kubernetes client stuff.
Expand Down
21 changes: 21 additions & 0 deletions provider/pkg/clients/fake/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ package fake

import (
"github.com/pulumi/pulumi-kubernetes/provider/v4/pkg/clients"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
kubeversion "k8s.io/apimachinery/pkg/version"
"k8s.io/client-go/restmapper"
kubetesting "k8s.io/client-go/testing"
"k8s.io/kubectl/pkg/scheme"
)

Expand Down Expand Up @@ -76,6 +78,7 @@ func NewSimpleDynamicClient(opts ...NewDynamicClientOption) (*clients.DynamicCli
// make a fake clientset for testing purposes, backed by an testing.ObjectTracker with pre-populated objects.
// see also: https://github.com/kubernetes/client-go/blob/kubernetes-1.29.0/examples/fake-client/main_test.go
client := NewSimpleDynamicCLient(options.Scheme, options.Objects...)
client.PrependReactor("create", "*", AdmitCreate())

cs := &clients.DynamicClientSet{
GenericClient: client,
Expand All @@ -84,3 +87,21 @@ func NewSimpleDynamicClient(opts ...NewDynamicClientOption) (*clients.DynamicCli
}
return cs, disco, mapper, client
}

func AdmitCreate() kubetesting.ReactionFunc {
return func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
if action, ok := action.(kubetesting.CreateAction); ok {
objMeta, err := meta.Accessor(action.GetObject())
if err != nil {
return false, nil, err
}

// implement GenerateName since underlying object tracker doesn't natively support this.
if objMeta.GetGenerateName() != "" && objMeta.GetName() == "" {
name := objMeta.GetGenerateName() + "generated"
objMeta.SetName(name)
}
}
return false, nil, nil
}
}
40 changes: 29 additions & 11 deletions provider/pkg/metadata/naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,7 @@ import (
// All auto-named resources get the annotation `pulumi.com/autonamed` for tooling purposes.
func AssignNameIfAutonamable(randomSeed []byte, obj *unstructured.Unstructured, propMap resource.PropertyMap, urn resource.URN) {
contract.Assertf(urn.Name() != "", "expected non-empty name in URN: %s", urn)
// 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 {
if name, ok := md["name"]; ok && name.IsComputed() {
return
}
}

if obj.GetName() == "" {
if !IsNamed(obj, propMap) && !IsGenerateName(obj, propMap) {
prefix := urn.Name() + "-"
autoname, err := resource.NewUniqueName(randomSeed, prefix, 0, 0, nil)
contract.AssertNoErrorf(err, "unexpected error while creating NewUniqueName")
Expand All @@ -42,14 +35,39 @@ func AssignNameIfAutonamable(randomSeed []byte, obj *unstructured.Unstructured,

// AdoptOldAutonameIfUnnamed checks if `newObj` has a name, and if not, "adopts" the name of `oldObj`
// instead. If `oldObj` was autonamed, then we mark `newObj` as autonamed, too.
func AdoptOldAutonameIfUnnamed(newObj, oldObj *unstructured.Unstructured) {
contract.Assertf(oldObj.GetName() != "", "expected nonempty name for object: %s", oldObj)
if newObj.GetName() == "" && IsAutonamed(oldObj) {
// Note that autonaming is preferred over generateName for backwards compatibility.
func AdoptOldAutonameIfUnnamed(newObj, oldObj *unstructured.Unstructured, newObjMap resource.PropertyMap) {
if !IsNamed(newObj, newObjMap) && IsAutonamed(oldObj) {
contract.Assertf(oldObj.GetName() != "", "expected object name to be non-empty: %v", oldObj)
newObj.SetName(oldObj.GetName())
SetAnnotationTrue(newObj, AnnotationAutonamed)
}
}

// IsAutonamed checks if the object is auto-named by Pulumi.
func IsAutonamed(obj *unstructured.Unstructured) bool {
return IsAnnotationTrue(obj, AnnotationAutonamed)
}

// IsGenerateName checks if the object is auto-named by Kubernetes.
func IsGenerateName(obj *unstructured.Unstructured, propMap resource.PropertyMap) bool {
if IsNamed(obj, propMap) {
return false
}
if md, ok := propMap["metadata"].V.(resource.PropertyMap); ok {
if name, ok := md["generateName"]; ok && name.IsComputed() {
return true
}
}
return obj.GetGenerateName() != ""
}

// IsNamed checks if the object has an assigned name (may be a known or computed value).
func IsNamed(obj *unstructured.Unstructured, propMap resource.PropertyMap) bool {
if md, ok := propMap["metadata"].V.(resource.PropertyMap); ok {
if name, ok := md["name"]; ok && name.IsComputed() {
return true
}
}
return obj.GetName() != ""
}
81 changes: 68 additions & 13 deletions provider/pkg/metadata/naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
package metadata

import (
"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"
"strings"
"testing"

"github.com/pulumi/pulumi/sdk/v3/go/common/tokens"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource"

"github.com/stretchr/testify/assert"
Expand All @@ -37,32 +38,54 @@ func TestAssignNameIfAutonamable(t *testing.T) {
assert.Len(t, o1.GetName(), 12)

// o2 has a name, so autonaming fails.
o2 := &unstructured.Unstructured{
Object: map[string]any{"metadata": map[string]any{"name": "bar"}},
}
pm2 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"name": resource.NewStringProperty("bar"),
}),
}
o2 := propMapToUnstructured(pm2)
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())

// o3 has a computed name, so autonaming fails.
o3 := &unstructured.Unstructured{
Object: map[string]any{"metadata": map[string]any{"name": "[Computed]"}},
}
pm3 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"name": resource.MakeComputed(resource.NewStringProperty("bar")),
}),
}
o3 := propMapToUnstructured(pm3)
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())
assert.Equal(t, "", o3.GetName())

// o4 has a generateName, so autonaming fails.
pm4 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"generateName": resource.NewStringProperty("bar-"),
}),
}
o4 := propMapToUnstructured(pm4)
AssignNameIfAutonamable(nil, o4, pm4, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
tokens.Type(""), tokens.Type("bang:boom/fizzle:AnotherResource"), "bar"))
assert.False(t, IsAutonamed(o4))
assert.Equal(t, "bar-", o4.GetGenerateName())
assert.Equal(t, "", o4.GetName())

// o5 has a computed generateName, so autonaming fails.
pm5 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"generateName": resource.MakeComputed(resource.NewStringProperty("bar")),
}),
}
o5 := propMapToUnstructured(pm5)
AssignNameIfAutonamable(nil, o5, pm5, resource.NewURN(tokens.QName("teststack"), tokens.PackageName("testproj"),
tokens.Type(""), tokens.Type("bang:boom/fizzle:MajorResource"), "foo"))
assert.False(t, IsAutonamed(o5))
assert.Equal(t, "", o5.GetGenerateName())
assert.Equal(t, "", o5.GetName())
}

func TestAdoptName(t *testing.T) {
Expand All @@ -77,10 +100,13 @@ func TestAdoptName(t *testing.T) {
},
},
}
new1 := &unstructured.Unstructured{
Object: map[string]any{"metadata": map[string]any{"name": "new1"}},
pm1 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"name": resource.NewStringProperty("new1"),
}),
}
AdoptOldAutonameIfUnnamed(new1, old1)
new1 := propMapToUnstructured(pm1)
AdoptOldAutonameIfUnnamed(new1, old1, pm1)
assert.Equal(t, "old1", old1.GetName())
assert.True(t, IsAutonamed(old1))
assert.Equal(t, "new1", new1.GetName())
Expand All @@ -90,22 +116,51 @@ func TestAdoptName(t *testing.T) {
new2 := &unstructured.Unstructured{
Object: map[string]any{},
}
AdoptOldAutonameIfUnnamed(new2, old1)
pm2 := resource.NewPropertyMap(struct{}{})
AdoptOldAutonameIfUnnamed(new2, old1, pm2)
assert.Equal(t, "old1", new2.GetName())
assert.True(t, IsAutonamed(new2))

// old2 is not autonamed, so new3 DOES NOT adopt old2's name.
new3 := &unstructured.Unstructured{
Object: map[string]any{},
}
pm3 := resource.NewPropertyMap(struct{}{})
old2 := &unstructured.Unstructured{
Object: map[string]any{
"metadata": map[string]any{
"name": "old1",
},
},
}
AdoptOldAutonameIfUnnamed(new3, old2)
AdoptOldAutonameIfUnnamed(new3, old2, pm3)
assert.Equal(t, "", new3.GetName())
assert.False(t, IsAutonamed(new3))

// new4 has a computed name and therefore DOES NOT adopt old1's name.
pm4 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"name": resource.MakeComputed(resource.NewStringProperty("new4")),
}),
}
new4 := propMapToUnstructured(pm4)
assert.Equal(t, "", new4.GetName())
AdoptOldAutonameIfUnnamed(new4, old1, pm4)
assert.Equal(t, "", new4.GetName())
assert.False(t, IsAutonamed(new4))

// new5 has a generateName and therefore DOES adopt old1's name.
pm5 := resource.PropertyMap{
"metadata": resource.NewObjectProperty(resource.PropertyMap{
"generateName": resource.NewStringProperty("new5-"),
}),
}
new5 := propMapToUnstructured(pm5)
AdoptOldAutonameIfUnnamed(new5, old1, pm5)
assert.Equal(t, "old1", new2.GetName())
assert.True(t, IsAutonamed(new2))
}

func propMapToUnstructured(pm resource.PropertyMap) *unstructured.Unstructured {
return &unstructured.Unstructured{Object: pm.MapRepl(nil, nil)}
}
Loading

0 comments on commit cc3c25d

Please sign in to comment.