Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).

### Fixed

- `--min-savings-pct` now rejects values below 0 or above 100 at validation.
Previously a negative value was accepted as no filter and a value above 100
silently dropped every recommendation (#2161).
- `configure-gcp`: skipping (or giving an unknown answer at) the "Create Service
Account" step no longer continues with the default
`cudly-service-account@<project>` email that was never created. The wizard now
Expand Down
12 changes: 9 additions & 3 deletions cmd/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func requireFinite(name string, v float64) error {
return nil
}

// validateCoverageAndSavingsPct checks --coverage (finite, 0-100) and that
// --min-savings-pct is finite. target-coverage and min-pool-size call
// validateCoverageAndSavingsPct checks that --coverage and --min-savings-pct
// are finite and within 0-100. target-coverage and min-pool-size call
// requireFinite in their own validators.
func validateCoverageAndSavingsPct() error {
if err := requireFinite("coverage", toolCfg.Coverage); err != nil {
Expand All @@ -155,7 +155,13 @@ func validateCoverageAndSavingsPct() error {
if toolCfg.Coverage < 0 || toolCfg.Coverage > 100 {
return fmt.Errorf("coverage percentage must be between 0 and 100, got: %.2f", toolCfg.Coverage)
}
return requireFinite("min-savings-pct", toolCfg.MinSavingsPct)
if err := requireFinite("min-savings-pct", toolCfg.MinSavingsPct); err != nil {
return err
}
if toolCfg.MinSavingsPct < 0 || toolCfg.MinSavingsPct > 100 {
return fmt.Errorf("min-savings-pct must be between 0 and 100, got: %.2f", toolCfg.MinSavingsPct)
}
return nil
}

// validateMinPoolSize checks that --min-pool-size is a non-negative whole number.
Expand Down
10 changes: 7 additions & 3 deletions cmd/validators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -815,10 +815,11 @@ func TestCheckDuplicates_HonorsIdempotencyWindow(t *testing.T) {
}
}

// TestValidateNumericFlagsRejectNonFinite drives the real flag set through
// TestValidateNumericFlagsRejectInvalid drives the real flag set through
// rootCmd.ParseFlags (cobra/pflag parse "NaN" and "Inf" as floats) and then
// the validators. Nothing here calls runTool, so no AWS client is built.
func TestValidateNumericFlagsRejectNonFinite(t *testing.T) {
// the validators, covering non-finite and out-of-range values. Nothing here
// calls runTool, so no AWS client is built.
func TestValidateNumericFlagsRejectInvalid(t *testing.T) {
saved := toolCfg
reset := func() {
toolCfg = Config{}
Expand Down Expand Up @@ -848,6 +849,9 @@ func TestValidateNumericFlagsRejectNonFinite(t *testing.T) {
{name: "min-pool-size NaN", args: []string{"--min-pool-size", "NaN"}, wantErr: "min-pool-size must be a finite number, got: NaN"},
{name: "min-savings-pct NaN", args: []string{"--min-savings-pct", "NaN"}, wantErr: "min-savings-pct must be a finite number, got: NaN"},
{name: "min-savings-pct Inf", args: []string{"--min-savings-pct", "Inf"}, wantErr: "min-savings-pct must be a finite number, got: +Inf"},
{name: "min-savings-pct negative", args: []string{"--min-savings-pct", "-1"}, wantErr: "min-savings-pct must be between 0 and 100, got: -1.00"},
{name: "min-savings-pct above 100", args: []string{"--min-savings-pct", "100.5"}, wantErr: "min-savings-pct must be between 0 and 100, got: 100.50"},
{name: "min-savings-pct bounds stay valid", args: []string{"--min-savings-pct", "100"}},
{name: "overflow rejected by the flag parser", args: []string{"--target-coverage", "1e999"}, parseErr: true},
{name: "explicit target 0 stays valid", args: []string{"--target-coverage", "0"}},
{name: "valid values pass", args: []string{"--target-coverage", "95", "--coverage", "50", "--min-pool-size", "2", "--min-savings-pct", "5"}},
Expand Down
2 changes: 1 addition & 1 deletion docs/cli/filtering.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ cudly --services rds --min-count 3
--min-savings-pct <float> Minimum savings percentage (0 = no filter)
```

Drop recommendations whose estimated savings percentage falls below this threshold.
Drop recommendations whose estimated savings percentage falls below this threshold. The value must be between 0 and 100.

**Important naming distinction:** `--min-savings-pct` is a **percentage** value (e.g. `10` means "at least 10% savings"). This is different from the GUI and API `min_savings` parameter, which filters by **dollar amount**. A value of `30` in `--min-savings-pct` means "30% savings", but `min_savings=30` in the API means "$30 in savings". Mixing up the two by copying a CLI value into the GUI filter (or vice versa) produces silent, incorrect filtering.

Expand Down
Loading