diff --git a/internal/backend/local/backend_local.go b/internal/backend/local/backend_local.go index 962663af3573..4e900de9efd7 100644 --- a/internal/backend/local/backend_local.go +++ b/internal/backend/local/backend_local.go @@ -132,7 +132,9 @@ func (b *Local) localRun(op *backendrun.Operation) (*backendrun.LocalRun, *confi // If validation is enabled, validate if b.OpValidation { log.Printf("[TRACE] backend/local: running validation operation") - validateDiags := ret.Core.Validate(ret.Config, nil) + // TODO: Implement query validate command. op.Query is false when running the command "terraform validate" + opts := &terraform.ValidateOpts{Query: op.Query} + validateDiags := ret.Core.Validate(ret.Config, opts) diags = diags.Append(validateDiags) } } diff --git a/internal/terraform/context_apply2_test.go b/internal/terraform/context_apply2_test.go index 098288715d56..d9e6b16e23b2 100644 --- a/internal/terraform/context_apply2_test.go +++ b/internal/terraform/context_apply2_test.go @@ -4271,3 +4271,78 @@ resource "test_resource" "foo" { t.Fatalf("missing identity in state, got %q", fooState.Current.IdentityJSON) } } + +func TestContext2Apply_noListValidated(t *testing.T) { + tests := map[string]struct { + name string + mainConfig string + queryConfig string + query bool + }{ + "query files not validated in default validate mode": { + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + `, + queryConfig: ` + // This config is invalid, but should not be validated in default validate mode + list "test_resource" "test" { + provider = test + + config { + filter = { + attr = list.non_existent.attr + } + } + } + + locals { + test = list.non_existent.attr + } + `, + query: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + configFiles := map[string]string{"main.tf": tc.mainConfig} + if tc.queryConfig != "" { + configFiles["main.tfquery.hcl"] = tc.queryConfig + } + + opts := []configs.Option{} + if tc.query { + opts = append(opts, configs.MatchQueryFiles()) + } + + m := testModuleInline(t, configFiles, opts...) + + p := testProvider("test") + p.GetProviderSchemaResponse = getListProviderSchemaResp() + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + diags := ctx.Validate(m, &ValidateOpts{ + Query: tc.query, + }) + tfdiags.AssertNoErrors(t, diags) + + plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables))) + tfdiags.AssertNoErrors(t, diags) + + _, diags = ctx.Apply(plan, m, nil) + tfdiags.AssertNoErrors(t, diags) + }) + } +} diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 99a482682957..f9d8782397ea 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -176,12 +176,11 @@ list "test_resource" "test1" { }, }, "query run, action references resource": { - toBeImplemented: true, // TODO: Fix the graph built by query operations. module: map[string]string{ "main.tf": ` action "test_action" "hello" { config { - attr = resource.test_object.a + attr = resource.test_object.a.name } } resource "test_object" "a" { @@ -3313,7 +3312,17 @@ resource "test_object" "a" { t.Skip("Test not implemented yet") } - m := testModuleInline(t, tc.module) + opts := SimplePlanOpts(plans.NormalMode, InputValues{}) + if tc.planOpts != nil { + opts = tc.planOpts + } + + configOpts := []configs.Option{} + if opts.Query { + configOpts = append(configOpts, configs.MatchQueryFiles()) + } + + m := testModuleInline(t, tc.module, configOpts...) p := &testing_provider.MockProvider{ GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{ @@ -3444,7 +3453,9 @@ resource "test_object" "a" { }, }) - diags := ctx.Validate(m, &ValidateOpts{}) + diags := ctx.Validate(m, &ValidateOpts{ + Query: opts.Query, + }) if tc.expectValidateDiagnostics != nil { tfdiags.AssertDiagnosticsMatch(t, diags, tc.expectValidateDiagnostics(m)) } else if tc.assertValidateDiagnostics != nil { @@ -3462,11 +3473,6 @@ resource "test_object" "a" { prevRunState = states.BuildState(tc.buildState) } - opts := SimplePlanOpts(plans.NormalMode, InputValues{}) - if tc.planOpts != nil { - opts = tc.planOpts - } - plan, diags := ctx.Plan(m, prevRunState, opts) if tc.expectPlanDiagnostics != nil { diff --git a/internal/terraform/context_plan_query_test.go b/internal/terraform/context_plan_query_test.go index 7578fe0c1d8e..902f6cf972bc 100644 --- a/internal/terraform/context_plan_query_test.go +++ b/internal/terraform/context_plan_query_test.go @@ -5,7 +5,6 @@ package terraform import ( "fmt" - "maps" "sort" "strings" "testing" @@ -14,6 +13,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" @@ -24,6 +24,44 @@ import ( ) func TestContext2Plan_queryList(t *testing.T) { + listResourceFn := func(request providers.ListResourceRequest) providers.ListResourceResponse { + instanceTypes := []string{"ami-123456", "ami-654321", "ami-789012"} + madeUp := []cty.Value{} + for i := range len(instanceTypes) { + madeUp = append(madeUp, cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal(instanceTypes[i])})) + } + + ids := []cty.Value{} + for i := range madeUp { + ids = append(ids, cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), + })) + } + + resp := []cty.Value{} + for i, v := range madeUp { + mp := map[string]cty.Value{ + "identity": ids[i], + "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), + } + if request.IncludeResourceObject { + mp["state"] = v + } + resp = append(resp, cty.ObjectVal(mp)) + } + + ret := map[string]cty.Value{ + "data": cty.TupleVal(resp), + } + for k, v := range request.Config.AsValueMap() { + if k != "data" { + ret[k] = v + } + } + + return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} + } + cases := []struct { name string mainConfig string @@ -33,6 +71,7 @@ func TestContext2Plan_queryList(t *testing.T) { transformSchema func(*providers.GetProviderSchemaResponse) assertState func(*states.State) assertValidateDiags func(t *testing.T, diags tfdiags.Diagnostics) + assertPlanDiags func(t *testing.T, diags tfdiags.Diagnostics) assertChanges func(providers.ProviderSchema, *plans.ChangesSrc) listResourceFn func(request providers.ListResourceRequest) providers.ListResourceResponse }{ @@ -75,39 +114,8 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - generatedPath: t.TempDir(), - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-654321")}), - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-789012")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - mp := map[string]cty.Value{ - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - } - if request.IncludeResourceObject { - mp["state"] = v - } - resp = append(resp, cty.ObjectVal(mp)) - } - - ret := request.Config.AsValueMap() - maps.Copy(ret, map[string]cty.Value{ - "data": cty.TupleVal(resp), - }) - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + generatedPath: t.TempDir(), + listResourceFn: listResourceFn, assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { expectedResources := []string{"list.test_resource.test", "list.test_resource.test2"} actualResources := make([]string, 0) @@ -189,38 +197,7 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-654321")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - resp = append(resp, cty.ObjectVal(map[string]cty.Value{ - "state": v, - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - })) - } - - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } - } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + listResourceFn: listResourceFn, assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { expectedResources := []string{"list.test_resource.test[0]", "list.test_resource.test2"} actualResources := make([]string, 0) @@ -233,7 +210,7 @@ func TestContext2Plan_queryList(t *testing.T) { } // Verify instance types - expectedTypes := []string{"ami-123456", "ami-654321"} + expectedTypes := []string{"ami-123456", "ami-654321", "ami-789012"} actualTypes := make([]string, 0) obj := cs.Results.Value.GetAttr("data") if obj.IsNull() { @@ -369,38 +346,7 @@ func TestContext2Plan_queryList(t *testing.T) { } }, - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-654321")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - resp = append(resp, cty.ObjectVal(map[string]cty.Value{ - "state": v, - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - })) - } - - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } - } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + listResourceFn: listResourceFn, assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { expectedResources := []string{"list.test_resource.test"} actualResources := make([]string, 0) @@ -412,22 +358,22 @@ func TestContext2Plan_queryList(t *testing.T) { t.Fatalf("failed to decode change: %s", err) } - // Verify instance types - expectedTypes := []string{"ami-123456", "ami-654321"} + // Verify identities + expectedTypes := []string{"i-v1", "i-v2", "i-v3"} actualTypes := make([]string, 0) obj := cs.Results.Value.GetAttr("data") if obj.IsNull() { t.Fatalf("Expected 'data' attribute to be present, but it is null") } obj.ForEachElement(func(key cty.Value, val cty.Value) bool { - val = val.GetAttr("state") + val = val.GetAttr("identity") if val.IsNull() { - t.Fatalf("Expected 'state' attribute to be present, but it is null") + t.Fatalf("Expected 'identity' attribute to be present, but it is null") } - if val.GetAttr("instance_type").IsNull() { - t.Fatalf("Expected 'instance_type' attribute to be present, but it is missing") + if val.GetAttr("id").IsNull() { + t.Fatalf("Expected 'id' attribute to be present, but it is missing") } - actualTypes = append(actualTypes, val.GetAttr("instance_type").AsString()) + actualTypes = append(actualTypes, val.GetAttr("id").AsString()) return false }) sort.Strings(actualTypes) @@ -584,37 +530,7 @@ func TestContext2Plan_queryList(t *testing.T) { tfdiags.AssertDiagnosticsMatch(t, diags, exp) }, - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - resp = append(resp, cty.ObjectVal(map[string]cty.Value{ - "state": v, - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - })) - } - - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } - } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + listResourceFn: listResourceFn, }, { name: "circular reference between list resources", @@ -702,37 +618,7 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - resp = append(resp, cty.ObjectVal(map[string]cty.Value{ - "state": v, - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - })) - } - - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } - } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + listResourceFn: listResourceFn, assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { expectedResources := []string{"list.test_resource.test1", "list.test_resource.test2"} actualResources := make([]string, 0) @@ -745,7 +631,7 @@ func TestContext2Plan_queryList(t *testing.T) { } // Verify instance types - expectedTypes := []string{"ami-123456"} + expectedTypes := []string{"ami-123456", "ami-654321", "ami-789012"} actualTypes := make([]string, 0) obj := cs.Results.Value.GetAttr("data") if obj.IsNull() { @@ -812,37 +698,7 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - listResourceFn: func(request providers.ListResourceRequest) providers.ListResourceResponse { - madeUp := []cty.Value{ - cty.ObjectVal(map[string]cty.Value{"instance_type": cty.StringVal("ami-123456")}), - } - ids := []cty.Value{} - for i := range madeUp { - ids = append(ids, cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal(fmt.Sprintf("i-v%d", i+1)), - })) - } - - resp := []cty.Value{} - for i, v := range madeUp { - resp = append(resp, cty.ObjectVal(map[string]cty.Value{ - "state": v, - "identity": ids[i], - "display_name": cty.StringVal(fmt.Sprintf("Instance %d", i+1)), - })) - } - - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } - } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} - }, + listResourceFn: listResourceFn, assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { expectedResources := []string{"list.test_resource.test1[\"foo\"]", "list.test_resource.test1[\"bar\"]", "list.test_resource.test2[\"foo\"]", "list.test_resource.test2[\"bar\"]"} actualResources := make([]string, 0) @@ -855,7 +711,7 @@ func TestContext2Plan_queryList(t *testing.T) { } // Verify instance types - expectedTypes := []string{"ami-123456"} + expectedTypes := []string{"ami-123456", "ami-654321", "ami-789012"} actualTypes := make([]string, 0) obj := cs.Results.Value.GetAttr("data") if obj.IsNull() { @@ -885,16 +741,135 @@ func TestContext2Plan_queryList(t *testing.T) { } }, }, + { + name: ".tf file blocks should not be evaluated in query mode unless in path of list resources", + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + + locals { + foo = "bar" + // This local variable is not evaluated in query mode, but it is still validated + bar = resource.test_resource.example.instance_type + } + + // This would produce a plan error if triggered, but we expect it to be ignored in query mode + // because no list resource depends on it + resource "test_resource" "example" { + instance_type = "ami-123456" + + lifecycle { + precondition { + condition = local.foo != "bar" + error_message = "This should not be executed" + } + } + } + + `, + queryConfig: ` + list "test_resource" "test" { + provider = test + include_resource = true + + config { + filter = { + attr = "foo" + } + } + } + `, + listResourceFn: listResourceFn, + assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { + expectedResources := []string{"list.test_resource.test"} + actualResources := make([]string, 0) + for _, change := range changes.Queries { + actualResources = append(actualResources, change.Addr.String()) + } + if diff := cmp.Diff(expectedResources, actualResources); diff != "" { + t.Fatalf("Expected resources to match, but they differ: %s", diff) + } + }, + }, + { + name: "when list provider depends on managed resource", + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + + locals { + foo = "bar" + bar = resource.test_resource.example.instance_type + } + + provider "test" { + alias = "example" + region = resource.test_resource.example.instance_type + } + + // The list resource depends on this via the provider, + // so this resource will be evaluated as well. + resource "test_resource" "example" { + instance_type = "ami-123456" + } + + `, + queryConfig: ` + list "test_resource" "test" { + provider = test.example + include_resource = true + + config { + filter = { + attr = "foo" + } + } + } + `, + listResourceFn: listResourceFn, + assertChanges: func(ps providers.ProviderSchema, changes *plans.ChangesSrc) { + expectedListResources := []string{"list.test_resource.test"} + actualResources := make([]string, 0) + for _, change := range changes.Queries { + actualResources = append(actualResources, change.Addr.String()) + } + if diff := cmp.Diff(expectedListResources, actualResources); diff != "" { + t.Fatalf("Expected resources to match, but they differ: %s", diff) + } + + expectedManagedResources := []string{"test_resource.example"} + actualResources = make([]string, 0) + for _, change := range changes.Resources { + actualResources = append(actualResources, change.Addr.String()) + } + if diff := cmp.Diff(expectedManagedResources, actualResources); diff != "" { + t.Fatalf("Expected resources to match, but they differ: %s", diff) + } + + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - configs := map[string]string{"main.tf": tc.mainConfig} + configFiles := map[string]string{"main.tf": tc.mainConfig} if tc.queryConfig != "" { - configs["main.tfquery.hcl"] = tc.queryConfig + configFiles["main.tfquery.hcl"] = tc.queryConfig } - mod := testModuleInline(t, configs) + mod := testModuleInline(t, configFiles, configs.MatchQueryFiles()) providerAddr := addrs.NewDefaultProvider("test") provider := testProvider("test") provider.ConfigureProvider(providers.ConfigureProviderRequest{}) @@ -922,7 +897,9 @@ func TestContext2Plan_queryList(t *testing.T) { }) tfdiags.AssertNoDiagnostics(t, diags) - diags = ctx.Validate(mod, &ValidateOpts{}) + diags = ctx.Validate(mod, &ValidateOpts{ + Query: true, + }) if tc.assertValidateDiags != nil { tc.assertValidateDiags(t, diags) return @@ -936,7 +913,12 @@ func TestContext2Plan_queryList(t *testing.T) { Query: true, GenerateConfigPath: tc.generatedPath, }) - tfdiags.AssertNoDiagnostics(t, diags) + if tc.assertPlanDiags != nil { + tc.assertPlanDiags(t, diags) + return + } else { + tfdiags.AssertNoDiagnostics(t, diags) + } if tc.assertChanges != nil { sch, err := ctx.Schemas(mod, states.NewState()) @@ -1040,10 +1022,10 @@ func TestContext2Plan_queryListArgs(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - configs := map[string]string{"main.tf": mainConfig} - configs["main.tfquery.hcl"] = tc.queryConfig + configFiles := map[string]string{"main.tf": mainConfig} + configFiles["main.tfquery.hcl"] = tc.queryConfig - mod := testModuleInline(t, configs) + mod := testModuleInline(t, configFiles, configs.MatchQueryFiles()) providerAddr := addrs.NewDefaultProvider("test") provider := testProvider("test") @@ -1125,6 +1107,14 @@ func getListProviderSchemaResp() *providers.GetProviderSchemaResponse { } return getProviderSchemaResponseFromProviderSchema(&providerSchema{ + Provider: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "region": { + Type: cty.String, + Optional: true, + }, + }, + }, ResourceTypes: map[string]*configschema.Block{ "list": { Attributes: map[string]*configschema.Attribute{ diff --git a/internal/terraform/context_validate.go b/internal/terraform/context_validate.go index 5d22330e75db..d154590afccc 100644 --- a/internal/terraform/context_validate.go +++ b/internal/terraform/context_validate.go @@ -34,6 +34,9 @@ type ValidateOpts struct { // not available to this function. Therefore, it is the responsibility of // the caller to ensure that the provider configurations are valid. ExternalProviders map[addrs.RootProviderConfig]providers.Interface + + // When true, query files will also be validated. + Query bool } // Validate performs semantic validation of a configuration, and returns @@ -105,6 +108,7 @@ func (c *Context) Validate(config *configs.Config, opts *ValidateOpts) tfdiags.D Operation: walkValidate, ExternalProviderConfigs: opts.ExternalProviders, ImportTargets: c.findImportTargets(config), + queryPlan: opts.Query, }).Build(addrs.RootModuleInstance) diags = diags.Append(moreDiags) if moreDiags.HasErrors() { diff --git a/internal/terraform/context_validate_test.go b/internal/terraform/context_validate_test.go index 9344bbc0bfd8..44083b467e93 100644 --- a/internal/terraform/context_validate_test.go +++ b/internal/terraform/context_validate_test.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/providers" testing_provider "github.com/hashicorp/terraform/internal/providers/testing" @@ -3521,13 +3522,13 @@ func TestContext2Validate_queryList(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - configs := map[string]string{"main.tf": tc.mainConfig} + configFiles := map[string]string{"main.tf": tc.mainConfig} if tc.queryConfig != "" { - configs["main.tfquery.hcl"] = tc.queryConfig + configFiles["main.tfquery.hcl"] = tc.queryConfig } - maps.Copy(configs, tc.extraConfig) + maps.Copy(configFiles, tc.extraConfig) - m := testModuleInline(t, configs) + m := testModuleInline(t, configFiles, configs.MatchQueryFiles()) providerAddr := addrs.NewDefaultProvider("test") provider := testProvider("test") @@ -3555,7 +3556,9 @@ func TestContext2Validate_queryList(t *testing.T) { // true externally. provider.ConfigureProviderCalled = true - diags = ctx.Validate(m, nil) + diags = ctx.Validate(m, &ValidateOpts{ + Query: true, + }) if len(diags) != tc.diagCount { t.Fatalf("expected %d diagnostics, got %d \n -diags: %s", tc.diagCount, len(diags), diags) } @@ -3775,3 +3778,71 @@ resource "test_instance" "foo" { }) } } + +func TestContext2Validate_noListValidated(t *testing.T) { + tests := map[string]struct { + name string + mainConfig string + queryConfig string + query bool + }{ + "query files not validated in default validate mode": { + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + `, + queryConfig: ` + list "test_resource" "test" { + provider = test + + config { + filter = { + attr = list.non_existent.attr + } + } + } + + locals { + test = list.non_existent.attr + } + `, + query: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + configFiles := map[string]string{"main.tf": tc.mainConfig} + if tc.queryConfig != "" { + configFiles["main.tfquery.hcl"] = tc.queryConfig + } + + opts := []configs.Option{} + if tc.query { + opts = append(opts, configs.MatchQueryFiles()) + } + + m := testModuleInline(t, configFiles, opts...) + + p := testProvider("test") + p.GetProviderSchemaResponse = getListProviderSchemaResp() + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(p), + }, + }) + + diags := ctx.Validate(m, &ValidateOpts{ + Query: tc.query, + }) + tfdiags.AssertNoDiagnostics(t, diags) + }) + } +} diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index b81f1b6f2a2d..326a05a2dc7a 100644 --- a/internal/terraform/graph_builder_apply.go +++ b/internal/terraform/graph_builder_apply.go @@ -122,10 +122,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &ConfigTransformer{ Concrete: concreteResource, Config: b.Config, - resourceMatcher: func(mode addrs.ResourceMode) bool { - // list resources are not added to the graph during apply - return mode != addrs.ListResourceMode - }, }, // Add dynamic values diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 87a8dab98582..479db64030cb 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -159,14 +159,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { ConcreteAction: b.ConcreteAction, Config: b.Config, destroy: b.Operation == walkDestroy || b.Operation == walkPlanDestroy, - resourceMatcher: func(mode addrs.ResourceMode) bool { - // all resources are included during validation. - if b.Operation == walkValidate { - return true - } - - return b.queryPlan == (mode == addrs.ListResourceMode) - }, importTargets: b.ImportTargets, @@ -290,6 +282,9 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Target &TargetsTransformer{Targets: b.Targets}, + // Filter the graph to only include nodes that are relevant to the query operation. + &QueryTransformer{queryPlan: b.queryPlan, validate: b.Operation == walkValidate}, + // Detect when create_before_destroy must be forced on for a particular // node due to dependency edges, to avoid graph cycles during apply. &ForcedCBDTransformer{}, diff --git a/internal/terraform/terraform_test.go b/internal/terraform/terraform_test.go index a988d30fc6f4..14be79f5375b 100644 --- a/internal/terraform/terraform_test.go +++ b/internal/terraform/terraform_test.go @@ -89,7 +89,7 @@ func testModuleWithSnapshot(t *testing.T, name string) (*configs.Config, *config // testModuleInline takes a map of path -> config strings and yields a config // structure with those files loaded from disk -func testModuleInline(t testing.TB, sources map[string]string) *configs.Config { +func testModuleInline(t testing.TB, sources map[string]string, parserOpts ...configs.Option) *configs.Config { t.Helper() cfgPath, err := filepath.EvalSymlinks(t.TempDir()) @@ -97,8 +97,6 @@ func testModuleInline(t testing.TB, sources map[string]string) *configs.Config { t.Fatal(err) } - var queryOpt configs.Option - for path, configStr := range sources { dir := filepath.Dir(path) if dir != "." { @@ -118,15 +116,6 @@ func testModuleInline(t testing.TB, sources map[string]string) *configs.Config { if err != nil { t.Fatalf("Error creating temporary file for config: %s", err) } - - if strings.HasSuffix(path, "tfquery.hcl") || strings.HasSuffix(path, "tfquery.json") { - queryOpt = configs.MatchQueryFiles() - } - } - - var parserOpts []configs.Option - if queryOpt != nil { - parserOpts = append(parserOpts, queryOpt) } loader, cleanup := configload.NewLoaderForTests(t, parserOpts...) diff --git a/internal/terraform/transform_config.go b/internal/terraform/transform_config.go index 236e8ee6a945..8198faad96c2 100644 --- a/internal/terraform/transform_config.go +++ b/internal/terraform/transform_config.go @@ -53,8 +53,6 @@ type ConfigTransformer struct { // try to delete the imported resource unless the config is updated // manually. generateConfigPathForImportTargets string - - resourceMatcher func(addrs.ResourceMode) bool } func (t *ConfigTransformer) Transform(g *Graph) error { @@ -163,11 +161,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er continue } - if t.resourceMatcher != nil && !t.resourceMatcher(r.Mode) { - // Skip resources that do not match the filter - continue - } - // Verify that any actions referenced in the resource's ActionTriggers exist in this module var diags tfdiags.Diagnostics if r.Managed != nil && r.Managed.ActionTriggers != nil { @@ -262,10 +255,6 @@ func (t *ConfigTransformer) transformSingle(g *Graph, config *configs.Config) er // If any import targets were not claimed by resources we may be // generating configuration. Add them to the graph for validation. for _, i := range importTargets { - if t.resourceMatcher != nil && !t.resourceMatcher(i.Config.ToResource.Resource.Mode) { - // Skip resources that do not match the filter - continue - } log.Printf("[DEBUG] ConfigTransformer: adding config generation node for %s", i.Config.ToResource) diff --git a/internal/terraform/transform_query.go b/internal/terraform/transform_query.go new file mode 100644 index 000000000000..64c725772f58 --- /dev/null +++ b/internal/terraform/transform_query.go @@ -0,0 +1,56 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/dag" +) + +type QueryTransformer struct { + // queryPlan is true when we are planning list resources. + queryPlan bool + + // if validate is true, we are in validate mode and should not exclude any resources. + validate bool +} + +func (t *QueryTransformer) Transform(g *Graph) error { + if !t.queryPlan { + // if we are not running a query-specific operation, we don't need to transform the graph + // as query-related files would not have been part of the parsed config. + return nil + } + + if t.validate { + // if we are validating query files, we validate all resources + return nil + } + + // a set to hold the resources that we want to keep and vertices along its path. + keep := dag.Set{} + + for v := range dag.SelectSeq[GraphNodeConfigResource](g.VerticesSeq()) { + // we only get here if we are building a query plan, but not validating. + // + // By now, the graph already contains all resources from the config, including non-list resources. + // We start from each list resource node, look at its ancestors, and keep all vertices along its path. + if v.ResourceAddr().Resource.Mode == addrs.ListResourceMode { + keep.Add(v) + deps := g.Ancestors(v) + for node := range deps { + keep.Add(node) + } + } + } + + // Remove all nodes that are not in the keep set. + for v := range g.VerticesSeq() { + if _, ok := keep[v]; !ok { + g.Remove(v) + } + } + + return nil +}