Add config parsing and validation for PSS (#37178)

* Enable parsing of  blocks in individual files

Co-authored-by: Radek Simko <radeksimko@users.noreply.github.com>

* Enable handling of state stores when parsing a module from its constituent files.

This includes: validations of duplicates and clashes, supporting override files.

Co-authored-by: Radek Simko <radeksimko@users.noreply.github.com>

* Add tests for validation that detects conflicts between state_store blocks and others

* Add tests for state_store override behavior

* Add tests for validation that detects when conflicting state-related blocks are used, either in the same file or across separate files

* Update error message summaries to explicitly say blocks conflict

* Add small changes to assertions in state_store override tests

* Update state_store block parsing to expect scoped provider block

* Update tests following syntax change

* Make config parsing experimental

* Remove testModuleFromDirWithExperiment, as testModuleFromDirWithExperiments exists!

* Update code comment

---------

Co-authored-by: Radek Simko <radeksimko@users.noreply.github.com>
Co-authored-by: Radek Simko <radek.simko@gmail.com>
pull/37227/head
Sarah French 12 months ago committed by GitHub
parent e53a24ae82
commit b3d7dae793
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -3104,6 +3104,36 @@ func TestInit_testsWithModule(t *testing.T) {
}
}
func TestInit_stateStoreBlockIsExperimental(t *testing.T) {
// Create a temporary working directory that is empty
td := t.TempDir()
testCopyDir(t, testFixturePath("init-with-state-store"), td)
defer testChdir(t, td)()
ui := new(cli.MockUi)
view, done := testView(t)
c := &InitCommand{
Meta: Meta{
Ui: ui,
View: view,
AllowExperimentalFeatures: false,
},
}
args := []string{}
code := c.Run(args)
testOutput := done(t)
if code != 1 {
t.Fatalf("unexpected output: \n%s", testOutput.All())
}
// Check output
output := testOutput.Stderr()
if !strings.Contains(output, `Blocks of type "state_store" are not expected here`) {
t.Fatalf("doesn't look like experiment is blocking access': %s", output)
}
}
// newMockProviderSource is a helper to succinctly construct a mock provider
// source that contains a set of packages matching the given provider versions
// that are available for installation (from temporary local files).

@ -0,0 +1,5 @@
terraform {
state_store "foo_foo" {
}
}

