From bd6ba6cf9946e535faa99ba46340ed92452937ca Mon Sep 17 00:00:00 2001 From: CJ Horton <17039873+radditude@users.noreply.github.com> Date: Fri, 12 May 2023 15:14:44 -0700 Subject: [PATCH] check for duplicate import blocks (#33190) Importing to the same target address twice or importing the same ID to multiple different resources of the same type is not allowed. --- .../duplicate_import_ids/main.tf | 15 ++++++++ .../duplicate_import_ids/output.json | 34 +++++++++++++++++++ .../duplicate_import_targets/main.tf | 12 +++++++ .../duplicate_import_targets/output.json | 34 +++++++++++++++++++ internal/command/validate_test.go | 24 +++++++++++++ internal/configs/module.go | 24 +++++++++++++ 6 files changed, 143 insertions(+) create mode 100644 internal/command/testdata/validate-invalid/duplicate_import_ids/main.tf create mode 100644 internal/command/testdata/validate-invalid/duplicate_import_ids/output.json create mode 100644 internal/command/testdata/validate-invalid/duplicate_import_targets/main.tf create mode 100644 internal/command/testdata/validate-invalid/duplicate_import_targets/output.json diff --git a/internal/command/testdata/validate-invalid/duplicate_import_ids/main.tf b/internal/command/testdata/validate-invalid/duplicate_import_ids/main.tf new file mode 100644 index 0000000000..bf096e4373 --- /dev/null +++ b/internal/command/testdata/validate-invalid/duplicate_import_ids/main.tf @@ -0,0 +1,15 @@ +resource "aws_instance" "web" { +} + +resource "aws_instance" "other_web" { +} + +import { + to = aws_instance.web + id = "test" +} + +import { + to = aws_instance.other_web + id = "test" +} diff --git a/internal/command/testdata/validate-invalid/duplicate_import_ids/output.json b/internal/command/testdata/validate-invalid/duplicate_import_ids/output.json new file mode 100644 index 0000000000..c1764f8c7c --- /dev/null +++ b/internal/command/testdata/validate-invalid/duplicate_import_ids/output.json @@ -0,0 +1,34 @@ +{ + "format_version": "1.0", + "valid": false, + "error_count": 1, + "warning_count": 0, + "diagnostics": [ + { + "severity": "error", + "summary": "Duplicate import for ID \"test\"", + "detail": "An import block for the ID \"test\" and a resource of type \"aws_instance\" was already declared at testdata/validate-invalid/duplicate_import_ids/main.tf:7,1-7. The same resource cannot be imported twice.", + "range": { + "filename": "testdata/validate-invalid/duplicate_import_ids/main.tf", + "start": { + "line": 12, + "column": 1, + "byte": 126 + }, + "end": { + "line": 12, + "column": 7, + "byte": 132 + } + }, + "snippet": { + "context": null, + "code": "import {", + "start_line": 12, + "highlight_start_offset": 0, + "highlight_end_offset": 6, + "values": [] + } + } + ] +} \ No newline at end of file diff --git a/internal/command/testdata/validate-invalid/duplicate_import_targets/main.tf b/internal/command/testdata/validate-invalid/duplicate_import_targets/main.tf new file mode 100644 index 0000000000..3c663bd105 --- /dev/null +++ b/internal/command/testdata/validate-invalid/duplicate_import_targets/main.tf @@ -0,0 +1,12 @@ +resource "aws_instance" "web" { +} + +import { + to = aws_instance.web + id = "test" +} + +import { + to = aws_instance.web + id = "test2" +} diff --git a/internal/command/testdata/validate-invalid/duplicate_import_targets/output.json b/internal/command/testdata/validate-invalid/duplicate_import_targets/output.json new file mode 100644 index 0000000000..bad183b0c9 --- /dev/null +++ b/internal/command/testdata/validate-invalid/duplicate_import_targets/output.json @@ -0,0 +1,34 @@ +{ + "format_version": "1.0", + "valid": false, + "error_count": 1, + "warning_count": 0, + "diagnostics": [ + { + "severity": "error", + "summary": "Duplicate import configuration for \"aws_instance.web\"", + "detail": "An import block for the resource \"aws_instance.web\" was already declared at testdata/validate-invalid/duplicate_import_targets/main.tf:4,1-7. A resource can have only one import block.", + "range": { + "filename": "testdata/validate-invalid/duplicate_import_targets/main.tf", + "start": { + "line": 9, + "column": 1, + "byte": 85 + }, + "end": { + "line": 9, + "column": 7, + "byte": 91 + } + }, + "snippet": { + "context": null, + "code": "import {", + "start_line": 9, + "highlight_start_offset": 0, + "highlight_end_offset": 6, + "values": [] + } + } + ] +} diff --git a/internal/command/validate_test.go b/internal/command/validate_test.go index 03b91aa387..dfee4807f1 100644 --- a/internal/command/validate_test.go +++ b/internal/command/validate_test.go @@ -150,6 +150,28 @@ func TestSameResourceMultipleTimesShouldFail(t *testing.T) { } } +func TestSameImportTargetMultipleTimesShouldFail(t *testing.T) { + output, code := setupTest(t, "validate-invalid/duplicate_import_targets") + if code != 1 { + t.Fatalf("Should have failed: %d\n\n%s", code, output.Stderr()) + } + wantError := `Error: Duplicate import configuration for "aws_instance.web"` + if !strings.Contains(output.Stderr(), wantError) { + t.Fatalf("Missing error string %q\n\n'%s'", wantError, output.Stderr()) + } +} + +func TestSameImportIDMultipleTimesShouldFail(t *testing.T) { + output, code := setupTest(t, "validate-invalid/duplicate_import_ids") + if code != 1 { + t.Fatalf("Should have failed: %d\n\n%s", code, output.Stderr()) + } + wantError := `Error: Duplicate import for ID "test"` + if !strings.Contains(output.Stderr(), wantError) { + t.Fatalf("Missing error string %q\n\n'%s'", wantError, output.Stderr()) + } +} + func TestOutputWithoutValueShouldFail(t *testing.T) { output, code := setupTest(t, "validate-invalid/outputs") if code != 1 { @@ -218,6 +240,8 @@ func TestValidate_json(t *testing.T) { {"validate-invalid/multiple_providers", false}, {"validate-invalid/multiple_modules", false}, {"validate-invalid/multiple_resources", false}, + {"validate-invalid/duplicate_import_targets", false}, + {"validate-invalid/duplicate_import_ids", false}, {"validate-invalid/outputs", false}, {"validate-invalid/incorrectmodulename", false}, {"validate-invalid/interpolation", false}, diff --git a/internal/configs/module.go b/internal/configs/module.go index ce5125830c..eb0f70e23b 100644 --- a/internal/configs/module.go +++ b/internal/configs/module.go @@ -403,6 +403,30 @@ func (m *Module) appendFile(file *File) hcl.Diagnostics { } for _, i := range file.Import { + for _, mi := range m.Import { + if i.To.Equal(mi.To) { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate import configuration for %q", i.To), + Detail: fmt.Sprintf("An import block for the resource %q was already declared at %s. A resource can have only one import block.", i.To, mi.DeclRange), + Subject: &i.DeclRange, + }) + continue + } + + if i.ID == mi.ID { + if i.To.Resource.Resource.Type == mi.To.Resource.Resource.Type { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: fmt.Sprintf("Duplicate import for ID %q", i.ID), + Detail: fmt.Sprintf("An import block for the ID %q and a resource of type %q was already declared at %s. The same resource cannot be imported twice.", i.ID, i.To.Resource.Resource.Type, mi.DeclRange), + Subject: &i.DeclRange, + }) + continue + } + } + } + if i.ProviderConfigRef != nil { i.Provider = m.ProviderForLocalConfig(addrs.LocalProviderConfig{ LocalName: i.ProviderConfigRef.Name,