From e38a5a769d4b0f072618ade958bf8c3a83287dec Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 30 Oct 2018 12:59:45 -0400 Subject: [PATCH] copy timouts into plan and apply state helper/schema will remove "timeouts" from the config, and stash them in the diff.Meta map. Terraform sees "timeouts" as a regular config block, so needs them to be present in the state in order to not show a diff. Have the GRPCProviderServer shim copy all timeout values into any state it returns to provide consistent diffs in core. --- helper/plugin/grpc_provider.go | 57 ++++++++++++++++++++++++++----- helper/resource/testing_config.go | 2 +- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index 16885bc76..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,6 +463,7 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl return resp, nil } } + priorState.Meta = priorPrivate // turn the proposed state into a legacy configuration @@ -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/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()