terraform: manually interpolate variables in walker functions

This avoids issues where we were interpolating when we didn't need to

terraform/context.go

@@ -416,6 +416,11 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 		}
 
 		if !diff.Destroy {
+			// Since we need the configuration, interpolate the variables
+			if err := r.Config.interpolate(c); err != nil {
+				return err
+			}
+
 			var err error
 			diff, err = r.Provider.Diff(r.State, r.Config)
 			if err != nil {
@@ -503,9 +508,13 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc {
 			// This is an orphan (no config), so we mark it to be destroyed
 			diff = &ResourceDiff{Destroy: true}
 		} else {
-			log.Printf("[DEBUG] %s: Executing diff", r.Id)
+			// Make sure the configuration is interpolated
+			if err := r.Config.interpolate(c); err != nil {
+				return err
+			}
 
 			// Get a diff from the newest state
+			log.Printf("[DEBUG] %s: Executing diff", r.Id)
 			var err error
 			diff, err = r.Provider.Diff(r.State, r.Config)
 			if err != nil {
@@ -703,23 +712,12 @@ func (c *Context) genericWalkFn(cb genericWalkFunc) depgraph.WalkFunc {
 
 		rn := n.Meta.(*GraphNodeResource)
 
-		if rn.Config != nil {
-			if err := c.computeVars(rn.Config.RawConfig); err != nil {
-				panic(fmt.Sprintf("Interpolate error: %s", err))
-			}
-
-			// Force the config to be set later
-			rn.Resource.Config = nil
-		}
-
 		// Make sure that at least some resource configuration is set
 		if !rn.Orphan {
-			if rn.Resource.Config == nil {
-				if rn.Config == nil {
-					rn.Resource.Config = new(ResourceConfig)
-				} else {
-					rn.Resource.Config = NewResourceConfig(rn.Config.RawConfig)
-				}
+			if rn.Config == nil {
+				rn.Resource.Config = new(ResourceConfig)
+			} else {
+				rn.Resource.Config = NewResourceConfig(rn.Config.RawConfig)
 			}
 		} else {
 			rn.Resource.Config = nil

terraform/context_test.go

@@ -1115,6 +1115,50 @@ func TestContextRefresh_state(t *testing.T) {
 	}
 }
 
+func TestContextRefresh_vars(t *testing.T) {
+	p := testProvider("aws")
+	c := testConfig(t, "refresh-vars")
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: &State{
+			Resources: map[string]*ResourceState{
+				"aws_instance.web": &ResourceState{
+					ID:   "foo",
+					Type: "aws_instance",
+				},
+			},
+		},
+	})
+
+	p.RefreshFn = nil
+	p.RefreshReturn = &ResourceState{
+		ID: "foo",
+	}
+
+	s, err := ctx.Refresh()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if !p.RefreshCalled {
+		t.Fatal("refresh should be called")
+	}
+	if p.RefreshState.ID != "foo" {
+		t.Fatalf("bad: %#v", p.RefreshState)
+	}
+	if !reflect.DeepEqual(s.Resources["aws_instance.web"], p.RefreshReturn) {
+		t.Fatalf("bad: %#v", s.Resources["aws_instance.web"])
+	}
+
+	for _, r := range s.Resources {
+		if r.Type == "" {
+			t.Fatalf("no type: %#v", r)
+		}
+	}
+}
+
 func testContext(t *testing.T, opts *ContextOpts) *Context {
 	return NewContext(opts)
 }

terraform/resource.go

@@ -2,6 +2,11 @@ package terraform
 
 import (
 	"fmt"
+	"reflect"
+	"strconv"
+	"strings"
+
+	"github.com/hashicorp/terraform/config"
 )
 
 // Resource encapsulates a resource, its configuration, its provider,
@@ -29,3 +34,109 @@ func (r *Resource) Vars() map[string]string {
 
 	return vars
 }
+
+// ResourceConfig holds the configuration given for a resource. This is
+// done instead of a raw `map[string]interface{}` type so that rich
+// methods can be added to it to make dealing with it easier.
+type ResourceConfig struct {
+	ComputedKeys []string
+	Raw          map[string]interface{}
+	Config       map[string]interface{}
+
+	raw *config.RawConfig
+}
+
+// NewResourceConfig creates a new ResourceConfig from a config.RawConfig.
+func NewResourceConfig(c *config.RawConfig) *ResourceConfig {
+	result := &ResourceConfig{raw: c}
+	result.interpolate(nil)
+	return result
+}
+
+// CheckSet checks that the given list of configuration keys is
+// properly set. If not, errors are returned for each unset key.
+//
+// This is useful to be called in the Validate method of a ResourceProvider.
+func (c *ResourceConfig) CheckSet(keys []string) []error {
+	var errs []error
+
+	for _, k := range keys {
+		if !c.IsSet(k) {
+			errs = append(errs, fmt.Errorf("%s must be set", k))
+		}
+	}
+
+	return errs
+}
+
+// Get looks up a configuration value by key and returns the value.
+//
+// The second return value is true if the get was successful. Get will
+// not succeed if the value is being computed.
+func (c *ResourceConfig) Get(k string) (interface{}, bool) {
+	parts := strings.Split(k, ".")
+
+	var current interface{} = c.Raw
+	for _, part := range parts {
+		if current == nil {
+			return nil, false
+		}
+
+		cv := reflect.ValueOf(current)
+		switch cv.Kind() {
+		case reflect.Map:
+			v := cv.MapIndex(reflect.ValueOf(part))
+			if !v.IsValid() {
+				return nil, false
+			}
+			current = v.Interface()
+		case reflect.Slice:
+			i, err := strconv.ParseInt(part, 0, 0)
+			if err != nil {
+				return nil, false
+			}
+			current = cv.Index(int(i)).Interface()
+		default:
+			panic(fmt.Sprintf("Unknown kind: %s", cv.Kind()))
+		}
+	}
+
+	return current, true
+}
+
+// IsSet checks if the key in the configuration is set. A key is set if
+// it has a value or the value is being computed (is unknown currently).
+//
+// This function should be used rather than checking the keys of the
+// raw configuration itself, since a key may be omitted from the raw
+// configuration if it is being computed.
+func (c *ResourceConfig) IsSet(k string) bool {
+	if c == nil {
+		return false
+	}
+
+	for _, ck := range c.ComputedKeys {
+		if ck == k {
+			return true
+		}
+	}
+
+	if _, ok := c.Get(k); ok {
+		return true
+	}
+
+	return false
+}
+
+func (c *ResourceConfig) interpolate(ctx *Context) error {
+	if ctx != nil {
+		if err := ctx.computeVars(c.raw); err != nil {
+			return err
+		}
+	}
+
+	c.ComputedKeys = c.raw.UnknownKeys()
+	c.Raw = c.raw.Raw
+	c.Config = c.raw.Config()
+	return nil
+}

terraform/resource_provider.go

@@ -1,14 +1,5 @@
 package terraform
 
-import (
-	"fmt"
-	"reflect"
-	"strconv"
-	"strings"
-
-	"github.com/hashicorp/terraform/config"
-)
-
 // ResourceProvider is an interface that must be implemented by any
 // resource provider: the thing that creates and manages the resources in
 // a Terraform configuration.
@@ -69,15 +60,6 @@ type ResourceProvider interface {
 	Refresh(*ResourceState) (*ResourceState, error)
 }
 
-// ResourceConfig holds the configuration given for a resource. This is
-// done instead of a raw `map[string]interface{}` type so that rich
-// methods can be added to it to make dealing with it easier.
-type ResourceConfig struct {
-	ComputedKeys []string
-	Raw          map[string]interface{}
-	Config       map[string]interface{}
-}
-
 // ResourceType is a type of resource that a resource provider can manage.
 type ResourceType struct {
 	Name string
@@ -87,90 +69,6 @@ type ResourceType struct {
 // of a resource provider.
 type ResourceProviderFactory func() (ResourceProvider, error)
 
-// NewResourceConfig creates a new ResourceConfig from a config.RawConfig.
-func NewResourceConfig(c *config.RawConfig) *ResourceConfig {
-	return &ResourceConfig{
-		ComputedKeys: c.UnknownKeys(),
-		Raw:          c.Raw,
-		Config:       c.Config(),
-	}
-}
-
-// CheckSet checks that the given list of configuration keys is
-// properly set. If not, errors are returned for each unset key.
-//
-// This is useful to be called in the Validate method of a ResourceProvider.
-func (c *ResourceConfig) CheckSet(keys []string) []error {
-	var errs []error
-
-	for _, k := range keys {
-		if !c.IsSet(k) {
-			errs = append(errs, fmt.Errorf("%s must be set", k))
-		}
-	}
-
-	return errs
-}
-
-// Get looks up a configuration value by key and returns the value.
-//
-// The second return value is true if the get was successful. Get will
-// not succeed if the value is being computed.
-func (c *ResourceConfig) Get(k string) (interface{}, bool) {
-	parts := strings.Split(k, ".")
-
-	var current interface{} = c.Raw
-	for _, part := range parts {
-		if current == nil {
-			return nil, false
-		}
-
-		cv := reflect.ValueOf(current)
-		switch cv.Kind() {
-		case reflect.Map:
-			v := cv.MapIndex(reflect.ValueOf(part))
-			if !v.IsValid() {
-				return nil, false
-			}
-			current = v.Interface()
-		case reflect.Slice:
-			i, err := strconv.ParseInt(part, 0, 0)
-			if err != nil {
-				return nil, false
-			}
-			current = cv.Index(int(i)).Interface()
-		default:
-			panic(fmt.Sprintf("Unknown kind: %s", cv.Kind()))
-		}
-	}
-
-	return current, true
-}
-
-// IsSet checks if the key in the configuration is set. A key is set if
-// it has a value or the value is being computed (is unknown currently).
-//
-// This function should be used rather than checking the keys of the
-// raw configuration itself, since a key may be omitted from the raw
-// configuration if it is being computed.
-func (c *ResourceConfig) IsSet(k string) bool {
-	if c == nil {
-		return false
-	}
-
-	for _, ck := range c.ComputedKeys {
-		if ck == k {
-			return true
-		}
-	}
-
-	if _, ok := c.Get(k); ok {
-		return true
-	}
-
-	return false
-}
-
 func ProviderSatisfies(p ResourceProvider, n string) bool {
 	for _, rt := range p.Resources() {
 		if rt.Name == n {

terraform/test-fixtures/refresh-vars/main.tf

@@ -0,0 +1,5 @@
+resource "aws_instance" "web" {}
+
+resource "aws_instance" "db" {
+  ami = "${aws_instance.web.id}"
+}