Skip to content

Commit

Permalink
Fix stack tags to always replace on update. (#206)
Browse files Browse the repository at this point in the history
Stack tags don't really need to be updated since they're not stateful.
We can always just replace the tag, with a delete before replace. This
ensures that it's always `Create` that is called, rather than `Update`.
This keeps things simple and avoids potential bugs in the Update code,
of which there have been multiple. [Exhibit
A](#75) [Exhibit
B](#86)

Fixes #169
  • Loading branch information
komalali authored Dec 18, 2023
1 parent af0aab1 commit a467ccf
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
### Bug Fixes

- Fix `Read` for TeamStackPermission so resources are not deleted from state on refresh. Note: TeamStackPermission resources created before v0.17.0 will now return an error if attempting a refresh, but those (re)created with the new version will support `refresh`. [#205](https://github.com/pulumi/pulumi-pulumiservice/pull/205)
- Fix `Update` for StackTags so tag names can be updated. [#206](https://github.com/pulumi/pulumi-pulumiservice/pull/206)

71 changes: 33 additions & 38 deletions provider/pkg/provider/stack_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/pulumi/pulumi-pulumiservice/provider/pkg/internal/pulumiapi"
"github.com/pulumi/pulumi-pulumiservice/provider/pkg/internal/serde"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
)

Expand Down Expand Up @@ -42,17 +43,39 @@ func (st *PulumiServiceStackTagResource) Name() string {
}

func (st *PulumiServiceStackTagResource) Diff(req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
changed, err := serde.DiffOldsAndNews(req)
olds, err := plugin.UnmarshalProperties(req.GetOlds(), plugin.MarshalOptions{KeepUnknowns: false, SkipNulls: true})
if err != nil {
return nil, err
}
changes := pulumirpc.DiffResponse_DIFF_NONE
if len(changed) > 0 {
changes = pulumirpc.DiffResponse_DIFF_SOME

news, err := plugin.UnmarshalProperties(req.GetNews(), plugin.MarshalOptions{KeepUnknowns: true, SkipNulls: true})
if err != nil {
return nil, err
}

diffs := olds.Diff(news)
if diffs == nil {
return &pulumirpc.DiffResponse{
Changes: pulumirpc.DiffResponse_DIFF_NONE,
}, nil
}

dd := plugin.NewDetailedDiffFromObjectDiff(diffs, false)

detailedDiffs := map[string]*pulumirpc.PropertyDiff{}
for k, v := range dd {
v.Kind = v.Kind.AsReplace()
detailedDiffs[k] = &pulumirpc.PropertyDiff{
Kind: pulumirpc.PropertyDiff_Kind(v.Kind),
InputDiff: v.InputDiff,
}
}
return &pulumirpc.DiffResponse{

Changes: changes,
return &pulumirpc.DiffResponse{
Changes: pulumirpc.DiffResponse_DIFF_SOME,
DetailedDiff: detailedDiffs,
DeleteBeforeReplace: true,
HasDetailedDiff: true,
}, nil
}

Expand Down Expand Up @@ -106,33 +129,8 @@ func (st *PulumiServiceStackTagResource) Check(req *pulumirpc.CheckRequest) (*pu
}

func (st *PulumiServiceStackTagResource) Update(req *pulumirpc.UpdateRequest) (*pulumirpc.UpdateResponse, error) {
ctx := context.Background()
var inputs PulumiServiceStackTagInput
err := serde.FromProperties(req.GetNews(), structTagKey, &inputs)
if err != nil {
return nil, err
}
stackName := pulumiapi.StackName{
OrgName: inputs.Organization,
ProjectName: inputs.Project,
StackName: inputs.Stack,
}
stackTag := pulumiapi.StackTag{
Name: inputs.Name,
Value: inputs.Value,
}
err = st.client.DeleteStackTag(ctx, stackName, stackTag.Name)
if err != nil {
return nil, err
}
err = st.client.CreateTag(ctx, stackName, stackTag)
if err != nil {
return nil, err
}

return &pulumirpc.UpdateResponse{
Properties: req.GetNews(),
}, nil
// all updates are destructive, so we just call Create.
return nil, fmt.Errorf("unexpected call to update, expected create to be called instead")
}

func (st *PulumiServiceStackTagResource) Read(req *pulumirpc.ReadRequest) (*pulumirpc.ReadResponse, error) {
Expand Down Expand Up @@ -163,12 +161,9 @@ func (st *PulumiServiceStackTagResource) Read(req *pulumirpc.ReadRequest) (*pulu
return &pulumirpc.ReadResponse{
Id: req.Id,
Properties: props,
Inputs: props,
}, nil
}

func (st *PulumiServiceStackTagResource) Invoke(s *pulumiserviceProvider, req *pulumirpc.InvokeRequest) (*pulumirpc.InvokeResponse, error) {
return &pulumirpc.InvokeResponse{Return: nil}, fmt.Errorf("unknown function '%s'", req.Tok)
}

func (st *PulumiServiceStackTagResource) Configure(config PulumiServiceConfig) {
func (st *PulumiServiceStackTagResource) Configure(_ PulumiServiceConfig) {
}
11 changes: 2 additions & 9 deletions provider/pkg/provider/stack_tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

func TestStackTagsUpdate(t *testing.T) {
t.Run("Calls Delete then Create on Resource", func(t *testing.T) {
t.Run("Calls to Update return an error", func(t *testing.T) {
var gotMethods []string

server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -50,14 +50,7 @@ func TestStackTagsUpdate(t *testing.T) {
}

_, err = st.Update(&upReq)
if err != nil {
t.Error(err)
}

// expect DELETE first, then POST
wantMethods := []string{http.MethodDelete, http.MethodPost}
assert.Equal(t, wantMethods, gotMethods)

assert.ErrorContains(t, err, "unexpected call to update, expected create to be called instead")
})

}

0 comments on commit a467ccf

Please sign in to comment.