core: Better handle providers failing updates with no new value
A provider may react to a create or update failing by returning error diagnostics and a partially-updated or nil new value, in which case we do not expect our AssertObjectCompatible consistency check to succeed: the provider is just assumed to be doing the best it can to preserve whatever partial outcome it was able to achieve. However, if errors are accompanied with a nil new value after an update, we'll assume that the provider is telling us it wasn't able to get far enough to make any change at all, and so we'll retain the prior value in state. This ensures that a provider can't cause an object to be forgotten from the state just because an update failed.
This commit is contained in:
Martin Atkins
parent
303e5cc82e
commit
12a6d22589
|
|
@ -7097,6 +7097,159 @@ func TestContext2Apply_error(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestContext2Apply_errorCreateInvalidNew(t *testing.T) {
|
||||
m := testModule(t, "apply-error")
|
||||
|
||||
p := testProvider("aws")
|
||||
p.GetSchemaReturn = &ProviderSchema{
|
||||
ResourceTypes: map[string]*configschema.Block{
|
||||
"aws_instance": {
|
||||
Attributes: map[string]*configschema.Attribute{
|
||||
"value": {Type: cty.String, Optional: true},
|
||||
"foo": {Type: cty.String, Optional: true},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
|
||||
return providers.PlanResourceChangeResponse{
|
||||
PlannedState: req.ProposedNewState,
|
||||
}
|
||||
}
|
||||
p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
|
||||
// We're intentionally returning an inconsistent new state here
|
||||
// because we want to test that Terraform ignores the inconsistency
|
||||
// when accompanied by another error.
|
||||
return providers.ApplyResourceChangeResponse{
|
||||
NewState: cty.ObjectVal(map[string]cty.Value{
|
||||
"value": cty.StringVal("wrong wrong wrong wrong"),
|
||||
"foo": cty.StringVal("absolutely brimming over with wrongability"),
|
||||
}),
|
||||
Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("forced error")),
|
||||
}
|
||||
}
|
||||
|
||||
ctx := testContext2(t, &ContextOpts{
|
||||
Config: m,
|
||||
ProviderResolver: providers.ResolverFixed(
|
||||
map[string]providers.Factory{
|
||||
"aws": testProviderFuncFixed(p),
|
||||
},
|
||||
),
|
||||
})
|
||||
|
||||
if _, diags := ctx.Plan(); diags.HasErrors() {
|
||||
t.Fatalf("plan errors: %s", diags.Err())
|
||||
}
|
||||
|
||||
state, diags := ctx.Apply()
|
||||
if diags == nil {
|
||||
t.Fatal("should have error")
|
||||
}
|
||||
if got, want := len(diags), 1; got != want {
|
||||
// There should be no additional diagnostics generated by Terraform's own eval logic,
|
||||
// because the provider's own error supersedes them.
|
||||
t.Errorf("wrong number of diagnostics %d; want %d\n%s", got, want, diags.Err())
|
||||
}
|
||||
if got, want := diags.Err().Error(), "forced error"; !strings.Contains(got, want) {
|
||||
t.Errorf("returned error does not contain %q, but it should\n%s", want, diags.Err())
|
||||
}
|
||||
if got, want := len(state.RootModule().Resources), 2; got != want {
|
||||
t.Errorf("%d resources in state before prune; should have %d\n%s", got, want, spew.Sdump(state))
|
||||
}
|
||||
state.PruneResourceHusks() // aws_instance.bar with no instances gets left behind when we bail out, but that's okay
|
||||
if got, want := len(state.RootModule().Resources), 1; got != want {
|
||||
t.Errorf("%d resources in state after prune; should have only one (aws_instance.foo, tainted)\n%s", got, spew.Sdump(state))
|
||||
}
|
||||
}
|
||||
|
||||
func TestContext2Apply_errorUpdateNullNew(t *testing.T) {
|
||||
m := testModule(t, "apply-error")
|
||||
|
||||
p := testProvider("aws")
|
||||
p.GetSchemaReturn = &ProviderSchema{
|
||||
ResourceTypes: map[string]*configschema.Block{
|
||||
"aws_instance": {
|
||||
Attributes: map[string]*configschema.Attribute{
|
||||
"value": {Type: cty.String, Optional: true},
|
||||
"foo": {Type: cty.String, Optional: true},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
p.PlanResourceChangeFn = func(req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse {
|
||||
return providers.PlanResourceChangeResponse{
|
||||
PlannedState: req.ProposedNewState,
|
||||
}
|
||||
}
|
||||
p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) providers.ApplyResourceChangeResponse {
|
||||
// We're intentionally returning no NewState here because we want to
|
||||
// test that Terraform retains the prior state, rather than treating
|
||||
// the returned null as "no state" (object deleted).
|
||||
return providers.ApplyResourceChangeResponse{
|
||||
Diagnostics: tfdiags.Diagnostics(nil).Append(fmt.Errorf("forced error")),
|
||||
}
|
||||
}
|
||||
|
||||
ctx := testContext2(t, &ContextOpts{
|
||||
Config: m,
|
||||
State: states.BuildState(func(ss *states.SyncState) {
|
||||
ss.SetResourceInstanceCurrent(
|
||||
addrs.Resource{
|
||||
Mode: addrs.ManagedResourceMode,
|
||||
Type: "aws_instance",
|
||||
Name: "foo",
|
||||
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
|
||||
&states.ResourceInstanceObjectSrc{
|
||||
Status: states.ObjectReady,
|
||||
AttrsJSON: []byte(`{"value":"old"}`),
|
||||
},
|
||||
addrs.ProviderConfig{
|
||||
Type: "aws",
|
||||
}.Absolute(addrs.RootModuleInstance),
|
||||
)
|
||||
}),
|
||||
ProviderResolver: providers.ResolverFixed(
|
||||
map[string]providers.Factory{
|
||||
"aws": testProviderFuncFixed(p),
|
||||
},
|
||||
),
|
||||
})
|
||||
|
||||
if _, diags := ctx.Plan(); diags.HasErrors() {
|
||||
t.Fatalf("plan errors: %s", diags.Err())
|
||||
}
|
||||
|
||||
state, diags := ctx.Apply()
|
||||
if diags == nil {
|
||||
t.Fatal("should have error")
|
||||
}
|
||||
if got, want := len(diags), 1; got != want {
|
||||
// There should be no additional diagnostics generated by Terraform's own eval logic,
|
||||
// because the provider's own error supersedes them.
|
||||
t.Errorf("wrong number of diagnostics %d; want %d\n%s", got, want, diags.Err())
|
||||
}
|
||||
if got, want := diags.Err().Error(), "forced error"; !strings.Contains(got, want) {
|
||||
t.Errorf("returned error does not contain %q, but it should\n%s", want, diags.Err())
|
||||
}
|
||||
state.PruneResourceHusks()
|
||||
if got, want := len(state.RootModule().Resources), 1; got != want {
|
||||
t.Fatalf("%d resources in state; should have only one (aws_instance.foo, unmodified)\n%s", got, spew.Sdump(state))
|
||||
}
|
||||
|
||||
is := state.ResourceInstance(addrs.Resource{
|
||||
Mode: addrs.ManagedResourceMode,
|
||||
Type: "aws_instance",
|
||||
Name: "foo",
|
||||
}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance))
|
||||
if is == nil {
|
||||
t.Fatalf("aws_instance.foo is not in the state after apply")
|
||||
}
|
||||
if got, want := is.Current.AttrsJSON, []byte(`"old"`); !bytes.Contains(got, want) {
|
||||
t.Fatalf("incorrect attributes for aws_instance.foo\ngot: %s\nwant: JSON containing %s\n\n%s", got, want, spew.Sdump(is))
|
||||
}
|
||||
}
|
||||
|
||||
func TestContext2Apply_errorPartial(t *testing.T) {
|
||||
errored := false
|
||||
|
||||
|
|
|
|||
|
|
@ -173,11 +173,16 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
|
|||
newVal = cty.UnknownAsNull(newVal)
|
||||
}
|
||||
|
||||
if change.Action != plans.Delete {
|
||||
if change.Action != plans.Delete && !diags.HasErrors() {
|
||||
// Only values that were marked as unknown in the planned value are allowed
|
||||
// to change during the apply operation. (We do this after the unknown-ness
|
||||
// check above so that we also catch anything that became unknown after
|
||||
// being known during plan.)
|
||||
//
|
||||
// If we are returning other errors anyway then we'll give this
|
||||
// a pass since the other errors are usually the explanation for
|
||||
// this one and so it's more helpful to let the user focus on the
|
||||
// root cause rather than distract with this extra problem.
|
||||
if errs := objchange.AssertObjectCompatible(schema, change.After, newVal); len(errs) > 0 {
|
||||
if resp.LegacyTypeSystem {
|
||||
// The shimming of the old type system in the legacy SDK is not precise
|
||||
|
|
@ -240,6 +245,20 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
|
|||
}
|
||||
}
|
||||
|
||||
// Sometimes providers return a null value when an operation fails for some
|
||||
// reason, but for any action other than delete we'd rather keep the prior
|
||||
// state so that the error can be corrected on a subsequent run. We must
|
||||
// only do this for null new value though, or else we may discard partial
|
||||
// updates the provider was able to complete.
|
||||
if change.Action != plans.Delete && diags.HasErrors() && newVal.IsNull() {
|
||||
// Otherwise, we'll continue but using the prior state as the new value,
|
||||
// making this effectively a no-op. If the item really _has_ been
|
||||
// deleted then our next refresh will detect that and fix it up.
|
||||
// If change.Action is Create then change.Before will also be null,
|
||||
// which is fine.
|
||||
newVal = change.Before
|
||||
}
|
||||
|
||||
var newState *states.ResourceInstanceObject
|
||||
if !newVal.IsNull() { // null value indicates that the object is deleted, so we won't set a new state in that case
|
||||
newState = &states.ResourceInstanceObject{
|
||||
|
|
|
|||
Loading…
Reference in New Issue