Skip to content

Commit 3c752e9

Browse files
committed
query: propagate graph node removal to descendants
1 parent 1e41449 commit 3c752e9

File tree

7 files changed

+149
-25
lines changed

7 files changed

+149
-25
lines changed

internal/backend/local/backend_local.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ func (b *Local) localRun(op *backendrun.Operation) (*backendrun.LocalRun, *confi
132132
// If validation is enabled, validate
133133
if b.OpValidation {
134134
log.Printf("[TRACE] backend/local: running validation operation")
135-
validateDiags := ret.Core.Validate(ret.Config, nil)
135+
// TODO: Implement query validate command. op.Query is false when running the command "terraform validate"
136+
opts := &terraform.ValidateOpts{Query: op.Query}
137+
validateDiags := ret.Core.Validate(ret.Config, opts)
136138
diags = diags.Append(validateDiags)
137139
}
138140
}

internal/terraform/context_plan_query_test.go

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -885,6 +885,94 @@ func TestContext2Plan_queryList(t *testing.T) {
885885
}
886886
},
887887
},
888+
{
889+
name: ".tf file blocks should not be processed in query mode",
890+
mainConfig: `
891+
terraform {
892+
required_providers {
893+
test = {
894+
source = "hashicorp/test"
895+
version = "1.0.0"
896+
}
897+
}
898+
}
899+
900+
locals {
901+
foo = "bar"
902+
}
903+
904+
// This would produce an error if triggered, but we expect it to be ignored in query mode
905+
resource "test_resource" "example" {
906+
instance_type = "ami-123456"
907+
908+
lifecycle {
909+
precondition {
910+
condition = local.foo != "bar"
911+
error_message = "This should not be executed"
912+
}
913+
}
914+
}
915+
916+
// This would produce an error if triggered, but we expect it to be ignored in query mode
917+
// output "resource_attr" {
918+
// value = sensitive(test_resource.example.instance_type)
919+
// }
920+
`,
921+
queryConfig: `
922+
list "test_resource" "test" {
923+
provider = test
924+
include_resource = true
925+
926+
config {
927+
filter = {
928+
attr = "foo"
929+
}
930+
}
931+
}
932+
`,
933+
listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse {
934+
madeUp := []cty.Value{
935+
cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}),
936+
cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-654321")}),
937+
cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-789012")}),
938+
}
939+
ids := []cty.Value{}
940+
for i := range madeUp {
941+
ids = append(ids, cty.ObjectVal(map[string]cty.Value{
942+
"id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)),
943+
}))
944+
}
945+
946+
resp := []cty.Value{}
947+
for i, v := range madeUp {
948+
mp := map[string]cty.Value{
949+
"identity": ids[i],
950+
"display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)),
951+
}
952+
if request.IncludeResourceObject {
953+
mp["state"] = v
954+
}
955+
resp = append(resp, cty.ObjectVal(mp))
956+
}
957+
958+
ret := request.Config.AsValueMap()
959+
maps.Copy(ret, map[string]cty.Value{
960+
"data": cty.TupleVal(resp),
961+
})
962+
963+
return providers.ListResourceResponse{Result: cty.ObjectVal(ret)}
964+
},
965+
assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) {
966+
expectedResources := []string{"list.test_resource.test"}
967+
actualResources := make([]string, 0)
968+
for _, change := range changes.Queries {
969+
actualResources = append(actualResources, change.Addr.String())
970+
}
971+
if diff := cmp.Diff(expectedResources, actualResources); diff != "" {
972+
t.Fatalf("Expected resources to match, but they differ: %s", diff)
973+
}
974+
},
975+
},
888976
}
889977

890978
for _, tc := range cases {
@@ -922,7 +1010,9 @@ func TestContext2Plan_queryList(t *testing.T) {
9221010
})
9231011
tfdiags.AssertNoDiagnostics(t, diags)
9241012

