From 52058f5ccbbf23fbe79a0361fba29f8044c1b8a3 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 10:32:09 +0100 Subject: [PATCH 01/12] feat: Parsing a configuration directory can include .tfmigrate.hcl files including `migrate_from_backend` blocks. --- internal/configs/module.go | 31 +++++ internal/configs/parser_config.go | 11 ++ internal/configs/parser_config_dir.go | 26 ++++ internal/configs/parser_config_dir_test.go | 72 +++++++++- internal/configs/parser_file_matcher.go | 41 ++++-- internal/configs/state_migrate_file.go | 126 ++++++++++++++++++ .../1-file.tfmigrate.hcl | 3 + .../2-file.tfmigrate.hcl | 3 + .../main.tfmigrate.hcl | 7 + .../migration-from-backend/main.tfmigrate.hcl | 3 + 10 files changed, 313 insertions(+), 10 deletions(-) create mode 100644 internal/configs/state_migrate_file.go create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/1-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/2-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file/main.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/valid/migration-from-backend/main.tfmigrate.hcl diff --git a/internal/configs/module.go b/internal/configs/module.go index 5437360713c9..9c78b36ccd33 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -41,6 +41,8 @@ type Module struct { ProviderLocalNames map[addrs.Provider]string ProviderMetas map[addrs.Provider]*ProviderMeta + StateMigrationInstructions *StateMigrationInstructions + Variables map[string]*Variable Locals map[string]*Local Outputs map[string]*Output @@ -651,6 +653,35 @@ func (m *Module) appendQueryFile(file *QueryFile) hcl.Diagnostics { return diags } +// appendStateMigrationFile controls how multiple .tfmigrate.hcl files are combined +// to result in the final state migration configuration. This enables multiple blocks +// to be defined across multiple files. +func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnostics { + var diags hcl.Diagnostics + + if file == nil { + panic("appendStateMigrationFile receives a nil *StateMigrationFile pointer") + } + + // Validate process of combining data from across multiple files. + // Note: Validation of individual files should have happened earlier when they were parsed. + if file.MigrateFromBackend != nil { + if m.StateMigrationInstructions.MigrateFromBackend == nil { + m.StateMigrationInstructions.MigrateFromBackend = file.MigrateFromBackend + } else { + // Duplicate + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "migrate_from_backend" configuration block`, + Detail: fmt.Sprintf(`A "migrate_from_backend" block was already declared at %s. Only one of these blocks can be include in a module's state migration files.`, m.StateMigrationInstructions.MigrateFromBackend.DeclRange), + Subject: &file.MigrateFromBackend.DeclRange, + }) + } + } + + return diags +} + func (m *Module) mergeFile(file *File) hcl.Diagnostics { var diags hcl.Diagnostics diff --git a/internal/configs/parser_config.go b/internal/configs/parser_config.go index 401092f2da43..f4403fd70945 100644 --- a/internal/configs/parser_config.go +++ b/internal/configs/parser_config.go @@ -59,6 +59,17 @@ func (p *Parser) LoadQueryFile(path string) (*QueryFile, hcl.Diagnostics) { return query, diags } +func (p *Parser) LoadStateMigrationFile(path string) (*StateMigrationFile, hcl.Diagnostics) { + body, diags := p.LoadHCLFile(path) + if body == nil { + return nil, diags + } + + stateMigrations, stateMigrationsDiags := loadStateMigrationFile(body) + diags = diags.Extend(stateMigrationsDiags) + return stateMigrations, diags +} + // LoadMockDataFile reads the file at the given path and parses it as a // Terraform mock data file. // diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 8f4fc607d533..b7da03da4505 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -28,6 +28,7 @@ const ( // MatchTestFiles option, or from the default test directory. // If this option is not specified, test files will not be loaded. // Query files (.tfquery.hcl) are also loaded from the given directory. +// State Migration files (.tfmigrate.hcl) are also loaded from the given directory. // // If this method returns nil, that indicates that the given directory does not // exist at all or could not be opened for some reason. Callers may wish to @@ -79,6 +80,18 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno } } } + // Check if we need to load state migration files + if len(fileSet.StateMigrations) > 0 { + mod.StateMigrationInstructions = &StateMigrationInstructions{} + + stateMigrationFiles, fDiags := p.loadStateMigrateFiles(path, fileSet.StateMigrations) + diags = append(diags, fDiags...) + if mod != nil { + for _, smf := range stateMigrationFiles { + diags = diags.Extend(mod.appendStateMigrationFile(smf)) + } + } + } if mod != nil { mod.SourceDir = path @@ -220,6 +233,19 @@ func (p *Parser) loadQueryFiles(basePath string, paths []string) ([]*QueryFile, return files, diags } +func (p *Parser) loadStateMigrateFiles(basePath string, paths []string) ([]*StateMigrationFile, hcl.Diagnostics) { + var diags hcl.Diagnostics + + files := make([]*StateMigrationFile, 0, len(paths)) + for _, path := range paths { + f, fDiags := p.LoadStateMigrationFile(path) + diags = append(diags, fDiags...) + files = append(files, f) + } + + return files, diags +} + // fileExt returns the Terraform configuration extension of the given // path, or a blank string if it is not a recognized extension. func fileExt(path string) string { diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index c6f0478b712b..96eaec753c2f 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -238,8 +238,77 @@ func TestParserLoadConfigDirWithQueries(t *testing.T) { } } -func TestParserLoadTestFiles_Invalid(t *testing.T) { +// Testing happy path use of migrate_from_backend block. +func TestParserLoadConfigDirWithStateMigrations_migrate_from_backend(t *testing.T) { + testFixtures := "testdata/state-migration-files/valid/migration-from-backend" + // Below are specified in the config above + backendType := "s3" + bucketName := "foobar" + + // Parse the directory, including .tfmigrate.hcl files + parser := NewParser(nil) + mod, diags := parser.LoadConfigDir(testFixtures, MatchStateMigrateFiles()) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags) + } + if mod.StateMigrationInstructions == nil || mod.StateMigrationInstructions.MigrateFromBackend == nil { + t.Fatalf("expected mod.StateMigrationInstructions.MigrateFromBackend to be initialized, got:\n mod.StateMigrationInstructions = %#v\n mod.StateMigrationInstructions.MigrateFromBackend = %#v", + mod.StateMigrationInstructions, + mod.StateMigrationInstructions.MigrateFromBackend, + ) + } + + // Assert that the module includes expected information from migrate_from_backend block + b := mod.StateMigrationInstructions.MigrateFromBackend + if b.Type != backendType { + t.Fatalf("wrong backend type, got %q, want %q", b.Type, backendType) + } + attributes, diags := b.Config.JustAttributes() + if diags.HasErrors() { + t.Fatalf("unexpected error inspecting backend config: %s", diags) + } + gotBucketName, diags := attributes["bucket"].Expr.Value(nil) + if diags.HasErrors() { + t.Fatalf("unexpected error inspecting bucket attribute: %s", diags) + } + if gotBucketName.AsString() != bucketName { + t.Fatalf("wrong bucket name, got %q, want %q", gotBucketName, bucketName) + } +} + +func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { + tests := []struct { + name string + directory string + diagnosticSummary string + }{ + { + name: "backend duplicated in single file", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file", + diagnosticSummary: "Duplicate \"migrate_from_backend\" configuration block", + }, + { + name: "backend duplicated across multiple files", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files", + diagnosticSummary: "Duplicate \"migrate_from_backend\" configuration block", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + parser := NewParser(nil) + _, diags := parser.LoadConfigDir(test.directory, MatchStateMigrateFiles()) + if !diags.HasErrors() { + t.Fatalf("expected errors but got none: %s", diags) + } + if !strings.Contains(diags.Error(), test.diagnosticSummary) { + t.Fatalf("expected error to contain %q, but got %q", test.diagnosticSummary, diags.Error()) + } + }) + } +} + +func TestParserLoadTestFiles_Invalid(t *testing.T) { tcs := map[string][]string{ "duplicate_data_overrides": { "duplicate_data_overrides.tftest.hcl:7,3-16: Duplicate override_data block; An override_data block targeting data.aws_instance.test has already been defined at duplicate_data_overrides.tftest.hcl:2,3-16.", @@ -424,7 +493,6 @@ func TestParserLoadConfigDirFailure(t *testing.T) { } }) } - } func TestIsEmptyDir(t *testing.T) { diff --git a/internal/configs/parser_file_matcher.go b/internal/configs/parser_file_matcher.go index e9e66c40ae28..ccd241a7ba81 100644 --- a/internal/configs/parser_file_matcher.go +++ b/internal/configs/parser_file_matcher.go @@ -16,10 +16,11 @@ import ( // ConfigFileSet holds the different types of configuration files found in a directory. type ConfigFileSet struct { - Primary []string // Regular .tf and .tf.json files - Override []string // Override files (override.tf or *_override.tf) - Tests []string // Test files (.tftest.hcl or .tftest.json) - Queries []string // Query files (.tfquery.hcl) + Primary []string // Regular .tf and .tf.json files + Override []string // Override files (override.tf or *_override.tf) + Tests []string // Test files (.tftest.hcl or .tftest.json) + Queries []string // Query files (.tfquery.hcl) + StateMigrations []string // State migration files (.tfmigrate.hcl) } // FileMatcher is an interface for components that can match and process specific file types @@ -51,10 +52,11 @@ type parserConfig struct { func (p *Parser) dirFileSet(dir string, opts ...Option) (ConfigFileSet, hcl.Diagnostics) { var diags hcl.Diagnostics fileSet := ConfigFileSet{ - Primary: []string{}, - Override: []string{}, - Tests: []string{}, - Queries: []string{}, + Primary: []string{}, + Override: []string{}, + Tests: []string{}, + Queries: []string{}, + StateMigrations: []string{}, } // Set up the parser configuration @@ -122,6 +124,8 @@ func (p *Parser) rootFiles(dir string, matchers []FileMatcher, fileSet *ConfigFi fileSet.Tests = append(fileSet.Tests, fullPath) case *queryFiles: fileSet.Queries = append(fileSet.Queries, fullPath) + case *stateMigrateFiles: + fileSet.StateMigrations = append(fileSet.StateMigrations, fullPath) } break // Stop checking other matchers once a match is found } @@ -146,6 +150,13 @@ func MatchQueryFiles() Option { } } +// MatchStateMigrateFiles adds a matcher for Terraform state migrate files (.tfmigrate.hcl only) +func MatchStateMigrateFiles() Option { + return func(o *parserConfig) { + o.matchers = append(o.matchers, &stateMigrateFiles{}) + } +} + // moduleFiles matches regular Terraform configuration files (.tf and .tf.json) type moduleFiles struct{} @@ -242,3 +253,17 @@ func (q *queryFiles) Matches(name string) bool { func (q *queryFiles) DirFiles(dir string, options *parserConfig, fileSet *ConfigFileSet) hcl.Diagnostics { return nil } + +// stateMigrateFiles matches Terraform state migrate files (.tfmigrate.hcl only) +type stateMigrateFiles struct{} + +var _ FileMatcher = (*stateMigrateFiles)(nil) + +func (s *stateMigrateFiles) Matches(name string) bool { + return strings.HasSuffix(name, ".tfmigrate.hcl") +} + +func (s *stateMigrateFiles) DirFiles(dir string, options *parserConfig, fileSet *ConfigFileSet) hcl.Diagnostics { + // There are no special directories for .tfmigrate.hcl files. + return nil +} diff --git a/internal/configs/state_migrate_file.go b/internal/configs/state_migrate_file.go new file mode 100644 index 000000000000..c40ed62a3c01 --- /dev/null +++ b/internal/configs/state_migrate_file.go @@ -0,0 +1,126 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package configs + +import ( + "fmt" + + "github.com/hashicorp/hcl/v2" +) + +// StateMigrationInstructions represents the sum of all state migration files within a +// configuration directory. +// +// A state migration file contains blocks that define how resource state has previously +// been stored for a given project. In combination with an updated Terraform configuration, +// the two pieces of information describe the source and destination of state that the user +// wishes to migrate. +// +// When creating a StateMigrationInstructions struct, calling code must ensure that there +// are no duplicated or mutually-exclusive pieces of information in the original file(s). +type StateMigrationInstructions struct { + StateStoreProvider *Provider + + MigrateFromStateStore *StateStore + MigrateFromBackend *Backend +} + +// StateMigrationFile represents a single state migration file within a configuration directory. +// A project can include multiple files of this type, and their contents is aggregated. +type StateMigrationFile struct { + StateStoreProvider *Provider + + MigrateFromStateStore *StateStore + MigrateFromBackend *Backend +} + +func loadStateMigrationFile(body hcl.Body) (*StateMigrationFile, hcl.Diagnostics) { + var diags hcl.Diagnostics + file := &StateMigrationFile{} + + content, contentDiags := body.Content(stateMigrationFileSchema) + diags = append(diags, contentDiags...) + + for _, block := range content.Blocks { + switch block.Type { + case "state_store_provider": + // TODO + case "migrate_from_state_store": + // TODO + case "migrate_from_backend": + b, bDiags := decodeMigrateFromBackendBlock(block) + diags = diags.Extend(bDiags) + + if file.MigrateFromBackend != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "migrate_from_backend" configuration block`, + Detail: `Only one "migrate_from_backend" block is allowed in a directory's .tfmigrate.hcl files.`, + Subject: block.DefRange.Ptr(), + }) + continue // Keep file.MigrateFromBackend as first parsed block in this scenario + } + + if b != nil { + file.MigrateFromBackend = b + } + default: + // We don't expect other block types in state migration files. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid block type", + Detail: fmt.Sprintf("This block type is not valid within a state migration file: %s", block.Type), + Subject: block.DefRange.Ptr(), + }) + } + } + + // Check for mutually exclusive blocks, etc. + + // Defining two conflicting sources of state for migration. + if file.MigrateFromBackend != nil && file.MigrateFromStateStore != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "migrate_from_backend" and "migrate_from_state_store"`, + Detail: `The "migrate_from_backend" and "migrate_from_state_store" blocks are mutually-exclusive, only one should be used in a directory's .tfmigrate.hcl files..`, + }) + } + // Unnecessary state store-related data supplied alongside description of a backend. + if file.MigrateFromBackend != nil && file.StateStoreProvider != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, + Detail: `The "state_store_provider" block can only be used in combination with "migrate_from_state_store" blocks. Either remove the unused "state_store_provider" block, or replace the "migrate_from_backend" block with a "migrate_from_state_store" block.`, + }) + } + + return file, diags +} + +func decodeMigrateFromBackendBlock(block *hcl.Block) (*Backend, hcl.Diagnostics) { + // migrate_from_backend blocks are essentially the same as backend blocks, so reuse logic. + return decodeBackendBlock(block) +} + +// stateMigrationFileSchema is the schema for a .tfmigrate.hcl file, for use with +// the `state migrate` command. +// Whereas the current Terraform config (.tf) defines the destination that state should +// be migrated to, these files define how a backend or state store was previously configured. +// Due to this, these files define the source where migrated state is copied from. +var stateMigrationFileSchema = &hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{ + { + Type: "state_store_provider", + LabelNames: []string{"type"}, + }, + { + Type: "migrate_from_state_store", + LabelNames: []string{"type"}, + }, + { + Type: "migrate_from_backend", + LabelNames: []string{"type"}, + }, + }, +} diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/1-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/1-file.tfmigrate.hcl new file mode 100644 index 000000000000..fc3746c53ecf --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/1-file.tfmigrate.hcl @@ -0,0 +1,3 @@ +migrate_from_backend "s3" { + bucket = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/2-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/2-file.tfmigrate.hcl new file mode 100644 index 000000000000..f868e0d2f5b7 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files/2-file.tfmigrate.hcl @@ -0,0 +1,3 @@ +migrate_from_backend "gcs" { + bucket = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file/main.tfmigrate.hcl new file mode 100644 index 000000000000..bda9c9856563 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file/main.tfmigrate.hcl @@ -0,0 +1,7 @@ +migrate_from_backend "s3" { + bucket = "foobar" +} + +migrate_from_backend "gcs" { + bucket = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/valid/migration-from-backend/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/valid/migration-from-backend/main.tfmigrate.hcl new file mode 100644 index 000000000000..fc3746c53ecf --- /dev/null +++ b/internal/configs/testdata/state-migration-files/valid/migration-from-backend/main.tfmigrate.hcl @@ -0,0 +1,3 @@ +migrate_from_backend "s3" { + bucket = "foobar" +} \ No newline at end of file From 869c169a5317f155533921ae50301994e9ebe542 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 15:09:49 +0100 Subject: [PATCH 02/12] feat: Add ability to parse `state_store_provider` blocks from .tfmigrate.hcl files --- internal/configs/module.go | 12 ++ internal/configs/parser_config_dir_test.go | 15 ++ internal/configs/state_migrate_file.go | 202 +++++++++++++++++- .../1-file.tfmigrate.hcl | 6 + .../2-file.tfmigrate.hcl | 6 + .../main.tfmigrate.hcl | 13 ++ .../main.tfmigrate.hcl | 6 + 7 files changed, 255 insertions(+), 5 deletions(-) create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/1-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/2-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-same-file/main.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block/main.tfmigrate.hcl diff --git a/internal/configs/module.go b/internal/configs/module.go index 9c78b36ccd33..abc1e957d071 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -665,6 +665,18 @@ func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnost // Validate process of combining data from across multiple files. // Note: Validation of individual files should have happened earlier when they were parsed. + if file.StateStoreProvider != nil { + if m.StateMigrationInstructions.StateStoreProvider == nil { + m.StateMigrationInstructions.StateStoreProvider = file.StateStoreProvider + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "state_store_provider" configuration block`, + Detail: fmt.Sprintf(`A "state_store_provider" block was already declared at %s. Only one of these blocks can be include in a module's state migration files.`, m.StateMigrationInstructions.StateStoreProvider.DeclRange), + Subject: &file.StateStoreProvider.DeclRange, + }) + } + } if file.MigrateFromBackend != nil { if m.StateMigrationInstructions.MigrateFromBackend == nil { m.StateMigrationInstructions.MigrateFromBackend = file.MigrateFromBackend diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 96eaec753c2f..84b49ae68739 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -292,6 +292,21 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-multiple-files", diagnosticSummary: "Duplicate \"migrate_from_backend\" configuration block", }, + { + name: "state_store_provider duplicated in single file", + directory: "testdata/state-migration-files/invalid/duplicate-state-store-provider-block-same-file", + diagnosticSummary: "Duplicate \"state_store_provider\" configuration block", + }, + { + name: "state_store_provider duplicated in multiple files", + directory: "testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files", + diagnosticSummary: "Duplicate \"state_store_provider\" configuration block", + }, + { + name: "invalid version constraint in state_store_provider block", + directory: "testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block", + diagnosticSummary: `Non-specific version constraint in "state_store_provider" configuration block`, + }, } for _, test := range tests { diff --git a/internal/configs/state_migrate_file.go b/internal/configs/state_migrate_file.go index c40ed62a3c01..ff155f52edbc 100644 --- a/internal/configs/state_migrate_file.go +++ b/internal/configs/state_migrate_file.go @@ -5,8 +5,15 @@ package configs import ( "fmt" + "maps" + "slices" + "strings" + "github.com/apparentlymart/go-versions/versions" + version "github.com/hashicorp/go-version" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/zclconf/go-cty/cty" ) // StateMigrationInstructions represents the sum of all state migration files within a @@ -20,7 +27,7 @@ import ( // When creating a StateMigrationInstructions struct, calling code must ensure that there // are no duplicated or mutually-exclusive pieces of information in the original file(s). type StateMigrationInstructions struct { - StateStoreProvider *Provider + StateStoreProvider *RequiredProvider MigrateFromStateStore *StateStore MigrateFromBackend *Backend @@ -29,7 +36,7 @@ type StateMigrationInstructions struct { // StateMigrationFile represents a single state migration file within a configuration directory. // A project can include multiple files of this type, and their contents is aggregated. type StateMigrationFile struct { - StateStoreProvider *Provider + StateStoreProvider *RequiredProvider MigrateFromStateStore *StateStore MigrateFromBackend *Backend @@ -45,7 +52,22 @@ func loadStateMigrationFile(body hcl.Body) (*StateMigrationFile, hcl.Diagnostics for _, block := range content.Blocks { switch block.Type { case "state_store_provider": - // TODO + p, pDiags := decodeStateStoreProviderBlock(block) + diags = diags.Extend(pDiags) + + if file.StateStoreProvider != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "state_store_provider" configuration block`, + Detail: `Only one "state_store_provider" block is allowed in a directory's .tfmigrate.hcl files.`, + Subject: block.DefRange.Ptr(), + }) + continue // Keep file.StateStoreProvider as first parsed block in this scenario + } + + if p != nil { + file.StateStoreProvider = p + } case "migrate_from_state_store": // TODO case "migrate_from_backend": @@ -98,6 +120,177 @@ func loadStateMigrationFile(body hcl.Body) (*StateMigrationFile, hcl.Diagnostics return file, diags } +func decodeStateStoreProviderBlock(block *hcl.Block) (*RequiredProvider, hcl.Diagnostics) { + // state_store_provider blocks are similar to required_provider blocks but different, so we need logic + // similar to that in decodeProviderRequirementsBlock but distinct. E.g. version constraints must be + // exact versions, not a range. The similarity is sufficient that we can return a RequiredProvider pointer. + + var diags hcl.Diagnostics + attrs, hclDiags := block.Body.JustAttributes() + diags = diags.Extend(hclDiags) + + // Only one provider should be in the block + localNames := slices.Collect(maps.Keys(attrs)) + if len(localNames) != 1 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Unexpected number of providers described in "state_store_provider" configuration block.`, + Detail: fmt.Sprintf(`The "state_store_provider" block is only expected to include a single provider, but %d were found.`, len(localNames)), + Subject: block.DefRange.Ptr(), + }) + return nil, diags + } + localName := localNames[0] // Local name + attr := attrs[localName] // Block containing source and version info + + // verify that the local name is already localized or produce an error. + nameDiags := checkProviderNameNormalized(localName, attr.Expr.Range()) + if nameDiags.HasErrors() { + diags = append(diags, nameDiags...) + return nil, diags + } + + kvs, mapDiags := hcl.ExprMap(attr.Expr) + if mapDiags.HasErrors() { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid "state_store_provider" object`, + Detail: "The provider described inside state_store_provider must be an object", + Subject: attr.Expr.Range().Ptr(), + }) + return nil, diags + } + + // Process the data inside the object describing the provider + ssProvider := RequiredProvider{ + Name: localName, + DeclRange: attr.Range, + } + for _, kv := range kvs { + key, keyDiags := kv.Key.Value(nil) + if keyDiags.HasErrors() { + diags = append(diags, keyDiags...) + return nil, diags + } + + if key.Type() != cty.String { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid Attribute", + Detail: fmt.Sprintf("Invalid attribute value for provider requirement described by state_store_provider block: %#v", key), + Subject: kv.Key.Range().Ptr(), + }) + return nil, diags + } + + switch key.AsString() { + case "version": + vc := VersionConstraint{ + DeclRange: attr.Range, + } + + constraint, valDiags := kv.Value.Value(nil) + if valDiags.HasErrors() || !constraint.Type().Equals(cty.String) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "Version must be specified as a string.", + Subject: kv.Value.Range().Ptr(), + }) + continue + } + + constraintStr := constraint.AsString() + constraints, err := version.NewConstraint(constraintStr) + if err != nil { + // NewConstraint doesn't return user-friendly errors, so we'll just + // ignore the provided error and produce our own generic one. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "This string does not use correct version constraint syntax.", + Subject: kv.Value.Range().Ptr(), + }) + return nil, diags + } + + // Assert we have a single constraint, for a specific version + if len(constraints) != 1 { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "The version attribute inside the state_store_provider block must specify a single, specific version (e.g. \"= 1.0.0\").", + Subject: kv.Value.Range().Ptr(), + }) + return nil, diags + } + + // A constraint to use v1.2.3 could have an = operator or no operator at all. + constraintStr = strings.TrimPrefix(constraintStr, "=") // Remove a preceding `=`, if it exists. + constraintStr = strings.TrimSpace(constraintStr) // There might have been whitespace between the operator and the version. + + _, err = versions.ParseVersion(constraintStr) + if err != nil { + // Errors indicate that the constraint wasn't a specific version. + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Non-specific version constraint in "state_store_provider" configuration block`, + Detail: "The version constraint defined in a state_store_provider block must specify a single, specific version (e.g. \"= 1.0.0\", or \"1.0.0\").", + Subject: kv.Value.Range().Ptr(), + }) + return nil, diags + } + + // We capture the required version as a constraint, but + // we know the constraint is to a single version. + vc.Required = constraints + ssProvider.Requirement = vc + + case "source": + source, err := kv.Value.Value(nil) + if err != nil || !source.Type().Equals(cty.String) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid source", + Detail: "Source must be specified as a string.", + Subject: kv.Value.Range().Ptr(), + }) + return nil, diags + } + + fqn, sourceDiags := addrs.ParseProviderSourceString(source.AsString()) + if sourceDiags.HasErrors() { + hclDiags := sourceDiags.ToHCL() + // The diagnostics from ParseProviderSourceString don't contain + // source location information because it has no context to compute + // them from, and so we'll add those in quickly here before we + // return. + for _, diag := range hclDiags { + if diag.Subject == nil { + diag.Subject = kv.Value.Range().Ptr() + } + } + diags = append(diags, hclDiags...) + return nil, diags + } + + ssProvider.Source = source.AsString() + ssProvider.Type = fqn + default: + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid state_store_provider object", + Detail: `state_store_provider objects can only contain "version" and "source" attributes.`, + Subject: kv.Key.Range().Ptr(), + }) + return nil, diags + } + + } + + return &ssProvider, diags +} + func decodeMigrateFromBackendBlock(block *hcl.Block) (*Backend, hcl.Diagnostics) { // migrate_from_backend blocks are essentially the same as backend blocks, so reuse logic. return decodeBackendBlock(block) @@ -111,8 +304,7 @@ func decodeMigrateFromBackendBlock(block *hcl.Block) (*Backend, hcl.Diagnostics) var stateMigrationFileSchema = &hcl.BodySchema{ Blocks: []hcl.BlockHeaderSchema{ { - Type: "state_store_provider", - LabelNames: []string{"type"}, + Type: "state_store_provider", }, { Type: "migrate_from_state_store", diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/1-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/1-file.tfmigrate.hcl new file mode 100644 index 000000000000..f8dd052da68a --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/1-file.tfmigrate.hcl @@ -0,0 +1,6 @@ +state_store_provider { + test1 = { + source = "hashicorp/test1" + version = "1.0.0" + } +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/2-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/2-file.tfmigrate.hcl new file mode 100644 index 000000000000..7daddd8c33b5 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-multiple-files/2-file.tfmigrate.hcl @@ -0,0 +1,6 @@ +state_store_provider { + test2 = { + source = "hashicorp/test2" + version = "1.0.0" + } +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-same-file/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-same-file/main.tfmigrate.hcl new file mode 100644 index 000000000000..93fed0c260c7 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-state-store-provider-block-same-file/main.tfmigrate.hcl @@ -0,0 +1,13 @@ +state_store_provider { + test1 = { + source = "hashicorp/test1" + version = "1.0.0" + } +} + +state_store_provider { + test2 = { + source = "hashicorp/test2" + version = "1.0.0" + } +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block/main.tfmigrate.hcl new file mode 100644 index 000000000000..a40a24855975 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block/main.tfmigrate.hcl @@ -0,0 +1,6 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "~>1.0.0" + } +} \ No newline at end of file From 868a4554528112d7c1d854b663f10df1b44126ef Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 15:44:17 +0100 Subject: [PATCH 03/12] feat: Block using both `migrate_from_backend` and `state_store_provider` blocks across multiple .tfmigrate.hcl files --- internal/configs/module.go | 20 +++++++++++++++++++ internal/configs/parser_config_dir_test.go | 10 ++++++++++ .../migrate_from_backend.tfmigrate.hcl | 3 +++ .../state_store_provider.tfmigrate.hcl | 6 ++++++ .../main.tfmigrate.hcl | 10 ++++++++++ 5 files changed, 49 insertions(+) create mode 100644 internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/migrate_from_backend.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/state_store_provider.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file/main.tfmigrate.hcl diff --git a/internal/configs/module.go b/internal/configs/module.go index abc1e957d071..33cf752320e7 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -663,10 +663,24 @@ func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnost panic("appendStateMigrationFile receives a nil *StateMigrationFile pointer") } + backendConflictStateStoreProviderDiag := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, + Detail: `The "state_store_provider" block can only be used in combination with "migrate_from_state_store" blocks. Either remove the unused "state_store_provider" block, or replace the "migrate_from_backend" block with a "migrate_from_state_store" block.`, + // Sourceless because we don't know which block isn't needed. + } + // Validate process of combining data from across multiple files. + // This includes identifying duplications or conflicts across files. // Note: Validation of individual files should have happened earlier when they were parsed. if file.StateStoreProvider != nil { if m.StateMigrationInstructions.StateStoreProvider == nil { + if m.StateMigrationInstructions.MigrateFromBackend != nil { + // Parsed a "state_store_provider" block after a "migrate_from_backend" block + diags = diags.Append(backendConflictStateStoreProviderDiag) + return diags + } + m.StateMigrationInstructions.StateStoreProvider = file.StateStoreProvider } else { diags = append(diags, &hcl.Diagnostic{ @@ -679,6 +693,12 @@ func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnost } if file.MigrateFromBackend != nil { if m.StateMigrationInstructions.MigrateFromBackend == nil { + if m.StateMigrationInstructions.StateStoreProvider != nil { + // Parsed a "migrate_from_backend" block after a "state_store_provider" block + diags = diags.Append(backendConflictStateStoreProviderDiag) + return diags + } + m.StateMigrationInstructions.MigrateFromBackend = file.MigrateFromBackend } else { // Duplicate diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 84b49ae68739..38e62f0a250a 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -307,6 +307,16 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block", diagnosticSummary: `Non-specific version constraint in "state_store_provider" configuration block`, }, + { + name: "backend and state_store_provider mutually exclusive", + directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file", + diagnosticSummary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, + }, + { + name: "backend and state_store_provider mutually exclusive across multiple files", + directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files", + diagnosticSummary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, + }, } for _, test := range tests { diff --git a/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/migrate_from_backend.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/migrate_from_backend.tfmigrate.hcl new file mode 100644 index 000000000000..28b5a8f01dc2 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/migrate_from_backend.tfmigrate.hcl @@ -0,0 +1,3 @@ +migrate_from_backend "s3" { + bucket = "foobar" +} diff --git a/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/state_store_provider.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/state_store_provider.tfmigrate.hcl new file mode 100644 index 000000000000..45e8e5ba94aa --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files/state_store_provider.tfmigrate.hcl @@ -0,0 +1,6 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file/main.tfmigrate.hcl new file mode 100644 index 000000000000..b4648f30d9d8 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file/main.tfmigrate.hcl @@ -0,0 +1,10 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} + +migrate_from_backend "s3" { + bucket = "foobar" +} From 1ceba3ec25b4682b46ecd27d07d34f533d865aae Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 16:40:44 +0100 Subject: [PATCH 04/12] feat: Add validation that catches if a `state_store_provider` block is missing its counterpart `migrate_from_state_store` block --- internal/configs/parser_config_dir.go | 9 +++++++++ internal/configs/parser_config_dir_test.go | 5 +++++ .../only-state-store-provider-block/main.tfmigrate.hcl | 6 ++++++ 3 files changed, 20 insertions(+) create mode 100644 internal/configs/testdata/state-migration-files/invalid/only-state-store-provider-block/main.tfmigrate.hcl diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index b7da03da4505..69228e953d05 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -90,6 +90,15 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno for _, smf := range stateMigrationFiles { diags = diags.Extend(mod.appendStateMigrationFile(smf)) } + // Final checks that can only be done after all files are loaded + if mod.StateMigrationInstructions.StateStoreProvider != nil && + mod.StateMigrationInstructions.MigrateFromStateStore == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Missing "migrate_from_state_store" block for state store migration`, + Detail: `The configuration includes a "state_store_provider" block but is missing the required "migrate_from_state_store" block. Add a "migrate_from_state_store" block to specify the state store to migrate from.`, + }) + } } } diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 38e62f0a250a..392cd032dbea 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -317,6 +317,11 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files", diagnosticSummary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, }, + { + name: "only state_store_provider block", + directory: "testdata/state-migration-files/invalid/only-state-store-provider-block", + diagnosticSummary: `Missing "migrate_from_state_store" block for state store migration`, + }, } for _, test := range tests { diff --git a/internal/configs/testdata/state-migration-files/invalid/only-state-store-provider-block/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/only-state-store-provider-block/main.tfmigrate.hcl new file mode 100644 index 000000000000..693b9182ddc0 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/only-state-store-provider-block/main.tfmigrate.hcl @@ -0,0 +1,6 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} From 278b0b100baec10c96adee6937c695f642a9d2a1 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 17:22:25 +0100 Subject: [PATCH 05/12] feat: Add ability to parse `migrate_from_state_store` blocks from .tfmigrate.hcl files. Add tests for error paths and happy path. --- internal/configs/module.go | 28 ++++++++ internal/configs/parser_config_dir.go | 8 +++ internal/configs/parser_config_dir_test.go | 66 ++++++++++++++++++- internal/configs/state_migrate_file.go | 22 ++++++- .../1-file.tfmigrate.hcl | 13 ++++ .../2-file.tfmigrate.hcl | 6 ++ .../main.tfmigrate.hcl | 20 ++++++ .../main.tfmigrate.hcl | 6 ++ .../main.tfmigrate.hcl | 13 ++++ 9 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/1-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/2-file.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file/main.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/invalid/only-migrate-from-state-store-block/main.tfmigrate.hcl create mode 100644 internal/configs/testdata/state-migration-files/valid/migration-from-state-store/main.tfmigrate.hcl diff --git a/internal/configs/module.go b/internal/configs/module.go index 33cf752320e7..0b2bd08a6666 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -669,6 +669,12 @@ func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnost Detail: `The "state_store_provider" block can only be used in combination with "migrate_from_state_store" blocks. Either remove the unused "state_store_provider" block, or replace the "migrate_from_backend" block with a "migrate_from_state_store" block.`, // Sourceless because we don't know which block isn't needed. } + backendConflictMigrateFromStateStoreDiag := &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Invalid combination of "migrate_from_backend" and "migrate_from_state_store"`, + Detail: `A configuration cannot include both "migrate_from_backend" and "migrate_from_state_store" blocks. Remove one of these blocks, and the remaining block should describe where your existing state should be migrated from.`, + // Sourceless because we don't know which block isn't needed. + } // Validate process of combining data from across multiple files. // This includes identifying duplications or conflicts across files. @@ -691,8 +697,30 @@ func (m *Module) appendStateMigrationFile(file *StateMigrationFile) hcl.Diagnost }) } } + if file.MigrateFromStateStore != nil { + if m.StateMigrationInstructions.MigrateFromStateStore == nil { + if m.StateMigrationInstructions.MigrateFromBackend != nil { + // Parsed a "migrate_from_state_store" block after a "migrate_from_backend" block + diags = diags.Append(backendConflictMigrateFromStateStoreDiag) + return diags + } + m.StateMigrationInstructions.MigrateFromStateStore = file.MigrateFromStateStore + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "migrate_from_state_store" configuration block`, + Detail: fmt.Sprintf(`A "migrate_from_state_store" block was already declared at %s. Only one of these blocks can be include in a module's state migration files.`, m.StateMigrationInstructions.MigrateFromStateStore.DeclRange), + Subject: &file.MigrateFromStateStore.DeclRange, + }) + } + } if file.MigrateFromBackend != nil { if m.StateMigrationInstructions.MigrateFromBackend == nil { + if m.StateMigrationInstructions.MigrateFromStateStore != nil { + // Parsed a "migrate_from_backend" block after a "migrate_from_state_store" block + diags = diags.Append(backendConflictMigrateFromStateStoreDiag) + return diags + } if m.StateMigrationInstructions.StateStoreProvider != nil { // Parsed a "migrate_from_backend" block after a "state_store_provider" block diags = diags.Append(backendConflictStateStoreProviderDiag) diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 69228e953d05..4d25cce23a46 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -91,6 +91,14 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno diags = diags.Extend(mod.appendStateMigrationFile(smf)) } // Final checks that can only be done after all files are loaded + if mod.StateMigrationInstructions.MigrateFromStateStore != nil && + mod.StateMigrationInstructions.StateStoreProvider == nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Missing "state_store_provider" block for state store migration`, + Detail: `The configuration includes a "migrate_from_state_store" block but is missing the required "state_store_provider" block. Add a "state_store_provider" block to specify the provider to use when migrating state out of that state store.`, + }) + } if mod.StateMigrationInstructions.StateStoreProvider != nil && mod.StateMigrationInstructions.MigrateFromStateStore == nil { diags = diags.Append(&hcl.Diagnostic{ diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 392cd032dbea..a621640fb809 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/terraform/internal/addrs" ) // TestParseLoadConfigDirSuccess is a simple test that just verifies that @@ -120,7 +121,6 @@ func TestParserLoadConfigDirSuccess(t *testing.T) { } }) } - } func TestParserLoadConfigDirWithTests(t *testing.T) { @@ -137,7 +137,6 @@ func TestParserLoadConfigDirWithTests(t *testing.T) { for _, directory := range directories { t.Run(directory, func(t *testing.T) { - testDirectory := DefaultTestDirectory if directory == "testdata/valid-modules/with-tests-very-nested" { testDirectory = "very/nested" @@ -276,6 +275,54 @@ func TestParserLoadConfigDirWithStateMigrations_migrate_from_backend(t *testing. } } +// Testing happy path use of migrate_from_state_store block. This requires use of the state_store_provider +// block as well, so this also checks the happy path for that block. +func TestParserLoadConfigDirWithStateMigrations_migrate_from_state_store(t *testing.T) { + testFixtures := "testdata/state-migration-files/valid/migration-from-state-store" + // Below are specified in the config above + stateStoreType := "test_store" + + // Parse the directory, including .tfmigrate.hcl files + parser := NewParser(nil) + mod, diags := parser.LoadConfigDir(testFixtures, MatchStateMigrateFiles()) + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags) + } + if mod.StateMigrationInstructions == nil || mod.StateMigrationInstructions.MigrateFromStateStore == nil || mod.StateMigrationInstructions.StateStoreProvider == nil { + t.Fatalf("expected MigrateFromStateStore and StateStoreProvider to be initialized, got:\n mod.StateMigrationInstructions = %#v\n mod.StateMigrationInstructions.MigrateFromStateStore = %#v\n mod.StateMigrationInstructions.StateStoreProvider = %#v", + mod.StateMigrationInstructions, + mod.StateMigrationInstructions.MigrateFromStateStore, + mod.StateMigrationInstructions.StateStoreProvider, + ) + } + + // Assert that the module includes expected information from migrate_from_state_store block + ss := mod.StateMigrationInstructions.MigrateFromStateStore + if ss.Type != stateStoreType { + t.Fatalf("wrong state store type, got %q, want %q", ss.Type, stateStoreType) + } + if ss.Config == nil { + t.Fatalf("expected config to be non-nil") + } + // Populating provider info isn't done in parsing of config directly. + // TODO: Need to decide on where this info is resolved in context of the state migrate command. + if !ss.ProviderAddr.IsZero() { + t.Fatalf("expected provider addr to be empty, got %q", ss.ProviderAddr) + } + + // Assert that the module includes expected information from state_store_provider block + ssp := mod.StateMigrationInstructions.StateStoreProvider + if ssp.Name != "test" || ssp.Source != "hashicorp/test" || !ssp.Type.Equals(addrs.NewDefaultProvider("test")) { + t.Fatalf("unexpected state store provider info, got:\n Name: %q\n Source: %q\n Type: %q\n VersionConstraint: %q", + ssp.Name, ssp.Source, ssp.Type, ssp.Requirement, + ) + } + expectedConstraint := "1.0.0" + if ssp.Requirement.Required.String() != expectedConstraint { + t.Fatalf("unexpected version constraint, got %q, want %q", ssp.Requirement.Required.String(), expectedConstraint) + } +} + func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { tests := []struct { name string @@ -322,6 +369,21 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/only-state-store-provider-block", diagnosticSummary: `Missing "migrate_from_state_store" block for state store migration`, }, + { + name: "migrate_from_state_store duplicated in single file", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file", + diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", + }, + { + name: "migrate_from_state_store duplicated in multiple files", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files", + diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", + }, + { + name: "only migrate_from_state_store block", + directory: "testdata/state-migration-files/invalid/only-migrate-from-state-store-block", + diagnosticSummary: `Missing "state_store_provider" block for state store migration`, + }, } for _, test := range tests { diff --git a/internal/configs/state_migrate_file.go b/internal/configs/state_migrate_file.go index ff155f52edbc..98eeb2f84a7e 100644 --- a/internal/configs/state_migrate_file.go +++ b/internal/configs/state_migrate_file.go @@ -69,7 +69,22 @@ func loadStateMigrationFile(body hcl.Body) (*StateMigrationFile, hcl.Diagnostics file.StateStoreProvider = p } case "migrate_from_state_store": - // TODO + ss, ssDiags := decodeMigrateFromStateStoreBlock(block) + diags = diags.Extend(ssDiags) + + if file.MigrateFromStateStore != nil { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Duplicate "migrate_from_state_store" configuration block`, + Detail: `Only one "migrate_from_state_store" block is allowed in a directory's .tfmigrate.hcl files.`, + Subject: block.DefRange.Ptr(), + }) + continue // Keep file.MigrateFromStateStore as first parsed block in this scenario + } + + if ss != nil { + file.MigrateFromStateStore = ss + } case "migrate_from_backend": b, bDiags := decodeMigrateFromBackendBlock(block) diags = diags.Extend(bDiags) @@ -291,6 +306,11 @@ func decodeStateStoreProviderBlock(block *hcl.Block) (*RequiredProvider, hcl.Dia return &ssProvider, diags } +func decodeMigrateFromStateStoreBlock(block *hcl.Block) (*StateStore, hcl.Diagnostics) { + // migrate_from_state_store blocks are essentially the same as state_store blocks, so reuse logic. + return decodeStateStoreBlock(block) +} + func decodeMigrateFromBackendBlock(block *hcl.Block) (*Backend, hcl.Diagnostics) { // migrate_from_backend blocks are essentially the same as backend blocks, so reuse logic. return decodeBackendBlock(block) diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/1-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/1-file.tfmigrate.hcl new file mode 100644 index 000000000000..8103cfd810bb --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/1-file.tfmigrate.hcl @@ -0,0 +1,13 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} + +migrate_from_state_store "test_store1" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/2-file.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/2-file.tfmigrate.hcl new file mode 100644 index 000000000000..3efcb2d64e2e --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files/2-file.tfmigrate.hcl @@ -0,0 +1,6 @@ +migrate_from_state_store "test_store2" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file/main.tfmigrate.hcl new file mode 100644 index 000000000000..b802daf2391b --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file/main.tfmigrate.hcl @@ -0,0 +1,20 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} + +migrate_from_state_store "test_store1" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} + +migrate_from_state_store "test_store2" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/invalid/only-migrate-from-state-store-block/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/only-migrate-from-state-store-block/main.tfmigrate.hcl new file mode 100644 index 000000000000..b47b40326f94 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/only-migrate-from-state-store-block/main.tfmigrate.hcl @@ -0,0 +1,6 @@ +migrate_from_state_store "test_store" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file diff --git a/internal/configs/testdata/state-migration-files/valid/migration-from-state-store/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/valid/migration-from-state-store/main.tfmigrate.hcl new file mode 100644 index 000000000000..f5c54613ad74 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/valid/migration-from-state-store/main.tfmigrate.hcl @@ -0,0 +1,13 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } +} + +migrate_from_state_store "test_store" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file From cbf5a4fb4154b8b6dfb1ff611497fd1a856243ef Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 5 May 2026 23:02:24 +0100 Subject: [PATCH 06/12] refactor: Avoid potentially nil `mod` variable --- internal/configs/parser_config_dir.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 4d25cce23a46..5d7556444485 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -82,11 +82,11 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno } // Check if we need to load state migration files if len(fileSet.StateMigrations) > 0 { - mod.StateMigrationInstructions = &StateMigrationInstructions{} - stateMigrationFiles, fDiags := p.loadStateMigrateFiles(path, fileSet.StateMigrations) diags = append(diags, fDiags...) + if mod != nil { + mod.StateMigrationInstructions = &StateMigrationInstructions{} for _, smf := range stateMigrationFiles { diags = diags.Extend(mod.appendStateMigrationFile(smf)) } From 657a47f7f1f198d65e5483f7fef48b886dc12bc2 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 11:11:46 +0100 Subject: [PATCH 07/12] fix: Add validation checks to ensure the same provider is described across migrate_from_state_store and state_store_provider blocks. When they match, copy provider data across to description of the state store. Copying that data across is similar to how we use methods called `resolveStateStoreProviderType` when paring config (via NewModule or FinalizeConfig functions). --- internal/configs/parser_config_dir.go | 19 +++++++++++++++++++ internal/configs/parser_config_dir_test.go | 16 ++++++++++++---- .../main.tfmigrate.hcl | 15 +++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 internal/configs/testdata/state-migration-files/invalid/different-providers-between-blocks/main.tfmigrate.hcl diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 5d7556444485..d9e759139753 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -107,6 +107,25 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno Detail: `The configuration includes a "state_store_provider" block but is missing the required "migrate_from_state_store" block. Add a "migrate_from_state_store" block to specify the state store to migrate from.`, }) } + + // In non-error scenarios, migrate_from_state_store and state_store_provider blocks are either both present or absent. + // If they're both present, are they in agreement with each other? + if mod.StateMigrationInstructions.MigrateFromStateStore != nil && + mod.StateMigrationInstructions.StateStoreProvider != nil { + if mod.StateMigrationInstructions.MigrateFromStateStore.Provider.Name != mod.StateMigrationInstructions.StateStoreProvider.Name { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Inconsistent provider information for state migration`, + Detail: fmt.Sprintf(`The configuration's "state_store_provider" block defines a provider called %q but the "migrate_from_state_store" block uses a provider called %q instead. Please update the blocks so that they are in agreement.`, + mod.StateMigrationInstructions.StateStoreProvider.Name, + mod.StateMigrationInstructions.MigrateFromStateStore.Provider.Name, + ), + }) + } else { + // They match, so copy across relevant data. + mod.StateMigrationInstructions.MigrateFromStateStore.ProviderAddr = mod.StateMigrationInstructions.StateStoreProvider.Type + } + } } } diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index a621640fb809..f26b3333243a 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -304,10 +304,13 @@ func TestParserLoadConfigDirWithStateMigrations_migrate_from_state_store(t *test if ss.Config == nil { t.Fatalf("expected config to be non-nil") } - // Populating provider info isn't done in parsing of config directly. - // TODO: Need to decide on where this info is resolved in context of the state migrate command. - if !ss.ProviderAddr.IsZero() { - t.Fatalf("expected provider addr to be empty, got %q", ss.ProviderAddr) + if !ss.ProviderAddr.Equals(mod.StateMigrationInstructions.StateStoreProvider.Type) { + t.Fatalf("expected state store description's provider addr to have been populated with %q, but got %q", mod.StateMigrationInstructions.StateStoreProvider.Type.ForDisplay(), ss.ProviderAddr.ForDisplay()) + } + if ss.ProviderSupplyMode != "" { + // This is expected to be populated by calling code + // that is reading the config, not by the parser itself. + t.Fatal("unexpected data in ProviderSupplyMode") } // Assert that the module includes expected information from state_store_provider block @@ -384,6 +387,11 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/only-migrate-from-state-store-block", diagnosticSummary: `Missing "state_store_provider" block for state store migration`, }, + { + name: "different providers in migrate_from_state_store and state_store_provider blocks", + directory: "testdata/state-migration-files/invalid/different-providers-between-blocks", + diagnosticSummary: `Inconsistent provider information for state migration`, + }, } for _, test := range tests { diff --git a/internal/configs/testdata/state-migration-files/invalid/different-providers-between-blocks/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/different-providers-between-blocks/main.tfmigrate.hcl new file mode 100644 index 000000000000..394c966d0f40 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/different-providers-between-blocks/main.tfmigrate.hcl @@ -0,0 +1,15 @@ +state_store_provider { + foobar = { + source = "hashicorp/foobar" + version = "1.0.0" + } +} + +# The state store below references a different provider to the definition above + +migrate_from_state_store "test_store" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file From 45b0ec8f020ab7b71ef61f13e785d8c07db31d7a Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 12:28:33 +0100 Subject: [PATCH 08/12] feat: Add validation that blocks .tfmigrate.hcl files being present but empty. --- internal/configs/parser_config_dir.go | 10 ++++++++++ internal/configs/parser_config_dir_test.go | 5 +++++ .../invalid/no-blocks/empty.tfmigrate.hcl | 1 + 3 files changed, 16 insertions(+) create mode 100644 internal/configs/testdata/state-migration-files/invalid/no-blocks/empty.tfmigrate.hcl diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index d9e759139753..ddfccedd60d3 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -107,6 +107,16 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno Detail: `The configuration includes a "state_store_provider" block but is missing the required "migrate_from_state_store" block. Add a "migrate_from_state_store" block to specify the state store to migrate from.`, }) } + if mod.StateMigrationInstructions.StateStoreProvider == nil && + mod.StateMigrationInstructions.MigrateFromStateStore == nil && + mod.StateMigrationInstructions.MigrateFromBackend == nil { + // There are state migration file present, but they're empty. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Empty state migration configuration`, + Detail: `The configuration includes .tfmigrate.hcl files, but they are empty. Please make sure they include the necessary blocks to define a state migration, or remove the files from your project.`, + }) + } // In non-error scenarios, migrate_from_state_store and state_store_provider blocks are either both present or absent. // If they're both present, are they in agreement with each other? diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index f26b3333243a..4bc91232ccda 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -392,6 +392,11 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/different-providers-between-blocks", diagnosticSummary: `Inconsistent provider information for state migration`, }, + { + name: "no blocks present in the files", + directory: "testdata/state-migration-files/invalid/no-blocks", + diagnosticSummary: `Empty state migration configuration`, + }, } for _, test := range tests { diff --git a/internal/configs/testdata/state-migration-files/invalid/no-blocks/empty.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/no-blocks/empty.tfmigrate.hcl new file mode 100644 index 000000000000..e8fa1fcb2dab --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/no-blocks/empty.tfmigrate.hcl @@ -0,0 +1 @@ +# No blocks here! \ No newline at end of file From 984ad8d7c78ee8a501e34aa4fd42e4c455efe399 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 12:29:38 +0100 Subject: [PATCH 09/12] refactor: Replace if...else blocks with switch statement for clarity. --- internal/configs/parser_config_dir.go | 51 +++++++++++++-------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index ddfccedd60d3..5774f618438e 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -90,50 +90,49 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno for _, smf := range stateMigrationFiles { diags = diags.Extend(mod.appendStateMigrationFile(smf)) } - // Final checks that can only be done after all files are loaded - if mod.StateMigrationInstructions.MigrateFromStateStore != nil && - mod.StateMigrationInstructions.StateStoreProvider == nil { + + // Now, we perform some final checks that can only be done once all .tfmigrate.hcl files are loaded. + // Note: Other checks, like mutual exclusivity, were already performed when parsing single files or appending files. + ssp := mod.StateMigrationInstructions.StateStoreProvider + ss := mod.StateMigrationInstructions.MigrateFromStateStore + b := mod.StateMigrationInstructions.MigrateFromBackend + switch { + case ssp == nil && ss == nil && b == nil: + // Files present but all empty + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: `Empty state migration configuration`, + Detail: `The configuration includes .tfmigrate.hcl files, but they are empty. Please make sure they include the necessary blocks to define a state migration, or remove the files from your project.`, + }) + case ss != nil && ssp == nil: + // Missing state_store_provider block diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Missing "state_store_provider" block for state store migration`, Detail: `The configuration includes a "migrate_from_state_store" block but is missing the required "state_store_provider" block. Add a "state_store_provider" block to specify the provider to use when migrating state out of that state store.`, }) - } - if mod.StateMigrationInstructions.StateStoreProvider != nil && - mod.StateMigrationInstructions.MigrateFromStateStore == nil { + case ss == nil && ssp != nil: + // Missing migrate_from_state_store block diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Missing "migrate_from_state_store" block for state store migration`, Detail: `The configuration includes a "state_store_provider" block but is missing the required "migrate_from_state_store" block. Add a "migrate_from_state_store" block to specify the state store to migrate from.`, }) - } - if mod.StateMigrationInstructions.StateStoreProvider == nil && - mod.StateMigrationInstructions.MigrateFromStateStore == nil && - mod.StateMigrationInstructions.MigrateFromBackend == nil { - // There are state migration file present, but they're empty. - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: `Empty state migration configuration`, - Detail: `The configuration includes .tfmigrate.hcl files, but they are empty. Please make sure they include the necessary blocks to define a state migration, or remove the files from your project.`, - }) - } - - // In non-error scenarios, migrate_from_state_store and state_store_provider blocks are either both present or absent. - // If they're both present, are they in agreement with each other? - if mod.StateMigrationInstructions.MigrateFromStateStore != nil && - mod.StateMigrationInstructions.StateStoreProvider != nil { - if mod.StateMigrationInstructions.MigrateFromStateStore.Provider.Name != mod.StateMigrationInstructions.StateStoreProvider.Name { + case ss != nil && ssp != nil: + // Both migrate_from_state_store and state_store_provider blocks are present, + // but are they in agreement with each other? + if ss.Provider.Name != ssp.Name { diags = diags.Append(&hcl.Diagnostic{ Severity: hcl.DiagError, Summary: `Inconsistent provider information for state migration`, Detail: fmt.Sprintf(`The configuration's "state_store_provider" block defines a provider called %q but the "migrate_from_state_store" block uses a provider called %q instead. Please update the blocks so that they are in agreement.`, - mod.StateMigrationInstructions.StateStoreProvider.Name, - mod.StateMigrationInstructions.MigrateFromStateStore.Provider.Name, + ssp.Name, + ss.Provider.Name, ), }) } else { // They match, so copy across relevant data. - mod.StateMigrationInstructions.MigrateFromStateStore.ProviderAddr = mod.StateMigrationInstructions.StateStoreProvider.Type + ss.ProviderAddr = ssp.Type } } } From 9880c0716f221878f90321ef5b64368f76d0f29f Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 13:01:18 +0100 Subject: [PATCH 10/12] feat: Add validation that state_store_provider blocks contain only a single provider. Also, add early returns to avoid repeated or incorrect error diagnostics. Incorrect errors can occur if an error prevents a block being processed, and then downstream code reacts to the lack of that data with another error. That would be misleading (e.g. an error in a state_store_provider block meaning that later an incorrect error is raised reporting a missing state_store_provider block, when it is incorrect not missing). --- internal/configs/parser_config_dir.go | 13 +++++++++++++ internal/configs/parser_config_dir_test.go | 11 +++++++++++ .../main.tfmigrate.hcl | 17 +++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 internal/configs/testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block/main.tfmigrate.hcl diff --git a/internal/configs/parser_config_dir.go b/internal/configs/parser_config_dir.go index 5774f618438e..25976dd7e41d 100644 --- a/internal/configs/parser_config_dir.go +++ b/internal/configs/parser_config_dir.go @@ -84,6 +84,12 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno if len(fileSet.StateMigrations) > 0 { stateMigrationFiles, fDiags := p.loadStateMigrateFiles(path, fileSet.StateMigrations) diags = append(diags, fDiags...) + // If there are errors they may be duplicated below, so return early. + // We return an incomplete module representation. + if diags.HasErrors() { + mod.SourceDir = path + return mod, diags + } if mod != nil { mod.StateMigrationInstructions = &StateMigrationInstructions{} @@ -91,6 +97,13 @@ func (p *Parser) LoadConfigDir(path string, opts ...Option) (*Module, hcl.Diagno diags = diags.Extend(mod.appendStateMigrationFile(smf)) } + // If there are errors that might raise false positive below, so return early. + // We return an incomplete module representation. + if diags.HasErrors() { + mod.SourceDir = path + return mod, diags + } + // Now, we perform some final checks that can only be done once all .tfmigrate.hcl files are loaded. // Note: Other checks, like mutual exclusivity, were already performed when parsing single files or appending files. ssp := mod.StateMigrationInstructions.StateStoreProvider diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 4bc91232ccda..14c33761ba89 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -392,6 +392,11 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/different-providers-between-blocks", diagnosticSummary: `Inconsistent provider information for state migration`, }, + { + name: "multiple providers described in a state_store_provider block", + directory: "testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block", + diagnosticSummary: `Unexpected number of providers described in "state_store_provider" configuration block.`, + }, { name: "no blocks present in the files", directory: "testdata/state-migration-files/invalid/no-blocks", @@ -406,6 +411,12 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { if !diags.HasErrors() { t.Fatalf("expected errors but got none: %s", diags) } + if len(diags) != 1 { + for _, diag := range diags { + t.Log(diag) + } + t.Fatalf("expected only a single diagnostic to be returned, but got %d: \n%#v", len(diags), diags) + } if !strings.Contains(diags.Error(), test.diagnosticSummary) { t.Fatalf("expected error to contain %q, but got %q", test.diagnosticSummary, diags.Error()) } diff --git a/internal/configs/testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block/main.tfmigrate.hcl new file mode 100644 index 000000000000..976a57d0f764 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block/main.tfmigrate.hcl @@ -0,0 +1,17 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + } + foobar = { + source = "hashicorp/foobar" + version = "1.0.0" + } +} + +migrate_from_state_store "test_store" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file From 6c093dfdf7ec5dff4145044acfd281baf8e6934b Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 13:21:00 +0100 Subject: [PATCH 11/12] test: Add test case demonstrating parsing of objects inside the state_store_provider block --- internal/configs/parser_config_dir_test.go | 5 +++++ .../main.tfmigrate.hcl | 14 ++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 internal/configs/testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block/main.tfmigrate.hcl diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 14c33761ba89..7f25a19fbd21 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -357,6 +357,11 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block", diagnosticSummary: `Non-specific version constraint in "state_store_provider" configuration block`, }, + { + name: "unexpected attribute in state_store_provider block", + directory: "testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block", + diagnosticSummary: `Invalid state_store_provider object; state_store_provider objects can only contain "version" and "source" attributes.`, + }, { name: "backend and state_store_provider mutually exclusive", directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file", diff --git a/internal/configs/testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block/main.tfmigrate.hcl b/internal/configs/testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block/main.tfmigrate.hcl new file mode 100644 index 000000000000..2435fbd40727 --- /dev/null +++ b/internal/configs/testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block/main.tfmigrate.hcl @@ -0,0 +1,14 @@ +state_store_provider { + test = { + source = "hashicorp/test" + version = "1.0.0" + foobar = "this shouldn't be here" + } +} + +migrate_from_state_store "test_store" { + provider "test" { + provider_attr = "foobar" + } + store_attr = "foobar" +} \ No newline at end of file From c7b58cb7d7041f334a5825a5354a8ffaac0a8aa6 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 6 May 2026 13:23:58 +0100 Subject: [PATCH 12/12] test: Reorganise and group test cases for error scenarios parsing .tfmigrate.hcl files --- internal/configs/parser_config_dir_test.go | 44 ++++++++++++---------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/internal/configs/parser_config_dir_test.go b/internal/configs/parser_config_dir_test.go index 7f25a19fbd21..b9374cd45b1b 100644 --- a/internal/configs/parser_config_dir_test.go +++ b/internal/configs/parser_config_dir_test.go @@ -332,6 +332,7 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory string diagnosticSummary string }{ + // Duplicated blocks { name: "backend duplicated in single file", directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-backend-block-same-file", @@ -353,15 +354,16 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { diagnosticSummary: "Duplicate \"state_store_provider\" configuration block", }, { - name: "invalid version constraint in state_store_provider block", - directory: "testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block", - diagnosticSummary: `Non-specific version constraint in "state_store_provider" configuration block`, + name: "migrate_from_state_store duplicated in single file", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file", + diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", }, { - name: "unexpected attribute in state_store_provider block", - directory: "testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block", - diagnosticSummary: `Invalid state_store_provider object; state_store_provider objects can only contain "version" and "source" attributes.`, + name: "migrate_from_state_store duplicated in multiple files", + directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files", + diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", }, + // Mutually exclusive blocks { name: "backend and state_store_provider mutually exclusive", directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-same-file", @@ -372,25 +374,32 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/backend-and-state-store-provider-multiple-files", diagnosticSummary: `Invalid combination of "migrate_from_backend" and "state_store_provider"`, }, + // Missing blocks { name: "only state_store_provider block", directory: "testdata/state-migration-files/invalid/only-state-store-provider-block", diagnosticSummary: `Missing "migrate_from_state_store" block for state store migration`, }, { - name: "migrate_from_state_store duplicated in single file", - directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-same-file", - diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", + name: "only migrate_from_state_store block", + directory: "testdata/state-migration-files/invalid/only-migrate-from-state-store-block", + diagnosticSummary: `Missing "state_store_provider" block for state store migration`, }, { - name: "migrate_from_state_store duplicated in multiple files", - directory: "testdata/state-migration-files/invalid/duplicate-migrate-from-state-store-block-multiple-files", - diagnosticSummary: "Duplicate \"migrate_from_state_store\" configuration block", + name: "no blocks present in the files", + directory: "testdata/state-migration-files/invalid/no-blocks", + diagnosticSummary: `Empty state migration configuration`, }, + // Invalid contents of state_store_provider block { - name: "only migrate_from_state_store block", - directory: "testdata/state-migration-files/invalid/only-migrate-from-state-store-block", - diagnosticSummary: `Missing "state_store_provider" block for state store migration`, + name: "invalid version constraint in state_store_provider block", + directory: "testdata/state-migration-files/invalid/invalid-version-constraint-state-store-provider-block", + diagnosticSummary: `Non-specific version constraint in "state_store_provider" configuration block`, + }, + { + name: "unexpected attribute in state_store_provider block", + directory: "testdata/state-migration-files/invalid/unexpected-attribute-state-store-provider-block", + diagnosticSummary: `Invalid state_store_provider object; state_store_provider objects can only contain "version" and "source" attributes.`, }, { name: "different providers in migrate_from_state_store and state_store_provider blocks", @@ -402,11 +411,6 @@ func TestParserLoadConfigDirWithStateMigrations_error_cases(t *testing.T) { directory: "testdata/state-migration-files/invalid/multiple-providers-in-state-store-provider-block", diagnosticSummary: `Unexpected number of providers described in "state_store_provider" configuration block.`, }, - { - name: "no blocks present in the files", - directory: "testdata/state-migration-files/invalid/no-blocks", - diagnosticSummary: `Empty state migration configuration`, - }, } for _, test := range tests {