From 3a1d27f19bc0786c193da68040bfe068e40e3ca1 Mon Sep 17 00:00:00 2001
From: Kyle Carberry <kyle@carberry.com>
Date: Mon, 12 Sep 2022 11:31:25 -0500
Subject: [PATCH 1/2] fix: Add check for duplicate metadata keys

Fixes coder/coder#3721.
---
 go.mod                             |  1 +
 go.sum                             |  1 +
 internal/provider/provider.go      | 15 +++++++++----
 internal/provider/provider_test.go | 36 ++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/go.mod b/go.mod
index 67776fea..db38910d 100644
--- a/go.mod
+++ b/go.mod
@@ -7,6 +7,7 @@ require (
 	github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.20.0
 	github.com/stretchr/testify v1.8.0
+	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
 )
 
 require (
diff --git a/go.sum b/go.sum
index 510cd74b..318f05a2 100644
--- a/go.sum
+++ b/go.sum
@@ -336,6 +336,7 @@ golang.org/x/tools v0.0.0-20200713011307-fd294ab11aed/go.mod h1:njjCfa9FT2d7l9Bc
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
 golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4=
diff --git a/internal/provider/provider.go b/internal/provider/provider.go
index 5d3a5a16..b5f6fa3d 100644
--- a/internal/provider/provider.go
+++ b/internal/provider/provider.go
@@ -2,7 +2,6 @@ package provider
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/url"
 	"os"
@@ -16,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+	"golang.org/x/xerrors"
 )
 
 type config struct {
@@ -509,7 +509,7 @@ func populateIsNull(resourceData *schema.ResourceData) (result interface{}, err
 	// The cty package reports type mismatches by panicking
 	defer func() {
 		if r := recover(); r != nil {
-			err = errors.New(fmt.Sprintf("panic while handling coder_metadata: %#v", r))
+			err = xerrors.Errorf("panic while handling coder_metadata: %#v", r)
 		}
 	}()
 
@@ -517,9 +517,16 @@ 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":       valueAsString(item.GetAttr("key")),
+			"key":       key,
 			"value":     valueAsString(item.GetAttr("value")),
 			"sensitive": valueAsBool(item.GetAttr("sensitive")),
 		}
@@ -534,7 +541,7 @@ func populateIsNull(resourceData *schema.ResourceData) (result interface{}, err
 
 // valueAsString takes a cty.Value that may be a string or null, and converts it to either a Go string
 // or a nil interface{}
-func valueAsString(value cty.Value) interface{} {
+func valueAsString(value cty.Value) string {
 	if value.IsNull() {
 		return ""
 	}
diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go
index 07cd7eba..2e7acbbb 100644
--- a/internal/provider/provider_test.go
+++ b/internal/provider/provider_test.go
@@ -1,6 +1,7 @@
 package provider_test
 
 import (
+	"regexp"
 	"runtime"
 	"testing"
 
@@ -324,3 +325,38 @@ func TestMetadata(t *testing.T) {
 		}},
 	})
 }
+
+func TestMetadataDuplicateKeys(t *testing.T) {
+	t.Parallel()
+	prov := provider.New()
+	resource.Test(t, resource.TestCase{
+		Providers: map[string]*schema.Provider{
+			"coder": prov,
+		},
+		IsUnitTest: true,
+		Steps: []resource.TestStep{{
+			Config: `
+				provider "coder" {
+				}
+				resource "coder_agent" "dev" {
+					os = "linux"
+					arch = "amd64"
+				}
+				resource "coder_metadata" "agent" {
+					resource_id = coder_agent.dev.id
+					hide = true
+					icon = "/icons/storage.svg"
+					item {
+						key = "foo"
+						value = "bar"
+					}
+					item {
+						key = "foo"
+						value = "bar"
+					}
+				}
+				`,
+			ExpectError: regexp.MustCompile("duplicate metadata key"),
+		}},
+	})
+}

From 29469cd11fa408a92cadacbc76a363b1a4fdaa5c Mon Sep 17 00:00:00 2001
From: Kyle Carberry <kyle@carberry.com>
Date: Tue, 13 Sep 2022 11:53:18 -0500
Subject: [PATCH 2/2] Update internal/provider/provider.go

Co-authored-by: David Wahler <david@coder.com>
---
 internal/provider/provider.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internal/provider/provider.go b/internal/provider/provider.go
index b5f6fa3d..487707d0 100644
--- a/internal/provider/provider.go
+++ b/internal/provider/provider.go
@@ -539,7 +539,8 @@ func populateIsNull(resourceData *schema.ResourceData) (result interface{}, err
 	return resultItems, nil
 }
 
-// valueAsString takes a cty.Value that may be a string or null, and converts it to either a Go string
+// valueAsString takes a cty.Value that may be a string or null, and converts it to a Go string,
+// which will be empty if the input value was null.
 // or a nil interface{}
 func valueAsString(value cty.Value) string {
 	if value.IsNull() {