Merge pull request #19222 from hashicorp/jbardin/timeout

provider timeouts

builtin/providers/test/provider.go

@@ -20,6 +20,7 @@ func Provider() terraform.ResourceProvider {
 			"test_resource":                  testResource(),
 			"test_resource_gh12183":          testResourceGH12183(),
 			"test_resource_with_custom_diff": testResourceCustomDiff(),
+			"test_resource_timeout":          testResourceTimeout(),
 		},
 		DataSourcesMap: map[string]*schema.Resource{
 			"test_data_source":    testDataSource(),

builtin/providers/test/resource_timeout.go

@@ -0,0 +1,120 @@
+package test
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/hashicorp/terraform/helper/schema"
+)
+
+func testResourceTimeout() *schema.Resource {
+	return &schema.Resource{
+		Create: testResourceTimeoutCreate,
+		Read:   testResourceTimeoutRead,
+		Update: testResourceTimeoutUpdate,
+		Delete: testResourceTimeoutDelete,
+
+		Timeouts: &schema.ResourceTimeout{
+			Create: schema.DefaultTimeout(time.Second),
+			Update: schema.DefaultTimeout(time.Second),
+			Delete: schema.DefaultTimeout(time.Second),
+		},
+
+		Importer: &schema.ResourceImporter{
+			State: schema.ImportStatePassthrough,
+		},
+
+		Schema: map[string]*schema.Schema{
+			"create_delay": {
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"read_delay": {
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"update_delay": {
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"delete_delay": {
+				Type:     schema.TypeString,
+				Optional: true,
+			},
+		},
+	}
+}
+
+func testResourceTimeoutCreate(d *schema.ResourceData, meta interface{}) error {
+	delayString := d.Get("create_delay").(string)
+	var delay time.Duration
+	var err error
+	if delayString != "" {
+		delay, err = time.ParseDuration(delayString)
+		if err != nil {
+			return err
+		}
+	}
+
+	if delay > d.Timeout(schema.TimeoutCreate) {
+		return fmt.Errorf("timeout while creating resource")
+	}
+
+	d.SetId("testId")
+
+	return testResourceRead(d, meta)
+}
+
+func testResourceTimeoutRead(d *schema.ResourceData, meta interface{}) error {
+	delayString := d.Get("read_delay").(string)
+	var delay time.Duration
+	var err error
+	if delayString != "" {
+		delay, err = time.ParseDuration(delayString)
+		if err != nil {
+			return err
+		}
+	}
+
+	if delay > d.Timeout(schema.TimeoutRead) {
+		return fmt.Errorf("timeout while reading resource")
+	}
+
+	return nil
+}
+
+func testResourceTimeoutUpdate(d *schema.ResourceData, meta interface{}) error {
+	delayString := d.Get("update_delay").(string)
+	var delay time.Duration
+	var err error
+	if delayString != "" {
+		delay, err = time.ParseDuration(delayString)
+		if err != nil {
+			return err
+		}
+	}
+
+	if delay > d.Timeout(schema.TimeoutUpdate) {
+		return fmt.Errorf("timeout while updating resource")
+	}
+	return nil
+}
+
+func testResourceTimeoutDelete(d *schema.ResourceData, meta interface{}) error {
+	delayString := d.Get("delete_delay").(string)
+	var delay time.Duration
+	var err error
+	if delayString != "" {
+		delay, err = time.ParseDuration(delayString)
+		if err != nil {
+			return err
+		}
+	}
+
+	if delay > d.Timeout(schema.TimeoutDelete) {
+		return fmt.Errorf("timeout while deleting resource")
+	}
+
+	d.SetId("")
+	return nil
+}

builtin/providers/test/resource_timeout_test.go

@@ -0,0 +1,91 @@
+package test
+
+import (
+	"regexp"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/terraform/helper/resource"
+)
+
+func TestResourceTimeout_create(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+	create_delay = "2s"
+	timeouts {
+		create = "1s"
+	}
+}
+				`),
+				ExpectError: regexp.MustCompile("timeout while creating resource"),
+			},
+		},
+	})
+}
+func TestResourceTimeout_update(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+	update_delay = "1s"
+	timeouts {
+		update = "1s"
+	}
+}
+				`),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+	update_delay = "2s"
+	timeouts {
+		update = "1s"
+	}
+}
+				`),
+				ExpectError: regexp.MustCompile("timeout while updating resource"),
+			},
+		},
+	})
+}
+
+func TestResourceTimeout_read(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+}
+				`),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+	read_delay = "30m"
+}
+				`),
+				ExpectError: regexp.MustCompile("timeout while reading resource"),
+			},
+			// we need to remove the read_delay so that the resource can be
+			// destroyed in the final step, but expect an error here from the
+			// pre-existing delay.
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_timeout" "foo" {
+}
+				`),
+				ExpectError: regexp.MustCompile("timeout while reading resource"),
+			},
+		},
+	})
+}

