Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command: Taint should respect required_version #26345

Merged
merged 1 commit into from
Sep 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions command/taint.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package command
import (
"context"
"fmt"
"os"
"strings"

"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/command/clistate"
"github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/terraform"
"github.com/hashicorp/terraform/tfdiags"
)

Expand Down Expand Up @@ -62,6 +64,34 @@ func (c *TaintCommand) Run(args []string) int {
return 1
}

// Load the config and check the core version requirements are satisfied
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but I'd be curious to see if the earlyconfig loader (examples in init.go) is sufficient and perhaps faster for this purpose. loadSingleModule for e.g. will only load the root module:

// loadSingleModule reads configuration from the given directory and returns
// a description of that module only, without attempting to assemble a module
// tree for referenced child modules.

Again, this might not actually be faster or better, but it might be worth looking at.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I hadn't thought of that! Unfortunately I think we need to load and check the entire tree's version constraints. In particular, many public registry modules have fairly strict version requirements that we need to honour.

loader, err := c.initConfigLoader()
if err != nil {
diags = diags.Append(err)
c.showDiagnostics(diags)
return 1
}

pwd, err := os.Getwd()
if err != nil {
c.Ui.Error(fmt.Sprintf("Error getting pwd: %s", err))
return 1
}

config, configDiags := loader.LoadConfig(pwd)
diags = diags.Append(configDiags)
if diags.HasErrors() {
c.showDiagnostics(diags)
return 1
}

versionDiags := terraform.CheckCoreVersionRequirements(config)
diags = diags.Append(versionDiags)
if diags.HasErrors() {
c.showDiagnostics(diags)
return 1
}

// Load the backend
b, backendDiags := c.Backend(nil)
diags = diags.Append(backendDiags)
Expand Down
52 changes: 52 additions & 0 deletions command/taint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/mitchellh/cli"

"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/helper/copy"
"github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/terraform"
)
Expand Down Expand Up @@ -407,6 +408,57 @@ func TestTaint_module(t *testing.T) {
testStateOutput(t, statePath, testTaintModuleStr)
}

func TestTaint_checkRequiredVersion(t *testing.T) {
// Create a temporary working directory that is empty
td := tempDir(t)
copy.CopyDir(testFixturePath("taint-check-required-version"), td)
defer os.RemoveAll(td)
defer testChdir(t, td)()

// Write the temp state
state := &terraform.State{
Modules: []*terraform.ModuleState{
{
Path: []string{"root"},
Resources: map[string]*terraform.ResourceState{
"test_instance.foo": {
Type: "test_instance",
Primary: &terraform.InstanceState{
ID: "bar",
},
},
},
},
},
}
path := testStateFileDefault(t, state)

ui := cli.NewMockUi()
c := &TaintCommand{
Meta: Meta{
testingOverrides: metaOverridesForProvider(testProvider()),
Ui: ui,
},
}

args := []string{"test_instance.foo"}
if code := c.Run(args); code != 1 {
t.Fatalf("got exit status %d; want 1\nstderr:\n%s\n\nstdout:\n%s", code, ui.ErrorWriter.String(), ui.OutputWriter.String())
}

// State is unchanged
testStateOutput(t, path, testTaintDefaultStr)

// Required version diags are correct
errStr := ui.ErrorWriter.String()
if !strings.Contains(errStr, `required_version = "~> 0.9.0"`) {
t.Fatalf("output should point to unmet version constraint, but is:\n\n%s", errStr)
}
if strings.Contains(errStr, `required_version = ">= 0.13.0"`) {
t.Fatalf("output should not point to met version constraint, but is:\n\n%s", errStr)
}
}

const testTaintStr = `
test_instance.foo: (tainted)
ID = bar
Expand Down
7 changes: 7 additions & 0 deletions command/testdata/taint-check-required-version/main.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
terraform {
required_version = "~> 0.9.0"
}

terraform {
required_version = ">= 0.13.0"
}