fix: validate agent & resource metadata keys during plan

Author: ethanndickson

integration/integration_test.go

@@ -87,7 +87,7 @@ func TestIntegration(t *testing.T) {
 		},
 		{
 			name:       "workspace-owner",
-			minVersion: "v2.12.0",
+			minVersion: "v2.13.0",
 			expectedOutput: map[string]string{
 				"provisioner.arch":                  runtime.GOARCH,
 				"provisioner.id":                    `[a-zA-Z0-9-]+`,
@@ -103,7 +103,7 @@ func TestIntegration(t *testing.T) {
 				"workspace.transition":              `start`,
 				"workspace_owner.email":             `testing@coder\.com`,
 				"workspace_owner.full_name":         `default`,
-				"workspace_owner.groups":            `\[\]`,
+				"workspace_owner.groups":            `\[\"Everyone\"\]`,
 				"workspace_owner.id":                `[a-zA-Z0-9-]+`,
 				"workspace_owner.name":              `testing`,
 				"workspace_owner.oidc_access_token": `^$`, // TODO: test OIDC integration

provider/agent.go

@@ -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 {
@@ -272,6 +260,23 @@ func agentResource() *schema.Resource {
 				Optional:    true,
 			},
 		},
+		CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i interface{}) error {
+			if !rd.HasChange("metadata") {
+				return nil
+			}
+
+			keys := map[string]struct{}{}
+			for _, t := range rd.Get("metadata").([]interface{}) {
+				taint := t.(map[string]interface{})
+				key := taint["key"].(string)
+				_, ok := keys[key]
+				if ok {
+					return xerrors.Errorf("duplicate agent metadata key %q", key)
+				}
+				keys[key] = struct{}{}
+			}
+			return nil
+		},
 	}
 }
 

provider/agent_test.go

@@ -254,6 +254,7 @@ func TestAgent_MetadataDuplicateKeys(t *testing.T) {
 				}
 				`,
 			ExpectError: regexp.MustCompile("duplicate agent metadata key"),
+			PlanOnly:    true,
 		}},
 	})
 }
@@ -281,7 +282,7 @@ func TestAgent_DisplayApps(t *testing.T) {
 							web_terminal = false
 							port_forwarding_helper = false
 							ssh_helper = false
-						} 
+						}
 					}
 					`,
 				Check: func(state *terraform.State) error {
@@ -331,7 +332,7 @@ func TestAgent_DisplayApps(t *testing.T) {
 						display_apps {
 							vscode_insiders = true
 							web_terminal = true
-						} 
+						}
 					}
 					`,
 				Check: func(state *terraform.State) error {
@@ -426,7 +427,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.`),

provider/metadata.go

@@ -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 {
@@ -111,5 +112,22 @@ func metadataResource() *schema.Resource {
 				},
 			},
 		},
+		CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i interface{}) error {
+			if !rd.HasChange("item") {
+				return nil
+			}
+
+			keys := map[string]struct{}{}
+			for _, t := range rd.Get("item").([]interface{}) {
+				taint := t.(map[string]interface{})
+				key := taint["key"].(string)
+				_, ok := keys[key]
+				if ok {
+					return xerrors.Errorf("duplicate resource metadata key %q", key)
+				}
+				keys[key] = struct{}{}
+			}
+			return nil
+		},
 	}
 }

provider/metadata_test.go

@@ -123,7 +123,8 @@ func TestMetadataDuplicateKeys(t *testing.T) {
 					}
 				}
 				`,
-			ExpectError: regexp.MustCompile("duplicate metadata key"),
+			PlanOnly:    true,
+			ExpectError: regexp.MustCompile("duplicate resource metadata key"),
 		}},
 	})
 }

provider/provider.go

@@ -97,14 +97,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")),