Skip to content

Improve CLI flags parsing#624

Open
frjcomp wants to merge 16 commits intomainfrom
pr-621
Open

Improve CLI flags parsing#624
frjcomp wants to merge 16 commits intomainfrom
pr-621

Conversation

@frjcomp
Copy link
Copy Markdown
Collaborator

@frjcomp frjcomp commented May 6, 2026

No description provided.

Copilot AI review requested due to automatic review settings May 6, 2026 12:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves configuration/flag handling across the CLI by standardizing explicit AutoBindFlags mappings, documenting precedence (flags > env > file > defaults), and adding a pipeleek config gen command plus refreshed example configuration output.

Changes:

  • Introduces pipeleek config gen and a Makefile target to regenerate pipeleek.example.yaml.
  • Refactors multiple scan commands to use centralized flagBindings maps and read additional options from Viper keys.
  • Adds tests for config source precedence and for flag-binding coverage/behavior.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
pkg/config/loader_priority_chain_test.go Adds tests asserting config precedence order across flags/env/file/defaults.
pkg/config/gen/gen.go Adds example-config generator (currently implemented as a static template).
pkg/config/gen/gen_test.go Adds validation tests for generated example config content/structure.
pkg/config/config_coverage_test.go Adds tests documenting/validating binding behaviors and a scan-flag “coverage” checklist.
pipeleek.example.yaml Updates example config to reflect new precedence docs and expanded key/flag/envvar references.
Makefile Adds gen-config target to regenerate pipeleek.example.yaml via the CLI.
internal/cmd/root.go Registers new config command group and tweaks default log-level message.
internal/cmd/configcmd/config.go Adds pipeleek config root command.
internal/cmd/configcmd/gen/gen.go Implements pipeleek config gen command (stdout or file output).
internal/cmd/configcmd/gen/gen_test.go Adds basic command tests for config gen.
internal/cmd/configcmd/gen/file.go Adds file-writing helper for generated config output.
internal/cmd/gitlab/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/gitlab/scan/scan_test.go Adds binding and env-var tests for GitLab scan keys.
internal/cmd/github/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/github/scan/scan_flag_test.go Adds binding and env-var tests for GitHub scan keys.
internal/cmd/bitbucket/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/bitbucket/scan/scan_test.go Adds binding and env-var tests for BitBucket scan keys.
internal/cmd/devops/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/devops/scan/scan_test.go Adds binding and env-var tests for DevOps scan keys.
internal/cmd/gitea/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/gitea/scan/scan_test.go Adds binding and env-var tests for Gitea scan keys.
internal/cmd/jenkins/scan/scan.go Moves scan flag bindings into a map and reads more scan options from config keys (incl. hit-timeout parsing).
internal/cmd/jenkins/scan/scan_test.go Adds binding and env-var tests for Jenkins scan keys.
internal/cmd/gitlab/tf/tf.go Moves TF flag bindings into a map and reads hit-timeout from config key.
internal/cmd/gitlab/tf/tf_test.go Adds binding and env-var tests for GitLab TF keys.
internal/cmd/circle/scan/scan.go Introduces explicit flagBindings map and uses it for AutoBindFlags in Circle scan.
internal/cmd/circle/scan/scan_test.go Adds binding and env-var tests for Circle scan keys.
docs/introduction/configuration.md Updates docs to recommend pipeleek config gen and links to the example config.

Comment thread pkg/config/gen/gen.go Outdated
Comment thread pkg/config/gen/gen_test.go Outdated
Comment thread pkg/config/config_coverage_test.go Outdated
Comment thread internal/cmd/root.go Outdated
Comment thread internal/cmd/circle/scan/scan.go
Comment thread internal/cmd/configcmd/gen/file.go Outdated
Comment thread pipeleek.example.yaml Outdated
@frjcomp
Copy link
Copy Markdown
Collaborator Author

frjcomp commented May 6, 2026

Add config get <key.id> and config set <key.id> so one can set config file values dynamically. If no leave key was defined, return the object below

@frjcomp
Copy link
Copy Markdown
Collaborator Author

frjcomp commented May 6, 2026

refactor gen with an actual yml lib

Copilot AI and others added 9 commits May 6, 2026 15:01
…tion

- Documents 'pipeleek config gen' command for generating config templates
- Explains usage with --output flag and make target
- Placed after Quick Start section for easy discovery by new users
- Add loader_priority_chain_test.go: 9 tests verifying the complete configuration precedence order
  * FlagOverridesAll: CLI flag > env var > config file > default
  * EnvVarOverridesFileAndDefault: env var > config file > default
  * ConfigFileOverridesDefault: config file > default
  * PartialOverrides: selective override behavior for multiple keys
  * AllLevelsSet: complete precedence verification for single key
  * EmptyFlagDoesNotOverride: empty flag doesn't override lower sources
  * MultipleKeysIndependent: precedence applied per-key independently

