From 67a8757b69c06b2d7e007cdb3f7085e83258b8ea Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 2 Oct 2018 10:58:49 -0700 Subject: [PATCH] core: Properly handle deferral (or non-deferral) of data resources (this is a WIP prototype) --- terraform/context_plan_test.go | 45 ++-- terraform/eval_read_data.go | 245 ++++++++++++++++++++++ terraform/node_data_refresh.go | 89 ++++---- terraform/node_resource_apply_instance.go | 23 +- terraform/node_resource_plan_instance.go | 18 +- 5 files changed, 343 insertions(+), 77 deletions(-) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 1a4efde4f..0ae37733b 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1907,6 +1907,13 @@ func TestContext2Plan_computedInFunction(t *testing.T) { }, } p.DiffFn = testDiffFn + p.ReadDataSourceResponse = providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "computed": cty.ListVal([]cty.Value{ + cty.StringVal("foo"), + }), + }), + } ctx := testContext2(t, &ContextOpts{ Config: m, @@ -1917,34 +1924,26 @@ func TestContext2Plan_computedInFunction(t *testing.T) { ), }) - defer ctx.acquireRun("validate")() + diags := ctx.Validate() + assertNoErrors(t, diags) - pgb := &PlanGraphBuilder{ - Config: ctx.config, - State: ctx.state, - Components: ctx.components, - Schemas: ctx.schemas, - Targets: ctx.targets, - } - graph, _ := pgb.Build(addrs.RootModuleInstance) + state, diags := ctx.Refresh() // data resource is read in this step + assertNoErrors(t, diags) - // walk - walker := &ContextGraphWalker{ - Context: ctx, - State: ctx.state.SyncWrapper(), - Changes: ctx.changes.SyncWrapper(), - Operation: walkPlan, - StopContext: ctx.runContext, - RootVariableValues: ctx.variables, + if !p.ReadDataSourceCalled { + t.Fatalf("ReadDataSource was not called on provider during refresh; should've been called") } - watchStop, watchWait := ctx.watchStop(walker) - diags := graph.Walk(walker) - close(watchStop) - <-watchWait + p.ReadDataSourceCalled = false // reset for next call - if diags.HasErrors() { - t.Fatalf("unexpected errors: %s", diags.Err()) + t.Logf("state after refresh:\n%s", state) + + _, diags = ctx.Plan() // should do nothing with data resource in this step, since it was already read + assertNoErrors(t, diags) + + if p.ReadDataSourceCalled { + t.Fatalf("ReadDataSource was called on provider during plan; should not have been called") } + } func TestContext2Plan_computedDataCountResource(t *testing.T) { diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 5510d5ff9..fa3d86814 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -2,6 +2,7 @@ package terraform import ( "fmt" + "log" "github.com/zclconf/go-cty/cty" @@ -14,6 +15,250 @@ import ( "github.com/hashicorp/terraform/tfdiags" ) +// EvalReadData is an EvalNode implementation that deals with the main part +// of the data resource lifecycle: either actually reading from the data source +// or generating a plan to do so. +type EvalReadData struct { + Addr addrs.ResourceInstance + Config *configs.Resource + Provider *providers.Interface + ProviderAddr addrs.AbsProviderConfig + ProviderSchema **ProviderSchema + + // Planned is set when dealing with data resources that were deferred to + // the apply walk, to let us see what was planned. If this is set, the + // evaluation of the config is required to produce a wholly-known + // configuration which is consistent with the partial object included + // in this planned change. + Planned **plans.ResourceInstanceChange + + // The result from this EvalNode has a few different possibilities + // depending on the input: + // - If Planned is nil then we assume we're aiming to _produce_ the plan, + // and so the following two outcomes are possible: + // - OutputChange.Action is plans.NoOp and OutputState is the complete + // result of reading from the data source. This is the easy path. + // - OutputChange.Action is plans.Read and OutputState is a planned + // object placeholder (states.ObjectPlanned). In this case, the + // returned change must be recorded in the overral changeset and + // eventually passed to another instance of this struct during the + // apply walk. + // - If Planned is non-nil then we assume we're aiming to complete a + // planned read from an earlier plan walk. In this case the only possible + // non-error outcome is to set Output.Action (if non-nil) to a plans.NoOp + // change and put the complete resulting state in OutputState, ready to + // be saved in the overall state and used for expression evaluation. + OutputChange **plans.ResourceInstanceChange + OutputValue *cty.Value + OutputConfigValue *cty.Value + OutputState **states.ResourceInstanceObject +} + +func (n *EvalReadData) Eval(ctx EvalContext) (interface{}, error) { + absAddr := n.Addr.Absolute(ctx.Path()) + log.Printf("[TRACE] EvalReadData: working on %s", absAddr) + + if n.ProviderSchema == nil || *n.ProviderSchema == nil { + return nil, fmt.Errorf("provider schema not available for %s", n.Addr) + } + + var diags tfdiags.Diagnostics + var change *plans.ResourceInstanceChange + var configVal cty.Value + + // TODO: Do we need to handle Delete changes here? EvalReadDataDiff and + // EvalReadDataApply did, but it seems like we should handle that via a + // separate mechanism since it boils down to just deleting the object from + // the state... and we do that on every plan anyway, forcing the data + // resource to re-read. + + config := *n.Config + provider := *n.Provider + providerSchema := *n.ProviderSchema + schema := providerSchema.DataSources[n.Addr.Resource.Type] + if schema == nil { + // Should be caught during validation, so we don't bother with a pretty error here + return nil, fmt.Errorf("provider %q does not support data source %q", n.ProviderAddr.ProviderConfig.Type, n.Addr.Resource.Type) + } + + // We'll always start by evaluating the configuration. What we do after + // that will depend on the evaluation result along with what other inputs + // we were given. + objTy := schema.ImpliedType() + priorVal := cty.NullVal(objTy) // for data resources, prior is always null because we start fresh every time + + keyData := EvalDataForInstanceKey(n.Addr.Key) + + var configDiags tfdiags.Diagnostics + configVal, _, configDiags = ctx.EvaluateBlock(config.Config, schema, nil, keyData) + diags = diags.Append(configDiags) + if configDiags.HasErrors() { + return nil, diags.Err() + } + + proposedNewVal := objchange.ProposedNewObject(schema, priorVal, configVal) + + // If our configuration contains any unknown values then we must defer the + // read to the apply phase by producing a "Read" change for this resource, + // and a placeholder value for it in the state. + if !configVal.IsWhollyKnown() { + // If the configuration is still unknown when we're applying a planned + // change then that indicates a bug in Terraform, since we should have + // everything resolved by now. + if n.Planned != nil && *n.Planned != nil { + return nil, fmt.Errorf( + "configuration for %s still contains unknown values during apply (this is a bug in Terraform; please report it!)", + absAddr, + ) + } + log.Printf("[TRACE] EvalReadData: %s configuration not fully known yet, so deferring to apply phase", absAddr) + + err := ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(absAddr, states.CurrentGen, priorVal, proposedNewVal) + }) + if err != nil { + return nil, err + } + + change = &plans.ResourceInstanceChange{ + Addr: absAddr, + ProviderAddr: n.ProviderAddr, + Change: plans.Change{ + Action: plans.Read, + Before: priorVal, + After: proposedNewVal, + }, + } + + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(absAddr, states.CurrentGen, change.Action, priorVal, proposedNewVal) + }) + if err != nil { + return nil, err + } + + if n.OutputChange != nil { + *n.OutputChange = change + } + if n.OutputValue != nil { + *n.OutputValue = change.After + } + if n.OutputConfigValue != nil { + *n.OutputConfigValue = configVal + } + if n.OutputState != nil { + state := &states.ResourceInstanceObject{ + Value: change.After, + Status: states.ObjectPlanned, // because the partial value in the plan must be used for now + } + *n.OutputState = state + } + + return nil, diags.ErrWithWarnings() + } + + if n.Planned != nil && *n.Planned != nil && (*n.Planned).Action != plans.Read { + // If any other action gets in here then that's always a bug; this + // EvalNode only deals with reading. + return nil, fmt.Errorf( + "invalid action %s for %s: only Read is supported (this is a bug in Terraform; please report it!)", + (*n.Planned).Action, absAddr, + ) + } + + // If we get down here then our configuration is complete and we're read + // to actually call the provider to read the data. + log.Printf("[TRACE] EvalReadData: %s configuration is complete, so reading from provider", absAddr) + + err := ctx.Hook(func(h Hook) (HookAction, error) { + // We don't have a state yet, so we'll just give the hook an + // empty one to work with. + return h.PreRefresh(absAddr, states.CurrentGen, cty.NullVal(cty.DynamicPseudoType)) + }) + if err != nil { + return nil, err + } + + resp := provider.ReadDataSource(providers.ReadDataSourceRequest{ + TypeName: n.Addr.Resource.Type, + Config: configVal, + }) + diags = diags.Append(resp.Diagnostics.InConfigBody(n.Config.Config)) + if diags.HasErrors() { + return nil, diags.Err() + } + newVal := resp.State + if newVal == cty.NilVal { + // This can happen with incompletely-configured mocks. We'll allow it + // and treat it as an alias for a properly-typed null value. + newVal = cty.NullVal(schema.ImpliedType()) + } + + for _, err := range newVal.Type().TestConformance(schema.ImpliedType()) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced invalid object", + fmt.Sprintf( + "Provider %q produced an invalid value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, tfdiags.FormatErrorPrefixed(err, absAddr.String()), + ), + )) + } + if diags.HasErrors() { + return nil, diags.Err() + } + + if newVal.IsNull() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced null object", + fmt.Sprintf( + "Provider %q produced a null value for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ProviderAddr.ProviderConfig.Type, absAddr, + ), + )) + } + + // Since we've completed the read, we actually have no change to make, but + // we'll produce a NoOp one anyway to preserve the usual flow of the + // plan phase and allow it to produce a complete plan. + change = &plans.ResourceInstanceChange{ + Addr: absAddr, + ProviderAddr: n.ProviderAddr, + Change: plans.Change{ + Action: plans.NoOp, + Before: newVal, + After: newVal, + }, + } + state := &states.ResourceInstanceObject{ + Value: change.After, + Status: states.ObjectReady, // because we completed the read from the provider + } + + err = ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostRefresh(absAddr, states.CurrentGen, change.Before, newVal) + }) + if err != nil { + return nil, err + } + + if n.OutputChange != nil { + *n.OutputChange = change + } + if n.OutputValue != nil { + *n.OutputValue = change.After + } + if n.OutputConfigValue != nil { + *n.OutputConfigValue = configVal + } + if n.OutputState != nil { + *n.OutputState = state + } + + return nil, diags.ErrWithWarnings() +} + // EvalReadDataDiff is an EvalNode implementation that executes a data // resource's ReadDataDiff method to discover what attributes it exports. type EvalReadDataDiff struct { diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 19e30c6cd..057207824 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -144,29 +144,13 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { ProviderSchema: &providerSchema, }, - &EvalReadDataDiff{ - Addr: addr.Resource, - Config: n.Config, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - Output: &change, - OutputConfigValue: &configVal, - OutputState: &state, - }, - - // The rest of this pass can proceed only if there are no - // computed values in our config. - // (If there are, we'll deal with this during the plan and - // apply phases.) &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if !configVal.IsWhollyKnown() { - return true, EvalEarlyExitError{} - } - // If the config explicitly has a depends_on for this // data source, assume the intention is to prevent - // refreshing ahead of that dependency. + // refreshing ahead of that dependency, and therefore + // we need to deal with this resource during the apply + // phase.. if len(n.Config.DependsOn) > 0 { return true, EvalEarlyExitError{} } @@ -176,25 +160,60 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { Then: EvalNoop{}, }, - &EvalReadDataApply{ - Addr: addr.Resource, - Config: n.Config, - Change: &change, - Provider: &provider, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - Output: &state, - StateReferences: n.StateReferences(), + // EvalReadData will _attempt_ to read the data source, but may + // generate an incomplete planned object if the configuration + // includes values that won't be known until apply. + &EvalReadData{ + Addr: addr.Resource, + Config: n.Config, + Provider: &provider, + ProviderAddr: n.ResolvedProvider, + ProviderSchema: &providerSchema, + OutputChange: &change, + OutputConfigValue: &configVal, + OutputState: &state, }, - &EvalWriteState{ - Addr: addr.Resource, - ProviderAddr: n.ResolvedProvider, - State: &state, - ProviderSchema: &providerSchema, + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + return (*state).Status != states.ObjectPlanned, nil + }, + Then: &EvalSequence{ + Nodes: []EvalNode{ + &EvalWriteState{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + State: &state, + ProviderSchema: &providerSchema, + }, + &EvalUpdateStateHook{}, + }, + }, + Else: &EvalSequence{ + // We can't deal with this yet, so we'll repeat this step + // during the plan walk to produce a planned change to read + // this during the apply walk. However, we do still need to + // save the generated change and partial state so that + // results from it can be included in other data resources + // or provider configurations during the refresh walk. + // (The planned object we save in the state here will be + // pruned out at the end of the refresh walk, returning + // it back to being unset again for subsequent walks.) + Nodes: []EvalNode{ + &EvalWriteDiff{ + Addr: addr.Resource, + Change: &change, + ProviderSchema: &providerSchema, + }, + &EvalWriteState{ + Addr: addr.Resource, + ProviderAddr: n.ResolvedProvider, + State: &state, + ProviderSchema: &providerSchema, + }, + }, + }, }, - - &EvalUpdateStateHook{}, }, } } diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index cab2ad365..55df4d8b1 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -123,7 +123,6 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou var providerSchema *ProviderSchema var change *plans.ResourceInstanceChange var state *states.ResourceInstanceObject - var configVal cty.Value return &EvalSequence{ Nodes: []EvalNode{ @@ -151,29 +150,19 @@ func (n *NodeApplyableResourceInstance) evalTreeDataResource(addr addrs.AbsResou Then: EvalNoop{}, }, - // Make a new diff, in case we've learned new values in the state - // during apply which we can now incorporate. - &EvalReadDataDiff{ + // In this particular call to EvalReadData we include our planned + // change, which signals that we expect this read to complete fully + // with no unknown values; it'll produce an error if not. + &EvalReadData{ Addr: addr.Resource, Config: n.Config, + Planned: &change, // setting this indicates that the result must be complete + Provider: &provider, ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, - Output: &change, - OutputValue: &configVal, OutputState: &state, }, - &EvalReadDataApply{ - Addr: addr.Resource, - Config: n.Config, - Change: &change, - Provider: &provider, - ProviderAddr: n.ResolvedProvider, - ProviderSchema: &providerSchema, - Output: &state, - StateReferences: n.StateReferences(), - }, - &EvalWriteState{ Addr: addr.Resource, ProviderAddr: n.ResolvedProvider, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 37285ec82..552951198 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -76,18 +76,32 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou Output: &state, }, + // If we already have a non-planned state then we already dealt + // with this during the refresh walk and so we have nothing to do + // here. + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + if state != nil && state.Status != states.ObjectPlanned { + return true, EvalEarlyExitError{} + } + return true, nil + }, + Then: EvalNoop{}, + }, + &EvalValidateSelfRef{ Addr: addr.Resource, Config: config.Config, ProviderSchema: &providerSchema, }, - &EvalReadDataDiff{ + &EvalReadData{ Addr: addr.Resource, Config: n.Config, + Provider: &provider, ProviderAddr: n.ResolvedProvider, ProviderSchema: &providerSchema, - Output: &change, + OutputChange: &change, OutputValue: &configVal, OutputState: &state, },