Skip to content

Commit 49fa596

Browse files
moulalismoulalis
committed
Merge remote-tracking branch 'upstream/rhoai'
2 parents 1abb8b1 + 58feb28 commit 49fa596

9 files changed

Lines changed: 149 additions & 312 deletions

‎.claude/rules/.rules‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../.rules

‎.claude/rules/review-instructions.md‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎.rules/api-types.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
paths: ["api/**/*.go", "docs/COMPONENT_INTEGRATION.md"]
3+
---
4+
5+
# API Type Conventions
6+
7+
User-facing config fields belong in `XxxCommonSpec`, inlined into both `XxxSpec` and `DSCXxx`.
8+
Fields only in `XxxSpec` (not in `XxxCommonSpec`) must be operator-written only (e.g. gateway domain from `GatewayConfig.Status.Domain`).
9+
10+
After modifying types, run: `make generate manifests api-docs`
11+
12+
DSC, DSCI, and component CRs are cluster-scoped singletons. Component CR naming: `default-<component>`.

‎.rules/cloudmanager-controller.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
paths: ["**/cloudmanager/**/*.go"]
3+
---
4+
5+
# Cloud Manager Controller Patterns
6+
7+
Use reconciler builder pattern with `WithDynamicOwnership()`. Each cloud provider has its own controller under `internal/controller/cloudmanager/<provider>/`.
8+
9+
Action execution order matters: sequential, stops on first error. GC action MUST be last.
10+
11+
RBAC: cloudmanager controllers have hand-maintained `kubebuilder_rbac.go` per provider + `common/kubebuilder_rbac.go`. After RBAC changes run `make manifests`.
12+
13+
Key differences from component/service controllers:
14+
- Config passed via `*operatorconfig.CloudManagerConfig`
15+
16+
File locations for provider `<provider>` (azure, coreweave):
17+
- Controller: `internal/controller/cloudmanager/<provider>/*_controller.go`
18+
- Actions: `internal/controller/cloudmanager/<provider>/*_actions.go`
19+
- RBAC: `internal/controller/cloudmanager/<provider>/kubebuilder_rbac.go`
20+
- Shared: `internal/controller/cloudmanager/common/`
21+
22+
Follow patterns in `internal/controller/cloudmanager/azure/azurekubernetesengine_controller.go`.

‎.rules/component-controller.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
paths: ["internal/controller/components/**/*.go"]
3+
---
4+
5+
# Component Controller Patterns
6+
7+
Use reconciler builder pattern:
8+
```go
9+
reconciler.ReconcilerFor(mgr, &componentApi.Xxx{}).
10+
Owns(&corev1.ConfigMap{}).
11+
WithAction(renderAction).
12+
WithAction(deployAction).
13+
WithAction(gcAction). // MUST be last
14+
Build(ctx)
15+
```
16+
17+
Action signature: `func(ctx context.Context, rr *types.ReconciliationRequest) error`
18+
19+
Component handler interface in `internal/controller/components/registry/registry.go`.
20+
21+
RBAC: component controllers use codegen. Do NOT add `kubebuilder_rbac.go` here — only top-level controllers (`dscinitialization`, `datasciencecluster`, `gateway`, `cloudmanager/*`) have hand-maintained RBAC markers.
22+
23+
Action execution order matters: sequential, stops on first error. Place actions deliberately — earlier actions set up state for later ones.
24+
25+
Use `StopError` to halt reconciliation without failure. Propagate errors via `WithError()` to update status conditions.

‎.rules/review-instructions.md‎

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,28 @@
11
# Review Instructions for AI Code Reviewers
22

3-
These instructions are meta-guidance for AI code review tools (CodeRabbit, etc.).
4-
They define review priorities and anti-patterns specific to this repository.
3+
Meta-guidance for AI code review tools (CodeRabbit, etc.). Repo-specific patterns.
54

6-
## Review Priority Order
5+
## Priority Order
76

8-
When multiple findings exist, prioritize in this order:
7+
1. Security vulnerabilities (CWE/CVE, severity, exploit scenario, remediation)
8+
2. RBAC gaps — trace `client.Client` calls to the relevant `kubebuilder_rbac.go`. For component controllers under `internal/controller/components/` (which have no RBAC markers), trace to the top-level controller (`datasciencecluster`, `dscinitialization`, `gateway`, `cloudmanager/*`) whose `kubebuilder_rbac.go` covers those operations
9+
3. Architectural anti-patterns
10+
4. Bug-prone patterns, error handling gaps
11+
5. Performance
912

10-
1. Security vulnerabilities (provide CWE/CVE IDs, severity, exploit scenario, and remediation code)
11-
2. RBAC permission gaps — trace `client.Client` calls to the calling controller's `kubebuilder_rbac.go`
12-
3. Architectural issues and anti-patterns
13-
4. Bug-prone patterns and error handling gaps
14-
5. Performance problems
13+
## Anti-Patterns (DO NOT flag)
1514