helper/plugin/grpc_provider.go

@@ -412,20 +412,22 @@ func (s *GRPCProviderServer) ReadResource(_ context.Context, req *proto.ReadReso
 	// helper/schema should always copy the ID over, but do it again just to be safe
 	newInstanceState.Attributes["id"] = newInstanceState.ID
 
-	newConfigVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType())
+	newStateVal, err := hcl2shim.HCL2ValueFromFlatmap(newInstanceState.Attributes, block.ImpliedType())
 	if err != nil {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
 		return resp, nil
 	}
 
-	newConfigMP, err := msgpack.Marshal(newConfigVal, block.ImpliedType())
+	newStateVal = copyTimeoutValues(newStateVal, stateVal)
+
+	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
 	if err != nil {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
 		return resp, nil
 	}
 
 	resp.NewState = &proto.DynamicValue{
-		Msgpack: newConfigMP,
+		Msgpack: newStateMP,
 	}
 
 	return resp, nil
@@ -461,9 +463,10 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
 			return resp, nil
 		}
 	}
+
 	priorState.Meta = priorPrivate
 
-	// turn the propsed state into a legacy configuration
+	// turn the proposed state into a legacy configuration
 	config := terraform.NewResourceConfigShimmed(proposedNewStateVal, block)
 
 	diff, err := s.provider.SimpleDiff(info, priorState, config)
@@ -488,6 +491,8 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
 		return resp, nil
 	}
 
+	plannedStateVal = copyTimeoutValues(plannedStateVal, proposedNewStateVal)
+
 	plannedMP, err := msgpack.Marshal(plannedStateVal, block.ImpliedType())
 	if err != nil {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@@ -498,12 +503,14 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
 	}
 
 	// the Meta field gets encoded into PlannedPrivate
-	plannedPrivate, err := json.Marshal(diff.Meta)
-	if err != nil {
-		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
-		return resp, nil
+	if diff.Meta != nil {
+		plannedPrivate, err := json.Marshal(diff.Meta)
+		if err != nil {
+			resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
+			return resp, nil
+		}
+		resp.PlannedPrivate = plannedPrivate
 	}
-	resp.PlannedPrivate = plannedPrivate
 
 	// collect the attributes that require instance replacement, and convert
 	// them to cty.Paths.
@@ -594,7 +601,10 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 			Meta:       make(map[string]interface{}),
 		}
 	}
-	diff.Meta = private
+
+	if private != nil {
+		diff.Meta = private
+	}
 
 	newInstanceState, err := s.provider.Apply(info, priorState, diff)
 	if err != nil {
@@ -614,6 +624,8 @@ func (s *GRPCProviderServer) ApplyResourceChange(_ context.Context, req *proto.A
 		}
 	}
 
+	newStateVal = copyTimeoutValues(newStateVal, plannedStateVal)
+
 	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
 	if err != nil {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@@ -726,6 +738,8 @@ func (s *GRPCProviderServer) ReadDataSource(_ context.Context, req *proto.ReadDa
 		return resp, nil
 	}
 
+	newStateVal = copyTimeoutValues(newStateVal, configVal)
+
 	newStateMP, err := msgpack.Marshal(newStateVal, block.ImpliedType())
 	if err != nil {
 		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
@@ -770,3 +784,28 @@ func pathToAttributePath(path cty.Path) *proto.AttributePath {
 
 	return &proto.AttributePath{Steps: steps}
 }
+
+// helper/schema throws away timeout values from the config and stores them in
+// the Private/Meta fields. we need to copy those values into the planned state
+// so that core doesn't see a perpetual diff with the timeout block.
+func copyTimeoutValues(to cty.Value, from cty.Value) cty.Value {
+	// if `from` is null, then there are no attributes, and if `to` is null we
+	// are planning to remove it altogether.
+	if from.IsNull() || to.IsNull() {
+		return to
+	}
+
+	fromAttrs := from.AsValueMap()
+	timeouts, ok := fromAttrs[schema.TimeoutsConfigKey]
+
+	// no timeouts to copy
+	// timeouts shouldn't be unknown, but don't copy possibly invalid values
+	if !ok || timeouts.IsNull() || !timeouts.IsWhollyKnown() {
+		return to
+	}
+
+	toAttrs := to.AsValueMap()
+	toAttrs[schema.TimeoutsConfigKey] = timeouts
+
+	return cty.ObjectVal(toAttrs)
+}

helper/plugin/grpc_provider_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -388,8 +389,8 @@ func TestApplyResourceChange(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// A propsed state with only the ID unknown will produce a nil diff, and
-	// should return the propsed state value.
+	// A proposed state with only the ID unknown will produce a nil diff, and
+	// should return the proposed state value.
 	plannedVal, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{
 		"id": cty.UnknownVal(cty.String),
 	}))
@@ -595,3 +596,44 @@ func TestPrepareProviderConfig(t *testing.T) {
 		})
 	}
 }
