deprecation warning for destroy provisioner refs

Add deprecation warning for references from destroy provisioners or
their connections to external resources or values. In order to ensure
resource destruction can be completed correctly, destroy nodes must be
able to evaluate with only their instance state.

We have sufficient information to validate destroy-time provisioners
early on during the config loading process. Later on these can be
converted to hard errors, and only allow self, count.index, and each.key
in destroy provisioners. Limited the provisioner and block evaluation
scope later on is tricky, but if the references can never be loaded,
then they will never be encountered during evaluation.
This commit is contained in:
James Bardin 2019-12-03 13:23:07 -05:00
parent 26131d948c
commit 8547603ff5
3 changed files with 104 additions and 2 deletions

View File

@ -50,6 +50,11 @@ func decodeProvisionerBlock(block *hcl.Block) (*Provisioner, hcl.Diagnostics) {
}
}
// destroy provisioners can only refer to self
if pv.When == ProvisionerWhenDestroy {
diags = append(diags, onlySelfRefs(config)...)
}
if attr, exists := content.Attributes["on_failure"]; exists {
expr, shimDiags := shimTraversalInString(attr.Expr, true)
diags = append(diags, shimDiags...)
@ -85,8 +90,11 @@ func decodeProvisionerBlock(block *hcl.Block) (*Provisioner, hcl.Diagnostics) {
}
seenConnection = block
//conn, connDiags := decodeConnectionBlock(block)
//diags = append(diags, connDiags...)
// destroy provisioners can only refer to self
if pv.When == ProvisionerWhenDestroy {
diags = append(diags, onlySelfRefs(block.Body)...)
}
pv.Connection = &Connection{
Config: block.Body,
DeclRange: block.DefRange,
@ -107,6 +115,49 @@ func decodeProvisionerBlock(block *hcl.Block) (*Provisioner, hcl.Diagnostics) {
return pv, diags
}
func onlySelfRefs(body hcl.Body) hcl.Diagnostics {
var diags hcl.Diagnostics
// Provisioners currently do not use any blocks in their configuration.
// Blocks are likely to remain solely for meta parameters, but in the case
// that blocks are supported for provisioners, we will want to extend this
// to find variables in nested blocks.
attrs, _ := body.JustAttributes()
for _, attr := range attrs {
for _, v := range attr.Expr.Variables() {
valid := false
switch v.RootName() {
case "self":
valid = true
case "count":
// count must use "index"
if len(v) == 2 {
if t, ok := v[1].(hcl.TraverseAttr); ok && t.Name == "index" {
valid = true
}
}
case "each":
if len(v) == 2 {
if t, ok := v[1].(hcl.TraverseAttr); ok && t.Name == "key" {
valid = true
}
}
}
if !valid {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagWarning,
Summary: "External references from destroy provisioners are deprecated",
Detail: "Destroy time provisioners and their connections may only reference values stored in the instance state, which include 'self', 'count.index', or 'each.key'.",
Subject: attr.Expr.Range().Ptr(),
})
}
}
}
return diags
}
// Connection represents a "connection" block when used within either a
// "resource" or "provisioner" block in a module or file.
type Connection struct {

View File

@ -273,6 +273,17 @@ func decodeResourceBlock(block *hcl.Block) (*Resource, hcl.Diagnostics) {
}
}
// Now we can validate the connection block references if there are any destroy provisioners.
// TODO: should we eliminate standalone connection blocks?
if r.Managed.Connection != nil {
for _, p := range r.Managed.Provisioners {
if p.When == ProvisionerWhenDestroy {
diags = append(diags, onlySelfRefs(r.Managed.Connection.Config)...)
break
}
}
}
return r, diags
}

View File

@ -0,0 +1,40 @@
locals {
user = "name"
}
resource "null_resource" "a" {
connection {
host = self.hostname
user = local.user # WARNING: External references from destroy provisioners are deprecated
}
provisioner "remote-exec" {
when = destroy
index = count.index
key = each.key
}
}
resource "null_resource" "b" {
connection {
host = self.hostname
# this is OK since there is no destroy provisioner
user = local.user
}
provisioner "remote-exec" {
}
}
resource "null_resource" "b" {
provisioner "remote-exec" {
when = destroy
connection {
host = self.hostname
user = local.user # WARNING: External references from destroy provisioners are deprecated
}
command = "echo ${local.name}" # WARNING: External references from destroy provisioners are deprecated
}
}