From 677aabc767c03dcacd5f3ca4b99d404596bf0d6c Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Fri, 21 Aug 2020 16:10:06 -0400 Subject: [PATCH] command: Fix backend config override validation When loading a backend config override file, init was doing two things wrong: - First, if the file failed to parse, we accidentally didn't return, which caused a panic due to the parsed body being nil; - Secondly, we were overzealous with the validation of the file, allowing only attributes. While most backend configs are attributes only, the enhanced remote backend body also contains a `workspaces` block, which we need to support here. This commit fixes the first bug with an early return and adds test cases for missing file and intentionally-blank filename (to clear the config). We also add a schema validation for the backend block, based on the backend schema itself. This requires constructing an HCL body schema so that we can call `Content` and check for diagnostic errors. The result is more useful errors when an invalid backend config override file is used, while also supporting the enhanced remote backend config fully. Does not include tests specific to the remote backend, because the mocking involved to allow the backend to fully initialize is too involved to be worth it. --- command/init.go | 35 ++++++++--- command/init_test.go | 63 ++++++++++++++++++- .../init-backend-config-file/backend.config | 2 +- .../init-backend-config-file/invalid.config | 2 + 4 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 command/testdata/init-backend-config-file/invalid.config diff --git a/command/init.go b/command/init.go index 8053d4b12..fd7abcd40 100644 --- a/command/init.go +++ b/command/init.go @@ -856,16 +856,31 @@ func (c *InitCommand) backendConfigOverrideBody(flags rawFlags, schema *configsc // The value is interpreted as a filename. newBody, fileDiags := c.loadHCLFile(item.Value) diags = diags.Append(fileDiags) - // Verify that the file contains only key-values pairs, and not a - // full backend config block. JustAttributes() will return an error - // if blocks are found - _, attrDiags := newBody.JustAttributes() - if attrDiags.HasErrors() { - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Invalid backend configuration file", - fmt.Sprintf("The backend configuration file %q given on the command line must contain key-value pairs only, and not configuration blocks.", item.Value), - )) + if fileDiags.HasErrors() { + continue + } + // Generate an HCL body schema for the backend block. + var bodySchema hcl.BodySchema + for name, attr := range schema.Attributes { + bodySchema.Attributes = append(bodySchema.Attributes, hcl.AttributeSchema{ + Name: name, + Required: attr.Required, + }) + } + for name, block := range schema.BlockTypes { + var labelNames []string + if block.Nesting == configschema.NestingMap { + labelNames = append(labelNames, "key") + } + bodySchema.Blocks = append(bodySchema.Blocks, hcl.BlockHeaderSchema{ + Type: name, + LabelNames: labelNames, + }) + } + // Verify that the file body matches the expected backend schema. + _, schemaDiags := newBody.Content(&bodySchema) + diags = diags.Append(schemaDiags) + if schemaDiags.HasErrors() { continue } flushVals() // deal with any accumulated individual values first diff --git a/command/init_test.go b/command/init_test.go index 9d79ec194..afdf82f09 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -355,8 +355,8 @@ func TestInit_backendConfigFile(t *testing.T) { } }) - // the backend config file must be a set of key-value pairs and not a full backend {} block - t.Run("invalid-config-file", func(t *testing.T) { + // the backend config file must not be a full terraform block + t.Run("full-backend-config-file", func(t *testing.T) { ui := new(cli.MockUi) c := &InitCommand{ Meta: Meta{ @@ -368,10 +368,67 @@ func TestInit_backendConfigFile(t *testing.T) { if code := c.Run(args); code != 1 { t.Fatalf("expected error, got success\n") } - if !strings.Contains(ui.ErrorWriter.String(), "Invalid backend configuration file") { + if !strings.Contains(ui.ErrorWriter.String(), "Unsupported block type") { t.Fatalf("wrong error: %s", ui.ErrorWriter) } }) + + // the backend config file must match the schema for the backend + t.Run("invalid-config-file", func(t *testing.T) { + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + args := []string{"-backend-config", "invalid.config"} + if code := c.Run(args); code != 1 { + t.Fatalf("expected error, got success\n") + } + if !strings.Contains(ui.ErrorWriter.String(), "Unsupported argument") { + t.Fatalf("wrong error: %s", ui.ErrorWriter) + } + }) + + // missing file is an error + t.Run("missing-config-file", func(t *testing.T) { + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + args := []string{"-backend-config", "missing.config"} + if code := c.Run(args); code != 1 { + t.Fatalf("expected error, got success\n") + } + if !strings.Contains(ui.ErrorWriter.String(), "Failed to read file") { + t.Fatalf("wrong error: %s", ui.ErrorWriter) + } + }) + + // blank filename clears the backend config + t.Run("blank-config-file", func(t *testing.T) { + ui := new(cli.MockUi) + c := &InitCommand{ + Meta: Meta{ + testingOverrides: metaOverridesForProvider(testProvider()), + Ui: ui, + }, + } + args := []string{"-backend-config="} + if code := c.Run(args); code != 0 { + t.Fatalf("bad: \n%s", ui.ErrorWriter.String()) + } + + // Read our saved backend config and verify the backend config is empty + state := testDataStateRead(t, filepath.Join(DefaultDataDir, DefaultStateFilename)) + if got, want := normalizeJSON(t, state.Backend.ConfigRaw), `{"path":null,"workspace_dir":null}`; got != want { + t.Errorf("wrong config\ngot: %s\nwant: %s", got, want) + } + }) } func TestInit_backendConfigFilePowershellConfusion(t *testing.T) { diff --git a/command/testdata/init-backend-config-file/backend.config b/command/testdata/init-backend-config-file/backend.config index db3d3159b..c3d7524bc 100644 --- a/command/testdata/init-backend-config-file/backend.config +++ b/command/testdata/init-backend-config-file/backend.config @@ -1,5 +1,5 @@ // the -backend-config flag on init cannot be used to point to a "full" backend -// block, only key-value pairs (like terraform.tfvars) +// block terraform { backend "local" { path = "hello" diff --git a/command/testdata/init-backend-config-file/invalid.config b/command/testdata/init-backend-config-file/invalid.config new file mode 100644 index 000000000..39f97f5bf --- /dev/null +++ b/command/testdata/init-backend-config-file/invalid.config @@ -0,0 +1,2 @@ +path = "hello" +foo = "bar"