Skip to content

fix: use resource_id directly for coder_metadata association #18300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

fix: use resource_id directly for coder_metadata association #18300

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9d60f47
fix: respect resource_id for coder_metadata
sreya Jun 10, 2025
1232eaa
cache id
sreya Jun 10, 2025
6182af4
update tests
sreya Jun 12, 2025
49a3a2b
fix tests
sreya Jun 12, 2025
f1dabf7
make fmt
sreya Jun 12, 2025
cfb02c7
remove comment
sreya Jun 12, 2025
3d92225
handle duplicate TF IDs
sreya Jun 12, 2025
a2afc35
undo deleted file
sreya Jun 12, 2025
7e0c0a2
ci: fix linter errors
ThomasK33 Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update tests
  • Loading branch information
@sreya @ThomasK33
sreya authored and ThomasK33 committed Aug 8, 2025
commit 6182af413e11d5b79d9cc1c821beaea88617da81
302 changes: 111 additions & 191 deletions provisioner/terraform/resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1249,24 +1249,120 @@ 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("ResourceIDProvided", 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("ResourceIDNotFound", 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)

// When resource_id is not found, it falls back to graph traversal
// We can't easily verify the warning was logged without access to the log capture API
})

t.Run("ResourceIDNotProvided", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

dir := filepath.Join("testdata", "resources", "resource-id-not-provided")
tfStateRaw, err := os.ReadFile(filepath.Join(dir, "resource-id-not-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-not-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, 1)

// The metadata should 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) {
Expand Down Expand Up @@ -1608,179 +1704,3 @@ func sortExternalAuthProviders(providers []*proto.ExternalAuthProviderResource)
return strings.Compare(providers[i].Id, providers[j].Id) == -1
})
}

func TestMetadataResourceID(t *testing.T) {
t.Parallel()

t.Run("UsesResourceIDWhenProvided", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// Create a state with two resources and metadata that references the second one via resource_id
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
Resources: []*tfjson.StateResource{{
Address: "null_resource.first",
Type: "null_resource",
Name: "first",
Mode: tfjson.ManagedResourceMode,
AttributeValues: map[string]interface{}{
"id": "first-resource-id",
},
}, {
Address: "null_resource.second",
Type: "null_resource",
Name: "second",
Mode: tfjson.ManagedResourceMode,
AttributeValues: map[string]interface{}{
"id": "second-resource-id",
},
}, {
Address: "coder_metadata.example",
Type: "coder_metadata",
Name: "example",
Mode: tfjson.ManagedResourceMode,
DependsOn: []string{"null_resource.first"},
AttributeValues: map[string]interface{}{
"resource_id": "second-resource-id",
"item": []interface{}{
map[string]interface{}{
"key": "test",
"value": "value",
},
},
},
}},
}}, `digraph {
compound = "true"
newrank = "true"
subgraph "root" {
"[root] null_resource.first" [label = "null_resource.first", shape = "box"]
"[root] null_resource.second" [label = "null_resource.second", shape = "box"]
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
"[root] coder_metadata.example" -> "[root] null_resource.first"
}
}`, 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("FallsBackToGraphWhenResourceIDNotFound", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// Create a state where resource_id references a non-existent ID
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
Resources: []*tfjson.StateResource{{
Address: "null_resource.example",
Type: "null_resource",
Name: "example",
Mode: tfjson.ManagedResourceMode,
AttributeValues: map[string]interface{}{
"id": "example-resource-id",
},
}, {
Address: "coder_metadata.example",
Type: "coder_metadata",
Name: "example",
Mode: tfjson.ManagedResourceMode,
DependsOn: []string{"null_resource.example"},
AttributeValues: map[string]interface{}{
"resource_id": "non-existent-id",
"item": []interface{}{
map[string]interface{}{
"key": "test",
"value": "value",
},
},
},
}},
}}, `digraph {
compound = "true"
newrank = "true"
subgraph "root" {
"[root] null_resource.example" [label = "null_resource.example", shape = "box"]
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
"[root] coder_metadata.example" -> "[root] null_resource.example"
}
}`, 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)

// When resource_id is not found, it falls back to graph traversal
// We can't easily verify the warning was logged without access to the log capture API
})

t.Run("UsesGraphWhenResourceIDNotProvided", func(t *testing.T) {
t.Parallel()
ctx, logger := ctxAndLogger(t)

// Create a state without resource_id
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
Resources: []*tfjson.StateResource{{
Address: "null_resource.example",
Type: "null_resource",
Name: "example",
Mode: tfjson.ManagedResourceMode,
AttributeValues: map[string]interface{}{
"id": "example-resource-id",
},
}, {
Address: "coder_metadata.example",
Type: "coder_metadata",
Name: "example",
Mode: tfjson.ManagedResourceMode,
DependsOn: []string{"null_resource.example"},
AttributeValues: map[string]interface{}{
"item": []interface{}{
map[string]interface{}{
"key": "test",
"value": "value",
},
},
},
}},
}}, `digraph {
compound = "true"
newrank = "true"
subgraph "root" {
"[root] null_resource.example" [label = "null_resource.example", shape = "box"]
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
"[root] coder_metadata.example" -> "[root] null_resource.example"
}
}`, logger)
require.NoError(t, err)
require.Len(t, state.Resources, 1)

// The metadata should 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)
})
}
19 changes: 19 additions & 0 deletions provisioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
}
9 changes: 9 additions & 0 deletions ...ioner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.dot
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions ...oner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfplan.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions ...oner/terraform/testdata/resources/resource-id-not-found/resource-id-not-found.tfstate.dot
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading