Skip to content

Commit

Permalink
provider/template: don't diff when there's no diff
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
phinze committed May 11, 2015
1 parent a268cc3 commit a96a337
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 24 deletions.
48 changes: 32 additions & 16 deletions builtin/providers/template/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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[:])
}
1 change: 0 additions & 1 deletion builtin/providers/template/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ output "rendered" {
}
return nil
},
TransientResource: true,
},
},
})
Expand Down
8 changes: 1 addition & 7 deletions helper/resource/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit a96a337

Please sign in to comment.