diff --git a/docs/roadmap/audit/catalog.md b/docs/roadmap/audit/catalog.md index 0bbb73d..9b75934 100644 --- a/docs/roadmap/audit/catalog.md +++ b/docs/roadmap/audit/catalog.md @@ -1,6 +1,6 @@ # Consolidated audit catalog -Status: **8 distinct open findings: 8 P3**, consolidated from 30 +Status: **7 distinct open findings: 7 P3**, consolidated from 30 numbered entries in seven audit documents. Reviewed audit state: `db732e3a1de55c844f77870e1c6db683c2685d5d`. @@ -388,6 +388,10 @@ or a helper for each loop merely to shorten the function. ## F18 — P3: Configuration preflight interleaves shared and operation-specific rules +**Complete.** Operation checks, scope selection and two-sided input preparation +have separate private functions; comparison flags are grouped. Focused preflight, +help, scope, override, show and comparison tests pass. + The configuration command builder and `prepareConfigOperation` mix flag combinations, help selection, capabilities, pointer parsing, plan assembly, ordinary inputs, and independent comparison inputs. Understanding one operation diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 52dc1aa..aa1680f 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Audit remediation implementation plan -Status: in progress; Stages 1–17 complete. +Status: in progress; Stages 1–18 complete. Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint, intervening changes recorded decisions and retired the completed older @@ -585,6 +585,11 @@ actual emitted paths rather than every valid full path. ## Stage 18 — Separate configuration operation preparation +**Complete.** Operation checks, scope/capability selection, comparison input +preparation, and comparison flags have distinct private responsibilities. +Focused preflight, help, scope, show, override and comparison tests passed in +the offline aligned Go 1.27.0 workspace with `-mod=readonly`. + Findings: F18. Source: ckconfig `input.go` and its command builder. 1. Extract operation-specific option checks, shared scope/capability selection, diff --git a/integrations/ckconfig/input.go b/integrations/ckconfig/input.go index 4ee12ec..fed78d2 100644 --- a/integrations/ckconfig/input.go +++ b/integrations/ckconfig/input.go @@ -85,18 +85,7 @@ func newConfigOperationCommand[I, S any](prepared *preparedSuite[I, S], operatio }), } if operation == Diff { - flags = append(flags, - commandkit.BoolFlag(commandkit.FlagMeta{Name: "details", Usage: "Show native before and after difference evidence."}, false, func(input **configCommandInput[I, S], value commandkit.Supplied[bool]) { (*input).details = value }), - commandkit.BoolFlag(commandkit.FlagMeta{Name: "check", Usage: "Return status 3 when differences are found."}, false, func(input **configCommandInput[I, S], value commandkit.Supplied[bool]) { (*input).check = value }), - commandkit.ValueFlag(commandkit.FlagMeta{Name: "against-config", Usage: "Configuration source for the right side.", Completion: commandkit.CompleteFiles()}, (*string)(nil), parseConfigString, - func(input **configCommandInput[I, S], value commandkit.Supplied[string]) { - (*input).against.Config = value - }), - commandkit.ValueFlag(commandkit.FlagMeta{Name: "against-overlay", Usage: "Overlay source for the right side.", Completion: commandkit.CompleteFiles()}, (*string)(nil), parseConfigString, - func(input **configCommandInput[I, S], value commandkit.Supplied[string]) { - (*input).against.Overlay = value - }), - ) + flags = append(flags, comparisonOperationFlags[I, S]()...) } if operation == Validate { flags = append(flags, @@ -169,6 +158,21 @@ func newConfigOperationCommand[I, S any](prepared *preparedSuite[I, S], operatio }) } +func comparisonOperationFlags[I, S any]() []commandkit.Flag[*configCommandInput[I, S]] { + return []commandkit.Flag[*configCommandInput[I, S]]{ + commandkit.BoolFlag(commandkit.FlagMeta{Name: "details", Usage: "Show native before and after difference evidence."}, false, func(input **configCommandInput[I, S], value commandkit.Supplied[bool]) { (*input).details = value }), + commandkit.BoolFlag(commandkit.FlagMeta{Name: "check", Usage: "Return status 3 when differences are found."}, false, func(input **configCommandInput[I, S], value commandkit.Supplied[bool]) { (*input).check = value }), + commandkit.ValueFlag(commandkit.FlagMeta{Name: "against-config", Usage: "Configuration source for the right side.", Completion: commandkit.CompleteFiles()}, (*string)(nil), parseConfigString, + func(input **configCommandInput[I, S], value commandkit.Supplied[string]) { + (*input).against.Config = value + }), + commandkit.ValueFlag(commandkit.FlagMeta{Name: "against-overlay", Usage: "Overlay source for the right side.", Completion: commandkit.CompleteFiles()}, (*string)(nil), parseConfigString, + func(input **configCommandInput[I, S], value commandkit.Supplied[string]) { + (*input).against.Overlay = value + }), + } +} + func operationDescription(operation Operation) string { if operation == Explain { return "Use --list to discover paths or --path POINTER to explain one path." @@ -193,35 +197,38 @@ func operationDeclaration(operation Operation) (string, string) { } } -func prepareConfigOperation[I, S any](prepared *preparedSuite[I, S], operation Operation, input *configCommandInput[I, S]) error { +func checkConfigOperationOptions[I, S any](operation Operation, input *configCommandInput[I, S]) (bool, error) { if operation == Validate && !input.details.Value && (len(input.severities) > 0 || len(input.codes) > 0) { - return fmt.Errorf("--severity and --code require --details") + return false, fmt.Errorf("--severity and --code require --details") } if operation == Explain { if input.list.Value && input.path.Set { - return fmt.Errorf("--list and --path cannot be combined") + return false, fmt.Errorf("--list and --path cannot be combined") } if !input.list.Value && (input.historical.Set || input.under.Set || input.explicit.Set) { - return fmt.Errorf("--historical, --under, and --explicit require --list") + return false, fmt.Errorf("--historical, --under, and --explicit require --list") } if !input.list.Value && !input.path.Set { - input.plan = &configOperationPlan[I, S]{prepared: prepared, operation: operation, help: true} - return nil + return true, nil } } + return false, nil +} + +func selectConfigScopes[I, S any](prepared *preparedSuite[I, S], operation Operation, input *configCommandInput[I, S]) (string, []*preparedScope[I, S], error) { scopeName := "app" if input.scope.Set { scopeName = input.scope.Value } if scopeName == "" { - return fmt.Errorf("configuration scope must not be empty") + return "", nil, fmt.Errorf("configuration scope must not be empty") } if scopeName == "all" && operation != Validate && operation != Show { - return fmt.Errorf("scope all is supported only for validate and show") + return "", nil, fmt.Errorf("scope all is supported only for validate and show") } if scopeName == "all" && operation == Show && input.path.Set { - return fmt.Errorf("path-selected show requires one scope") + return "", nil, fmt.Errorf("path-selected show requires one scope") } var selected []*preparedScope[I, S] @@ -230,24 +237,62 @@ func prepareConfigOperation[I, S any](prepared *preparedSuite[I, S], operation O } else if scope, exists := prepared.byName[scopeName]; exists { selected = []*preparedScope[I, S]{scope} } else { - return fmt.Errorf("unknown configuration scope %q (available: %s)", scopeName, prepared.scopeChoices) + return "", nil, fmt.Errorf("unknown configuration scope %q (available: %s)", scopeName, prepared.scopeChoices) } for _, scope := range selected { if !containsOperation(scope.scope.Operations, operation) { - return fmt.Errorf("configuration scope %q does not support %s (available: %s)", scope.scope.Name, operation, joinOperations(scope.scope.Operations)) + return "", nil, fmt.Errorf("configuration scope %q does not support %s (available: %s)", scope.scope.Name, operation, joinOperations(scope.scope.Operations)) } if (len(input.selection.Overrides) > 0 || len(input.against.Overrides) > 0) && !scope.scope.Features.Overrides { - return fmt.Errorf("configuration scope %q does not support overrides", scope.scope.Name) + return "", nil, fmt.Errorf("configuration scope %q does not support overrides", scope.scope.Name) } if operation == Explain && input.list.Value && !scope.scope.Features.PathDiscovery { - return fmt.Errorf("configuration scope %q does not support path discovery", scope.scope.Name) + return "", nil, fmt.Errorf("configuration scope %q does not support path discovery", scope.scope.Name) } if operation == Show && input.path.Set && !scope.scope.Features.PathShow { - return fmt.Errorf("configuration scope %q does not support path-selected show", scope.scope.Name) + return "", nil, fmt.Errorf("configuration scope %q does not support path-selected show", scope.scope.Name) } } + return scopeName, selected, nil +} + +func prepareComparisonInputs[I, S any](prepared *preparedSuite[I, S], input *configCommandInput[I, S], scope *preparedScope[I, S], plan *configOperationPlan[I, S]) error { + input.selection = cloneSelection(input.selection) + input.against = cloneSelection(input.against) + left, err := prepareScopeInput(prepared, input.values, scope, scope.bindings, input.selection, true) + if err != nil { + return fmt.Errorf("configuration scope %q left input: %w", scope.scope.Name, err) + } + right, err := prepareScopeInput(prepared, input.againstValues, scope, scope.againstBindings, input.against, true) + if err != nil { + return fmt.Errorf("configuration scope %q right input: %w", scope.scope.Name, err) + } + plan.comparison = &preparedComparison[I, S]{ + scope: scope, + request: ComparisonRequest[I]{ + Left: ComparisonSide[I]{Selection: input.selection, Input: left}, + Right: ComparisonSide[I]{Selection: input.against, Input: right}, + }, + } + return nil +} + +func prepareConfigOperation[I, S any](prepared *preparedSuite[I, S], operation Operation, input *configCommandInput[I, S]) error { + help, err := checkConfigOperationOptions(operation, input) + if err != nil { + return err + } + if help { + input.plan = &configOperationPlan[I, S]{prepared: prepared, operation: operation, help: true} + return nil + } + scopeName, selected, err := selectConfigScopes(prepared, operation, input) + if err != nil { + return err + } + var path configkit.Path if (operation == Explain || operation == Show) && input.path.Set { parsed, err := configkit.ParsePath(input.path.Value) @@ -283,23 +328,8 @@ func prepareConfigOperation[I, S any](prepared *preparedSuite[I, S], operation O scopes: make([]selectedScopeInput[I, S], 0, len(selected)), } if operation == Diff { - scope := selected[0] - input.selection = cloneSelection(input.selection) - input.against = cloneSelection(input.against) - left, err := prepareScopeInput(prepared, input.values, scope, scope.bindings, input.selection, true) - if err != nil { - return fmt.Errorf("configuration scope %q left input: %w", scope.scope.Name, err) - } - right, err := prepareScopeInput(prepared, input.againstValues, scope, scope.againstBindings, input.against, true) - if err != nil { - return fmt.Errorf("configuration scope %q right input: %w", scope.scope.Name, err) - } - plan.comparison = &preparedComparison[I, S]{ - scope: scope, - request: ComparisonRequest[I]{ - Left: ComparisonSide[I]{Selection: input.selection, Input: left}, - Right: ComparisonSide[I]{Selection: input.against, Input: right}, - }, + if err := prepareComparisonInputs(prepared, input, selected[0], plan); err != nil { + return err } input.plan = plan return nil