Merge pull request #31 from hashicorp/f-taint

Tainted Resources

builtin/provisioners/file/resource_provisioner.go

@@ -14,31 +14,31 @@ import (
 type ResourceProvisioner struct{}
 
 func (p *ResourceProvisioner) Apply(s *terraform.ResourceState,
-	c *terraform.ResourceConfig) (*terraform.ResourceState, error) {
+	c *terraform.ResourceConfig) error {
 	// Ensure the connection type is SSH
 	if err := helper.VerifySSH(s); err != nil {
-		return s, err
+		return err
 	}
 
 	// Get the SSH configuration
 	conf, err := helper.ParseSSHConfig(s)
 	if err != nil {
-		return s, err
+		return err
 	}
 
 	// Get the source and destination
 	sRaw := c.Config["source"]
 	src, ok := sRaw.(string)
 	if !ok {
-		return s, fmt.Errorf("Unsupported 'source' type! Must be string.")
+		return fmt.Errorf("Unsupported 'source' type! Must be string.")
 	}
 
 	dRaw := c.Config["destination"]
 	dst, ok := dRaw.(string)
 	if !ok {
-		return s, fmt.Errorf("Unsupported 'destination' type! Must be string.")
+		return fmt.Errorf("Unsupported 'destination' type! Must be string.")
 	}
-	return s, p.copyFiles(conf, src, dst)
+	return p.copyFiles(conf, src, dst)
 }
 
 func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) {

builtin/provisioners/local-exec/resource_provisioner.go

@@ -21,16 +21,16 @@ type ResourceProvisioner struct{}
 
 func (p *ResourceProvisioner) Apply(
 	s *terraform.ResourceState,
-	c *terraform.ResourceConfig) (*terraform.ResourceState, error) {
+	c *terraform.ResourceConfig) error {
 
 	// Get the command
 	commandRaw, ok := c.Config["command"]
 	if !ok {
-		return s, fmt.Errorf("local-exec provisioner missing 'command'")
+		return fmt.Errorf("local-exec provisioner missing 'command'")
 	}
 	command, ok := commandRaw.(string)
 	if !ok {
-		return s, fmt.Errorf("local-exec provisioner command must be a string")
+		return fmt.Errorf("local-exec provisioner command must be a string")
 	}
 
 	// Execute the command using a shell
@@ -51,10 +51,10 @@ func (p *ResourceProvisioner) Apply(
 
 	// Run the command to completion
 	if err := cmd.Run(); err != nil {
-		return s, fmt.Errorf("Error running command '%s': %v. Output: %s",
+		return fmt.Errorf("Error running command '%s': %v. Output: %s",
 			command, err, output.Bytes())
 	}
-	return s, nil
+	return nil
 }
 
 func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) ([]string, []error) {

builtin/provisioners/local-exec/resource_provisioner_test.go

@@ -21,8 +21,7 @@ func TestResourceProvider_Apply(t *testing.T) {
 	})
 
 	p := new(ResourceProvisioner)
-	_, err := p.Apply(nil, c)
+	if err := p.Apply(nil, c); err != nil {
-	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 

builtin/provisioners/remote-exec/resource_provisioner.go

@@ -23,22 +23,22 @@ const (
 type ResourceProvisioner struct{}
 
 func (p *ResourceProvisioner) Apply(s *terraform.ResourceState,
-	c *terraform.ResourceConfig) (*terraform.ResourceState, error) {
+	c *terraform.ResourceConfig) error {
 	// Ensure the connection type is SSH
 	if err := helper.VerifySSH(s); err != nil {
-		return s, err
+		return err
 	}
 
 	// Get the SSH configuration
 	conf, err := helper.ParseSSHConfig(s)
 	if err != nil {
-		return s, err
+		return err
 	}
 
 	// Collect the scripts
 	scripts, err := p.collectScripts(c)
 	if err != nil {
-		return s, err
+		return err
 	}
 	for _, s := range scripts {
 		defer s.Close()
@@ -46,9 +46,9 @@ func (p *ResourceProvisioner) Apply(s *terraform.ResourceState,
 
 	// Copy and execute each script
 	if err := p.runScripts(conf, scripts); err != nil {
-		return s, err
+		return err
 	}
-	return s, nil
+	return nil
 }
 
 func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) {

rpc/resource_provisioner.go

@@ -37,7 +37,7 @@ func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) ([]string, [
 
 func (p *ResourceProvisioner) Apply(
 	s *terraform.ResourceState,
-	c *terraform.ResourceConfig) (*terraform.ResourceState, error) {
+	c *terraform.ResourceConfig) error {
 	var resp ResourceProvisionerApplyResponse
 	args := &ResourceProvisionerApplyArgs{
 		State:  s,
@@ -46,13 +46,13 @@ func (p *ResourceProvisioner) Apply(
 
 	err := p.Client.Call(p.Name+".Apply", args, &resp)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	if resp.Error != nil {
 		err = resp.Error
 	}
 
-	return resp.State, err
+	return err
 }
 
 type ResourceProvisionerValidateArgs struct {
@@ -70,7 +70,6 @@ type ResourceProvisionerApplyArgs struct {
 }
 
 type ResourceProvisionerApplyResponse struct {
-	State *terraform.ResourceState
 	Error *BasicError
 }
 
@@ -83,9 +82,8 @@ type ResourceProvisionerServer struct {
 func (s *ResourceProvisionerServer) Apply(
 	args *ResourceProvisionerApplyArgs,
 	result *ResourceProvisionerApplyResponse) error {
-	state, err := s.Provisioner.Apply(args.State, args.Config)
+	err := s.Provisioner.Apply(args.State, args.Config)
 	*result = ResourceProvisionerApplyResponse{
-		State: state,
 		Error: NewBasicError(err),
 	}
 	return nil

rpc/resource_provisioner_test.go

@@ -21,14 +21,10 @@ func TestResourceProvisioner_apply(t *testing.T) {
 	}
 	provisioner := &ResourceProvisioner{Client: client, Name: name}
 
-	p.ApplyReturn = &terraform.ResourceState{
-		ID: "bob",
-	}
-
 	// Apply
 	state := &terraform.ResourceState{}
 	conf := &terraform.ResourceConfig{}
-	newState, err := provisioner.Apply(state, conf)
+	err = provisioner.Apply(state, conf)
 	if !p.ApplyCalled {
 		t.Fatal("apply should be called")
 	}
@@ -38,9 +34,6 @@ func TestResourceProvisioner_apply(t *testing.T) {
 	if err != nil {
 		t.Fatalf("bad: %#v", err)
 	}
-	if !reflect.DeepEqual(p.ApplyReturn, newState) {
-		t.Fatalf("bad: %#v", newState)
-	}
 }
 
 func TestResourceProvisioner_validate(t *testing.T) {

terraform/context.go

@@ -554,10 +554,11 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 		//
 		// Additionally, we need to be careful to not run this if there
 		// was an error during the provider apply.
+		tainted := false
 		if applyerr == nil && r.State.ID == "" && len(r.Provisioners) > 0 {
-			rs, err = c.applyProvisioners(r, rs)
+			if err := c.applyProvisioners(r, rs); err != nil {
-			if err != nil {
 				errs = append(errs, err)
+				tainted = true
 			}
 		}
 
@@ -567,6 +568,10 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 			delete(c.state.Resources, r.Id)
 		} else {
 			c.state.Resources[r.Id] = rs
+
+			if tainted {
+				c.state.Tainted[r.Id] = struct{}{}
+			}
 		}
 		c.sl.Unlock()
 
@@ -591,9 +596,7 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc {
 
 // applyProvisioners is used to run any provisioners a resource has
 // defined after the resource creation has already completed.
-func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceState, error) {
+func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) error {
-	var err error
-
 	// Store the original connection info, restore later
 	origConnInfo := rs.ConnInfo
 	defer func() {
@@ -604,13 +607,13 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt
 		// Interpolate since we may have variables that depend on the
 		// local resource.
 		if err := prov.Config.interpolate(c); err != nil {
-			return rs, err
+			return err
 		}
 
 		// Interpolate the conn info, since it may contain variables
 		connInfo := NewResourceConfig(prov.ConnInfo)
 		if err := connInfo.interpolate(c); err != nil {
-			return rs, err
+			return err
 		}
 
 		// Merge the connection information
@@ -643,12 +646,12 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt
 		rs.ConnInfo = overlay
 
 		// Invoke the Provisioner
-		rs, err = prov.Provisioner.Apply(rs, prov.Config)
+		if err := prov.Provisioner.Apply(rs, prov.Config); err != nil {
-		if err != nil {
+			return err
-			return rs, err
 		}
 	}
-	return rs, nil
+
+	return nil
 }
 
 func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc {
@@ -678,7 +681,13 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc {
 			// 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)
+			state := r.State
+			if r.Tainted {
+				// If we're tainted, we pretend to create a new thing.
+				state = new(ResourceState)
+				state.Type = r.State.Type
+			}
+			diff, err = r.Provider.Diff(state, r.Config)
 			if err != nil {
 				return err
 			}
@@ -688,6 +697,11 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc {
 			diff = new(ResourceDiff)
 		}
 
+		if r.Tainted {
+			// Tainted resources must also be destroyed
+			diff.Destroy = true
+		}
+
 		if diff.RequiresNew() && r.State.ID != "" {
 			// This will also require a destroy
 			diff.Destroy = true

terraform/context_test.go

@@ -414,12 +414,13 @@ func TestContextApply_Provisioner_compute(t *testing.T) {
 	pr := testProvisioner()
 	p.ApplyFn = testApplyFn
 	p.DiffFn = testDiffFn
-	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error {
 		val, ok := c.Config["foo"]
 		if !ok || val != "computed_dynamical" {
 			t.Fatalf("bad value for foo: %v %#v", val, c)
 		}
-		return rs, nil
+
+		return nil
 	}
 	ctx := testContext(t, &ContextOpts{
 		Config: c,
@@ -455,6 +456,46 @@ func TestContextApply_Provisioner_compute(t *testing.T) {
 	}
 }
 
+func TestContextApply_provisionerFail(t *testing.T) {
+	c := testConfig(t, "apply-provisioner-fail")
+	p := testProvider("aws")
+	pr := testProvisioner()
+	p.ApplyFn = testApplyFn
+	p.DiffFn = testDiffFn
+
+	pr.ApplyFn = func(*ResourceState, *ResourceConfig) error {
+		return fmt.Errorf("EXPLOSION")
+	}
+
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		Provisioners: map[string]ResourceProvisionerFactory{
+			"shell": testProvisionerFuncFixed(pr),
+		},
+		Variables: map[string]string{
+			"value": "1",
+		},
+	})
+
+	if _, err := ctx.Plan(nil); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	state, err := ctx.Apply()
+	if err == nil {
+		t.Fatal("should error")
+	}
+
+	actual := strings.TrimSpace(state.String())
+	expected := strings.TrimSpace(testTerraformApplyProvisionerFailStr)
+	if actual != expected {
+		t.Fatalf("bad: \n%s", actual)
+	}
+}
+
 func TestContextApply_outputDiffVars(t *testing.T) {
 	c := testConfig(t, "apply-good")
 	p := testProvider("aws")
@@ -527,7 +568,7 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) {
 	}
 	p.DiffFn = testDiffFn
 
-	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+	pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error {
 		conn := rs.ConnInfo
 		if conn["type"] != "telnet" {
 			t.Fatalf("Bad: %#v", conn)
@@ -544,7 +585,8 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) {
 		if conn["pass"] != "test" {
 			t.Fatalf("Bad: %#v", conn)
 		}
-		return rs, nil
+
+		return nil
 	}
 
 	ctx := testContext(t, &ContextOpts{
@@ -1415,6 +1457,44 @@ func TestContextPlan_state(t *testing.T) {
 	}
 }
 
+func TestContextPlan_taint(t *testing.T) {
+	c := testConfig(t, "plan-taint")
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	s := &State{
+		Resources: map[string]*ResourceState{
+			"aws_instance.foo": &ResourceState{
+				ID:         "bar",
+				Type:       "aws_instance",
+				Attributes: map[string]string{"num": "2"},
+			},
+			"aws_instance.bar": &ResourceState{
+				ID:   "baz",
+				Type: "aws_instance",
+			},
+		},
+		Tainted: map[string]struct{}{"aws_instance.bar": struct{}{}},
+	}
+	ctx := testContext(t, &ContextOpts{
+		Config: c,
+		Providers: map[string]ResourceProviderFactory{
+			"aws": testProviderFuncFixed(p),
+		},
+		State: s,
+	})
+
+	plan, err := ctx.Plan(nil)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	actual := strings.TrimSpace(plan.String())
+	expected := strings.TrimSpace(testTerraformPlanTaintStr)
+	if actual != expected {
+		t.Fatalf("bad:\n%s", actual)
+	}
+}
+
 func TestContextRefresh(t *testing.T) {
 	p := testProvider("aws")
 	c := testConfig(t, "refresh-basic")

terraform/graph.go

@@ -178,6 +178,12 @@ func graphAddConfigResources(
 				index = i
 			}
 
+			// Determine if this resource is tainted
+			tainted := false
+			if s != nil && s.Tainted != nil {
+				_, tainted = s.Tainted[r.Id()]
+			}
+
 			var state *ResourceState
 			if s != nil {
 				state = s.Resources[name]
@@ -212,6 +218,7 @@ func graphAddConfigResources(
 						Id:      name,
 						State:   state,
 						Config:  NewResourceConfig(r.RawConfig),
+						Tainted: tainted,
 					},
 				},
 			}

terraform/resource.go

@@ -31,6 +31,7 @@ type Resource struct {
 	Provider     ResourceProvider
 	State        *ResourceState
 	Provisioners []*ResourceProvisionerConfig
+	Tainted      bool
 }
 
 // Vars returns the mapping of variables that should be replaced in

terraform/resource_provisioner.go

@@ -20,7 +20,7 @@ type ResourceProvisioner interface {
 	// resource state along with an error. Instead of a diff, the ResourceConfig
 	// is provided since provisioners only run after a resource has been
 	// newly created.
-	Apply(*ResourceState, *ResourceConfig) (*ResourceState, error)
+	Apply(*ResourceState, *ResourceConfig) error
 }
 
 // ResourceProvisionerFactory is a function type that creates a new instance

terraform/resource_provisioner_mock.go

@@ -9,8 +9,7 @@ type MockResourceProvisioner struct {
 	ApplyCalled      bool
 	ApplyState       *ResourceState
 	ApplyConfig      *ResourceConfig
-	ApplyFn          func(*ResourceState, *ResourceConfig) (*ResourceState, error)
+	ApplyFn          func(*ResourceState, *ResourceConfig) error
-	ApplyReturn      *ResourceState
 	ApplyReturnError error
 
 	ValidateCalled       bool
@@ -29,12 +28,12 @@ func (p *MockResourceProvisioner) Validate(c *ResourceConfig) ([]string, []error
 	return p.ValidateReturnWarns, p.ValidateReturnErrors
 }
 
-func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) (*ResourceState, error) {
+func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) error {
 	p.ApplyCalled = true
 	p.ApplyState = state
 	p.ApplyConfig = c
 	if p.ApplyFn != nil {
 		return p.ApplyFn(state, c)
 	}
-	return p.ApplyReturn, p.ApplyReturnError
+	return p.ApplyReturnError
 }

terraform/state.go

@@ -18,13 +18,20 @@ import (
 type State struct {
 	Outputs   map[string]string
 	Resources map[string]*ResourceState
+	Tainted   map[string]struct{}
 
 	once sync.Once
 }
 
 func (s *State) init() {
 	s.once.Do(func() {
+		if s.Resources == nil {
 			s.Resources = make(map[string]*ResourceState)
+		}
+
+		if s.Tainted == nil {
+			s.Tainted = make(map[string]struct{})
+		}
 	})
 }
 
@@ -97,7 +104,12 @@ func (s *State) String() string {
 			id = "<not created>"
 		}
 
-		buf.WriteString(fmt.Sprintf("%s:\n", k))
+		taintStr := ""
+		if _, ok := s.Tainted[k]; ok {
+			taintStr = " (tainted)"
+		}
+
+		buf.WriteString(fmt.Sprintf("%s:%s\n", k, taintStr))
 		buf.WriteString(fmt.Sprintf("  ID = %s\n", id))
 
 		attrKeys := make([]string, 0, len(rs.Attributes))

terraform/terraform_test.go

@@ -130,6 +130,15 @@ aws_instance.foo:
   type = aws_instance
 `
 
+const testTerraformApplyProvisionerFailStr = `
+aws_instance.bar: (tainted)
+  ID = foo
+aws_instance.foo:
+  ID = foo
+  num = 2
+  type = aws_instance
+`
+
 const testTerraformApplyDestroyStr = `
 <no state>
 `
@@ -402,3 +411,19 @@ STATE:
 aws_instance.foo:
   ID = bar
 `
+
+const testTerraformPlanTaintStr = `
+DIFF:
+
+DESTROY: aws_instance.bar
+  foo:  "" => "2"
+  type: "" => "aws_instance"
+
+STATE:
+
+aws_instance.bar: (tainted)
+  ID = baz
+aws_instance.foo:
+  ID = bar
+  num = 2
+`

terraform/test-fixtures/apply-provisioner-fail/main.tf

@@ -0,0 +1,7 @@
+resource "aws_instance" "foo" {
+    num = "2"
+}
+
+resource "aws_instance" "bar" {
+    provisioner "shell" {}
+}

terraform/test-fixtures/plan-taint/main.tf

@@ -0,0 +1,7 @@
+resource "aws_instance" "foo" {
+    num = "2"
+}
+
+resource "aws_instance" "bar" {
+    foo = "${aws_instance.foo.num}"
+}