diff --git a/.changes/v1.14/BUG FIXES-20250924-110416.yaml b/.changes/v1.14/BUG FIXES-20250924-110416.yaml new file mode 100644 index 000000000000..3ddb833d6ea4 --- /dev/null +++ b/.changes/v1.14/BUG FIXES-20250924-110416.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'console and test: return explicit diagnostics when referencing resources that were not included in the most recent operation.' +time: 2025-09-24T11:04:16.860364+02:00 +custom: + Issue: "37663" diff --git a/.changes/v1.14/BUG FIXES-20250926-113318.yaml b/.changes/v1.14/BUG FIXES-20250926-113318.yaml new file mode 100644 index 000000000000..3adfd48017f1 --- /dev/null +++ b/.changes/v1.14/BUG FIXES-20250926-113318.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'query: generate unique resource identifiers for results of expanded list resources' +time: 2025-09-26T11:33:18.241184+02:00 +custom: + Issue: "37681" diff --git a/.changes/v1.14/BUG FIXES-20250927-184134.yaml b/.changes/v1.14/BUG FIXES-20250927-184134.yaml new file mode 100644 index 000000000000..2473ffeff8cc --- /dev/null +++ b/.changes/v1.14/BUG FIXES-20250927-184134.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: The CLI now summarizes the number of actions invoked during `terraform apply`, matching the plan output. +time: 2025-09-27T18:41:34.771437+02:00 +custom: + Issue: "37689" diff --git a/.changes/v1.14/ENHANCEMENTS-20250919-115253.yaml b/.changes/v1.14/ENHANCEMENTS-20250919-115253.yaml new file mode 100644 index 000000000000..6f521d0c0173 --- /dev/null +++ b/.changes/v1.14/ENHANCEMENTS-20250919-115253.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: '`terraform stacks` command support for `-help` flag' +time: 2025-09-19T11:52:53.923764-04:00 +custom: + Issue: "37645" diff --git a/.changes/v1.14/ENHANCEMENTS-20250925-151237.yaml b/.changes/v1.14/ENHANCEMENTS-20250925-151237.yaml new file mode 100644 index 000000000000..6e6c3381c79f --- /dev/null +++ b/.changes/v1.14/ENHANCEMENTS-20250925-151237.yaml @@ -0,0 +1,5 @@ +kind: ENHANCEMENTS +body: "query: support offline validation of query files via -query flag in the validate command" +time: 2025-09-25T15:12:37.198573+02:00 +custom: + Issue: "37671" diff --git a/.changes/v1.14/NEW FEATURES-20250829-183404.yaml b/.changes/v1.14/NEW FEATURES-20250829-183404.yaml new file mode 100644 index 000000000000..e0751ba14201 --- /dev/null +++ b/.changes/v1.14/NEW FEATURES-20250829-183404.yaml @@ -0,0 +1,3 @@ +kind: NEW FEATURES +body: "**List Resources**: List resources can be defined in `*.tfquery.hcl` files and allow querying and filterting existing infrastructure." +time: 2025-08-29T18:34:04.250038+02:00 diff --git a/.changes/v1.14/NEW FEATURES-20250829-184206.yaml b/.changes/v1.14/NEW FEATURES-20250829-184206.yaml new file mode 100644 index 000000000000..ae79073e2f9a --- /dev/null +++ b/.changes/v1.14/NEW FEATURES-20250829-184206.yaml @@ -0,0 +1,3 @@ +kind: NEW FEATURES +body: "A new Terraform command `terraform query`: Executes list operations against existing infrastructure and displays the results. The command can optionally generate configuration for importing results into Terraform." +time: 2025-08-29T18:42:06.659172+02:00 diff --git a/CHANGELOG.md b/CHANGELOG.md index 002c7e0fc361..b1ccc4cdec1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,14 @@ NEW FEATURES: +* **List Resources**: List resources can be defined in `*.tfquery.hcl` files and allow querying and filterting existing infrastructure. + +* A new Terraform command `terraform query`: Executes list operations against existing infrastructure and displays the results. The command can optionally generate configuration for importing results into Terraform. + * A new GenerateResourceConfiguration RPC allows providers to create more precise configuration values during import. ([#37515](https://github.com/hashicorp/terraform/issues/37515)) +* New top-level Actions block: Actions are provider defined and meant to codify use cases outside the normal CRUD model in your Terraform configuration. Providers can define Actions like `aws_lambda_invoke` or `aws_cloudfront_create_invalidation` that do something imparative outside of Terraforms normal CRUD model. You can configure such a side-effect with an action block and have actions triggered through the lifecycle of a resource or through passing the `-invoke` CLI flag. ([#37553](https://github.com/hashicorp/terraform/issues/37553)) + ENHANCEMENTS: @@ -12,6 +18,8 @@ ENHANCEMENTS: * terraform test: ignore prevent_destroy attribute during when cleaning up tests" ([#37364](https://github.com/hashicorp/terraform/issues/37364)) +* `terraform stacks` command support for `-help` flag ([#37645](https://github.com/hashicorp/terraform/issues/37645)) + BUG FIXES: @@ -19,6 +27,8 @@ BUG FIXES: * Fix OSS backend proxy support by adding a proxy layer for OSS backend operations. Resolves hashicorp/terraform#36897. ([#36897](https://github.com/hashicorp/terraform/issues/36897)) +* console and test: return explicit diagnostics when referencing resources that were not included in the most recent operation. ([#37663](https://github.com/hashicorp/terraform/issues/37663)) + UPGRADE NOTES: diff --git a/CODEOWNERS b/CODEOWNERS index 3942a70431e1..d0ca00eb8506 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -32,6 +32,3 @@ builtin/provisioners/file @hashicorp/terraform-core builtin/provisioners/local-exec @hashicorp/terraform-core builtin/provisioners/remote-exec @hashicorp/terraform-core - -# temporary go.mod protection during freeze -/go.mod @tommyokeefe \ No newline at end of file diff --git a/commands.go b/commands.go index 79424845043b..b8489519b40b 100644 --- a/commands.go +++ b/commands.go @@ -276,6 +276,12 @@ func initCommands( }, nil }, + "query": func() (cli.Command, error) { + return &command.QueryCommand{ + Meta: meta, + }, nil + }, + "refresh": func() (cli.Command, error) { return &command.RefreshCommand{ Meta: meta, @@ -451,12 +457,6 @@ func initCommands( }, nil } - Commands["query"] = func() (cli.Command, error) { - return &command.QueryCommand{ - Meta: meta, - }, nil - } - Commands["test cleanup"] = func() (cli.Command, error) { return &command.TestCleanupCommand{ Meta: meta, 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/cloud/backend_query.go b/internal/cloud/backend_query.go index 3fa78d4bfa8f..a8759c5f5cb9 100644 --- a/internal/cloud/backend_query.go +++ b/internal/cloud/backend_query.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/terraform/internal/genconfig" "github.com/hashicorp/terraform/internal/terraform" "github.com/hashicorp/terraform/internal/tfdiags" + "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" ) @@ -276,7 +277,7 @@ func (b *Cloud) cancelQueryRun(cancelCtx context.Context, op *backendrun.Operati // formatIdentity formats the identity map into a string representation. // It flattens the map into a string of key=value pairs, separated by commas. func formatIdentity(identity map[string]json.RawMessage) string { - parts := make([]string, 0, len(identity)) + ctyObj := make(map[string]cty.Value, len(identity)) for key, value := range identity { ty, err := ctyjson.ImpliedType(value) if err != nil { @@ -286,9 +287,9 @@ func formatIdentity(identity map[string]json.RawMessage) string { if err != nil { continue } - parts = append(parts, fmt.Sprintf("%s=%s", key, tfdiags.ValueToString(v))) + ctyObj[key] = v } - return strings.Join(parts, ",") + return tfdiags.ObjectToString(cty.ObjectVal(ctyObj)) } const queryDefaultHeader = ` diff --git a/internal/cloud/backend_query_test.go b/internal/cloud/backend_query_test.go index 31fab4db2402..83feb260a6dc 100644 --- a/internal/cloud/backend_query_test.go +++ b/internal/cloud/backend_query_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/hashicorp/cli" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/backend/backendrun" @@ -121,8 +122,15 @@ func TestCloud_queryJSONBasic(t *testing.T) { outp := close(t) gotOut := outp.Stdout() - if !strings.Contains(gotOut, "list.concept_pet.pets id=complete-gannet,legs=6 This is a complete-gannet") { - t.Fatalf("expected query results in output: %s", gotOut) + expectedOut := `list.concept_pet.pets id=large-roughy,legs=2 This is a large-roughy +list.concept_pet.pets id=able-werewolf,legs=5 This is a able-werewolf +list.concept_pet.pets id=complete-gannet,legs=6 This is a complete-gannet +list.concept_pet.pets id=charming-beagle,legs=3 This is a charming-beagle +list.concept_pet.pets id=legal-lamprey,legs=2 This is a legal-lamprey + +` + if diff := cmp.Diff(expectedOut, gotOut); diff != "" { + t.Fatalf("expected query results output to be %s, got %s: diff: %s", expectedOut, gotOut, diff) } stateMgr, _ := b.StateMgr(testBackendSingleWorkspaceName) diff --git a/internal/cloud/testdata/query-json-basic/main.tfquery.hcl b/internal/cloud/testdata/query-json-basic/main.tfquery.hcl index 4652ae7d19da..a3276bd012a1 100644 --- a/internal/cloud/testdata/query-json-basic/main.tfquery.hcl +++ b/internal/cloud/testdata/query-json-basic/main.tfquery.hcl @@ -1 +1,3 @@ -list "concept_pet" "pets" {} +list "concept_pet" "pets" { + provider = concept +} diff --git a/internal/cloud/testdata/query/main.tfquery.hcl b/internal/cloud/testdata/query/main.tfquery.hcl index f8c5bb1924c3..58f63c04942e 100644 --- a/internal/cloud/testdata/query/main.tfquery.hcl +++ b/internal/cloud/testdata/query/main.tfquery.hcl @@ -1 +1,3 @@ -list "null_resource" "foo" {} +list "null_resource" "foo" { + provider = null +} diff --git a/internal/command/arguments/validate.go b/internal/command/arguments/validate.go index 7df4b2c5d9e8..4347f39eb1cf 100644 --- a/internal/command/arguments/validate.go +++ b/internal/command/arguments/validate.go @@ -24,6 +24,9 @@ type Validate struct { // ViewType specifies which output format to use: human, JSON, or "raw". ViewType ViewType + + // Query indicates that Terraform should also validate .tfquery files. + Query bool } // ParseValidate processes CLI arguments, returning a Validate value and errors. @@ -40,6 +43,7 @@ func ParseValidate(args []string) (*Validate, tfdiags.Diagnostics) { cmdFlags.BoolVar(&jsonOutput, "json", false, "json") cmdFlags.StringVar(&validate.TestDirectory, "test-directory", "tests", "test-directory") cmdFlags.BoolVar(&validate.NoTests, "no-tests", false, "no-tests") + cmdFlags.BoolVar(&validate.Query, "query", false, "query") if err := cmdFlags.Parse(args); err != nil { diags = diags.Append(tfdiags.Sourceless( diff --git a/internal/command/e2etest/providers_schema_test.go b/internal/command/e2etest/providers_schema_test.go index 7a5d7dea8d47..582a21710c21 100644 --- a/internal/command/e2etest/providers_schema_test.go +++ b/internal/command/e2etest/providers_schema_test.go @@ -134,6 +134,21 @@ func TestProvidersSchema(t *testing.T) { } } }, + "list_resource_schemas": { + "simple_resource": { + "version": 0, + "block": { + "attributes": { + "value": { + "type": "string", + "description_kind": "plain", + "optional": true + } + }, + "description_kind": "plain" + } + } + }, "resource_identity_schemas": { "simple_resource": { "version": 0, @@ -213,6 +228,21 @@ func TestProvidersSchema(t *testing.T) { } } }, + "list_resource_schemas": { + "simple_resource": { + "version": 0, + "block": { + "attributes": { + "value": { + "type": "string", + "description_kind": "plain", + "optional": true + } + }, + "description_kind": "plain" + } + } + }, "functions": { "noop": { "description": "noop takes any single argument and returns the same value", diff --git a/internal/command/jsonformat/plan_test.go b/internal/command/jsonformat/plan_test.go index f971c08d02af..5f99c2fd9e7e 100644 --- a/internal/command/jsonformat/plan_test.go +++ b/internal/command/jsonformat/plan_test.go @@ -8071,7 +8071,7 @@ func runTestCases(t *testing.T, testCases map[string]testCase) { return } - jsonschemas := jsonprovider.MarshalForRenderer(tfschemas, false) + jsonschemas := jsonprovider.MarshalForRenderer(tfschemas) change := structured.FromJsonChange(jsonchanges[0].Change, attribute_path.AlwaysMatcher()) renderer := Renderer{Colorize: color} diff := diff{ @@ -8421,7 +8421,7 @@ func TestResourceChange_deferredActions(t *testing.T) { } renderer := Renderer{Colorize: color} - jsonschemas := jsonprovider.MarshalForRenderer(fullSchema, false) + jsonschemas := jsonprovider.MarshalForRenderer(fullSchema) diffs := precomputeDiffs(Plan{ DeferredChanges: deferredChanges, ProviderSchemas: jsonschemas, @@ -8714,7 +8714,7 @@ func TestResourceChange_actions(t *testing.T) { }, }, } - jsonschemas := jsonprovider.MarshalForRenderer(fullSchema, false) + jsonschemas := jsonprovider.MarshalForRenderer(fullSchema) diffs := precomputeDiffs(Plan{ ResourceChanges: []jsonplan.ResourceChange{defaultResourceChange}, ActionInvocations: tc.actionInvocations, diff --git a/internal/command/jsonformat/state_test.go b/internal/command/jsonformat/state_test.go index 9c4eafa50b23..b8350f29bbbb 100644 --- a/internal/command/jsonformat/state_test.go +++ b/internal/command/jsonformat/state_test.go @@ -86,7 +86,7 @@ func TestState(t *testing.T) { RootModule: root, RootModuleOutputs: outputs, ProviderFormatVersion: jsonprovider.FormatVersion, - ProviderSchemas: jsonprovider.MarshalForRenderer(tt.Schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(tt.Schemas), }) result := done(t).All() diff --git a/internal/command/jsonprovider/provider.go b/internal/command/jsonprovider/provider.go index 0ebb76f82b91..10fdcf7831f4 100644 --- a/internal/command/jsonprovider/provider.go +++ b/internal/command/jsonprovider/provider.go @@ -45,22 +45,22 @@ func newProviders() *Providers { // schema into the public structured JSON versions. // // This is a format that can be read by the structured plan renderer. -func MarshalForRenderer(s *terraform.Schemas, includeExperimentalSchemas bool) map[string]*Provider { +func MarshalForRenderer(s *terraform.Schemas) map[string]*Provider { schemas := make(map[string]*Provider, len(s.Providers)) for k, v := range s.Providers { - schemas[k.String()] = marshalProvider(v, includeExperimentalSchemas) + schemas[k.String()] = marshalProvider(v) } return schemas } -func Marshal(s *terraform.Schemas, includeExperimentalSchemas bool) ([]byte, error) { +func Marshal(s *terraform.Schemas) ([]byte, error) { providers := newProviders() - providers.Schemas = MarshalForRenderer(s, includeExperimentalSchemas) + providers.Schemas = MarshalForRenderer(s) ret, err := json.Marshal(providers) return ret, err } -func marshalProvider(tps providers.ProviderSchema, includeExperimentalSchemas bool) *Provider { +func marshalProvider(tps providers.ProviderSchema) *Provider { p := &Provider{ Provider: marshalSchema(tps.Provider), ResourceSchemas: marshalSchemas(tps.ResourceTypes), @@ -71,20 +71,18 @@ func marshalProvider(tps providers.ProviderSchema, includeExperimentalSchemas bo ActionSchemas: marshalActionSchemas(tps.Actions), } - if includeExperimentalSchemas { - // List resource schemas are nested under a "config" block, so we need to - // extract that block to get the actual provider schema for the list resource. - // When getting the provider schemas, Terraform adds this extra level to - // better match the actual configuration structure. - listSchemas := make(map[string]providers.Schema, len(tps.ListResourceTypes)) - for k, v := range tps.ListResourceTypes { - listSchemas[k] = providers.Schema{ - Body: &v.Body.BlockTypes["config"].Block, - Version: v.Version, - } + // List resource schemas are nested under a "config" block, so we need to + // extract that block to get the actual provider schema for the list resource. + // When getting the provider schemas, Terraform adds this extra level to + // better match the actual configuration structure. + listSchemas := make(map[string]providers.Schema, len(tps.ListResourceTypes)) + for k, v := range tps.ListResourceTypes { + listSchemas[k] = providers.Schema{ + Body: &v.Body.BlockTypes["config"].Block, + Version: v.Version, } - p.ListResourceSchemas = marshalSchemas(listSchemas) } + p.ListResourceSchemas = marshalSchemas(listSchemas) return p } diff --git a/internal/command/jsonprovider/provider_test.go b/internal/command/jsonprovider/provider_test.go index e65e4867869b..bb9ca97486b1 100644 --- a/internal/command/jsonprovider/provider_test.go +++ b/internal/command/jsonprovider/provider_test.go @@ -20,25 +20,23 @@ var cmpOpts = cmpopts.IgnoreUnexported(Provider{}) func TestMarshalProvider(t *testing.T) { tests := []struct { - Input providers.ProviderSchema - IncludeExperimental bool - Want *Provider + Input providers.ProviderSchema + Want *Provider }{ { providers.ProviderSchema{}, - false, &Provider{ Provider: &Schema{}, ResourceSchemas: map[string]*Schema{}, DataSourceSchemas: map[string]*Schema{}, EphemeralResourceSchemas: map[string]*Schema{}, ResourceIdentitySchemas: map[string]*IdentitySchema{}, + ListResourceSchemas: map[string]*Schema{}, ActionSchemas: map[string]*ActionSchema{}, }, }, { testProvider(), - false, &Provider{ Provider: &Schema{ Block: &Block{ @@ -212,53 +210,6 @@ func TestMarshalProvider(t *testing.T) { }, }, }, - ResourceIdentitySchemas: map[string]*IdentitySchema{}, - ActionSchemas: map[string]*ActionSchema{}, - }, - }, - { - providers.ProviderSchema{ - ListResourceTypes: map[string]providers.Schema{ - "test_list_resource": { - Version: 1, - Body: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "data": { - Type: cty.DynamicPseudoType, - Computed: true, - }, - }, - BlockTypes: map[string]*configschema.NestedBlock{ - "config": { - Block: configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "filter": {Type: cty.String, Optional: true}, - "items": {Type: cty.List(cty.String), Required: true}, - }, - }, - Nesting: configschema.NestingSingle, - }, - }, - }, - }, - }, - Actions: map[string]providers.ActionSchema{ - "test_action": { - ConfigSchema: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "opt_attr": {Type: cty.String, Optional: true}, - "req_attr": {Type: cty.List(cty.String), Required: true}, - }, - }, - }, - }, - }, - true, - &Provider{ - Provider: &Schema{}, - ResourceSchemas: map[string]*Schema{}, - DataSourceSchemas: map[string]*Schema{}, - EphemeralResourceSchemas: map[string]*Schema{}, ListResourceSchemas: map[string]*Schema{ "test_list_resource": { Version: 1, @@ -305,7 +256,7 @@ func TestMarshalProvider(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprint(i), func(t *testing.T) { - got := marshalProvider(test.Input, test.IncludeExperimental) + got := marshalProvider(test.Input) if diff := cmp.Diff(test.Want, got, cmpOpts); diff != "" { t.Fatalf("wrong result:\n %s\n", diff) } @@ -431,5 +382,15 @@ func testProvider() providers.ProviderSchema { }, }, }, + Actions: map[string]providers.ActionSchema{ + "test_action": { + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "opt_attr": {Type: cty.String, Optional: true}, + "req_attr": {Type: cty.List(cty.String), Required: true}, + }, + }, + }, + }, } } diff --git a/internal/command/meta.go b/internal/command/meta.go index 0dc89ecdd0d8..1626109fbfb4 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -272,6 +272,9 @@ type Meta struct { // Used with commands which write state to allow users to write remote // state even if the remote and local Terraform versions don't match. ignoreRemoteVersion bool + + // set to true if query files should be parsed + includeQueryFiles bool } type testingOverrides struct { diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index 3887d77db420..6485a4347916 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -38,7 +38,7 @@ func (m *Meta) normalizePath(path string) string { // loadConfig reads a configuration from the given directory, which should // contain a root module and have already have any required descendant modules // installed. -func (m *Meta) loadConfig(rootDir string, parserOpts ...configs.Option) (*configs.Config, tfdiags.Diagnostics) { +func (m *Meta) loadConfig(rootDir string) (*configs.Config, tfdiags.Diagnostics) { var diags tfdiags.Diagnostics rootDir = m.normalizePath(rootDir) @@ -48,7 +48,7 @@ func (m *Meta) loadConfig(rootDir string, parserOpts ...configs.Option) (*config return nil, diags } - config, hclDiags := loader.LoadConfig(rootDir, parserOpts...) + config, hclDiags := loader.LoadConfig(rootDir) diags = diags.Append(hclDiags) return config, diags } @@ -353,8 +353,9 @@ func (m *Meta) registerSynthConfigSource(filename string, src []byte) { func (m *Meta) initConfigLoader() (*configload.Loader, error) { if m.configLoader == nil { loader, err := configload.NewLoader(&configload.Config{ - ModulesDir: m.modulesDir(), - Services: m.Services, + ModulesDir: m.modulesDir(), + Services: m.Services, + IncludeQueryFiles: m.includeQueryFiles, }) if err != nil { return nil, err diff --git a/internal/command/plan.go b/internal/command/plan.go index a6f0b1400181..73336696c4b5 100644 --- a/internal/command/plan.go +++ b/internal/command/plan.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/hashicorp/terraform/internal/backend/backendrun" + "github.com/hashicorp/terraform/internal/backend/local" "github.com/hashicorp/terraform/internal/command/arguments" "github.com/hashicorp/terraform/internal/command/views" "github.com/hashicorp/terraform/internal/tfdiags" @@ -78,6 +79,18 @@ func (c *PlanCommand) Run(rawArgs []string) int { return 1 } + if len(args.Operation.ActionTargets) > 0 { + if _, ok := be.(*local.Local); !ok { + // Temporarily block invoking actions when executing anything other + // than locally. + // TODO: Remove this when TFC supports remote operation of action + // invoke plans. + diags = diags.Append(tfdiags.Sourceless(tfdiags.Error, "Invalid argument", "The -invoke argument can currently only be used when Terraform is executing locally.")) + view.Diagnostics(diags) + return 1 + } + } + // Build the operation request opReq, opDiags := c.OperationRequest(be, view, args.ViewType, args.Operation, args.OutPath, args.GenerateConfigPath) diags = diags.Append(opDiags) diff --git a/internal/command/plan_test.go b/internal/command/plan_test.go index e77ae0eb6ce6..b3e726c8c00f 100644 --- a/internal/command/plan_test.go +++ b/internal/command/plan_test.go @@ -1588,6 +1588,56 @@ func TestPlan_jsonGoldenReference(t *testing.T) { checkGoldenReference(t, output, "plan") } +// Tests related to how plan command behaves when there are query files in the configuration path +func TestPlan_QueryFiles(t *testing.T) { + // a plan succeeds regardless of valid or invalid + // tfquery files in the configuration path + t.Run("with invalid query files in the config path", func(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath("query/invalid-syntax"), td) + t.Chdir(td) + + p := planFixtureProvider() + view, done := testView(t) + c := &PlanCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + + args := []string{} + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: %d\n\n%s", code, output.Stderr()) + } + }) + + // the duplicate in the query should not matter because query files are not processed + t.Run("with duplicate variables across query and plan file", func(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath("query/duplicate-variables"), td) + t.Chdir(td) + + p := planFixtureProvider() + view, done := testView(t) + c := &PlanCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(p), + View: view, + }, + } + + args := []string{"-var", "instance_name=foo"} + code := c.Run(args) + output := done(t) + if code != 0 { + t.Fatalf("bad: %d\n\n%s", code, output.Stderr()) + } + }) +} + // planFixtureSchema returns a schema suitable for processing the // configuration in testdata/plan . This schema should be // assigned to a mock provider named "test". diff --git a/internal/command/providers_schema.go b/internal/command/providers_schema.go index 0d51769133d3..919e1a57078c 100644 --- a/internal/command/providers_schema.go +++ b/internal/command/providers_schema.go @@ -107,7 +107,7 @@ func (c *ProvidersSchemaCommand) Run(args []string) int { return 1 } - jsonSchemas, err := jsonprovider.Marshal(schemas, c.AllowExperimentalFeatures) + jsonSchemas, err := jsonprovider.Marshal(schemas) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to marshal provider schemas to json: %s", err)) return 1 diff --git a/internal/command/query.go b/internal/command/query.go index 6e1d00d73097..5faf2f0616d8 100644 --- a/internal/command/query.go +++ b/internal/command/query.go @@ -75,6 +75,7 @@ func (c *QueryCommand) Run(rawArgs []string) int { // migrated to views. c.Meta.color = !common.NoColor c.Meta.Color = c.Meta.color + c.Meta.includeQueryFiles = true // Parse and validate flags args, diags := arguments.ParseQuery(rawArgs) diff --git a/internal/command/query_test.go b/internal/command/query_test.go index c95b91c5c4f0..e36c3e0fd1fb 100644 --- a/internal/command/query_test.go +++ b/internal/command/query_test.go @@ -79,7 +79,7 @@ configuration file (.tfquery.hcl file) and try again. name: "invalid query syntax", directory: "invalid-syntax", expectedOut: "", - initCode: 1, + initCode: 0, expectedErr: []string{` Error: Unsupported block type @@ -172,21 +172,6 @@ value. Use a -var or -var-file command line argument to provide a value for this variable. `}, }, - { - name: "error - duplicate variable across .tf and .tfquery files", - directory: "duplicate-variables", - expectedOut: "", - expectedErr: []string{` -Error: Duplicate variable declaration - - on query.tfquery.hcl line 2: - 2: variable "instance_name" { - -A variable named "instance_name" was already declared at main.tf:15,1-25. -Variable names must be unique within a module. -`}, - initCode: 1, - }, } for _, ts := range tests { @@ -285,7 +270,9 @@ func queryFixtureProvider() *testing_provider.MockProvider { }, }, }, - Nesting: configschema.NestingSingle, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, }, }, } @@ -306,7 +293,9 @@ func queryFixtureProvider() *testing_provider.MockProvider { }, }, }, - Nesting: configschema.NestingSingle, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, }, }, } @@ -546,7 +535,7 @@ func TestQuery_JSON(t *testing.T) { "ami": "ami-12345", "id": "test-instance-1", }, - "config": "resource \"test_instance\" \"example_0\" {\n provider = test\n ami = \"ami-12345\"\n id = \"test-instance-1\"\n}", + "config": "resource \"test_instance\" \"example_0\" {\n provider = test\n ami = \"ami-12345\"\n}", "import_config": "import {\n to = test_instance.example_0\n provider = test\n identity = {\n id = \"test-instance-1\"\n }\n}", }, "type": "list_resource_found", @@ -566,7 +555,7 @@ func TestQuery_JSON(t *testing.T) { "ami": "ami-67890", "id": "test-instance-2", }, - "config": "resource \"test_instance\" \"example_1\" {\n provider = test\n ami = \"ami-67890\"\n id = \"test-instance-2\"\n}", + "config": "resource \"test_instance\" \"example_1\" {\n provider = test\n ami = \"ami-67890\"\n}", "import_config": "import {\n to = test_instance.example_1\n provider = test\n identity = {\n id = \"test-instance-2\"\n }\n}", }, "type": "list_resource_found", diff --git a/internal/command/stacks.go b/internal/command/stacks.go index 472d0ede4ec6..95e310648dd0 100644 --- a/internal/command/stacks.go +++ b/internal/command/stacks.go @@ -82,7 +82,6 @@ func (c *StacksCommand) realRun(args []string, stdout, stderr io.Writer) int { args = c.Meta.process(args) cmdFlags := c.Meta.defaultFlagSet("stacks") cmdFlags.StringVar(&pluginCacheDirOverride, "plugin-cache-dir", "", "plugin cache directory") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } cmdFlags.Parse(args) if pluginCacheDirOverride != "" { @@ -379,8 +378,17 @@ func (c *StacksCommand) Run(args []string) int { // Help returns help text for the stacks command. func (c *StacksCommand) Help() string { helpText := new(bytes.Buffer) - if exitCode := c.realRun([]string{}, helpText, io.Discard); exitCode != 0 { - return "" + errorText := new(bytes.Buffer) + + parsedArgs := []string{} + for _, arg := range os.Args[1:] { + if arg == "stacks" { + continue // skip stacks command name + } + parsedArgs = append(parsedArgs, arg) + } + if exitCode := c.realRun(parsedArgs, helpText, errorText); exitCode != 0 { + return errorText.String() } return helpText.String() diff --git a/internal/command/state_show.go b/internal/command/state_show.go index 4ef3bd80977c..b76d398ccd45 100644 --- a/internal/command/state_show.go +++ b/internal/command/state_show.go @@ -155,7 +155,7 @@ func (c *StateShowCommand) Run(args []string) int { ProviderFormatVersion: jsonprovider.FormatVersion, RootModule: root, RootModuleOutputs: outputs, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), } renderer := jsonformat.Renderer{ diff --git a/internal/command/test_test.go b/internal/command/test_test.go index 36b05cb9e418..7d8e6a91c5b4 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -412,11 +412,6 @@ func TestTest_Runs(t *testing.T) { "no-tests": { code: 0, }, - "expect-failures-assertions": { - expectedOut: []string{"0 passed, 1 failed."}, - expectedErr: []string{"Test assertion failed"}, - code: 1, - }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { @@ -5454,6 +5449,130 @@ func TestTest_JUnitOutput(t *testing.T) { } } +func TestTest_ReferencesIntoIncompletePlan(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(path.Join("test", "expect-failures-assertions")), td) + t.Chdir(td) + + provider := testing_command.NewProvider(nil) + providerSource, close := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer close() + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + ui := new(cli.MockUi) + + meta := Meta{ + testingOverrides: metaOverridesForProvider(provider.Provider), + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + } + + init := &InitCommand{ + Meta: meta, + } + + if code := init.Run(nil); code != 0 { + output := done(t) + t.Fatalf("expected status code %d but got %d: %s", 0, code, output.All()) + } + + // Reset the streams for the next command. + streams, done = terminal.StreamsForTesting(t) + meta.Streams = streams + meta.View = views.NewView(streams) + + c := &TestCommand{ + Meta: meta, + } + + code := c.Run([]string{"-no-color"}) + if code != 1 { + t.Errorf("expected status code %d but got %d", 0, code) + } + output := done(t) + + out, err := output.Stdout(), output.Stderr() + + expectedOut := `main.tftest.hcl... in progress + run "fail"... fail +main.tftest.hcl... tearing down +main.tftest.hcl... fail + +Failure! 0 passed, 1 failed. +` + + if diff := cmp.Diff(out, expectedOut); len(diff) > 0 { + t.Errorf("expected:\n%s\nactual:\n%s\ndiff:\n%s", expectedOut, out, diff) + } + + if !strings.Contains(err, "Reference to uninitialized resource") { + t.Errorf("missing reference to uninitialized resource error: \n%s", err) + } + + if !strings.Contains(err, "Reference to uninitialized local") { + t.Errorf("missing reference to uninitialized local error: \n%s", err) + } +} + +func TestTest_ReferencesIntoTargetedPlan(t *testing.T) { + td := t.TempDir() + testCopyDir(t, testFixturePath(path.Join("test", "invalid-reference-with-target")), td) + t.Chdir(td) + + provider := testing_command.NewProvider(nil) + providerSource, close := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer close() + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + ui := new(cli.MockUi) + + meta := Meta{ + testingOverrides: metaOverridesForProvider(provider.Provider), + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + } + + init := &InitCommand{ + Meta: meta, + } + + if code := init.Run(nil); code != 0 { + output := done(t) + t.Fatalf("expected status code %d but got %d: %s", 0, code, output.All()) + } + + // Reset the streams for the next command. + streams, done = terminal.StreamsForTesting(t) + meta.Streams = streams + meta.View = views.NewView(streams) + + c := &TestCommand{ + Meta: meta, + } + + code := c.Run([]string{"-no-color"}) + if code != 1 { + t.Errorf("expected status code %d but got %d", 0, code) + } + output := done(t) + + err := output.Stderr() + + if !strings.Contains(err, "Reference to uninitialized variable") { + t.Errorf("missing reference to uninitialized variable error: \n%s", err) + } +} + // https://github.com/hashicorp/terraform/issues/37546 func TestTest_TeardownOrder(t *testing.T) { td := t.TempDir() diff --git a/internal/command/testdata/query/invalid-traversal/main.tf b/internal/command/testdata/query/invalid-traversal/main.tf new file mode 100644 index 000000000000..4a6f9760d0ec --- /dev/null +++ b/internal/command/testdata/query/invalid-traversal/main.tf @@ -0,0 +1,9 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +} + +provider "test" {} diff --git a/internal/command/testdata/query/invalid-traversal/main.tfquery.hcl b/internal/command/testdata/query/invalid-traversal/main.tfquery.hcl new file mode 100644 index 000000000000..71139321a6bf --- /dev/null +++ b/internal/command/testdata/query/invalid-traversal/main.tfquery.hcl @@ -0,0 +1,21 @@ +variable "input" { + type = string + default = "foo" +} + +list "test_instance" "test" { + provider = test + + config { + ami = var.input + } +} + +list "test_instance" "test2" { + provider = test + + config { + // this traversal is invalid for a list resource + ami = list.test_instance.test.state.instance_type + } +} diff --git a/internal/command/testdata/test/invalid-reference-with-target/main.tf b/internal/command/testdata/test/invalid-reference-with-target/main.tf new file mode 100644 index 000000000000..1b912297a3b5 --- /dev/null +++ b/internal/command/testdata/test/invalid-reference-with-target/main.tf @@ -0,0 +1,10 @@ + +variable "input" { + type = string +} + +resource "test_resource" "one" { + value = var.input +} + +resource "test_resource" "two" {} \ No newline at end of file diff --git a/internal/command/testdata/test/invalid-reference-with-target/main.tftest.hcl b/internal/command/testdata/test/invalid-reference-with-target/main.tftest.hcl new file mode 100644 index 000000000000..9b9aac66126d --- /dev/null +++ b/internal/command/testdata/test/invalid-reference-with-target/main.tftest.hcl @@ -0,0 +1,17 @@ + +run "test" { + command = plan + + plan_options { + target = [test_resource.two] + } + + variables { + input = "hello" + } + + assert { + condition = var.input == "hello" + error_message = "wrong input" + } +} \ No newline at end of file diff --git a/internal/command/validate.go b/internal/command/validate.go index a574f00c0fc2..f679e2fae8b8 100644 --- a/internal/command/validate.go +++ b/internal/command/validate.go @@ -19,6 +19,8 @@ import ( // ValidateCommand is a Command implementation that validates the terraform files type ValidateCommand struct { Meta + + ParsedArgs *arguments.Validate } func (c *ValidateCommand) Run(rawArgs []string) int { @@ -34,6 +36,7 @@ func (c *ValidateCommand) Run(rawArgs []string) int { return 1 } + c.ParsedArgs = args view := views.NewValidate(args.ViewType, c.View) // After this point, we must only produce JSON output if JSON mode is @@ -54,7 +57,7 @@ func (c *ValidateCommand) Run(rawArgs []string) int { return view.Results(diags) } - validateDiags := c.validate(dir, args.TestDirectory, args.NoTests) + validateDiags := c.validate(dir) diags = diags.Append(validateDiags) // Validating with dev overrides in effect means that the result might @@ -66,47 +69,54 @@ func (c *ValidateCommand) Run(rawArgs []string) int { return view.Results(diags) } -func (c *ValidateCommand) validate(dir, testDir string, noTests bool) tfdiags.Diagnostics { +func (c *ValidateCommand) validate(dir string) tfdiags.Diagnostics { var diags tfdiags.Diagnostics var cfg *configs.Config - if noTests { + // If the query flag is set, include query files in the validation. + c.includeQueryFiles = c.ParsedArgs.Query + + if c.ParsedArgs.NoTests { cfg, diags = c.loadConfig(dir) } else { - cfg, diags = c.loadConfigWithTests(dir, testDir) + cfg, diags = c.loadConfigWithTests(dir, c.ParsedArgs.TestDirectory) } if diags.HasErrors() { return diags } - validate := func(cfg *configs.Config) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics + diags = diags.Append(c.validateConfig(cfg)) - opts, err := c.contextOpts() - if err != nil { - diags = diags.Append(err) - return diags - } + // Unless excluded, we'll also do a quick validation of the Terraform test files. These live + // outside the Terraform graph so we have to do this separately. + if !c.ParsedArgs.NoTests { + diags = diags.Append(c.validateTestFiles(cfg)) + } - tfCtx, ctxDiags := terraform.NewContext(opts) - diags = diags.Append(ctxDiags) - if ctxDiags.HasErrors() { - return diags - } + return diags +} - return diags.Append(tfCtx.Validate(cfg, nil)) - } +func (c *ValidateCommand) validateConfig(cfg *configs.Config) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics - diags = diags.Append(validate(cfg)) + opts, err := c.contextOpts() + if err != nil { + diags = diags.Append(err) + return diags + } - if noTests { + tfCtx, ctxDiags := terraform.NewContext(opts) + diags = diags.Append(ctxDiags) + if ctxDiags.HasErrors() { return diags } - validatedModules := make(map[string]bool) + return diags.Append(tfCtx.Validate(cfg, nil)) +} - // We'll also do a quick validation of the Terraform test files. These live - // outside the Terraform graph so we have to do this separately. +func (c *ValidateCommand) validateTestFiles(cfg *configs.Config) tfdiags.Diagnostics { + diags := tfdiags.Diagnostics{} + validatedModules := make(map[string]bool) for _, file := range cfg.Module.Tests { // The file validation only returns warnings so we'll just add them @@ -131,7 +141,7 @@ func (c *ValidateCommand) validate(dir, testDir string, noTests bool) tfdiags.Di // not validate the same thing multiple times. validatedModules[run.Module.Source.String()] = true - diags = diags.Append(validate(run.ConfigUnderTest)) + diags = diags.Append(c.validateConfig(run.ConfigUnderTest)) } } @@ -188,6 +198,8 @@ Options: -no-tests If specified, Terraform will not validate test files. -test-directory=path Set the Terraform test directory, defaults to "tests". + + -query If specified, the command will also validate .tfquery.hcl files. ` return strings.TrimSpace(helpText) } diff --git a/internal/command/validate_test.go b/internal/command/validate_test.go index 6651b3435a52..3cbeb4a02be7 100644 --- a/internal/command/validate_test.go +++ b/internal/command/validate_test.go @@ -449,3 +449,85 @@ func TestValidate_json(t *testing.T) { }) } } + +func TestValidateWithInvalidListResource(t *testing.T) { + td := t.TempDir() + cases := []struct { + name string + path string + wantError string + args []string + code int + }{ + { + name: "invalid-traversal with validate -query command", + path: "query/invalid-traversal", + wantError: ` +Error: Invalid list resource traversal + + on main.tfquery.hcl line 19, in list "test_instance" "test2": + 19: ami = list.test_instance.test.state.instance_type + +The first step in the traversal for a list resource must be an attribute +"data". +`, + args: []string{"-query"}, + code: 1, + }, + { + name: "invalid-traversal with no -query", + path: "query/invalid-traversal", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + testCopyDir(t, testFixturePath(tc.path), td) + t.Chdir(td) + + streams, done := terminal.StreamsForTesting(t) + view := views.NewView(streams) + ui := new(cli.MockUi) + + provider := queryFixtureProvider() + providerSource, close := newMockProviderSource(t, map[string][]string{ + "test": {"1.0.0"}, + }) + defer close() + + meta := Meta{ + testingOverrides: metaOverridesForProvider(provider), + Ui: ui, + View: view, + Streams: streams, + ProviderSource: providerSource, + } + + init := &InitCommand{ + Meta: meta, + } + + if code := init.Run(nil); code != 0 { + t.Fatalf("expected status code 0 but got %d: %s", code, ui.ErrorWriter) + } + + c := &ValidateCommand{ + Meta: meta, + } + + var args []string + args = append(args, "-no-color") + args = append(args, tc.args...) + + code := c.Run(args) + output := done(t) + + if code != tc.code { + t.Fatalf("Expected status code %d but got %d: %s", tc.code, code, output.Stderr()) + } + + if diff := cmp.Diff(tc.wantError, output.Stderr()); diff != "" { + t.Fatalf("Expected error string %q but got %q\n\ndiff: \n%s", tc.wantError, output.Stderr(), diff) + } + }) + } +} diff --git a/internal/command/views/apply.go b/internal/command/views/apply.go index 5c623bf62557..b8a64a94654a 100644 --- a/internal/command/views/apply.go +++ b/internal/command/views/apply.go @@ -61,27 +61,27 @@ type ApplyHuman struct { var _ Apply = (*ApplyHuman)(nil) func (v *ApplyHuman) ResourceCount(stateOutPath string) { + var summary string if v.destroy { - v.view.streams.Printf( - v.view.colorize.Color("[reset][bold][green]\nDestroy complete! Resources: %d destroyed.\n"), - v.countHook.Removed, - ) + summary = fmt.Sprintf("Destroy complete! Resources: %d destroyed.", v.countHook.Removed) } else if v.countHook.Imported > 0 { - v.view.streams.Printf( - v.view.colorize.Color("[reset][bold][green]\nApply complete! Resources: %d imported, %d added, %d changed, %d destroyed.\n"), + summary = fmt.Sprintf("Apply complete! Resources: %d imported, %d added, %d changed, %d destroyed.", v.countHook.Imported, v.countHook.Added, v.countHook.Changed, - v.countHook.Removed, - ) + v.countHook.Removed) } else { - v.view.streams.Printf( - v.view.colorize.Color("[reset][bold][green]\nApply complete! Resources: %d added, %d changed, %d destroyed.\n"), + summary = fmt.Sprintf("Apply complete! Resources: %d added, %d changed, %d destroyed.", v.countHook.Added, v.countHook.Changed, - v.countHook.Removed, - ) + v.countHook.Removed) } + v.view.streams.Print(v.view.colorize.Color("[reset][bold][green]\n" + summary)) + if v.countHook.ActionInvocation > 0 { + actionsStr := fmt.Sprintf("[reset][bold][green] Actions: %d invoked.", v.countHook.ActionInvocation) + v.view.streams.Print(v.view.colorize.Color(actionsStr)) + } + v.view.streams.Print("\n") if (v.countHook.Added > 0 || v.countHook.Changed > 0) && stateOutPath != "" { v.view.streams.Printf("\n%s\n\n", format.WordWrap(stateOutPathPostApply, v.view.outputColumns())) v.view.streams.Printf("State path: %s\n", stateOutPath) diff --git a/internal/command/views/operation.go b/internal/command/views/operation.go index ae9beca1b190..3b132991f9fa 100644 --- a/internal/command/views/operation.go +++ b/internal/command/views/operation.go @@ -110,7 +110,7 @@ func (v *OperationHuman) Plan(plan *plans.Plan, schemas *terraform.Schemas) { OutputChanges: outputs, ResourceChanges: changed, ResourceDrift: drift, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), RelevantAttributes: attrs, ActionInvocations: actions, } diff --git a/internal/command/views/show.go b/internal/command/views/show.go index 50429aa315f8..7eb76bf17e3d 100644 --- a/internal/command/views/show.go +++ b/internal/command/views/show.go @@ -83,7 +83,7 @@ func (v *ShowHuman) Display(config *configs.Config, plan *plans.Plan, planJSON * OutputChanges: outputs, ResourceChanges: changed, ResourceDrift: drift, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), RelevantAttributes: attrs, ActionInvocations: actions, } @@ -113,7 +113,7 @@ func (v *ShowHuman) Display(config *configs.Config, plan *plans.Plan, planJSON * ProviderFormatVersion: jsonprovider.FormatVersion, RootModule: root, RootModuleOutputs: outputs, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), } renderer.RenderHumanState(jstate) diff --git a/internal/command/views/test.go b/internal/command/views/test.go index 0feb2bab780d..e95e6afdc62a 100644 --- a/internal/command/views/test.go +++ b/internal/command/views/test.go @@ -203,7 +203,7 @@ func (t *TestHuman) Run(run *moduletest.Run, file *moduletest.File, progress mod ProviderFormatVersion: jsonprovider.FormatVersion, RootModule: root, RootModuleOutputs: outputs, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), } t.view.streams.Println() // Separate the state from any previous statements. @@ -225,7 +225,7 @@ func (t *TestHuman) Run(run *moduletest.Run, file *moduletest.File, progress mod OutputChanges: outputs, ResourceChanges: changed, ResourceDrift: drift, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), RelevantAttributes: attrs, ActionInvocations: actions, } @@ -568,7 +568,7 @@ func (t *TestJSON) Run(run *moduletest.Run, file *moduletest.File, progress modu ProviderFormatVersion: jsonprovider.FormatVersion, RootModule: root, RootModuleOutputs: outputs, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), } t.view.log.Info( @@ -592,7 +592,7 @@ func (t *TestJSON) Run(run *moduletest.Run, file *moduletest.File, progress modu OutputChanges: outputs, ResourceChanges: changed, ResourceDrift: drift, - ProviderSchemas: jsonprovider.MarshalForRenderer(schemas, false), + ProviderSchemas: jsonprovider.MarshalForRenderer(schemas), RelevantAttributes: attrs, ActionInvocations: actions, } diff --git a/internal/configs/config_build_test.go b/internal/configs/config_build_test.go index 496d03b2492b..1537de6e62cc 100644 --- a/internal/configs/config_build_test.go +++ b/internal/configs/config_build_test.go @@ -227,7 +227,12 @@ func TestBuildConfigInvalidModules(t *testing.T) { path := filepath.Join(testDir, name) parser.AllowLanguageExperiments(true) - mod, diags := parser.LoadConfigDirWithTests(path, "tests") + opts := []Option{MatchTestFiles("tests")} + if name == "list-in-child-module" { + opts = append(opts, MatchQueryFiles()) + } + + mod, diags := parser.LoadConfigDir(path, opts...) if diags.HasErrors() { // these tests should only trigger errors that are caught in // the config loader. @@ -265,7 +270,7 @@ func TestBuildConfigInvalidModules(t *testing.T) { // for simplicity, these tests will treat all source // addresses as relative to the root module sourcePath := filepath.Join(path, req.SourceAddr.String()) - mod, diags := parser.LoadConfigDir(sourcePath) + mod, diags := parser.LoadConfigDir(sourcePath, opts...) version, _ := version.NewVersion("1.0.0") return mod, version, diags }), diff --git a/internal/configs/configload/loader.go b/internal/configs/configload/loader.go index 07d68b30dd7e..07dfff0db6dd 100644 --- a/internal/configs/configload/loader.go +++ b/internal/configs/configload/loader.go @@ -27,6 +27,8 @@ type Loader struct { // modules is used to install and locate descendant modules that are // referenced (directly or indirectly) from the root module. modules moduleMgr + + parserOpts []configs.Option } // Config is used with NewLoader to specify configuration arguments for the @@ -43,6 +45,10 @@ type Config struct { // not supported, which should be true only in specialized circumstances // such as in tests. Services *disco.Disco + + // IncludeQueryFiles is set to true if query files should be parsed + // when running query commands. + IncludeQueryFiles bool } // NewLoader creates and returns a loader that reads configuration from the @@ -65,6 +71,7 @@ func NewLoader(config *Config) (*Loader, error) { Services: config.Services, Registry: reg, }, + parserOpts: make([]configs.Option, 0), } err := ret.modules.readModuleManifestSnapshot() @@ -72,6 +79,10 @@ func NewLoader(config *Config) (*Loader, error) { return nil, fmt.Errorf("failed to read module manifest: %s", err) } + if config.IncludeQueryFiles { + ret.parserOpts = append(ret.parserOpts, configs.MatchQueryFiles()) + } + return ret, nil } @@ -122,7 +133,7 @@ func (l *Loader) Sources() map[string][]byte { // least one Terraform configuration file. This is a wrapper around calling // the same method name on the loader's parser. func (l *Loader) IsConfigDir(path string) bool { - return l.parser.IsConfigDir(path) + return l.parser.IsConfigDir(path, l.parserOpts...) } // ImportSources writes into the receiver's source code map the given source diff --git a/internal/configs/configload/loader_load.go b/internal/configs/configload/loader_load.go index f7b776b86082..3b9407a32b6c 100644 --- a/internal/configs/configload/loader_load.go +++ b/internal/configs/configload/loader_load.go @@ -22,14 +22,14 @@ import ( // // LoadConfig performs the basic syntax and uniqueness validations that are // required to process the individual modules -func (l *Loader) LoadConfig(rootDir string, parserOpts ...configs.Option) (*configs.Config, hcl.Diagnostics) { - return l.loadConfig(l.parser.LoadConfigDir(rootDir, parserOpts...)) +func (l *Loader) LoadConfig(rootDir string) (*configs.Config, hcl.Diagnostics) { + return l.loadConfig(l.parser.LoadConfigDir(rootDir, l.parserOpts...)) } // LoadConfigWithTests matches LoadConfig, except the configs.Config contains // any relevant .tftest.hcl files. func (l *Loader) LoadConfigWithTests(rootDir string, testDir string) (*configs.Config, hcl.Diagnostics) { - return l.loadConfig(l.parser.LoadConfigDirWithTests(rootDir, testDir)) + return l.loadConfig(l.parser.LoadConfigDir(rootDir, append(l.parserOpts, configs.MatchTestFiles(testDir))...)) } func (l *Loader) loadConfig(rootMod *configs.Module, diags hcl.Diagnostics) (*configs.Config, hcl.Diagnostics) { diff --git a/internal/configs/configload/loader_snapshot.go b/internal/configs/configload/loader_snapshot.go index d8260cddd7ca..46efab362c6e 100644 --- a/internal/configs/configload/loader_snapshot.go +++ b/internal/configs/configload/loader_snapshot.go @@ -24,7 +24,7 @@ import ( // creates an in-memory snapshot of the configuration files used, which can // be later used to create a loader that may read only from this snapshot. func (l *Loader) LoadConfigWithSnapshot(rootDir string) (*configs.Config, *Snapshot, hcl.Diagnostics) { - rootMod, diags := l.parser.LoadConfigDir(rootDir) + rootMod, diags := l.parser.LoadConfigDir(rootDir, l.parserOpts...) if rootMod == nil { return nil, nil, diags } diff --git a/internal/configs/configload/testing.go b/internal/configs/configload/testing.go index bc90b2ef3cbc..537e86ac2c54 100644 --- a/internal/configs/configload/testing.go +++ b/internal/configs/configload/testing.go @@ -4,9 +4,10 @@ package configload import ( - "io/ioutil" "os" "testing" + + "github.com/hashicorp/terraform/internal/configs" ) // NewLoaderForTests is a variant of NewLoader that is intended to be more @@ -20,10 +21,10 @@ import ( // In the case of any errors, t.Fatal (or similar) will be called to halt // execution of the test, so the calling test does not need to handle errors // itself. -func NewLoaderForTests(t testing.TB) (*Loader, func()) { +func NewLoaderForTests(t testing.TB, parserOpts ...configs.Option) (*Loader, func()) { t.Helper() - modulesDir, err := ioutil.TempDir("", "tf-configs") + modulesDir, err := os.MkdirTemp("", "tf-configs") if err != nil { t.Fatalf("failed to create temporary modules dir: %s", err) return nil, func() {} @@ -36,6 +37,7 @@ func NewLoaderForTests(t testing.TB) (*Loader, func()) { loader, err := NewLoader(&Config{ ModulesDir: modulesDir, }) + loader.parserOpts = append(loader.parserOpts, parserOpts...) if err != nil { cleanup() t.Fatalf("failed to create config loader: %s", err) diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 9c296bc01323..a4c220febc8e 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -153,8 +153,8 @@ func (p Parser) ConfigDirFiles(dir string, opts ...Option) (primary, override [] // exists and contains at least one Terraform config file (with a .tf or // .tf.json extension.). Note, we explicitely exclude checking for tests here // as tests must live alongside actual .tf config files. Same goes for query files. -func (p *Parser) IsConfigDir(path string) bool { - pathSet, _ := p.dirFileSet(path) +func (p *Parser) IsConfigDir(path string, opts ...Option) bool { + pathSet, _ := p.dirFileSet(path, opts...) return (len(pathSet.Primary) + len(pathSet.Override)) > 0 } diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index b1921a794ece..2fe176ca6822 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -167,27 +167,23 @@ func TestParserLoadConfigDirWithQueries(t *testing.T) { diagnostics []string listResources int managedResources int - allowExperiments bool }{ { - name: "simple", - directory: "testdata/query-files/valid/simple", - listResources: 3, - allowExperiments: true, + name: "simple", + directory: "testdata/query-files/valid/simple", + listResources: 3, }, { name: "mixed", directory: "testdata/query-files/valid/mixed", listResources: 3, managedResources: 1, - allowExperiments: true, }, { name: "loading query lists with no-experiments", directory: "testdata/query-files/valid/mixed", managedResources: 1, - listResources: 0, - allowExperiments: false, + listResources: 3, }, { name: "no-provider", @@ -195,7 +191,6 @@ func TestParserLoadConfigDirWithQueries(t *testing.T) { diagnostics: []string{ "testdata/query-files/invalid/no-provider/main.tfquery.hcl:1,1-27: Missing \"provider\" attribute; You must specify a provider attribute when defining a list block.", }, - allowExperiments: true, }, { name: "with-depends-on", @@ -203,16 +198,14 @@ func TestParserLoadConfigDirWithQueries(t *testing.T) { diagnostics: []string{ "testdata/query-files/invalid/with-depends-on/main.tfquery.hcl:23,3-13: Unsupported argument; An argument named \"depends_on\" is not expected here.", }, - listResources: 2, - allowExperiments: true, + listResources: 2, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { parser := NewParser(nil) - parser.AllowLanguageExperiments(test.allowExperiments) - mod, diags := parser.LoadConfigDir(test.directory) + mod, diags := parser.LoadConfigDir(test.directory, MatchQueryFiles()) if len(test.diagnostics) > 0 { if !diags.HasErrors() { t.Errorf("expected errors, but found none") diff --git a/internal/configs/parser_file_matcher.go b/internal/configs/parser_file_matcher.go index cb282a1a7d35..6644b421ee6c 100644 --- a/internal/configs/parser_file_matcher.go +++ b/internal/configs/parser_file_matcher.go @@ -64,9 +64,6 @@ func (p *Parser) dirFileSet(dir string, opts ...Option) (ConfigFileSet, hcl.Diag testDirectory: DefaultTestDirectory, fs: p.fs, } - if p.AllowsLanguageExperiments() { - cfg.matchers = append(cfg.matchers, &queryFiles{}) - } for _, opt := range opts { opt(cfg) } @@ -142,6 +139,13 @@ func MatchTestFiles(dir string) Option { } } +// MatchQueryFiles adds a matcher for Terraform query files (.tfquery.hcl and .tfquery.json) +func MatchQueryFiles() Option { + return func(o *parserConfig) { + o.matchers = append(o.matchers, &queryFiles{}) + } +} + // moduleFiles matches regular Terraform configuration files (.tf and .tf.json) type moduleFiles struct{} diff --git a/internal/genconfig/generate_config.go b/internal/genconfig/generate_config.go index 80a6f017343b..746a04220dba 100644 --- a/internal/genconfig/generate_config.go +++ b/internal/genconfig/generate_config.go @@ -138,6 +138,9 @@ type ResourceListElement struct { Config cty.Value Identity cty.Value + + // ExpansionEnum is a unique enumeration of the list resource address relative to its expanded siblings. + ExpansionEnum int } func GenerateListResourceContents(addr addrs.AbsResourceInstance, @@ -158,12 +161,18 @@ func GenerateListResourceContents(addr addrs.AbsResourceInstance, Resource: addrs.Resource{ Mode: addrs.ManagedResourceMode, Type: addr.Resource.Resource.Type, - Name: fmt.Sprintf("%s_%d", addr.Resource.Resource.Name, idx), }, - Key: addr.Resource.Key, }, } + // If the list resource instance is keyed, the expansion counter is included in the address + // to ensure uniqueness across the entire configuration. + if addr.Resource.Key == addrs.NoKey { + resAddr.Resource.Resource.Name = fmt.Sprintf("%s_%d", addr.Resource.Resource.Name, idx) + } else { + resAddr.Resource.Resource.Name = fmt.Sprintf("%s_%d_%d", addr.Resource.Resource.Name, res.ExpansionEnum, idx) + } + content, gDiags := GenerateResourceContents(resAddr, schema, pc, res.Config, true) if gDiags.HasErrors() { diags = diags.Append(gDiags) diff --git a/internal/initwd/testing.go b/internal/initwd/testing.go index 8c99778e1c6b..4331529131da 100644 --- a/internal/initwd/testing.go +++ b/internal/initwd/testing.go @@ -53,7 +53,7 @@ func LoadConfigForTests(t *testing.T, rootDir string, testsDir string) (*configs t.Fatalf("failed to refresh modules after installation: %s", err) } - config, hclDiags := loader.LoadConfig(rootDir, configs.MatchTestFiles(testsDir)) + config, hclDiags := loader.LoadConfigWithTests(rootDir, testsDir) diags = diags.Append(hclDiags) return config, loader, cleanup, diags } diff --git a/internal/instances/expander.go b/internal/instances/expander.go index 414ede32c507..93a38b4060aa 100644 --- a/internal/instances/expander.go +++ b/internal/instances/expander.go @@ -5,6 +5,7 @@ package instances import ( "fmt" + "slices" "sort" "sync" @@ -338,6 +339,16 @@ func (e *Expander) ExpandResource(resourceAddr addrs.AbsResource) []addrs.AbsRes return ret } +// ResourceExpansionEnum returns the expansion enum for the given resource instance address +// within the sorted list of resource instances belonging to the same resource config within +// the same module instance. +func (e *Expander) ResourceExpansionEnum(resourceAddr addrs.AbsResourceInstance) int { + res := e.ExpandResource(resourceAddr.ContainingResource()) + return slices.IndexFunc(res, func(addr addrs.AbsResourceInstance) bool { + return addr.Equal(resourceAddr) + }) +} + // UnknownResourceInstances finds a set of patterns that collectively cover // all of the possible resource instance addresses that could appear for the // given static resource once all of the intermediate module expansions are diff --git a/internal/plans/deferring/deferred.go b/internal/plans/deferring/deferred.go index e5d89122a642..4a9a17b71a65 100644 --- a/internal/plans/deferring/deferred.go +++ b/internal/plans/deferring/deferred.go @@ -7,6 +7,7 @@ import ( "fmt" "sync" + "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" @@ -729,64 +730,51 @@ func (d *Deferred) ReportActionDeferred(addr addrs.AbsActionInstance, reason pro configMap.Put(addr, reason) } -// ShouldDeferActionInvocation returns true if there is a reason to defer the action invocation instance -// We want to defer an action invocation if -// a) the resource was deferred -// or -// b) a previously run action was deferred -func (d *Deferred) ShouldDeferActionInvocation(ai plans.ActionInvocationInstance) bool { +// ShouldDeferActionInvocation returns true if there is a reason to defer the +// action invocation instance. We want to defer an action invocation only if +// the triggering resource was deferred. In addition, we will check if the +// underlying action was deferred via a reference, and consider it an error if +// the triggering resource wasn't also deferred. +// +// The reason behind the slightly different behaviour here, is that if an +// action invocation is deferred, then that implies the triggering action +// should also be deferred. +// +// We don't yet have the capability to retroactively defer a resource, so for +// now actions initiating deferrals themselves is considered an error. +func (d *Deferred) ShouldDeferActionInvocation(ai plans.ActionInvocationInstance, triggerRange *hcl.Range) (bool, tfdiags.Diagnostics) { d.mu.Lock() defer d.mu.Unlock() - // The expansion of the action itself is deferred - if ai.Addr.Action.Key == addrs.WildcardKey { - return true - } - - if c, ok := d.actionExpansionDeferred.GetOk(ai.Addr.ConfigAction()); ok { - if c.Has(ai.Addr) { - return true - } - - for _, k := range c.Keys() { - if k.Action.Key == addrs.WildcardKey { - return true - } - } - } - - if d.partialExpandedActionsDeferred.Has(ai.Addr.ConfigAction()) { - return true - } + var diags tfdiags.Diagnostics // We only want to defer actions that are lifecycle triggered at, ok := ai.ActionTrigger.(*plans.LifecycleActionTrigger) if !ok { - return false + return false, diags } // If the resource was deferred, we also need to defer any action potentially triggering from this if configResourceMap, ok := d.resourceInstancesDeferred.GetOk(at.TriggeringResourceAddr.ConfigResource()); ok { if configResourceMap.Has(at.TriggeringResourceAddr) { - return true + return true, diags } } - // Since all actions plan in order we can just check if an action for this resource instance - // has been deferred already - for _, deferred := range d.actionInvocationDeferred { - deferredAt, deferredOk := deferred.ActionInvocationInstance.ActionTrigger.(*plans.LifecycleActionTrigger) - if !deferredOk { - continue // We only care about lifecycle triggered actions here - } - - if deferredAt.TriggeringResourceAddr.Equal(at.TriggeringResourceAddr) { - return true + if c, ok := d.actionExpansionDeferred.GetOk(ai.Addr.ConfigAction()); ok { + if c.Has(ai.Addr) { + // Then in this case, the resource wasn't deferred but the action + // was and so we will consider this to be an error. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid action deferral", + Detail: fmt.Sprintf("The action %s was marked as deferred, but was triggered by a non-deferred resource %s. To work around this, use the -target argument to first apply only the resources that the action block depends on.", ai.Addr, at.TriggeringResourceAddr), + Subject: triggerRange, + }) } } - // We found no reason, so we return false - return false + return false, diags } // ShouldDeferAction returns true if the action should be deferred. This is the case if a diff --git a/internal/plugin/convert/schema.go b/internal/plugin/convert/schema.go index d07f91645fd7..d3b003d561ae 100644 --- a/internal/plugin/convert/schema.go +++ b/internal/plugin/convert/schema.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/providers" proto "github.com/hashicorp/terraform/internal/tfplugin5" + "github.com/zclconf/go-cty/cty" ) // ConfigSchemaToProto takes a *configschema.Block and converts it to a @@ -111,6 +112,45 @@ func ProtoToActionSchema(s *proto.ActionSchema) providers.ActionSchema { } } +func ProtoToListSchema(s *proto.Schema) providers.Schema { + listSchema := ProtoToProviderSchema(s, nil) + itemCount := 0 + // check if the provider has set some attributes/blocks as required. + // When yes, then we set minItem = 1, which + // validates that the configuration contains a "config" block. + for _, attrS := range listSchema.Body.Attributes { + if attrS.Required { + itemCount = 1 + break + } + } + for _, block := range listSchema.Body.BlockTypes { + if block.MinItems > 0 { + itemCount = 1 + break + } + } + return providers.Schema{ + Version: s.Version, + Body: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "data": { + Type: cty.DynamicPseudoType, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "config": { + Block: *listSchema.Body, + Nesting: configschema.NestingSingle, + MinItems: itemCount, + MaxItems: itemCount, + }, + }, + }, + } +} + // ProtoToConfigSchema takes the GetSchcema_Block from a grpc response and converts it // to a terraform *configschema.Block. func ProtoToConfigSchema(b *proto.Schema_Block) *configschema.Block { diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go index c3c0793dc27c..3f8251b4a11d 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -20,7 +20,6 @@ import ( "google.golang.org/grpc/status" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/internal/plugin/convert" "github.com/hashicorp/terraform/internal/providers" @@ -171,24 +170,7 @@ func (p *GRPCProvider) GetProviderSchema() providers.GetProviderSchemaResponse { } for name, list := range protoResp.ListResourceSchemas { - ret := convert.ProtoToProviderSchema(list, nil) - resp.ListResourceTypes[name] = providers.Schema{ - Version: ret.Version, - Body: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "data": { - Type: cty.DynamicPseudoType, - Computed: true, - }, - }, - BlockTypes: map[string]*configschema.NestedBlock{ - "config": { - Block: *ret.Body, - Nesting: configschema.NestingSingle, - }, - }, - }, - } + resp.ListResourceTypes[name] = convert.ProtoToListSchema(list) } for name, action := range protoResp.ActionSchemas { @@ -381,10 +363,12 @@ func (p *GRPCProvider) ValidateListResourceConfig(r providers.ValidateListResour } configSchema := listResourceSchema.Body.BlockTypes["config"] - config := cty.NullVal(configSchema.ImpliedType()) - if r.Config.Type().HasAttribute("config") { - config = r.Config.GetAttr("config") + if !r.Config.Type().HasAttribute("config") { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("missing required attribute \"config\"; this is a bug in Terraform - please report it")) + return resp } + + config := r.Config.GetAttr("config") mp, err := msgpack.Marshal(config, configSchema.ImpliedType()) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) @@ -1342,10 +1326,12 @@ func (p *GRPCProvider) ListResource(r providers.ListResourceRequest) providers.L } configSchema := listResourceSchema.Body.BlockTypes["config"] - config := cty.NullVal(configSchema.ImpliedType()) - if r.Config.Type().HasAttribute("config") { - config = r.Config.GetAttr("config") + if !r.Config.Type().HasAttribute("config") { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("missing required attribute \"config\"; this is a bug in Terraform - please report it")) + return resp } + + config := r.Config.GetAttr("config") mp, err := msgpack.Marshal(config, configSchema.ImpliedType()) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) diff --git a/internal/plugin/grpc_provider_test.go b/internal/plugin/grpc_provider_test.go index 657b8b7f33d3..34017d724406 100644 --- a/internal/plugin/grpc_provider_test.go +++ b/internal/plugin/grpc_provider_test.go @@ -146,6 +146,23 @@ func providerProtoSchema() *proto.GetProviderSchema_Response { Required: true, }, }, + BlockTypes: []*proto.Schema_NestedBlock{ + { + TypeName: "nested_filter", + Nesting: proto.Schema_NestedBlock_SINGLE, + Block: &proto.Schema_Block{ + Attributes: []*proto.Schema_Attribute{ + { + Name: "nested_attr", + Type: []byte(`"string"`), + Required: false, + }, + }, + }, + MinItems: 1, + MaxItems: 1, + }, + }, }, }, }, @@ -466,7 +483,7 @@ func TestGRPCProvider_ValidateListResourceConfig(t *testing.T) { gomock.Any(), ).Return(&proto.ValidateListResourceConfig_Response{}, nil) - cfg := hcl2shim.HCL2ValueFromConfigValue(map[string]interface{}{"config": map[string]interface{}{"filter_attr": "value"}}) + cfg := hcl2shim.HCL2ValueFromConfigValue(map[string]interface{}{"config": map[string]interface{}{"filter_attr": "value", "nested_filter": map[string]interface{}{"nested_attr": "value"}}}) resp := p.ValidateListResourceConfig(providers.ValidateListResourceConfigRequest{ TypeName: "list", Config: cfg, @@ -478,8 +495,18 @@ func TestGRPCProvider_ValidateListResourceConfig_OptionalCfg(t *testing.T) { ctrl := gomock.NewController(t) client := mockproto.NewMockProviderClient(ctrl) sch := providerProtoSchema() - sch.ListResourceSchemas["list"].Block.Attributes[0].Optional = true - sch.ListResourceSchemas["list"].Block.Attributes[0].Required = false + + // mock the schema in a way that makes the config attributes optional + listSchema := sch.ListResourceSchemas["list"].Block + // filter_attr is optional + listSchema.Attributes[0].Optional = true + listSchema.Attributes[0].Required = false + + // nested_filter is optional + listSchema.BlockTypes[0].MinItems = 0 + listSchema.BlockTypes[0].MaxItems = 0 + + sch.ListResourceSchemas["list"].Block = listSchema // we always need a GetSchema method client.EXPECT().GetSchema( gomock.Any(), @@ -502,10 +529,15 @@ func TestGRPCProvider_ValidateListResourceConfig_OptionalCfg(t *testing.T) { gomock.Any(), ).Return(&proto.ValidateListResourceConfig_Response{}, nil) - cfg := hcl2shim.HCL2ValueFromConfigValue(map[string]interface{}{}) + converted := convert.ProtoToListSchema(sch.ListResourceSchemas["list"]) + cfg := hcl2shim.HCL2ValueFromConfigValue(map[string]any{}) + coercedCfg, err := converted.Body.CoerceValue(cfg) + if err != nil { + t.Fatalf("failed to coerce config: %v", err) + } resp := p.ValidateListResourceConfig(providers.ValidateListResourceConfigRequest{ TypeName: "list", - Config: cfg, + Config: coercedCfg, }) checkDiags(t, resp.Diagnostics) } @@ -1438,8 +1470,25 @@ func TestGRPCProvider_GetSchema_ListResourceTypes(t *testing.T) { Required: true, }, }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_filter": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "nested_attr": { + Type: cty.String, + Required: false, + }, + }, + }, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, + }, + }, }, - Nesting: configschema.NestingSingle, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, }, }, }, @@ -1485,6 +1534,9 @@ func TestGRPCProvider_Encode(t *testing.T) { Before: cty.NullVal(cty.Object(map[string]cty.Type{ "config": cty.Object(map[string]cty.Type{ "filter_attr": cty.String, + "nested_filter": cty.Object(map[string]cty.Type{ + "nested_attr": cty.String, + }), }), "data": cty.List(cty.Object(map[string]cty.Type{ "state": cty.Object(map[string]cty.Type{ @@ -1498,6 +1550,9 @@ func TestGRPCProvider_Encode(t *testing.T) { After: cty.ObjectVal(map[string]cty.Value{ "config": cty.ObjectVal(map[string]cty.Value{ "filter_attr": cty.StringVal("value"), + "nested_filter": cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + }), }), "data": cty.ListVal([]cty.Value{ cty.ObjectVal(map[string]cty.Value{ @@ -1649,6 +1704,9 @@ func TestGRPCProvider_ListResource(t *testing.T) { configVal := cty.ObjectVal(map[string]cty.Value{ "config": cty.ObjectVal(map[string]cty.Value{ "filter_attr": cty.StringVal("filter-value"), + "nested_filter": cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + }), }), }) request := providers.ListResourceRequest{ @@ -1731,6 +1789,9 @@ func TestGRPCProvider_ListResource_Error(t *testing.T) { configVal := cty.ObjectVal(map[string]cty.Value{ "config": cty.ObjectVal(map[string]cty.Value{ "filter_attr": cty.StringVal("filter-value"), + "nested_filter": cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + }), }), }) request := providers.ListResourceRequest{ @@ -1746,6 +1807,9 @@ func TestGRPCProvider_ListResource_Diagnostics(t *testing.T) { configVal := cty.ObjectVal(map[string]cty.Value{ "config": cty.ObjectVal(map[string]cty.Value{ "filter_attr": cty.StringVal("filter-value"), + "nested_filter": cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + }), }), }) request := providers.ListResourceRequest{ @@ -2009,6 +2073,9 @@ func TestGRPCProvider_ListResource_Limit(t *testing.T) { configVal := cty.ObjectVal(map[string]cty.Value{ "config": cty.ObjectVal(map[string]cty.Value{ "filter_attr": cty.StringVal("filter-value"), + "nested_filter": cty.ObjectVal(map[string]cty.Value{ + "nested_attr": cty.StringVal("value"), + }), }), }) request := providers.ListResourceRequest{ diff --git a/internal/plugin6/convert/schema.go b/internal/plugin6/convert/schema.go index a762590862ca..4ea6a8faf507 100644 --- a/internal/plugin6/convert/schema.go +++ b/internal/plugin6/convert/schema.go @@ -118,6 +118,45 @@ func ProtoToActionSchema(s *proto.ActionSchema) providers.ActionSchema { } } +func ProtoToListSchema(s *proto.Schema) providers.Schema { + listSchema := ProtoToProviderSchema(s, nil) + itemCount := 0 + // check if the provider has set some attributes/blocks as required. + // When yes, then we set minItem = 1, which + // validates that the configuration contains a "config" block. + for _, attrS := range listSchema.Body.Attributes { + if attrS.Required { + itemCount = 1 + break + } + } + for _, block := range listSchema.Body.BlockTypes { + if block.MinItems > 0 { + itemCount = 1 + break + } + } + return providers.Schema{ + Version: listSchema.Version, + Body: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "data": { + Type: cty.DynamicPseudoType, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "config": { + Block: *listSchema.Body, + Nesting: configschema.NestingSingle, + MinItems: itemCount, + MaxItems: itemCount, + }, + }, + }, + } +} + func ProtoToIdentitySchema(attributes []*proto.ResourceIdentitySchema_IdentityAttribute) *configschema.Object { obj := &configschema.Object{ Attributes: make(map[string]*configschema.Attribute), diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go index b991ed6b8d0f..0ab7da98b1a2 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -20,7 +20,6 @@ import ( "google.golang.org/grpc/status" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/internal/plugin6/convert" "github.com/hashicorp/terraform/internal/providers" @@ -172,24 +171,7 @@ func (p *GRPCProvider) GetProviderSchema() providers.GetProviderSchemaResponse { } for name, list := range protoResp.ListResourceSchemas { - ret := convert.ProtoToProviderSchema(list, nil) - resp.ListResourceTypes[name] = providers.Schema{ - Version: ret.Version, - Body: &configschema.Block{ - Attributes: map[string]*configschema.Attribute{ - "data": { - Type: cty.DynamicPseudoType, - Computed: true, - }, - }, - BlockTypes: map[string]*configschema.NestedBlock{ - "config": { - Block: *ret.Body, - Nesting: configschema.NestingSingle, - }, - }, - }, - } + resp.ListResourceTypes[name] = convert.ProtoToListSchema(list) } for name, store := range protoResp.StateStoreSchemas { @@ -377,11 +359,14 @@ func (p *GRPCProvider) ValidateListResourceConfig(r providers.ValidateListResour resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown list resource type %q", r.TypeName)) return resp } + configSchema := listResourceSchema.Body.BlockTypes["config"] - config := cty.NullVal(configSchema.ImpliedType()) - if r.Config.Type().HasAttribute("config") { - config = r.Config.GetAttr("config") + if !r.Config.Type().HasAttribute("config") { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("missing required attribute \"config\"; this is a bug in Terraform - please report it")) + return resp } + + config := r.Config.GetAttr("config") mp, err := msgpack.Marshal(config, configSchema.ImpliedType()) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) @@ -1337,10 +1322,12 @@ func (p *GRPCProvider) ListResource(r providers.ListResourceRequest) providers.L } configSchema := listResourceSchema.Body.BlockTypes["config"] - config := cty.NullVal(configSchema.ImpliedType()) - if r.Config.Type().HasAttribute("config") { - config = r.Config.GetAttr("config") + if !r.Config.Type().HasAttribute("config") { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("missing required attribute \"config\"; this is a bug in Terraform - please report it")) + return resp } + + config := r.Config.GetAttr("config") mp, err := msgpack.Marshal(config, configSchema.ImpliedType()) if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) diff --git a/internal/plugin6/grpc_provider_test.go b/internal/plugin6/grpc_provider_test.go index 5e375c66ce53..44fb722dcff3 100644 --- a/internal/plugin6/grpc_provider_test.go +++ b/internal/plugin6/grpc_provider_test.go @@ -522,10 +522,15 @@ func TestGRPCProvider_ValidateListResourceConfig_OptionalCfg(t *testing.T) { gomock.Any(), ).Return(&proto.ValidateListResourceConfig_Response{}, nil) + converted := convert.ProtoToListSchema(sch.ListResourceSchemas["list"]) cfg := hcl2shim.HCL2ValueFromConfigValue(map[string]interface{}{}) + coercedCfg, err := converted.Body.CoerceValue(cfg) + if err != nil { + t.Fatalf("failed to coerce config: %v", err) + } resp := p.ValidateListResourceConfig(providers.ValidateListResourceConfigRequest{ TypeName: "list", - Config: cfg, + Config: coercedCfg, }) checkDiags(t, resp.Diagnostics) } @@ -1458,7 +1463,9 @@ func TestGRPCProvider_GetSchema_ListResourceTypes(t *testing.T) { }, }, }, - Nesting: configschema.NestingSingle, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, }, }, }, diff --git a/internal/providers/provider.go b/internal/providers/provider.go index b19c1451a501..579626c3699f 100644 --- a/internal/providers/provider.go +++ b/internal/providers/provider.go @@ -220,6 +220,19 @@ func (a ActionSchema) IsNil() bool { return a.ConfigSchema == nil } +type ListResourceSchema struct { + // schema for the nested "config" block. + ConfigSchema *configschema.Block + + // schema for the entire block (including "config" block) + FullSchema *configschema.Block +} + +// IsNil() returns true if there is no list resource schema at all. +func (l ListResourceSchema) IsNil() bool { + return l.FullSchema == nil +} + // Schema pairs a provider or resource schema with that schema's version. // This is used to be able to upgrade the schema in UpgradeResourceState. // diff --git a/internal/providers/schemas.go b/internal/providers/schemas.go index 4d02c8bdc897..9cb64b5ecfef 100644 --- a/internal/providers/schemas.go +++ b/internal/providers/schemas.go @@ -47,3 +47,20 @@ func (ss ProviderSchema) SchemaForActionType(typeName string) (schema ActionSche } return ActionSchema{} } + +// SchemaForListResourceType attempts to find a schema for the given type. Returns an +// empty schema if none is available. +func (ss ProviderSchema) SchemaForListResourceType(typeName string) ListResourceSchema { + schema, ok := ss.ListResourceTypes[typeName] + ret := ListResourceSchema{FullSchema: schema.Body} + if !ok || schema.Body == nil { + return ret + } + // The configuration for the list block is nested within a "config" block. + configSchema, ok := schema.Body.BlockTypes["config"] + if !ok { + return ret + } + ret.ConfigSchema = &configSchema.Block + return ret +} diff --git a/internal/stacks/stackruntime/internal/stackeval/provider_config.go b/internal/stacks/stackruntime/internal/stackeval/provider_config.go index ff8a7915ce55..dd6c44cf7adb 100644 --- a/internal/stacks/stackruntime/internal/stackeval/provider_config.go +++ b/internal/stacks/stackruntime/internal/stackeval/provider_config.go @@ -86,7 +86,7 @@ func CheckProviderInLockfile(locks depsfile.Locks, providerType *ProviderType, d Severity: hcl.DiagError, Summary: "Provider missing from lockfile", Detail: fmt.Sprintf( - "Provider %q is not in the lockfile. This provider must be in the lockfile to be used in the configuration. Please run `tfstacks providers lock` to update the lockfile and run this operation again with an updated configuration.", + "Provider %q is not in the lockfile. This provider must be in the lockfile to be used in the configuration. Please run `terraform stacks providers lock` to update the lockfile and run this operation again with an updated configuration.", providerType.Addr(), ), Subject: declRange, diff --git a/internal/stacks/stackruntime/validate_test.go b/internal/stacks/stackruntime/validate_test.go index afcf02aa8403..a88e0b58d74d 100644 --- a/internal/stacks/stackruntime/validate_test.go +++ b/internal/stacks/stackruntime/validate_test.go @@ -478,7 +478,7 @@ Terraform uses references to decide a suitable order for performing operations, return diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Provider missing from lockfile", - Detail: "Provider \"registry.terraform.io/hashicorp/testing\" is not in the lockfile. This provider must be in the lockfile to be used in the configuration. Please run `tfstacks providers lock` to update the lockfile and run this operation again with an updated configuration.", + Detail: "Provider \"registry.terraform.io/hashicorp/testing\" is not in the lockfile. This provider must be in the lockfile to be used in the configuration. Please run `terraform stacks providers lock` to update the lockfile and run this operation again with an updated configuration.", Subject: &hcl.Range{ Filename: "git::https://example.com/test.git//with-single-input/input-from-component/input-from-component.tfcomponent.hcl", Start: hcl.Pos{Line: 8, Column: 1, Byte: 98}, diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index 49600d9552dd..63b7963c2249 100644 --- a/internal/states/instance_object_src.go +++ b/internal/states/instance_object_src.go @@ -117,7 +117,7 @@ func (os *ResourceInstanceObjectSrc) Decode(schema providers.Schema) (*ResourceI var identity cty.Value if os.decodeIdentityCache != cty.NilVal { identity = os.decodeIdentityCache - } else if os.IdentityJSON != nil { + } else if os.IdentityJSON != nil && schema.Identity != nil { identity, err = ctyjson.Unmarshal(os.IdentityJSON, schema.Identity.ImpliedType()) if err != nil { return nil, fmt.Errorf("failed to decode identity: %s. This is most likely a bug in the Provider, providers must not change the identity schema without updating the identity schema version", err.Error()) diff --git a/internal/states/instance_object_test.go b/internal/states/instance_object_test.go index 7b5a1a4bde8d..ba8cccb8efdf 100644 --- a/internal/states/instance_object_test.go +++ b/internal/states/instance_object_test.go @@ -61,22 +61,22 @@ func TestResourceInstanceObject_encode(t *testing.T) { // multiple instances may have been assigned the same deps slice objs := []*ResourceInstanceObject{ - &ResourceInstanceObject{ + { Value: value, Status: ObjectPlanned, Dependencies: depsOne, }, - &ResourceInstanceObject{ + { Value: value, Status: ObjectPlanned, Dependencies: depsTwo, }, - &ResourceInstanceObject{ + { Value: value, Status: ObjectPlanned, Dependencies: depsOne, }, - &ResourceInstanceObject{ + { Value: value, Status: ObjectPlanned, Dependencies: depsOne, @@ -91,10 +91,7 @@ func TestResourceInstanceObject_encode(t *testing.T) { var mu sync.Mutex for _, obj := range objs { - obj := obj - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { rios, err := obj.Encode(schema) if err != nil { t.Errorf("unexpected error: %s", err) @@ -102,7 +99,7 @@ func TestResourceInstanceObject_encode(t *testing.T) { mu.Lock() encoded = append(encoded, rios) mu.Unlock() - }() + }) } wg.Wait() 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_apply_action_test.go b/internal/terraform/context_apply_action_test.go index 454e7d2f04bd..7b1e95c505c1 100644 --- a/internal/terraform/context_apply_action_test.go +++ b/internal/terraform/context_apply_action_test.go @@ -38,6 +38,7 @@ func TestContextApply_actions(t *testing.T) { expectInvokeActionCalls []providers.InvokeActionRequest expectInvokeActionCallsAreUnordered bool expectDiagnostics func(m *configs.Config) tfdiags.Diagnostics + ignoreWarnings bool }{ "before_create triggered": { module: map[string]string{ @@ -1928,6 +1929,441 @@ resource "test_object" "a" { }, }, }, + + "targeted run": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + config { + attr = "hello" + } +} +action "action_example" "there" { + config { + attr = "there" + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } + action_trigger { + events = [after_create] + actions = [action.action_example.there] + } + } +} +action "action_example" "general" { + config { + attr = "general" + } +} +action "action_example" "kenobi" { + config { + attr = "kenobi" + } +} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.action_example.general] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + }, + { + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("there"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, + + "targeted run with ancestor that has actions": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + config { + attr = "hello" + } +} +action "action_example" "there" { + config { + attr = "there" + } +} +resource "test_object" "origin" { + name = "origin" + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } + } +} +resource "test_object" "a" { + name = test_object.origin.name + lifecycle { + action_trigger { + events = [after_create] + actions = [action.action_example.there] + } + } +} +action "action_example" "general" {} +action "action_example" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.action_example.general] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + }, + { + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("there"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, + + "targeted run with expansion": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + count = 3 + config { + attr = "hello-${count.index}" + } +} +action "action_example" "there" { + count = 3 + config { + attr = "there-${count.index}" + } +} +resource "test_object" "a" { + count = 3 + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello[count.index]] + } + action_trigger { + events = [after_create] + actions = [action.action_example.there[count.index]] + } + } +} +action "action_example" "general" {} +action "action_example" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.action_example.general] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + // action_example.hello[2] before_create + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello-2"), + }), + }, + { + // action_example.there[2] after_create + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("there-2"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance. + Resource(addrs.ManagedResourceMode, "test_object", "a"). + Instance(addrs.IntKey(2)), + }, + }, + }, + + "targeted run with resource reference": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "source" { + name = "src" +} +action "action_example" "hello" { + config { + attr = test_object.source.name + } +} +action "action_example" "there" { + config { + attr = "there" + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } + action_trigger { + events = [after_create] + actions = [action.action_example.there] + } + } +} +action "action_example" "general" {} +action "action_example" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.action_example.general] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + // action_example.hello before_create with config (attr = test_object.source.name -> "src") + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("src"), + }), + }, + { + // action_example.there after_create with static config attr = "there" + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("there"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, + + "targeted run with condition referencing another resource": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "source" { + name = "source" +} +action "action_example" "hello" { + config { + attr = test_object.source.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + condition = test_object.source.name == "source" + actions = [action.action_example.hello] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + // action_example.hello before_create with config (attr = test_object.source.name -> "source") + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("source"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, + + "targeted run with action referencing another resource that also triggers actions": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + config { + attr = "hello" + } +} +resource "test_object" "source" { + name = "source" + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } + } +} +action "action_example" "there" { + config { + attr = test_object.source.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.action_example.there] + } + } +} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } + } +} +`, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + // action_example.hello before_create with static config attr = "hello" + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + }, + { + // action_example.there after_create with config attr = source.name ("source") + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("source"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, + + "targeted run triggers resources and actions referenced by not-running actions": { + module: map[string]string{ + "main.tf": ` +action "action_example" "hello" { + config { + attr = "hello" + } +} +resource "test_object" "source" { +name = "source" +lifecycle { + action_trigger { + events = [before_create] + actions = [action.action_example.hello] + } +} +} +action "action_example" "there" { +config { + attr = test_object.source.name +} +} +resource "test_object" "a" { +lifecycle { + action_trigger { + events = [after_update] + actions = [action.action_example.there] + } +} +} +resource "test_object" "b" { +lifecycle { + action_trigger { + events = [before_update] + actions = [action.action_example.hello] + } +} +} + `, + }, + ignoreWarnings: true, + expectInvokeActionCalled: true, + expectInvokeActionCalls: []providers.InvokeActionRequest{ + { + ActionType: "action_example", + PlannedActionData: cty.ObjectVal(map[string]cty.Value{ + "attr": cty.StringVal("hello"), + }), + }, + }, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -2097,7 +2533,11 @@ resource "test_object" "a" { } plan, diags := ctx.Plan(m, tc.prevRunState, planOpts) - tfdiags.AssertNoDiagnostics(t, diags) + if tc.ignoreWarnings { + tfdiags.AssertNoErrors(t, diags) + } else { + tfdiags.AssertNoDiagnostics(t, diags) + } if !plan.Applyable { t.Fatalf("plan is not applyable but should be") @@ -2107,7 +2547,11 @@ resource "test_object" "a" { if tc.expectDiagnostics != nil { tfdiags.AssertDiagnosticsMatch(t, diags, tc.expectDiagnostics(m)) } else { - tfdiags.AssertNoDiagnostics(t, diags) + if tc.ignoreWarnings { + tfdiags.AssertNoErrors(t, diags) + } else { + tfdiags.AssertNoDiagnostics(t, diags) + } } if tc.expectInvokeActionCalled && len(invokeActionCalls) == 0 { @@ -2115,7 +2559,7 @@ resource "test_object" "a" { } if len(tc.expectInvokeActionCalls) > 0 && len(invokeActionCalls) != len(tc.expectInvokeActionCalls) { - t.Fatalf("expected %d invoke action calls, got %d", len(tc.expectInvokeActionCalls), len(invokeActionCalls)) + t.Fatalf("expected %d invoke action calls, got %d (%#v)", len(tc.expectInvokeActionCalls), len(invokeActionCalls), invokeActionCalls) } for i, expectedCall := range tc.expectInvokeActionCalls { diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 5707875b87d4..b79fd1c8fca3 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -4,6 +4,7 @@ package terraform import ( + "maps" "path/filepath" "slices" "sort" @@ -144,7 +145,70 @@ action "test_action" "hello" {} } }, }, - + "query run": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.hello] + } + } +} +`, + "main.tfquery.hcl": ` +list "test_resource" "test1" { + provider = "test" + config { + filter = { + attr = "foo" + } + } +} +`, + }, + expectPlanActionCalled: false, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Query: true, + }, + }, + "query run, action references resource": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" { + config { + attr = resource.test_object.a.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.hello] + } + } +} +`, + "main.tfquery.hcl": ` +list "test_resource" "test1" { + provider = "test" + config { + filter = { + attr = "foo" + } + } +} +`, + }, + expectPlanActionCalled: false, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + Query: true, + }, + }, "invalid config": { module: map[string]string{ "main.tf": ` @@ -1754,7 +1818,7 @@ resource "test_object" "a" { expectPlanActionCalled: true, planOpts: &PlanOpts{ Mode: plans.NormalMode, - DeferralAllowed: true, + DeferralAllowed: true, // actions should ignore this setting }, planActionFn: func(*testing.T, providers.PlanActionRequest) providers.PlanActionResponse { return providers.PlanActionResponse{ @@ -1763,25 +1827,13 @@ resource "test_object" "a" { }, } }, - - assertPlan: func(t *testing.T, p *plans.Plan) { - if len(p.Changes.ActionInvocations) != 0 { - t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) - } - - if len(p.DeferredActionInvocations) != 1 { - t.Fatalf("expected 1 deferred action in plan, got %d", len(p.DeferredActionInvocations)) - } - deferredActionInvocation := p.DeferredActionInvocations[0] - if deferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { - t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", deferredActionInvocation.DeferredReason) - } - if deferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { - t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", deferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) - } - - if deferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_action.hello" { - t.Fatalf("expected deferred action to be triggered by action.test_action.hello, but got %s", deferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Provider deferred changes when Terraform did not allow deferrals", + `The provider signaled a deferred action for "action.test_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`, + ), } }, }, @@ -1824,37 +1876,17 @@ resource "test_object" "a" { }, } }, - - assertPlan: func(t *testing.T, p *plans.Plan) { - if len(p.Changes.ActionInvocations) != 0 { - t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) - } - - if len(p.DeferredActionInvocations) != 2 { - t.Fatalf("expected 2 deferred actions in plan, got %d", len(p.DeferredActionInvocations)) - } - firstDeferredActionInvocation := p.DeferredActionInvocations[0] - if firstDeferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { - t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", firstDeferredActionInvocation.DeferredReason) - } - if firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { - t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) - } - - if firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_action.hello" { - t.Fatalf("expected deferred action to be triggered by action.test_action.hello, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) - } - - secondDeferredActionInvocation := p.DeferredActionInvocations[1] - if secondDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Fatalf("expected second deferred action to be deferred due to deferred prereq, but got %s", secondDeferredActionInvocation.DeferredReason) - } - if secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { - t.Fatalf("expected second deferred action to be triggered by test_object.a, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) - } - - if secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.ecosystem.world" { - t.Fatalf("expected second deferred action to be triggered by action.ecosystem.world, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { + // for now, it's just an error for any deferrals but when + // this gets implemented we should check that all the + // actions are deferred even though only one of them + // was actually marked as deferred. + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Provider deferred changes when Terraform did not allow deferrals", + `The provider signaled a deferred action for "action.test_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`, + ), } }, }, @@ -1901,50 +1933,17 @@ resource "test_object" "a" { }, } }, - - assertPlan: func(t *testing.T, p *plans.Plan) { - if len(p.Changes.ActionInvocations) != 0 { - t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) - } - - if len(p.DeferredActionInvocations) != 2 { - t.Fatalf("expected 2 deferred actions in plan, got %d", len(p.DeferredActionInvocations)) - } - firstDeferredActionInvocation := p.DeferredActionInvocations[0] - if firstDeferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { - t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", firstDeferredActionInvocation.DeferredReason) - } - if firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { - t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) - } - - if firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_action.hello" { - t.Fatalf("expected deferred action to be triggered by action.test_action.hello, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) - } - - secondDeferredActionInvocation := p.DeferredActionInvocations[1] - if secondDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Fatalf("expected second deferred action to be deferred due to deferred prereq, but got %s", secondDeferredActionInvocation.DeferredReason) - } - if secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { - t.Fatalf("expected second deferred action to be triggered by test_object.a, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(*plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) - } - - if secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.ecosystem.world" { - t.Fatalf("expected second deferred action to be triggered by action.ecosystem.world, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) - } - - if len(p.DeferredResources) != 1 { - t.Fatalf("expected 1 resource to be deferred, got %d", len(p.DeferredResources)) - } - deferredResource := p.DeferredResources[0] - - if deferredResource.ChangeSrc.Addr.String() != "test_object.a" { - t.Fatalf("Expected resource %s to be deferred, but it was not", deferredResource.ChangeSrc.Addr) - } - - if deferredResource.DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Fatalf("Expected deferred reason to be deferred prereq, got %s", deferredResource.DeferredReason) + expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { + // for now, it's just an error for any deferrals but when + // this gets implemented we should check that all the + // actions are deferred even though only one of them + // was actually marked as deferred. + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Provider deferred changes when Terraform did not allow deferrals", + `The provider signaled a deferred action for "action.test_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`, + ), } }, }, @@ -2066,26 +2065,17 @@ resource "test_object" "a" { }, }, }, - assertPlan: func(t *testing.T, p *plans.Plan) { - if len(p.DeferredActionInvocations) != 1 { - t.Fatalf("expected exactly one invocation, and found %d", len(p.DeferredActionInvocations)) - } - - if p.DeferredActionInvocations[0].DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Fatalf("expected.DeferredReasonDeferredPrereq, got %s", p.DeferredActionInvocations[0].DeferredReason) - } - - ai := p.DeferredActionInvocations[0].ActionInvocationInstanceSrc - if ai.Addr.String() != `action.test_action.hello["a"]` { - t.Fatalf(`expected action invocation for action.test_action.hello["a"], got %s`, ai.Addr.String()) + assertPlanDiagnostics: func(t *testing.T, diagnostics tfdiags.Diagnostics) { + if len(diagnostics) != 1 { + t.Fatal("wrong number of diagnostics") } - if len(p.DeferredResources) != 1 { - t.Fatalf("expected 1 deferred resource, got %d", len(p.DeferredResources)) + if diagnostics[0].Severity() != tfdiags.Error { + t.Error("expected error severity") } - if p.DeferredResources[0].ChangeSrc.Addr.String() != "test_object.a" { - t.Fatalf("expected test_object.a, got %s", p.DeferredResources[0].ChangeSrc.Addr.String()) + if diagnostics[0].Description().Summary != "Invalid for_each argument" { + t.Errorf("expected for_each argument to be source of error but was %s", diagnostics[0].Description().Summary) } }, }, @@ -2271,42 +2261,17 @@ resource "test_object" "a" { } }, - assertPlan: func(t *testing.T, p *plans.Plan) { - if len(p.DeferredActionInvocations) != 1 { - t.Errorf("Expected 1 deferred action invocation, got %d", len(p.DeferredActionInvocations)) - } - if p.DeferredActionInvocations[0].ActionInvocationInstanceSrc.Addr.String() != "action.test_action.hello" { - t.Errorf("Expected action. test_action.hello, got %s", p.DeferredActionInvocations[0].ActionInvocationInstanceSrc.Addr.String()) - } - if p.DeferredActionInvocations[0].DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Errorf("Expected DeferredReasonDeferredPrereq, got %s", p.DeferredActionInvocations[0].DeferredReason) + assertPlanDiagnostics: func(t *testing.T, diagnostics tfdiags.Diagnostics) { + if len(diagnostics) != 1 { + t.Fatal("wrong number of diagnostics") } - if len(p.DeferredResources) != 2 { - t.Fatalf("Expected 2 deferred resources, got %d", len(p.DeferredResources)) + if diagnostics[0].Severity() != tfdiags.Error { + t.Error("expected error diagnostics") } - slices.SortFunc(p.DeferredResources, func(a, b *plans.DeferredResourceInstanceChangeSrc) int { - if a.ChangeSrc.Addr.Less(b.ChangeSrc.Addr) { - return -1 - } - if b.ChangeSrc.Addr.Less(a.ChangeSrc.Addr) { - return 1 - } - return 0 - }) - - if p.DeferredResources[0].ChangeSrc.Addr.String() != "test_object.a" { - t.Errorf("Expected test_object.a to be first, got %s", p.DeferredResources[0].ChangeSrc.Addr.String()) - } - if p.DeferredResources[0].DeferredReason != providers.DeferredReasonDeferredPrereq { - t.Errorf("Expected DeferredReasonDeferredPrereq, got %s", p.DeferredResources[0].DeferredReason) - } - if p.DeferredResources[1].ChangeSrc.Addr.String() != "test_object.origin" { - t.Errorf("Expected test_object.origin to be second, got %s", p.DeferredResources[1].ChangeSrc.Addr.String()) - } - if p.DeferredResources[1].DeferredReason != providers.DeferredReasonAbsentPrereq { - t.Errorf("Expected DeferredReasonAbsentPrereq, got %s", p.DeferredResources[1].DeferredReason) + if diagnostics[0].Description().Summary != "Invalid action deferral" { + t.Errorf("expected deferral to be source of error was %s", diagnostics[0].Description().Summary) } }, }, @@ -3240,6 +3205,521 @@ resource "test_object" "a" { }, }, }, + // ======== TARGETING ======== + // -target flag behavior + // ======== TARGETING ======== + "targeting": { + "targeted run": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" {} +action "test_action" "there" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + action_trigger { + events = [after_create] + actions = [action.test_action.there] + } + } +} +action "test_action" "general" {} +action "test_action" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.general] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // We only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + // There is a warning related to targeting that we will just ignore + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Validate we are targeting resource a out of paranoia + if len(p.Changes.Resources) != 1 { + t.Fatalf("expected plan to have 1 resource change, got %d", len(p.Changes.Resources)) + } + if p.Changes.Resources[0].Addr.String() != "test_object.a" { + t.Fatalf("expected plan to target resource 'test_object.a', got %s", p.Changes.Resources[0].Addr.String()) + } + + // Ensure the actions for test_object.a are planned + if len(p.Changes.ActionInvocations) != 2 { + t.Fatalf("expected plan to have 2 action invocations, got %d", len(p.Changes.ActionInvocations)) + } + + actionAddrs := []string{ + p.Changes.ActionInvocations[0].Addr.String(), + p.Changes.ActionInvocations[1].Addr.String(), + } + + slices.Sort(actionAddrs) + if actionAddrs[0] != "action.test_action.hello" || actionAddrs[1] != "action.test_action.there" { + t.Fatalf("expected action addresses to be ['action.test_action.hello', 'action.test_action.there'], got %v", actionAddrs) + } + + }, + }, + "targeted run with ancestor that has actions": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" {} +action "test_action" "there" {} +resource "test_object" "origin" { + name = "origin" + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + } +} +resource "test_object" "a" { + name = test_object.origin.name + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_action.there] + } + } +} +action "test_action" "general" {} +action "test_action" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.general] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // We only target resource a + Targets: []addrs.Targetable{ + mustResourceInstanceAddr("test_object.a"), + }, + }, + // There is a warning related to targeting that we will just ignore + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Validate we are targeting resource a out of paranoia + if len(p.Changes.Resources) != 2 { + t.Fatalf("expected plan to have 2 resource changes, got %d", len(p.Changes.Resources)) + } + resourceAddrs := []string{ + p.Changes.Resources[0].Addr.String(), + p.Changes.Resources[1].Addr.String(), + } + + slices.Sort(resourceAddrs) + if resourceAddrs[0] != "test_object.a" || resourceAddrs[1] != "test_object.origin" { + t.Fatalf("expected resource addresses to be ['test_object.a', 'test_object.origin'], got %v", resourceAddrs) + } + + // Ensure the actions for test_object.a are planned + if len(p.Changes.ActionInvocations) != 2 { + t.Fatalf("expected plan to have 2 action invocations, got %d", len(p.Changes.ActionInvocations)) + } + + actionAddrs := []string{ + p.Changes.ActionInvocations[0].Addr.String(), + p.Changes.ActionInvocations[1].Addr.String(), + } + + slices.Sort(actionAddrs) + if actionAddrs[0] != "action.test_action.hello" || actionAddrs[1] != "action.test_action.there" { + t.Fatalf("expected action addresses to be ['action.test_action.hello', 'action.test_action.there'], got %v", actionAddrs) + } + + }, + }, + "targeted run with expansion": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" { + count = 3 +} +action "test_action" "there" { + count = 3 +} +resource "test_object" "a" { + count = 3 + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello[count.index]] + } + action_trigger { + events = [after_create] + actions = [action.test_action.there[count.index]] + } + } +} +action "test_action" "general" {} +action "test_action" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.general] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // We only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a").Instance(addrs.IntKey(2)), + }, + }, + // There is a warning related to targeting that we will just ignore + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Validate we are targeting resource a out of paranoia + if len(p.Changes.Resources) != 1 { + t.Fatalf("expected plan to have 1 resource change, got %d", len(p.Changes.Resources)) + } + if p.Changes.Resources[0].Addr.String() != "test_object.a[2]" { + t.Fatalf("expected plan to target resource 'test_object.a[2]', got %s", p.Changes.Resources[0].Addr.String()) + } + + // Ensure the actions for test_object.a are planned + if len(p.Changes.ActionInvocations) != 2 { + t.Fatalf("expected plan to have 2 action invocations, got %d", len(p.Changes.ActionInvocations)) + } + + actionAddrs := []string{ + p.Changes.ActionInvocations[0].Addr.String(), + p.Changes.ActionInvocations[1].Addr.String(), + } + + slices.Sort(actionAddrs) + if actionAddrs[0] != "action.test_action.hello[2]" || actionAddrs[1] != "action.test_action.there[2]" { + t.Fatalf("expected action addresses to be ['action.test_action.hello[2]', 'action.test_action.there[2]'], got %v", actionAddrs) + } + }, + }, + "targeted run with resource reference": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "source" {} +action "test_action" "hello" { + config { + attr = test_object.source.name + } +} +action "test_action" "there" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + action_trigger { + events = [after_create] + actions = [action.test_action.there] + } + } +} +action "test_action" "general" {} +action "test_action" "kenobi" {} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create, after_update] + actions = [action.test_action.general] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // We only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + // There is a warning related to targeting that we will just ignore + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Validate we are targeting resource a out of paranoia + if len(p.Changes.Resources) != 2 { + t.Fatalf("expected plan to have 2 resource changes, got %d", len(p.Changes.Resources)) + } + resourceAddrs := []string{ + p.Changes.Resources[0].Addr.String(), + p.Changes.Resources[1].Addr.String(), + } + slices.Sort(resourceAddrs) + if resourceAddrs[0] != "test_object.a" || resourceAddrs[1] != "test_object.source" { + t.Fatalf("expected resource addresses to be ['test_object.a', 'test_object.source'], got %v", resourceAddrs) + } + + // Ensure the actions for test_object.a are planned + if len(p.Changes.ActionInvocations) != 2 { + t.Fatalf("expected plan to have 2 action invocations, got %d", len(p.Changes.ActionInvocations)) + } + + actionAddrs := []string{ + p.Changes.ActionInvocations[0].Addr.String(), + p.Changes.ActionInvocations[1].Addr.String(), + } + + slices.Sort(actionAddrs) + if actionAddrs[0] != "action.test_action.hello" || actionAddrs[1] != "action.test_action.there" { + t.Fatalf("expected action addresses to be ['action.test_action.hello', 'action.test_action.there'], got %v", actionAddrs) + } + }, + }, + + "targeted run with condition referencing another resource": { + module: map[string]string{ + "main.tf": ` +resource "test_object" "source" { + name = "source" +} +action "test_action" "hello" { + config { + attr = test_object.source.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + condition = test_object.source.name == "source" + actions = [action.test_action.hello] + } + } +} + `, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // Only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Only resource a should be planned + if len(p.Changes.Resources) != 2 { + t.Fatalf("expected plan to have 2 resource changes, got %d", len(p.Changes.Resources)) + } + resourceAddrs := []string{p.Changes.Resources[0].Addr.String(), p.Changes.Resources[1].Addr.String()} + slices.Sort(resourceAddrs) + + if resourceAddrs[0] != "test_object.a" || resourceAddrs[1] != "test_object.source" { + t.Fatalf("expected resource addresses to be ['test_object.a', 'test_object.source'], got %v", resourceAddrs) + } + + // Only one action invocation for resource a + if len(p.Changes.ActionInvocations) != 1 { + t.Fatalf("expected plan to have 1 action invocation, got %d", len(p.Changes.ActionInvocations)) + } + if p.Changes.ActionInvocations[0].Addr.String() != "action.test_action.hello" { + t.Fatalf("expected action address to be 'action.test_action.hello', got '%s'", p.Changes.ActionInvocations[0].Addr) + } + }, + }, + + "targeted run with action referencing another resource that also triggers actions": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" {} +resource "test_object" "source" { + name = "source" + + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + } +} +action "test_action" "there" { + config { + attr = test_object.source.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_action.there] + } + } +} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + } +} + `, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // Only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Should plan for resource a and its dependency source, but not b + if len(p.Changes.Resources) != 2 { + t.Fatalf("expected plan to have 2 resource changes, got %d", len(p.Changes.Resources)) + } + resourceAddrs := []string{ + p.Changes.Resources[0].Addr.String(), + p.Changes.Resources[1].Addr.String(), + } + slices.Sort(resourceAddrs) + if resourceAddrs[0] != "test_object.a" || resourceAddrs[1] != "test_object.source" { + t.Fatalf("expected resource addresses to be ['test_object.a', 'test_object.source'], got %v", resourceAddrs) + } + + // Should plan both actions for resource a + if len(p.Changes.ActionInvocations) != 2 { + t.Fatalf("expected plan to have 2 action invocations, got %d", len(p.Changes.ActionInvocations)) + } + actionAddrs := []string{ + p.Changes.ActionInvocations[0].Addr.String(), + p.Changes.ActionInvocations[1].Addr.String(), + } + slices.Sort(actionAddrs) + if actionAddrs[0] != "action.test_action.hello" || actionAddrs[1] != "action.test_action.there" { + t.Fatalf("expected action addresses to be ['action.test_action.hello', 'action.test_action.there'], got %v", actionAddrs) + } + }, + }, + "targeted run with not-triggered action referencing another resource that also triggers actions": { + module: map[string]string{ + "main.tf": ` +action "test_action" "hello" {} +resource "test_object" "source" { + name = "source" + + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_action.hello] + } + } +} +action "test_action" "there" { + config { + attr = test_object.source.name + } +} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [after_update] + actions = [action.test_action.there] + } + } +} +resource "test_object" "b" { + lifecycle { + action_trigger { + events = [before_update] + actions = [action.test_action.hello] + } + } +} + `, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + // Only target resource a + Targets: []addrs.Targetable{ + addrs.RootModuleInstance.Resource(addrs.ManagedResourceMode, "test_object", "a"), + }, + }, + assertPlanDiagnostics: func(t *testing.T, d tfdiags.Diagnostics) { + if d.HasErrors() { + t.Fatalf("expected no errors, got %s", d.Err().Error()) + } + }, + assertPlan: func(t *testing.T, p *plans.Plan) { + // Should plan for resource a and its dependency source, but not b + if len(p.Changes.Resources) != 2 { + t.Fatalf("expected plan to have 2 resource changes, got %d", len(p.Changes.Resources)) + } + resourceAddrs := []string{ + p.Changes.Resources[0].Addr.String(), + p.Changes.Resources[1].Addr.String(), + } + slices.Sort(resourceAddrs) + if resourceAddrs[0] != "test_object.a" || resourceAddrs[1] != "test_object.source" { + t.Fatalf("expected resource addresses to be ['test_object.a', 'test_object.source'], got %v", resourceAddrs) + } + + // Should plan only the before_create action of the dependant resource + if len(p.Changes.ActionInvocations) != 1 { + t.Fatalf("expected plan to have 1 action invocation, got %d", len(p.Changes.ActionInvocations)) + } + if p.Changes.ActionInvocations[0].Addr.String() != "action.test_action.hello" { + t.Fatalf("expected action addresses to be 'action.test_action.hello', got %q", p.Changes.ActionInvocations[0].Addr.String()) + } + }, + }, + }, } { t.Run(topic, func(t *testing.T) { for name, tc := range tcs { @@ -3248,7 +3728,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{ @@ -3269,6 +3759,47 @@ resource "test_object" "a" { }, }, }, + ListResourceTypes: map[string]providers.Schema{ + "test_resource": { + Body: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "data": { + Type: cty.DynamicPseudoType, + Computed: true, + }, + }, + BlockTypes: map[string]*configschema.NestedBlock{ + "config": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "filter": { + Required: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Required: true, + }, + }, + }, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, + }, + }, + }, + }, + }, + ListResourceFn: func(req providers.ListResourceRequest) providers.ListResourceResponse { + resp := []cty.Value{} + ret := req.Config.AsValueMap() + maps.Copy(ret, map[string]cty.Value{ + "data": cty.TupleVal(resp), + }) + return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} }, } @@ -3338,7 +3869,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 { @@ -3356,11 +3889,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_identity_test.go b/internal/terraform/context_plan_identity_test.go index 1c6a573f7091..aa8535edc05a 100644 --- a/internal/terraform/context_plan_identity_test.go +++ b/internal/terraform/context_plan_identity_test.go @@ -309,6 +309,76 @@ func TestContext2Plan_resource_identity_refresh(t *testing.T) { } } +func TestContext2Plan_resource_identity_refresh_downgrade(t *testing.T) { + p := testProvider("aws") + m := testModule(t, "refresh-basic") + p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&providerSchema{ + ResourceTypes: map[string]*configschema.Block{ + "aws_instance": { + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "foo": { + Type: cty.String, + Optional: true, + Computed: true, + }, + }, + }, + }, + }) + + state := states.NewState() + root := state.EnsureModule(addrs.RootModuleInstance) + + root.SetResourceInstanceCurrent( + mustResourceInstanceAddr("aws_instance.web").Resource, + &states.ResourceInstanceObjectSrc{ + Status: states.ObjectReady, + AttrsJSON: []byte(`{"id":"foo","foo":"bar"}`), + IdentitySchemaVersion: 0, + IdentityJSON: []byte(`{"id": "foo"}`), + }, + mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`), + ) + + ctx := testContext2(t, &ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p), + }, + }) + + schema := p.GetProviderSchemaResponse.ResourceTypes["aws_instance"] + + p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse { + return providers.ReadResourceResponse{ + NewState: req.PriorState, + } + } + + s, diags := ctx.Plan(m, state, &PlanOpts{Mode: plans.RefreshOnlyMode}) + + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + if !p.ReadResourceCalled { + t.Fatal("ReadResource should be called") + } + + mod := s.PriorState.RootModule() + fromState, err := mod.Resources["aws_instance.web"].Instances[addrs.NoKey].Current.Decode(schema) + if err != nil { + t.Fatal(err) + } + + if !fromState.Identity.IsNull() { + t.Fatalf("wrong identity\nwant: null\ngot: %s", fromState.Identity.GoString()) + } +} + // This test validates if a resource identity that is deposed and will be destroyed // can be refreshed with an identity during the plan. func TestContext2Plan_resource_identity_refresh_destroy_deposed(t *testing.T) { diff --git a/internal/terraform/context_plan_query_test.go b/internal/terraform/context_plan_query_test.go index 3f95027dcd26..75b016003fe1 100644 --- a/internal/terraform/context_plan_query_test.go +++ b/internal/terraform/context_plan_query_test.go @@ -6,13 +6,16 @@ package terraform import ( "fmt" "maps" + "slices" "sort" "strings" "testing" "github.com/google/go-cmp/cmp" "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" @@ -23,16 +26,61 @@ 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]), + "id": cty.StringVal(fmt.Sprint(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)} + } + + type resources struct { + list map[string]bool // map of list resource addresses to whether they want the resource state included in the response + managed []string + } + cases := []struct { - name string - mainConfig string - queryConfig string - generatedPath string - diagCount int - expectedErrMsg []string - assertState func(*states.State) - assertChanges func(providers.ProviderSchema, *plans.ChangesSrc) - listResourceFn func(request providers.ListResourceRequest) providers.ListResourceResponse + name string + mainConfig string + queryConfig string + generatedPath string + transformSchema func(*providers.GetProviderSchemaResponse) + assertValidateDiags func(t *testing.T, diags tfdiags.Diagnostics) + assertPlanDiags func(t *testing.T, diags tfdiags.Diagnostics) + expectedResources resources }{ { name: "valid list reference - generates config", @@ -74,76 +122,12 @@ 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)} - }, - assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { - expectedResources := []string{"list.test_resource.test", "list.test_resource.test2"} - actualResources := make([]string, 0) - generatedCfgs := make([]string, 0) - for _, change := range changes.Queries { - actualResources = append(actualResources, change.Addr.String()) - schema := sch.ListResourceTypes[change.Addr.Resource.Resource.Type] - cs, err := change.Decode(schema) - if err != nil { - t.Fatalf("failed to decode change: %s", err) - } - - 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 { - if val.Type().HasAttribute("state") { - val = val.GetAttr("state") - if !val.IsNull() { - if val.GetAttr("instance_type").IsNull() { - t.Fatalf("Expected 'instance_type' attribute to be present, but it is missing") - } - } - } - - return false - }) - generatedCfgs = append(generatedCfgs, change.Generated.String()) - } - - if diff := cmp.Diff(expectedResources, actualResources); diff != "" { - t.Fatalf("Expected resources to match, but they differ: %s", diff) - } - - if diff := cmp.Diff([]string{testResourceCfg, testResourceCfg2}, generatedCfgs); diff != "" { - t.Fatalf("Expected generated configs to match, but they differ: %s", diff) - } + expectedResources: resources{ + list: map[string]bool{ + "list.test_resource.test": true, + "list.test_resource.test2": false, + }, + managed: []string{}, }, }, { @@ -187,78 +171,120 @@ 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")}), + expectedResources: resources{ + list: map[string]bool{"list.test_resource.test[0]": true, "list.test_resource.test2": true}, + managed: []string{}, + }, + }, + { + name: "with empty config when it is required", + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } } - 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)), - })) + `, + queryConfig: ` + variable "input" { + type = string + default = "foo" } - 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)), - })) + list "test_resource" "test" { + provider = test } + `, - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v - } + transformSchema: func(schema *providers.GetProviderSchemaResponse) { + schema.ListResourceTypes["test_resource"].Body.BlockTypes = map[string]*configschema.NestedBlock{ + "config": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "filter": { + Required: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, + }, + }, + Nesting: configschema.NestingSingle, + MinItems: 1, + MaxItems: 1, + }, } - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} }, - assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { - expectedResources := []string{"list.test_resource.test[0]", "list.test_resource.test2"} - actualResources := make([]string, 0) - for _, change := range changes.Queries { - actualResources = append(actualResources, change.Addr.String()) - schema := sch.ListResourceTypes[change.Addr.Resource.Resource.Type] - cs, err := change.Decode(schema) - if err != nil { - t.Fatalf("failed to decode change: %s", err) - } - - // Verify instance types - expectedTypes := []string{"ami-123456", "ami-654321"} - 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") - if val.IsNull() { - t.Fatalf("Expected 'state' 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") + assertValidateDiags: func(t *testing.T, diags tfdiags.Diagnostics) { + tfdiags.AssertDiagnosticCount(t, diags, 1) + var exp tfdiags.Diagnostics + exp = exp.Append(&hcl.Diagnostic{ + Summary: "Missing config block", + Detail: "A block of type \"config\" is required here.", + Subject: diags[0].Source().Subject.ToHCL().Ptr(), + }) + tfdiags.AssertDiagnosticsMatch(t, diags, exp) + }, + }, + { + name: "with empty optional config", + mainConfig: ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" } - actualTypes = append(actualTypes, val.GetAttr("instance_type").AsString()) - return false - }) - sort.Strings(actualTypes) - sort.Strings(expectedTypes) - if diff := cmp.Diff(expectedTypes, actualTypes); diff != "" { - t.Fatalf("Expected instance types to match, but they differ: %s", diff) } } - sort.Strings(actualResources) - sort.Strings(expectedResources) - if diff := cmp.Diff(expectedResources, actualResources); diff != "" { - t.Fatalf("Expected resources to match, but they differ: %s", diff) + `, + queryConfig: ` + variable "input" { + type = string + default = "foo" + } + + list "test_resource" "test" { + provider = test + } + `, + transformSchema: func(schema *providers.GetProviderSchemaResponse) { + schema.ListResourceTypes["test_resource"].Body.BlockTypes = map[string]*configschema.NestedBlock{ + "config": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "filter": { + Optional: true, + NestedType: &configschema.Object{ + Nesting: configschema.NestingSingle, + Attributes: map[string]*configschema.Attribute{ + "attr": { + Type: cty.String, + Optional: true, + }, + }, + }, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, } + + }, + expectedResources: resources{ + list: map[string]bool{"list.test_resource.test": false}, + managed: []string{}, }, }, { @@ -301,10 +327,17 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - diagCount: 1, - expectedErrMsg: []string{ - "Invalid list resource traversal", - "The first step in the traversal for a list resource must be an attribute \"data\"", + assertValidateDiags: func(t *testing.T, diags tfdiags.Diagnostics) { + tfdiags.AssertDiagnosticCount(t, diags, 1) + var exp tfdiags.Diagnostics + exp = exp.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid list resource traversal", + Detail: "The first step in the traversal for a list resource must be an attribute \"data\".", + Subject: diags[0].Source().Subject.ToHCL().Ptr(), + }) + + tfdiags.AssertDiagnosticsMatch(t, diags, exp) }, }, { @@ -332,9 +365,18 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - diagCount: 1, - expectedErrMsg: []string{ - "A list resource \"non_existent\" \"attr\" has not been declared in the root module.", + assertValidateDiags: func(t *testing.T, diags tfdiags.Diagnostics) { + tfdiags.AssertDiagnosticCount(t, diags, 1) + var exp tfdiags.Diagnostics + + exp = exp.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to undeclared resource", + Detail: "A list resource \"non_existent\" \"attr\" has not been declared in the root module.", + Subject: diags[0].Source().Subject.ToHCL().Ptr(), + }) + + tfdiags.AssertDiagnosticsMatch(t, diags, exp) }, }, { @@ -373,40 +415,18 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - diagCount: 1, - expectedErrMsg: []string{ - "Unsupported attribute: This object has no argument, nested block, or exported attribute named \"invalid_attr\".", - }, - 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 - } - } + assertValidateDiags: func(t *testing.T, diags tfdiags.Diagnostics) { + tfdiags.AssertDiagnosticCount(t, diags, 1) + var exp tfdiags.Diagnostics + + exp = exp.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Unsupported attribute", + Detail: "This object has no argument, nested block, or exported attribute named \"invalid_attr\".", + Subject: diags[0].Source().Subject.ToHCL().Ptr(), + }) - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} + tfdiags.AssertDiagnosticsMatch(t, diags, exp) }, }, { @@ -444,9 +464,14 @@ func TestContext2Plan_queryList(t *testing.T) { } } `, - diagCount: 1, - expectedErrMsg: []string{ - "Cycle: list.test_resource", + assertValidateDiags: func(t *testing.T, diags tfdiags.Diagnostics) { + tfdiags.AssertDiagnosticCount(t, diags, 1) + if !strings.Contains(diags[0].Description().Summary, "Cycle: list.test_resource") { + t.Errorf("Expected error message to contain 'Cycle: list.test_resource', got %q", diags[0].Description().Summary) + } + if diags[0].Severity() != tfdiags.Error { + t.Errorf("Expected error severity to be Error, got %s", diags[0].Severity()) + } }, }, { @@ -490,82 +515,13 @@ 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)} - }, - assertChanges: func(sch providers.ProviderSchema, changes *plans.ChangesSrc) { - expectedResources := []string{"list.test_resource.test1", "list.test_resource.test2"} - actualResources := make([]string, 0) - for _, change := range changes.Queries { - actualResources = append(actualResources, change.Addr.String()) - schema := sch.ListResourceTypes[change.Addr.Resource.Resource.Type] - cs, err := change.Decode(schema) - if err != nil { - t.Fatalf("failed to decode change: %s", err) - } - - // Verify instance types - expectedTypes := []string{"ami-123456"} - 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") - if val.IsNull() { - t.Fatalf("Expected 'state' 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") - } - actualTypes = append(actualTypes, val.GetAttr("instance_type").AsString()) - return false - }) - sort.Strings(actualTypes) - sort.Strings(expectedTypes) - if diff := cmp.Diff(expectedTypes, actualTypes); diff != "" { - t.Fatalf("Expected instance types to match, but they differ: %s", diff) - } - } - sort.Strings(actualResources) - sort.Strings(expectedResources) - if diff := cmp.Diff(expectedResources, actualResources); diff != "" { - t.Fatalf("Expected resources to match, but they differ: %s", diff) - } + expectedResources: resources{ + list: map[string]bool{"list.test_resource.test1": true, "list.test_resource.test2": true}, + managed: []string{}, }, }, { - // Test list reference with index but without data field - name: "list reference with index but without data field", + name: "list reference as for_each", mainConfig: ` terraform { required_providers { @@ -601,101 +557,139 @@ 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)), - })) + expectedResources: resources{ + list: map[string]bool{ + "list.test_resource.test1[\"foo\"]": true, + "list.test_resource.test1[\"bar\"]": true, + "list.test_resource.test2[\"foo\"]": true, + "list.test_resource.test2[\"bar\"]": true, + }, + managed: []string{}, + }, + }, + { + 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" + } + } } - - 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)), - })) + + 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 - ret := map[string]cty.Value{ - "data": cty.TupleVal(resp), - } - for k, v := range request.Config.AsValueMap() { - if k != "data" { - ret[k] = v + config { + filter = { + attr = "foo" + } } } - - return providers.ListResourceResponse{Result: cty.ObjectVal(ret)} + `, + expectedResources: resources{ + list: map[string]bool{ + "list.test_resource.test": true, + }, + managed: []string{}, }, - 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) - for _, change := range changes.Queries { - actualResources = append(actualResources, change.Addr.String()) - schema := sch.ListResourceTypes[change.Addr.Resource.Resource.Type] - cs, err := change.Decode(schema) - if err != nil { - t.Fatalf("failed to decode change: %s", err) + }, + { + 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 - // Verify instance types - expectedTypes := []string{"ami-123456"} - 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") - if val.IsNull() { - t.Fatalf("Expected 'state' 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") + config { + filter = { + attr = "foo" } - actualTypes = append(actualTypes, val.GetAttr("instance_type").AsString()) - return false - }) - sort.Strings(actualTypes) - sort.Strings(expectedTypes) - if diff := cmp.Diff(expectedTypes, actualTypes); diff != "" { - t.Fatalf("Expected instance types to match, but they differ: %s", diff) } } - sort.Strings(actualResources) - sort.Strings(expectedResources) - if diff := cmp.Diff(expectedResources, actualResources); diff != "" { - t.Fatalf("Expected resources to match, but they differ: %s", diff) - } + `, + expectedResources: resources{ + list: map[string]bool{ + "list.test_resource.test": true, + }, + managed: []string{"test_resource.example"}, }, }, } 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{}) provider.GetProviderSchemaResponse = getListProviderSchemaResp() + if tc.transformSchema != nil { + tc.transformSchema(provider.GetProviderSchemaResponse) + } var requestConfigs = make(map[string]cty.Value) provider.ListResourceFn = func(request providers.ListResourceRequest) providers.ListResourceResponse { - requestConfigs[request.TypeName] = request.Config - fn := tc.listResourceFn - if fn == nil { - return provider.ListResourceResponse + if request.Config.IsNull() || request.Config.GetAttr("config").IsNull() { + t.Fatalf("config should never be null, got null for %s", request.TypeName) } - return fn(request) + requestConfigs[request.TypeName] = request.Config + return listResourceFn(request) } ctx, diags := NewContext(&ContextOpts{ @@ -705,32 +699,110 @@ func TestContext2Plan_queryList(t *testing.T) { }) tfdiags.AssertNoDiagnostics(t, diags) + diags = ctx.Validate(mod, &ValidateOpts{ + Query: true, + }) + if tc.assertValidateDiags != nil { + tc.assertValidateDiags(t, diags) + return + } else { + tfdiags.AssertNoDiagnostics(t, diags) + } + plan, diags := ctx.Plan(mod, states.NewState(), &PlanOpts{ Mode: plans.NormalMode, SetVariables: testInputValuesUnset(mod.Module.Variables), Query: true, GenerateConfigPath: tc.generatedPath, }) - if len(diags) != tc.diagCount { - t.Fatalf("expected %d diagnostics, got %d \n -diags: %s", tc.diagCount, len(diags), diags) + if tc.assertPlanDiags != nil { + tc.assertPlanDiags(t, diags) + return + } else { + tfdiags.AssertNoDiagnostics(t, diags) } - if tc.assertChanges != nil { + // If no diags expected, assert that the plan is valid + if tc.assertValidateDiags == nil && tc.assertPlanDiags == nil { sch, err := ctx.Schemas(mod, states.NewState()) if err != nil { t.Fatalf("failed to get schemas: %s", err) } - tc.assertChanges(sch.Providers[providerAddr], plan.Changes) - } + expectedResources := slices.Collect(maps.Keys(tc.expectedResources.list)) + actualResources := make([]string, 0) + generatedCfgs := make([]string, 0) + for _, change := range plan.Changes.Queries { + actualResources = append(actualResources, change.Addr.String()) + schema := sch.Providers[providerAddr].ListResourceTypes[change.Addr.Resource.Resource.Type] + cs, err := change.Decode(schema) + if err != nil { + t.Fatalf("failed to decode change: %s", err) + } - if tc.diagCount > 0 { - for _, err := range tc.expectedErrMsg { - if !strings.Contains(diags.Err().Error(), err) { - t.Fatalf("expected error message %q, but got %q", err, diags.Err().Error()) + // Verify data. If the state is included, we check that, otherwise we check the id. + expectedData := []string{"ami-123456", "ami-654321", "ami-789012"} + includeState := tc.expectedResources.list[change.Addr.String()] + if !includeState { + expectedData = []string{"i-v1", "i-v2", "i-v3"} + } + actualData := 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 { + if includeState { + val = val.GetAttr("state") + if val.IsNull() { + t.Fatalf("Expected 'state' 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") + } + actualData = append(actualData, val.GetAttr("instance_type").AsString()) + } else { + val = val.GetAttr("identity") + if val.IsNull() { + t.Fatalf("Expected 'identity' attribute to be present, but it is null") + } + if val.GetAttr("id").IsNull() { + t.Fatalf("Expected 'id' attribute to be present, but it is missing") + } + actualData = append(actualData, val.GetAttr("id").AsString()) + } + return false + }) + sort.Strings(actualData) + sort.Strings(expectedData) + if diff := cmp.Diff(expectedData, actualData); diff != "" { + t.Fatalf("Expected instance types to match, but they differ: %s", diff) + } + + if tc.generatedPath != "" { + generatedCfgs = append(generatedCfgs, change.Generated.String()) } } - } + sort.Strings(actualResources) + sort.Strings(expectedResources) + if diff := cmp.Diff(expectedResources, actualResources); diff != "" { + t.Fatalf("Expected resources to match, but they differ: %s", diff) + } + + expectedManagedResources := tc.expectedResources.managed + actualResources = make([]string, 0) + for _, change := range plan.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) + } + if tc.generatedPath != "" { + if diff := cmp.Diff([]string{testResourceCfg, testResourceCfg2}, generatedCfgs); diff != "" { + t.Fatalf("Expected generated configs to match, but they differ: %s", diff) + } + } + } }) } } @@ -826,10 +898,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") @@ -837,6 +909,9 @@ func TestContext2Plan_queryListArgs(t *testing.T) { provider.GetProviderSchemaResponse = getListProviderSchemaResp() var recordedRequest providers.ListResourceRequest provider.ListResourceFn = func(request providers.ListResourceRequest) providers.ListResourceResponse { + if request.Config.IsNull() || request.Config.GetAttr("config").IsNull() { + t.Fatalf("config should never be null, got null for %s", request.TypeName) + } recordedRequest = request return provider.ListResourceResponse } @@ -908,6 +983,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{ @@ -924,6 +1007,10 @@ func getListProviderSchemaResp() *providers.GetProviderSchemaResponse { Computed: true, Optional: true, }, + "id": { + Type: cty.String, + Computed: true, + }, }, }, "test_child_resource": { @@ -962,6 +1049,239 @@ func getListProviderSchemaResp() *providers.GetProviderSchemaResponse { }) } +func TestContext2Plan_queryListConfigGeneration(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)} + } + + mainConfig := ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + ` + queryConfig := ` + variable "input" { + type = string + default = "foo" + } + + list "test_resource" "test2" { + for_each = toset(["§us-east-2", "§us-west-1"]) + provider = test + + config { + filter = { + attr = var.input + } + } + } + ` + + configFiles := map[string]string{"main.tf": mainConfig} + configFiles["main.tfquery.hcl"] = queryConfig + + mod := testModuleInline(t, configFiles, configs.MatchQueryFiles()) + providerAddr := addrs.NewDefaultProvider("test") + provider := testProvider("test") + provider.ConfigureProvider(providers.ConfigureProviderRequest{}) + provider.GetProviderSchemaResponse = getListProviderSchemaResp() + + var requestConfigs = make(map[string]cty.Value) + provider.ListResourceFn = func(request providers.ListResourceRequest) providers.ListResourceResponse { + if request.Config.IsNull() || request.Config.GetAttr("config").IsNull() { + t.Fatalf("config should never be null, got null for %s", request.TypeName) + } + requestConfigs[request.TypeName] = request.Config + return listResourceFn(request) + } + + ctx, diags := NewContext(&ContextOpts{ + Providers: map[addrs.Provider]providers.Factory{ + providerAddr: testProviderFuncFixed(provider), + }, + }) + tfdiags.AssertNoDiagnostics(t, diags) + + diags = ctx.Validate(mod, &ValidateOpts{ + Query: true, + }) + tfdiags.AssertNoDiagnostics(t, diags) + + generatedPath := t.TempDir() + plan, diags := ctx.Plan(mod, states.NewState(), &PlanOpts{ + Mode: plans.NormalMode, + SetVariables: testInputValuesUnset(mod.Module.Variables), + Query: true, + GenerateConfigPath: generatedPath, + }) + tfdiags.AssertNoDiagnostics(t, diags) + + sch, err := ctx.Schemas(mod, states.NewState()) + if err != nil { + t.Fatalf("failed to get schemas: %s", err) + } + + expectedResources := []string{ + `list.test_resource.test2["§us-east-2"]`, + `list.test_resource.test2["§us-west-1"]`, + } + actualResources := make([]string, 0) + generatedCfgs := make([]string, 0) + uniqCfgs := make(map[string]struct{}) + + for _, change := range plan.Changes.Queries { + actualResources = append(actualResources, change.Addr.String()) + schema := sch.Providers[providerAddr].ListResourceTypes[change.Addr.Resource.Resource.Type] + cs, err := change.Decode(schema) + if err != nil { + t.Fatalf("failed to decode change: %s", err) + } + + // Verify data. If the state is included, we check that, otherwise we check the id. + expectedData := []string{"ami-123456", "ami-654321", "ami-789012"} + includeState := change.Addr.String() == "list.test_resource.test" + if !includeState { + expectedData = []string{"i-v1", "i-v2", "i-v3"} + } + actualData := 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 { + if includeState { + val = val.GetAttr("state") + if val.IsNull() { + t.Fatalf("Expected 'state' 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") + } + actualData = append(actualData, val.GetAttr("instance_type").AsString()) + } else { + val = val.GetAttr("identity") + if val.IsNull() { + t.Fatalf("Expected 'identity' attribute to be present, but it is null") + } + if val.GetAttr("id").IsNull() { + t.Fatalf("Expected 'id' attribute to be present, but it is missing") + } + actualData = append(actualData, val.GetAttr("id").AsString()) + } + return false + }) + sort.Strings(actualData) + sort.Strings(expectedData) + if diff := cmp.Diff(expectedData, actualData); diff != "" { + t.Fatalf("Expected instance types to match, but they differ: %s", diff) + } + + generatedCfgs = append(generatedCfgs, change.Generated.String()) + uniqCfgs[change.Addr.String()] = struct{}{} + } + + sort.Strings(actualResources) + sort.Strings(expectedResources) + if diff := cmp.Diff(expectedResources, actualResources); diff != "" { + t.Fatalf("Expected resources to match, but they differ: %s", diff) + } + + // Verify no managed resources are created + if len(plan.Changes.Resources) != 0 { + t.Fatalf("Expected no managed resources, but got %d", len(plan.Changes.Resources)) + } + + // Verify generated configs match expected + expected := `resource "test_resource" "test2_0_0" { + provider = test + instance_type = "ami-123456" +} + +import { + to = test_resource.test2_0_0 + provider = test + identity = { + id = "i-v1" + } +} + +resource "test_resource" "test2_0_1" { + provider = test + instance_type = "ami-654321" +} + +import { + to = test_resource.test2_0_1 + provider = test + identity = { + id = "i-v2" + } +} + +resource "test_resource" "test2_0_2" { + provider = test + instance_type = "ami-789012" +} + +import { + to = test_resource.test2_0_2 + provider = test + identity = { + id = "i-v3" + } +} +` + joinedCfgs := strings.Join(generatedCfgs, "\n") + if !strings.Contains(joinedCfgs, expected) { + t.Fatalf("Expected config to contain expected resource, but it does not: %s", cmp.Diff(expected, joinedCfgs)) + } + + // Verify that the generated config is valid. + // The function panics if the config is invalid. + testModuleInline(t, map[string]string{ + "main.tf": strings.Join(generatedCfgs, "\n"), + }) +} + var ( testResourceCfg = `resource "test_resource" "test_0" { provider = test 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 67beadb889fc..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) } @@ -3575,60 +3578,114 @@ func TestContext2Validate_queryList(t *testing.T) { // Action Validation is largely exercised in context_plan_actions_test.go func TestContext2Validate_action(t *testing.T) { tests := map[string]struct { - config string - wantErr string + config string + wantErr string + expectValidateCalled bool }{ "valid config": { ` -terraform { - required_providers { - test = { - source = "hashicorp/test" - version = "1.0.0" + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + action "test_register" "foo" { + config { + host = "cmdb.snot" + } + } + resource "test_instance" "foo" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_register.foo] + } + } + } + `, + "", + true, + }, + "validly null config": { // this doesn't seem likely, but let's make sure nothing panics. + ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } } - } -} -action "test_register" "foo" { - config { - host = "cmdb.snot" - } -} -resource "test_instance" "foo" { - lifecycle { - action_trigger { - events = [after_create] - actions = [action.test_register.foo] - } - } -} -`, + action "test_other" "foo" { + } + resource "test_instance" "foo" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_other.foo] + } + } + } + `, "", + true, }, "missing required config": { ` -terraform { - required_providers { - test = { - source = "hashicorp/test" - version = "1.0.0" + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } } - } -} -action "test_register" "foo" { - config {} -} -resource "test_instance" "foo" { - lifecycle { - action_trigger { - events = [after_create] - actions = [action.test_register.foo] - } - } -} -`, - "host is null", + action "test_register" "foo" { + config {} + } + resource "test_instance" "foo" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_register.foo] + } + } + } + `, + "Missing required argument: The argument \"host\" is required, but no definition was found.", + false, }, - "invalid nil config config": { + "invalid required config (provider validation)": { + ` + terraform { + required_providers { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + } + } + action "test_register" "foo" { + config { + host = "invalid" + } + } + resource "test_instance" "foo" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_register.foo] + } + } + } + `, + "Invalid value for required argument \"host\" because I said so", + true, + }, + "invalid nil config": { ` terraform { required_providers { @@ -3649,7 +3706,8 @@ resource "test_instance" "foo" { } } `, - "config is null", + "Missing required argument: The argument \"host\" is required, but was not set.", + false, }, } @@ -3669,6 +3727,14 @@ resource "test_instance" "foo" { }, Actions: map[string]*providers.ActionSchema{ "test_register": { + ConfigSchema: &configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "host": {Type: cty.String, Required: true}, + "output": {Type: cty.String, Computed: true, Optional: true}, + }, + }, + }, + "test_other": { ConfigSchema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ "host": {Type: cty.String, Optional: true}, @@ -3678,12 +3744,8 @@ resource "test_instance" "foo" { }, }) p.ValidateActionConfigFn = func(r providers.ValidateActionConfigRequest) (resp providers.ValidateActionConfigResponse) { - if r.Config.IsNull() { - resp.Diagnostics = resp.Diagnostics.Append(errors.New("config is null")) - return - } - if r.Config.GetAttr("host").IsNull() { - resp.Diagnostics = resp.Diagnostics.Append(errors.New("host is null")) + if r.Config.GetAttr("host") == cty.StringVal("invalid") { + resp.Diagnostics = resp.Diagnostics.Append(errors.New("Invalid value for required argument \"host\" because I said so")) } return } @@ -3695,10 +3757,6 @@ resource "test_instance" "foo" { }) diags := ctx.Validate(m, nil) - if !p.ValidateActionConfigCalled { - t.Fatal("ValidateAction RPC was not called") - } - if test.wantErr != "" { if !diags.HasErrors() { t.Errorf("unexpected success\nwant errors: %s", test.wantErr) @@ -3710,6 +3768,81 @@ resource "test_instance" "foo" { t.Errorf("unexpected error\ngot: %s", diags.Err().Error()) } } + if p.ValidateActionConfigCalled != test.expectValidateCalled { + if test.expectValidateCalled { + t.Fatal("provider Validate RPC was expected, but not called") + } else { + t.Fatal("unexpected Validate RCP call") + } + } + }) + } +} + +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/evaluate.go b/internal/terraform/evaluate.go index 6a1cb5163cc5..68c9d8aba5b7 100644 --- a/internal/terraform/evaluate.go +++ b/internal/terraform/evaluate.go @@ -297,7 +297,18 @@ func (d *evaluationStateData) GetInputVariable(addr addrs.InputVariable, rng tfd return ret, diags } - val := d.Evaluator.NamedValues.GetInputVariableValue(d.ModulePath.InputVariable(addr.Name)) + var val cty.Value + if target := d.ModulePath.InputVariable(addr.Name); !d.Evaluator.NamedValues.HasInputVariableValue(target) { + val = cty.DynamicVal + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to uninitialized variable", + Detail: fmt.Sprintf("The variable %s was not processed by the most recent operation, this likely means the previous operation either failed or was incomplete due to targeting.", addr), + Subject: rng.ToHCL().Ptr(), + }) + } else { + val = d.Evaluator.NamedValues.GetInputVariableValue(target) + } // Mark if sensitive and/or ephemeral if config.Sensitive { @@ -342,11 +353,17 @@ func (d *evaluationStateData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.S return cty.DynamicVal, diags } - if target := addr.Absolute(d.ModulePath); d.Evaluator.NamedValues.HasLocalValue(target) { - return d.Evaluator.NamedValues.GetLocalValue(addr.Absolute(d.ModulePath)), diags + target := addr.Absolute(d.ModulePath) + if !d.Evaluator.NamedValues.HasLocalValue(target) { + return cty.DynamicVal, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to uninitialized local value", + Detail: fmt.Sprintf("The local value %s was not processed by the most recent operation, this likely means the previous operation either failed or was incomplete due to targeting.", addr), + Subject: rng.ToHCL().Ptr(), + }) } - return cty.DynamicVal, diags + return d.Evaluator.NamedValues.GetLocalValue(target), diags } func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) { @@ -556,7 +573,12 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc if addr.Mode == addrs.EphemeralResourceMode { unknownVal = unknownVal.Mark(marks.Ephemeral) } - return unknownVal, diags + return unknownVal, diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to uninitialized resource", + Detail: fmt.Sprintf("The resource %s was not processed by the most recent operation, this likely means the previous operation either failed or was incomplete due to targeting.", addr), + Subject: rng.ToHCL().Ptr(), + }) } if _, _, hasUnknownKeys := d.Evaluator.Instances.ResourceInstanceKeys(addr.Absolute(moduleAddr)); hasUnknownKeys { diff --git a/internal/terraform/evaluate_valid.go b/internal/terraform/evaluate_valid.go index 16315abd5da5..9ce292d25153 100644 --- a/internal/terraform/evaluate_valid.go +++ b/internal/terraform/evaluate_valid.go @@ -303,7 +303,7 @@ func staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid list resource traversal`, - Detail: fmt.Sprintf(`The first step in the traversal for a %s resource must be an attribute "data", but got %q instead.`, modeAdjective, remain[0]), + Detail: fmt.Sprintf(`The first step in the traversal for a %s resource must be an attribute "data".`, modeAdjective), Subject: rng.ToHCL().Ptr(), }) return diags @@ -315,7 +315,7 @@ func staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid list resource traversal`, - Detail: fmt.Sprintf(`The second step in the traversal for a %s resource must be an index, but got %q instead.`, modeAdjective, remain[0]), + Detail: fmt.Sprintf(`The second step in the traversal for a %s resource must be an index.`, modeAdjective), Subject: rng.ToHCL().Ptr(), }) return diags @@ -331,7 +331,7 @@ func staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid list resource traversal`, - Detail: fmt.Sprintf(`The third step in the traversal for a %s resource must be an attribute "state" or "identity", but got %q instead.`, modeAdjective, remain[0]), + Detail: fmt.Sprintf(`The third step in the traversal for a %s resource must be an attribute "state" or "identity".`, modeAdjective), Subject: rng.ToHCL().Ptr(), }) return diags @@ -340,7 +340,7 @@ func staticValidateResourceReference(modCfg *configs.Config, addr addrs.Resource diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Invalid list resource traversal`, - Detail: fmt.Sprintf(`The third step in the traversal for a %s resource must be an attribute "state" or "identity", but got %q instead.`, modeAdjective, stateOrIdent.Name), + Detail: fmt.Sprintf(`The third step in the traversal for a %s resource must be an attribute "state" or "identity".`, modeAdjective), Subject: rng.ToHCL().Ptr(), }) return diags diff --git a/internal/terraform/graph_builder_apply.go b/internal/terraform/graph_builder_apply.go index b81f1b6f2a2d..f835b82f41fb 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 @@ -156,6 +152,29 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { Config: b.Config, }, + &ActionTriggerConfigTransformer{ + Config: b.Config, + Operation: b.Operation, + ActionTargets: b.ActionTargets, + + ConcreteActionTriggerNodeFunc: func(node *nodeAbstractActionTriggerExpand, timing RelativeActionTiming) dag.Vertex { + return &nodeActionTriggerApplyExpand{ + nodeAbstractActionTriggerExpand: node, + + relativeTiming: timing, + } + }, + // we want before_* actions to run before and after_* actions to run after the resource + CreateNodesAsAfter: false, + }, + + &ActionInvokeApplyTransformer{ + Config: b.Config, + Operation: b.Operation, + ActionTargets: b.ActionTargets, + Changes: b.Changes, + }, + &ActionDiffTransformer{ Changes: b.Changes, Config: b.Config, diff --git a/internal/terraform/graph_builder_plan.go b/internal/terraform/graph_builder_plan.go index 69776cff541c..4a395ed7e0b3 100644 --- a/internal/terraform/graph_builder_plan.go +++ b/internal/terraform/graph_builder_plan.go @@ -159,24 +159,33 @@ 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, generateConfigPathForImportTargets: b.GenerateConfigPath, }, - &ActionPlanTransformer{ - Config: b.Config, - Operation: b.Operation, - Targets: b.ActionTargets, + &ActionTriggerConfigTransformer{ + Config: b.Config, + Operation: b.Operation, + ActionTargets: b.ActionTargets, + queryPlanMode: b.queryPlan, + + ConcreteActionTriggerNodeFunc: func(node *nodeAbstractActionTriggerExpand, _ RelativeActionTiming) dag.Vertex { + return &nodeActionTriggerPlanExpand{ + nodeAbstractActionTriggerExpand: node, + } + }, + + // We plan all actions after the resource is handled + CreateNodesAsAfter: true, + }, + + &ActionInvokePlanTransformer{ + Config: b.Config, + Operation: b.Operation, + ActionTargets: b.ActionTargets, + queryPlanMode: b.queryPlan, }, // Add dynamic values @@ -289,6 +298,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/node_action.go b/internal/terraform/node_action.go index 9647e754bbf3..21ec1c39d662 100644 --- a/internal/terraform/node_action.go +++ b/internal/terraform/node_action.go @@ -76,27 +76,23 @@ func (n *nodeExpandActionDeclaration) DynamicExpand(ctx EvalContext) (*Graph, tf _, knownInstKeys, haveUnknownKeys := expander.ActionInstanceKeys(absActAddr) if haveUnknownKeys { + // this should never happen, n.recordActionData explicitly sets + // allowUnknown to be false, so we should pick up diagnostics + // during that call instance reaching this branch. + panic("found unknown keys in action instance") + } + + // Expand the action instances for this module. + for _, knownInstKey := range knownInstKeys { node := NodeActionDeclarationInstance{ - Addr: absActAddr.Instance(addrs.WildcardKey), + Addr: absActAddr.Instance(knownInstKey), Config: &n.Config, Schema: n.Schema, ResolvedProvider: n.ResolvedProvider, Dependencies: n.Dependencies, } + g.Add(&node) - } else { - // Expand the action instances for this module. - for _, knownInstKey := range knownInstKeys { - node := NodeActionDeclarationInstance{ - Addr: absActAddr.Instance(knownInstKey), - Config: &n.Config, - Schema: n.Schema, - ResolvedProvider: n.ResolvedProvider, - Dependencies: n.Dependencies, - } - - g.Add(&node) - } } addRootNodeToGraph(&g) @@ -113,16 +109,15 @@ func (n *nodeExpandActionDeclaration) recordActionData(ctx EvalContext, addr add // to expand the module here to create all resources. expander := ctx.InstanceExpander() - // Allowing unknown values in count and for_each is a top-level plan option. - // - // If this is false then the codepaths that handle unknown values below - // become unreachable, because the evaluate functions will reject unknown - // values as an error. - allowUnknown := ctx.Deferrals().DeferralAllowed() + // For now, action instances cannot evaluate to unknown. When an action + // would have an unknown instance key, we'd want to defer executing that + // action, and in turn defer executing the triggering resource. Delayed + // deferrals are not currently possible (we need to reconfigure exactly how + // deferrals are checked) so for now deferred actions are simply blocked. switch { case n.Config.Count != nil: - count, countDiags := evaluateCountExpression(n.Config.Count, ctx, allowUnknown) + count, countDiags := evaluateCountExpression(n.Config.Count, ctx, false) diags = diags.Append(countDiags) if countDiags.HasErrors() { return diags @@ -131,12 +126,13 @@ func (n *nodeExpandActionDeclaration) recordActionData(ctx EvalContext, addr add if count >= 0 { expander.SetActionCount(addr.Module, n.Addr.Action, count) } else { - // -1 represents "unknown" - expander.SetActionCountUnknown(addr.Module, n.Addr.Action) + // this should not be possible as allowUnknown was set to false + // in the evaluateCountExpression function call. + panic("evaluateCountExpression returned unknown") } case n.Config.ForEach != nil: - forEach, known, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, allowUnknown) + forEach, known, forEachDiags := evaluateForEachExpression(n.Config.ForEach, ctx, false) diags = diags.Append(forEachDiags) if forEachDiags.HasErrors() { return diags @@ -147,7 +143,9 @@ func (n *nodeExpandActionDeclaration) recordActionData(ctx EvalContext, addr add if known { expander.SetActionForEach(addr.Module, n.Addr.Action, forEach) } else { - expander.SetActionForEachUnknown(addr.Module, n.Addr.Action) + // this should not be possible as allowUnknown was set to false + // in the evaluateForEachExpression function call. + panic("evaluateForEachExpression returned unknown") } default: diff --git a/internal/terraform/node_action_abstract.go b/internal/terraform/node_action_abstract.go index 2af00048d56c..3ca9a55b0d69 100644 --- a/internal/terraform/node_action_abstract.go +++ b/internal/terraform/node_action_abstract.go @@ -44,9 +44,8 @@ func (n NodeAbstractAction) Name() string { // abstract action to a concrete one of some type. type ConcreteActionNodeFunc func(*NodeAbstractAction) dag.Vertex -// I'm not sure why my ConcreteActionNodeFUnction kept being nil in tests, but -// this is much more robust. If it isn't a validate walk, we need -// nodeExpandActionDeclaration. +// DefaultConcreteActionNodeFunc is the default ConcreteActionNodeFunc used by +// everything except validate. func DefaultConcreteActionNodeFunc(a *NodeAbstractAction) dag.Vertex { return &nodeExpandActionDeclaration{ NodeAbstractAction: a, diff --git a/internal/terraform/node_action_instance.go b/internal/terraform/node_action_instance.go index f27a3d53521b..cec79da1d564 100644 --- a/internal/terraform/node_action_instance.go +++ b/internal/terraform/node_action_instance.go @@ -6,7 +6,6 @@ package terraform import ( "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/lang/langrefs" @@ -44,22 +43,8 @@ func (n *NodeActionDeclarationInstance) Path() addrs.ModuleInstance { func (n *NodeActionDeclarationInstance) Execute(ctx EvalContext, _ walkOperation) tfdiags.Diagnostics { var diags tfdiags.Diagnostics - deferrals := ctx.Deferrals() - - if n.Addr.Action.Key == addrs.WildcardKey { - if deferrals.DeferralAllowed() { - deferrals.ReportActionDeferred(n.Addr, providers.DeferredReasonInstanceCountUnknown) - } else { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Action expansion was deferred", - Detail: "Deferral is not allowed in this context", - Subject: n.Config.DeclRange.Ptr(), - }) - } - return diags - } + deferrals := ctx.Deferrals() if deferrals.DeferralAllowed() && deferrals.ShouldDeferAction(n.Dependencies) { deferrals.ReportActionDeferred(n.Addr, providers.DeferredReasonDeferredPrereq) return diags diff --git a/internal/terraform/node_action_trigger_abstract.go b/internal/terraform/node_action_trigger_abstract.go new file mode 100644 index 000000000000..60aceb955e03 --- /dev/null +++ b/internal/terraform/node_action_trigger_abstract.go @@ -0,0 +1,108 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/dag" + "github.com/hashicorp/terraform/internal/lang/langrefs" +) + +type RelativeActionTiming = string + +const ( + RelativeActionTimingBefore = "before" + RelativeActionTimingAfter = "after" +) + +// ConcreteActionTriggerNodeFunc is a callback type used to convert an +// abstract action trigger to a concrete one of some type. +type ConcreteActionTriggerNodeFunc func(*nodeAbstractActionTriggerExpand, RelativeActionTiming) dag.Vertex + +type nodeAbstractActionTriggerExpand struct { + Addr addrs.ConfigAction + resolvedProvider addrs.AbsProviderConfig + Config *configs.Action + + lifecycleActionTrigger *lifecycleActionTrigger +} + +type lifecycleActionTrigger struct { + resourceAddress addrs.ConfigResource + events []configs.ActionTriggerEvent + actionTriggerBlockIndex int + actionListIndex int + invokingSubject *hcl.Range + actionExpr hcl.Expression + conditionExpr hcl.Expression +} + +func (at *lifecycleActionTrigger) Name() string { + return fmt.Sprintf("%s.lifecycle.action_trigger[%d].actions[%d]", at.resourceAddress.String(), at.actionTriggerBlockIndex, at.actionListIndex) +} + +var ( + _ GraphNodeReferencer = (*nodeAbstractActionTriggerExpand)(nil) + _ GraphNodeProviderConsumer = (*nodeAbstractActionTriggerExpand)(nil) + _ GraphNodeModulePath = (*nodeAbstractActionTriggerExpand)(nil) +) + +func (n *nodeAbstractActionTriggerExpand) Name() string { + triggeredBy := "triggered by " + if n.lifecycleActionTrigger != nil { + triggeredBy += n.lifecycleActionTrigger.resourceAddress.String() + } else { + triggeredBy += "unknown" + } + + return fmt.Sprintf("%s %s", n.Addr.String(), triggeredBy) +} + +func (n *nodeAbstractActionTriggerExpand) ModulePath() addrs.Module { + return n.Addr.Module +} + +func (n *nodeAbstractActionTriggerExpand) References() []*addrs.Reference { + var refs []*addrs.Reference + refs = append(refs, &addrs.Reference{ + Subject: n.Addr.Action, + }) + + if n.lifecycleActionTrigger != nil { + refs = append(refs, &addrs.Reference{ + Subject: n.lifecycleActionTrigger.resourceAddress.Resource, + }) + + conditionRefs, _ := langrefs.ReferencesInExpr(addrs.ParseRef, n.lifecycleActionTrigger.conditionExpr) + refs = append(refs, conditionRefs...) + } + + return refs +} + +func (n *nodeAbstractActionTriggerExpand) ProvidedBy() (addr addrs.ProviderConfig, exact bool) { + if n.resolvedProvider.Provider.Type != "" { + return n.resolvedProvider, true + } + + // Since we always have a config, we can use it + relAddr := n.Config.ProviderConfigAddr() + return addrs.LocalProviderConfig{ + LocalName: relAddr.LocalName, + Alias: relAddr.Alias, + }, false +} + +func (n *nodeAbstractActionTriggerExpand) Provider() (provider addrs.Provider) { + return n.Config.Provider +} + +func (n *nodeAbstractActionTriggerExpand) SetProvider(config addrs.AbsProviderConfig) { + n.resolvedProvider = config +} diff --git a/internal/terraform/node_action_trigger_apply.go b/internal/terraform/node_action_trigger_apply.go index d24ba9c9de3b..2700611f5d16 100644 --- a/internal/terraform/node_action_trigger_apply.go +++ b/internal/terraform/node_action_trigger_apply.go @@ -6,248 +6,79 @@ package terraform import ( "fmt" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/lang/ephemeral" + "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/lang/langrefs" "github.com/hashicorp/terraform/internal/plans" - "github.com/hashicorp/terraform/internal/plans/objchange" - "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/tfdiags" ) -type nodeActionTriggerApply struct { - ActionInvocation *plans.ActionInvocationInstanceSrc - resolvedProvider addrs.AbsProviderConfig - ActionTriggerRange *hcl.Range - ConditionExpr hcl.Expression +type nodeActionTriggerApplyExpand struct { + *nodeAbstractActionTriggerExpand + + actionInvocationInstances []*plans.ActionInvocationInstanceSrc + relativeTiming RelativeActionTiming } var ( - _ GraphNodeExecutable = (*nodeActionTriggerApply)(nil) - _ GraphNodeReferencer = (*nodeActionTriggerApply)(nil) + _ GraphNodeDynamicExpandable = (*nodeActionTriggerApplyExpand)(nil) + _ GraphNodeReferencer = (*nodeActionTriggerApplyExpand)(nil) + _ GraphNodeProviderConsumer = (*nodeActionTriggerApplyExpand)(nil) + _ GraphNodeModulePath = (*nodeActionTriggerApplyExpand)(nil) ) -func (n *nodeActionTriggerApply) Name() string { - return n.ActionInvocation.Addr.String() + " (instance)" +func (n *nodeActionTriggerApplyExpand) Name() string { + return fmt.Sprintf("%s (apply - %s)", n.nodeAbstractActionTriggerExpand.Name(), n.relativeTiming) } -func (n *nodeActionTriggerApply) Execute(ctx EvalContext, wo walkOperation) tfdiags.Diagnostics { +func (n *nodeActionTriggerApplyExpand) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diagnostics) { + var g Graph var diags tfdiags.Diagnostics - actionInvocation := n.ActionInvocation - - if n.ConditionExpr != nil { - // We know this must be a lifecycle action, otherwise we would have no condition - at := actionInvocation.ActionTrigger.(*plans.LifecycleActionTrigger) - condition, conditionDiags := evaluateActionCondition(ctx, actionConditionContext{ - // For applying the triggering event is sufficient, if the condition could not have - // been evaluated due to in invalid mix of events we would have caught it durin planning. - events: []configs.ActionTriggerEvent{at.ActionTriggerEvent}, - conditionExpr: n.ConditionExpr, - resourceAddress: at.TriggeringResourceAddr, - }) - diags = diags.Append(conditionDiags) - if diags.HasErrors() { - return diags - } - - if !condition { - return diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Condition changed evaluation during apply", - Detail: "The condition evaluated to false during apply, but was true during planning. This may lead to unexpected behavior.", - Subject: n.ConditionExpr.Range().Ptr(), - }) - } - } - - ai := ctx.Changes().GetActionInvocation(actionInvocation.Addr, actionInvocation.ActionTrigger) - if ai == nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Action invocation not found in plan", - Detail: "Could not find action invocation for address " + actionInvocation.Addr.String(), - Subject: n.ActionTriggerRange, - }) - return diags - } - actionData, ok := ctx.Actions().GetActionInstance(ai.Addr) - if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Action instance not found", - Detail: "Could not find action instance for address " + ai.Addr.String(), - Subject: n.ActionTriggerRange, - }) - return diags - } - provider, schema, err := getProvider(ctx, actionData.ProviderAddr) - if err != nil { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Failed to get provider for %s", ai.Addr), - Detail: fmt.Sprintf("Failed to get provider: %s", err), - Subject: n.ActionTriggerRange, - }) - return diags - } - actionSchema, ok := schema.Actions[ai.Addr.Action.Action.Type] - if !ok { - // This should have been caught earlier - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: fmt.Sprintf("Action %s not found in provider schema", ai.Addr), - Detail: fmt.Sprintf("The action %s was not found in the provider schema for %s", ai.Addr.Action.Action.Type, actionData.ProviderAddr), - Subject: n.ActionTriggerRange, - }) - return diags + if n.lifecycleActionTrigger == nil { + panic("Only actions triggered by plan and apply are supported") } - configValue := actionData.ConfigValue - - // Validate that what we planned matches the action data we have. - errs := objchange.AssertObjectCompatible(actionSchema.ConfigSchema, ai.ConfigValue, ephemeral.RemoveEphemeralValues(configValue)) - for _, err := range errs { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Provider produced inconsistent final plan", - Detail: fmt.Sprintf("When expanding the plan for %s to include new values learned so far during apply, Terraform produced an invalid new value for %s.\n\nThis is a bug in Terraform, which should be reported.", - ai.Addr, tfdiags.FormatError(err)), - Subject: n.ActionTriggerRange, - }) - } - - if !configValue.IsWhollyKnown() { - return diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Action configuration unknown during apply", - Detail: fmt.Sprintf("The action %s was not fully known during apply.\n\nThis is a bug in Terraform, please report it.", ai.Addr.Action.String()), - Subject: n.ActionTriggerRange, - }) - } - - hookIdentity := HookActionIdentity{ - Addr: ai.Addr, - ActionTrigger: ai.ActionTrigger, - } - - ctx.Hook(func(h Hook) (HookAction, error) { - return h.StartAction(hookIdentity) - }) - - // We don't want to send the marks, but all marks are okay in the context - // of an action invocation. We can't reuse our ephemeral free value from - // above because we want the ephemeral values to be included. - unmarkedConfigValue, _ := configValue.UnmarkDeep() - resp := provider.InvokeAction(providers.InvokeActionRequest{ - ActionType: ai.Addr.Action.Action.Type, - PlannedActionData: unmarkedConfigValue, - ClientCapabilities: ctx.ClientCapabilities(), - }) - - respDiags := n.AddSubjectToDiagnostics(resp.Diagnostics) - diags = diags.Append(respDiags) - if respDiags.HasErrors() { - ctx.Hook(func(h Hook) (HookAction, error) { - return h.CompleteAction(hookIdentity, respDiags.Err()) - }) - return diags + invocationMap := map[*plans.ActionInvocationInstanceSrc]*nodeActionTriggerApplyInstance{} + // We already planned the action invocations, so we can just add them to the graph + for _, ai := range n.actionInvocationInstances { + node := &nodeActionTriggerApplyInstance{ + ActionInvocation: ai, + resolvedProvider: n.resolvedProvider, + ActionTriggerRange: n.lifecycleActionTrigger.invokingSubject.Ptr(), + ConditionExpr: n.lifecycleActionTrigger.conditionExpr, + } + g.Add(node) + invocationMap[ai] = node } - if resp.Events != nil { // should only occur in misconfigured tests - for event := range resp.Events { - switch ev := event.(type) { - case providers.InvokeActionEvent_Progress: - ctx.Hook(func(h Hook) (HookAction, error) { - return h.ProgressAction(hookIdentity, ev.Message) - }) - case providers.InvokeActionEvent_Completed: - // Enhance the diagnostics - diags = diags.Append(n.AddSubjectToDiagnostics(ev.Diagnostics)) - ctx.Hook(func(h Hook) (HookAction, error) { - return h.CompleteAction(hookIdentity, ev.Diagnostics.Err()) - }) - if ev.Diagnostics.HasErrors() { - return diags - } - default: - panic(fmt.Sprintf("unexpected action event type %T", ev)) - } + for _, ai := range n.actionInvocationInstances { + node := invocationMap[ai] + others := ai.FilterLaterActionInvocations(n.actionInvocationInstances) + for _, other := range others { + otherNode := invocationMap[other] + g.Connect(dag.BasicEdge(otherNode, node)) } - } else { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Provider return invalid response", - Detail: "Provider response did not include any events", - Subject: n.ActionTriggerRange, - }) } - return diags -} - -func (n *nodeActionTriggerApply) ProvidedBy() (addr addrs.ProviderConfig, exact bool) { - return n.ActionInvocation.ProviderAddr, true - + addRootNodeToGraph(&g) + return &g, diags } -func (n *nodeActionTriggerApply) Provider() (provider addrs.Provider) { - return n.ActionInvocation.ProviderAddr.Provider +func (n *nodeActionTriggerApplyExpand) SetActionInvocationInstances(instances []*plans.ActionInvocationInstanceSrc) { + n.actionInvocationInstances = instances } -func (n *nodeActionTriggerApply) SetProvider(config addrs.AbsProviderConfig) { - n.resolvedProvider = config -} - -func (n *nodeActionTriggerApply) References() []*addrs.Reference { +func (n *nodeActionTriggerApplyExpand) References() []*addrs.Reference { var refs []*addrs.Reference - refs = append(refs, &addrs.Reference{ - Subject: n.ActionInvocation.Addr.Action, + Subject: n.Addr.Action, }) - conditionRefs, refDiags := langrefs.ReferencesInExpr(addrs.ParseRef, n.ConditionExpr) - if refDiags.HasErrors() { - panic(fmt.Sprintf("error parsing references in expression: %v", refDiags)) - } - if conditionRefs != nil { + if n.lifecycleActionTrigger != nil { + conditionRefs, _ := langrefs.ReferencesInExpr(addrs.ParseRef, n.lifecycleActionTrigger.conditionExpr) refs = append(refs, conditionRefs...) } return refs } - -// GraphNodeReferencer -func (n *nodeActionTriggerApply) ModulePath() addrs.Module { - return n.ActionInvocation.Addr.Module.Module() -} - -// GraphNodeExecutable -func (n *nodeActionTriggerApply) Path() addrs.ModuleInstance { - return n.ActionInvocation.Addr.Module -} - -func (n *nodeActionTriggerApply) AddSubjectToDiagnostics(input tfdiags.Diagnostics) tfdiags.Diagnostics { - var diags tfdiags.Diagnostics - if len(input) > 0 { - severity := hcl.DiagWarning - message := "Warning when invoking action" - err := input.Warnings().ErrWithWarnings() - if input.HasErrors() { - severity = hcl.DiagError - message = "Error when invoking action" - err = input.ErrWithWarnings() - } - - diags = diags.Append(&hcl.Diagnostic{ - Severity: severity, - Summary: message, - Detail: err.Error(), - Subject: n.ActionTriggerRange, - }) - } - return diags -} diff --git a/internal/terraform/node_action_trigger_instance_apply.go b/internal/terraform/node_action_trigger_instance_apply.go new file mode 100644 index 000000000000..eb631ee4c8d3 --- /dev/null +++ b/internal/terraform/node_action_trigger_instance_apply.go @@ -0,0 +1,255 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/lang/ephemeral" + "github.com/hashicorp/terraform/internal/lang/langrefs" + "github.com/hashicorp/terraform/internal/plans" + "github.com/hashicorp/terraform/internal/plans/objchange" + "github.com/hashicorp/terraform/internal/providers" + "github.com/hashicorp/terraform/internal/tfdiags" +) + +type nodeActionTriggerApplyInstance struct { + ActionInvocation *plans.ActionInvocationInstanceSrc + resolvedProvider addrs.AbsProviderConfig + ActionTriggerRange *hcl.Range + ConditionExpr hcl.Expression +} + +var ( + _ GraphNodeExecutable = (*nodeActionTriggerApplyInstance)(nil) + _ GraphNodeReferencer = (*nodeActionTriggerApplyInstance)(nil) + _ GraphNodeProviderConsumer = (*nodeActionTriggerApplyInstance)(nil) + _ GraphNodeModulePath = (*nodeActionTriggerApplyInstance)(nil) +) + +func (n *nodeActionTriggerApplyInstance) Name() string { + return n.ActionInvocation.Addr.String() + " (instance)" +} + +func (n *nodeActionTriggerApplyInstance) Execute(ctx EvalContext, wo walkOperation) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + actionInvocation := n.ActionInvocation + + if n.ConditionExpr != nil { + // We know this must be a lifecycle action, otherwise we would have no condition + at := actionInvocation.ActionTrigger.(*plans.LifecycleActionTrigger) + condition, conditionDiags := evaluateActionCondition(ctx, actionConditionContext{ + // For applying the triggering event is sufficient, if the condition could not have + // been evaluated due to in invalid mix of events we would have caught it durin planning. + events: []configs.ActionTriggerEvent{at.ActionTriggerEvent}, + conditionExpr: n.ConditionExpr, + resourceAddress: at.TriggeringResourceAddr, + }) + diags = diags.Append(conditionDiags) + if diags.HasErrors() { + return diags + } + + if !condition { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Condition changed evaluation during apply", + Detail: "The condition evaluated to false during apply, but was true during planning. This may lead to unexpected behavior.", + Subject: n.ConditionExpr.Range().Ptr(), + }) + } + } + + ai := ctx.Changes().GetActionInvocation(actionInvocation.Addr, actionInvocation.ActionTrigger) + if ai == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Action invocation not found in plan", + Detail: "Could not find action invocation for address " + actionInvocation.Addr.String(), + Subject: n.ActionTriggerRange, + }) + return diags + } + actionData, ok := ctx.Actions().GetActionInstance(ai.Addr) + if !ok { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Action instance not found", + Detail: "Could not find action instance for address " + ai.Addr.String(), + Subject: n.ActionTriggerRange, + }) + return diags + } + provider, schema, err := getProvider(ctx, actionData.ProviderAddr) + if err != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Failed to get provider for %s", ai.Addr), + Detail: fmt.Sprintf("Failed to get provider: %s", err), + Subject: n.ActionTriggerRange, + }) + return diags + } + + actionSchema, ok := schema.Actions[ai.Addr.Action.Action.Type] + if !ok { + // This should have been caught earlier + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Action %s not found in provider schema", ai.Addr), + Detail: fmt.Sprintf("The action %s was not found in the provider schema for %s", ai.Addr.Action.Action.Type, actionData.ProviderAddr), + Subject: n.ActionTriggerRange, + }) + return diags + } + + configValue := actionData.ConfigValue + + // Validate that what we planned matches the action data we have. + errs := objchange.AssertObjectCompatible(actionSchema.ConfigSchema, ai.ConfigValue, ephemeral.RemoveEphemeralValues(configValue)) + for _, err := range errs { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Provider produced inconsistent final plan", + Detail: fmt.Sprintf("When expanding the plan for %s to include new values learned so far during apply, Terraform produced an invalid new value for %s.\n\nThis is a bug in Terraform, which should be reported.", + ai.Addr, tfdiags.FormatError(err)), + Subject: n.ActionTriggerRange, + }) + } + + if !configValue.IsWhollyKnown() { + return diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Action configuration unknown during apply", + Detail: fmt.Sprintf("The action %s was not fully known during apply.\n\nThis is a bug in Terraform, please report it.", ai.Addr.Action.String()), + Subject: n.ActionTriggerRange, + }) + } + + hookIdentity := HookActionIdentity{ + Addr: ai.Addr, + ActionTrigger: ai.ActionTrigger, + } + + ctx.Hook(func(h Hook) (HookAction, error) { + return h.StartAction(hookIdentity) + }) + + // We don't want to send the marks, but all marks are okay in the context + // of an action invocation. We can't reuse our ephemeral free value from + // above because we want the ephemeral values to be included. + unmarkedConfigValue, _ := configValue.UnmarkDeep() + resp := provider.InvokeAction(providers.InvokeActionRequest{ + ActionType: ai.Addr.Action.Action.Type, + PlannedActionData: unmarkedConfigValue, + ClientCapabilities: ctx.ClientCapabilities(), + }) + + respDiags := n.AddSubjectToDiagnostics(resp.Diagnostics) + diags = diags.Append(respDiags) + if respDiags.HasErrors() { + ctx.Hook(func(h Hook) (HookAction, error) { + return h.CompleteAction(hookIdentity, respDiags.Err()) + }) + return diags + } + + if resp.Events != nil { // should only occur in misconfigured tests + for event := range resp.Events { + switch ev := event.(type) { + case providers.InvokeActionEvent_Progress: + ctx.Hook(func(h Hook) (HookAction, error) { + return h.ProgressAction(hookIdentity, ev.Message) + }) + case providers.InvokeActionEvent_Completed: + // Enhance the diagnostics + diags = diags.Append(n.AddSubjectToDiagnostics(ev.Diagnostics)) + ctx.Hook(func(h Hook) (HookAction, error) { + return h.CompleteAction(hookIdentity, ev.Diagnostics.Err()) + }) + if ev.Diagnostics.HasErrors() { + return diags + } + default: + panic(fmt.Sprintf("unexpected action event type %T", ev)) + } + } + } else { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Provider return invalid response", + Detail: "Provider response did not include any events", + Subject: n.ActionTriggerRange, + }) + } + + return diags +} + +func (n *nodeActionTriggerApplyInstance) ProvidedBy() (addr addrs.ProviderConfig, exact bool) { + return n.ActionInvocation.ProviderAddr, true + +} + +func (n *nodeActionTriggerApplyInstance) Provider() (provider addrs.Provider) { + return n.ActionInvocation.ProviderAddr.Provider +} + +func (n *nodeActionTriggerApplyInstance) SetProvider(config addrs.AbsProviderConfig) { + n.resolvedProvider = config +} + +func (n *nodeActionTriggerApplyInstance) References() []*addrs.Reference { + var refs []*addrs.Reference + + refs = append(refs, &addrs.Reference{ + Subject: n.ActionInvocation.Addr.Action, + }) + + conditionRefs, refDiags := langrefs.ReferencesInExpr(addrs.ParseRef, n.ConditionExpr) + if refDiags.HasErrors() { + panic(fmt.Sprintf("error parsing references in expression: %v", refDiags)) + } + if conditionRefs != nil { + refs = append(refs, conditionRefs...) + } + + return refs +} + +// GraphNodeReferencer +func (n *nodeActionTriggerApplyInstance) ModulePath() addrs.Module { + return n.ActionInvocation.Addr.Module.Module() +} + +// GraphNodeExecutable +func (n *nodeActionTriggerApplyInstance) Path() addrs.ModuleInstance { + return n.ActionInvocation.Addr.Module +} + +func (n *nodeActionTriggerApplyInstance) AddSubjectToDiagnostics(input tfdiags.Diagnostics) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + if len(input) > 0 { + severity := hcl.DiagWarning + message := "Warning when invoking action" + err := input.Warnings().ErrWithWarnings() + if input.HasErrors() { + severity = hcl.DiagError + message = "Error when invoking action" + err = input.ErrWithWarnings() + } + + diags = diags.Append(&hcl.Diagnostic{ + Severity: severity, + Summary: message, + Detail: err.Error(), + Subject: n.ActionTriggerRange, + }) + } + return diags +} diff --git a/internal/terraform/node_action_trigger_instance_plan.go b/internal/terraform/node_action_trigger_instance_plan.go index 4d0dd0cb8136..a32b71f04f67 100644 --- a/internal/terraform/node_action_trigger_instance_plan.go +++ b/internal/terraform/node_action_trigger_instance_plan.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" @@ -17,7 +18,11 @@ import ( "github.com/hashicorp/terraform/internal/plans/deferring" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/tfdiags" - "github.com/zclconf/go-cty/cty" +) + +var ( + _ GraphNodeExecutable = (*nodeActionTriggerPlanInstance)(nil) + _ GraphNodeModulePath = (*nodeActionTriggerPlanInstance)(nil) ) type nodeActionTriggerPlanInstance struct { @@ -77,19 +82,20 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO } change := ctx.Changes().GetResourceInstanceChange(n.lifecycleActionTrigger.resourceAddress, n.lifecycleActionTrigger.resourceAddress.CurrentObject().DeposedKey) - // If we should defer the action invocation, we need to report it and if the resource instance - // was not deferred (and therefore was planned) we need to retroactively remove the change - if deferrals.ShouldDeferActionInvocation(ai) { + deferred, moreDiags := deferrals.ShouldDeferActionInvocation(ai, n.lifecycleActionTrigger.invokingSubject) + diags = diags.Append(moreDiags) + if deferred { deferrals.ReportActionInvocationDeferred(ai, providers.DeferredReasonDeferredPrereq) - if change != nil { - ctx.Changes().RemoveResourceInstanceChange(change.Addr, change.Addr.CurrentObject().DeposedKey) - deferrals.ReportResourceInstanceDeferred(change.Addr, providers.DeferredReasonDeferredPrereq, change) - } - return nil + return diags + } + + if moreDiags.HasErrors() { + return diags } if change == nil { - panic("change cannot be nil") + // nothing to do (this may be a refresh ) + return diags } if n.lifecycleActionTrigger == nil { @@ -150,10 +156,12 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO // We remove the marks for planning, we will record the sensitive values in the plans.ActionInvocationInstance unmarkedConfig, _ := actionInstance.ConfigValue.UnmarkDeepWithPaths() + cc := ctx.ClientCapabilities() + cc.DeferralAllowed = false // for now, deferrals in actions are always disabled resp := provider.PlanAction(providers.PlanActionRequest{ ActionType: n.actionAddress.Action.Action.Type, ProposedActionData: unmarkedConfig, - ClientCapabilities: ctx.ClientCapabilities(), + ClientCapabilities: cc, }) if len(resp.Diagnostics) > 0 { @@ -173,7 +181,9 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO Subject: n.lifecycleActionTrigger.invokingSubject, }) } - if resp.Deferred != nil && !deferrals.DeferralAllowed() { + if resp.Deferred != nil { + // we always set allow_deferrals to be false for actions, so this + // should not happen diags = diags.Append(deferring.UnexpectedProviderDeferralDiagnostic(n.actionAddress)) } if resp.Diagnostics.HasErrors() { @@ -185,16 +195,6 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO eventSpecificAi := ai.DeepCopy() // We need to set the triggering event on the action invocation eventSpecificAi.ActionTrigger = n.lifecycleActionTrigger.ActionTrigger(triggeredEvent) - - // If the action is deferred, we need to also defer the resource instance - if resp.Deferred != nil { - deferrals.ReportActionInvocationDeferred(*eventSpecificAi, resp.Deferred.Reason) - ctx.Changes().RemoveResourceInstanceChange(change.Addr, change.Addr.CurrentObject().DeposedKey) - deferrals.ReportResourceInstanceDeferred(change.Addr, providers.DeferredReasonDeferredPrereq, change) - - return diags - } - ctx.Changes().AppendActionInvocation(eventSpecificAi) } return diags diff --git a/internal/terraform/node_action_trigger_plan.go b/internal/terraform/node_action_trigger_plan.go index e9a2b9885ea2..04ff5d2d4222 100644 --- a/internal/terraform/node_action_trigger_plan.go +++ b/internal/terraform/node_action_trigger_plan.go @@ -6,52 +6,28 @@ package terraform import ( "fmt" - "github.com/hashicorp/hcl/v2" "github.com/zclconf/go-cty/cty" "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/instances" - "github.com/hashicorp/terraform/internal/lang/langrefs" "github.com/hashicorp/terraform/internal/tfdiags" ) type nodeActionTriggerPlanExpand struct { - Addr addrs.ConfigAction - resolvedProvider addrs.AbsProviderConfig - Config *configs.Action + *nodeAbstractActionTriggerExpand - lifecycleActionTrigger *lifecycleActionTrigger -} - -type lifecycleActionTrigger struct { - resourceAddress addrs.ConfigResource - events []configs.ActionTriggerEvent - actionTriggerBlockIndex int - actionListIndex int - invokingSubject *hcl.Range - actionExpr hcl.Expression - conditionExpr hcl.Expression -} - -func (at *lifecycleActionTrigger) Name() string { - return fmt.Sprintf("%s.lifecycle.action_trigger[%d].actions[%d]", at.resourceAddress.String(), at.actionTriggerBlockIndex, at.actionListIndex) + resourceTargets []addrs.Targetable } var ( _ GraphNodeDynamicExpandable = (*nodeActionTriggerPlanExpand)(nil) _ GraphNodeReferencer = (*nodeActionTriggerPlanExpand)(nil) + _ GraphNodeProviderConsumer = (*nodeActionTriggerPlanExpand)(nil) + _ GraphNodeModulePath = (*nodeActionTriggerPlanExpand)(nil) ) func (n *nodeActionTriggerPlanExpand) Name() string { - triggeredBy := "triggered by " - if n.lifecycleActionTrigger != nil { - triggeredBy += n.lifecycleActionTrigger.resourceAddress.String() - } else { - triggeredBy += "unknown" - } - - return fmt.Sprintf("%s %s", n.Addr.String(), triggeredBy) + return fmt.Sprintf("%s (plan)", n.nodeAbstractActionTriggerExpand.Name()) } func (n *nodeActionTriggerPlanExpand) DynamicExpand(ctx EvalContext) (*Graph, tfdiags.Diagnostics) { @@ -100,6 +76,24 @@ func (n *nodeActionTriggerPlanExpand) DynamicExpand(ctx EvalContext) (*Graph, tf for _, key := range keys { absResourceInstanceAddr := n.lifecycleActionTrigger.resourceAddress.Absolute(module).Instance(key) + // If the triggering resource was targeted, make sure the instance + // that triggered this was targeted specifically. + // This is necessary since the expansion of a resource instance (and of an action trigger) + // happens during the graph walk / execution, therefore the target transformer can not + // filter out individual instances, this needs to happen during the graph walk / execution. + if n.resourceTargets != nil { + matched := false + for _, resourceTarget := range n.resourceTargets { + if resourceTarget.TargetContains(absResourceInstanceAddr) { + matched = true + break + } + } + if !matched { + continue + } + } + // The n.Addr was derived from the ActionRef hcl.Expression referenced inside the resource's lifecycle block, and has not yet been // expanded or fully evaluated, so we will do that now. // Grab the instance key, necessary if the action uses [count.index] or [each.key] @@ -149,45 +143,6 @@ func (n *nodeActionTriggerPlanExpand) DynamicExpand(ctx EvalContext) (*Graph, tf return &g, diags } -func (n *nodeActionTriggerPlanExpand) ModulePath() addrs.Module { - return n.Addr.Module -} - -func (n *nodeActionTriggerPlanExpand) References() []*addrs.Reference { - var refs []*addrs.Reference - refs = append(refs, &addrs.Reference{ - Subject: n.Addr.Action, - }) - - if n.lifecycleActionTrigger != nil { - refs = append(refs, &addrs.Reference{ - Subject: n.lifecycleActionTrigger.resourceAddress.Resource, - }) - - conditionRefs, _ := langrefs.ReferencesInExpr(addrs.ParseRef, n.lifecycleActionTrigger.conditionExpr) - refs = append(refs, conditionRefs...) - } - - return refs -} - -func (n *nodeActionTriggerPlanExpand) ProvidedBy() (addr addrs.ProviderConfig, exact bool) { - if n.resolvedProvider.Provider.Type != "" { - return n.resolvedProvider, true - } - - // Since we always have a config, we can use it - relAddr := n.Config.ProviderConfigAddr() - return addrs.LocalProviderConfig{ - LocalName: relAddr.LocalName, - Alias: relAddr.Alias, - }, false -} - -func (n *nodeActionTriggerPlanExpand) Provider() (provider addrs.Provider) { - return n.Config.Provider -} - -func (n *nodeActionTriggerPlanExpand) SetProvider(config addrs.AbsProviderConfig) { - n.resolvedProvider = config +func (n *nodeActionTriggerPlanExpand) SetResourceTargets(addrs []addrs.Targetable) { + n.resourceTargets = addrs } diff --git a/internal/terraform/node_action_validate.go b/internal/terraform/node_action_validate.go index 4ad63a7254d1..8c36de6309ec 100644 --- a/internal/terraform/node_action_validate.go +++ b/internal/terraform/node_action_validate.go @@ -82,16 +82,24 @@ func (n *NodeValidatableAction) Execute(ctx EvalContext, _ walkOperation) tfdiag return diags } - var configVal cty.Value - var valDiags tfdiags.Diagnostics - if n.Config.Config != nil { - configVal, _, valDiags = ctx.EvaluateBlock(n.Config.Config, schema.ConfigSchema, nil, keyData) - diags = diags.Append(valDiags) - if valDiags.HasErrors() { + config := n.Config.Config + if n.Config.Config == nil { + config = hcl.EmptyBody() + } + + configVal, _, valDiags := ctx.EvaluateBlock(config, schema.ConfigSchema, nil, keyData) + if valDiags.HasErrors() { + // If there was no config block at all, we'll add a Context range to the returned diagnostic + if n.Config.Config == nil { + for _, diag := range valDiags.ToHCL() { + diag.Context = &n.Config.DeclRange + diags = diags.Append(diag) + } + return diags + } else { + diags = diags.Append(valDiags) return diags } - } else { - configVal = cty.NullVal(n.Schema.ConfigSchema.ImpliedType()) } // Use unmarked value for validate request diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index f9c790837aca..4dec19172fa1 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -938,6 +938,9 @@ func (n *NodePlannableResourceInstance) generateHCLListResourceDef(ctx EvalConte var listElements []genconfig.ResourceListElement + expander := ctx.InstanceExpander() + enum := expander.ResourceExpansionEnum(addr) + iter := state.ElementIterator() for iter.Next() { _, val := iter.Element() @@ -955,7 +958,7 @@ func (n *NodePlannableResourceInstance) generateHCLListResourceDef(ctx EvalConte } idVal := val.GetAttr("identity") - listElements = append(listElements, genconfig.ResourceListElement{Config: config, Identity: idVal}) + listElements = append(listElements, genconfig.ResourceListElement{Config: config, Identity: idVal, ExpansionEnum: enum}) } return genconfig.GenerateListResourceContents(addr, schema.Body, schema.Identity, providerAddr, listElements) @@ -974,10 +977,17 @@ func (n *NodePlannableResourceInstance) generateResourceConfig(ctx EvalContext, if diags.HasErrors() { return cty.DynamicVal, diags } - schema := providerSchema.SchemaForResourceAddr(n.Addr.Resource.Resource) + + // the calling node may be a list resource, in which case we still need to + // lookup the schema for the corresponding managed resource for generating + // configuration. + managedAddr := n.Addr.Resource.Resource + managedAddr.Mode = addrs.ManagedResourceMode + + schema := providerSchema.SchemaForResourceAddr(managedAddr) if schema.Body == nil { // Should be caught during validation, so we don't bother with a pretty error here - diags = diags.Append(fmt.Errorf("provider does not support resource type for %q", n.Addr)) + diags = diags.Append(fmt.Errorf("provider does not support resource type for %q", managedAddr)) return cty.DynamicVal, diags } diff --git a/internal/terraform/node_resource_plan_instance_query.go b/internal/terraform/node_resource_plan_instance_query.go index 2a517f3af8c4..c1ddf131fcb6 100644 --- a/internal/terraform/node_resource_plan_instance_query.go +++ b/internal/terraform/node_resource_plan_instance_query.go @@ -36,9 +36,15 @@ func (n *NodePlannableResourceInstance) listResourceExecute(ctx EvalContext) (di keyData = EvalDataForInstanceKey(addr.Resource.Key, forEach) } + schema := providerSchema.SchemaForListResourceType(n.Config.Type) + if schema.IsNil() { // Not possible, as the schema should have already been validated to exist + diags = diags.Append(fmt.Errorf("no schema available for %s; this is a bug in Terraform and should be reported", addr)) + return diags + } + // evaluate the list config block var configDiags tfdiags.Diagnostics - blockVal, _, configDiags := ctx.EvaluateBlock(config.Config, n.Schema.Body, nil, keyData) + blockVal, _, configDiags := ctx.EvaluateBlock(config.Config, schema.FullSchema, nil, keyData) diags = diags.Append(configDiags) if diags.HasErrors() { return diags @@ -79,6 +85,13 @@ func (n *NodePlannableResourceInstance) listResourceExecute(ctx EvalContext) (di } log.Printf("[TRACE] NodePlannableResourceInstance: Re-validating config for %s", n.Addr) + // if the config value is null, we still want to send a full object with all attributes being null + if !unmarkedBlockVal.IsNull() && unmarkedBlockVal.GetAttr("config").IsNull() { + mp := unmarkedBlockVal.AsValueMap() + mp["config"] = schema.ConfigSchema.EmptyValue() + unmarkedBlockVal = cty.ObjectVal(mp) + } + validateResp := provider.ValidateListResourceConfig( providers.ValidateListResourceConfigRequest{ TypeName: n.Config.Type, diff --git a/internal/terraform/node_resource_validate.go b/internal/terraform/node_resource_validate.go index 8d106bc16856..7dd60e24c177 100644 --- a/internal/terraform/node_resource_validate.go +++ b/internal/terraform/node_resource_validate.go @@ -471,8 +471,8 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag resp := provider.ValidateEphemeralResourceConfig(req) diags = diags.Append(resp.Diagnostics.InConfigBody(n.Config.Config, n.Addr.String())) case addrs.ListResourceMode: - schema := providerSchema.SchemaForResourceType(n.Config.Mode, n.Config.Type) - if schema.Body == nil { + schema := providerSchema.SchemaForListResourceType(n.Config.Type) + if schema.IsNil() { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: "Invalid list resource", @@ -482,7 +482,7 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag return diags } - blockVal, _, valDiags := ctx.EvaluateBlock(n.Config.Config, schema.Body, nil, keyData) + blockVal, _, valDiags := ctx.EvaluateBlock(n.Config.Config, schema.FullSchema, nil, keyData) diags = diags.Append(valDiags) if valDiags.HasErrors() { return diags @@ -502,6 +502,13 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag // Use unmarked value for validate request unmarkedBlockVal, _ := blockVal.UnmarkDeep() + + // if the config value is null, we still want to send a full object with all attributes being null + if !unmarkedBlockVal.IsNull() && unmarkedBlockVal.GetAttr("config").IsNull() { + mp := unmarkedBlockVal.AsValueMap() + mp["config"] = schema.ConfigSchema.EmptyValue() + unmarkedBlockVal = cty.ObjectVal(mp) + } req := providers.ValidateListResourceConfigRequest{ TypeName: n.Config.Type, Config: unmarkedBlockVal, diff --git a/internal/terraform/terraform_test.go b/internal/terraform/terraform_test.go index e6e9dc8d077d..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()) @@ -118,7 +118,7 @@ func testModuleInline(t testing.TB, sources map[string]string) *configs.Config { } } - loader, cleanup := configload.NewLoaderForTests(t) + loader, cleanup := configload.NewLoaderForTests(t, parserOpts...) defer cleanup() // We need to be able to exercise experimental features in our integration tests. @@ -139,7 +139,7 @@ func testModuleInline(t testing.TB, sources map[string]string) *configs.Config { t.Fatalf("failed to refresh modules after installation: %s", err) } - config, diags := loader.LoadConfig(cfgPath, configs.MatchTestFiles("tests")) + config, diags := loader.LoadConfigWithTests(cfgPath, "tests") if diags.HasErrors() { t.Fatal(diags.Error()) } diff --git a/internal/terraform/transform_action_diff.go b/internal/terraform/transform_action_diff.go index a4a1c2e481c6..6c42f6f7193b 100644 --- a/internal/terraform/transform_action_diff.go +++ b/internal/terraform/transform_action_diff.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" - "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/plans" ) @@ -21,85 +20,40 @@ type ActionDiffTransformer struct { func (t *ActionDiffTransformer) Transform(g *Graph) error { applyNodes := addrs.MakeMap[addrs.AbsResourceInstance, *NodeApplyableResourceInstance]() + actionTriggerNodes := addrs.MakeMap[addrs.ConfigResource, []*nodeActionTriggerApplyExpand]() for _, vs := range g.Vertices() { - applyableResource, ok := vs.(*NodeApplyableResourceInstance) - if !ok { - continue + if applyableResource, ok := vs.(*NodeApplyableResourceInstance); ok { + applyNodes.Put(applyableResource.Addr, applyableResource) } - applyNodes.Put(applyableResource.Addr, applyableResource) + if atn, ok := vs.(*nodeActionTriggerApplyExpand); ok { + configResource := actionTriggerNodes.Get(atn.lifecycleActionTrigger.resourceAddress) + actionTriggerNodes.Put(atn.lifecycleActionTrigger.resourceAddress, append(configResource, atn)) + } } - invocationMap := map[*plans.ActionInvocationInstanceSrc]*nodeActionTriggerApply{} - triggerMap := addrs.MakeMap[addrs.AbsResourceInstance, []*plans.ActionInvocationInstanceSrc]() - for _, action := range t.Changes.ActionInvocations { - // Add nodes for each action invocation - node := &nodeActionTriggerApply{ - ActionInvocation: action, + for _, ai := range t.Changes.ActionInvocations { + lat, ok := ai.ActionTrigger.(*plans.LifecycleActionTrigger) + if !ok { + continue } + isBefore := lat.ActionTriggerEvent == configs.BeforeCreate || lat.ActionTriggerEvent == configs.BeforeUpdate + isAfter := lat.ActionTriggerEvent == configs.AfterCreate || lat.ActionTriggerEvent == configs.AfterUpdate - // If the action invocations is triggered within the lifecycle of a resource - // we want to add information about the source location to the apply node - if at, ok := action.ActionTrigger.(*plans.LifecycleActionTrigger); ok { - moduleInstance := t.Config.DescendantForInstance(at.TriggeringResourceAddr.Module) - if moduleInstance == nil { - panic(fmt.Sprintf("Could not find module instance for resource %s in config", at.TriggeringResourceAddr.String())) - } - - resourceInstance := moduleInstance.Module.ResourceByAddr(at.TriggeringResourceAddr.Resource.Resource) - if resourceInstance == nil { - panic(fmt.Sprintf("Could not find resource instance for resource %s in config", at.TriggeringResourceAddr.String())) - } - - triggerBlock := resourceInstance.Managed.ActionTriggers[at.ActionTriggerBlockIndex] - if triggerBlock == nil { - panic(fmt.Sprintf("Could not find action trigger block %d for resource %s in config", at.ActionTriggerBlockIndex, at.TriggeringResourceAddr.String())) - } - - triggerMap.Put(at.TriggeringResourceAddr, append(triggerMap.Get(at.TriggeringResourceAddr), action)) - - act := triggerBlock.Actions[at.ActionsListIndex] - node.ActionTriggerRange = &act.Range - node.ConditionExpr = triggerBlock.Condition + atns, ok := actionTriggerNodes.GetOk(lat.TriggeringResourceAddr.ConfigResource()) + if !ok { + return fmt.Errorf("no action trigger nodes found for resource %s", lat.TriggeringResourceAddr) } + // We add the action invocations one by one + for _, atn := range atns { + beforeMatches := atn.relativeTiming == RelativeActionTimingBefore && isBefore + afterMatches := atn.relativeTiming == RelativeActionTimingAfter && isAfter - g.Add(node) - invocationMap[action] = node - - // Add edge to triggering resource - if lat, ok := action.ActionTrigger.(*plans.LifecycleActionTrigger); ok { - // Add edges for lifecycle action triggers - resourceNode, ok := applyNodes.GetOk(lat.TriggeringResourceAddr) - if !ok { - panic("Could not find resource node for lifecycle action trigger") - } - - switch lat.ActionTriggerEvent { - case configs.BeforeCreate, configs.BeforeUpdate, configs.BeforeDestroy: - g.Connect(dag.BasicEdge(resourceNode, node)) - case configs.AfterCreate, configs.AfterUpdate, configs.AfterDestroy: - g.Connect(dag.BasicEdge(node, resourceNode)) - default: - panic("Unknown event") + if (beforeMatches || afterMatches) && atn.lifecycleActionTrigger.actionTriggerBlockIndex == lat.ActionTriggerBlockIndex && atn.lifecycleActionTrigger.actionListIndex == lat.ActionsListIndex { + atn.actionInvocationInstances = append(atn.actionInvocationInstances, ai) } } } - // Find all dependencies between action invocations - for _, action := range t.Changes.ActionInvocations { - at, ok := action.ActionTrigger.(*plans.LifecycleActionTrigger) - if !ok { - // only add dependencies between lifecycle actions. invoke actions - // all act independently. - continue - } - - node := invocationMap[action] - others := action.FilterLaterActionInvocations(triggerMap.Get(at.TriggeringResourceAddr)) - for _, other := range others { - otherNode := invocationMap[other] - g.Connect(dag.BasicEdge(otherNode, node)) - } - } return nil } diff --git a/internal/terraform/transform_action_invoke_apply.go b/internal/terraform/transform_action_invoke_apply.go new file mode 100644 index 000000000000..4163d01bef7d --- /dev/null +++ b/internal/terraform/transform_action_invoke_apply.go @@ -0,0 +1,36 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" + "github.com/hashicorp/terraform/internal/plans" +) + +type ActionInvokeApplyTransformer struct { + Config *configs.Config + ActionTargets []addrs.Targetable + Operation walkOperation + Changes *plans.ChangesSrc + + queryPlanMode bool +} + +func (t *ActionInvokeApplyTransformer) Transform(g *Graph) error { + if t.Operation != walkApply || t.queryPlanMode || len(t.ActionTargets) == 0 { + return nil + } + + // We just want to add all invoke triggered action invocations + for _, action := range t.Changes.ActionInvocations { + // Add nodes for each action invocation + node := &nodeActionTriggerApplyInstance{ + ActionInvocation: action, + } + g.Add(node) + } + + return nil +} diff --git a/internal/terraform/transform_action_invoke_plan.go b/internal/terraform/transform_action_invoke_plan.go new file mode 100644 index 000000000000..66eabcb7f530 --- /dev/null +++ b/internal/terraform/transform_action_invoke_plan.go @@ -0,0 +1,57 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package terraform + +import ( + "fmt" + + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/configs" +) + +type ActionInvokePlanTransformer struct { + Config *configs.Config + ActionTargets []addrs.Targetable + Operation walkOperation + + queryPlanMode bool +} + +func (t *ActionInvokePlanTransformer) Transform(g *Graph) error { + if t.Operation != walkPlan || t.queryPlanMode || len(t.ActionTargets) == 0 { + return nil + } + + // Then we're invoking and we're just going to include the actions that + // have been specifically asked for. + for _, target := range t.ActionTargets { + var config *configs.Action + switch target := target.(type) { + case addrs.AbsAction: + module := t.Config.DescendantForInstance(target.Module) + if module != nil { + config = module.Module.Actions[target.Action.String()] + } + case addrs.AbsActionInstance: + module := t.Config.DescendantForInstance(target.Module) + if module != nil { + config = module.Module.Actions[target.Action.Action.String()] + } + default: + return fmt.Errorf("Targeted unknown action type %T", target) + } + + if config == nil { + return fmt.Errorf("action %s does not exist in the configuration", target.String()) + } + + g.Add(&nodeActionInvokeExpand{ + Target: target, + Config: config, + }) + } + + return nil + +} diff --git a/internal/terraform/transform_action_plan.go b/internal/terraform/transform_action_trigger_config.go similarity index 56% rename from internal/terraform/transform_action_plan.go rename to internal/terraform/transform_action_trigger_config.go index 752a8e9b2f35..e76da39bca8e 100644 --- a/internal/terraform/transform_action_plan.go +++ b/internal/terraform/transform_action_trigger_config.go @@ -12,55 +12,27 @@ import ( "github.com/hashicorp/terraform/internal/lang/langrefs" ) -type ActionPlanTransformer struct { - Config *configs.Config - Targets []addrs.Targetable - Operation walkOperation -} +type ActionTriggerConfigTransformer struct { + Config *configs.Config + ActionTargets []addrs.Targetable + Operation walkOperation -func (t *ActionPlanTransformer) Transform(g *Graph) error { - if t.Operation != walkPlan { - return nil - } + queryPlanMode bool - if len(t.Targets) > 0 { - // Then we're invoking and we're just going to include the actions that - // have been specifically asked for. - - for _, target := range t.Targets { - var config *configs.Action - switch target := target.(type) { - case addrs.AbsAction: - module := t.Config.DescendantForInstance(target.Module) - if module != nil { - config = module.Module.Actions[target.Action.String()] - } - case addrs.AbsActionInstance: - module := t.Config.DescendantForInstance(target.Module) - if module != nil { - config = module.Module.Actions[target.Action.Action.String()] - } - } - - if config == nil { - return fmt.Errorf("action %s does not exist in the configuration", target.String()) - } - - g.Add(&nodeActionInvokeExpand{ - Target: target, - Config: config, - }) - } + ConcreteActionTriggerNodeFunc ConcreteActionTriggerNodeFunc + CreateNodesAsAfter bool +} +func (t *ActionTriggerConfigTransformer) Transform(g *Graph) error { + // We don't want to run if we are using the query plan mode or have targets in place + if (t.Operation != walkPlan && t.Operation != walkApply) || t.queryPlanMode || len(t.ActionTargets) > 0 { return nil } - // otherwise, add all the action triggers from the config. - return t.transform(g, t.Config) } -func (t *ActionPlanTransformer) transform(g *Graph, config *configs.Config) error { +func (t *ActionTriggerConfigTransformer) transform(g *Graph, config *configs.Config) error { // Add our resources if err := t.transformSingle(g, config); err != nil { return err @@ -76,7 +48,7 @@ func (t *ActionPlanTransformer) transform(g *Graph, config *configs.Config) erro return nil } -func (t *ActionPlanTransformer) transformSingle(g *Graph, config *configs.Config) error { +func (t *ActionTriggerConfigTransformer) transformSingle(g *Graph, config *configs.Config) error { actionConfigs := addrs.MakeMap[addrs.ConfigAction, *configs.Action]() for _, a := range config.Module.Actions { actionConfigs.Put(a.Addr().InModule(config.Path), a) @@ -100,8 +72,19 @@ func (t *ActionPlanTransformer) transformSingle(g *Graph, config *configs.Config } for _, r := range config.Module.ManagedResources { - priorNodes := []*nodeActionTriggerPlanExpand{} + priorBeforeNodes := []dag.Vertex{} + priorAfterNodes := []dag.Vertex{} for i, at := range r.Managed.ActionTriggers { + containsBeforeEvent := false + containsAfterEvent := false + for _, event := range at.Events { + if event == configs.BeforeCreate || event == configs.BeforeUpdate { + containsBeforeEvent = true + } else if event == configs.AfterCreate || event == configs.AfterUpdate { + containsAfterEvent = true + } + } + for j, action := range at.Actions { refs, parseRefDiags := langrefs.ReferencesInExpr(addrs.ParseRef, action.Expr) if parseRefDiags != nil { @@ -136,7 +119,7 @@ func (t *ActionPlanTransformer) transformSingle(g *Graph, config *configs.Config panic(fmt.Sprintf("Could not find node for %s", resourceAddr)) } - nat := &nodeActionTriggerPlanExpand{ + abstract := &nodeAbstractActionTriggerExpand{ Addr: configAction, Config: actionConfig, lifecycleActionTrigger: &lifecycleActionTrigger{ @@ -150,18 +133,39 @@ func (t *ActionPlanTransformer) transformSingle(g *Graph, config *configs.Config }, } - g.Add(nat) + // If CreateNodesAsAfter is set we want all nodes to run after the resource + // If not we want expansion nodes only to exist if they are being used + if !t.CreateNodesAsAfter && containsBeforeEvent { + nat := t.ConcreteActionTriggerNodeFunc(abstract, RelativeActionTimingBefore) + g.Add(nat) + + // We want to run before the resource nodes + for _, node := range resourceNode { + g.Connect(dag.BasicEdge(node, nat)) + } - // We always want to plan after the resource is done planning - for _, node := range resourceNode { - g.Connect(dag.BasicEdge(nat, node)) + // We want to run after all prior nodes + for _, priorNode := range priorBeforeNodes { + g.Connect(dag.BasicEdge(nat, priorNode)) + } + priorBeforeNodes = append(priorBeforeNodes, nat) } - // We want to plan after all prior nodes - for _, priorNode := range priorNodes { - g.Connect(dag.BasicEdge(nat, priorNode)) + if t.CreateNodesAsAfter || containsAfterEvent { + nat := t.ConcreteActionTriggerNodeFunc(abstract, RelativeActionTimingAfter) + g.Add(nat) + + // We want to run after the resource nodes + for _, node := range resourceNode { + g.Connect(dag.BasicEdge(nat, node)) + } + + // We want to run after all prior nodes + for _, priorNode := range priorAfterNodes { + g.Connect(dag.BasicEdge(nat, priorNode)) + } + priorAfterNodes = append(priorAfterNodes, nat) } - priorNodes = append(priorNodes, nat) } } } 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 +} diff --git a/internal/terraform/transform_targets.go b/internal/terraform/transform_targets.go index f35332bcc434..b299e57ce9cb 100644 --- a/internal/terraform/transform_targets.go +++ b/internal/terraform/transform_targets.go @@ -63,7 +63,10 @@ func (t *TargetsTransformer) selectTargetedNodes(g *Graph, addrs []addrs.Targeta for _, v := range vertices { if t.nodeIsTarget(v, addrs) { - targetedNodes.Add(v) + // We need to add everything this node depends on or that is closely associated with + // this node. In case of resource nodes, action triggers are considered closely related + // since they belong to the resource. + t.addVertexDependenciesToTargetedNodes(g, v, targetedNodes, addrs) // We inform nodes that ask about the list of targets - helps for nodes // that need to dynamically expand. Note that this only occurs for nodes @@ -72,8 +75,13 @@ func (t *TargetsTransformer) selectTargetedNodes(g *Graph, addrs []addrs.Targeta tn.SetTargets(addrs) } - for _, d := range g.Ancestors(v) { - targetedNodes.Add(d) + if _, ok := v.(*nodeExpandPlannableResource); ok { + // We want to also set the resource instance triggers on the related action triggers + for _, d := range g.UpEdges(v) { + if actionTrigger, ok := d.(*nodeActionTriggerPlanExpand); ok { + actionTrigger.SetResourceTargets(addrs) + } + } } } } @@ -155,7 +163,7 @@ func (t *TargetsTransformer) nodeIsTarget(v dag.Vertex, targets []addrs.Targetab vertexAddr = r.ResourceAddr() case *nodeActionInvokeExpand: vertexAddr = r.Target - case *nodeActionTriggerApply: + case *nodeActionTriggerApplyInstance: vertexAddr = r.ActionInvocation.Addr default: @@ -188,3 +196,39 @@ func (t *TargetsTransformer) nodeIsTarget(v dag.Vertex, targets []addrs.Targetab return false } + +// addVertexDependenciesToTargetedNodes adds dependencies of the targeted vertex to the +// targetedNodes set. This includes all ancestors in the graph. +// It also includes all action trigger nodes in the graph. Actions are planned after the +// triggering node has planned so that we can ensure the actions are only planned if the triggering +// resource has an action (Create / Update) corresponding to one of the events in the action trigger +// blocks event list. +func (t *TargetsTransformer) addVertexDependenciesToTargetedNodes(g *Graph, v dag.Vertex, targetedNodes dag.Set, addrs []addrs.Targetable) { + if targetedNodes.Include(v) { + return + } + targetedNodes.Add(v) + + for _, d := range g.Ancestors(v) { + t.addVertexDependenciesToTargetedNodes(g, d, targetedNodes, addrs) + } + + if _, ok := v.(*nodeExpandPlannableResource); ok { + // We want to also add the action triggers related to this resource + for _, d := range g.UpEdges(v) { + if _, ok := d.(*nodeActionTriggerPlanExpand); ok { + t.addVertexDependenciesToTargetedNodes(g, d, targetedNodes, addrs) + } + } + } + + // An applyable resources might have an associated after_* triggered action. + // We need to add that action to the targeted nodes as well, together with all its dependencies. + if _, ok := v.(*nodeExpandApplyableResource); ok { + for _, f := range g.UpEdges(v) { + if _, ok := f.(*nodeActionTriggerApplyExpand); ok { + t.addVertexDependenciesToTargetedNodes(g, f, targetedNodes, addrs) + } + } + } +} diff --git a/internal/tfdiags/object.go b/internal/tfdiags/object.go index 56da886c67e7..8fb115673e3d 100644 --- a/internal/tfdiags/object.go +++ b/internal/tfdiags/object.go @@ -4,6 +4,7 @@ package tfdiags import ( "fmt" + "slices" "strings" "github.com/zclconf/go-cty/cty" @@ -23,29 +24,39 @@ func ObjectToString(obj cty.Value) string { return "" } - if obj.Type().IsObjectType() { - result := "" - it := obj.ElementIterator() - for it.Next() { - key, val := it.Element() - keyStr := key.AsString() + if !obj.Type().IsObjectType() { + panic("not an object") + } - if result != "" { - result += "," - } + it := obj.ElementIterator() + keys := make([]string, 0, obj.LengthInt()) + objMap := make(map[string]cty.Value) + result := "" + // store the keys for the object, and sort them + // before appending to the result so that the final value is deterministic. + for it.Next() { + key, val := it.Element() + keyStr := key.AsString() + keys = append(keys, keyStr) + objMap[keyStr] = val + } - if val.IsNull() { - result += fmt.Sprintf("%s=", keyStr) - continue - } + slices.Sort(keys) + for _, key := range keys { + val := objMap[key] + if result != "" { + result += "," + } - result += fmt.Sprintf("%s=%s", keyStr, ValueToString(val)) + if val.IsNull() { + result += fmt.Sprintf("%s=", key) + continue } - return result + result += fmt.Sprintf("%s=%s", key, ValueToString(val)) } - panic("not an object") + return result } func ValueToString(val cty.Value) string {