- Add config_coverage_test.go: 11 tests verifying flag binding completeness
  * TestScanCommandFlagCoverage: documents expected flags for each platform's scan command
  * TestAutoBindFlagsRejectsBadMappings: edge cases (nonexistent flags, empty mappings, dash conversion)
  * TestBindFlagsWithSubcommands: inherited flags from parent commands
  * TestBoolFlagBinding: boolean flag handling
  * TestIntFlagBinding: integer flag handling
  * TestStringSliceFlagBinding: string slice flag handling
  * TestRequireConfigKeysWithBoundFlags: required key validation

Tests verified at all levels:
- Flag bindings work correctly for all types (string, bool, int, string slice)
- Priority order is correctly enforced
- Environment variables properly override config files
- CLI flags properly override environment variables and config files
- Multiple keys can have independent precedence resolution
- Non-existent flags don't cause errors
- Empty/unset flags don't override lower priority sources
…ent tests

Fixes two binding bugs:
- circle/scan: artifacts flag was missing from flagBindings map (can't be set via config/env)
- gitlab/tf: hit-timeout was in map but code read from struct, ignoring config/env values

Refactoring for all 8 scan commands + tf:
- Extract inline AutoBindFlags maps into package-level 'flagBindings' vars
- Tests now use production bindings (single source of truth)
- Reduces test duplication and catches missed bindings automatically

New binding coverage tests:
- TestX_AllDefinedFlagsAreBound: Verifies every CLI flag is in flagBindings
- Tests run for: gitlab/scan, github/scan, bitbucket/scan, devops/scan, gitea/scan, jenkins/scan, circle/scan, gitlab/tf
- Updated all existing flag tests to use shared flagBindings var
- Added tests for circle/scan and gitlab/tf (previously missing)

All per-command binding tests now verify:
1. Every defined CLI flag has a config binding entry
2. Flag values bind correctly to Viper keys via AutoBindFlags
3. Environment variables (PIPELEEK_*) correctly populate config values
4. Config file values are correctly retrieved

This prevents future flag additions from silently bypassing config management.
- Add CommandSetup builder pattern to eliminate repetitive flag binding,
  validation, and key requirement checks across commands
- Add BindingsFromFlags() utility to auto-generate flag->config mappings
  reducing duplication of binding maps
- Add ParseBool() convenience function for boolean config values
- Refactor gitlab/enum and gitlab/schedule to demonstrate new pattern
- Reduces typical command Run function from ~20 lines to ~8 lines
- All tests pass; no behavioral changes
Replace repetitive AutoBindFlags/RequireConfigKeys/ValidateX boilerplate
across 30 command files with the fluent CommandSetup builder pattern:

  config.NewCommandSetup(cmd).
    WithFlagBindings(flagBindings).
    RequireKeys(...).
    AddValidator(...).
    MustBind()

Affected platforms: gitlab, github, gitea, bitbucket, circle, devops, jenkins

Also removes unused zerolog/log imports from files where log calls were
eliminated by the refactor.
frjcomp added 4 commits May 7, 2026 09:22
- Updated pkg/config/gen/paths.go to exclude non-leaf paths (intermediate
  command groups) from allowed config paths. Only flag leaves and empty
  containers are now allowed (e.g., gitlab.cicd.yaml.project allowed, but
  gitlab.cicd.yaml not allowed)
- Updated pkg/config/gen/paths_test.go test expectations to verify only
  leaf paths are in allowed paths
- Fixed pkg/config/loader.go YAML serialization to use yaml.v3 encoder
  with 2-space indentation for proper formatting
- Both 'config get' and 'config set' now properly reject intermediate
  non-leaf config keys

Tests: All unit tests (gen, get, set, gen) and e2e tests pass
…et examples

- Updated config gen Quick Start to use --output flag instead of stdout
- Added note explaining why --output is required (log output breaks YAML)
- Added new 'Managing Config Values' section with get/set examples
- Documented type inference for config set values
- Updated Full Example to reference config gen with --output flag
Move ResolveReadConfigPath() and ResolveWriteConfigPath() calls to after
validation checks in config get and set commands. This ensures that if
validation fails, no log messages (info/warn) are written to stderr before
the error is returned.

This prevents terminal state corruption that could occur when error output
contained log messages mixed with error text.

Now validation errors are returned cleanly without any prior logging:
  config set gitlab.cicd.yamlette val  → Error only (no logs)
  config get gitlab.notreal             → Error only (no logs)
The leaf-only schema validation introduced for config set also blocked
section reads in config get (e.g. ), which contradicted
existing docs.

- Add IsAllowedReadConfigPath() for read semantics:
  - allow exact leaf keys
  - allow section prefixes that contain valid leaves
- Use read validation in config get
- Add tests for read validation and section retrieval

This keeps config set strict (leaf-only) while restoring documented
config get behavior for sections.
@frjcomp
Copy link
Copy Markdown
Collaborator Author

frjcomp commented May 7, 2026

unify --namespace --org etc. on gitlab commands so its uniform

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants