Skip to content

Conversation

@EyalDelarea
Copy link
Contributor

@EyalDelarea EyalDelarea commented Apr 15, 2025

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

🔧 Summary

This PR updates the handling of boolean flags (BoolFlag) in the fillFlagMaps function.

✅ Behavior Change

Previously, BoolFlags were only added to the internal context (c.boolFlags) if they were explicitly set by the user:

if boolFlag, ok := flag.(BoolFlag); ok && baseContext.IsSet(boolFlag.Name) {

This caused flags with a default value of true to be skipped unless manually provided.

The updated logic:

if boolFlag, ok := flag.(BoolFlag); ok {
    val := getValueForBoolFlag(boolFlag, baseContext)
    if baseContext.IsSet(boolFlag.Name) || val {
        c.boolFlags[boolFlag.Name] = val
    }
}

Now, a boolean flag is included if:

  • It was explicitly set by the user (baseContext.IsSet(...))
  • OR its resolved value is true (e.g., default true)

This ensures all effective boolean flags are passed to plugin commands.

🛠 Fixes

This change resolves issues that broke certain CLI-Security plugin commands that rely on BoolFlags with DefaultValue = true.

@EyalDelarea EyalDelarea added the ignore for release Automatically generated release notes label Apr 15, 2025
@EyalDelarea EyalDelarea changed the title Remove default flag set Include default true bool flags in CLI flag mapping Apr 15, 2025
@github-actions
Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@EyalDelarea EyalDelarea merged commit 14320a8 into jfrog:dev Apr 16, 2025
7 checks passed
@EyalDelarea EyalDelarea added bug Something isn't working and removed ignore for release Automatically generated release notes labels Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant