diff --git a/.changes/v1.15/BUG FIXES-20260105-170648.yaml b/.changes/v1.15/BUG FIXES-20260105-170648.yaml new file mode 100644 index 000000000000..d0764cfb6622 --- /dev/null +++ b/.changes/v1.15/BUG FIXES-20260105-170648.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'stacks: send progress events if the plan fails for better UI integration' +time: 2026-01-05T17:06:48.252069+01:00 +custom: + Issue: "38039" diff --git a/internal/command/views/hook_count.go b/internal/command/views/hook_count.go index d36ce18ba9d7..2aeb33de0b35 100644 --- a/internal/command/views/hook_count.go +++ b/internal/command/views/hook_count.go @@ -88,10 +88,15 @@ func (h *countHook) PostApply(id terraform.HookResourceIdentity, dk addrs.Depose return terraform.HookActionContinue, nil } -func (h *countHook) PostDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (terraform.HookAction, error) { +func (h *countHook) PostDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (terraform.HookAction, error) { h.Lock() defer h.Unlock() + // Skip counting if there was an error + if err != nil { + return terraform.HookActionContinue, nil + } + // We don't count anything for data resources if id.Addr.Resource.Resource.Mode == addrs.DataResourceMode { return terraform.HookActionContinue, nil diff --git a/internal/command/views/hook_count_test.go b/internal/command/views/hook_count_test.go index f8297d1ff700..f81a1cb44f3a 100644 --- a/internal/command/views/hook_count_test.go +++ b/internal/command/views/hook_count_test.go @@ -46,7 +46,7 @@ func TestCountHookPostDiff_DestroyDeposed(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), states.DeposedKey("deadbeef"), plans.Delete, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), states.DeposedKey("deadbeef"), plans.Delete, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -77,7 +77,7 @@ func TestCountHookPostDiff_DestroyOnly(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Delete, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Delete, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -119,7 +119,7 @@ func TestCountHookPostDiff_AddOnly(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Create, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Create, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -164,7 +164,7 @@ func TestCountHookPostDiff_ChangeOnly(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Update, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.Update, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -195,7 +195,7 @@ func TestCountHookPostDiff_Mixed(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, a, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, a, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -227,7 +227,7 @@ func TestCountHookPostDiff_NoChange(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.NoOp, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, plans.NoOp, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) @@ -259,7 +259,7 @@ func TestCountHookPostDiff_DataSource(t *testing.T) { Name: k, }.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance) - h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, a, cty.DynamicVal, cty.DynamicVal) + h.PostDiff(testCountHookResourceID(addr), addrs.NotDeposed, a, cty.DynamicVal, cty.DynamicVal, nil) } expected := new(countHook) diff --git a/internal/rpcapi/stacks.go b/internal/rpcapi/stacks.go index 3afec745805b..bb4635f0ba31 100644 --- a/internal/rpcapi/stacks.go +++ b/internal/rpcapi/stacks.go @@ -595,7 +595,9 @@ func (s *stacksServer) ApplyStackChanges(req *stacks.ApplyStackChanges_Request, } depsHnd := handle[*depsfile.Locks](req.DependencyLocksHandle) var deps *depsfile.Locks - if !depsHnd.IsNil() { + if s.providerDependencyLockOverride != nil { + deps = s.providerDependencyLockOverride + } else if !depsHnd.IsNil() { deps = s.handles.DependencyLocks(depsHnd) if deps == nil { return status.Error(codes.InvalidArgument, "the given dependency locks handle is invalid") @@ -611,14 +613,23 @@ func (s *stacksServer) ApplyStackChanges(req *stacks.ApplyStackChanges_Request, return status.Error(codes.InvalidArgument, "the given provider cache handle is invalid") } } - // NOTE: providerCache can be nil if no handle was provided, in which - // case the call can only use built-in providers. All code below - // must avoid panicking when providerCache is nil, but is allowed to - // return an InvalidArgument error in that case. - // (providerFactoriesForLocks explicitly supports a nil providerCache) - providerFactories, err := providerFactoriesForLocks(deps, providerCache) - if err != nil { - return status.Errorf(codes.InvalidArgument, "provider dependencies are inconsistent: %s", err) + var providerFactories map[addrs.Provider]providers.Factory + if s.providerCacheOverride != nil { + // This is only used in tests to side load providers without needing a + // real provider cache. + providerFactories = s.providerCacheOverride + } else { + // NOTE: providerCache can be nil if no handle was provided, in which + // case the call can only use built-in providers. All code below + // must avoid panicking when providerCache is nil, but is allowed to + // return an InvalidArgument error in that case. + // (providerFactoriesForLocks explicitly supports a nil providerCache) + var err error + // (providerFactoriesForLocks explicitly supports a nil providerCache) + providerFactories, err = providerFactoriesForLocks(deps, providerCache) + if err != nil { + return status.Errorf(codes.InvalidArgument, "provider dependencies are inconsistent: %s", err) + } } inputValues, err := externalInputValuesFromProto(req.InputValues) diff --git a/internal/rpcapi/stacks_test.go b/internal/rpcapi/stacks_test.go index cd094b98f8e4..dd8b8a868074 100644 --- a/internal/rpcapi/stacks_test.go +++ b/internal/rpcapi/stacks_test.go @@ -488,7 +488,7 @@ func TestStacksPlanStackChanges(t *testing.T) { } } -func TestStackChangeProgress(t *testing.T) { +func TestStackChangeProgressDuringPlanNormal(t *testing.T) { tcs := map[string]struct { source string store *stacks_testing_provider.ResourceStore @@ -810,12 +810,664 @@ func TestStackChangeProgress(t *testing.T) { }, }, }, + "invalid - update": { + source: "git::https://example.com/invalid.git", + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("resource", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.NullVal(cty.String), + })). + Build(), + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject(t, "component.self.testing_resource.resource"), + NewStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "resource", + "value": nil, + }), + Status: states.ObjectReady, + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ResourceInstanceStatus_{ + ResourceInstanceStatus: &stacks.StackChangeProgress_ResourceInstanceStatus{ + Addr: &stacks.ResourceInstanceObjectInStackAddr{ + ComponentInstanceAddr: "component.self", + ResourceInstanceAddr: "testing_resource.resource", + }, + Status: stacks.StackChangeProgress_ResourceInstanceStatus_ERRORED, + ProviderAddr: "registry.terraform.io/hashicorp/testing", + }, + }, + }, + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_ERRORED, + }, + }, + }, + }, + diagnostics: []*terraform1.Diagnostic{ + { + Severity: terraform1.Diagnostic_ERROR, + Summary: "invalid configuration", + Detail: "configure_error attribute was set", + }, + { + Severity: terraform1.Diagnostic_ERROR, + Summary: "Provider configuration is invalid", + Detail: "Cannot decode the prior state for this resource instance because its provider configuration is invalid.", + }, + }, + }, + "invalid - create": { + source: "git::https://example.com/invalid.git", + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("resource", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.NullVal(cty.String), + })). + Build(), + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ResourceInstanceStatus_{ + ResourceInstanceStatus: &stacks.StackChangeProgress_ResourceInstanceStatus{ + Addr: &stacks.ResourceInstanceObjectInStackAddr{ + ComponentInstanceAddr: "component.self", + ResourceInstanceAddr: "testing_resource.resource", + }, + Status: stacks.StackChangeProgress_ResourceInstanceStatus_ERRORED, + ProviderAddr: "registry.terraform.io/hashicorp/testing", + }, + }, + }, + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_ERRORED, + }, + }, + }, + }, + diagnostics: []*terraform1.Diagnostic{ + { + Severity: terraform1.Diagnostic_ERROR, + Summary: "invalid configuration", + Detail: "configure_error attribute was set", + }, + { + Severity: terraform1.Diagnostic_ERROR, + Summary: "Provider configuration is invalid", + Detail: "Cannot plan changes for this resource because its associated provider configuration is invalid.", + }, + }, + }, + "no-op plan": { + source: "git::https://example.com/simple.git", + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("resource", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.NullVal(cty.String), + })). + Build(), + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject(t, "component.self.testing_resource.resource"), + NewStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "resource", + "value": nil, + }), + Status: states.ObjectReady, + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_PLANNED, + }, + }, + }, + }, + }, + "empty plan": { + source: "git::https://example.com/empty.git", + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_PLANNED, + }, + }, + }, + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + handles := newHandleTable() + stacksServer := newStacksServer(newStopper(), handles, disco.New(), &serviceOpts{}) + + // For this test, we do actually want to use a "real" provider. We'll + // use the providerCacheOverride to side-load the testing provider. + stacksServer.providerCacheOverride = make(map[addrs.Provider]providers.Factory) + stacksServer.providerCacheOverride[addrs.NewDefaultProvider("testing")] = func() (providers.Interface, error) { + return stacks_testing_provider.NewProviderWithData(t, tc.store), nil + } + lock := depsfile.NewLocks() + lock.SetProvider( + addrs.NewDefaultProvider("testing"), + providerreqs.MustParseVersion("0.0.0"), + providerreqs.MustParseVersionConstraints("=0.0.0"), + providerreqs.PreferredHashes([]providerreqs.Hash{}), + ) + stacksServer.providerDependencyLockOverride = lock + + sb, err := sourcebundle.OpenDir("testdata/sourcebundle") + if err != nil { + t.Fatal(err) + } + hnd := handles.NewSourceBundle(sb) + + client, close := grpcClientForTesting(ctx, t, func(srv *grpc.Server) { + stacks.RegisterStacksServer(srv, stacksServer) + }) + defer close() + + stacksClient := stacks.NewStacksClient(client) + + open, err := stacksClient.OpenStackConfiguration(ctx, &stacks.OpenStackConfiguration_Request{ + SourceBundleHandle: hnd.ForProtobuf(), + SourceAddress: &terraform1.SourceAddress{ + Source: tc.source, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + defer stacksClient.CloseStackConfiguration(ctx, &stacks.CloseStackConfiguration_Request{ + StackConfigHandle: open.StackConfigHandle, + }) + + resp, err := stacksClient.PlanStackChanges(ctx, &stacks.PlanStackChanges_Request{ + PlanMode: stacks.PlanMode_NORMAL, + StackConfigHandle: open.StackConfigHandle, + PreviousState: appliedChangeToRawState(t, tc.state), + InputValues: func() map[string]*stacks.DynamicValueWithSource { + values := make(map[string]*stacks.DynamicValueWithSource) + for name, value := range tc.inputs { + values[name] = &stacks.DynamicValueWithSource{ + Value: &stacks.DynamicValue{ + Msgpack: mustMsgpack(t, value, value.Type()), + }, + SourceRange: &terraform1.SourceRange{ + Start: &terraform1.SourcePos{}, + End: &terraform1.SourcePos{}, + }, + } + } + return values + }(), + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { + events := make([]*stacks.PlanStackChanges_Event, 0, len(tc.want)) + for _, want := range tc.want { + events = append(events, &stacks.PlanStackChanges_Event{ + Event: &stacks.PlanStackChanges_Event_Progress{ + Progress: want, + }, + }) + } + return events + }()) + + gotEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { + var events []*stacks.PlanStackChanges_Event + for { + event, err := resp.Recv() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + events = append(events, event) + } + return events + }()) + + // First, validate the diagnostics. Most of the tests are either + // expecting a specific single diagnostic so we do actually check + // everything. + // We don't care about the ordering since it's not guaranteed. + + if len(tc.diagnostics) != len(gotEvents.Diagnostics) { + t.Fatalf("expected %d diagnostics, got %d", len(tc.diagnostics), len(gotEvents.Diagnostics)) + } + + DIAGS: + for _, expectedDiag := range tc.diagnostics { + for _, gotDiagEvent := range gotEvents.Diagnostics { + gotDiag := gotDiagEvent.Event.(*stacks.PlanStackChanges_Event_Diagnostic).Diagnostic + + if diff := cmp.Diff(expectedDiag, gotDiag, protocmp.Transform()); diff == "" { + continue DIAGS // Found it + } + } + + // If we reach this point we did not find the diag + t.Errorf("missing expected diagnostic: %v, got %v", expectedDiag, gotEvents.Diagnostics) + } + + // Now we're going to manually verify the existence of some key events. + // We're not looking for every event because (a) the exact ordering of + // events is not guaranteed and (b) we don't want to start failing every + // time a new event is added. + + WantPlannedChange: + for _, want := range wantEvents.PlannedChanges { + for _, got := range gotEvents.PlannedChanges { + if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { + continue WantPlannedChange + } + } + t.Errorf("missing expected planned change: %v", want) + } + + WantMiscHook: + for _, want := range wantEvents.MiscHooks { + for _, got := range gotEvents.MiscHooks { + if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { + continue WantMiscHook + } + } + t.Errorf("missing expected event: %v", want) + } + + if t.Failed() { + // if the test failed, let's print out all the events we got to help + // with debugging. + for _, evt := range gotEvents.MiscHooks { + t.Logf(" returned event: %s", evt.String()) + } + + for _, evt := range gotEvents.PlannedChanges { + t.Logf(" returned event: %s", evt.String()) + } + } + }) + } +} + +func TestStackChangeProgressDuringPlanDestroy(t *testing.T) { + tcs := map[string]struct { + source string + store *stacks_testing_provider.ResourceStore + state []stackstate.AppliedChange + inputs map[string]cty.Value + want []*stacks.StackChangeProgress + diagnostics []*terraform1.Diagnostic + }{ + "destroy plan": { + source: "git::https://example.com/simple.git", + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("resource", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.NullVal(cty.String), + })). + Build(), + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject(t, "component.self.testing_resource.resource"), + NewStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "resource", + "value": nil, + }), + Status: states.ObjectReady, + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ResourceInstancePlannedChange_{ + ResourceInstancePlannedChange: &stacks.StackChangeProgress_ResourceInstancePlannedChange{ + Addr: &stacks.ResourceInstanceObjectInStackAddr{ + ComponentInstanceAddr: "component.self", + ResourceInstanceAddr: "testing_resource.resource", + }, + Actions: []stacks.ChangeType{ + stacks.ChangeType_DELETE, + }, + ProviderAddr: "registry.terraform.io/hashicorp/testing", + }, + }, + }, + { + Event: &stacks.StackChangeProgress_ComponentInstanceChanges_{ + ComponentInstanceChanges: &stacks.StackChangeProgress_ComponentInstanceChanges{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Total: 1, + Remove: 1, + }, + }, + }, + }, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + handles := newHandleTable() + stacksServer := newStacksServer(newStopper(), handles, disco.New(), &serviceOpts{}) + + // For this test, we do actually want to use a "real" provider. We'll + // use the providerCacheOverride to side-load the testing provider. + stacksServer.providerCacheOverride = make(map[addrs.Provider]providers.Factory) + stacksServer.providerCacheOverride[addrs.NewDefaultProvider("testing")] = func() (providers.Interface, error) { + return stacks_testing_provider.NewProviderWithData(t, tc.store), nil + } + lock := depsfile.NewLocks() + lock.SetProvider( + addrs.NewDefaultProvider("testing"), + providerreqs.MustParseVersion("0.0.0"), + providerreqs.MustParseVersionConstraints("=0.0.0"), + providerreqs.PreferredHashes([]providerreqs.Hash{}), + ) + stacksServer.providerDependencyLockOverride = lock + + sb, err := sourcebundle.OpenDir("testdata/sourcebundle") + if err != nil { + t.Fatal(err) + } + hnd := handles.NewSourceBundle(sb) + + client, close := grpcClientForTesting(ctx, t, func(srv *grpc.Server) { + stacks.RegisterStacksServer(srv, stacksServer) + }) + defer close() + + stacksClient := stacks.NewStacksClient(client) + + open, err := stacksClient.OpenStackConfiguration(ctx, &stacks.OpenStackConfiguration_Request{ + SourceBundleHandle: hnd.ForProtobuf(), + SourceAddress: &terraform1.SourceAddress{ + Source: tc.source, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + defer stacksClient.CloseStackConfiguration(ctx, &stacks.CloseStackConfiguration_Request{ + StackConfigHandle: open.StackConfigHandle, + }) + + resp, err := stacksClient.PlanStackChanges(ctx, &stacks.PlanStackChanges_Request{ + PlanMode: stacks.PlanMode_DESTROY, + StackConfigHandle: open.StackConfigHandle, + PreviousState: appliedChangeToRawState(t, tc.state), + InputValues: func() map[string]*stacks.DynamicValueWithSource { + values := make(map[string]*stacks.DynamicValueWithSource) + for name, value := range tc.inputs { + values[name] = &stacks.DynamicValueWithSource{ + Value: &stacks.DynamicValue{ + Msgpack: mustMsgpack(t, value, value.Type()), + }, + SourceRange: &terraform1.SourceRange{ + Start: &terraform1.SourcePos{}, + End: &terraform1.SourcePos{}, + }, + } + } + return values + }(), + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { + events := make([]*stacks.PlanStackChanges_Event, 0, len(tc.want)) + for _, want := range tc.want { + events = append(events, &stacks.PlanStackChanges_Event{ + Event: &stacks.PlanStackChanges_Event_Progress{ + Progress: want, + }, + }) + } + return events + }()) + + gotEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { + var events []*stacks.PlanStackChanges_Event + for { + event, err := resp.Recv() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + events = append(events, event) + } + return events + }()) + + // First, validate the diagnostics. Most of the tests are either + // expecting a specific single diagnostic so we do actually check + // everything. + // We don't care about the ordering since it's not guaranteed. + + if len(tc.diagnostics) != len(gotEvents.Diagnostics) { + t.Fatalf("expected %d diagnostics, got %d", len(tc.diagnostics), len(gotEvents.Diagnostics)) + } + + DIAGS: + for _, expectedDiag := range tc.diagnostics { + for _, gotDiagEvent := range gotEvents.Diagnostics { + gotDiag := gotDiagEvent.Event.(*stacks.PlanStackChanges_Event_Diagnostic).Diagnostic + + if diff := cmp.Diff(expectedDiag, gotDiag, protocmp.Transform()); diff == "" { + continue DIAGS // Found it + } + } + + // If we reach this point we did not find the diag + t.Errorf("missing expected diagnostic: %v, got %v", expectedDiag, gotEvents.Diagnostics) + } + + // Now we're going to manually verify the existence of some key events. + // We're not looking for every event because (a) the exact ordering of + // events is not guaranteed and (b) we don't want to start failing every + // time a new event is added. + + WantPlannedChange: + for _, want := range wantEvents.PlannedChanges { + for _, got := range gotEvents.PlannedChanges { + if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { + continue WantPlannedChange + } + } + t.Errorf("missing expected planned change: %v", want) + } + + WantMiscHook: + for _, want := range wantEvents.MiscHooks { + for _, got := range gotEvents.MiscHooks { + if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { + continue WantMiscHook + } + } + t.Errorf("missing expected event: %v", want) + } + + if t.Failed() { + // if the test failed, let's print out all the events we got to help + // with debugging. + for _, evt := range gotEvents.MiscHooks { + t.Logf(" returned event: %s", evt.String()) + } + + for _, evt := range gotEvents.PlannedChanges { + t.Logf(" returned event: %s", evt.String()) + } + } + }) + } +} + +func TestStackChangeProgressDuringApply(t *testing.T) { + tcs := map[string]struct { + mode stacks.PlanMode + source string + store *stacks_testing_provider.ResourceStore + state []stackstate.AppliedChange + inputs map[string]cty.Value + want []*stacks.StackChangeProgress + diagnostics []*terraform1.Diagnostic + }{ + "simple": { + mode: stacks.PlanMode_NORMAL, + source: "git::https://example.com/simple.git", + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_APPLIED, + }, + }, + }, + }, + }, + "no-op": { + mode: stacks.PlanMode_NORMAL, + source: "git::https://example.com/simple.git", + store: stacks_testing_provider.NewResourceStoreBuilder(). + AddResource("resource", cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("resource"), + "value": cty.NullVal(cty.String), + })). + Build(), + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + &stackstate.AppliedChangeResourceInstanceObject{ + ResourceInstanceObjectAddr: mustAbsResourceInstanceObject(t, "component.self.testing_resource.resource"), + NewStateSrc: &states.ResourceInstanceObjectSrc{ + AttrsJSON: mustMarshalJSONAttrs(map[string]interface{}{ + "id": "resource", + "value": nil, + }), + Status: states.ObjectReady, + }, + ProviderConfigAddr: mustDefaultRootProvider("testing"), + Schema: stacks_testing_provider.TestingResourceSchema, + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_APPLIED, + }, + }, + }, + }, + }, + "empty": { + mode: stacks.PlanMode_NORMAL, + source: "git::https://example.com/empty.git", + state: []stackstate.AppliedChange{ + &stackstate.AppliedChangeComponentInstance{ + ComponentAddr: mustAbsComponent(t, "component.self"), + ComponentInstanceAddr: mustAbsComponentInstance(t, "component.self"), + }, + }, + want: []*stacks.StackChangeProgress{ + { + Event: &stacks.StackChangeProgress_ComponentInstanceStatus_{ + ComponentInstanceStatus: &stacks.StackChangeProgress_ComponentInstanceStatus{ + Addr: &stacks.ComponentInstanceInStackAddr{ + ComponentAddr: "component.self", + ComponentInstanceAddr: "component.self", + }, + Status: stacks.StackChangeProgress_ComponentInstanceStatus_APPLIED, + }, + }, + }, + }, + }, } for name, tc := range tcs { t.Run(name, func(t *testing.T) { ctx := context.Background() - handles := newHandleTable() stacksServer := newStacksServer(newStopper(), handles, disco.New(), &serviceOpts{}) @@ -860,8 +1512,8 @@ func TestStackChangeProgress(t *testing.T) { StackConfigHandle: open.StackConfigHandle, }) - resp, err := stacksClient.PlanStackChanges(ctx, &stacks.PlanStackChanges_Request{ - PlanMode: stacks.PlanMode_NORMAL, + planResp, err := stacksClient.PlanStackChanges(ctx, &stacks.PlanStackChanges_Request{ + PlanMode: tc.mode, StackConfigHandle: open.StackConfigHandle, PreviousState: appliedChangeToRawState(t, tc.state), InputValues: func() map[string]*stacks.DynamicValueWithSource { @@ -884,11 +1536,68 @@ func TestStackChangeProgress(t *testing.T) { t.Fatalf("unexpected error: %s", err) } - wantEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { - events := make([]*stacks.PlanStackChanges_Event, 0, len(tc.want)) + planEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { + var events []*stacks.PlanStackChanges_Event + for { + event, err := planResp.Recv() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + events = append(events, event) + } + return events + }()) + + planStream, err := stacksClient.OpenPlan(ctx) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + for _, v := range planEvents.PlannedChanges { + for _, r := range v.GetPlannedChange().Raw { + planStream.Send(&stacks.OpenStackPlan_RequestItem{ + Raw: r, + }) + } + } + + planResult, err := planStream.CloseAndRecv() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + applyResp, err := stacksClient.ApplyStackChanges(ctx, &stacks.ApplyStackChanges_Request{ + StackConfigHandle: open.StackConfigHandle, + PlanHandle: planResult.PlanHandle, + InputValues: func() map[string]*stacks.DynamicValueWithSource { + values := make(map[string]*stacks.DynamicValueWithSource) + for name, value := range tc.inputs { + values[name] = &stacks.DynamicValueWithSource{ + Value: &stacks.DynamicValue{ + Msgpack: mustMsgpack(t, value, value.Type()), + }, + SourceRange: &terraform1.SourceRange{ + Start: &terraform1.SourcePos{}, + End: &terraform1.SourcePos{}, + }, + } + } + return values + }(), + }) + + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + wantEvents := splitApplyStackOperationEvents(func() []*stacks.ApplyStackChanges_Event { + events := make([]*stacks.ApplyStackChanges_Event, 0, len(tc.want)) for _, want := range tc.want { - events = append(events, &stacks.PlanStackChanges_Event{ - Event: &stacks.PlanStackChanges_Event_Progress{ + events = append(events, &stacks.ApplyStackChanges_Event{ + Event: &stacks.ApplyStackChanges_Event_Progress{ Progress: want, }, }) @@ -896,10 +1605,10 @@ func TestStackChangeProgress(t *testing.T) { return events }()) - gotEvents := splitStackOperationEvents(func() []*stacks.PlanStackChanges_Event { - var events []*stacks.PlanStackChanges_Event + gotApplyEvents := splitApplyStackOperationEvents(func() []*stacks.ApplyStackChanges_Event { + var events []*stacks.ApplyStackChanges_Event for { - event, err := resp.Recv() + event, err := applyResp.Recv() if err == io.EOF { break } @@ -917,21 +1626,21 @@ func TestStackChangeProgress(t *testing.T) { diagIx := 0 for ; diagIx < len(tc.diagnostics); diagIx++ { - if diagIx >= len(gotEvents.Diagnostics) { + if diagIx >= len(gotApplyEvents.Diagnostics) { // Then we have more expected diagnostics than we got. t.Errorf("missing expected diagnostic: %v", tc.diagnostics[diagIx]) continue } - diag := gotEvents.Diagnostics[diagIx].Event.(*stacks.PlanStackChanges_Event_Diagnostic).Diagnostic + diag := gotApplyEvents.Diagnostics[diagIx].Event.(*stacks.ApplyStackChanges_Event_Diagnostic).Diagnostic if diff := cmp.Diff(tc.diagnostics[diagIx], diag, protocmp.Transform()); diff != "" { // Then we have a diagnostic that doesn't match what we // expected. t.Errorf("wrong diagnostic\n%s", diff) } } - for ; diagIx < len(gotEvents.Diagnostics); diagIx++ { + for ; diagIx < len(gotApplyEvents.Diagnostics); diagIx++ { // Then we have more diagnostics than we expected. - t.Errorf("unexpected diagnostic: %v", gotEvents.Diagnostics[diagIx]) + t.Errorf("unexpected diagnostic: %v", gotApplyEvents.Diagnostics[diagIx]) } // Now we're going to manually verify the existence of some key events. @@ -940,8 +1649,8 @@ func TestStackChangeProgress(t *testing.T) { // time a new event is added. WantPlannedChange: - for _, want := range wantEvents.PlannedChanges { - for _, got := range gotEvents.PlannedChanges { + for _, want := range wantEvents.AppliedChanges { + for _, got := range gotApplyEvents.AppliedChanges { if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { continue WantPlannedChange } @@ -951,7 +1660,7 @@ func TestStackChangeProgress(t *testing.T) { WantMiscHook: for _, want := range wantEvents.MiscHooks { - for _, got := range gotEvents.MiscHooks { + for _, got := range gotApplyEvents.MiscHooks { if len(cmp.Diff(want, got, protocmp.Transform())) == 0 { continue WantMiscHook } @@ -962,11 +1671,11 @@ func TestStackChangeProgress(t *testing.T) { if t.Failed() { // if the test failed, let's print out all the events we got to help // with debugging. - for _, evt := range gotEvents.MiscHooks { + for _, evt := range gotApplyEvents.MiscHooks { t.Logf(" returned event: %s", evt.String()) } - for _, evt := range gotEvents.PlannedChanges { + for _, evt := range gotApplyEvents.AppliedChanges { t.Logf(" returned event: %s", evt.String()) } } @@ -1287,6 +1996,30 @@ func splitStackOperationEvents(all []*stacks.PlanStackChanges_Event) stackOperat return ret } +type stacksApplyOperationEventStreams struct { + AppliedChanges []*stacks.ApplyStackChanges_Event + Diagnostics []*stacks.ApplyStackChanges_Event + + // MiscHooks is the "everything else" category where the detailed begin/end + // events for individual Terraform Core operations appear. + MiscHooks []*stacks.ApplyStackChanges_Event +} + +func splitApplyStackOperationEvents(all []*stacks.ApplyStackChanges_Event) stacksApplyOperationEventStreams { + ret := stacksApplyOperationEventStreams{} + for _, evt := range all { + switch evt.Event.(type) { + case *stacks.ApplyStackChanges_Event_AppliedChange: + ret.AppliedChanges = append(ret.AppliedChanges, evt) + case *stacks.ApplyStackChanges_Event_Diagnostic: + ret.Diagnostics = append(ret.Diagnostics, evt) + default: + ret.MiscHooks = append(ret.MiscHooks, evt) + } + } + return ret +} + func mustMsgpack(t *testing.T, v cty.Value, ty cty.Type) []byte { t.Helper() diff --git a/internal/rpcapi/testdata/sourcebundle/empty/empty.tf b/internal/rpcapi/testdata/sourcebundle/empty/empty.tf new file mode 100644 index 000000000000..c651097a0f09 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/empty/empty.tf @@ -0,0 +1,10 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +# Empty by design diff --git a/internal/rpcapi/testdata/sourcebundle/empty/empty.tfcomponent.hcl b/internal/rpcapi/testdata/sourcebundle/empty/empty.tfcomponent.hcl new file mode 100644 index 000000000000..18c62cbcbfd2 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/empty/empty.tfcomponent.hcl @@ -0,0 +1,16 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +component "self" { + source = "./" + + providers = { + testing = provider.testing.default + } +} diff --git a/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tf b/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tf new file mode 100644 index 000000000000..d787664fa802 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tf @@ -0,0 +1,10 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +resource "testing_resource" "resource" {} diff --git a/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tfcomponent.hcl b/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tfcomponent.hcl new file mode 100644 index 000000000000..88aadd6ee672 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/invalid/invalid.tfcomponent.hcl @@ -0,0 +1,21 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" { + config { + // This provider is going to fail to configure. + configure_error = "invalid configuration" + } +} + +component "self" { + source = "./" + + providers = { + testing = provider.testing.default + } +} diff --git a/internal/rpcapi/testdata/sourcebundle/simple/simple.tf b/internal/rpcapi/testdata/sourcebundle/simple/simple.tf new file mode 100644 index 000000000000..d787664fa802 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/simple/simple.tf @@ -0,0 +1,10 @@ +terraform { + required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } + } +} + +resource "testing_resource" "resource" {} diff --git a/internal/rpcapi/testdata/sourcebundle/simple/simple.tfcomponent.hcl b/internal/rpcapi/testdata/sourcebundle/simple/simple.tfcomponent.hcl new file mode 100644 index 000000000000..18c62cbcbfd2 --- /dev/null +++ b/internal/rpcapi/testdata/sourcebundle/simple/simple.tfcomponent.hcl @@ -0,0 +1,16 @@ +required_providers { + testing = { + source = "hashicorp/testing" + version = "0.1.0" + } +} + +provider "testing" "default" {} + +component "self" { + source = "./" + + providers = { + testing = provider.testing.default + } +} diff --git a/internal/rpcapi/testdata/sourcebundle/terraform-sources.json b/internal/rpcapi/testdata/sourcebundle/terraform-sources.json index aba8a33459b2..6421282d51c8 100644 --- a/internal/rpcapi/testdata/sourcebundle/terraform-sources.json +++ b/internal/rpcapi/testdata/sourcebundle/terraform-sources.json @@ -30,6 +30,21 @@ "source": "git::https://example.com/removed.git", "local": "removed", "meta": {} + }, + { + "source": "git::https://example.com/invalid.git", + "local": "invalid", + "meta": {} + }, + { + "source": "git::https://example.com/simple.git", + "local": "simple", + "meta": {} + }, + { + "source": "git::https://example.com/empty.git", + "local": "empty", + "meta": {} } ] } diff --git a/internal/stacks/stackruntime/internal/stackeval/component_instance.go b/internal/stacks/stackruntime/internal/stackeval/component_instance.go index 144a7b3cafe3..5de779c15fc8 100644 --- a/internal/stacks/stackruntime/internal/stackeval/component_instance.go +++ b/internal/stacks/stackruntime/internal/stackeval/component_instance.go @@ -357,6 +357,8 @@ func (c *ComponentInstance) CheckModuleTreePlan(ctx context.Context) (*plans.Pla ReportComponentInstance(ctx, plan, h, seq, c) if plan.Complete { hookMore(ctx, seq, h.EndComponentInstancePlan, c.Addr()) + } else if plan.Errored { + hookMore(ctx, seq, h.ErrorComponentInstancePlan, c.Addr()) } else { hookMore(ctx, seq, h.DeferComponentInstancePlan, c.Addr()) } diff --git a/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go b/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go index fc219a9faaa0..390515e0edb1 100644 --- a/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go +++ b/internal/stacks/stackruntime/internal/stackeval/terraform_hook.go @@ -56,20 +56,28 @@ func (h *componentInstanceTerraformHook) resourceInstanceObjectAddr(riAddr addrs } } -func (h *componentInstanceTerraformHook) PreDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (terraform.HookAction, error) { +func (h *componentInstanceTerraformHook) PreDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (terraform.HookAction, error) { + status := hooks.ResourceInstancePlanning + if err != nil { + status = hooks.ResourceInstanceErrored + } hookMore(h.ctx, h.seq, h.hooks.ReportResourceInstanceStatus, &hooks.ResourceInstanceStatusHookData{ Addr: h.resourceInstanceObjectAddr(id.Addr, dk), ProviderAddr: id.ProviderAddr, - Status: hooks.ResourceInstancePlanning, + Status: status, }) return terraform.HookActionContinue, nil } -func (h *componentInstanceTerraformHook) PostDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (terraform.HookAction, error) { +func (h *componentInstanceTerraformHook) PostDiff(id terraform.HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (terraform.HookAction, error) { + status := hooks.ResourceInstancePlanned + if err != nil { + status = hooks.ResourceInstanceErrored + } hookMore(h.ctx, h.seq, h.hooks.ReportResourceInstanceStatus, &hooks.ResourceInstanceStatusHookData{ Addr: h.resourceInstanceObjectAddr(id.Addr, dk), ProviderAddr: id.ProviderAddr, - Status: hooks.ResourceInstancePlanned, + Status: status, }) return terraform.HookActionContinue, nil } diff --git a/internal/stacks/stackruntime/internal/stackeval/terraform_hook_test.go b/internal/stacks/stackruntime/internal/stackeval/terraform_hook_test.go index 72fce5c2a0ed..d294c05f3c58 100644 --- a/internal/stacks/stackruntime/internal/stackeval/terraform_hook_test.go +++ b/internal/stacks/stackruntime/internal/stackeval/terraform_hook_test.go @@ -72,7 +72,7 @@ func TestTerraformHook(t *testing.T) { t.Run("PreDiff", func(t *testing.T) { hook := makeHook() - action, err := hook.PreDiff(resourceIdentity, addrs.NotDeposed, cty.NilVal, cty.NilVal) + action, err := hook.PreDiff(resourceIdentity, addrs.NotDeposed, cty.NilVal, cty.NilVal, nil) if err != nil { t.Errorf("unexpected error: %s", err) } @@ -93,9 +93,9 @@ func TestTerraformHook(t *testing.T) { } }) - t.Run("PostDiff", func(t *testing.T) { + t.Run("PostDiff - success", func(t *testing.T) { hook := makeHook() - action, err := hook.PostDiff(resourceIdentity, addrs.NotDeposed, plans.Create, cty.NilVal, cty.NilVal) + action, err := hook.PostDiff(resourceIdentity, addrs.NotDeposed, plans.Create, cty.NilVal, cty.NilVal, nil) if err != nil { t.Errorf("unexpected error: %s", err) } @@ -116,6 +116,29 @@ func TestTerraformHook(t *testing.T) { } }) + t.Run("PostDiff - error", func(t *testing.T) { + hook := makeHook() + action, err := hook.PostDiff(resourceIdentity, addrs.NotDeposed, plans.Create, cty.NilVal, cty.NilVal, errors.New("oh no")) + if err != nil { + t.Errorf("unexpected error: %s", err) + } + if action != terraform.HookActionContinue { + t.Errorf("wrong action: %#v", action) + } + if hook.seq.tracking != "boop" { + t.Errorf("wrong tracking value: %#v", hook.seq.tracking) + } + + wantRihd := &hooks.ResourceInstanceStatusHookData{ + Addr: stackAddr, + ProviderAddr: providerAddr, + Status: hooks.ResourceInstanceErrored, + } + if diff := cmp.Diff(gotRihd, wantRihd); diff != "" { + t.Errorf("wrong status hook data:\n%s", diff) + } + }) + t.Run("PreApply", func(t *testing.T) { hook := makeHook() action, err := hook.PreApply(resourceIdentity, addrs.NotDeposed, plans.Create, cty.NilVal, cty.NilVal) diff --git a/internal/terraform/hook.go b/internal/terraform/hook.go index 47cb00c67191..fdb9791bab82 100644 --- a/internal/terraform/hook.go +++ b/internal/terraform/hook.go @@ -62,8 +62,8 @@ type Hook interface { // PreDiff and PostDiff are called before and after a provider is given // the opportunity to customize the proposed new state to produce the // planned new state. - PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (HookAction, error) - PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) + PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (HookAction, error) + PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (HookAction, error) // The provisioning hooks signal both the overall start end end of // provisioning for a particular instance and of each of the individual @@ -165,11 +165,11 @@ func (*NilHook) PostApply(id HookResourceIdentity, dk addrs.DeposedKey, newState return HookActionContinue, nil } -func (*NilHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (HookAction, error) { +func (*NilHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (HookAction, error) { return HookActionContinue, nil } -func (*NilHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) { +func (*NilHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (HookAction, error) { return HookActionContinue, nil } diff --git a/internal/terraform/hook_mock.go b/internal/terraform/hook_mock.go index cc6f5cf53210..d8971b326731 100644 --- a/internal/terraform/hook_mock.go +++ b/internal/terraform/hook_mock.go @@ -210,7 +210,7 @@ func (h *MockHook) PostApply(id HookResourceIdentity, dk addrs.DeposedKey, newSt return h.PostApplyReturn, h.PostApplyReturnError } -func (h *MockHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (HookAction, error) { +func (h *MockHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (HookAction, error) { h.Lock() defer h.Unlock() @@ -222,7 +222,7 @@ func (h *MockHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorSt return h.PreDiffReturn, h.PreDiffError } -func (h *MockHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) { +func (h *MockHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (HookAction, error) { h.Lock() defer h.Unlock() diff --git a/internal/terraform/hook_stop.go b/internal/terraform/hook_stop.go index fe34bc81f3cc..530f3fd97d43 100644 --- a/internal/terraform/hook_stop.go +++ b/internal/terraform/hook_stop.go @@ -31,11 +31,11 @@ func (h *stopHook) PostApply(id HookResourceIdentity, dk addrs.DeposedKey, newSt return h.hook() } -func (h *stopHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (HookAction, error) { +func (h *stopHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (HookAction, error) { return h.hook() } -func (h *stopHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) { +func (h *stopHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (HookAction, error) { return h.hook() } diff --git a/internal/terraform/hook_test.go b/internal/terraform/hook_test.go index 3e94aacd45d9..d871ec3d185f 100644 --- a/internal/terraform/hook_test.go +++ b/internal/terraform/hook_test.go @@ -51,14 +51,14 @@ func (h *testHook) PostApply(id HookResourceIdentity, dk addrs.DeposedKey, newSt return HookActionContinue, nil } -func (h *testHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value) (HookAction, error) { +func (h *testHook) PreDiff(id HookResourceIdentity, dk addrs.DeposedKey, priorState, proposedNewState cty.Value, err error) (HookAction, error) { h.mu.Lock() defer h.mu.Unlock() h.Calls = append(h.Calls, &testHookCall{"PreDiff", id.Addr.String()}) return HookActionContinue, nil } -func (h *testHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value) (HookAction, error) { +func (h *testHook) PostDiff(id HookResourceIdentity, dk addrs.DeposedKey, action plans.Action, priorState, plannedNewState cty.Value, err error) (HookAction, error) { h.mu.Lock() defer h.mu.Unlock() h.Calls = append(h.Calls, &testHookCall{"PostDiff", id.Addr.String()}) diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index 37321761bc14..23112ee74ecd 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -429,7 +429,7 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState // Call pre-diff hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreDiff(n.HookResourceIdentity(), deposedKey, currentState.Value, nullVal) + return h.PreDiff(n.HookResourceIdentity(), deposedKey, currentState.Value, nullVal, nil) })) if diags.HasErrors() { return plan, deferred, diags @@ -478,6 +478,9 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState } diags = diags.Append(resp.Diagnostics) if diags.HasErrors() { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.HookResourceIdentity(), deposedKey, plans.Delete, currentState.Value, nullVal, diags.Err()) + })) return plan, deferred, diags } @@ -498,7 +501,7 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState // Call post-refresh hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostDiff(n.HookResourceIdentity(), deposedKey, plans.Delete, currentState.Value, nullVal) + return h.PostDiff(n.HookResourceIdentity(), deposedKey, plans.Delete, currentState.Value, nullVal, nil) })) if diags.HasErrors() { return plan, deferred, diags @@ -945,7 +948,7 @@ func (n *NodeAbstractResourceInstance) plan( // Call pre-diff hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, priorVal, proposedNewVal) + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, priorVal, proposedNewVal, nil) })) if diags.HasErrors() { return nil, nil, deferred, keyData, diags @@ -994,6 +997,9 @@ func (n *NodeAbstractResourceInstance) plan( } diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) if diags.HasErrors() { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal, diags.Err()) + })) return nil, nil, deferred, keyData, diags } @@ -1207,6 +1213,9 @@ func (n *NodeAbstractResourceInstance) plan( // append these new diagnostics if there's at least one error inside. if resp.Diagnostics.HasErrors() { diags = diags.Append(resp.Diagnostics.InConfigBody(config.Config, n.Addr.String())) + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal, diags.Err()) + })) return nil, nil, deferred, keyData, diags } @@ -1233,6 +1242,9 @@ func (n *NodeAbstractResourceInstance) plan( )) } if diags.HasErrors() { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal, diags.Err()) + })) return nil, nil, deferred, keyData, diags } @@ -1251,6 +1263,9 @@ func (n *NodeAbstractResourceInstance) plan( diags = diags.Append(writeOnlyDiags) if writeOnlyDiags.HasErrors() { + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal, diags.Err()) + })) return nil, nil, deferred, keyData, diags } } @@ -1297,7 +1312,7 @@ func (n *NodeAbstractResourceInstance) plan( // Call post-refresh hook diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, action, priorVal, plannedNewVal) + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, action, priorVal, plannedNewVal, nil) })) if diags.HasErrors() { return nil, nil, deferred, keyData, diags @@ -1939,7 +1954,7 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, checkRule } diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { - return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal) + return h.PostDiff(n.HookResourceIdentity(), addrs.NotDeposed, plans.Read, priorVal, proposedNewVal, nil) })) return plannedChange, plannedNewState, deferred, keyData, diags diff --git a/internal/terraform/node_resource_plan_instance.go b/internal/terraform/node_resource_plan_instance.go index 74f7140a37c9..ff700b201ca5 100644 --- a/internal/terraform/node_resource_plan_instance.go +++ b/internal/terraform/node_resource_plan_instance.go @@ -255,6 +255,10 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) instanceRefreshState, readDiags = n.readResourceInstanceState(ctx, addr) diags = diags.Append(readDiags) if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags } } @@ -267,12 +271,20 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) // refresh step below. diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, prevRunState)) if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags } // Also the refreshState, because that should still reflect schema upgrades // even if it doesn't reflect upstream changes. diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, refreshState)) if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags } } @@ -319,6 +331,10 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) } if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags } } @@ -329,6 +345,10 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) // sure we still update any changes to CreateBeforeDestroy. diags = diags.Append(n.writeResourceInstanceState(ctx, instanceRefreshState, refreshState)) if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags } } @@ -349,6 +369,10 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) diags = diags.Append(n.replaceTriggered(ctx, repData)) if diags.HasErrors() { + // Pre-Diff error hook + diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) { + return h.PreDiff(n.HookResourceIdentity(), addrs.NotDeposed, cty.DynamicVal, cty.DynamicVal, diags.Err()) + })) return diags }