From a96a3372c66084c1511a94f761d59752a89cc0d0 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Fri, 8 May 2015 17:46:37 -0500
Subject: [PATCH] provider/template: don't diff when there's no diff

This reworks the template lifecycle a bit such that we get nicer diff
behavior.

First, we tick ForceNew on for both filename and vars, so that the diff
indicates that the template will be "replaced" on change. This is mostly
cosmetic, but it also tracks conceptually with the fact that the
identifier we use is a hash of the contents, so any change essentially
makes a "new resource".

Second, we change the Exists implementation to only return `false` when
there has been a change in the rendered template. This lets descendent
resources see the computed value changing so that they'll properly
trigger in the plan.

Fixes #1898
Refs #1866 (but does not fix, there's another deeper issue there)
---
 builtin/providers/template/resource.go      | 48 ++++++++++++++-------
 builtin/providers/template/resource_test.go |  1 -
 helper/resource/testing.go                  |  8 +---
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/builtin/providers/template/resource.go b/builtin/providers/template/resource.go
index 9e31ced1d..d1a8176be 100644
--- a/builtin/providers/template/resource.go
+++ b/builtin/providers/template/resource.go
@@ -16,22 +16,23 @@ import (
 func resource() *schema.Resource {
 	return &schema.Resource{
 		Create: Create,
-		Read:   Read,
-		Update: Update,
 		Delete: Delete,
 		Exists: Exists,
+		Read:   Read,
 
 		Schema: map[string]*schema.Schema{
 			"filename": &schema.Schema{
 				Type:        schema.TypeString,
 				Required:    true,
 				Description: "file to read template from",
+				ForceNew:    true,
 			},
 			"vars": &schema.Schema{
 				Type:        schema.TypeMap,
 				Optional:    true,
 				Default:     make(map[string]interface{}),
 				Description: "variables to substitute",
+				ForceNew:    true,
 			},
 			"rendered": &schema.Schema{
 				Type:        schema.TypeString,
@@ -42,43 +43,58 @@ func resource() *schema.Resource {
 	}
 }
 
-func Create(d *schema.ResourceData, meta interface{}) error { return eval(d) }
-func Update(d *schema.ResourceData, meta interface{}) error { return eval(d) }
-func Read(d *schema.ResourceData, meta interface{}) error   { return nil }
+func Create(d *schema.ResourceData, meta interface{}) error {
+	rendered, err := render(d)
+	if err != nil {
+		return err
+	}
+	d.Set("rendered", rendered)
+	d.SetId(hash(rendered))
+	return nil
+}
+
 func Delete(d *schema.ResourceData, meta interface{}) error {
 	d.SetId("")
 	return nil
 }
+
 func Exists(d *schema.ResourceData, meta interface{}) (bool, error) {
-	// Reload every time in case something has changed.
-	// This should be cheap, and cache invalidation is hard.
-	return false, nil
+	rendered, err := render(d)
+	if err != nil {
+		return false, err
+	}
+	return hash(rendered) == d.Id(), nil
+}
+
+func Read(d *schema.ResourceData, meta interface{}) error {
+	// Logic is handled in Exists, which only returns true if the rendered
+	// contents haven't changed. That means if we get here there's nothing to
+	// do.
+	return nil
 }
 
 var readfile func(string) ([]byte, error) = ioutil.ReadFile // testing hook
 
-func eval(d *schema.ResourceData) error {
+func render(d *schema.ResourceData) (string, error) {
 	filename := d.Get("filename").(string)
 	vars := d.Get("vars").(map[string]interface{})
 
 	path, err := homedir.Expand(filename)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	buf, err := readfile(path)
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	rendered, err := execute(string(buf), vars)
 	if err != nil {
-		return fmt.Errorf("failed to render %v: %v", filename, err)
+		return "", fmt.Errorf("failed to render %v: %v", filename, err)
 	}
 
-	d.Set("rendered", rendered)
-	d.SetId(hash(rendered))
-	return nil
+	return rendered, nil
 }
 
 // execute parses and executes a template using vars.
@@ -122,5 +138,5 @@ func execute(s string, vars map[string]interface{}) (string, error) {
 
 func hash(s string) string {
 	sha := sha256.Sum256([]byte(s))
-	return hex.EncodeToString(sha[:])[:20]
+	return hex.EncodeToString(sha[:])
 }
diff --git a/builtin/providers/template/resource_test.go b/builtin/providers/template/resource_test.go
index e9d3f4207..13b6cf052 100644
--- a/builtin/providers/template/resource_test.go
+++ b/builtin/providers/template/resource_test.go
@@ -50,7 +50,6 @@ output "rendered" {
 						}
 						return nil
 					},
-					TransientResource: true,
 				},
 			},
 		})
diff --git a/helper/resource/testing.go b/helper/resource/testing.go
index a2ea003af..6832146b2 100644
--- a/helper/resource/testing.go
+++ b/helper/resource/testing.go
@@ -75,12 +75,6 @@ type TestStep struct {
 
 	// Destroy will create a destroy plan if set to true.
 	Destroy bool
-
-	// TransientResource indicates that resources created as part
-	// of this test step are temporary and might be recreated anew
-	// with every planning step. This should only be set for
-	// pseudo-resources, like the null resource or templates.
-	TransientResource bool
 }
 
 // Test performs an acceptance test on a resource.
@@ -269,7 +263,7 @@ func testStep(
 	if p, err := ctx.Plan(); err != nil {
 		return state, fmt.Errorf("Error on second follow-up plan: %s", err)
 	} else {
-		if p.Diff != nil && !p.Diff.Empty() && !step.TransientResource {
+		if p.Diff != nil && !p.Diff.Empty() {
 			return state, fmt.Errorf(
 				"After applying this step and refreshing, the plan was not empty:\n\n%s", p)
 		}