common: add WithOptions helper and embeddable mixin

[klaskosk]

Introduce generic WithOptions for functional options on builders, EmbeddableWithOptions mixin, and testhelper coverage with pluggable WithOptionsFunc for builder methods and common.WithOptions.

Made-with: Cursor

Summary by CodeRabbit

• New Features

  • Standardized functional-option support for builders: validates builders early, skips nil options, applies options sequentially, promotes replacement builders, and stops on first option error.
  • Embeddable mixin to let builders expose the standardized options behavior.

• Tests

  • New parallelized test suite and helpers, sentinel error classification, and table-driven tests covering success, nil-skips, replacement promotion, short-circuits, and failure propagation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9c4d4606-a29b-40bd-b380-fe5f7bb69b30

📥 Commits

Reviewing files that changed from the base of the PR and between bd07b90 and 32f48fa.

📒 Files selected for processing (5)

• pkg/internal/common/common.go
• pkg/internal/common/common_test.go
• pkg/internal/common/embeddable_withoptions.go
• pkg/internal/common/testhelper/utils.go
• pkg/internal/common/testhelper/withoptionshelper.go

✅ Files skipped from review due to trivial changes (1)

• pkg/internal/common/common.go

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

• pkg/internal/common/testhelper/utils.go
• pkg/internal/common/embeddable_withoptions.go

---

📝 Walkthrough

Walkthrough

Added a generic functional-options mechanism: new AdditionalOption[SB] constraint and WithOptions function that validates a builder, applies variadic options sequentially (skipping nil), promotes replacement builders, and stores/returns on first option error. Added an embeddable mixin and comprehensive test helpers and tests.

Changes

Cohort / File(s)	Summary
Core Options Framework pkg/internal/common/common.go	Added AdditionalOption[SB] constraint and generic WithOptions[O, B, SO, SB, AO](builder SB, options ...AO) SB which validates the builder, logs target resource, skips nil options, applies options sequentially, promotes non-nil replacement builders, and on first error calls builder.SetError(err) and returns.
Embeddable Mixin pkg/internal/common/embeddable_withoptions.go	Added EmbeddableWithOptions[O, B, SO, SB, AO] with field base SB, SetBase(base SB), and chainable WithOptions(options ...AO) SB delegating to the core WithOptions function.
Tests (unit) pkg/internal/common/common_test.go	Added TestWithOptions and extended test mock with receiver WithOptions(options ...func(*mockClusterScopedBuilder) (*mockClusterScopedBuilder, error)) *mockClusterScopedBuilder delegating to common.WithOptions to exercise the generic helper.
Test helpers - utils pkg/internal/common/testhelper/utils.go	Added sentinel errOptionFailure, helper isOptionFailure(err error) bool, and generic test option constructors: testAnnotationOption[...](), testFailingOption[...](), testNilBuilderOption[...](), and testFailingOptionWithReplacementBuilder[...]() to simulate annotations, failures, nil-promotion, and replacement-builder scenarios.
Test helpers - with-options pkg/internal/common/testhelper/withoptionshelper.go	Added WithOptionsUser[...] interface and WithOptionsTestConfig[...] plus NewWithOptionsTestConfig(...), Name(), and ExecuteTests(t *testing.T) implementing scheme/GVK checks and table-driven subtests covering valid application, invalid-builder short-circuit, nil-option skipping, failure propagation, replacement-builder behavior, and annotation assertions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 64.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'common: add WithOptions helper and embeddable mixin' clearly summarizes the main changes: introducing a generic WithOptions helper function and an EmbeddableWithOptions mixin type, aligning with the primary objectives of the pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/internal/common/common.go`:
- Around line 624-629: The code overwrites builder with the option result before
checking err, which will panic if option returns (nil, err); change the call to
use a temporary (e.g., newBuilder, err := option(builder)), check if err != nil
first, and if so call builder.SetError(err) only after guarding builder != nil
(or handle nil appropriately), then only assign builder = newBuilder when err ==
nil; keep references to the existing variables/methods: builder, option(...),
and builder.SetError.

🪄 Autofix (Beta)

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff2f580e-7dae-4e92-94f2-3622a347398e

📥 Commits

Reviewing files that changed from the base of the PR and between 08ec2cb and 73d934c.

📒 Files selected for processing (5)

• pkg/internal/common/common.go
• pkg/internal/common/common_test.go
• pkg/internal/common/embeddable_withoptions.go
• pkg/internal/common/testhelper/utils.go
• pkg/internal/common/testhelper/withoptionshelper.go

[coderabbitai]

🧹 Nitpick comments (1)

pkg/internal/common/testhelper/withoptionshelper.go (1)

120-129: Optional: remove duplicated annotation retrieval in assertions.

Small readability improvement: fetch annotations once before branching.

♻️ Proposed cleanup

-			if result.GetError() == nil {
-				annotations := result.GetDefinition().GetAnnotations()
+			annotations := result.GetDefinition().GetAnnotations()
+			if result.GetError() == nil {
 				require.NotNil(t, annotations)
 				assert.Equal(t, testAnnotationValue, annotations[testAnnotationKey])
-			} else {
-				annotations := result.GetDefinition().GetAnnotations()
-				if annotations != nil {
-					_, exists := annotations[testAnnotationKey]
-					assert.False(t, exists, "annotation should not be set when option fails")
-				}
+			} else if annotations != nil {
+				_, exists := annotations[testAnnotationKey]
+				assert.False(t, exists, "annotation should not be set when option fails")
 			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/internal/common/testhelper/withoptionshelper.go` around lines 120 - 129,
The code duplicates fetching annotations from
result.GetDefinition().GetAnnotations() in both branches; refactor the test in
withoptionshelper.go to call annotations :=
result.GetDefinition().GetAnnotations() once before the if result.GetError() ==
nil check, then use annotations in the success branch to assert
annotations[testAnnotationKey] equals testAnnotationValue and in the error
branch to check annotations != nil and that testAnnotationKey does not exist;
keep references to result.GetError(), result.GetDefinition().GetAnnotations(),
testAnnotationKey, and testAnnotationValue to locate and update the assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/internal/common/testhelper/withoptionshelper.go`:
- Around line 120-129: The code duplicates fetching annotations from
result.GetDefinition().GetAnnotations() in both branches; refactor the test in
withoptionshelper.go to call annotations :=
result.GetDefinition().GetAnnotations() once before the if result.GetError() ==
nil check, then use annotations in the success branch to assert
annotations[testAnnotationKey] equals testAnnotationValue and in the error
branch to check annotations != nil and that testAnnotationKey does not exist;
keep references to result.GetError(), result.GetDefinition().GetAnnotations(),
testAnnotationKey, and testAnnotationValue to locate and update the assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 62dd6763-b96d-4976-9113-d9bd16edc949

📥 Commits

Reviewing files that changed from the base of the PR and between 73d934c and e1a5718.

📒 Files selected for processing (5)

• pkg/internal/common/common.go
• pkg/internal/common/common_test.go
• pkg/internal/common/embeddable_withoptions.go
• pkg/internal/common/testhelper/utils.go
• pkg/internal/common/testhelper/withoptionshelper.go

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

• pkg/internal/common/common.go
• pkg/internal/common/embeddable_withoptions.go