coder · coadler · Dec 21, 2022 · Dec 15, 2022 · Dec 21, 2022
diff --git a/coderd/templateversions.go b/coderd/templateversions.go
@@ -1069,12 +1069,12 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht
 
 // templateVersionResources returns the workspace agent resources associated
 // with a template version. A template can specify more than one resource to be
-// provisioned, each resource can have an agent that dials back to coderd.
-// The agents returned are informative of the template version, and do not
-// return agents associated with any particular workspace.
+// provisioned, each resource can have an agent that dials back to coderd. The
+// agents returned are informative of the template version, and do not return
+// agents associated with any particular workspace.
 func (api *API) templateVersionResources(rw http.ResponseWriter, r *http.Request) {
-	ctx := r.Context()
 	var (
+		ctx             = r.Context()
 		templateVersion = httpmw.TemplateVersionParam(r)
 		template        = httpmw.TemplateParam(r)
 	)
@@ -1100,8 +1100,8 @@ func (api *API) templateVersionResources(rw http.ResponseWriter, r *http.Request
 // and not any build logs for a workspace.
 // Eg: Logs returned from 'terraform plan' when uploading a new terraform file.
 func (api *API) templateVersionLogs(rw http.ResponseWriter, r *http.Request) {
-	ctx := r.Context()
 	var (
+		ctx             = r.Context()
 		templateVersion = httpmw.TemplateVersionParam(r)
 		template        = httpmw.TemplateParam(r)
 	)

diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go
@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -270,7 +269,7 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
 		return "", ctx.Err()
 	}
 
-	var out bytes.Buffer
+	var out strings.Builder
 	cmd := exec.CommandContext(killCtx, e.binaryPath, "graph") // #nosec
 	cmd.Stdout = &out
 	cmd.Dir = e.workdir
@@ -289,14 +288,13 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
 	return out.String(), nil
 }
 
-// revive:disable-next-line:flag-parameter
 func (e *executor) apply(
 	ctx, killCtx context.Context, plan []byte, env []string, logr logSink,
 ) (*proto.Provision_Response, error) {
 	e.mut.Lock()
 	defer e.mut.Unlock()
 
-	planFile, err := ioutil.TempFile("", "coder-terrafrom-plan")
+	planFile, err := os.CreateTemp("", "coder-terrafrom-plan")
 	if err != nil {
 		return nil, xerrors.Errorf("create plan file: %w", err)
 	}

diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go
@@ -67,8 +67,8 @@ type metadataItem struct {
 	IsNull    bool   `mapstructure:"is_null"`
 }
 
-// ConvertResources consumes Terraform state and a GraphViz representation produced by
-// `terraform graph` to produce resources consumable by Coder.
+// ConvertResources consumes Terraform state and a GraphViz representation
+// produced by `terraform graph` to produce resources consumable by Coder.
 // nolint:gocyclo
 func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Resource, error) {
 	parsedGraph, err := gographviz.ParseString(rawGraph)
@@ -83,13 +83,9 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
 	resources := make([]*proto.Resource, 0)
 	resourceAgents := map[string][]*proto.Agent{}
 
-	// Indexes Terraform resources by their label and ID.
-	// The label is what "terraform graph" uses to reference nodes, and the ID
-	// is used by "coder_metadata" resources to refer to their targets. (The ID
-	// field is only available when reading a state file, and not when reading a
-	// plan file.)
+	// Indexes Terraform resources by their label.
+	// The label is what "terraform graph" uses to reference nodes.
 	tfResourceByLabel := map[string]*tfjson.StateResource{}
-	resourceLabelByID := map[string]string{}
 	var findTerraformResources func(mod *tfjson.StateModule)
 	findTerraformResources = func(mod *tfjson.StateModule) {
 		for _, module := range mod.ChildModules {
@@ -99,14 +95,6 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
 			label := convertAddressToLabel(resource.Address)
 			// index by label
 			tfResourceByLabel[label] = resource
-			// index by ID, if it exists
-			id, ok := resource.AttributeValues["id"]
-			if ok {
-				idString, ok := id.(string)
-				if ok {
-					resourceLabelByID[idString] = label
-				}
-			}
 		}
 	}
 	findTerraformResources(module)
@@ -310,58 +298,48 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
 		if resource.Type != "coder_metadata" {
 			continue
 		}
+
 		var attrs metadataAttributes
 		err = mapstructure.Decode(resource.AttributeValues, &attrs)
 		if err != nil {
 			return nil, xerrors.Errorf("decode metadata attributes: %w", err)
 		}
 
-		var targetLabel string
-		// This occurs in a plan, because there is no resource ID.
-		// We attempt to find the closest node, just so we can hide it from the UI.
-		if attrs.ResourceID == "" {
-			resourceLabel := convertAddressToLabel(resource.Address)
+		resourceLabel := convertAddressToLabel(resource.Address)
 
-			var attachedNode *gographviz.Node
-			for _, node := range graph.Nodes.Lookup {
-				// The node attributes surround the label with quotes.
-				if strings.Trim(node.Attrs["label"], `"`) != resourceLabel {
-					continue
-				}
-				attachedNode = node
-				break
+		var attachedNode *gographviz.Node
+		for _, node := range graph.Nodes.Lookup {
+			// The node attributes surround the label with quotes.
+			if strings.Trim(node.Attrs["label"], `"`) != resourceLabel {
+				continue
 			}
-			if attachedNode == nil {
+			attachedNode = node
+			break
+		}
+		if attachedNode == nil {
+			continue
+		}
+		var attachedResource *graphResource
+		for _, resource := range findResourcesInGraph(graph, tfResourceByLabel, attachedNode.Name, 0, false) {
+			if attachedResource == nil {
+				// Default to the first resource because we have nothing to compare!
+				attachedResource = resource
 				continue
 			}
-			var attachedResource *graphResource
-			for _, resource := range findResourcesInGraph(graph, tfResourceByLabel, attachedNode.Name, 0, false) {
-				if attachedResource == nil {
-					// Default to the first resource because we have nothing to compare!
-					attachedResource = resource
-					continue
-				}
-				if resource.Depth < attachedResource.Depth {
-					// There's a closer resource!
-					attachedResource = resource
-					continue
-				}
-				if resource.Depth == attachedResource.Depth && resource.Label < attachedResource.Label {
-					attachedResource = resource
-					continue
-				}
+			if resource.Depth < attachedResource.Depth {
+				// There's a closer resource!
+				attachedResource = resource
+				continue
 			}
-			if attachedResource == nil {
+			if resource.Depth == attachedResource.Depth && resource.Label < attachedResource.Label {
+				attachedResource = resource
 				continue
 			}
-			targetLabel = attachedResource.Label
-		}
-		if targetLabel == "" {
-			targetLabel = resourceLabelByID[attrs.ResourceID]
 		}
-		if targetLabel == "" {
+		if attachedResource == nil {
 			continue
 		}
+		targetLabel := attachedResource.Label
 
 		resourceHidden[targetLabel] = attrs.Hide
 		resourceIcon[targetLabel] = attrs.Icon
@@ -407,9 +385,11 @@ func ConvertResources(module *tfjson.StateModule, rawGraph string) ([]*proto.Res
 }
 
 // convertAddressToLabel returns the Terraform address without the count
-// specifier. eg. "module.ec2_dev.ec2_instance.dev[0]" becomes "module.ec2_dev.ec2_instance.dev"
+// specifier.
+// eg. "module.ec2_dev.ec2_instance.dev[0]" becomes "module.ec2_dev.ec2_instance.dev"
 func convertAddressToLabel(address string) string {
-	return strings.Split(address, "[")[0]
+	cut, _, _ := strings.Cut(address, "[")
+	return cut
 }
 
 type graphResource struct {

diff --git a/provisioner/terraform/resources_test.go b/provisioner/terraform/resources_test.go
@@ -8,14 +8,14 @@ import (
 	"sort"
 	"testing"
 
+	protobuf "github.com/golang/protobuf/proto"
 	tfjson "github.com/hashicorp/terraform-json"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/slices"
 
 	"github.com/coder/coder/cryptorand"
 	"github.com/coder/coder/provisioner/terraform"
 	"github.com/coder/coder/provisionersdk/proto"
-
-	protobuf "github.com/golang/protobuf/proto"
 )
 
 func TestConvertResources(t *testing.T) {
@@ -165,6 +165,53 @@ func TestConvertResources(t *testing.T) {
 				Sensitive: true,
 			}},
 		}},
+		// Tests that resources with the same id correctly get metadata applied
+		// to them.
+		"kubernetes-metadata": {{
+			Name: "coder_workspace",
+			Type: "kubernetes_service_account",
+		}, {
+			Name: "coder_workspace",
+			Type: "kubernetes_config_map",
+		}, {
+			Name: "coder_workspace",
+			Type: "kubernetes_role",
+		}, {
+			Name: "coder_workspace",
+			Type: "kubernetes_role_binding",
+		}, {
+			Name: "coder_workspace",
+			Type: "kubernetes_secret",
+		}, {
+			Name: "main",
+			Type: "kubernetes_pod",
+			Metadata: []*proto.Resource_Metadata{{
+				Key:   "cpu",
+				Value: "1",
+			}, {
+				Key:   "memory",
+				Value: "1Gi",
+			}, {
+				Key:   "gpu",
+				Value: "1",
+			}},
+			Agents: []*proto.Agent{{
+				Name:            "main",
+				OperatingSystem: "linux",
+				Architecture:    "amd64",
+				StartupScript:   "    #!/bin/bash\n    # home folder can be empty, so copying default bash settings\n    if [ ! -f ~/.profile ]; then\n      cp /etc/skel/.profile $HOME\n    fi\n    if [ ! -f ~/.bashrc ]; then\n      cp /etc/skel/.bashrc $HOME\n    fi\n    # install and start code-server\n    curl -fsSL https://code-server.dev/install.sh | sh  | tee code-server-install.log\n    code-server --auth none --port 13337 | tee code-server-install.log &\n",
+				Apps: []*proto.App{
+					{
+						Icon:        "/icon/code.svg",
+						Slug:        "code-server",
+						DisplayName: "code-server",
+						Url:         "http://localhost:13337?folder=/home/coder",
+					},
+				},
+				Auth:                     &proto.Agent_Token{},
+				ConnectionTimeoutSeconds: 120,
+			}},
+		}},
 	} {
 		folderName := folderName
 		expected := expected
@@ -210,6 +257,17 @@ func TestConvertResources(t *testing.T) {
 				err = json.Unmarshal(data, &resourcesMap)
 				require.NoError(t, err)
 
+				slices.SortFunc(expectedNoMetadataMap, func(a, b map[string]interface{}) bool {
+					//nolint:forcetypeassert
+					return a["name"].(string)+a["type"].(string) <
+						b["name"].(string)+b["type"].(string)
+				})
+				slices.SortFunc(resourcesMap, func(a, b map[string]interface{}) bool {
+					//nolint:forcetypeassert
+					return a["name"].(string)+a["type"].(string) <
+						b["name"].(string)+b["type"].(string)
+				})
+
 				require.Equal(t, expectedNoMetadataMap, resourcesMap)
 			})
 
@@ -251,6 +309,17 @@ func TestConvertResources(t *testing.T) {
 				err = json.Unmarshal(data, &resourcesMap)
 				require.NoError(t, err)
 
+				slices.SortFunc(expectedMap, func(a, b map[string]interface{}) bool {
+					//nolint:forcetypeassert
+					return a["name"].(string)+a["type"].(string) <
+						b["name"].(string)+b["type"].(string)
+				})
+				slices.SortFunc(resourcesMap, func(a, b map[string]interface{}) bool {
+					//nolint:forcetypeassert
+					return a["name"].(string)+a["type"].(string) <
+						b["name"].(string)+b["type"].(string)
+				})
+
 				require.Equal(t, expectedMap, resourcesMap)
 			})
 		})

diff --git a/provisioner/terraform/testdata/calling-module/calling-module.tfplan.json b/provisioner/terraform/testdata/calling-module/calling-module.tfplan.json
diff --git a/provisioner/terraform/testdata/calling-module/calling-module.tfstate.json b/provisioner/terraform/testdata/calling-module/calling-module.tfstate.json
diff --git a/provisioner/terraform/testdata/chaining-resources/chaining-resources.tfplan.json b/provisioner/terraform/testdata/chaining-resources/chaining-resources.tfplan.json