From cba012deaed98a8c7dfe2e9163105c36e2ff2bc1 Mon Sep 17 00:00:00 2001 From: magodo Date: Fri, 25 Jul 2025 12:43:44 +1000 Subject: [PATCH 1/4] tidy up code --- internal/meta/base_meta.go | 17 +- internal/meta/config_info.go | 244 +++++++- .../config_info_dependencies_utils_test.go | 65 --- ...info_populate_parent_child_dependencies.go | 58 -- ...populate_parent_child_dependencies_test.go | 216 ------- ...ig_info_populate_reference_dependencies.go | 83 --- ...fo_populate_reference_dependencies_test.go | 166 ------ internal/meta/config_info_test.go | 541 ++++++++++++++++++ internal/meta/hcl_edit.go | 93 --- internal/meta/hcl_edit_test.go | 259 --------- 10 files changed, 781 insertions(+), 961 deletions(-) delete mode 100644 internal/meta/config_info_dependencies_utils_test.go delete mode 100644 internal/meta/config_info_populate_parent_child_dependencies.go delete mode 100644 internal/meta/config_info_populate_parent_child_dependencies_test.go delete mode 100644 internal/meta/config_info_populate_reference_dependencies.go delete mode 100644 internal/meta/config_info_populate_reference_dependencies_test.go create mode 100644 internal/meta/config_info_test.go delete mode 100644 internal/meta/hcl_edit_test.go diff --git a/internal/meta/base_meta.go b/internal/meta/base_meta.go index 6596721..80b777a 100644 --- a/internal/meta/base_meta.go +++ b/internal/meta/base_meta.go @@ -1092,11 +1092,10 @@ func (meta baseMeta) stateToConfig(ctx context.Context, list ImportList) (Config } out = append(out, ConfigInfo{ ImportItem: importedList[i], - hcl: f, - dependencies: Dependencies{ - refDeps: make(map[string]Dependency), - parentChildDeps: make(map[Dependency]bool), - ambiguousRefDeps: make(map[string][]Dependency), + HCL: f, + Dependencies: Dependencies{ + ByRef: make(map[string]Dependency), + ByRefAmbiguous: make(map[string][]Dependency), }, }) } @@ -1148,7 +1147,7 @@ func (meta baseMeta) lifecycleAddon(configs ConfigInfos) (ConfigInfos, error) { for i, cfg := range configs { switch cfg.TFAddr.Type { case "azurerm_application_insights_web_test": - if err := hclBlockAppendLifecycle(cfg.hcl.Body().Blocks()[0].Body(), []string{"tags"}); err != nil { + if err := hclBlockAppendLifecycle(cfg.HCL.Body().Blocks()[0].Body(), []string{"tags"}); err != nil { return nil, fmt.Errorf("azurerm_application_insights_web_test: %v", err) } } @@ -1158,12 +1157,12 @@ func (meta baseMeta) lifecycleAddon(configs ConfigInfos) (ConfigInfos, error) { } func (meta baseMeta) addDependency(configs ConfigInfos) (ConfigInfos, error) { - if err := configs.PopulateReferenceDependencies(); err != nil { + if err := configs.PopulateReferenceDeps(); err != nil { return nil, fmt.Errorf("populating reference dependencies: %v", err) } - configs.populateParentChildDependency() + configs.PopulateRelationDeps() - if err := configs.applyDependenciesToHclBlock(); err != nil { + if err := configs.ApplyDepsToHCL(); err != nil { return nil, fmt.Errorf("applying dependencies to HCL blocks: %v", err) } diff --git a/internal/meta/config_info.go b/internal/meta/config_info.go index fc74589..a2a30d1 100644 --- a/internal/meta/config_info.go +++ b/internal/meta/config_info.go @@ -1,10 +1,16 @@ package meta import ( + "fmt" "io" + "sort" + "strings" "github.com/Azure/aztfexport/internal/tfaddr" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/zclconf/go-cty/cty" ) type ConfigInfos []ConfigInfo @@ -12,29 +18,123 @@ type ConfigInfos []ConfigInfo type ConfigInfo struct { ImportItem - dependencies Dependencies + Dependencies Dependencies - hcl *hclwrite.File + HCL *hclwrite.File } func (cfg ConfigInfo) DumpHCL(w io.Writer) (int, error) { - out := hclwrite.Format(cfg.hcl.Bytes()) + out := hclwrite.Format(cfg.HCL.Bytes()) return w.Write(out) } +func (cfg *ConfigInfo) applyRefDepsToHCL() { + var applyF func(*hclwrite.Body, map[string]Dependency) + applyF = func(body *hclwrite.Body, deps map[string]Dependency) { + if len(deps) == 0 { + return + } + for name, attr := range body.Attributes() { + tokens := attr.Expr().BuildTokens(nil) + newTokens := hclwrite.Tokens{} + tokensModified := false + + for i := 0; i < len(tokens); i++ { + refDep, refDepExists := deps[string(tokens[i].Bytes)] + // Parsing process guaranteed QuotedLit is surrounded by Opening and Closing quote + if tokens[i].Type == hclsyntax.TokenQuotedLit && refDepExists { + newTokens[len(newTokens)-1] = &hclwrite.Token{ + Type: hclsyntax.TokenIdent, + Bytes: fmt.Appendf(nil, "%s.id", refDep.TFAddr), + SpacesBefore: tokens[i-1].SpacesBefore, + } + tokensModified = true + i += 1 // Skip the next token as it was already processed + } else { + newTokens = append(newTokens, tokens[i]) + } + } + + if tokensModified { + body.SetAttributeRaw(name, newTokens) + } + for _, nestedBlock := range body.Blocks() { + applyF(nestedBlock.Body(), deps) + } + } + } + applyF(cfg.HCL.Body(), cfg.Dependencies.ByRef) +} + +func (cfg *ConfigInfo) applyExplicitDepsToHCL() error { + body := cfg.HCL.Body() + + relationDep := cfg.Dependencies.ByRelation + if relationDep != nil { + // Skip this relation dependency if it's already covered by any of the other applied dependencies. + var otherDepAzureIds []string + for _, dep := range cfg.Dependencies.ByRef { + otherDepAzureIds = append(otherDepAzureIds, dep.AzureResourceId) + } + var covered bool + for _, dep := range otherDepAzureIds { + if strings.HasPrefix(dep, relationDep.AzureResourceId) { + covered = true + break + } + } + if covered { + relationDep = nil + } + } + + ambiguousDeps := cfg.Dependencies.ByRefAmbiguous + // TODO: Skip the ambiguous depedencies that are covered by any of the other applied Dependencies. + + if len(ambiguousDeps) == 0 && relationDep == nil { + return nil + } + + src := "depends_on = [\n" + if relationDep != nil { + src += relationDep.TFAddr.String() + ",\n" + } + if len(ambiguousDeps) > 0 { + ambiguousDepsComments := make([]string, 0, len(ambiguousDeps)) + for _, deps := range ambiguousDeps { + tfAddrs := make([]string, 0, len(deps)) + for _, dep := range deps { + tfAddrs = append(tfAddrs, dep.TFAddr.String()) + } + sort.Strings(tfAddrs) + ambiguousDepsComments = append(ambiguousDepsComments, fmt.Sprintf("# One of %s (can't auto-resolve as their ids are identical)", strings.Join(tfAddrs, ","))) + } + sort.Strings(ambiguousDepsComments) + src += strings.Join(ambiguousDepsComments, "\n") + "\n" + } + src += "]\n" + expr, diags := hclwrite.ParseConfig([]byte(src), "f", hcl.InitialPos) + if diags.HasErrors() { + return fmt.Errorf(`building "depends_on" attribute: %s`, diags.Error()) + } + body.SetAttributeRaw("depends_on", expr.Body().GetAttribute("depends_on").Expr().BuildTokens(nil)) + + return nil +} + type Dependencies struct { - // Dependencies inferred by scanning for resource id values, will be applied by substituting with TF address - // Key is TFResourceId - refDeps map[string]Dependency + // Dependencies inferred by scanning for resource id values + // The key is TFResourceId. + ByRef map[string]Dependency - // Similar to refDeps, but due to multiple Azure resources can map to a same TF resource id, we can't decide which Azure resource - // is depended on. Hence these will end up as comments inside "depends_on" block for the user to manually resolve. + // Similar to ByRef, but due to multiple Azure resources can map to a same TF resource id (being referenced), + // this is regarded as ambiguous references. // The key is TFResourceId. - ambiguousRefDeps map[string][]Dependency + ByRefAmbiguous map[string][]Dependency - // Dependencies inferred via Azure resource id parent lookup, and will be applied in the "depends_on" block. - // Especially, any dependency that is (transitively) present via refDepds will be filtered. - parentChildDeps map[Dependency]bool + // Dependencies inferred via Azure resource id parent lookup. + // At most one such dependency can exist. + ByRelation *Dependency } type Dependency struct { @@ -42,3 +142,123 @@ type Dependency struct { AzureResourceId string TFAddr tfaddr.TFAddr } + +// Look at the Azure resource id and determine if parent dependency exist. +// For example, /subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foos/foo1 +// has a parent /subscriptions/123/resourceGroups/rg1, which is the resource group. +func (cfgs ConfigInfos) PopulateRelationDeps() { + for i, cfg := range cfgs { + parentId := cfg.AzureResourceID.Parent() + + // This resource is either a root scope or a RP id (which doesn't exist) + if parentId == nil { + continue + } + + // This resource is the first level resource under the (root) scope. + // E.g. + // - (root scoped) /subscriptions/sub1/foos/foo + // - (scoped) /subscriptions/sub1/providers/Microsoft.Foo/foos/foo1 + // Regard the parent scope as its parent. + if parentId.Parent() == nil { + parentId = cfg.AzureResourceID.ParentScope() + } + + // Adding the direct parent resource as its dependency + for _, ocfg := range cfgs { + if parentId.Equal(ocfg.AzureResourceID) { + cfg.Dependencies.ByRelation = &Dependency{ + TFResourceId: ocfg.TFResourceId, + AzureResourceId: ocfg.AzureResourceID.String(), + TFAddr: ocfg.TFAddr, + } + break + } + } + cfgs[i] = cfg + } +} + +// Scan the HCL files for references to other resources. +// For example the HCL attribute `foo_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foos/foo1"` +// will yield a dependency to that foo TF resource address. +// Note that a single TF resource id can map to multiple resources, in which case the dependencies is regarded as ambiguous. +func (cfgs ConfigInfos) PopulateReferenceDeps() error { + // key: TFResourceId + m := map[string][]*ConfigInfo{} + for _, cfg := range cfgs { + m[cfg.TFResourceId] = append(m[cfg.TFResourceId], &cfg) + } + + for i, cfg := range cfgs { + file, err := hclsyntax.ParseConfig(cfg.HCL.Bytes(), "main.tf", hcl.InitialPos) + if err != nil { + return fmt.Errorf("parsing hcl for %s: %v", cfg.AzureResourceID, err) + } + hclsyntax.VisitAll(file.Body.(*hclsyntax.Body), func(node hclsyntax.Node) hcl.Diagnostics { + expr, ok := node.(*hclsyntax.LiteralValueExpr) + if !ok { + return nil + } + val := expr.Val + if !expr.Val.IsKnown() || !val.Type().Equals(cty.String) { + return nil + } + maybeTFId := val.AsString() + + // Try to look up this string attribute from the TF id map. If there is a match, we regard it as a valid TF resource id. + // This is safe to match case sensitively given the TF id are consistent across the provider. Otherwise, it is a provider bug. + dependingConfigsRaw, ok := m[maybeTFId] + if !ok { + return nil + } + depTFResId := maybeTFId + + var dependingConfigs []*ConfigInfo + for _, depCfg := range dependingConfigsRaw[:] { + // Ignore the self dependency + if cfg.AzureResourceID.String() == depCfg.AzureResourceID.String() { + continue + } + // Ignore the dependency on the child resource (which will cause circular dependency) + if cfg.AzureResourceID.Equal(depCfg.AzureResourceID.Parent()) { + continue + } + dependingConfigs = append(dependingConfigs, depCfg) + } + + if len(dependingConfigs) == 1 { + cfg.Dependencies.ByRef[depTFResId] = Dependency{ + TFResourceId: depTFResId, + AzureResourceId: dependingConfigs[0].AzureResourceID.String(), + TFAddr: dependingConfigs[0].TFAddr, + } + } else if len(dependingConfigs) > 1 { + deps := make([]Dependency, 0, len(dependingConfigs)) + for _, depCfg := range dependingConfigs { + deps = append(deps, Dependency{ + TFResourceId: depTFResId, + AzureResourceId: depCfg.AzureResourceID.String(), + TFAddr: depCfg.TFAddr, + }) + } + cfg.Dependencies.ByRefAmbiguous[depTFResId] = deps + } + + return nil + }) + cfgs[i] = cfg + } + return nil +} + +func (configs ConfigInfos) ApplyDepsToHCL() error { + for i, cfg := range configs { + cfg.applyRefDepsToHCL() + if err := cfg.applyExplicitDepsToHCL(); err != nil { + return fmt.Errorf("applying explicit dependencies to %s: %w", cfg.TFResourceId, err) + } + configs[i] = cfg + } + return nil +} diff --git a/internal/meta/config_info_dependencies_utils_test.go b/internal/meta/config_info_dependencies_utils_test.go deleted file mode 100644 index ef1f35b..0000000 --- a/internal/meta/config_info_dependencies_utils_test.go +++ /dev/null @@ -1,65 +0,0 @@ -package meta - -import ( - "fmt" - - "github.com/Azure/aztfexport/internal/tfaddr" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclwrite" - "github.com/magodo/armid" -) - -func mustParseTFAddr(s string) tfaddr.TFAddr { - tfAddr, err := tfaddr.ParseTFResourceAddr(s) - if err != nil { - panic(fmt.Sprintf("failed to parse TF resource address %s: %v", s, err)) - } - return *tfAddr -} - -func configInfoWithDeps( - azureResourceIdStr string, - tFResourceId string, - tfAddr tfaddr.TFAddr, - hclStr string, - refDeps map[string]Dependency, - ambiguousDeps map[string][]Dependency, -) ConfigInfo { - azureResourceId, err := armid.ParseResourceId(string(azureResourceIdStr)) - if err != nil { - panic(fmt.Sprintf("failed to parse Azure resource ID %s: %v", azureResourceIdStr, err)) - } - hcl, diag := hclwrite.ParseConfig([]byte(hclStr), "main.tf", hcl.InitialPos) - if diag.HasErrors() { - panic(fmt.Sprintf("failed to parse HCL for Azure resource ID %s: %v", azureResourceIdStr, diag)) - } - return ConfigInfo{ - ImportItem: ImportItem{ - AzureResourceID: azureResourceId, - TFResourceId: tFResourceId, - TFAddr: tfAddr, - }, - dependencies: Dependencies{ - refDeps: refDeps, - parentChildDeps: make(map[Dependency]bool), - ambiguousRefDeps: ambiguousDeps, - }, - hcl: hcl, - } -} - -func configInfo( - azureResourceIdStr string, - tFResourceId string, - tfAddr tfaddr.TFAddr, - hclStr string, -) ConfigInfo { - return configInfoWithDeps( - azureResourceIdStr, - tFResourceId, - tfAddr, - hclStr, - make(map[string]Dependency), - make(map[string][]Dependency), - ) -} diff --git a/internal/meta/config_info_populate_parent_child_dependencies.go b/internal/meta/config_info_populate_parent_child_dependencies.go deleted file mode 100644 index 1815aa6..0000000 --- a/internal/meta/config_info_populate_parent_child_dependencies.go +++ /dev/null @@ -1,58 +0,0 @@ -package meta - -import ( - "strings" - - "github.com/magodo/armid" -) - -// Look at the resource id and determine if parent dependency exist. -// For example, /subscriptions/123/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachines/vm1 -// has a parent /subscriptions/123/resourceGroups/rg1, which is the resource group. -// Unless present as reference dependency (maybe transitively), this parent will be added as an explicit dependency -// (via depends_on meta arg). -func (cfgs ConfigInfos) populateParentChildDependency() { - for i, cfg := range cfgs { - parentId := cfg.AzureResourceID.Parent() - - // This resource is either a root scope or a root scoped resource - if parentId == nil { - // Root scope: ignore as it has no parent - if cfg.AzureResourceID.ParentScope() == nil { - continue - } - // Root scoped resource: use its parent scope as its parent - parentId = cfg.AzureResourceID.ParentScope() - } else if parentId.Parent() == nil { - // The cfg resource is the RP 1st level resource, we regard its parent scope as its parent - parentId = cfg.AzureResourceID.ParentScope() - } - - // Adding the direct parent resource as its dependency - for _, ocfg := range cfgs { - if cfg.AzureResourceID.Equal(ocfg.AzureResourceID) { - continue - } - if parentId.Equal(ocfg.AzureResourceID) && - // Only add parent dependency if it is not already (maybe transitively) a reference dependency. - !hasReferenceDepWithPrefix(cfg.dependencies.refDeps, ocfg.AzureResourceID) { - cfg.dependencies.parentChildDeps[Dependency{ - TFResourceId: ocfg.TFResourceId, - AzureResourceId: ocfg.AzureResourceID.String(), - TFAddr: ocfg.TFAddr, - }] = true - break - } - } - cfgs[i] = cfg - } -} - -func hasReferenceDepWithPrefix(refDeps map[string]Dependency, prefix armid.ResourceId) bool { - for _, dep := range refDeps { - if strings.HasPrefix(dep.AzureResourceId, prefix.String()) { - return true - } - } - return false -} diff --git a/internal/meta/config_info_populate_parent_child_dependencies_test.go b/internal/meta/config_info_populate_parent_child_dependencies_test.go deleted file mode 100644 index 51f39c4..0000000 --- a/internal/meta/config_info_populate_parent_child_dependencies_test.go +++ /dev/null @@ -1,216 +0,0 @@ -package meta - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestPopulateParentChildDependencies(t *testing.T) { - testCases := []struct { - name string - inputConfigs ConfigInfos - expectedParentChildDeps map[string]map[Dependency]bool // key: AzureResourceId - }{ - { - name: "no parent-child relationships", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-0"), - ` -resource "azurerm_foo_resource" "res-0" { - name = "foo1" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - mustParseTFAddr("azurerm_bar_resource.res-1"), - ` -resource "azurerm_bar_resource" "res-1" { - name = "bar1" -} -`, - ), - }, - expectedParentChildDeps: map[string]map[Dependency]bool{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, - }, - }, - { - name: "res-0 is a parent of res-1: expect explicit dep from res-1 to res-0", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1", - "/subscriptions/123/resourceGroups/rg1", - mustParseTFAddr("azurerm_resource_group.res-0"), - ` -resource "azurerm_resource_group" "res-0" { - name = "rg1" - location = "West Europe" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-1"), - ` -resource "azurerm_foo_resource" "res-1" { - name = "foo1" - resource_group_name = "rg1" -} -`, - ), - }, - expectedParentChildDeps: map[string]map[Dependency]bool{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { - { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1", - }: true, - }, - "/subscriptions/123/resourceGroups/rg1": {}, - }, - }, - { - name: "res-2 -> res-1 -> res-0 connected by reference dependency, res-2 is child of res-0: expect no explicit dep because it has been satisfied transitively by reference dep", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1", - "/subscriptions/123/resourceGroups/rg1", - mustParseTFAddr("azurerm_resource_group.res-0"), - ` -resource "azurerm_resource_group" "res-0" { - name = "rg1" - location = "West Europe" -} -`, - ), - configInfoWithDeps( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-1"), - ` -resource "azurerm_foo_resource" "res-1" { - name = "foo1" - resource_group_id = "/subscriptions/123/resourceGroups/rg1" -} -`, - map[string]Dependency{ - "/subscriptions/123/resourceGroups/rg1": { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1", - }, - }, - make(map[string][]Dependency), - ), - configInfoWithDeps( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - mustParseTFAddr("azurerm_bar_resource.res-2"), - ` -resource "azurerm_bar_resource" "res-2" { - name = "bar1" - foo_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1" -} -`, - map[string]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }, - }, - make(map[string][]Dependency), - ), - }, - expectedParentChildDeps: map[string]map[Dependency]bool{ - "/subscriptions/123/resourceGroups/rg1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, - }, - }, - { - name: "res-0 and res-1 are ambiguous (different azureResourceId, same tfResourceId), res-2 is child of res-0, res-2 has ambiguous refDep to res-0 and res-1: expect parentChildDep to be added to res-2", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_sub1_resource.res-0"), - ` -resource "azurerm_foo_sub1_resource" "res-0" { - name = "res0" - location = "West Europe" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_sub2_resource.res-1"), - ` -resource "azurerm_foo_sub2_resource" "res-1" { - name = "res1" - location = "West Europe" -} -`, - ), - configInfoWithDeps( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1/deep1/deep1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1/deep1/deep1", - mustParseTFAddr("azurerm_foo_sub1_deep1_resource.res-2"), - ` -resource "azurerm_foo_sub1_deep1_resource" "res-2" { - name = "res2" - foo_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1" -} -`, - map[string]Dependency{}, - map[string][]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { - { - TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }, - { - TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }, - }, - }, - ), - }, - expectedParentChildDeps: map[string]map[Dependency]bool{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1/deep1/deep1": { - { - TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }: true, - }, - }, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - testCase.inputConfigs.populateParentChildDependency() - for _, cfg := range testCase.inputConfigs { - azureResourceId := cfg.AzureResourceID.String() - expectedExplicitDeps := testCase.expectedParentChildDeps[azureResourceId] - assert.Equal(t, expectedExplicitDeps, cfg.dependencies.parentChildDeps, "parentChildDeps matches expectation, azureResourceId: %s", azureResourceId) - } - }) - } -} diff --git a/internal/meta/config_info_populate_reference_dependencies.go b/internal/meta/config_info_populate_reference_dependencies.go deleted file mode 100644 index 0065053..0000000 --- a/internal/meta/config_info_populate_reference_dependencies.go +++ /dev/null @@ -1,83 +0,0 @@ -package meta - -import ( - "fmt" - - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" - "github.com/zclconf/go-cty/cty" -) - -// Scan the HCL files for references to other resources. -// For example the HCL attribute `key_vault_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.KeyVault/vaults/vault1"` -// will yield a dependency to the TF resource with address `azurerm_key_vault.vault1`. -// Note that a single TF resource id can map to multiple resources -- in which case the dependencies will be categorised -// as ambiguous. -func (cfgs ConfigInfos) PopulateReferenceDependencies() error { - // key: TFResourceId - m := map[string][]*ConfigInfo{} - for _, cfg := range cfgs { - m[cfg.TFResourceId] = append(m[cfg.TFResourceId], &cfg) - } - - for i, cfg := range cfgs { - file, err := hclsyntax.ParseConfig(cfg.hcl.Bytes(), "main.tf", hcl.InitialPos) - if err != nil { - return fmt.Errorf("parsing hcl for %s: %v", cfg.AzureResourceID, err) - } - hclsyntax.VisitAll(file.Body.(*hclsyntax.Body), func(node hclsyntax.Node) hcl.Diagnostics { - expr, ok := node.(*hclsyntax.LiteralValueExpr) - if !ok { - return nil - } - val := expr.Val - if !expr.Val.IsKnown() || !val.Type().Equals(cty.String) { - return nil - } - maybeTFId := val.AsString() - - // This is safe to match case sensitively given the TF id are consistent across the provider. Otherwise, it is a provider bug. - dependingConfigsRaw, ok := m[maybeTFId] - if !ok { - return nil - } - - depTFResId := maybeTFId - - var dependingConfigs []*ConfigInfo - for _, depCfg := range dependingConfigsRaw[:] { - // Ignore the self dependency - if cfg.AzureResourceID.String() == depCfg.AzureResourceID.String() { - continue - } - // Ignore the dependency on the child resource (which will cause circular dependency) - if cfg.AzureResourceID.Equal(depCfg.AzureResourceID.Parent()) { - continue - } - dependingConfigs = append(dependingConfigs, depCfg) - } - - if len(dependingConfigs) == 1 { - cfg.dependencies.refDeps[depTFResId] = Dependency{ - TFResourceId: depTFResId, - AzureResourceId: dependingConfigs[0].AzureResourceID.String(), - TFAddr: dependingConfigs[0].TFAddr, - } - } else if len(dependingConfigs) > 1 { - deps := make([]Dependency, 0, len(dependingConfigs)) - for _, depCfg := range dependingConfigs { - deps = append(deps, Dependency{ - TFResourceId: depTFResId, - AzureResourceId: depCfg.AzureResourceID.String(), - TFAddr: depCfg.TFAddr, - }) - } - cfg.dependencies.ambiguousRefDeps[depTFResId] = deps - } - - return nil - }) - cfgs[i] = cfg - } - return nil -} diff --git a/internal/meta/config_info_populate_reference_dependencies_test.go b/internal/meta/config_info_populate_reference_dependencies_test.go deleted file mode 100644 index a3dcf58..0000000 --- a/internal/meta/config_info_populate_reference_dependencies_test.go +++ /dev/null @@ -1,166 +0,0 @@ -package meta - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestPopulateReferenceDependencies(t *testing.T) { - testCases := []struct { - name string - inputConfigs ConfigInfos - expectedReferenceDeps map[string]map[string]Dependency // key: AzureResourceId, inner key: TFResourceId - expectedAmbiguousDeps map[string]map[string][]Dependency // key: AzureResourceId, inner key: TFResourceId - }{ - { - name: "no dependencies between resources", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-0"), - ` -resource "azurerm_foo_resource" "res-0" { - name = "foo1" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - mustParseTFAddr("azurerm_bar_resource.res-1"), - ` -resource "azurerm_bar_resource" "res-1" { - name = "bar1" -} -`, - ), - }, - expectedReferenceDeps: map[string]map[string]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, - }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, - }, - }, - { - name: "res-0 is a resource group, rites-1 refer to it by id: expect reference dep from res-1 to res-0", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1", - "/subscriptions/123/resourceGroups/rg1", - mustParseTFAddr("azurerm_resource_group.res-0"), - ` -resource "azurerm_resource_group" "res-0" { - name = "rg1" - location = "West Europe" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-1"), - ` -resource "azurerm_foo_resource" "res-1" { - name = "foo1" - resource_group_id = "/subscriptions/123/resourceGroups/rg1" -} -`, - ), - }, - expectedReferenceDeps: map[string]map[string]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { - "/subscriptions/123/resourceGroups/rg1": { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1", - }, - }, - "/subscriptions/123/resourceGroups/rg1": make(map[string]Dependency), - }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1": {}, - }, - }, - { - name: "res-0 and res-1 have different azure resource id, but same TF resource id, res-2 refer to this TF resource id: expect res-2 to have ambiguous dep to the TF resource id", - inputConfigs: []ConfigInfo{ - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-0"), - ` -resource "azurerm_foo_sub1_resource" "res-0" { - name = "foo1_sub1" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - mustParseTFAddr("azurerm_foo_resource.res-1"), - ` -resource "azurerm_foo_sub2_resource" "res-1" { - name = "foo1_sub2" -} -`, - ), - configInfo( - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", - mustParseTFAddr("azurerm_bar_resource.res-2"), - ` -resource "azurerm_bar_resource" "res-2" { - name = "bar1" - foo_resource_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1" -} -`, - ), - }, - expectedReferenceDeps: map[string]map[string]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, - }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": { - "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { - { - TFAddr: mustParseTFAddr("azurerm_foo_resource.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }, - { - TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", - TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", - }, - }, - }, - }, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - err := testCase.inputConfigs.PopulateReferenceDependencies() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - for _, cfg := range testCase.inputConfigs { - expectedRefDeps := testCase.expectedReferenceDeps[cfg.AzureResourceID.String()] - assert.Equal(t, expectedRefDeps, cfg.dependencies.refDeps, "referenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) - expectedAmbiguousRefDeps := testCase.expectedAmbiguousDeps[cfg.AzureResourceID.String()] - assert.Equal(t, expectedAmbiguousRefDeps, cfg.dependencies.ambiguousRefDeps, "ambiguousDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) - } - }) - } -} diff --git a/internal/meta/config_info_test.go b/internal/meta/config_info_test.go new file mode 100644 index 0000000..9041c14 --- /dev/null +++ b/internal/meta/config_info_test.go @@ -0,0 +1,541 @@ +package meta + +import ( + "fmt" + "testing" + + "github.com/Azure/aztfexport/internal/tfaddr" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/stretchr/testify/assert" +) + +func newConfigInfo(azid, tfid, tfaddr, hcl string, deps *Dependencies) ConfigInfo { + cinfo := ConfigInfo{ + ImportItem: ImportItem{ + AzureResourceID: mustParseResourceId(azid), + TFResourceId: tfid, + TFAddr: mustParseTFAddr(tfaddr), + }, + HCL: mustHclWriteParse(hcl), + Dependencies: Dependencies{ + ByRef: make(map[string]Dependency), + ByRefAmbiguous: make(map[string][]Dependency), + }, + } + if deps != nil { + cinfo.Dependencies = *deps + } + return cinfo +} + +func TestConfigInfos_PopulateReferenceDeps(t *testing.T) { + testCases := []struct { + name string + inputConfigs ConfigInfos + expectedReferenceDeps map[string]map[string]Dependency // key: AzureResourceId, inner key: TFResourceId + expectedAmbiguousDeps map[string]map[string][]Dependency // key: AzureResourceId, inner key: TFResourceId + }{ + { + name: "no dependencies between resources", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-0", + ` +resource "azurerm_foo_resource" "res-0" { + name = "foo1" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "azurerm_bar_resource.res-1", + ` +resource "azurerm_bar_resource" "res-1" { + name = "bar1" +} +`, + nil, + ), + }, + expectedReferenceDeps: map[string]map[string]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, + }, + expectedAmbiguousDeps: map[string]map[string][]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, + }, + }, + { + name: "res-1 reference res-2", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1", + "/subscriptions/123/resourceGroups/rg1", + "azurerm_resource_group.res-0", + ` +resource "azurerm_resource_group" "res-0" { + name = "rg1" + location = "West Europe" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-1", + ` +resource "azurerm_foo_resource" "res-1" { + name = "foo1" + resource_group_id = "/subscriptions/123/resourceGroups/rg1" +} +`, + nil, + ), + }, + expectedReferenceDeps: map[string]map[string]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { + "/subscriptions/123/resourceGroups/rg1": { + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/rg1", + TFResourceId: "/subscriptions/123/resourceGroups/rg1", + }, + }, + "/subscriptions/123/resourceGroups/rg1": make(map[string]Dependency), + }, + expectedAmbiguousDeps: map[string]map[string][]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, + }, + }, + { + name: "res-0 and res-1 have different azure resource id, but same TF resource id, res-2 refer to this TF resource id: expect res-2 to have ambiguous dep to the TF resource id", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-0", + `resource "azurerm_foo_sub1_resource" "res-0" { + name = "foo1_sub1" +}`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-1", + ` +resource "azurerm_foo_sub2_resource" "res-1" { + name = "foo1_sub2" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "azurerm_bar_resource.res-2", + ` +resource "azurerm_bar_resource" "res-2" { + name = "bar1" + foo_resource_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1" +} +`, + nil, + ), + }, + expectedReferenceDeps: map[string]map[string]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, + }, + expectedAmbiguousDeps: map[string]map[string][]Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": { + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { + { + TFAddr: mustParseTFAddr("azurerm_foo_resource.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", + TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + }, + { + TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), + AzureResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2", + TFResourceId: "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + }, + }, + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + err := testCase.inputConfigs.PopulateReferenceDeps() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, cfg := range testCase.inputConfigs { + expectedRefDeps := testCase.expectedReferenceDeps[cfg.AzureResourceID.String()] + assert.Equal(t, expectedRefDeps, cfg.Dependencies.ByRef, "referenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) + expectedAmbiguousRefDeps := testCase.expectedAmbiguousDeps[cfg.AzureResourceID.String()] + assert.Equal(t, expectedAmbiguousRefDeps, cfg.Dependencies.ByRefAmbiguous, "ambiguousDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) + } + }) + } +} + +func TestConfigInfos_PopulateRelationDeps(t *testing.T) { + testCases := []struct { + name string + inputConfigs ConfigInfos + expectedRelationDep map[string]*Dependency // key: AzureResourceId + }{ + { + name: "no parent-child relationships", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-0", + ` +resource "azurerm_foo_resource" "res-0" { + name = "foo1" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1", + "azurerm_bar_resource.res-1", + ` +resource "azurerm_bar_resource" "res-1" { + name = "bar1" +} +`, + nil, + ), + }, + expectedRelationDep: map[string]*Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": nil, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": nil, + }, + }, + { + name: "res-0 is a parent of res-1: expect explicit dep from res-1 to res-0", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1", + "/subscriptions/123/resourceGroups/rg1", + "azurerm_resource_group.res-0", + ` +resource "azurerm_resource_group" "res-0" { + name = "rg1" + location = "West Europe" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-1", + ` +resource "azurerm_foo_resource" "res-1" { + name = "foo1" + resource_group_name = "rg1" +} +`, + nil, + ), + }, + expectedRelationDep: map[string]*Dependency{ + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/rg1", + TFResourceId: "/subscriptions/123/resourceGroups/rg1", + }, + "/subscriptions/123/resourceGroups/rg1": nil, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testCase.inputConfigs.PopulateRelationDeps() + for _, cfg := range testCase.inputConfigs { + azureResourceId := cfg.AzureResourceID.String() + expectedExplicitDeps := testCase.expectedRelationDep[azureResourceId] + assert.Equal(t, expectedExplicitDeps, cfg.Dependencies.ByRelation, "parentChildDeps matches expectation, azureResourceId: %s", azureResourceId) + } + }) + } +} + +func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { + testCases := []struct { + name string + inputHcl string + depsByRef map[string]Dependency // key: TFResourceId + expectedHcl string + }{ + { + name: "no reference dependencies", + inputHcl: ` + name = "test" + resource_group_name = "test" +`, + depsByRef: make(map[string]Dependency), + expectedHcl: ` + name = "test" + resource_group_name = "test" +`, + }, + { + name: "single reference dependency in top level attribute", + inputHcl: ` + name = "test" + foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" +`, + depsByRef: map[string]Dependency{ + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), + }, + }, + expectedHcl: ` + name = "test" + foo_id = azurerm_foo_resource.res-1.id +`, + }, + { + name: "multiple reference dependency in top level and nested block", + inputHcl: ` + name = "test" + foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" + + some_block { + bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" + } +`, + depsByRef: map[string]Dependency{ + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { + TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + }, + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { + TFAddr: mustParseTFAddr("azurerm_bar_resource.res-2"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + }, + }, + expectedHcl: ` + name = "test" + foo_id = azurerm_foo_resource.res-1.id + + some_block { + bar_id = azurerm_bar_resource.res-2.id + } +`, + }, + { + name: "multiple reference dependency in array and maps", + inputHcl: ` + name = "test" + foo_ids = ["/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123"] + + bar_ids_map = { + bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" + } +`, + depsByRef: map[string]Dependency{ + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { + TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + }, + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { + TFAddr: mustParseTFAddr("azurerm_bar_resource.res-2"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + }, + }, + expectedHcl: ` + name = "test" + foo_ids = [azurerm_foo_resource.res-1.id] + + bar_ids_map = { + bar_id = azurerm_bar_resource.res-2.id + } +`, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + cinfo := ConfigInfo{ + Dependencies: Dependencies{ + ByRef: testCase.depsByRef, + }, + HCL: mustHclWriteParse(testCase.inputHcl), + } + cinfo.applyRefDepsToHCL() + assert.Equal(t, testCase.expectedHcl, string(cinfo.HCL.BuildTokens(nil).Bytes())) + }) + } +} + +func TestConfigInfo_ApplyExplicitDepsToHCL(t *testing.T) { + testCases := []struct { + name string + inputHcl string + depsByByRelation *Dependency + depsByRefAmbiguous map[string][]Dependency // key: TFResourceId + depsByRef map[string]Dependency + expectedHcl string + }{ + { + name: "no explicit or ambiguous dependencies", + inputHcl: ` + name = "test" + foo_id = azurerm_foo_resource.res-1.id +`, + depsByByRelation: nil, + depsByRefAmbiguous: make(map[string][]Dependency), + expectedHcl: ` + name = "test" + foo_id = azurerm_foo_resource.res-1.id +`, + }, + { + name: "single parent child dependency", + inputHcl: ` + name = "test" + resource_group_name = "test" +`, + depsByByRelation: &Dependency{ + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/123", + TFResourceId: "/subscriptions/123/resourceGroups/123", + }, + depsByRefAmbiguous: make(map[string][]Dependency), + expectedHcl: ` + name = "test" + resource_group_name = "test" +depends_on= [ +azurerm_resource_group.res-0, +] +`, + }, + { + name: "single parent child dependency, but is covered by a reference dependency", + inputHcl: ` + name = "test" + resource_group_name = "test" +`, + depsByByRelation: &Dependency{ + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/123", + TFResourceId: "/subscriptions/123/resourceGroups/123", + }, + depsByRef: map[string]Dependency{ + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1": { + TFAddr: mustParseTFAddr("azurerm_resource_foo.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1", + }, + }, + depsByRefAmbiguous: make(map[string][]Dependency), + expectedHcl: ` + name = "test" + resource_group_name = "test" +`, + }, + { + name: "multiple ambiguous dependencies", + inputHcl: ` + name = "test" + resource_group_name = "test" +`, + depsByByRelation: nil, + depsByRefAmbiguous: map[string][]Dependency{ + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { + { + TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-1"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123/sub1/sub1", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + }, + { + TFAddr: mustParseTFAddr("azurerm_foo_sub2_resource.res-2"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123/sub2/sub2", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", + }, + }, + "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { + { + TFAddr: mustParseTFAddr("azurerm_bar_sub1_resource.res-3"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456/sub1/sub1", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + }, + { + TFAddr: mustParseTFAddr("azurerm_bar_sub2_resource.res-4"), + AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456/sub2/sub2", + TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", + }, + }, + }, + expectedHcl: ` + name = "test" + resource_group_name = "test" +depends_on= [ +# One of azurerm_bar_sub1_resource.res-3,azurerm_bar_sub2_resource.res-4 (can't auto-resolve as their ids are identical) +# One of azurerm_foo_sub1_resource.res-1,azurerm_foo_sub2_resource.res-2 (can't auto-resolve as their ids are identical) +] +`, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + cinfo := ConfigInfo{ + Dependencies: Dependencies{ + ByRelation: testCase.depsByByRelation, + ByRef: testCase.depsByRef, + ByRefAmbiguous: testCase.depsByRefAmbiguous, + }, + HCL: mustHclWriteParse(testCase.inputHcl), + } + + assert.NoError(t, cinfo.applyExplicitDepsToHCL()) + actualHcl := string(cinfo.HCL.BuildTokens(nil).Bytes()) + assert.Equal(t, testCase.expectedHcl, actualHcl) + }) + } +} + +func mustHclWriteParse(input string) *hclwrite.File { + file, diag := hclwrite.ParseConfig([]byte(input), "input.hcl", hcl.InitialPos) + if diag.HasErrors() { + panic(fmt.Sprintf("failed to parse HCL: %v", diag)) + } + return file +} + +func mustParseTFAddr(s string) tfaddr.TFAddr { + tfAddr, err := tfaddr.ParseTFResourceAddr(s) + if err != nil { + panic(fmt.Sprintf("failed to parse TF resource address %s: %v", s, err)) + } + return *tfAddr +} diff --git a/internal/meta/hcl_edit.go b/internal/meta/hcl_edit.go index c1cfbfe..e28fb5a 100644 --- a/internal/meta/hcl_edit.go +++ b/internal/meta/hcl_edit.go @@ -2,105 +2,12 @@ package meta import ( "fmt" - "sort" "strings" "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hclwrite" ) -func (configs ConfigInfos) applyDependenciesToHclBlock() error { - for i, cfg := range configs { - applyReferenceDependenciesToHcl(cfg.hcl.Body().Blocks()[0].Body(), cfg.dependencies.refDeps) - if err := applyParentChildAndAmbiguousDepsToHclBlock( - cfg.hcl.Body().Blocks()[0].Body(), - cfg.dependencies.parentChildDeps, - cfg.dependencies.ambiguousRefDeps); err != nil { - return fmt.Errorf("applying explicit and ambiguous dependencies to %s: %w", cfg.TFResourceId, err) - } - configs[i] = cfg - } - return nil -} - -func applyReferenceDependenciesToHcl(body *hclwrite.Body, refDeps map[string]Dependency) { - if len(refDeps) == 0 { - return - } - - for name, attr := range body.Attributes() { - tokens := attr.Expr().BuildTokens(nil) - newTokens := hclwrite.Tokens{} - tokensModified := false - - for i := 0; i < len(tokens); i++ { - refDep, refDepExists := refDeps[string(tokens[i].Bytes)] - // Parsing process guaranteed QuotedLit is surrounded by Opening and Closing quote - if tokens[i].Type == hclsyntax.TokenQuotedLit && refDepExists { - newTokens[len(newTokens)-1] = &hclwrite.Token{ - Type: hclsyntax.TokenIdent, - Bytes: fmt.Appendf(nil, "%s.id", refDep.TFAddr), - SpacesBefore: tokens[i-1].SpacesBefore, - } - tokensModified = true - i += 1 // Skip the next token as it was already processed - } else { - newTokens = append(newTokens, tokens[i]) - } - } - - if tokensModified { - body.SetAttributeRaw(name, newTokens) - } - - for _, nestedBlock := range body.Blocks() { - applyReferenceDependenciesToHcl(nestedBlock.Body(), refDeps) - } - } -} - -func applyParentChildAndAmbiguousDepsToHclBlock( - body *hclwrite.Body, - parentChildDeps map[Dependency]bool, - ambiguousDeps map[string][]Dependency, -) error { - if len(parentChildDeps)+len(ambiguousDeps) == 0 { - return nil - } - - src := "depends_on = [\n" - if len(parentChildDeps) > 0 { - tfAddrs := make([]string, 0, len(parentChildDeps)) - for dep := range parentChildDeps { - tfAddrs = append(tfAddrs, dep.TFAddr.String()) - } - sort.Strings(tfAddrs) - src += strings.Join(tfAddrs, ",\n") + "\n" - } - if len(ambiguousDeps) > 0 { - ambiguousDepsComments := make([]string, 0, len(ambiguousDeps)) - for _, deps := range ambiguousDeps { - tfAddrs := make([]string, 0, len(deps)) - for _, dep := range deps { - tfAddrs = append(tfAddrs, dep.TFAddr.String()) - } - sort.Strings(tfAddrs) - ambiguousDepsComments = append(ambiguousDepsComments, fmt.Sprintf("# One of %s (can't auto-resolve as their ids are identical)", strings.Join(tfAddrs, ","))) - } - sort.Strings(ambiguousDepsComments) - src += strings.Join(ambiguousDepsComments, "\n") + "\n" - } - src += "]\n" - expr, diags := hclwrite.ParseConfig([]byte(src), "f", hcl.InitialPos) - if diags.HasErrors() { - return fmt.Errorf(`building "depends_on" attribute: %s`, diags.Error()) - } - body.SetAttributeRaw("depends_on", expr.Body().GetAttribute("depends_on").Expr().BuildTokens(nil)) - - return nil -} - func hclBlockAppendLifecycle(body *hclwrite.Body, ignoreChanges []string) error { srcs := map[string][]byte{} if len(ignoreChanges) > 0 { diff --git a/internal/meta/hcl_edit_test.go b/internal/meta/hcl_edit_test.go deleted file mode 100644 index 7fd448f..0000000 --- a/internal/meta/hcl_edit_test.go +++ /dev/null @@ -1,259 +0,0 @@ -package meta - -import ( - "fmt" - "testing" - - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/hcl/v2/hclwrite" - "github.com/stretchr/testify/assert" -) - -func TestApplyReferenceDependenciesToHcl(t *testing.T) { - testCases := []struct { - name string - inputHcl string - refDeps map[string]Dependency // key: TFResourceId - expectedHcl string - }{ - { - name: "no reference dependencies", - inputHcl: ` - name = "test" - resource_group_name = "test" -`, - refDeps: make(map[string]Dependency), - expectedHcl: ` - name = "test" - resource_group_name = "test" -`, - }, - { - name: "single reference dependency in top level attribute", - inputHcl: ` - name = "test" - foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" -`, - refDeps: map[string]Dependency{ - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), - }, - }, - expectedHcl: ` - name = "test" - foo_id = azurerm_foo_resource.res-1.id -`, - }, - { - name: "multiple reference dependency in top level and nested block", - inputHcl: ` - name = "test" - foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" - - some_block { - bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" - } -`, - refDeps: map[string]Dependency{ - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { - TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - }, - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { - TFAddr: mustParseTFAddr("azurerm_bar_resource.res-2"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - }, - }, - expectedHcl: ` - name = "test" - foo_id = azurerm_foo_resource.res-1.id - - some_block { - bar_id = azurerm_bar_resource.res-2.id - } -`, - }, - { - name: "multiple reference dependency in array and maps", - inputHcl: ` - name = "test" - foo_ids = ["/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123"] - - bar_ids_map = { - bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" - } -`, - refDeps: map[string]Dependency{ - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { - TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - }, - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { - TFAddr: mustParseTFAddr("azurerm_bar_resource.res-2"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - }, - }, - expectedHcl: ` - name = "test" - foo_ids = [azurerm_foo_resource.res-1.id] - - bar_ids_map = { - bar_id = azurerm_bar_resource.res-2.id - } -`, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - body := mustHclWriteParse(testCase.inputHcl) - applyReferenceDependenciesToHcl(body, testCase.refDeps) - assert.Equal(t, testCase.expectedHcl, string(body.BuildTokens(nil).Bytes())) - }) - } -} - -func TestApplyParentChildAndAmbiguousDepsToHclBlock(t *testing.T) { - testCases := []struct { - name string - inputHcl string - parentChildDeps map[Dependency]bool - ambiguousDeps map[string][]Dependency // key: TFResourceId - expectedHcl string - }{ - { - name: "no explicit or ambiguous dependencies", - inputHcl: ` - name = "test" - foo_id = azurerm_foo_resource.res-1.id -`, - parentChildDeps: make(map[Dependency]bool), - ambiguousDeps: make(map[string][]Dependency), - expectedHcl: ` - name = "test" - foo_id = azurerm_foo_resource.res-1.id -`, - }, - { - name: "single parent child dependency", - inputHcl: ` - name = "test" - resource_group_name = "test" -`, - parentChildDeps: map[Dependency]bool{ - { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/123", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/123", - }: true, - }, - ambiguousDeps: make(map[string][]Dependency), - expectedHcl: ` - name = "test" - resource_group_name = "test" -depends_on= [ -azurerm_resource_group.res-0 -] -`, - }, - { - name: "multiple parent child dependencies", - inputHcl: ` - name = "test" - resource_group_name = "test" -`, - parentChildDeps: map[Dependency]bool{ - { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/123", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/123", - }: true, - { - TFAddr: mustParseTFAddr("azurerm_resource_group.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/124", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.ResourceGroup/resourceGroup/124", - }: true, - }, - ambiguousDeps: make(map[string][]Dependency), - expectedHcl: ` - name = "test" - resource_group_name = "test" -depends_on= [ -azurerm_resource_group.res-0, -azurerm_resource_group.res-1 -] -`, - }, - { - name: "multiple ambiguous dependencies", - inputHcl: ` - name = "test" - resource_group_name = "test" -`, - parentChildDeps: make(map[Dependency]bool), - ambiguousDeps: map[string][]Dependency{ - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { - { - TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-1"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123/sub1/sub1", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - }, - { - TFAddr: mustParseTFAddr("azurerm_foo_sub2_resource.res-2"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123/sub2/sub2", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", - }, - }, - "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456": { - { - TFAddr: mustParseTFAddr("azurerm_bar_sub1_resource.res-3"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456/sub1/sub1", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - }, - { - TFAddr: mustParseTFAddr("azurerm_bar_sub2_resource.res-4"), - AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456/sub2/sub2", - TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456", - }, - }, - }, - expectedHcl: ` - name = "test" - resource_group_name = "test" -depends_on= [ -# One of azurerm_bar_sub1_resource.res-3,azurerm_bar_sub2_resource.res-4 (can't auto-resolve as their ids are identical) -# One of azurerm_foo_sub1_resource.res-1,azurerm_foo_sub2_resource.res-2 (can't auto-resolve as their ids are identical) -] -`, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - body := mustHclWriteParse(testCase.inputHcl) - err := applyParentChildAndAmbiguousDepsToHclBlock( - body, - testCase.parentChildDeps, - testCase.ambiguousDeps, - ) - assert.NoError(t, err) - - actualHcl := string(body.BuildTokens(nil).Bytes()) - assert.Equal(t, testCase.expectedHcl, actualHcl) - }) - } -} - -func mustHclWriteParse(input string) *hclwrite.Body { - file, diag := hclwrite.ParseConfig([]byte(input), "input.hcl", hcl.InitialPos) - if diag.HasErrors() { - panic(fmt.Sprintf("failed to parse HCL: %v", diag)) - } - return file.Body() -} From 8aecb94dfbe130f6b89495a59a02d264450e2ce3 Mon Sep 17 00:00:00 2001 From: magodo Date: Fri, 25 Jul 2025 18:07:41 +1000 Subject: [PATCH 2/4] Support implicit dependency based on top level `resource_group_name` attribute. The dependency injection logic now also add supports for replacing the top level `resource_group_name`'s value by a reference to the resource's `name` attribute if that resource's name has the same value, and that resource is a resource group. This is almost only relevant to `azurerm` provider as `azapi` provider has no such top level attribute. Note that we support for `resource_group_name` based on the fact that this is a well known attribute in `azurerm` to represent the residing resource group's name of the current resource. That's also the reason why we only support it at the top level, since there can be resources who has nested block, that also expose a `resource_group_name` and a `subscription_id` (e.g. `azurerm_data_share_dataset_blob_storage`), in which case the actual resource group can be different than the resource group in the current exported resource list (i.e. they have the same name, but from the different subscriptions). --- internal/meta/base_meta.go | 4 +- internal/meta/config_info.go | 172 +++++++++++++-------- internal/meta/config_info_test.go | 243 ++++++++++++++++++++++++------ 3 files changed, 306 insertions(+), 113 deletions(-) diff --git a/internal/meta/base_meta.go b/internal/meta/base_meta.go index 80b777a..a41d9fd 100644 --- a/internal/meta/base_meta.go +++ b/internal/meta/base_meta.go @@ -1094,8 +1094,8 @@ func (meta baseMeta) stateToConfig(ctx context.Context, list ImportList) (Config ImportItem: importedList[i], HCL: f, Dependencies: Dependencies{ - ByRef: make(map[string]Dependency), - ByRefAmbiguous: make(map[string][]Dependency), + ByIdRef: make(map[string]Dependency), + ByIdRefAmbiguous: make(map[string][]Dependency), }, }) } diff --git a/internal/meta/config_info.go b/internal/meta/config_info.go index a2a30d1..99eeab6 100644 --- a/internal/meta/config_info.go +++ b/internal/meta/config_info.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hclwrite" + "github.com/magodo/armid" "github.com/zclconf/go-cty/cty" ) @@ -23,62 +24,99 @@ type ConfigInfo struct { HCL *hclwrite.File } +type Dependencies struct { + // Dependencies inferred by scanning for resource id values + // The key is TFResourceId. + ByIdRef map[string]Dependency + + // Similar to ByIdRef, but due to multiple Azure resources can map to a same TF resource id (being referenced), + // this is regarded as ambiguous references. + // The key is TFResourceId. + ByIdRefAmbiguous map[string][]Dependency + + // Dependencies inferred by resource group name reference. + // NOTE: This holds since the azurerm/azapi provider is guaranteed to work for a single subscription. + ByRgNameRef *Dependency + + // Dependencies inferred via Azure resource id parent lookup. + // At most one such dependency can exist. + ByRelation *Dependency +} + +type Dependency struct { + TFResourceId string + AzureResourceId string + TFAddr tfaddr.TFAddr +} + func (cfg ConfigInfo) DumpHCL(w io.Writer) (int, error) { out := hclwrite.Format(cfg.HCL.Bytes()) return w.Write(out) } func (cfg *ConfigInfo) applyRefDepsToHCL() { - var applyF func(*hclwrite.Body, map[string]Dependency) - applyF = func(body *hclwrite.Body, deps map[string]Dependency) { - if len(deps) == 0 { - return + var applyF func(*hclwrite.Body, map[string]Dependency, *Dependency) + applyF = func(body *hclwrite.Body, idDeps map[string]Dependency, rgDep *Dependency) { + // Apply the rg name reference + if rgDep != nil { + if _, ok := body.Attributes()["resource_group_name"]; ok { + body.SetAttributeTraversal("resource_group_name", hcl.Traversal{ + hcl.TraverseRoot{Name: rgDep.TFAddr.Type}, + hcl.TraverseAttr{Name: rgDep.TFAddr.Name}, + hcl.TraverseAttr{Name: "name"}, + }) + } } - for name, attr := range body.Attributes() { - tokens := attr.Expr().BuildTokens(nil) - newTokens := hclwrite.Tokens{} - tokensModified := false - for i := 0; i < len(tokens); i++ { - refDep, refDepExists := deps[string(tokens[i].Bytes)] - // Parsing process guaranteed QuotedLit is surrounded by Opening and Closing quote - if tokens[i].Type == hclsyntax.TokenQuotedLit && refDepExists { - newTokens[len(newTokens)-1] = &hclwrite.Token{ - Type: hclsyntax.TokenIdent, - Bytes: fmt.Appendf(nil, "%s.id", refDep.TFAddr), - SpacesBefore: tokens[i-1].SpacesBefore, + // Apply the id reference + if len(idDeps) != 0 { + for name, attr := range body.Attributes() { + tokens := attr.Expr().BuildTokens(nil) + newTokens := hclwrite.Tokens{} + toApply := false + for i := 0; i < len(tokens); i++ { + refDep, refDepExists := idDeps[string(tokens[i].Bytes)] + // Parsing process guaranteed QuotedLit is surrounded by Opening and Closing quote + if tokens[i].Type == hclsyntax.TokenQuotedLit && refDepExists { + newTokens[len(newTokens)-1] = &hclwrite.Token{ + Type: hclsyntax.TokenIdent, + Bytes: fmt.Appendf(nil, "%s.id", refDep.TFAddr), + SpacesBefore: tokens[i-1].SpacesBefore, + } + toApply = true + i += 1 // Skip the next token as it was already processed + } else { + newTokens = append(newTokens, tokens[i]) } - tokensModified = true - i += 1 // Skip the next token as it was already processed - } else { - newTokens = append(newTokens, tokens[i]) } - } - - if tokensModified { - body.SetAttributeRaw(name, newTokens) - } - for _, nestedBlock := range body.Blocks() { - applyF(nestedBlock.Body(), deps) + if toApply { + body.SetAttributeRaw(name, newTokens) + } + for _, nestedBlock := range body.Blocks() { + applyF(nestedBlock.Body(), idDeps, nil) + } } } } - applyF(cfg.HCL.Body(), cfg.Dependencies.ByRef) + applyF(cfg.HCL.Body().Blocks()[0].Body(), cfg.Dependencies.ByIdRef, cfg.Dependencies.ByRgNameRef) } func (cfg *ConfigInfo) applyExplicitDepsToHCL() error { - body := cfg.HCL.Body() + body := cfg.HCL.Body().Blocks()[0].Body() relationDep := cfg.Dependencies.ByRelation if relationDep != nil { // Skip this relation dependency if it's already covered by any of the other applied dependencies. - var otherDepAzureIds []string - for _, dep := range cfg.Dependencies.ByRef { - otherDepAzureIds = append(otherDepAzureIds, dep.AzureResourceId) + var appliedDepIds []string + for _, dep := range cfg.Dependencies.ByIdRef { + appliedDepIds = append(appliedDepIds, dep.AzureResourceId) + } + if dep := cfg.Dependencies.ByRgNameRef; dep != nil { + appliedDepIds = append(appliedDepIds, dep.AzureResourceId) } var covered bool - for _, dep := range otherDepAzureIds { - if strings.HasPrefix(dep, relationDep.AzureResourceId) { + for _, id := range appliedDepIds { + if strings.HasPrefix(id, relationDep.AzureResourceId) { covered = true break } @@ -88,8 +126,10 @@ func (cfg *ConfigInfo) applyExplicitDepsToHCL() error { } } - ambiguousDeps := cfg.Dependencies.ByRefAmbiguous - // TODO: Skip the ambiguous depedencies that are covered by any of the other applied Dependencies. + // There isn't a case that other applied dependencies will cover all the possible ambiguous dependencies. + // Whilst if in the future, there are more dependencies being added that can cover the ambiguous deps, + // we shall do the same skip check as above for the relation dependencies. + ambiguousDeps := cfg.Dependencies.ByIdRefAmbiguous if len(ambiguousDeps) == 0 && relationDep == nil { return nil @@ -122,27 +162,6 @@ func (cfg *ConfigInfo) applyExplicitDepsToHCL() error { return nil } -type Dependencies struct { - // Dependencies inferred by scanning for resource id values - // The key is TFResourceId. - ByRef map[string]Dependency - - // Similar to ByRef, but due to multiple Azure resources can map to a same TF resource id (being referenced), - // this is regarded as ambiguous references. - // The key is TFResourceId. - ByRefAmbiguous map[string][]Dependency - - // Dependencies inferred via Azure resource id parent lookup. - // At most one such dependency can exist. - ByRelation *Dependency -} - -type Dependency struct { - TFResourceId string - AzureResourceId string - TFAddr tfaddr.TFAddr -} - // Look at the Azure resource id and determine if parent dependency exist. // For example, /subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foos/foo1 // has a parent /subscriptions/123/resourceGroups/rg1, which is the resource group. @@ -179,22 +198,41 @@ func (cfgs ConfigInfos) PopulateRelationDeps() { } } -// Scan the HCL files for references to other resources. -// For example the HCL attribute `foo_id = "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foos/foo1"` -// will yield a dependency to that foo TF resource address. -// Note that a single TF resource id can map to multiple resources, in which case the dependencies is regarded as ambiguous. +// Scan the HCL files for references to other resources. There are two references will be detected: +// 1. Reference by (TF) resource id. This can be detected any where in the expression. +// Especially, a single TF resource id can map to multiple resources, in which case the dependencies is regarded as ambiguous. +// 2. Refernece by resoruce group name. This only applies to the top level attribute named `resource_group_name`. func (cfgs ConfigInfos) PopulateReferenceDeps() error { // key: TFResourceId - m := map[string][]*ConfigInfo{} + allResMap := map[string][]*ConfigInfo{} + // key: resource group name + allRgMap := map[string]*ConfigInfo{} for _, cfg := range cfgs { - m[cfg.TFResourceId] = append(m[cfg.TFResourceId], &cfg) + allResMap[cfg.TFResourceId] = append(allResMap[cfg.TFResourceId], &cfg) + if id, ok := cfg.AzureResourceID.(*armid.ResourceGroup); ok && len(id.AttrTypes) == 0 { + allRgMap[id.Name] = &cfg + } } - for i, cfg := range cfgs { file, err := hclsyntax.ParseConfig(cfg.HCL.Bytes(), "main.tf", hcl.InitialPos) if err != nil { return fmt.Errorf("parsing hcl for %s: %v", cfg.AzureResourceID, err) } + // Scan for the top level resource group name reference + if attr, ok := file.Body.(*hclsyntax.Body).Blocks[0].Body.Attributes["resource_group_name"]; ok { + if tplExpr, ok := attr.Expr.(*hclsyntax.TemplateExpr); ok && tplExpr.IsStringLiteral() { + val, _ := tplExpr.Value(nil) + if rgCfg, ok := allRgMap[val.AsString()]; ok { + cfg.Dependencies.ByRgNameRef = &Dependency{ + AzureResourceId: rgCfg.ImportItem.AzureResourceID.String(), + TFResourceId: rgCfg.ImportItem.TFResourceId, + TFAddr: rgCfg.ImportItem.TFAddr, + } + } + } + } + + // Scan for resource id reference hclsyntax.VisitAll(file.Body.(*hclsyntax.Body), func(node hclsyntax.Node) hcl.Diagnostics { expr, ok := node.(*hclsyntax.LiteralValueExpr) if !ok { @@ -208,7 +246,7 @@ func (cfgs ConfigInfos) PopulateReferenceDeps() error { // Try to look up this string attribute from the TF id map. If there is a match, we regard it as a valid TF resource id. // This is safe to match case sensitively given the TF id are consistent across the provider. Otherwise, it is a provider bug. - dependingConfigsRaw, ok := m[maybeTFId] + dependingConfigsRaw, ok := allResMap[maybeTFId] if !ok { return nil } @@ -228,7 +266,7 @@ func (cfgs ConfigInfos) PopulateReferenceDeps() error { } if len(dependingConfigs) == 1 { - cfg.Dependencies.ByRef[depTFResId] = Dependency{ + cfg.Dependencies.ByIdRef[depTFResId] = Dependency{ TFResourceId: depTFResId, AzureResourceId: dependingConfigs[0].AzureResourceID.String(), TFAddr: dependingConfigs[0].TFAddr, @@ -242,7 +280,7 @@ func (cfgs ConfigInfos) PopulateReferenceDeps() error { TFAddr: depCfg.TFAddr, }) } - cfg.Dependencies.ByRefAmbiguous[depTFResId] = deps + cfg.Dependencies.ByIdRefAmbiguous[depTFResId] = deps } return nil diff --git a/internal/meta/config_info_test.go b/internal/meta/config_info_test.go index 9041c14..bf0ba86 100644 --- a/internal/meta/config_info_test.go +++ b/internal/meta/config_info_test.go @@ -19,8 +19,8 @@ func newConfigInfo(azid, tfid, tfaddr, hcl string, deps *Dependencies) ConfigInf }, HCL: mustHclWriteParse(hcl), Dependencies: Dependencies{ - ByRef: make(map[string]Dependency), - ByRefAmbiguous: make(map[string][]Dependency), + ByIdRef: make(map[string]Dependency), + ByIdRefAmbiguous: make(map[string][]Dependency), }, } if deps != nil { @@ -31,10 +31,11 @@ func newConfigInfo(azid, tfid, tfaddr, hcl string, deps *Dependencies) ConfigInf func TestConfigInfos_PopulateReferenceDeps(t *testing.T) { testCases := []struct { - name string - inputConfigs ConfigInfos - expectedReferenceDeps map[string]map[string]Dependency // key: AzureResourceId, inner key: TFResourceId - expectedAmbiguousDeps map[string]map[string][]Dependency // key: AzureResourceId, inner key: TFResourceId + name string + inputConfigs ConfigInfos + expectedRgNameReferenceDeps map[string]*Dependency // key: AzureResourceId + expectedIdReferenceDeps map[string]map[string]Dependency // key: AzureResourceId, inner key: TFResourceId + expectedIdReferenceAmbiguousDeps map[string]map[string][]Dependency // key: AzureResourceId, inner key: TFResourceId }{ { name: "no dependencies between resources", @@ -62,17 +63,17 @@ resource "azurerm_bar_resource" "res-1" { nil, ), }, - expectedReferenceDeps: map[string]map[string]Dependency{ + expectedIdReferenceDeps: map[string]map[string]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ + expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, }, }, { - name: "res-1 reference res-2", + name: "res-1 reference res-2 by id", inputConfigs: []ConfigInfo{ newConfigInfo( "/subscriptions/123/resourceGroups/rg1", @@ -99,7 +100,7 @@ resource "azurerm_foo_resource" "res-1" { nil, ), }, - expectedReferenceDeps: map[string]map[string]Dependency{ + expectedIdReferenceDeps: map[string]map[string]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { "/subscriptions/123/resourceGroups/rg1": { TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), @@ -109,13 +110,13 @@ resource "azurerm_foo_resource" "res-1" { }, "/subscriptions/123/resourceGroups/rg1": make(map[string]Dependency), }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ + expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, "/subscriptions/123/resourceGroups/rg1": {}, }, }, { - name: "res-0 and res-1 have different azure resource id, but same TF resource id, res-2 refer to this TF resource id: expect res-2 to have ambiguous dep to the TF resource id", + name: "res-0 and res-1 have different azure resource id, but same TF resource id, res-2 refer to this TF resource id: expect res-2 to have ambiguous id dep to the TF resource id", inputConfigs: []ConfigInfo{ newConfigInfo( "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1", @@ -150,12 +151,12 @@ resource "azurerm_bar_resource" "res-2" { nil, ), }, - expectedReferenceDeps: map[string]map[string]Dependency{ + expectedIdReferenceDeps: map[string]map[string]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": {}, }, - expectedAmbiguousDeps: map[string]map[string][]Dependency{ + expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub1/sub1": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1/sub2/sub2": {}, "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Bar/bar/bar1": { @@ -174,6 +175,51 @@ resource "azurerm_bar_resource" "res-2" { }, }, }, + { + name: "res-1 reference resource group by `resource_group_name`", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1", + "/subscriptions/123/resourceGroups/rg1", + "azurerm_resource_group.res-0", + ` +resource "azurerm_resource_group" "res-0" { + name = "rg1" + location = "West Europe" +} +`, + nil, + ), + newConfigInfo( + "/providers/Microsoft.Foo/foo/foo1", + "/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-1", + ` +resource "azurerm_foo_resource" "res-1" { + name = "foo1" + resource_group_name = "rg1" +} +`, + nil, + ), + }, + expectedRgNameReferenceDeps: map[string]*Dependency{ + "/providers/Microsoft.Foo/foo/foo1": { + AzureResourceId: "/subscriptions/123/resourceGroups/rg1", + TFResourceId: "/subscriptions/123/resourceGroups/rg1", + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + }, + "/subscriptions/123/resourceGroups/rg1": nil, + }, + expectedIdReferenceDeps: map[string]map[string]Dependency{ + "/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, + }, + expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ + "/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, + }, + }, } for _, testCase := range testCases { @@ -184,10 +230,14 @@ resource "azurerm_bar_resource" "res-2" { } for _, cfg := range testCase.inputConfigs { - expectedRefDeps := testCase.expectedReferenceDeps[cfg.AzureResourceID.String()] - assert.Equal(t, expectedRefDeps, cfg.Dependencies.ByRef, "referenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) - expectedAmbiguousRefDeps := testCase.expectedAmbiguousDeps[cfg.AzureResourceID.String()] - assert.Equal(t, expectedAmbiguousRefDeps, cfg.Dependencies.ByRefAmbiguous, "ambiguousDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) + expectedIdRefDeps := testCase.expectedIdReferenceDeps[cfg.AzureResourceID.String()] + assert.Equal(t, expectedIdRefDeps, cfg.Dependencies.ByIdRef, "idReferenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) + + expectedAmbiguousIdRefDeps := testCase.expectedIdReferenceAmbiguousDeps[cfg.AzureResourceID.String()] + assert.Equal(t, expectedAmbiguousIdRefDeps, cfg.Dependencies.ByIdRefAmbiguous, "ambiguousIdReferenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) + + expectedRgNameRefDeps := testCase.expectedRgNameReferenceDeps[cfg.AzureResourceID.String()] + assert.Equal(t, expectedRgNameRefDeps, cfg.Dependencies.ByRgNameRef, "nameReferenceDeps matches expectation, azureResourceId: %s", cfg.AzureResourceID.String()) } }) } @@ -283,30 +333,37 @@ resource "azurerm_foo_resource" "res-1" { func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { testCases := []struct { - name string - inputHcl string - depsByRef map[string]Dependency // key: TFResourceId - expectedHcl string + name string + inputHcl string + depsByIdRef map[string]Dependency // key: TFResourceId + depsByRgNameRef *Dependency + expectedHcl string }{ { name: "no reference dependencies", inputHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, - depsByRef: make(map[string]Dependency), + depsByIdRef: make(map[string]Dependency), expectedHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, }, { name: "single reference dependency in top level attribute", inputHcl: ` +resource azurerm_resource_bar self { name = "test" foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" +} `, - depsByRef: map[string]Dependency{ + depsByIdRef: map[string]Dependency{ "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", @@ -314,21 +371,74 @@ func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { }, }, expectedHcl: ` +resource azurerm_resource_bar self { name = "test" foo_id = azurerm_foo_resource.res-1.id +} +`, + }, + { + name: "single rg name dependency in top level attribute", + inputHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name = "rg1" +} +`, + depsByRgNameRef: &Dependency{ + TFResourceId: "/subscriptions/123/resourceGroups/rg1", + AzureResourceId: "/subscriptions/123/resourceGroups/rg1", + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + }, + expectedHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name =azurerm_resource_group.res-0.name +} +`, + }, + { + name: "Mixed rg name dependency and id dependencies", + inputHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name = "rg1" + bar_id = "/subscriptions/123/providers/Microsoft.Bar/bars/bar1" +} +`, + depsByIdRef: map[string]Dependency{ + "/subscriptions/123/providers/Microsoft.Bar/bars/bar1": { + TFResourceId: "/subscriptions/123/providers/Microsoft.Bar/bars/bar1", + AzureResourceId: "/subscriptions/123/providers/Microsoft.Bar/bars/bar1", + TFAddr: mustParseTFAddr("azurerm_resource_bar.res-0"), + }, + }, + depsByRgNameRef: &Dependency{ + TFResourceId: "/subscriptions/123/resourceGroups/rg1", + AzureResourceId: "/subscriptions/123/resourceGroups/rg1", + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + }, + expectedHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name =azurerm_resource_group.res-0.name + bar_id = azurerm_resource_bar.res-0.id +} `, }, { name: "multiple reference dependency in top level and nested block", inputHcl: ` +resource azurerm_resource_foo self { name = "test" foo_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123" some_block { bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" } +} `, - depsByRef: map[string]Dependency{ + depsByIdRef: map[string]Dependency{ "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", @@ -341,25 +451,29 @@ func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { }, }, expectedHcl: ` +resource azurerm_resource_foo self { name = "test" foo_id = azurerm_foo_resource.res-1.id some_block { bar_id = azurerm_bar_resource.res-2.id } +} `, }, { name: "multiple reference dependency in array and maps", inputHcl: ` +resource azurerm_resource_foo self { name = "test" foo_ids = ["/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123"] bar_ids_map = { bar_id = "/subscriptions/123/resourceGroups/123/providers/Microsoft.Bar/bar/456" } +} `, - depsByRef: map[string]Dependency{ + depsByIdRef: map[string]Dependency{ "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { TFAddr: mustParseTFAddr("azurerm_foo_resource.res-1"), AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123", @@ -372,12 +486,14 @@ func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { }, }, expectedHcl: ` +resource azurerm_resource_foo self { name = "test" foo_ids = [azurerm_foo_resource.res-1.id] bar_ids_map = { bar_id = azurerm_bar_resource.res-2.id } +} `, }, } @@ -386,7 +502,8 @@ func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { cinfo := ConfigInfo{ Dependencies: Dependencies{ - ByRef: testCase.depsByRef, + ByIdRef: testCase.depsByIdRef, + ByRgNameRef: testCase.depsByRgNameRef, }, HCL: mustHclWriteParse(testCase.inputHcl), } @@ -398,78 +515,113 @@ func TestConfigInfo_ApplyReferenceDepsToHCL(t *testing.T) { func TestConfigInfo_ApplyExplicitDepsToHCL(t *testing.T) { testCases := []struct { - name string - inputHcl string - depsByByRelation *Dependency - depsByRefAmbiguous map[string][]Dependency // key: TFResourceId - depsByRef map[string]Dependency - expectedHcl string + name string + inputHcl string + depsByByRelation *Dependency + depsByIdRefAmbiguous map[string][]Dependency // key: TFResourceId + depsByIdRef map[string]Dependency + depsByRgNameRef *Dependency + expectedHcl string }{ { name: "no explicit or ambiguous dependencies", inputHcl: ` +resource azurerm_resource_foo self { name = "test" foo_id = azurerm_foo_resource.res-1.id +} `, - depsByByRelation: nil, - depsByRefAmbiguous: make(map[string][]Dependency), expectedHcl: ` +resource azurerm_resource_foo self { name = "test" foo_id = azurerm_foo_resource.res-1.id +} `, }, { name: "single parent child dependency", inputHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, depsByByRelation: &Dependency{ TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), AzureResourceId: "/subscriptions/123/resourceGroups/123", TFResourceId: "/subscriptions/123/resourceGroups/123", }, - depsByRefAmbiguous: make(map[string][]Dependency), expectedHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" depends_on= [ azurerm_resource_group.res-0, ] +} `, }, { - name: "single parent child dependency, but is covered by a reference dependency", + name: "single parent child dependency, but is covered by an id reference dependency", inputHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, depsByByRelation: &Dependency{ TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), AzureResourceId: "/subscriptions/123/resourceGroups/123", TFResourceId: "/subscriptions/123/resourceGroups/123", }, - depsByRef: map[string]Dependency{ + depsByIdRef: map[string]Dependency{ "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1": { TFAddr: mustParseTFAddr("azurerm_resource_foo.res-0"), AzureResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1", TFResourceId: "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foos/foo1", }, }, - depsByRefAmbiguous: make(map[string][]Dependency), expectedHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name = "test" +} +`, + }, + { + name: "single parent child dependency, but is covered by a resource group name reference dependency", + inputHcl: ` +resource azurerm_resource_foo self { + name = "test" + resource_group_name = "test" +} +`, + depsByByRelation: &Dependency{ + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/test", + TFResourceId: "/subscriptions/123/resourceGroups/test", + }, + depsByRgNameRef: &Dependency{ + TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), + AzureResourceId: "/subscriptions/123/resourceGroups/test", + TFResourceId: "/subscriptions/123/resourceGroups/test", + }, + expectedHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, }, { name: "multiple ambiguous dependencies", inputHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" +} `, - depsByByRelation: nil, - depsByRefAmbiguous: map[string][]Dependency{ + depsByIdRefAmbiguous: map[string][]Dependency{ "/subscriptions/123/resourceGroups/123/providers/Microsoft.Foo/foo/123": { { TFAddr: mustParseTFAddr("azurerm_foo_sub1_resource.res-1"), @@ -496,12 +648,14 @@ azurerm_resource_group.res-0, }, }, expectedHcl: ` +resource azurerm_resource_foo self { name = "test" resource_group_name = "test" depends_on= [ # One of azurerm_bar_sub1_resource.res-3,azurerm_bar_sub2_resource.res-4 (can't auto-resolve as their ids are identical) # One of azurerm_foo_sub1_resource.res-1,azurerm_foo_sub2_resource.res-2 (can't auto-resolve as their ids are identical) ] +} `, }, } @@ -510,9 +664,10 @@ depends_on= [ t.Run(testCase.name, func(t *testing.T) { cinfo := ConfigInfo{ Dependencies: Dependencies{ - ByRelation: testCase.depsByByRelation, - ByRef: testCase.depsByRef, - ByRefAmbiguous: testCase.depsByRefAmbiguous, + ByRelation: testCase.depsByByRelation, + ByIdRef: testCase.depsByIdRef, + ByIdRefAmbiguous: testCase.depsByIdRefAmbiguous, + ByRgNameRef: testCase.depsByRgNameRef, }, HCL: mustHclWriteParse(testCase.inputHcl), } From 46a7a72b8777545c5275d26a89410a68ee0190f7 Mon Sep 17 00:00:00 2001 From: magodo Date: Mon, 28 Jul 2025 14:34:58 +1000 Subject: [PATCH 3/4] Avoid rg from another sub --- internal/meta/config_info.go | 24 ++++++++++---- internal/meta/config_info_test.go | 55 +++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/internal/meta/config_info.go b/internal/meta/config_info.go index 99eeab6..9b4a0a5 100644 --- a/internal/meta/config_info.go +++ b/internal/meta/config_info.go @@ -116,7 +116,7 @@ func (cfg *ConfigInfo) applyExplicitDepsToHCL() error { } var covered bool for _, id := range appliedDepIds { - if strings.HasPrefix(id, relationDep.AzureResourceId) { + if isParentOf(relationDep.AzureResourceId, id) { covered = true break } @@ -222,11 +222,17 @@ func (cfgs ConfigInfos) PopulateReferenceDeps() error { if attr, ok := file.Body.(*hclsyntax.Body).Blocks[0].Body.Attributes["resource_group_name"]; ok { if tplExpr, ok := attr.Expr.(*hclsyntax.TemplateExpr); ok && tplExpr.IsStringLiteral() { val, _ := tplExpr.Value(nil) - if rgCfg, ok := allRgMap[val.AsString()]; ok { - cfg.Dependencies.ByRgNameRef = &Dependency{ - AzureResourceId: rgCfg.ImportItem.AzureResourceID.String(), - TFResourceId: rgCfg.ImportItem.TFResourceId, - TFAddr: rgCfg.ImportItem.TFAddr, + rgName := val.AsString() + if rgCfg, ok := allRgMap[rgName]; ok { + // Ensure the referenced resource group is really the parent resource group of the current resource. + // This is to avoid the case that the referenced resource group is from another subscription. + // Sicne the resource group name is equal, we only need to further check its subscription id. + if isParentOf(rgCfg.AzureResourceID.String(), cfg.AzureResourceID.String()) { + cfg.Dependencies.ByRgNameRef = &Dependency{ + AzureResourceId: rgCfg.ImportItem.AzureResourceID.String(), + TFResourceId: rgCfg.ImportItem.TFResourceId, + TFAddr: rgCfg.ImportItem.TFAddr, + } } } } @@ -300,3 +306,9 @@ func (configs ConfigInfos) ApplyDepsToHCL() error { } return nil } + +// isParentOf is a utility to tell whether the "pid" is a top level of the "id". +// Given both "pid" and "id" are Azure resource ids. +func isParentOf(pid, id string) bool { + return strings.HasPrefix(strings.ToLower(id), strings.ToLower(pid)) +} diff --git a/internal/meta/config_info_test.go b/internal/meta/config_info_test.go index bf0ba86..6cfec24 100644 --- a/internal/meta/config_info_test.go +++ b/internal/meta/config_info_test.go @@ -191,8 +191,8 @@ resource "azurerm_resource_group" "res-0" { nil, ), newConfigInfo( - "/providers/Microsoft.Foo/foo/foo1", - "/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", "azurerm_foo_resource.res-1", ` resource "azurerm_foo_resource" "res-1" { @@ -204,7 +204,7 @@ resource "azurerm_foo_resource" "res-1" { ), }, expectedRgNameReferenceDeps: map[string]*Dependency{ - "/providers/Microsoft.Foo/foo/foo1": { + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": { AzureResourceId: "/subscriptions/123/resourceGroups/rg1", TFResourceId: "/subscriptions/123/resourceGroups/rg1", TFAddr: mustParseTFAddr("azurerm_resource_group.res-0"), @@ -212,12 +212,53 @@ resource "azurerm_foo_resource" "res-1" { "/subscriptions/123/resourceGroups/rg1": nil, }, expectedIdReferenceDeps: map[string]map[string]Dependency{ - "/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, }, expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ - "/providers/Microsoft.Foo/foo/foo1": {}, - "/subscriptions/123/resourceGroups/rg1": {}, + "/subscriptions/123/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, + }, + }, + { + name: "res-1 reference resource group by `resource_group_name`, but in a different subscription", + inputConfigs: []ConfigInfo{ + newConfigInfo( + "/subscriptions/123/resourceGroups/rg1", + "/subscriptions/123/resourceGroups/rg1", + "azurerm_resource_group.res-0", + ` +resource "azurerm_resource_group" "res-0" { + name = "rg1" + location = "West Europe" +} +`, + nil, + ), + newConfigInfo( + "/subscriptions/456/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "/subscriptions/456/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1", + "azurerm_foo_resource.res-1", + ` +resource "azurerm_foo_resource" "res-1" { + name = "foo1" + resource_group_name = "rg1" +} +`, + nil, + ), + }, + expectedRgNameReferenceDeps: map[string]*Dependency{ + "/subscriptions/456/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": nil, + "/subscriptions/123/resourceGroups/rg1": nil, + }, + expectedIdReferenceDeps: map[string]map[string]Dependency{ + "/subscriptions/456/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, + }, + expectedIdReferenceAmbiguousDeps: map[string]map[string][]Dependency{ + "/subscriptions/456/resourceGroups/rg1/providers/Microsoft.Foo/foo/foo1": {}, + "/subscriptions/123/resourceGroups/rg1": {}, }, }, } From 61cf5555f6d42e97bde1ca8614642487f360fb2d Mon Sep 17 00:00:00 2001 From: magodo Date: Tue, 29 Jul 2025 12:36:20 +1000 Subject: [PATCH 4/4] resolve comments --- internal/meta/{hcl_edit.go => append_lifecycle.go} | 0 internal/meta/config_info.go | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename internal/meta/{hcl_edit.go => append_lifecycle.go} (100%) diff --git a/internal/meta/hcl_edit.go b/internal/meta/append_lifecycle.go similarity index 100% rename from internal/meta/hcl_edit.go rename to internal/meta/append_lifecycle.go diff --git a/internal/meta/config_info.go b/internal/meta/config_info.go index 9b4a0a5..e482ec5 100644 --- a/internal/meta/config_info.go +++ b/internal/meta/config_info.go @@ -201,7 +201,7 @@ func (cfgs ConfigInfos) PopulateRelationDeps() { // Scan the HCL files for references to other resources. There are two references will be detected: // 1. Reference by (TF) resource id. This can be detected any where in the expression. // Especially, a single TF resource id can map to multiple resources, in which case the dependencies is regarded as ambiguous. -// 2. Refernece by resoruce group name. This only applies to the top level attribute named `resource_group_name`. +// 2. Reference by resoruce group name. This only applies to the top level attribute named `resource_group_name`. func (cfgs ConfigInfos) PopulateReferenceDeps() error { // key: TFResourceId allResMap := map[string][]*ConfigInfo{} @@ -226,7 +226,7 @@ func (cfgs ConfigInfos) PopulateReferenceDeps() error { if rgCfg, ok := allRgMap[rgName]; ok { // Ensure the referenced resource group is really the parent resource group of the current resource. // This is to avoid the case that the referenced resource group is from another subscription. - // Sicne the resource group name is equal, we only need to further check its subscription id. + // Since the resource group name is equal, we only need to further check its subscription id. if isParentOf(rgCfg.AzureResourceID.String(), cfg.AzureResourceID.String()) { cfg.Dependencies.ByRgNameRef = &Dependency{ AzureResourceId: rgCfg.ImportItem.AzureResourceID.String(),