feat(apiserver): opt-in metadata.generation semantics in DefaultStrategy by trevex · Pull Request #129 · opendefensecloud/apiserver-kit

trevex · 2026-09-21T10:47:09Z

What

DefaultStrategy never touched metadata.generation, and neither does the upstream machinery around it: BeforeCreate sets no generation at all, and BeforeUpdate only copies the stored value forward so clients cannot forge it. Upstream leaves the increment to each resource's own strategy, so a resource served through this kit stayed at generation 0 for its whole life.

That is quietly destructive for controllers, because it makes every status.observedGeneration == metadata.generation comparison trivially true: an idempotence short-circuit written against it latches after the first reconcile and the resource is never reconciled for a spec edit again. It is also invisible to envtest, which serves real CRDs through the real apiserver and does increment generation, so the behaviour only diverges once a resource is served from an aggregated server built with this kit.

A resource opts in by implementing GenerationTracker, which answers the one question a generic strategy cannot: whether its spec changed. Implementations compare their own typed spec, so there is no reflection and no guessing at which field is the "spec". Opt-in rather than automatic because turning it on changes what every controller watching a resource observes.

Only a spec change increments: metadata-only edits must not make controllers re-reconcile. Status updates never reach this path, because the status subresource installs a strategy that calls only its own override.

Why

Handling generations is a hassle, this PR eases this somewhat. :)

Testing

Unit tests

Notes for reviewers

No breaking changes, purely opt-in!

Checklist

Tests added/updated
No breaking changes (or upgrade path documented above)
Readable commit history (squashed and cleaned up as desired)
AI code review considered and comments resolved

Summary by CodeRabbit

New Features
- Added opt-in generation tracking for supported API resources.
- Resource generation is initialized to 1 when created.
- Generation increments when the resource specification changes.
- Metadata-only edits and status updates do not change generation values.
- Resources that do not opt in retain their existing generation behavior.
- Updates involving mismatched resource types are treated as specification changes.

DefaultStrategy never touched metadata.generation, and neither does the upstream machinery around it: BeforeCreate sets no generation at all, and BeforeUpdate only copies the stored value forward so clients cannot forge it. Upstream leaves the increment to each resource's own strategy, so a resource served through this kit stayed at generation 0 for its whole life. That is quietly destructive for controllers, because it makes every `status.observedGeneration == metadata.generation` comparison trivially true: an idempotence short-circuit written against it latches after the first reconcile and the resource is never reconciled for a spec edit again. It is also invisible to envtest, which serves real CRDs through the real apiserver and does increment generation, so the behaviour only diverges once a resource is served from an aggregated server built with this kit. A resource opts in by implementing GenerationTracker, which answers the one question a generic strategy cannot: whether its spec changed. Implementations compare their own typed spec, so there is no reflection and no guessing at which field is the "spec". Opt-in rather than automatic because turning it on changes what every controller watching a resource observes. Only a spec change increments: metadata-only edits must not make controllers re-reconcile. Status updates never reach this path, because the status subresource installs a strategy that calls only its own override.

coderabbitai · 2026-09-21T10:47:15Z

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Important

🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 842f3483-ea8b-4353-ae6e-730d52047ece

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 8e9256e2-a3e0-400c-af9b-e09d74eb0915

📥 Commits

Reviewing files that changed from the base of the PR and between 57769af and 1481f7e.

📒 Files selected for processing (3)

apiserver/rest/interface.go
apiserver/rest/strategy.go
apiserver/rest/strategy_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The REST package adds an opt-in GenerationTracker contract. DefaultStrategy initializes generation on create and increments it when SpecChanged reports a change. Tests cover opted-in and non-opted-in resources.

Changes: Generation tracking

Layer / File(s)	Summary
Generation tracking contract and strategy `apiserver/rest/interface.go`, `apiserver/rest/strategy.go`	Adds `GenerationTracker`. `DefaultStrategy` sets generation to 1 on create and increments it on specification changes during update.
Generation behavior tests `apiserver/rest/strategy_test.go`	Adds a test resource and verifies create, specification-change, metadata-only, and non-opted-in generation behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant DefaultStrategy
  participant Resource
  participant ObjectMeta
  DefaultStrategy->>Resource: Check GenerationTracker
  DefaultStrategy->>Resource: Call SpecChanged(old)
  Resource-->>DefaultStrategy: Return specification change result
  DefaultStrategy->>ObjectMeta: Set generation to 1 or old generation + 1

Suggested reviewers: cbrgm

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
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.
Title check	✅ Passed	The title clearly and concisely describes the main change: opt-in metadata.generation semantics in DefaultStrategy.
Description check	✅ Passed	The description explains the change, motivation, testing, reviewer notes, and checklist status. The issue reference is not completed, and the Why section is brief, but the description is otherwise com…

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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

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

coveralls · 2026-09-21T10:53:36Z

Coverage Report for CI Build 38731468667

Coverage increased (+1.3%) to 39.419%

Details

Coverage increased (+1.3%) from the base build.
Patch coverage: 10 of 10 lines across 1 file are fully covered (100%).
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	482
Covered Lines:	190
Line Coverage:	39.42%
Coverage Strength:	0.85 hits per line

💛 - Coveralls

trevex marked this pull request as ready for review September 21, 2026 10:49

rebEllieous approved these changes Sep 22, 2026

View reviewed changes

yocaba added 2 commits October 15, 2026 11:45

Merge branch 'main' into feat/generation-semantics

e785d5d

Merge branch 'main' into feat/generation-semantics

1f8adca

yocaba enabled auto-merge October 15, 2026 10:03

Merge branch 'main' into feat/generation-semantics

acf3f83

Navigation Menu

Repository navigation

feat(apiserver): opt-in metadata.generation semantics in DefaultStrategy - #129

feat(apiserver): opt-in metadata.generation semantics in DefaultStrategy#129
trevex wants to merge 4 commits into
mainfrom
feat/generation-semantics

trevex commented Sep 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 21, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

coveralls commented Sep 21, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

trevex commented Sep 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Testing

Notes for reviewers

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

coveralls commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 38731468667

Coverage increased (+1.3%) to 39.419%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

trevex commented Sep 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 21, 2026 •

edited

Loading

coveralls commented Sep 21, 2026 •

edited

Loading