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

Instance state only destroys #24083

Merged
merged 17 commits into from
Feb 14, 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
7 changes: 4 additions & 3 deletions command/apply_destroy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestApply_destroyTargeted(t *testing.T) {
Mode: addrs.ManagedResourceMode,
Type: "test_instance",
Name: "foo",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this change in this test? Seems suspicious.

Copy link
Member Author

Choose a reason for hiding this comment

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

The original test didn't match because the instance has count = 3.

&states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"id":"i-ab123"}`),
Status: states.ObjectReady,
Expand All @@ -212,8 +212,9 @@ func TestApply_destroyTargeted(t *testing.T) {
Name: "foo",
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
&states.ResourceInstanceObjectSrc{
AttrsJSON: []byte(`{"id":"i-abc123"}`),
Status: states.ObjectReady,
AttrsJSON: []byte(`{"id":"i-abc123"}`),
Dependencies: []addrs.AbsResource{mustResourceAddr("test_instance.foo")},
Status: states.ObjectReady,
},
addrs.AbsProviderConfig{
Provider: addrs.NewLegacyProvider("test"),
Expand Down
59 changes: 59 additions & 0 deletions configs/parser_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,3 +224,62 @@ func TestParserLoadConfigFileWarning(t *testing.T) {
})
}
}

// TestParseLoadConfigFileError is a test that verifies files from
// testdata/warning-files produce particular errors.
//
// This test does not verify that reading these files produces the correct
// file element contents in spite of those errors. More detailed assertions
// may be made on some subset of these configuration files in other tests.
func TestParserLoadConfigFileError(t *testing.T) {
files, err := ioutil.ReadDir("testdata/error-files")
if err != nil {
t.Fatal(err)
}

for _, info := range files {
name := info.Name()
t.Run(name, func(t *testing.T) {
src, err := ioutil.ReadFile(filepath.Join("testdata/error-files", name))
if err != nil {
t.Fatal(err)
}

// First we'll scan the file to see what warnings are expected.
// That's declared inside the files themselves by using the
// string "ERROR: " somewhere on each line that is expected
// to produce a warning, followed by the expected warning summary
// text. A single-line comment (with #) is the main way to do that.
const marker = "ERROR: "
sc := bufio.NewScanner(bytes.NewReader(src))
wantErrors := make(map[int]string)
lineNum := 1
for sc.Scan() {
lineText := sc.Text()
if idx := strings.Index(lineText, marker); idx != -1 {
summaryText := lineText[idx+len(marker):]
wantErrors[lineNum] = summaryText
}
lineNum++
}

parser := testParser(map[string]string{
name: string(src),
})

_, diags := parser.LoadConfigFile(name)

gotErrors := make(map[int]string)
for _, diag := range diags {
if diag.Severity != hcl.DiagError || diag.Subject == nil {
continue
}
gotErrors[diag.Subject.Start.Line] = diag.Summary
}

if diff := cmp.Diff(wantErrors, gotErrors); diff != "" {
t.Errorf("wrong errors\n%s", diff)
}
})
}
}
4 changes: 2 additions & 2 deletions configs/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func onlySelfRefs(body hcl.Body) hcl.Diagnostics {

if !valid {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "External references from destroy provisioners are deprecated",
Severity: hcl.DiagError,
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

Summary: "Invalid reference from destroy provisioner",
Detail: "Destroy-time provisioners and their connection configurations may only " +
"reference attributes of the related resource, via 'self', 'count.index', " +
"or 'each.key'.\n\nReferences to other resources during the destroy phase " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ locals {
resource "null_resource" "a" {
connection {
host = self.hostname
user = local.user # WARNING: External references from destroy provisioners are deprecated
user = local.user # ERROR: Invalid reference from destroy provisioner
}

provisioner "remote-exec" {
Expand Down Expand Up @@ -36,9 +36,9 @@ resource "null_resource" "b" {
when = destroy
connection {
host = self.hostname
user = local.user # WARNING: External references from destroy provisioners are deprecated
user = local.user # ERROR: Invalid reference from destroy provisioner
}

command = "echo ${local.name}" # WARNING: External references from destroy provisioners are deprecated
command = "echo ${local.name}" # ERROR: Invalid reference from destroy provisioner
}
}
Loading