Skip to content

Commit eb5ffb1

Browse files
author
David Magton
committed
[cursor-rules] Improve controller reconcile helper documentation
- Unify placeholder type names: rename SKN/SomeKindName/EON to EK/ExampleKind for consistency across all helper rules - Add explicit inline comments to bad examples clarifying why ctx and client.Client are forbidden in apply/construction/is-in-sync helpers - Rewrite ensure helper examples to use proper flow.BeginEnsure() + defer ef.OnEnd(&outcome) pattern - Fix flow.Outcome → flow.EnsureOutcome naming in ensure rules - Align terminology: "desired" → "intended/target" in compute rules - Add implementation example section to patch helper rules showing optimistic lock handling - Fix confusing step function example in reconciliation-flow rules - Remove duplicate line in controller-reconcile-helper.mdc Signed-off-by: David Magton <david.magton@flant.com>
1 parent 1495697 commit eb5ffb1

11 files changed

+183
-116
lines changed

‎.cursor/rules/controller-reconcile-helper-apply.mdc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ func applyFoo(
190190

191191
❌ Doing any Kubernetes API I/O (client usage / API calls in apply):
192192
```go
193+
// forbidden: apply helpers MUST NOT accept ctx (they are non-I/O)
194+
// forbidden: apply helpers MUST NOT accept client.Client
193195
func applyFoo(ctx context.Context, c client.Client, obj *v1alpha1.Foo, target TargetFoo) error {
194196
// forbidden: apply helpers are non-I/O
195197
return c.Update(ctx, obj)
@@ -198,6 +200,7 @@ func applyFoo(ctx context.Context, c client.Client, obj *v1alpha1.Foo, target Ta
198200

199201
❌ Executing patches or making patch decisions inside apply:
200202
```go
203+
// forbidden: apply helpers MUST NOT accept ctx or client.Client
201204
func applyFoo(ctx context.Context, c client.Client, obj, base *v1alpha1.Foo, target TargetFoo) error {
202205
// forbidden: patch execution belongs to Reconcile methods / PatchReconcileHelpers
203206
obj.Spec = target.Spec
@@ -224,9 +227,10 @@ func applyFoo(obj *v1alpha1.Foo, target TargetFoo) flow.ReconcileOutcome {
224227

225228
❌ Adding logging/phases to apply helpers (they must stay tiny and have no `ctx`):
226229
```go
230+
// forbidden: apply helpers MUST NOT accept ctx
227231
func applyFoo(ctx context.Context, obj *v1alpha1.Foo, target TargetFoo) error {
228232
l := log.FromContext(ctx)
229-
l.Info("applying target foo") // forbidden: apply helpers do not log / do not accept ctx
233+
l.Info("applying target foo") // forbidden: apply helpers do not log
230234
obj.Spec = target.Spec
231235
return nil
232236
}

‎.cursor/rules/controller-reconcile-helper-compute.mdc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func computeIntendedFoo(ctx context.Context, obj *v1alpha1.Foo, out *IntendedFoo
325325

326326
✅ Separate **target** values (GOOD)
327327
```go
328-
func (r *Reconciler) computeTargetX(obj *v1alpha1.X, intended IntendedX, actual ActualX) (targetMain TargetLabels, targetStatus TargetXStatus, err error)
328+
func (r *Reconciler) computeTargetX(obj *v1alpha1.X, intended IntendedX, actual ActualX) (targetMain TargetLabels, targetStatus TargetEKStatus, err error)
329329
```
330330

331331
❌ Mixed **target** main+status (BAD)
@@ -334,14 +334,14 @@ func (r *Reconciler) computeTargetX(obj *v1alpha1.X, intended IntendedX, actual
334334
```
335335

336336
Notes (SHOULD):
337-
- “Main” typically includes metadata/spec of the root object and/or child objects (desired or actual, depending on the helper).
338-
- “Status” typically includes conditions, observed generation, and other status-only values (desired or actual, depending on the helper).
337+
- “Main” typically includes metadata/spec of the root object and/or child objects (intended/target or actual, depending on the helper).
338+
- “Status” typically includes conditions, observed generation, and other status-only values (intended/target or actual, depending on the helper).
339339

340340
---
341341

342342
## Composition
343343

344-
- A **ComputeReconcileHelper** MAY compute multiple related outputs (desired and/or actual) in one pass.
344+
- A **ComputeReconcileHelper** MAY compute multiple related outputs (intended/target and/or actual) in one pass.
345345
- If these outputs are **not distinguishable for external code** (they represent one conceptual “state”), it SHOULD return them as **one object** (small struct, anonymous struct, slice/map).
346346
- If these outputs **are distinguishable for external code** (they are meaningfully different and will be used independently), it SHOULD return them as **separate objects**.
347347
- A `computeIntended*` / `ComputeIntended*` helper MAY call other `computeIntended*` helpers (pure composition).
@@ -375,7 +375,8 @@ Notes (SHOULD):
375375
```go
376376
func (r *Reconciler) computeActualFoo(ctx context.Context, obj *v1alpha1.Foo) (ActualFoo, error) {
377377
var cm corev1.ConfigMap
378-
if err := r.client.Get(ctx, nn, &cm); err != nil { // forbidden: I/O in compute
378+
key := client.ObjectKey{Namespace: obj.Namespace, Name: "some-cm"}
379+
if err := r.client.Get(ctx, key, &cm); err != nil { // forbidden: I/O in compute
379380
return ActualFoo{}, err
380381
}
381382
return ActualFoo{}, nil

‎.cursor/rules/controller-reconcile-helper-construction.mdc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,8 @@ Important distinctions:
271271
❌ Doing any Kubernetes API I/O:
272272

273273
```go
274+
// forbidden: construction helpers MUST NOT accept ctx
275+
// forbidden: construction helpers MUST NOT accept client.Client
274276
func newFoo(ctx context.Context, c client.Client, obj *v1alpha1.Foo) (FooOut, error) {
275277
// forbidden: I/O in ConstructionReconcileHelper
276278
_ = c.Get(ctx, client.ObjectKeyFromObject(obj), &corev1.ConfigMap{})

‎.cursor/rules/controller-reconcile-helper-create.mdc‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ Typical create helpers are used for child resources to encapsulate the mechanica
4646
Examples:
4747
- `createCM(...)` (or `createConfigMap(...)`)
4848
- `createSVC(...)` (or `createService(...)`)
49-
- `createSKN(...)` (or `createSomeKindName(...)`)
49+
- `createEK(...)` (or `createExampleKind(...)`)
5050
- **CreateReconcileHelpers** names MUST NOT imply orchestration or existence checks (`ensureCreated`, `reconcileCreate`, `createIfNeeded`) — branching and policy belong to **Reconcile methods**.
5151

5252
---
@@ -58,9 +58,9 @@ Typical create helpers are used for child resources to encapsulate the mechanica
5858

5959
### Simple create
6060
```go
61-
func (r *Reconciler) createSKN(
61+
func (r *Reconciler) createEK(
6262
ctx context.Context,
63-
obj *v1alpha1.SomeKindName,
63+
obj *v1alpha1.ExampleKind,
6464
) error
6565
```
6666

@@ -161,9 +161,9 @@ See the common read-only contract in `controller-reconcile-helper.mdc` (especial
161161

162162
❌ Doing existence checks (`Get/List`) or any extra Kubernetes API calls:
163163
```go
164-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
164+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
165165
// forbidden: extra API call
166-
var existing v1alpha1.EON
166+
var existing v1alpha1.EK
167167
if err := r.client.Get(ctx, client.ObjectKeyFromObject(obj), &existing); err == nil {
168168
return nil // "already exists" decision belongs to Reconcile methods
169169
}
@@ -178,7 +178,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
178178

179179
❌ Performing more than one write (`Create` + `Update/Patch/Delete`, retries-as-extra-calls, fallback logic):
180180
```go
181-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
181+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
182182
if err := r.client.Create(ctx, obj); err != nil {
183183
// forbidden: "fallback" write makes it >1 API call
184184
return r.client.Update(ctx, obj)
@@ -189,8 +189,8 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
189189

190190
❌ Creating on a temporary object and dropping it (caller-owned `obj` is not updated with UID/RV/defaults):
191191
```go
192-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
193-
tmp := &v1alpha1.EON{
192+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
193+
tmp := &v1alpha1.EK{
194194
ObjectMeta: metav1.ObjectMeta{
195195
Namespace: obj.Namespace,
196196
Name: obj.Name,
@@ -208,7 +208,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
208208

209209
❌ Using `DeepCopy` in create helpers:
210210
```go
211-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
211+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
212212
base := obj.DeepCopy() // forbidden: DeepCopy belongs to Reconcile methods, not create helpers
213213
_ = base
214214
return r.client.Create(ctx, obj)
@@ -217,7 +217,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
217217

218218
❌ Writing status as part of create (or “relying on status in the create request”):
219219
```go
220-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
220+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
221221
obj.Status.Phase = "Ready" // forbidden: status writes (report/controller-owned state) are a separate request
222222
if err := r.client.Create(ctx, obj); err != nil {
223223
return err
@@ -229,7 +229,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
229229

230230
❌ Executing patches inside create helpers:
231231
```go
232-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON, base *v1alpha1.EON) error {
232+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK, base *v1alpha1.EK) error {
233233
// forbidden: patch execution belongs to PatchReconcileHelpers / Reconcile methods
234234
if err := r.client.Create(ctx, obj); err != nil {
235235
return err
@@ -240,7 +240,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON, base *v1a
240240

241241
❌ Creating multiple objects in a single create helper:
242242
```go
243-
func (r *Reconciler) createEONs(ctx context.Context, objs []*v1alpha1.EON) error {
243+
func (r *Reconciler) createEKs(ctx context.Context, objs []*v1alpha1.EK) error {
244244
for _, obj := range objs {
245245
if err := r.client.Create(ctx, obj); err != nil { // forbidden: multiple API calls
246246
return err
@@ -252,7 +252,7 @@ func (r *Reconciler) createEONs(ctx context.Context, objs []*v1alpha1.EON) error
252252

253253
❌ Hidden I/O / nondeterministic request payload (time/random/env, nondeterministic ordering):
254254
```go
255-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
255+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
256256
obj.Annotations["createdAt"] = time.Now().Format(time.RFC3339) // forbidden
257257
obj.Labels["nonce"] = uuid.NewString() // forbidden
258258
obj.Spec.Seed = rand.Int() // forbidden
@@ -262,7 +262,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
262262

263263
❌ Using `GenerateName` / random naming for resources that must be stable in reconciliation:
264264
```go
265-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
265+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK) error {
266266
obj.Name = ""
267267
obj.GenerateName = "eon-" // anti-pattern: server adds a random suffix => nondeterministic identity
268268
return r.client.Create(ctx, obj)
@@ -271,7 +271,7 @@ func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON) error {
271271

272272
❌ Mutating shared templates/defaults through aliasing while preparing `obj`:
273273
```go
274-
func (r *Reconciler) createEON(ctx context.Context, obj *v1alpha1.EON, template *v1alpha1.EON) error {
274+
func (r *Reconciler) createEK(ctx context.Context, obj *v1alpha1.EK, template *v1alpha1.EK) error {
275275
// forbidden: template labels map is shared; mutating it mutates the template
276276
labels := template.GetLabels()
277277
labels["app"] = "eon"

‎.cursor/rules/controller-reconcile-helper-delete.mdc‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Typical delete helpers encapsulate the mechanical delete call (including “alre
4545
Examples:
4646
- `deleteCM(...)` (or `deleteConfigMap(...)`)
4747
- `deleteSVC(...)` (or `deleteService(...)`)
48-
- `deleteSKN(...)` (or `deleteSomeKindName(...)`)
48+
- `deleteEK(...)` (or `deleteExampleKind(...)`)
4949
- **DeleteReconcileHelpers** names MUST NOT imply orchestration or multi-step cleanup (`reconcileDelete`, `deleteAll`, `deleteAndWait`) — ordering and lifecycle policy belong to **Reconcile methods**.
5050

5151
---
@@ -57,9 +57,9 @@ Typical delete helpers encapsulate the mechanical delete call (including “alre
5757

5858
### Simple delete
5959
```go
60-
func (r *Reconciler) deleteSKN(
60+
func (r *Reconciler) deleteEK(
6161
ctx context.Context,
62-
obj *v1alpha1.SomeKindName,
62+
obj *v1alpha1.ExampleKind,
6363
) error
6464
```
6565

@@ -160,9 +160,9 @@ See the common read-only contract in `controller-reconcile-helper.mdc` (especial
160160

161161
❌ Doing existence checks (`Get/List`) or any extra Kubernetes API calls:
162162
```go
163-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
163+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
164164
// forbidden: extra API call
165-
var existing v1alpha1.EON
165+
var existing v1alpha1.EK
166166
if err := r.client.Get(ctx, client.ObjectKeyFromObject(obj), &existing); err != nil {
167167
return err
168168
}
@@ -177,9 +177,10 @@ func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
177177

178178
❌ Performing more than one write (`Delete` + `Patch/Update/Create`, retries-as-extra-calls, fallback logic):
179179
```go
180-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
180+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
181181
if err := r.client.Delete(ctx, obj); err != nil {
182182
// forbidden: "fallback" write makes it >1 API call
183+
// also forbidden: DeepCopy in delete helper (see I/O boundaries)
183184
return r.client.Patch(ctx, obj, client.MergeFrom(obj.DeepCopy()))
184185
}
185186
return nil
@@ -188,7 +189,7 @@ func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
188189

189190
❌ Mutating the object as part of deletion (“marking”, finalizer edits, status writes):
190191
```go
191-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
192+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
192193
obj.Finalizers = nil // forbidden: mutation belongs to ensure/apply + patch
193194
obj.Status.Phase = "Deleting" // forbidden: status writes (report/controller-owned state) belong elsewhere
194195
return r.client.Delete(ctx, obj)
@@ -197,7 +198,7 @@ func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
197198

198199
❌ Trying to “prepare for delete” inside the delete helper (remove finalizer + delete):
199200
```go
200-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
201+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
201202
// forbidden: any patch/update belongs to Reconcile methods and is a separate patch domain write
202203
base := obj.DeepCopy() // also forbidden: DeepCopy in delete helper
203204
obj.Finalizers = []string{} // forbidden: mutation
@@ -210,15 +211,15 @@ func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
210211

211212
❌ Calling `DeepCopy` inside delete helpers:
212213
```go
213-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
214+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
214215
_ = obj.DeepCopy() // forbidden: DeepCopy belongs to Reconcile methods
215216
return r.client.Delete(ctx, obj)
216217
}
217218
```
218219

219220
❌ Deleting multiple objects in a single delete helper:
220221
```go
221-
func (r *Reconciler) deleteEONs(ctx context.Context, objs []*v1alpha1.EON) error {
222+
func (r *Reconciler) deleteEKs(ctx context.Context, objs []*v1alpha1.EK) error {
222223
for _, obj := range objs {
223224
if err := r.client.Delete(ctx, obj); err != nil { // forbidden: multiple API calls
224225
return err
@@ -230,7 +231,7 @@ func (r *Reconciler) deleteEONs(ctx context.Context, objs []*v1alpha1.EON) error
230231

231232
❌ Hidden I/O / nondeterminism (time/random/env/extra network calls):
232233
```go
233-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
234+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
234235
if os.Getenv("DELETE_FAST") == "1" { // forbidden: env read in helper
235236
// ...
236237
}
@@ -241,22 +242,22 @@ func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
241242

242243
❌ Using `DeleteAllOf` or broad deletes from a delete helper:
243244
```go
244-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
245+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
245246
// forbidden: not “exactly one object delete”
246-
return r.client.DeleteAllOf(ctx, &v1alpha1.EON{}, client.InNamespace(obj.Namespace))
247+
return r.client.DeleteAllOf(ctx, &v1alpha1.EK{}, client.InNamespace(obj.Namespace))
247248
}
248249
```
249250

250251
❌ Doing “wait until gone” polling inside the delete helper:
251252
```go
252-
func (r *Reconciler) deleteEON(ctx context.Context, obj *v1alpha1.EON) error {
253+
func (r *Reconciler) deleteEK(ctx context.Context, obj *v1alpha1.EK) error {
253254
if err := r.client.Delete(ctx, obj); err != nil {
254255
return err
255256
}
256257

257258
// forbidden: extra API calls / orchestration belongs to Reconcile methods
258259
for {
259-
var cur v1alpha1.EON
260+
var cur v1alpha1.EK
260261
err := r.client.Get(ctx, client.ObjectKeyFromObject(obj), &cur)
261262
if apierrors.IsNotFound(err) {
262263
return nil

0 commit comments

Comments
 (0)