+
+func TestGetSchemaTimeouts(t *testing.T) {
+	r := &schema.Resource{
+		SchemaVersion: 4,
+		Timeouts: &schema.ResourceTimeout{
+			Create:  schema.DefaultTimeout(time.Second),
+			Read:    schema.DefaultTimeout(2 * time.Second),
+			Update:  schema.DefaultTimeout(3 * time.Second),
+			Default: schema.DefaultTimeout(10 * time.Second),
+		},
+		Schema: map[string]*schema.Schema{
+			"foo": {
+				Type:     schema.TypeInt,
+				Optional: true,
+			},
+		},
+	}
+
+	// verify that the timeouts appear in the schema as defined
+	block := r.CoreConfigSchema()
+	timeoutsBlock := block.BlockTypes["timeouts"]
+	if timeoutsBlock == nil {
+		t.Fatal("missing timeouts in schema")
+	}
+
+	if timeoutsBlock.Attributes["create"] == nil {
+		t.Fatal("missing create timeout in schema")
+	}
+	if timeoutsBlock.Attributes["read"] == nil {
+		t.Fatal("missing read timeout in schema")
+	}
+	if timeoutsBlock.Attributes["update"] == nil {
+		t.Fatal("missing update timeout in schema")
+	}
+	if d := timeoutsBlock.Attributes["delete"]; d != nil {
+		t.Fatalf("unexpected delete timeout in schema: %#v", d)
+	}
+	if timeoutsBlock.Attributes["default"] == nil {
+		t.Fatal("missing default timeout in schema")
+	}
+}

helper/resource/testing_config.go

@@ -77,7 +77,7 @@ func testStep(opts terraform.ContextOpts, state *terraform.State, step TestStep,
 		}
 
 		// We need to keep a copy of the state prior to destroying
-		// such that destroy steps can verify their behaviour in the check
+		// such that destroy steps can verify their behavior in the check
 		// function
 		stateBeforeApplication := state.DeepCopy()
 

helper/schema/core_schema.go

@@ -172,6 +172,57 @@ func (r *Resource) CoreConfigSchema() *configschema.Block {
 		}
 	}
 
+	_, timeoutsAttr := block.Attributes[TimeoutsConfigKey]
+	_, timeoutsBlock := block.BlockTypes[TimeoutsConfigKey]
+
+	// Insert configured timeout values into the schema, as long as the schema
+	// didn't define anything else by that name.
+	if r.Timeouts != nil && !timeoutsAttr && !timeoutsBlock {
+		timeouts := configschema.Block{
+			Attributes: map[string]*configschema.Attribute{},
+		}
+
+		if r.Timeouts.Create != nil {
+			timeouts.Attributes[TimeoutCreate] = &configschema.Attribute{
+				Type:     cty.String,
+				Optional: true,
+			}
+		}
+
+		if r.Timeouts.Read != nil {
+			timeouts.Attributes[TimeoutRead] = &configschema.Attribute{
+				Type:     cty.String,
+				Optional: true,
+			}
+		}
+
+		if r.Timeouts.Update != nil {
+			timeouts.Attributes[TimeoutUpdate] = &configschema.Attribute{
+				Type:     cty.String,
+				Optional: true,
+			}
+		}
+
+		if r.Timeouts.Delete != nil {
+			timeouts.Attributes[TimeoutDelete] = &configschema.Attribute{
+				Type:     cty.String,
+				Optional: true,
+			}
+		}
+
+		if r.Timeouts.Default != nil {
+			timeouts.Attributes[TimeoutDefault] = &configschema.Attribute{
+				Type:     cty.String,
+				Optional: true,
+			}
+		}
+
+		block.BlockTypes[TimeoutsConfigKey] = &configschema.NestedBlock{
+			Nesting: configschema.NestingSingle,
+			Block:   timeouts,
+		}
+	}
+
 	return block
 }
 

terraform/resource.go

@@ -276,7 +276,7 @@ func newResourceConfigShimmedComputedKeys(obj cty.Value, schema *configschema.Bl
 		}
 
 		blockVal := obj.GetAttr(typeName)
-		if !blockVal.IsKnown() || blockVal.IsNull() {
+		if blockVal.IsNull() || !blockVal.IsKnown() {
 			continue
 		}