refactor(simulate): move run state off Flags and exit codes out

cmd/crossplane/main.go

@@ -51,8 +51,8 @@ import (
 	"github.com/crossplane/cli/v2/cmd/crossplane/xr"
 	"github.com/crossplane/cli/v2/cmd/crossplane/xrd"
 	"github.com/crossplane/cli/v2/internal/config"
+	"github.com/crossplane/cli/v2/internal/exitcode"
 	"github.com/crossplane/cli/v2/internal/maturity"
-	isimulate "github.com/crossplane/cli/v2/internal/simulate"
 	"github.com/crossplane/cli/v2/internal/terminal"
 	clixpkg "github.com/crossplane/cli/v2/internal/xpkg"
 
@@ -122,7 +122,7 @@ func main() {
 	flagVal, err := configFlag(os.Args[1:])
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "crossplane: %v\n", err)
-		os.Exit(isimulate.ExitCodeUsage)
+		os.Exit(exitcode.ExitCodeUsage)
 	}
 	cfgPath := config.ResolvePath(flagVal)
 
@@ -181,7 +181,7 @@ func main() {
 	stop()
 	// A silent ExitError is a result, such as changes found, not an error,
 	// so it exits with its code and prints nothing.
-	code, silent := isimulate.SilentExit(err)
+	code, silent := exitcode.SilentExit(err)
 	if interrupted {
 		// Exit with the conventional 128+SIGINT status.
 		if err != nil && !silent {

cmd/crossplane/project/integration_plan_output_test.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	dp "github.com/crossplane/cli/v2/internal/diff/diffprocessor"
+	"github.com/crossplane/cli/v2/internal/exitcode"
 	"github.com/crossplane/cli/v2/internal/plan"
 	"github.com/crossplane/cli/v2/internal/simulate"
 	"github.com/crossplane/cli/v2/internal/testutils/integration"
@@ -486,7 +487,7 @@ func TestSimulateNameRejected(t *testing.T) {
 			reason:   "namespace/name with --namespace is ambiguous and is rejected.",
 			args:     []string{"--namespace=default", "--name=default/test-resource"},
 			wantErr:  "can't be combined with --namespace",
-			wantCode: simulate.ExitCodeUsage,
+			wantCode: exitcode.ExitCodeUsage,
 		},
 		"NamePartiallyUnmatchedFailsBeforeRendering": {
 			reason:   "one --name no composite matches fails the run, and no plan is rendered.",

cmd/crossplane/simulate/simulate.go

@@ -38,6 +38,7 @@ import (
 	k8 "github.com/crossplane/cli/v2/internal/diff/client/kubernetes"
 	"github.com/crossplane/cli/v2/internal/diffserver"
 	"github.com/crossplane/cli/v2/internal/docker"
+	"github.com/crossplane/cli/v2/internal/exitcode"
 	"github.com/crossplane/cli/v2/internal/kube"
 	"github.com/crossplane/cli/v2/internal/plan"
 	isimulate "github.com/crossplane/cli/v2/internal/simulate"
@@ -155,15 +156,15 @@ func (c *Cmd) Run(ctx context.Context, k *kong.Context, logger logging.Logger, s
 }
 
 // exitStatus ranks the plans' outcome: an error, which exits 1, when any
-// plan errored; otherwise a silent isimulate.ExitError that exits 3 when a plan
+// plan errored; otherwise a silent exitcode.ExitError that exits 3 when a plan
 // would change anything; otherwise nil.
 func exitStatus(plans []*plan.ResourcePlan) error {
 	s := plan.Summarize(plans)
 	if s.Errored > 0 {
 		return errors.Errorf("cannot plan %d of %d resources; see their errors in the output", s.Errored, len(plans))
 	}
 	if s.Changes() > 0 {
-		return isimulate.ExitError{Code: isimulate.ExitCodeChanges}
+		return exitcode.ExitError{Code: exitcode.ExitCodeChanges}
 	}
 	return nil
 }

cmd/crossplane/simulate/simulate_test.go

@@ -21,8 +21,8 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 
+	"github.com/crossplane/cli/v2/internal/exitcode"
 	"github.com/crossplane/cli/v2/internal/plan"
-	isimulate "github.com/crossplane/cli/v2/internal/simulate"
 )
 
 func TestExitStatus(t *testing.T) {
@@ -61,7 +61,7 @@ func TestExitStatus(t *testing.T) {
 			// other error with 1.
 			got := 0
 			err := exitStatus(tc.args.plans)
-			if code, ok := isimulate.SilentExit(err); ok {
+			if code, ok := exitcode.SilentExit(err); ok {
 				got = code
 			} else if err != nil {
 				got = 1

internal/simulate/exit.go → internal/exitcode/exit.go

@@ -14,17 +14,20 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package simulate
+// Package exitcode holds the exit statuses and the ExitError the commands
+// share.
+package exitcode
 
 import "errors"
 
-// Exit statuses the simulate commands share.
+// Exit statuses the commands share.
 const (
 	// ExitCodeUsage is the status kong exits with on a parse error. Kong does
 	// not export it, so it is mirrored here for the flag errors main finds
 	// before Parse.
 	ExitCodeUsage = 80
-	// ExitCodeSchemaValidation means only schema validation failed.
+	// ExitCodeSchemaValidation means only schema validation failed in a
+	// simulate command.
 	ExitCodeSchemaValidation = 2
 	// ExitCodeChanges is the status a simulate command exits with when the
 	// simulation succeeded and found changes.

internal/simulate/exit_test.go → internal/exitcode/exit_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package simulate
+package exitcode
 
 import (
 	"testing"

internal/simulate/annotate.go

@@ -367,7 +367,7 @@ const probe = "CROSSPLANE-PROBE-7f3c"
 // are the field's, because sigsyaml writes keys sorted, so the lines before
 // and after a value don't depend on it, and the line diff keeps lines whole.
 //
-// ponytail: a field is found only where setPath reaches it, so a keyed
+// NOTE: a field is found only where setPath reaches it, so a keyed
 // item or index the desired side lacks gets no mark in a modified diff, and a
 // keyed segment matching several items marks the first "+" line from the
 // first match to the last. The header, the summary and -o plan still say

internal/simulate/flags.go

@@ -19,34 +19,23 @@ package simulate
 import (
 	"time"
 
-	un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
 
 	"github.com/crossplane/crossplane-runtime/v2/pkg/errors"
 
 	dp "github.com/crossplane/cli/v2/internal/diff/diffprocessor"
-	"github.com/crossplane/cli/v2/internal/diffserver"
 	"github.com/crossplane/cli/v2/internal/kube"
 	xprender "github.com/crossplane/cli/v2/pkg/render"
 )
 
-// Flags are the flags, and the run state, of a simulate command.
+// Flags are the flags of a simulate command.
 type Flags struct {
 	// EngineFlags select the crossplane render backend, as for render. Its
 	// SessionID labels the diff server containers; the render's containers
 	// are labelled with the run's own ID, which reclaims them.
 	xprender.EngineFlags `prefix:"" set:"session_id_help=Session ID to apply as the simulate.crossplane.io/session label on diff server containers. Defaults to a random per-process ID. Ignored with --skip-plan."`
 
-	// Run sets these; nil with --skip-plan (servers also when Hooks.Router is set).
-	xrPlanner *XRPlanner
-	// keep is the equal-diff filter the engine and xrPlanner share.
-	keep    func(*un.Unstructured) bool
-	servers *diffserver.Manager
-	// noServersNoted is set once the "no diff servers" line is printed.
-	noServersNoted bool
-	// sel is Inputs.Selection, which Run copies in; zero without one.
-	sel      CompositionFlags
 	CacheDir string `env:"CROSSPLANE_XPKG_CACHE" help:"Directory for cached xpkg package contents (functions and providers)." name:"cache-dir"`
 
 	Context       string        `help:"Kubernetes context to simulate against. Defaults to the current context." predictor:"context"                                                          short:"c"`

internal/simulate/run.go

@@ -53,6 +53,7 @@ import (
 	dt "github.com/crossplane/cli/v2/internal/diff/result"
 	"github.com/crossplane/cli/v2/internal/diffserver"
 	"github.com/crossplane/cli/v2/internal/docker"
+	"github.com/crossplane/cli/v2/internal/exitcode"
 	"github.com/crossplane/cli/v2/internal/kube"
 	"github.com/crossplane/cli/v2/internal/project"
 	"github.com/crossplane/cli/v2/internal/terminal"
@@ -79,7 +80,7 @@ type Packages struct {
 // only ICs and Client. Run stops c.servers when it is set. Planning
 // shares the engine's clients' REST mapper and CRD cache, so a run discovers
 // the cluster once and fetches each CRD at most once.
-func (c *Flags) planOptions(sigCtx context.Context, k *kong.Context, logger logging.Logger, sp terminal.SpinnerPrinter, restCfg *rest.Config, clients *client.Clients, ns string, pkgs *Packages) ([]dp.ProcessorOption, error) {
+func (c *runner) planOptions(sigCtx context.Context, k *kong.Context, logger logging.Logger, sp terminal.SpinnerPrinter, restCfg *rest.Config, clients *client.Clients, ns string, pkgs *Packages) ([]dp.ProcessorOption, error) {
 	rmapper, crds := clients.Core.Mapper, clients.K8sClients.Schema
 	kubeClient, err := NewKubeClient(restCfg, rmapper)
 	if err != nil {
@@ -157,7 +158,7 @@ func providerSource(providers string, project bool) source {
 
 // noteNoServers prints NoteNoDiffServers to stderr once, for -o
 // diff, which has no notes; the other formats carry it in their notes.
-func (c *Flags) noteNoServers(k *kong.Context, notes []string) {
+func (c *runner) noteNoServers(k *kong.Context, notes []string) {
 	if c.noServersNoted || c.Output != string(renderer.OutputFormatDiff) || !slices.Contains(notes, NoteNoDiffServers) {
 		return
 	}
@@ -214,7 +215,7 @@ func changedFunctions(ctx context.Context, rc k8client.ResourceClient, digests m
 // diffCompositions returns the diff simulate runs for Compositions: their
 // impact on the composite resources in scope, rendered to k's writers.
 // Errors are wrapped with msg.
-func (c *Flags) diffCompositions(k *kong.Context, logger logging.Logger, proc dp.CompDiffProcessor, comps []*un.Unstructured, msg string) func(context.Context) (bool, error) {
+func (c *runner) diffCompositions(k *kong.Context, logger logging.Logger, proc dp.CompDiffProcessor, comps []*un.Unstructured, msg string) func(context.Context) (bool, error) {
 	return func(ctx context.Context) (bool, error) {
 		output, err := proc.DiffComposition(ctx, comps, c.sel.ns, c.sel.refs)
 
@@ -234,7 +235,7 @@ func (c *Flags) diffCompositions(k *kong.Context, logger logging.Logger, proc dp
 // crossplane-diff's xr command. A claim is simulated as its XR, but the
 // report names the input, so its result is listed under the claim's name
 // (crossplane/cli#367).
-func (c *Flags) diffComposites(k *kong.Context, logger logging.Logger, proc dp.DiffProcessor, xp *xpclient.Clients, ns string, xrs []*un.Unstructured) func(context.Context) (bool, error) {
+func (c *runner) diffComposites(k *kong.Context, logger logging.Logger, proc dp.DiffProcessor, xp *xpclient.Clients, ns string, xrs []*un.Unstructured) func(context.Context) (bool, error) {
 	return func(ctx context.Context) (bool, error) {
 		if err := checkComposites(ctx, xp.Definition, ns, xrs); err != nil {
 			return false, err
@@ -275,7 +276,7 @@ type processor interface {
 // exits 3; one that fails only schema validation returns one that exits 2.
 // ctx must not be cancelled by its owner until simulate returns, so a
 // cancellation reads as an interrupt.
-func (c *Flags) simulate(ctx context.Context, logger logging.Logger, initClients func(context.Context, logging.Logger) error, proc processor, diff func(context.Context) (bool, error)) (err error) {
+func (c *runner) simulate(ctx context.Context, logger logging.Logger, initClients func(context.Context, logging.Logger) error, proc processor, diff func(context.Context) (bool, error)) (err error) {
 	// An interrupted run reports the interruption, not the cancelled calls it
 	// caused. main exits 130 for it.
 	defer func() {
@@ -299,10 +300,10 @@ func (c *Flags) simulate(ctx context.Context, logger logging.Logger, initClients
 	hasDiffs, err := diff(ctx)
 	switch dp.DetermineExitCode(err, hasDiffs) {
 	case dp.ExitCodeSchemaValidation:
-		return ExitError{Err: err, Code: ExitCodeSchemaValidation}
+		return exitcode.ExitError{Err: err, Code: exitcode.ExitCodeSchemaValidation}
 	case dp.ExitCodeDiffDetected:
 		// Changes found are a result, not an error, so nothing is printed.
-		return ExitError{Code: ExitCodeChanges}
+		return exitcode.ExitError{Code: exitcode.ExitCodeChanges}
 	}
 
 	return err
@@ -342,7 +343,7 @@ func (c *CompositionFlags) scope() (string, []k8stypes.NamespacedName, error) {
 // XR and composition processors get the same options, so they share one advisory collector. fns are the project's
 // functions, of which embedded are the locally built ones, and ics rewrite
 // the other functions' images.
-func (c *Flags) processorOptions(k *kong.Context, logger logging.Logger, creds []corev1.Secret, fns []pkgv1.Function, embedded map[string]name.Digest, ics []pkgv1beta1.ImageConfig) []dp.ProcessorOption {
+func (c *runner) processorOptions(k *kong.Context, logger logging.Logger, creds []corev1.Secret, fns []pkgv1.Function, embedded map[string]name.Digest, ics []pkgv1beta1.ImageConfig) []dp.ProcessorOption {
 	c.SetDefaultCrossplaneDockerNetwork(fns)
 
 	opts := []dp.ProcessorOption{
@@ -416,7 +417,7 @@ func functionProviderFactory(fns []pkgv1.Function, embedded map[string]name.Dige
 // writing to k's stdout and stderr. The processors return reports; the command
 // renders them with these options. Diffs are colorized only when a person
 // reads them: on a terminal, or as if on one under the test hooks.
-func (c *Flags) diffOptions(k *kong.Context) renderer.DiffOptions {
+func (c *runner) diffOptions(k *kong.Context) renderer.DiffOptions {
 	opts := renderer.DefaultDiffOptions()
 	opts.UseColors = !c.NoColor && (c.Hooks != nil && c.Hooks.Terminal || isTerminal(k.Stdout))
 	opts.Compact = c.Compact
@@ -427,7 +428,7 @@ func (c *Flags) diffOptions(k *kong.Context) renderer.DiffOptions {
 
 // providerPlans returns the provider plans the run collected, or nil when
 // planning was skipped.
-func (c *Flags) providerPlans() *ProviderPlans {
+func (c *runner) providerPlans() *ProviderPlans {
 	if c.xrPlanner == nil {
 		return nil
 	}
@@ -447,7 +448,7 @@ func planNotes(skipPlan, planned bool, routed, considered int) []string {
 	return nil
 }
 
-func (c *Flags) notes() []string {
+func (c *runner) notes() []string {
 	if c.xrPlanner == nil {
 		return planNotes(c.SkipPlan, false, 0, 0)
 	}
@@ -456,7 +457,7 @@ func (c *Flags) notes() []string {
 
 // renderXRReport writes an xr run's report in the format -o selects, as
 // renderReport does.
-func (c *Flags) renderXRReport(k *kong.Context, logger logging.Logger, r *dt.XRReport, runErr error) (bool, error) {
+func (c *runner) renderXRReport(k *kong.Context, logger logging.Logger, r *dt.XRReport, runErr error) (bool, error) {
 	plans := c.providerPlans()
 	return c.renderReport(k, logger, runErr, FromXRReport(r, plans, c.keep), r.HasChanges(), dt.XRDryRunWarnings(r.Groups), func(opts renderer.DiffOptions) error {
 		// Annotated even without plans, so a Secret's values are redacted.
@@ -467,7 +468,7 @@ func (c *Flags) renderXRReport(k *kong.Context, logger logging.Logger, r *dt.XRR
 
 // renderCompReport writes a composition run's report in the format -o
 // selects, as renderReport does.
-func (c *Flags) renderCompReport(k *kong.Context, logger logging.Logger, o *dt.CompDiffOutput, runErr error) (bool, error) {
+func (c *runner) renderCompReport(k *kong.Context, logger logging.Logger, o *dt.CompDiffOutput, runErr error) (bool, error) {
 	plans := c.providerPlans()
 	return c.renderReport(k, logger, runErr, FromCompReport(o, plans, c.keep), o.HasChanges(), dt.CompDryRunWarnings(o), func(opts renderer.DiffOptions) error {
 		opts.MinimizeComposition = c.sel.MinimizeComposition
@@ -482,7 +483,7 @@ func (c *Flags) renderCompReport(k *kong.Context, logger logging.Logger, o *dt.C
 // whether the run changed anything, provider plans included, and runErr
 // with any render failure joined to it, so neither is lost. changed is
 // whether the report itself has changes.
-func (c *Flags) renderReport(k *kong.Context, logger logging.Logger, runErr error, res Result, changed bool, warnings []dt.OutputWarning, renderDiff func(renderer.DiffOptions) error) (bool, error) {
+func (c *runner) renderReport(k *kong.Context, logger logging.Logger, runErr error, res Result, changed bool, warnings []dt.OutputWarning, renderDiff func(renderer.DiffOptions) error) (bool, error) {
 	opts := c.diffOptions(k)
 	plans := c.providerPlans()
 	if plans != nil {
@@ -512,7 +513,7 @@ func (c *Flags) renderReport(k *kong.Context, logger logging.Logger, runErr erro
 // writes derived, the warnings derived from the report, to stderr, as the
 // engine's renderers do. The run's other warnings went to stderr when they
 // were raised.
-func (c *Flags) renderPlan(k *kong.Context, res Result, derived []dt.OutputWarning) error {
+func (c *runner) renderPlan(k *kong.Context, res Result, derived []dt.OutputWarning) error {
 	r, err := NewRenderer(c.Output)
 	if err != nil {
 		return err
@@ -556,6 +557,27 @@ func isTerminal(w io.Writer) bool {
 // or composite resource or claim files, with the cluster's Compositions and
 // functions.
 func (c *Flags) Run(sigCtx context.Context, k *kong.Context, logger logging.Logger, sp terminal.SpinnerPrinter, in *Inputs) error {
+	return (&runner{Flags: c}).run(sigCtx, k, logger, sp, in)
+}
+
+// runner is the state of one Run: the flags it was configured by, and what
+// planning sets up along the way.
+type runner struct {
+	*Flags
+
+	// xrPlanner and servers are set by planOptions; nil with --skip-plan
+	// (servers also when Hooks.Router is set).
+	xrPlanner *XRPlanner
+	// keep is the equal-diff filter the engine and xrPlanner share.
+	keep    func(*un.Unstructured) bool
+	servers *diffserver.Manager
+	// noServersNoted is set once the "no diff servers" line is printed.
+	noServersNoted bool
+	// sel is Inputs.Selection, which run copies in; zero without one.
+	sel CompositionFlags
+}
+
+func (c *runner) run(sigCtx context.Context, k *kong.Context, logger logging.Logger, sp terminal.SpinnerPrinter, in *Inputs) error {
 	if in.Selection != nil {
 		c.sel = *in.Selection
 	}