Skip to content
Closed
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
94 changes: 94 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,100 @@ reviews:

# Test fixtures and data
- "!**/testdata/**"
path_instructions:
# --- Rules derived from PR review patterns ---
# Generated by teams:coderabbit-rules-from-pr-reviews (2026-03-27)
# Analyzed 100 recent merged PRs, 480 human review comments from 24 reviewers

- path: "api/**/*_types.go"
instructions: |
Do not use +kubebuilder:validation:Pattern for string validation. Use CEL
XValidation rules instead with a human-readable message explaining the
expected format. For example, instead of:
+kubebuilder:validation:Pattern=`^arn:`
Use:
+kubebuilder:validation:XValidation:rule="self.matches('^arn:...')",message="must be a valid ARN in the format arn:<partition>:..."

Enum values in kubebuilder:validation:Enum markers must use PascalCase
(e.g., Standard, IO1, GP2) following Kubernetes API conventions. Avoid
lowercase (e.g., "enabled"/"disabled") or camelCase enum values.
Also prefer specific descriptive terms over generic Enable/Disable
(e.g., HTTPEndpoint/None instead of Enabled/Disabled).

Every API field with constrained values (enums, formatted strings, ranges)
must document in the godoc comment:
1. The list of valid values and what each means to an end user
2. The expected format for string fields (e.g., "must be a valid IPv4 or IPv6 address")
3. The behavior when the field is omitted (e.g., "When omitted, the platform
is left to choose a reasonable default, which is subject to change over time.
The current default is ...")
Do not assume users will read kubebuilder markers — the godoc should be
self-contained.

Follow OpenShift API conventions for pointer usage:
- Do NOT use pointers for struct fields when the zero value is not a valid
user choice. Use value types with omitzero instead.
- Do NOT use pointers for enum fields unless empty string "" is a valid enum
value.
- Do NOT use pointers for numeric fields when the minimum value is > 0.
- Use omitzero (not omitempty) for struct value types — omitempty does
nothing for non-pointer structs.
- Only use pointers when you need to distinguish between "not set" and
"explicitly set to zero value" and both are valid user choices.
See: https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md

All string fields in API types must have both MinLength and MaxLength
kubebuilder validation markers unless there is a documented reason not to.
Common values:
- IPv4/IPv6 addresses: MinLength=3, MaxLength=39
- DNS names: MinLength=1, MaxLength=253
- Kubernetes resource names: MinLength=1, MaxLength=253
Empty string is rarely a valid value — if not, set MinLength=1 at minimum.

When a struct has mutually exclusive sub-structs (e.g., platform-specific
config), use a properly marked discriminated union:
1. Add +union marker to the parent struct
2. Add +unionDiscriminator to the discriminator field
3. Add +unionMember to each member field
4. Add bidirectional CEL validation that enforces "required when discriminator
matches AND forbidden otherwise":
rule="self.type == 'Foo' ? has(self.foo) : !has(self.foo)"
5. Consider placing the union in a child struct to keep the parent clean for
future non-union fields.
Do not use separate has() rules without the forbidden-otherwise check.

Do not import and embed upstream Kubernetes types (e.g.,
corev1.LocalObjectReference, corev1.SecretReference) in CRD API types.
Create your own minimal reference types instead. Upstream types can change
unexpectedly or add fields you don't want to support, breaking your API
contract. Only import upstream types when passing them through verbatim to
another Kubernetes API.

Do not use +patchStrategy or +patchMergeKey markers on CRD types — they
have no effect. CRDs use Server-Side Apply (SSA) with +listType=map and
+listMapKey markers instead for merge semantics on list fields.

- path: "**/*_test.go"
instructions: |
Test case names should follow the Gherkin convention pattern:
"When <condition>, it should <expected behavior>".
For example: "When cluster has no nodes, it should clear NodesInfo status"
This makes test output self-documenting and easier to understand in CI logs.

- path: "**/*.go"
instructions: |
When writing data to files or performing operations where order matters,
prefer slices of structs over maps to ensure deterministic iteration order.
Go map iteration is randomized and can cause flaky tests or inconsistent
output.

New exported functions and non-trivial logic changes should include unit
tests. Tests should cover:
1. Happy path
2. Error/edge cases (malformed input, empty values, nil)
3. Platform-specific behavior for all relevant platforms (e.g., if adding
AWS support, consider whether GCP/Azure also need test coverage)
Check that corresponding _test.go files are included in the PR.
tools:
golangci-lint:
enabled: true
Expand Down
Loading