925-
diags = ctx.Validate(mod, &ValidateOpts{})
1013+
diags = ctx.Validate(mod, &ValidateOpts{
1014+
Query: true,
1015+
})
9261016
if tc.assertValidateDiags != nil {
9271017
tc.assertValidateDiags(t, diags)
9281018
return

internal/terraform/context_validate.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ type ValidateOpts struct {
3434
// not available to this function. Therefore, it is the responsibility of
3535
// the caller to ensure that the provider configurations are valid.
3636
ExternalProviders map[addrs.RootProviderConfig]providers.Interface
37+
38+
// When true, query files will also be validated.
39+
Query bool
3740
}
3841

3942
// Validate performs semantic validation of a configuration, and returns
@@ -105,6 +108,7 @@ func (c *Context) Validate(config *configs.Config, opts *ValidateOpts) tfdiags.D
105108
Operation: walkValidate,
106109
ExternalProviderConfigs: opts.ExternalProviders,
107110
ImportTargets: c.findImportTargets(config),
111+
queryPlan: opts.Query,
108112
}).Build(addrs.RootModuleInstance)
109113
diags = diags.Append(moreDiags)
110114
if moreDiags.HasErrors() {

internal/terraform/graph_builder_apply.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
122122
&ConfigTransformer{
123123
Concrete: concreteResource,
124124
Config: b.Config,
125-
resourceMatcher: func(mode addrs.ResourceMode) bool {
126-
// list resources are not added to the graph during apply
127-
return mode != addrs.ListResourceMode
128-
},
129125
},
130126

131127
// Add dynamic values
@@ -239,6 +235,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
239235
// Target
240236
&TargetsTransformer{Targets: b.Targets, ActionTargets: b.ActionTargets},
241237

238+
// Exclude list resources and their descendants from the apply graph
239+
&QueryTransformer{includeLists: false},
240+
242241
// Close any ephemeral resource instances.
243242
&ephemeralResourceCloseTransformer{},
244243

internal/terraform/graph_builder_plan.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
159159
ConcreteAction: b.ConcreteAction,
160160
Config: b.Config,
161161
destroy: b.Operation == walkDestroy || b.Operation == walkPlanDestroy,
162-
resourceMatcher: func(mode addrs.ResourceMode) bool {
163-
// all resources are included during validation.
164-
if b.Operation == walkValidate {
165-
return true
166-
}
167-
168-
return b.queryPlan == (mode == addrs.ListResourceMode)
169-
},
170162

171163
importTargets: b.ImportTargets,
172164

@@ -290,6 +282,11 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
290282
// Target
291283
&TargetsTransformer{Targets: b.Targets},
292284

285+
// Filter the graph to only include nodes that are relevant to the
286+
// current operation. In case of a query, this will filter out nodes
287+
// that are not relevant to the query.
288+
&QueryTransformer{includeLists: b.queryPlan},
289+
293290
// Detect when create_before_destroy must be forced on for a particular
294291
// node due to dependency edges, to avoid graph cycles during apply.
295292
&ForcedCBDTransformer{},

internal/terraform/transform_config.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ type ConfigTransformer struct {
5353
// try to delete the imported resource unless the config is updated
5454
// manually.
5555
generateConfigPathForImportTargets string
56-
57-
resourceMatcher func(addrs.ResourceMode) bool
5856
}
5957

6058
func (t *ConfigTransformer) Transform(g *Graph) error {
@@ -163,11 +161,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er
163161
continue
164162
}
165163

166-
if t.resourceMatcher != nil && !t.resourceMatcher(r.Mode) {
167-
// Skip resources that do not match the filter
168-
continue
169-
}
170-
171164
// Verify that any actions referenced in the resource's ActionTriggers exist in this module
172165
var diags tfdiags.Diagnostics
173166
if r.Managed != nil && r.Managed.ActionTriggers != nil {
@@ -262,10 +255,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er
262255
// If any import targets were not claimed by resources we may be
263256
// generating configuration. Add them to the graph for validation.
264257
for _, i := range importTargets {
265-
if t.resourceMatcher != nil && !t.resourceMatcher(i.Config.ToResource.Resource.Mode) {
266-
// Skip resources that do not match the filter
267-
continue
268-
}
269258

270259
log.Printf("[DEBUG] ConfigTransformer: adding config generation node for %s", i.Config.ToResource)
271260

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) HashiCorp, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package terraform
5+
6+
import (
7+
"github.com/hashicorp/terraform/internal/addrs"
8+
"github.com/hashicorp/terraform/internal/dag"
9+
)
10+
11+
type QueryTransformer struct {
12+
// includeLists is a flag that determines whether list resources should be included in the query.
13+
// If true, list resources and their dependencies will be included in the query. If false, list resources and their dependencies will be excluded.
14+
includeLists bool
15+
}
16+
17+
func (t *QueryTransformer) Transform(g *Graph) error {
18+
var nodesToRemove []dag.Vertex
19+
for v := range dag.SelectSeq[GraphNodeConfigResource](g.VerticesSeq()) {
20+
mode := v.ResourceAddr().Resource.Mode
21+
// The first condition checks if we want to include list resources, in which case we should remove all
22+
// non-list resources.
23+
// The second condition checks if we want to exclude list resources, in which case we should remove all
24+
// list resources.
25+
shouldRemove := (mode != addrs.ListResourceMode && t.includeLists) ||
26+
(mode == addrs.ListResourceMode && !t.includeLists)
27+
28+
// If the node is to be removed, we need to remove it and its descendants from the graph.
29+
if shouldRemove {
30+
deps := g.Descendants(v)
31+
deps.Add(v)
32+
for node := range deps {
33+
nodesToRemove = append(nodesToRemove, node)
34+
}
35+
}
36+
}
37+
38+
for _, node := range nodesToRemove {
39+
g.Remove(node)
40+
}
41+
42+
return nil
43+
}

0 commit comments

Comments
 (0)