Skip to content

Commit 9d60f47

Browse files
sreyaThomasK33
sreya
authored and
ThomasK33
committed
fix: respect resource_id for coder_metadata
1 parent b200fc8 commit 9d60f47

File tree

2 files changed

+238
-26
lines changed

2 files changed

+238
-26
lines changed

‎provisioner/terraform/resources.go

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -684,41 +684,77 @@ func ConvertState(ctx context.Context, modules []*tfjson.StateModule, rawGraph s
684684
if err != nil {
685685
return nil, xerrors.Errorf("decode metadata attributes: %w", err)
686686
}
687-
resourceLabel := convertAddressToLabel(resource.Address)
688687

689-
var attachedNode *gographviz.Node
690-
for _, node := range graph.Nodes.Lookup {
691-
// The node attributes surround the label with quotes.
692-
if strings.Trim(node.Attrs["label"], `"`) != resourceLabel {
693-
continue
688+
var targetLabel string
689+
690+
// First, check if ResourceID is provided and try to find the resource by ID
691+
if attrs.ResourceID != "" {
692+
// Look for a resource with matching ID
693+
foundByID := false
694+
for label, tfResources := range tfResourcesByLabel {
695+
for _, tfResource := range tfResources {
696+
// Check if this resource's ID matches the ResourceID
697+
idAttr, hasID := tfResource.AttributeValues["id"]
698+
if hasID {
699+
idStr, ok := idAttr.(string)
700+
if ok && idStr == attrs.ResourceID {
701+
targetLabel = label
702+
foundByID = true
703+
break
704+
}
705+
}
706+
}
707+
if foundByID {
708+
break
709+
}
710+
}
711+
712+
// If we couldn't find by ID, fall back to graph traversal
713+
if !foundByID {
714+
logger.Warn(ctx, "coder_metadata resource_id not found, falling back to graph traversal",
715+
slog.F("resource_id", attrs.ResourceID),
716+
slog.F("metadata_address", resource.Address))
694717
}
695-
attachedNode = node
696-
break
697-
}
698-
if attachedNode == nil {
699-
continue
700718
}
701-
var attachedResource *graphResource
702-
for _, resource := range findResourcesInGraph(graph, tfResourcesByLabel, attachedNode.Name, 0, false) {
703-
if attachedResource == nil {
704-
// Default to the first resource because we have nothing to compare!
705-
attachedResource = resource
706-
continue
719+
720+
// If ResourceID wasn't provided or wasn't found, use graph traversal
721+
if targetLabel == "" {
722+
resourceLabel := convertAddressToLabel(resource.Address)
723+
724+
var attachedNode *gographviz.Node
725+
for _, node := range graph.Nodes.Lookup {
726+
// The node attributes surround the label with quotes.
727+
if strings.Trim(node.Attrs["label"], `"`) != resourceLabel {
728+
continue
729+
}
730+
attachedNode = node
731+
break
707732
}
708-
if resource.Depth < attachedResource.Depth {
709-
// There's a closer resource!
710-
attachedResource = resource
733+
if attachedNode == nil {
711734
continue
712735
}
713-
if resource.Depth == attachedResource.Depth && resource.Label < attachedResource.Label {
714-
attachedResource = resource
736+
var attachedResource *graphResource
737+
for _, resource := range findResourcesInGraph(graph, tfResourcesByLabel, attachedNode.Name, 0, false) {
738+
if attachedResource == nil {
739+
// Default to the first resource because we have nothing to compare!
740+
attachedResource = resource
741+
continue
742+
}
743+
if resource.Depth < attachedResource.Depth {
744+
// There's a closer resource!
745+
attachedResource = resource
746+
continue
747+
}
748+
if resource.Depth == attachedResource.Depth && resource.Label < attachedResource.Label {
749+
attachedResource = resource
750+
continue
751+
}
752+
}
753+
if attachedResource == nil {
715754
continue
716755
}
756+
targetLabel = attachedResource.Label
717757
}
718-
if attachedResource == nil {
719-
continue
720-
}
721-
targetLabel := attachedResource.Label
722758

723759
if metadataTargetLabels[targetLabel] {
724760
return nil, xerrors.Errorf("duplicate metadata resource: %s", targetLabel)

‎provisioner/terraform/resources_test.go

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1608,3 +1608,179 @@ func sortExternalAuthProviders(providers []*proto.ExternalAuthProviderResource)
16081608
return strings.Compare(providers[i].Id, providers[j].Id) == -1
16091609
})
16101610
}
1611+
1612+
func TestMetadataResourceID(t *testing.T) {
1613+
t.Parallel()
1614+
1615+
t.Run("UsesResourceIDWhenProvided", func(t *testing.T) {
1616+
t.Parallel()
1617+
ctx, logger := ctxAndLogger(t)
1618+
1619+
// Create a state with two resources and metadata that references the second one via resource_id
1620+
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
1621+
Resources: []*tfjson.StateResource{{
1622+
Address: "null_resource.first",
1623+
Type: "null_resource",
1624+
Name: "first",
1625+
Mode: tfjson.ManagedResourceMode,
1626+
AttributeValues: map[string]interface{}{
1627+
"id": "first-resource-id",
1628+
},
1629+
}, {
1630+
Address: "null_resource.second",
1631+
Type: "null_resource",
1632+
Name: "second",
1633+
Mode: tfjson.ManagedResourceMode,
1634+
AttributeValues: map[string]interface{}{
1635+
"id": "second-resource-id",
1636+
},
1637+
}, {
1638+
Address: "coder_metadata.example",
1639+
Type: "coder_metadata",
1640+
Name: "example",
1641+
Mode: tfjson.ManagedResourceMode,
1642+
DependsOn: []string{"null_resource.first"},
1643+
AttributeValues: map[string]interface{}{
1644+
"resource_id": "second-resource-id",
1645+
"item": []interface{}{
1646+
map[string]interface{}{
1647+
"key": "test",
1648+
"value": "value",
1649+
},
1650+
},
1651+
},
1652+
}},
1653+
}}, `digraph {
1654+
compound = "true"
1655+
newrank = "true"
1656+
subgraph "root" {
1657+
"[root] null_resource.first" [label = "null_resource.first", shape = "box"]
1658+
"[root] null_resource.second" [label = "null_resource.second", shape = "box"]
1659+
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
1660+
"[root] coder_metadata.example" -> "[root] null_resource.first"
1661+
}
1662+
}`, logger)
1663+
require.NoError(t, err)
1664+
require.Len(t, state.Resources, 2)
1665+
1666+
// Find the resources
1667+
var firstResource, secondResource *proto.Resource
1668+
for _, res := range state.Resources {
1669+
if res.Name == "first" && res.Type == "null_resource" {
1670+
firstResource = res
1671+
} else if res.Name == "second" && res.Type == "null_resource" {
1672+
secondResource = res
1673+
}
1674+
}
1675+
1676+
require.NotNil(t, firstResource)
1677+
require.NotNil(t, secondResource)
1678+
1679+
// The metadata should be on the second resource (as specified by resource_id),
1680+
// not the first one (which is the closest in the graph)
1681+
require.Len(t, firstResource.Metadata, 0, "first resource should have no metadata")
1682+
require.Len(t, secondResource.Metadata, 1, "second resource should have metadata")
1683+
require.Equal(t, "test", secondResource.Metadata[0].Key)
1684+
require.Equal(t, "value", secondResource.Metadata[0].Value)
1685+
})
1686+
1687+
t.Run("FallsBackToGraphWhenResourceIDNotFound", func(t *testing.T) {
1688+
t.Parallel()
1689+
ctx, logger := ctxAndLogger(t)
1690+
1691+
// Create a state where resource_id references a non-existent ID
1692+
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
1693+
Resources: []*tfjson.StateResource{{
1694+
Address: "null_resource.example",
1695+
Type: "null_resource",
1696+
Name: "example",
1697+
Mode: tfjson.ManagedResourceMode,
1698+
AttributeValues: map[string]interface{}{
1699+
"id": "example-resource-id",
1700+
},
1701+
}, {
1702+
Address: "coder_metadata.example",
1703+
Type: "coder_metadata",
1704+
Name: "example",
1705+
Mode: tfjson.ManagedResourceMode,
1706+
DependsOn: []string{"null_resource.example"},
1707+
AttributeValues: map[string]interface{}{
1708+
"resource_id": "non-existent-id",
1709+
"item": []interface{}{
1710+
map[string]interface{}{
1711+
"key": "test",
1712+
"value": "value",
1713+
},
1714+
},
1715+
},
1716+
}},
1717+
}}, `digraph {
1718+
compound = "true"
1719+
newrank = "true"
1720+
subgraph "root" {
1721+
"[root] null_resource.example" [label = "null_resource.example", shape = "box"]
1722+
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
1723+
"[root] coder_metadata.example" -> "[root] null_resource.example"
1724+
}
1725+
}`, logger)
1726+
require.NoError(t, err)
1727+
require.Len(t, state.Resources, 1)
1728+
1729+
// The metadata should still be applied via graph traversal
1730+
require.Equal(t, "example", state.Resources[0].Name)
1731+
require.Len(t, state.Resources[0].Metadata, 1)
1732+
require.Equal(t, "test", state.Resources[0].Metadata[0].Key)
1733+
require.Equal(t, "value", state.Resources[0].Metadata[0].Value)
1734+
1735+
// When resource_id is not found, it falls back to graph traversal
1736+
// We can't easily verify the warning was logged without access to the log capture API
1737+
})
1738+
1739+
t.Run("UsesGraphWhenResourceIDNotProvided", func(t *testing.T) {
1740+
t.Parallel()
1741+
ctx, logger := ctxAndLogger(t)
1742+
1743+
// Create a state without resource_id
1744+
state, err := terraform.ConvertState(ctx, []*tfjson.StateModule{{
1745+
Resources: []*tfjson.StateResource{{
1746+
Address: "null_resource.example",
1747+
Type: "null_resource",
1748+
Name: "example",
1749+
Mode: tfjson.ManagedResourceMode,
1750+
AttributeValues: map[string]interface{}{
1751+
"id": "example-resource-id",
1752+
},
1753+
}, {
1754+
Address: "coder_metadata.example",
1755+
Type: "coder_metadata",
1756+
Name: "example",
1757+
Mode: tfjson.ManagedResourceMode,
1758+
DependsOn: []string{"null_resource.example"},
1759+
AttributeValues: map[string]interface{}{
1760+
"item": []interface{}{
1761+
map[string]interface{}{
1762+
"key": "test",
1763+
"value": "value",
1764+
},
1765+
},
1766+
},
1767+
}},
1768+
}}, `digraph {
1769+
compound = "true"
1770+
newrank = "true"
1771+
subgraph "root" {
1772+
"[root] null_resource.example" [label = "null_resource.example", shape = "box"]
1773+
"[root] coder_metadata.example" [label = "coder_metadata.example", shape = "box"]
1774+
"[root] coder_metadata.example" -> "[root] null_resource.example"
1775+
}
1776+
}`, logger)
1777+
require.NoError(t, err)
1778+
require.Len(t, state.Resources, 1)
1779+
1780+
// The metadata should be applied via graph traversal
1781+
require.Equal(t, "example", state.Resources[0].Name)
1782+
require.Len(t, state.Resources[0].Metadata, 1)
1783+
require.Equal(t, "test", state.Resources[0].Metadata[0].Key)
1784+
require.Equal(t, "value", state.Resources[0].Metadata[0].Value)
1785+
})
1786+
}

0 commit comments

Comments
 (0)