diff --git a/cmd/apply_service.go b/cmd/apply_service.go index be3ed93..3add5ec 100644 --- a/cmd/apply_service.go +++ b/cmd/apply_service.go @@ -85,31 +85,7 @@ func (s applyService) run(ctx context.Context, req applyRequest) (runErr error) // connectAndPlan connects to the selected target when needed and builds the // plan used by the subsequent execution step. func (s applyService) connectAndPlan(ctx context.Context, workflow *resolvedWorkflow, dryRun bool) (openshell.Client, *plan.Plan, plan.CurrentState, error) { - var ( - client openshell.Client - err error - ) - client, err = s.newClient(ctx, workflow.Target) - if err != nil { - desc := targetDescription(workflow.Target) - if !dryRun { - return nil, nil, plan.CurrentState{}, fmt.Errorf("connecting to %s: %w", desc, err) - } - out := s.stderr - if out == nil { - out = io.Discard - } - fmt.Fprintf(out, "warning: %s unreachable: %v (rendering desired config only)\n", desc, err) - } - - planned, current, err := workflow.buildPlan(ctx, client) - if err != nil { - if client != nil { - _ = client.Close() - } - return nil, nil, plan.CurrentState{}, err - } - return client, planned, current, nil + return connectAndBuildPlan(ctx, s.newClient, workflow, dryRun, s.stderr) } // executeResolvedWorkflow runs the fully resolved and planned workflow through diff --git a/cmd/connect_plan.go b/cmd/connect_plan.go new file mode 100644 index 0000000..d260ae9 --- /dev/null +++ b/cmd/connect_plan.go @@ -0,0 +1,44 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "io" + + "github.com/stackrox/harness-openshell/internal/openshell" + "github.com/stackrox/harness-openshell/internal/plan" +) + +// connectAndBuildPlan uses the same target connection and offline fallback for +// plan and apply --dry-run. A failed read-only connection is represented by an +// uninspected plan; it must not be rendered as a confirmed missing provider. +func connectAndBuildPlan( + ctx context.Context, + newClient openshell.Factory, + workflow *resolvedWorkflow, + dryRun bool, + stderr io.Writer, +) (openshell.Client, *plan.Plan, plan.CurrentState, error) { + client, err := newClient(ctx, workflow.Target) + if err != nil { + desc := targetDescription(workflow.Target) + if !dryRun { + return nil, nil, plan.CurrentState{}, fmt.Errorf("connecting to %s: %w", desc, err) + } + if stderr == nil { + stderr = io.Discard + } + fmt.Fprintf(stderr, "warning: %s unreachable: %v (rendering desired config only)\n", desc, err) + } + + planned, current, err := workflow.buildPlan(ctx, client) + if err != nil { + planErr := fmt.Errorf("building workflow plan: %w", err) + if client != nil { + return nil, nil, plan.CurrentState{}, errors.Join(planErr, client.Close()) + } + return nil, nil, plan.CurrentState{}, planErr + } + return client, planned, current, nil +} diff --git a/cmd/connect_plan_test.go b/cmd/connect_plan_test.go new file mode 100644 index 0000000..a6c8a47 --- /dev/null +++ b/cmd/connect_plan_test.go @@ -0,0 +1,104 @@ +package cmd + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/stackrox/harness-openshell/internal/config" + "github.com/stackrox/harness-openshell/internal/openshell" + "github.com/stackrox/harness-openshell/internal/plan" + "github.com/stackrox/harness-openshell/internal/testutil" +) + +func TestConnectAndBuildPlanOfflineKeepsProvidersUninspected(t *testing.T) { + workflow := &resolvedWorkflow{ + Desired: &config.Harness{Spec: config.Spec{ + Sandbox: config.Sandbox{Providers: []string{"github"}}, + }}, + Target: openshell.Target{}, + } + var stderr strings.Builder + client, planned, current, err := connectAndBuildPlan( + context.Background(), + func(context.Context, openshell.Target) (openshell.Client, error) { + return nil, errors.New("gateway unavailable") + }, + workflow, + true, + &stderr, + ) + if err != nil { + t.Fatalf("connectAndBuildPlan: %v", err) + } + if client != nil { + t.Fatal("offline preview returned a client") + } + if current.Inspected { + t.Fatal("offline preview marked gateway as inspected") + } + if got := planned.Groups[1].Resources[0].Action; got != plan.ActionNotInspected { + t.Fatalf("provider action = %q, want %q", got, plan.ActionNotInspected) + } + if !strings.Contains(stderr.String(), "active gateway unreachable") { + t.Fatalf("warning = %q, want active gateway context", stderr.String()) + } +} + +func TestConnectAndBuildPlanNonDryRunRejectsConnectionFailure(t *testing.T) { + workflow := &resolvedWorkflow{Desired: &config.Harness{}, Target: openshell.Target{Gateway: "ci"}} + _, _, _, err := connectAndBuildPlan( + context.Background(), + func(context.Context, openshell.Target) (openshell.Client, error) { + return nil, errors.New("gateway unavailable") + }, + workflow, + false, + nil, + ) + if err == nil || !strings.Contains(err.Error(), `connecting to gateway "ci"`) { + t.Fatalf("error = %v, want connection context", err) + } +} + +type planErrorClient struct { + openshell.Client + planErr error + closeErr error +} + +func (c planErrorClient) Health(context.Context) (openshell.Health, error) { + return openshell.Health{}, c.planErr +} + +func (c planErrorClient) Close() error { return c.closeErr } + +func TestConnectAndBuildPlanPreservesPlanAndCloseErrors(t *testing.T) { + planErr := errors.New("state read failed") + closeErr := errors.New("close failed") + client := planErrorClient{ + Client: testutil.NewFake("default"), + planErr: planErr, + closeErr: closeErr, + } + workflow := &resolvedWorkflow{Desired: &config.Harness{}, Target: openshell.Target{Gateway: "ci"}} + + gotClient, gotPlan, gotState, err := connectAndBuildPlan( + context.Background(), + func(context.Context, openshell.Target) (openshell.Client, error) { return client, nil }, + workflow, + true, + nil, + ) + if gotClient != nil || gotPlan != nil || !reflect.DeepEqual(gotState, plan.CurrentState{}) { + t.Fatalf("failure result = client %v, plan %v, state %+v; want nil, nil, zero", gotClient, gotPlan, gotState) + } + if !errors.Is(err, planErr) || !errors.Is(err, closeErr) { + t.Fatalf("error = %v, want plan and close errors", err) + } + if !strings.Contains(err.Error(), "building workflow plan") { + t.Fatalf("error = %v, want plan operation context", err) + } +} diff --git a/cmd/plan.go b/cmd/plan.go index 3415b24..191affc 100644 --- a/cmd/plan.go +++ b/cmd/plan.go @@ -47,23 +47,13 @@ uses this same resolved desired object and action-decision engine.`, return err } - // An empty target means the active/default OpenShell gateway, so use - // the same factory path as apply. If it cannot be reached, preserve - // the read-only fallback and render the desired config without - // claiming that references are absent. - var client openshell.Client - client, err = newClient(cmd.Context(), workflow.Target) - if err != nil { - desc := targetDescription(workflow.Target) - fmt.Fprintf(cmd.ErrOrStderr(), "warning: %s unreachable: %v (rendering desired config only)\n", desc, err) - } else if client != nil { - defer client.Close() - } - - p, _, err := workflow.buildPlan(cmd.Context(), client) + client, p, _, err := connectAndBuildPlan(cmd.Context(), newClient, workflow, true, cmd.ErrOrStderr()) if err != nil { return err } + if client != nil { + defer client.Close() + } p = redactedPlan(p, workflow.Desired, workflow.Input) if format != formatTable {