Merge pull request #23559 from hashicorp/jbardin/deprecate-destroy-references

deprecation warning for destroy provisioner refs

configs/provisioner.go

@@ -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)
+			// destroy provisioners can only refer to self
-			//diags = append(diags, connDiags...)
+			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 {

configs/resource.go

@@ -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
 }
 

configs/testdata/warning-files/destroy-provisioners.tf

@@ -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
+  }
+}