diff --git a/internal/stacks/stackruntime/internal/stackeval/component_config.go b/internal/stacks/stackruntime/internal/stackeval/component_config.go index e14f54fb14..ba0f89623b 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_config.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_config.go @@ -151,11 +151,70 @@ func (c *ComponentConfig) CheckModuleTree(ctx context.Context) (*configs.Config, return nil, diags } + // We also have a small selection of additional static validation + // rules that apply only to modules used within stack components. + diags = diags.Append(c.validateModuleTreeForStacks(configRoot)) + return configRoot, diags }, ) } +// validateModuleTreeForStacks imposes some additional validation constraints +// on a module tree after it's been loaded by the main configuration packages. +// +// These rules deal with a small number of exceptions where the modules language +// as used by stacks is a subset of the modules language from traditional +// Terraform. Not all such exceptions are handled in this way because +// some of them cannot be handled statically, but this is a reasonable place +// to handle the simpler concerns and allows us to return error messages that +// talk specifically about stacks, which would be harder to achieve if these +// exceptions were made at a different layer. +func (c *ComponentConfig) validateModuleTreeForStacks(startNode *configs.Config) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + diags = diags.Append(c.validateModuleForStacks(startNode.Path, startNode.Module)) + for _, childNode := range startNode.Children { + diags = diags.Append(c.validateModuleTreeForStacks(childNode)) + } + return diags +} + +func (c *ComponentConfig) validateModuleForStacks(moduleAddr addrs.Module, module *configs.Module) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + // Inline provider configurations are not allowed when running under stacks, + // because provider configurations live in the stack configuration and + // then get passed in to the modules as special arguments. + for _, pc := range module.ProviderConfigs { + // We use some slightly different language for the topmost module + // that's being directly called from the stack configuration, because + // we can give some direct advice for how to correct the problem there, + // whereas for a nested module we assume that it's a third-party module + // written for much older versions of Terraform before we deprecated + // inline provider configurations and thus the solution is most likely + // to be selecting a different module that is Stacks-compatible, because + // removing a legacy inline provider configuration from a shared module + // would be a breaking change to that module. + if moduleAddr.IsRoot() { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Inline provider configuration not allowed", + Detail: "A module used as a stack component must have all of its provider configurations passed from the stack configuration, using the \"providers\" argument within the component configuration block.", + Subject: pc.DeclRange.Ptr(), + }) + } else { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Inline provider configuration not allowed", + Detail: "This module is not compatible with Terraform Stacks, because it declares an inline provider configuration.\n\nTo be used with stacks, this module must instead accept provider configurations from its caller.", + Subject: pc.DeclRange.Ptr(), + }) + } + } + + return diags +} + // InputsType returns an object type that the object representing the caller's // values for this component's input variables must conform to. func (c *ComponentConfig) InputsType(ctx context.Context) (cty.Type, *typeexpr.Defaults) { diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/terraform-sources.json b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/terraform-sources.json index 1ebd4e45ae..ce81f49255 100644 --- a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/terraform-sources.json +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/terraform-sources.json @@ -24,6 +24,10 @@ { "source": "https://testing.invalid/planning.tar.gz", "local": "planning" + }, + { + "source": "https://testing.invalid/validating.tar.gz", + "local": "validating" } ] } diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf new file mode 100644 index 0000000000..22a95a5f5f --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf @@ -0,0 +1,20 @@ +terraform { + required_providers { + test = { + source = "terraform.io/builtin/test" + } + } +} + +provider "test" { + arg = "foo" +} + +module "b" { + # FIXME: The following is an absolute remote address only because at the + # time of writing this test the stacks runtime's module loader can't deal + # with relative paths in this location. + source = "https://testing.invalid/validating.tar.gz//modules_with_provider_configs/module-b" + # Once that's been fixed, this should instead be: + # source = "../module-b" +} diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf new file mode 100644 index 0000000000..260de54648 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf @@ -0,0 +1,11 @@ +terraform { + required_providers { + test = { + source = "terraform.io/builtin/test" + } + } +} + +provider "test" { + arg = "foo" +} diff --git a/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/modules-with-provider-configs.tfstack.hcl b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/modules-with-provider-configs.tfstack.hcl new file mode 100644 index 0000000000..3afa6087e5 --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/testdata/sourcebundle/validating/modules_with_provider_configs/modules-with-provider-configs.tfstack.hcl @@ -0,0 +1,3 @@ +component "a" { + source = "./module-a" +} diff --git a/internal/stacks/stackruntime/internal/stackeval/validating_test.go b/internal/stacks/stackruntime/internal/stackeval/validating_test.go new file mode 100644 index 0000000000..f007e1e3ec --- /dev/null +++ b/internal/stacks/stackruntime/internal/stackeval/validating_test.go @@ -0,0 +1,86 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package stackeval + +import ( + "context" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" + + "github.com/hashicorp/terraform/internal/tfdiags" +) + +func TestValidate_modulesWithProviderConfigs(t *testing.T) { + // This test checks that we're correctly prohibiting inline provider + // configurations in Terraform modules used as stack components, which + // is forbidden because the stacks language is responsible for provider + // configurations. + // + // The underlying modules runtime isn't configured with any ability to + // instantiate provider plugins itself, so failing to prohibit this + // at the stacks language layer would just cause a lower-quality and + // more confusing error message to be emited by the modules runtime. + + cfg := testStackConfig(t, "validating", "modules_with_provider_configs") + main := NewForValidating(cfg, ValidateOpts{}) + + inPromisingTask(t, func(ctx context.Context, t *testing.T) { + diags := main.ValidateAll(ctx) + if !diags.HasErrors() { + t.Fatalf("succeeded; want errors") + } + diags.Sort() + + // We'll use the ForRPC method just as a convenient way to discard + // the specific diagnostic object types, so that we can compare + // the objects without worrying about exactly which diagnostic + // implementation each is using. + gotDiags := diags.ForRPC() + + var wantDiags tfdiags.Diagnostics + // TEMP: Because we're currently essentially just tricking the module + // loader into reading from a source bundle without actually knowing + // that's what its doing, these diagnostics show local filesystem + // paths instead of source addresses. Once we fix that in future, + // the following wanted diagnostics should switch to refer to + // source addresses starting with: + // https://testing.invalid/validating.tar.gz//modules_with_provider_configs/ + // ...which is the fake source address form used by the testStackConfig + // helper we used above. + + // Configurations in the root module get a different detail message + // than those in descendent modules, because for descendents we don't + // assume that the author is empowered to make the module + // stacks-compatible, while for the root it's more likely to be + // directly intended for stacks use, at least for now while things are + // relatively early. (We could revisit this tradeoff later.) + wantDiags = wantDiags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Inline provider configuration not allowed", + Detail: `A module used as a stack component must have all of its provider configurations passed from the stack configuration, using the "providers" argument within the component configuration block.`, + Subject: &hcl.Range{ + Filename: "testdata/sourcebundle/validating/modules_with_provider_configs/module-a/modules-with-provider-configs-a.tf", + Start: hcl.Pos{Line: 9, Column: 1, Byte: 104}, + End: hcl.Pos{Line: 9, Column: 16, Byte: 119}, + }, + }) + wantDiags = wantDiags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Inline provider configuration not allowed", + Detail: "This module is not compatible with Terraform Stacks, because it declares an inline provider configuration.\n\nTo be used with stacks, this module must instead accept provider configurations from its caller.", + Subject: &hcl.Range{ + Filename: "testdata/sourcebundle/validating/modules_with_provider_configs/module-b/modules-with-provider-configs-b.tf", + Start: hcl.Pos{Line: 9, Column: 1, Byte: 104}, + End: hcl.Pos{Line: 9, Column: 16, Byte: 119}, + }, + }) + wantDiags = wantDiags.ForRPC() + + if diff := cmp.Diff(wantDiags, gotDiags); diff != "" { + t.Errorf("wrong diagnostics\n%s", diff) + } + }) +}