diff --git a/builtin/providers/test/provider.go b/builtin/providers/test/provider.go index 6c71bfb18..1f005b24f 100644 --- a/builtin/providers/test/provider.go +++ b/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(), diff --git a/builtin/providers/test/resource_timeout.go b/builtin/providers/test/resource_timeout.go new file mode 100644 index 000000000..cf32bcaf5 --- /dev/null +++ b/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 +} diff --git a/builtin/providers/test/resource_timeout_test.go b/builtin/providers/test/resource_timeout_test.go new file mode 100644 index 000000000..9edbba89a --- /dev/null +++ b/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"), + }, + }, + }) +} diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index c8f329268..136336e45 100644 --- a/helper/plugin/grpc_provider.go +++ b/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) +} diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index 5162d7d57..03f059a0b 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/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") + } +} diff --git a/helper/resource/testing_config.go b/helper/resource/testing_config.go index ea1f41298..c8cc587bd 100644 --- a/helper/resource/testing_config.go +++ b/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() diff --git a/helper/schema/core_schema.go b/helper/schema/core_schema.go index f16cf3e64..85e8b8c5b 100644 --- a/helper/schema/core_schema.go +++ b/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 } diff --git a/terraform/resource.go b/terraform/resource.go index 3e87d719b..473d73d8f 100644 --- a/terraform/resource.go +++ b/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 }