diff --git a/cli/vpndaemon_darwin.go b/cli/vpndaemon_darwin.go index a1b836dd6b0c3..0e019a728ac71 100644 --- a/cli/vpndaemon_darwin.go +++ b/cli/vpndaemon_darwin.go @@ -10,7 +10,7 @@ import ( "github.com/coder/serpent" ) -func (r *RootCmd) vpnDaemonRun() *serpent.Command { +func (*RootCmd) vpnDaemonRun() *serpent.Command { var ( rpcReadFD int64 rpcWriteFD int64 diff --git a/provisioner/terraform/resources.go b/provisioner/terraform/resources.go index 9642751e7466a..a47c450dd6fed 100644 --- a/provisioner/terraform/resources.go +++ b/provisioner/terraform/resources.go @@ -208,6 +208,10 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s // The label is what "terraform graph" uses to reference nodes. tfResourcesByLabel := map[string]map[string]*tfjson.StateResource{} + // Map resource IDs to labels for efficient lookup when processing metadata + // Multiple resources can have the same ID, so we store a slice of labels + labelsByResourceID := map[string][]string{} + // Extra array to preserve the order of rich parameters. tfResourcesRichParameters := make([]*tfjson.StateResource, 0) tfResourcesPresets := make([]*tfjson.StateResource, 0) @@ -233,6 +237,13 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s tfResourcesByLabel[label] = map[string]*tfjson.StateResource{} } tfResourcesByLabel[label][resource.Address] = resource + + // Build the ID to labels map - multiple resources can have the same ID + if idAttr, hasID := resource.AttributeValues["id"]; hasID { + if idStr, ok := idAttr.(string); ok && idStr != "" { + labelsByResourceID[idStr] = append(labelsByResourceID[idStr], label) + } + } } } for _, module := range modules { @@ -684,41 +695,69 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s if err != nil { return nil, xerrors.Errorf("decode metadata attributes: %w", err) } - 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 + var targetLabel string + + // First, check if ResourceID is provided and try to find the resource by ID + if attrs.ResourceID != "" { + // Look for a resource with matching ID + foundLabels := labelsByResourceID[attrs.ResourceID] + switch len(foundLabels) { + case 0: + // If we couldn't find by ID, fall back to graph traversal + logger.Warn(ctx, "coder_metadata resource_id not found, falling back to graph traversal", + slog.F("resource_id", attrs.ResourceID), + slog.F("metadata_address", resource.Address)) + case 1: + // Single match - use it + targetLabel = foundLabels[0] + default: + // Multiple resources with same ID - this creates ambiguity + logger.Warn(ctx, "multiple resources found with same resource_id, falling back to graph traversal", + slog.F("resource_id", attrs.ResourceID), + slog.F("metadata_address", resource.Address), + slog.F("matching_labels", foundLabels)) } - attachedNode = node - break } - if attachedNode == nil { - continue - } - var attachedResource *graphResource - for _, resource := range findResourcesInGraph(graph, tfResourcesByLabel, attachedNode.Name, 0, false) { - if attachedResource == nil { - // Default to the first resource because we have nothing to compare! - attachedResource = resource - continue + + // If ResourceID wasn't provided or wasn't found, use graph traversal + if targetLabel == "" { + 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 } - if resource.Depth < attachedResource.Depth { - // There's a closer resource! - attachedResource = resource + if attachedNode == nil { continue } - if resource.Depth == attachedResource.Depth && resource.Label < attachedResource.Label { - attachedResource = resource + var attachedResource *graphResource + for _, resource := range findResourcesInGraph(graph, tfResourcesByLabel, 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 attachedResource == nil { continue } + targetLabel = attachedResource.Label } - if attachedResource == nil { - continue - } - targetLabel := attachedResource.Label if metadataTargetLabels[targetLabel] { return nil, xerrors.Errorf("duplicate metadata resource: %s", targetLabel) diff --git a/provisioner/terraform/resources_test.go b/provisioner/terraform/resources_test.go index 1575c6c9c159e..d4987e2effa5f 100644 --- a/provisioner/terraform/resources_test.go +++ b/provisioner/terraform/resources_test.go @@ -1249,24 +1249,93 @@ func TestAgentNameDuplicate(t *testing.T) { require.ErrorContains(t, err, "duplicate agent name") } -func TestMetadataResourceDuplicate(t *testing.T) { +func TestMetadata(t *testing.T) { t.Parallel() - ctx, logger := ctxAndLogger(t) - // Load the multiple-apps state file and edit it. - dir := filepath.Join("testdata", "resources", "resource-metadata-duplicate") - tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "resource-metadata-duplicate.tfplan.json")) - require.NoError(t, err) - var tfPlan tfjson.Plan - err = json.Unmarshal(tfPlanRaw, &tfPlan) - require.NoError(t, err) - tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "resource-metadata-duplicate.tfplan.dot")) - require.NoError(t, err) + t.Run("Duplicate", func(t *testing.T) { + t.Parallel() + ctx, logger := ctxAndLogger(t) + // Load the multiple-apps state file and edit it. + dir := filepath.Join("testdata", "resources", "resource-metadata-duplicate") + tfPlanRaw, err := os.ReadFile(filepath.Join(dir, "resource-metadata-duplicate.tfplan.json")) + require.NoError(t, err) + var tfPlan tfjson.Plan + err = json.Unmarshal(tfPlanRaw, &tfPlan) + require.NoError(t, err) + tfPlanGraph, err := os.ReadFile(filepath.Join(dir, "resource-metadata-duplicate.tfplan.dot")) + require.NoError(t, err) - state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger) - require.Nil(t, state) - require.Error(t, err) - require.ErrorContains(t, err, "duplicate metadata resource: null_resource.about") + state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfPlan.PlannedValues.RootModule}, string(tfPlanGraph), logger) + require.Nil(t, state) + require.Error(t, err) + require.ErrorContains(t, err, "duplicate metadata resource: null_resource.about") + }) + + t.Run("ResourceID", func(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + ctx, logger := ctxAndLogger(t) + + dir := filepath.Join("testdata", "resources", "resource-id-provided") + tfStateRaw, err := os.ReadFile(filepath.Join(dir, "resource-id-provided.tfstate.json")) + require.NoError(t, err) + var tfState tfjson.State + err = json.Unmarshal(tfStateRaw, &tfState) + require.NoError(t, err) + tfStateGraph, err := os.ReadFile(filepath.Join(dir, "resource-id-provided.tfstate.dot")) + require.NoError(t, err) + + state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph), logger) + require.NoError(t, err) + require.Len(t, state.Resources, 2) + + // Find the resources + var firstResource, secondResource *proto.Resource + for _, res := range state.Resources { + if res.Name == "first" && res.Type == "null_resource" { + firstResource = res + } else if res.Name == "second" && res.Type == "null_resource" { + secondResource = res + } + } + + require.NotNil(t, firstResource) + require.NotNil(t, secondResource) + + // The metadata should be on the second resource (as specified by resource_id), + // not the first one (which is the closest in the graph) + require.Len(t, firstResource.Metadata, 0, "first resource should have no metadata") + require.Len(t, secondResource.Metadata, 1, "second resource should have metadata") + require.Equal(t, "test", secondResource.Metadata[0].Key) + require.Equal(t, "value", secondResource.Metadata[0].Value) + }) + + t.Run("NotFound", func(t *testing.T) { + t.Parallel() + ctx, logger := ctxAndLogger(t) + + dir := filepath.Join("testdata", "resources", "resource-id-not-found") + tfStateRaw, err := os.ReadFile(filepath.Join(dir, "resource-id-not-found.tfstate.json")) + require.NoError(t, err) + var tfState tfjson.State + err = json.Unmarshal(tfStateRaw, &tfState) + require.NoError(t, err) + tfStateGraph, err := os.ReadFile(filepath.Join(dir, "resource-id-not-found.tfstate.dot")) + require.NoError(t, err) + + state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{tfState.Values.RootModule}, string(tfStateGraph), logger) + require.NoError(t, err) + require.Len(t, state.Resources, 1) + + // The metadata should still be applied via graph traversal + require.Equal(t, "example", state.Resources[0].Name) + require.Len(t, state.Resources[0].Metadata, 1) + require.Equal(t, "test", state.Resources[0].Metadata[0].Key) + require.Equal(t, "value", state.Resources[0].Metadata[0].Value) + }) + }) } func TestParameterValidation(t *testing.T) { diff --git a/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tf b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tf new file mode 100644 index 0000000000000..6ccd83d6233a9 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tf @@ -0,0 +1,19 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + version = ">=2.0.0" + } + } +} + +resource "null_resource" "example" {} + +resource "coder_metadata" "example" { + resource_id = "non-existent-id" + depends_on = [null_resource.example] + item { + key = "test" + value = "value" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.dot b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.dot new file mode 100644 index 0000000000000..9a77a06ec3737 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.dot @@ -0,0 +1,17 @@ +digraph { + compound = "true" + newrank = "true" + subgraph "root" { + "[root] coder_metadata.example (expand)" [label = "coder_metadata.example", shape = "box"] + "[root] null_resource.example (expand)" [label = "null_resource.example", shape = "box"] + "[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"] + "[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"] + "[root] coder_metadata.example (expand)" -> "[root] null_resource.example (expand)" + "[root] coder_metadata.example (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]" + "[root] null_resource.example (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_metadata.example (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.example (expand)" + "[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" + "[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.json b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.json new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.dot b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.dot new file mode 100644 index 0000000000000..9a77a06ec3737 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.dot @@ -0,0 +1,17 @@ +digraph { + compound = "true" + newrank = "true" + subgraph "root" { + "[root] coder_metadata.example (expand)" [label = "coder_metadata.example", shape = "box"] + "[root] null_resource.example (expand)" [label = "null_resource.example", shape = "box"] + "[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"] + "[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"] + "[root] coder_metadata.example (expand)" -> "[root] null_resource.example (expand)" + "[root] coder_metadata.example (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]" + "[root] null_resource.example (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_metadata.example (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.example (expand)" + "[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" + "[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.json b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.json new file mode 100644 index 0000000000000..971a980fd975b --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.json @@ -0,0 +1,54 @@ +{ + "format_version": "1.0", + "terraform_version": "1.11.0", + "values": { + "root_module": { + "resources": [ + { + "address": "coder_metadata.example", + "mode": "managed", + "type": "coder_metadata", + "name": "example", + "provider_name": "registry.terraform.io/coder/coder", + "schema_version": 1, + "values": { + "daily_cost": null, + "hide": null, + "icon": null, + "id": "metadata-id", + "item": [ + { + "is_null": false, + "key": "test", + "sensitive": false, + "value": "value" + } + ], + "resource_id": "non-existent-id" + }, + "sensitive_values": { + "item": [ + {} + ] + }, + "depends_on": [ + "null_resource.example" + ] + }, + { + "address": "null_resource.example", + "mode": "managed", + "type": "null_resource", + "name": "example", + "provider_name": "registry.terraform.io/hashicorp/null", + "schema_version": 0, + "values": { + "id": "7576374697426687487", + "triggers": null + }, + "sensitive_values": {} + } + ] + } + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tf b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tf new file mode 100644 index 0000000000000..ba44bf172ce7f --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tf @@ -0,0 +1,21 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + version = ">=2.0.0" + } + } +} + +resource "null_resource" "first" {} + +resource "null_resource" "second" {} + +resource "coder_metadata" "example" { + resource_id = null_resource.second.id + depends_on = [null_resource.first] + item { + key = "test" + value = "value" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfplan.dot b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfplan.dot new file mode 100644 index 0000000000000..937a312445a06 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfplan.dot @@ -0,0 +1,21 @@ +digraph { + compound = "true" + newrank = "true" + subgraph "root" { + "[root] coder_metadata.example (expand)" [label = "coder_metadata.example", shape = "box"] + "[root] null_resource.first (expand)" [label = "null_resource.first", shape = "box"] + "[root] null_resource.second (expand)" [label = "null_resource.second", shape = "box"] + "[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"] + "[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"] + "[root] coder_metadata.example (expand)" -> "[root] null_resource.first (expand)" + "[root] coder_metadata.example (expand)" -> "[root] null_resource.second (expand)" + "[root] coder_metadata.example (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]" + "[root] null_resource.first (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] null_resource.second (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_metadata.example (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.first (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.second (expand)" + "[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" + "[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfplan.json b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfplan.json new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.dot b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.dot new file mode 100644 index 0000000000000..937a312445a06 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.dot @@ -0,0 +1,21 @@ +digraph { + compound = "true" + newrank = "true" + subgraph "root" { + "[root] coder_metadata.example (expand)" [label = "coder_metadata.example", shape = "box"] + "[root] null_resource.first (expand)" [label = "null_resource.first", shape = "box"] + "[root] null_resource.second (expand)" [label = "null_resource.second", shape = "box"] + "[root] provider[\"registry.terraform.io/coder/coder\"]" [label = "provider[\"registry.terraform.io/coder/coder\"]", shape = "diamond"] + "[root] provider[\"registry.terraform.io/hashicorp/null\"]" [label = "provider[\"registry.terraform.io/hashicorp/null\"]", shape = "diamond"] + "[root] coder_metadata.example (expand)" -> "[root] null_resource.first (expand)" + "[root] coder_metadata.example (expand)" -> "[root] null_resource.second (expand)" + "[root] coder_metadata.example (expand)" -> "[root] provider[\"registry.terraform.io/coder/coder\"]" + "[root] null_resource.first (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] null_resource.second (expand)" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"]" + "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" -> "[root] coder_metadata.example (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.first (expand)" + "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" -> "[root] null_resource.second (expand)" + "[root] root" -> "[root] provider[\"registry.terraform.io/coder/coder\"] (close)" + "[root] root" -> "[root] provider[\"registry.terraform.io/hashicorp/null\"] (close)" + } +} diff --git a/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.json b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.json new file mode 100644 index 0000000000000..ed30a9d927d08 --- /dev/null +++ b/provisioner/terraform/testdata/resources/resource-id-provided/resource-id-provided.tfstate.json @@ -0,0 +1,68 @@ +{ + "format_version": "1.0", + "terraform_version": "1.11.4", + "values": { + "root_module": { + "resources": [ + { + "address": "coder_metadata.example", + "mode": "managed", + "type": "coder_metadata", + "name": "example", + "provider_name": "registry.terraform.io/coder/coder", + "schema_version": 1, + "values": { + "daily_cost": null, + "hide": null, + "icon": null, + "id": "79e8977e-3277-4856-93a5-6200d9b8b156", + "item": [ + { + "is_null": false, + "key": "test", + "sensitive": false, + "value": "value" + } + ], + "resource_id": "6187209037590702578" + }, + "sensitive_values": { + "item": [ + {} + ] + }, + "depends_on": [ + "null_resource.first", + "null_resource.second" + ] + }, + { + "address": "null_resource.first", + "mode": "managed", + "type": "null_resource", + "name": "first", + "provider_name": "registry.terraform.io/hashicorp/null", + "schema_version": 0, + "values": { + "id": "529239707770856465", + "triggers": null + }, + "sensitive_values": {} + }, + { + "address": "null_resource.second", + "mode": "managed", + "type": "null_resource", + "name": "second", + "provider_name": "registry.terraform.io/hashicorp/null", + "schema_version": 0, + "values": { + "id": "6187209037590702578", + "triggers": null + }, + "sensitive_values": {} + } + ] + } + } +}