16-
## Anti-Patterns to Avoid
15+
- **Tautological test oracles**: E2E tests intentionally use independent oracles. DO NOT suggest mirroring production code path.
16+
- **Removing fallback logic**: Three-way branch (success / IsNotFound|IsNoMatchError / other) for OpenShift resources is intentional. Fallback IS the primary path on non-OpenShift clusters.
17+
- **Missing `kubebuilder_rbac.go` in component controllers**: Components under `internal/controller/components/` use codegen. Only top-level controllers (`dscinitialization`, `datasciencecluster`, `gateway`, `cloudmanager/*`) have hand-maintained RBAC.
18+
- **Suggesting manual OwnerReferences**: Reconciler builder handles these. Only flag for cross-namespace ownership.
19+
- **PR description format**: Template enforces this. Only flag if completely empty.
1720

18-
DO NOT suggest the following — these are common false positives in this codebase:
21+
## Must Always Flag
1922

20-
- **Tautological test oracles**: DO NOT suggest that e2e tests call the same production helper or API resource they are validating. Independent oracles are intentional. See `AGENTS.md` "Test Oracle Independence" section.
21-
- **Removing fallback logic**: DO NOT flag OpenShift-to-vanilla-K8s fallback paths as dead code or suggest simplifying the three-way error branch (`success / IsNotFound|IsNoMatchError / other error`). The fallback is the primary path on non-OpenShift clusters.
22-
- **Missing `kubebuilder_rbac.go` in component controllers**: Component controllers under `internal/controller/components/` use codegen for RBAC. Only top-level controllers (`dscinitialization`, `datasciencecluster`, `gateway`, `cloudmanager/*`) have hand-maintained `kubebuilder_rbac.go` files.
23-
- **Suggesting `OwnerReferences` be set manually**: The reconciler builder pattern handles OwnerReferences. DO NOT suggest adding them in action code unless there is a cross-namespace ownership scenario.
24-
- **PR description format comments**: The PR template already enforces description requirements. DO NOT comment on PR description format unless it is completely empty.
25-
26-
## Patterns That Must Always Be Flagged
27-
28-
Always flag these regardless of context:
29-
30-
- Any `+kubebuilder:rbac` change without a corresponding `make manifests` regeneration
31-
- New `client.Client` operations in `pkg/` without RBAC coverage in all calling controllers
32-
- Status conditions that don't update during deletion or error transitions
23+
- New `client.Client` ops in `pkg/` without RBAC coverage in all calling controllers
24+
- Status conditions not updated during deletion or error transitions
3325
- `InsecureSkipVerify: true` in non-test code
34-
- Wildcard verbs or resources in RBAC rules
35-
- Secrets, tokens, or credentials logged at any verbosity level
36-
- User-facing component configuration added only to an internal component CRD spec (`XxxSpec`)
37-
without a corresponding field in `DSCXxx` (via `XxxCommonSpec`). Any field a user must set to
38-
configure component behaviour belongs in `CommonSpec` so it is reachable through the DSC API.
39-
The only legitimate use of internal-only spec fields (those in `XxxSpec` but NOT in
40-
`XxxCommonSpec`) is for values written exclusively by the operator itself (e.g. the gateway
41-
domain stamped from `GatewayConfig.Status.Domain`). See `docs/COMPONENT_INTEGRATION.md` for
42-
the correct pattern.
26+
- Wildcard verbs/resources in RBAC rules
27+
- Secrets/tokens/credentials logged at any verbosity
28+
- User-facing config in `XxxSpec` only (not in `XxxCommonSpec`/`DSCXxx`). Internal-only spec fields are for operator-written values only. See `docs/COMPONENT_INTEGRATION.md`.

‎.rules/service-controller.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
paths: ["internal/controller/services/**/*.go"]
3+
---
4+
5+
# Service Controller Patterns
6+
7+
Use reconciler builder pattern (same as components). Services implement `ServiceHandler` interface from `internal/controller/services/registry/registry.go`.
8+
9+
Action execution order matters: sequential, stops on first error. GC action MUST be last.
10+
11+
RBAC: service controllers use codegen (no `kubebuilder_rbac.go`) — except `gateway` which has hand-maintained RBAC markers.
12+
13+
File locations for service `<svc>` (auth, monitoring, setup, etc.):
14+
- Handler: `internal/controller/services/<svc>/<svc>.go`
15+
- Controller: `internal/controller/services/<svc>/<svc>_controller.go`
16+
- Actions: `internal/controller/services/<svc>/<svc>_controller_actions.go`
17+
- Tests: `internal/controller/services/<svc>/*_test.go`
18+
19+
Follow patterns in `internal/controller/services/auth/auth_controller.go`.

‎.rules/testing.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
paths: ["**/*_test.go", "tests/**/*.go"]
3+
---
4+
5+
# Testing Patterns
6+
7+
Unit tests: use `fake.NewClientBuilder()` with explicit scheme registration via `pkg/utils/test/scheme`.
8+
E2E tests: use `TestContext` (`tc.Client()`, `tc.Context()`).
9+
10+
E2E test oracles MUST be structurally independent from production code. Never call same production function or read same API resource as code-under-test — derive expectations from independent signals.
11+
12+
Follow patterns in:
13+
- Unit: `internal/controller/components/dashboard/dashboard_controller_actions_test.go`
14+
- E2E: `tests/e2e/`

0 commit comments

Comments
 (0)