Skip to content

Commit

Permalink
backport(fix): validate agent & resource metadata keys during plan (#282
Browse files Browse the repository at this point in the history
)
  • Loading branch information
ethanndickson authored Sep 6, 2024
1 parent da10a93 commit e0c0dbe
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 22 deletions.
38 changes: 26 additions & 12 deletions provider/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,6 @@ func agentResource() *schema.Resource {
}
}

rawPlan := resourceData.GetRawPlan()
items := rawPlan.GetAttr("metadata").AsValueSlice()
itemKeys := map[string]struct{}{}
for _, item := range items {
key := valueAsString(item.GetAttr("key"))
_, exists := itemKeys[key]
if exists {
return diag.FromErr(xerrors.Errorf("duplicate agent metadata key %q", key))
}
itemKeys[key] = struct{}{}
}

return updateInitScript(resourceData, i)
},
ReadWithoutTimeout: func(ctx context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics {
Expand Down Expand Up @@ -307,6 +295,32 @@ func agentResource() *schema.Resource {
Optional: true,
},
},
CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i any) error {
if !rd.HasChange("metadata") {
return nil
}

keys := map[string]bool{}
metadata, ok := rd.Get("metadata").([]any)
if !ok {
return xerrors.Errorf("unexpected type %T for metadata, expected []any", rd.Get("metadata"))
}
for _, t := range metadata {
obj, ok := t.(map[string]any)
if !ok {
return xerrors.Errorf("unexpected type %T for metadata, expected map[string]any", t)
}
key, ok := obj["key"].(string)
if !ok {
return xerrors.Errorf("unexpected type %T for metadata key, expected string", obj["key"])
}
if keys[key] {
return xerrors.Errorf("duplicate agent metadata key %q", key)
}
keys[key] = true
}
return nil
},
}
}

Expand Down
7 changes: 4 additions & 3 deletions provider/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ func TestAgent_MetadataDuplicateKeys(t *testing.T) {
}
`,
ExpectError: regexp.MustCompile("duplicate agent metadata key"),
PlanOnly: true,
}},
})
}
Expand Down Expand Up @@ -313,7 +314,7 @@ func TestAgent_DisplayApps(t *testing.T) {
web_terminal = false
port_forwarding_helper = false
ssh_helper = false
}
}
}
`,
Check: func(state *terraform.State) error {
Expand Down Expand Up @@ -363,7 +364,7 @@ func TestAgent_DisplayApps(t *testing.T) {
display_apps {
vscode_insiders = true
web_terminal = true
}
}
}
`,
Check: func(state *terraform.State) error {
Expand Down Expand Up @@ -458,7 +459,7 @@ func TestAgent_DisplayApps(t *testing.T) {
web_terminal = false
port_forwarding_helper = false
ssh_helper = false
}
}
}
`,
ExpectError: regexp.MustCompile(`An argument named "fake_app" is not expected here.`),
Expand Down
27 changes: 27 additions & 0 deletions provider/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/google/uuid"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"golang.org/x/xerrors"
)

func metadataResource() *schema.Resource {
Expand Down Expand Up @@ -111,5 +112,31 @@ func metadataResource() *schema.Resource {
},
},
},
CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i interface{}) error {
if !rd.HasChange("item") {
return nil
}

keys := map[string]bool{}
metadata, ok := rd.Get("item").([]any)
if !ok {
return xerrors.Errorf("unexpected type %T for items, expected []any", rd.Get("metadata"))
}
for _, t := range metadata {
obj, ok := t.(map[string]any)
if !ok {
return xerrors.Errorf("unexpected type %T for item, expected map[string]any", t)
}
key, ok := obj["key"].(string)
if !ok {
return xerrors.Errorf("unexpected type %T for items 'key' attribute, expected string", obj["key"])
}
if keys[key] {
return xerrors.Errorf("duplicate resource metadata key %q", key)
}
keys[key] = true
}
return nil
},
}
}
3 changes: 2 additions & 1 deletion provider/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ func TestMetadataDuplicateKeys(t *testing.T) {
}
}
`,
ExpectError: regexp.MustCompile("duplicate metadata key"),
PlanOnly: true,
ExpectError: regexp.MustCompile("duplicate resource metadata key"),
}},
})
}
6 changes: 0 additions & 6 deletions provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,8 @@ func populateIsNull(resourceData *schema.ResourceData) (result interface{}, err
items := rawPlan.GetAttr("item").AsValueSlice()

var resultItems []interface{}
itemKeys := map[string]struct{}{}
for _, item := range items {
key := valueAsString(item.GetAttr("key"))
_, exists := itemKeys[key]
if exists {
return nil, xerrors.Errorf("duplicate metadata key %q", key)
}
itemKeys[key] = struct{}{}
resultItem := map[string]interface{}{
"key": key,
"value": valueAsString(item.GetAttr("value")),
Expand Down

0 comments on commit e0c0dbe

Please sign in to comment.