fix(options): validate flags that are currently accepted at any value

[mrueg]

What this PR does / why we need it:

Three related gaps in Options.Validate(), each of which lets a misconfiguration through silently.

1. The general validations never ran without --node

Validate() returned early when --node was empty, leaving every check below that guard unreachable for the ordinary cluster-wide deployment:

shardableResource := "pods"
if o.Node == "" {
	return nil            // <- everything below is skipped
}
...
if o.AutoGoMemlimitRatio <= 0.0 || o.AutoGoMemlimitRatio > 1.0 { ... }
if o.ObjectLimit < 0 { ... }

So --auto-gomemlimit-ratio and --object-limit were accepted at any value unless --node happened to be set, even though their flag help documents the bounds. The values are then used: the ratio reaches memlimit.SetGoMemLimitWithOpts(memlimit.WithRatio(...)), where anything above 1 sets GOMEMLIMIT above the detected container limit and defeats the flag; the object limit reaches the list options as Limit. Demonstrated on main: without --node, AutoGoMemlimitRatio = 5.0 and ObjectLimit = -1 both pass; setting --node makes the identical values fail.

The resource check is now scoped to the node case and the rest run on every path.

2. NaN passed the ratio range check

--auto-gomemlimit-ratio=NaN parses as a valid float64, and every comparison against NaN is false, so it satisfied both <= 0.0 and > 1.0 and reached memlimit.WithRatio. Rejected explicitly now. (Thanks to the review comment for catching this.)

3. A shard configuration matching no objects was accepted

The shard an object belongs to is jump.Hash(fnv64a(uid), totalShards) compared against the configured index. jump.Hash returns -1 for a non-positive bucket count, and no index can equal an out-of-range one, so --total-shards=0 (or negative) and --shard >= --total-shards both filter out every object. KSM starts normally, lists and watches everything at full cost, and serves an empty /metrics with HTTP 200 and nothing logged anywhere.

Autosharding is unaffected — it configures shards through ConfigureSharding at runtime rather than through the flags this validates.

Testing

TestValidate covers all three, node and non-node paths, and the valid boundary cases. The new cases fail on main and pass here.

Behaviour change worth noting: a deployment currently passing an out-of-range --auto-gomemlimit-ratio, a negative --object-limit, or an unsatisfiable shard configuration starts today and will now fail fast with the documented error. That is what the existing validation intends, and the defaults are unaffected.

How does this change affect the cardinality of KSM: does not change cardinality

Which issue(s) this PR fixes: N/A

Summary by CodeRabbit

• Bug Fixes

  • Improved validation for memory-limit ratios, object limits, and shard configurations.
  • Rejected invalid values such as NaN, non-positive shard counts, and out-of-range shard indexes.
  • Applied node-specific resource checks only when node settings are configured.

• Tests

  • Added comprehensive validation coverage for valid and invalid configuration scenarios.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mrueg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mrueg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kubernetes-prow]

This issue is currently awaiting triage.

If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 577e337a-ffef-41f9-b5d7-296792f4ab41

📥 Commits

Reviewing files that changed from the base of the PR and between eae7c9c and a62565c.

📒 Files selected for processing (2)

• pkg/options/options.go
• pkg/options/options_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/options/options_test.go

---

📝 Walkthrough

Walkthrough

Options.Validate now applies universal resource checks, scopes node-specific checks to configured nodes, and rejects invalid shard counts or indexes. Table-driven tests cover these validation paths.

Changes

Validation scope and shard constraints

Layer / File(s)	Summary
Update option validation and coverage pkg/options/options.go, pkg/options/options_test.go	Options.Validate rejects NaN and out-of-range memory ratios, applies memory and object limits to all configurations, scopes resource restrictions to configured nodes, and validates shard counts and indexes. Table-driven tests cover valid and invalid configurations.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: catherinef-dev, dgrisonnet, skoef

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: stricter validation for option flags that previously accepted invalid values.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/options/options.go`:
- Around line 209-216: Update Validate for AutoGoMemlimitRatio to reject NaN by
replacing the current comparisons with an inverted range check that treats
values outside the valid bounds, including NaN, as invalid. Add a math.NaN()
table-driven validation case alongside the existing ratio cases.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6f0ea12e-405e-4b1b-854d-c2319fb9b25f

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad03e8 and 69675c7.

📒 Files selected for processing (2)

• pkg/options/options.go
• pkg/options/options_test.go

[coderabbitai]

🧹 Nitpick comments (1)

pkg/options/options_test.go (1)

133-140: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add an explicit non-node resource case.

The table rejects deployments only when Node is set. Add a case with Node unset and an unshardable resource. This directly verifies that the resource restriction remains node-scoped.

Suggested test case

 		{
 			name:    "node scoped run with an unshardable resource",
 			mutate:  func(o *Options) { o.Node = "node-1"; o.Resources = ResourceSet{"deployments": struct{}{}} },
 			wantErr: true,
 		},
+		{
+			name:   "non-node run with an unshardable resource",
+			mutate: func(o *Options) { o.Resources = ResourceSet{"deployments": struct{}{}} },
+		},

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/options/options_test.go` around lines 133 - 140, Add a table entry in the
Options validation tests with Node unset and an unshardable resource such as
deployments, asserting the expected validation result. Keep the existing
node-scoped deployments case and pods case unchanged, so the test explicitly
covers the non-node-scoped behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/options/options_test.go`:
- Around line 133-140: Add a table entry in the Options validation tests with
Node unset and an unshardable resource such as deployments, asserting the
expected validation result. Keep the existing node-scoped deployments case and
pods case unchanged, so the test explicitly covers the non-node-scoped behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e1b19ada-5d18-4bc9-be36-624eba8bea48

📥 Commits

Reviewing files that changed from the base of the PR and between 69675c7 and eae7c9c.

📒 Files selected for processing (2)

• pkg/options/options.go
• pkg/options/options_test.go