@ -34,6 +34,7 @@ type Module struct {
ActiveExperiments experiments.Set
Backend *Backend
StateStore *StateStore
CloudConfig *CloudConfig
ProviderConfigs map[string]*Provider
ProviderRequirements *RequiredProviders
@ -78,6 +79,7 @@ type File struct {
ActiveExperiments experiments.Set
Backends []*Backend
StateStores []*StateStore
CloudConfigs []*CloudConfig
ProviderConfigs []*Provider
ProviderMetas []*ProviderMeta
@ -222,15 +224,30 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
if m.Backend != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Duplicate backend configuration",
Detail: fmt.Sprintf("A module may have only one backend configuration. The backend was previously configured at %s.", m.Backend.DeclRange),
Summary: "Duplicate 'backend' configuration block",
Detail: fmt.Sprintf("A module may have only one 'backend' configuration block. The backend was previously configured at %s.", m.Backend.DeclRange),
Subject: &b.DeclRange,
})
continue
}
m.Backend = b
}
for _, ss := range file.StateStores {
if m.StateStore != nil {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Duplicate 'state_store' configuration block",
Detail: fmt.Sprintf("A module may have only one 'state_store' configuration block. The state store was previously configured at %s.", m.StateStore.DeclRange),
Subject: &ss.DeclRange,
})
continue
}
m.StateStore = ss
}
for _, c := range file.CloudConfigs {
if m.CloudConfig != nil {
diags = append(diags, &hcl.Diagnostic{
@ -245,12 +262,43 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics {
m.CloudConfig = c
}
if m.Backend != nil && m.CloudConfig != nil {
// Handle conflicting blocks of different types
switch {
case m.CloudConfig != nil && m.StateStore != nil && m.Backend != nil:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Both a backend and cloud configuration are present",
Detail: fmt.Sprintf("A module may declare either one 'cloud' block OR one 'backend' block configuring a state backend. The 'cloud' block is configured at %s; a backend is configured at %s. Remove the backend block to configure HCP Terraform or Terraform Enteprise.", m.CloudConfig.DeclRange, m.Backend.DeclRange),
Subject: &m.Backend.DeclRange,
Summary: "Only one of 'cloud', 'state_store', or 'backend' configuration blocks are allowed",
Detail: fmt.Sprintf("A module may only declare one 'cloud', 'state_store' OR 'backend' block when configuring state storage. "+
"The 'cloud' block is configured at %s; a 'state_store' is configured at %s; a 'backend' is configured at %s. Remove two of these blocks.",
m.CloudConfig.DeclRange, m.StateStore.DeclRange, m.Backend.DeclRange),
Subject: &m.Backend.DeclRange,
})
case m.CloudConfig != nil && m.Backend != nil:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Conflicting 'cloud' and 'backend' configuration blocks are present",
Detail: fmt.Sprintf("A module may declare either one 'cloud' block OR one 'backend' block for configuring state storage. "+
"The 'cloud' block is configured at %s; a 'backend' is configured at %s. Remove the 'backend' block to configure HCP Terraform or Terraform Enterprise.",
m.CloudConfig.DeclRange, m.Backend.DeclRange),
Subject: &m.Backend.DeclRange,
})
case m.CloudConfig != nil && m.StateStore != nil:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Conflicting 'cloud' and 'state_store' configuration blocks are present",
Detail: fmt.Sprintf("A module may declare either one 'cloud' block OR one 'state_store' block for configuring state storage. "+
"A 'cloud' block is configured at %s; a 'state_store' is configured at %s. Remove the 'state_store' block to configure HCP Terraform or Terraform Enterprise.",
m.CloudConfig.DeclRange, m.StateStore.DeclRange),
Subject: &m.StateStore.DeclRange,
})
case m.StateStore != nil && m.Backend != nil:
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Conflicting 'state_store' and 'backend' configuration blocks are present",
Detail: fmt.Sprintf("A module may declare either one 'state_store' block OR one 'backend' block when configuring state storage. "+
"A 'state_store' block is configured at %s; a 'backend' is configured at %s. Remove one of these blocks.",
m.StateStore.DeclRange, m.Backend.DeclRange),
Subject: &m.Backend.DeclRange,
})
}
@ -613,8 +661,11 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
if len(file.Backends) != 0 {
switch len(file.Backends) {
case 1:
m.CloudConfig = nil // A backend block is mutually exclusive with a cloud one, and overwrites any cloud config
// The cloud, state_store, and backend blocks are mutually exclusive
// By setting a backend we must unset others
m.Backend = file.Backends[0]
m.CloudConfig = nil
m.StateStore = nil
default:
// An override file with multiple backends is still invalid, even
// though it can override backends from _other_ files.
@ -627,11 +678,34 @@ func (m *Module) mergeFile(file *File) hcl.Diagnostics {
}
}
if len(file.StateStores) != 0 {
switch len(file.StateStores) {
case 1:
// The cloud, state_store, and backend blocks are mutually exclusive
// By setting a state_store we must unset others
m.Backend = nil
m.CloudConfig = nil
m.StateStore = file.StateStores[0]
default:
// An override file with multiple state storages is still invalid, even
// though it can override state storages from _other_ files.
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Duplicate state storage configuration",
Detail: fmt.Sprintf("Each override file may have only one state storage configuration. A state storage was previously configured at %s.", file.StateStores[0].DeclRange),
Subject: &file.StateStores[1].DeclRange,
})
}
}
if len(file.CloudConfigs) != 0 {
switch len(file.CloudConfigs) {
case 1:
m.Backend = nil // A cloud block is mutually exclusive with a backend one, and overwrites any backend
// The cloud, state_store, and backend blocks are mutually exclusive
// By setting a cloud block we must unset others
m.Backend = nil
m.CloudConfig = file.CloudConfigs[0]
m.StateStore = nil
default:
// An override file with multiple cloud blocks is still invalid, even
// though it can override cloud/backend blocks from _other_ files.

@ -7,6 +7,7 @@ import (
"strings"
"testing"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/terraform/internal/addrs"
"github.com/zclconf/go-cty/cty"
)
@ -466,7 +467,7 @@ func TestModule_backend_multiple(t *testing.T) {
t.Fatal("module should have error diags, but does not")
}
want := `Duplicate backend configuration`
want := `Duplicate 'backend' configuration block`
if got := diags.Error(); !strings.Contains(got, want) {
t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got)
}
@ -488,28 +489,188 @@ func TestModule_cloud_multiple(t *testing.T) {
})
}
// Cannot combine use of backend, cloud blocks.
func TestModule_conflicting_backend_cloud(t *testing.T) {
// Cannot combine use of backend, cloud, state_store blocks.
func TestModule_conflicting_backend_cloud_stateStore(t *testing.T) {
testCases := map[string]struct {
dir string
wantMsg string
allowExperiments bool
}{
"cloud backends conflict": {
// detects when both cloud and backend blocks are in the same terraform block
dir: "testdata/invalid-modules/conflict-cloud-backend",
wantMsg: `Conflicting 'cloud' and 'backend' configuration blocks are present`,
},
"cloud backends conflict separate": {
// it detects when both cloud and backend blocks are present in the same module in separate files
dir: "testdata/invalid-modules/conflict-cloud-backend-separate-files",
wantMsg: `Conflicting 'cloud' and 'backend' configuration blocks are present`,
},
"cloud state store conflict": {
// detects when both cloud and state_store blocks are in the same terraform block
dir: "testdata/invalid-modules/conflict-cloud-statestore",
wantMsg: `Conflicting 'cloud' and 'state_store' configuration blocks are present`,
allowExperiments: true,
},
"cloud state store conflict separate": {
// it detects when both cloud and state_store blocks are present in the same module in separate files
dir: "testdata/invalid-modules/conflict-cloud-statestore-separate-files",
wantMsg: `Conflicting 'cloud' and 'state_store' configuration blocks are present`,
allowExperiments: true,
},
"state store backend conflict": {
// it detects when both state_store and backend blocks are in the same terraform block
dir: "testdata/invalid-modules/conflict-statestore-backend",
wantMsg: `Conflicting 'state_store' and 'backend' configuration blocks are present`,
allowExperiments: true,
},
"state store backend conflict separate": {
// it detects when both state_store and backend blocks are present in the same module in separate files
dir: "testdata/invalid-modules/conflict-statestore-backend-separate-files",
wantMsg: `Conflicting 'state_store' and 'backend' configuration blocks are present`,
allowExperiments: true,
},
"cloud backend state store conflict": {
// it detects all 3 of cloud, state_storage and backend blocks are in the same terraform block
dir: "testdata/invalid-modules/conflict-cloud-backend-statestore",
wantMsg: `Only one of 'cloud', 'state_store', or 'backend' configuration blocks are allowed`,
allowExperiments: true,
},
}
for _, tc := range testCases {
t.Run(tc.dir, func(t *testing.T) {
var diags hcl.Diagnostics
if tc.allowExperiments {
_, diags = testModuleFromDirWithExperiments(tc.dir)
} else {
_, diags = testModuleFromDir(tc.dir)
}
if !diags.HasErrors() {
t.Fatal("module should have error diags, but does not")
}
if got := diags.Error(); !strings.Contains(got, tc.wantMsg) {
t.Fatalf("expected error to contain %q\nerror was:\n%s", tc.wantMsg, got)
}
})
}
}
t.Run("it detects when both cloud and backend blocks are in the same terraform block", func(t *testing.T) {
_, diags := testModuleFromDir("testdata/invalid-modules/conflict-cloud-backend")
if !diags.HasErrors() {
t.Fatal("module should have error diags, but does not")
func TestModule_stateStore_overrides_stateStore(t *testing.T) {
t.Run("it can override a state_store block with a different state_store block", func(t *testing.T) {
mod, diags := testModuleFromDir("testdata/valid-modules/override-state-store")
if diags.HasErrors() {
t.Fatal(diags.Error())
}
want := `Both a backend and cloud configuration are present`
if got := diags.Error(); !strings.Contains(got, want) {
t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got)
if mod.StateStore == nil {
t.Fatal("expected parsed module to include a state store, found none")
}
// Check type override
gotType := mod.StateStore.Type
wantType := "foo_override"
if gotType != wantType {
t.Errorf("wrong result for state_store type: got %#v, want %#v\n", gotType, wantType)
}
// Check custom attribute override
attrs, _ := mod.StateStore.Config.JustAttributes()
gotAttr, diags := attrs["custom_attr"].Expr.Value(nil)
if diags.HasErrors() {
t.Fatal(diags.Error())
}
wantAttr := cty.StringVal("override")
if !gotAttr.RawEquals(wantAttr) {
t.Errorf("wrong result for state_store 'custom_attr': got %#v, want %#v\n", gotAttr, wantAttr)
}
// Check provider reference override
wantLocalName := "bar"
if mod.StateStore.Provider.Name != wantLocalName {
t.Errorf("wrong result for state_store 'provider' value's local name: got %#v, want %#v\n", mod.StateStore.Provider.Name, wantLocalName)
}
})
}
// Unlike most other overrides, state_store blocks do not require a base configuration in a primary
// configuration file, as an omitted backend there implies the local backend.
func TestModule_stateStore_override_no_base(t *testing.T) {
t.Run("it can introduce a state_store block via overrides when the base config has has no cloud, backend, or state_store blocks", func(t *testing.T) {
mod, diags := testModuleFromDir("testdata/valid-modules/override-state-store-no-base")
if diags.HasErrors() {
t.Fatal(diags.Error())
}
if mod.StateStore == nil {
t.Errorf("expected module StateStore not to be nil")
}
})
}
func TestModule_stateStore_overrides_backend(t *testing.T) {
t.Run("it can override a backend block with a state_store block", func(t *testing.T) {
mod, diags := testModuleFromDir("testdata/valid-modules/override-backend-with-state-store")
if diags.HasErrors() {
t.Fatal(diags.Error())
}
// Backend not set
if mod.Backend != nil {
t.Errorf("backend should not be set: got %#v\n", mod.Backend)
}
// Check state_store
if mod.StateStore == nil {
t.Fatal("expected parsed module to include a state store, found none")
}
gotType := mod.StateStore.Type
wantType := "foo_override"
if gotType != wantType {
t.Errorf("wrong result for state_store type: got %#v, want %#v\n", gotType, wantType)
}
// Not necessary to assert all values in state_store
})
}
func TestModule_stateStore_overrides_cloud(t *testing.T) {
t.Run("it can override a cloud block with a state_store block", func(t *testing.T) {
mod, diags := testModuleFromDir("testdata/valid-modules/override-cloud-with-state-store")
if diags.HasErrors() {
t.Fatal(diags.Error())
}
// CloudConfig not set
if mod.CloudConfig != nil {
t.Errorf("backend should not be set: got %#v\n", mod.Backend)
}
// Check state_store
if mod.StateStore == nil {
t.Fatal("expected parsed module to include a state store, found none")
}
gotType := mod.StateStore.Type
wantType := "foo_override"
if gotType != wantType {
t.Errorf("wrong result for state_store type: got %#v, want %#v\n", gotType, wantType)
}
// Not necessary to assert all values in state_store
})
}
func TestModule_state_store_multiple(t *testing.T) {
t.Run("it detects when two state_store blocks are present within the same module in separate files", func(t *testing.T) {
t.Run("it detects when both cloud and backend blocks are present within the same module in separate files", func(t *testing.T) {
_, diags := testModuleFromDir("testdata/invalid-modules/conflict-cloud-backend-separate-files")
_, diags := testModuleFromDir("testdata/invalid-modules/multiple-state-store")
if !diags.HasErrors() {
t.Fatal("module should have error diags, but does not")
}
want := `Both a backend and cloud configuration are present`
want := `Duplicate 'state_store' configuration block`
if got := diags.Error(); !strings.Contains(got, want) {
t.Fatalf("expected error to contain %q\nerror was:\n%s", want, got)
}

@ -104,6 +104,14 @@ func parseConfigFile(body hcl.Body, diags hcl.Diagnostics, override, allowExperi
switch block.Type {
case "terraform":
// TODO: Update once pluggable state store is out of experimental phase
if allowExperiments {
terraformBlockSchema.Blocks = append(terraformBlockSchema.Blocks,
hcl.BlockHeaderSchema{
Type: "state_store",
LabelNames: []string{"type"},
})
}
content, contentDiags := block.Body.Content(terraformBlockSchema)
diags = append(diags, contentDiags...)
@ -121,6 +129,12 @@ func parseConfigFile(body hcl.Body, diags hcl.Diagnostics, override, allowExperi
file.Backends = append(file.Backends, backendCfg)
}
case "state_store":
stateStoreCfg, cfgDiags := decodeStateStoreBlock(innerBlock)
diags = append(diags, cfgDiags...)
if stateStoreCfg != nil {
file.StateStores = append(file.StateStores, stateStoreCfg)
}
case "cloud":
cloudCfg, cfgDiags := decodeCloudBlock(innerBlock)
diags = append(diags, cfgDiags...)

@ -0,0 +1,80 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package configs
import (
"github.com/hashicorp/hcl/v2"
)
// StateStore represents a "state_store" block inside a "terraform" block
// in a module or file.
type StateStore struct {
Type string
Config hcl.Body
Provider *Provider
TypeRange hcl.Range
DeclRange hcl.Range
}
func decodeStateStoreBlock(block *hcl.Block) (*StateStore, hcl.Diagnostics) {
var diags hcl.Diagnostics
ss := &StateStore{
Type: block.Labels[0],
TypeRange: block.LabelRanges[0],
Config: block.Body,
DeclRange: block.DefRange,
}
content, remain, moreDiags := block.Body.PartialContent(StateStorageBlockSchema)
diags = append(diags, moreDiags...)
ss.Config = remain
if len(content.Blocks) == 0 {
return nil, append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Missing provider block",
Detail: "A 'provider' block is required in 'state_store' blocks",
Subject: block.Body.MissingItemRange().Ptr(),
})
}
if len(content.Blocks) > 1 {
return nil, append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Duplicate provider block",
Detail: "Only one 'provider' block should be present in a 'state_store' block",
Subject: &content.Blocks[1].DefRange,
})
}
providerBlock := content.Blocks[0]
provider, providerDiags := decodeProviderBlock(providerBlock, false)
if providerDiags.HasErrors() {
return nil, append(diags, providerDiags...)
}
if provider.AliasRange != nil {
// This block is in its own namespace in the state_store block; aliases are irrelevant
return nil, append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Unexpected provider alias",
Detail: "Aliases are disallowed in the 'provider' block in the 'state_store' block",
Subject: provider.AliasRange,
})
}
ss.Provider = provider
return ss, diags
}
var StateStorageBlockSchema = &hcl.BodySchema{
Blocks: []hcl.BlockHeaderSchema{
{
Type: "provider",
LabelNames: []string{"type"},
},
},
}

@ -0,0 +1,16 @@
terraform {
backend "foo" {}
cloud {
organization = "foo"
workspaces {
name = "value"
}
}
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,8 @@
terraform {
cloud {
organization = "foo"
workspaces {
name = "value"
}
}
}

@ -0,0 +1,7 @@
terraform {
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,14 @@
terraform {
cloud {
organization = "foo"
workspaces {
name = "value"
}
}
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,7 @@
terraform {
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,9 @@
terraform {
backend "foo" {}
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,7 @@
terraform {
state_store "foo_bar" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,7 @@
terraform {
state_store "bar_bar" {
provider "bar" {}
custom_attr = "override"
}
}

@ -0,0 +1,13 @@
terraform {
backend "foo" {
path = "relative/path/to/terraform.tfstate"
}
}
resource "aws_instance" "web" {
ami = "ami-1234"
security_groups = [
"foo",
"bar",
]
}

@ -0,0 +1,8 @@
terraform {
// Note: not valid config - a paired entry in required_providers is usually needed
state_store "foo_override" {
provider "foo" {}
custom_attr = "override"
}
}

@ -0,0 +1,13 @@
terraform {
cloud {
organization = "foo"
}
}
resource "aws_instance" "web" {
ami = "ami-1234"
security_groups = [
"foo",
"bar",
]
}

@ -0,0 +1,10 @@
terraform {
// Note: not valid config - a paired entry in required_providers is usually needed
state_store "foo_override" {
provider "foo" {}
custom_attr = "override"
}
}
provider "bar" {}

@ -0,0 +1,7 @@
resource "aws_instance" "web" {
ami = "ami-1234"
security_groups = [
"foo",
"bar",
]
}

@ -0,0 +1,10 @@
terraform {
// Note: not valid config - a paired entry in required_providers is usually needed
state_store "foo_bar" {
provider "foo" {}
custom_attr = "foobar"
}
}
provider "foo" {}

@ -0,0 +1,16 @@
terraform {
// Note: not valid config - a paired entry in required_providers is usually needed
state_store "foo_bar" {
provider "foo" {}
custom_attr = "foobar"
}
}
resource "aws_instance" "web" {
ami = "ami-1234"
security_groups = [
"foo",
"bar",
]
}

@ -0,0 +1,10 @@
terraform {
// Note: not valid config - a paired entry in required_providers is usually needed
state_store "foo_override" {
provider "bar" {}
custom_attr = "override"
}
}
provider "bar" {}
Loading…
Cancel
Save