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

apiserver/rest/interface.go

@@ -70,6 +70,36 @@ type PrepareForUpdater interface {
 	PrepareForUpdate(ctx context.Context, old runtime.Object)
 }
 
+// GenerationTracker opts a resource into Kubernetes generation semantics: DefaultStrategy sets
+// metadata.generation to 1 on create and increments it on every update that changes the spec.
+//
+// This is opt-in rather than automatic because a generic strategy cannot know which part of an
+// arbitrary runtime.Object is its "spec", and because turning it on changes what every controller
+// watching the resource observes. Upstream Kubernetes does the same work per resource: BeforeCreate
+// sets no generation at all, and BeforeUpdate only copies the stored value forward so clients
+// cannot forge it, leaving the increment to each resource's own strategy. A resource that does not
+// implement this interface therefore stays at generation 0 for its whole life — which silently
+// makes every `status.observedGeneration == metadata.generation` comparison a controller performs
+// trivially true, so idempotence short-circuits latch and spec edits are never acted on.
+//
+// Implementations compare their own typed spec, which is exact and needs no reflection:
+//
+//	func (o *Widget) SpecChanged(old runtime.Object) bool {
+//		p, ok := old.(*Widget)
+//
+//		return !ok || !apiequality.Semantic.DeepEqual(o.Spec, p.Spec)
+//	}
+//
+// Only the spec counts. Metadata-only edits (labels, annotations) must not make controllers
+// re-reconcile, and status updates never reach this path: the status subresource installs a
+// strategy that calls only its own override.
+type GenerationTracker interface {
+	// SpecChanged reports whether this object's spec differs from old's. It must report true
+	// when old is not of the same type, so an unexpected pairing is treated as a change rather
+	// than silently skipping the increment.
+	SpecChanged(old runtime.Object) bool
+}
+
 // TableConverter implements an adapted version of rest.TableConverter
 // it can be used by objects to override DefaultStrategy behaviour.
 type TableConverter interface {

apiserver/rest/strategy.go

@@ -82,15 +82,24 @@ func (d DefaultStrategy) NamespaceScoped() bool {
 }
 
 // PrepareForCreate normalizes the object before creation, delegating to PrepareForCreater if implemented.
+// A resource that implements GenerationTracker also starts at metadata.generation 1.
 func (DefaultStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	if v, ok := obj.(PrepareForCreater); ok {
 		v.PrepareForCreate(ctx)
 	}
+	// BeforeCreate sets no generation, so a resource that tracks it starts here. See
+	// GenerationTracker for why this is opt-in.
+	if _, ok := obj.(GenerationTracker); ok {
+		if m, err := meta.Accessor(obj); err == nil {
+			m.SetGeneration(1)
+		}
+	}
 }
 
 // PrepareForUpdate normalizes the object before update.
 // If the object has a status subresource, status is copied from old to new.
 // If PrepareForUpdater is implemented, it is called to further normalize.
+// If GenerationTracker is implemented, metadata.generation is incremented when the spec changed.
 func (DefaultStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
 	if v, ok := obj.(resource.ObjectWithStatusSubResource); ok {
 		// Copy status from old to new to avoid spec-only updates modifying status.
@@ -99,6 +108,17 @@ func (DefaultStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
 	if v, ok := obj.(PrepareForUpdater); ok {
 		v.PrepareForUpdate(ctx, old)
 	}
+	// BeforeUpdate has already copied the stored generation onto the incoming object, and it
+	// calls this before persisting, so an increment here is what lands. Only a spec change
+	// counts; status updates never reach this method, because the status subresource installs a
+	// strategy that calls only its own override.
+	if v, ok := obj.(GenerationTracker); ok && v.SpecChanged(old) {
+		newMeta, newErr := meta.Accessor(obj)
+		oldMeta, oldErr := meta.Accessor(old)
+		if newErr == nil && oldErr == nil {
+			newMeta.SetGeneration(oldMeta.GetGeneration() + 1)
+		}
+	}
 }
 
 // Validate delegates to the object's Validater interface if present, otherwise returns no errors.

apiserver/rest/strategy_test.go

@@ -295,3 +295,77 @@ var _ = Describe("PrepareForUpdaterStrategy", func() {
 		Expect(func() { s.PrepareForUpdate(context.Background(), obj, old) }).ToNot(Panic())
 	})
 })
+
+// genObj opts into generation semantics. It deliberately does NOT implement
+// resource.ObjectWithStatusSubResource, so these specs exercise the generation path alone.
+type genObj struct {
+	metav1.TypeMeta
+	metav1.ObjectMeta
+	Spec string
+}
+
+func (g *genObj) DeepCopyObject() runtime.Object {
+	if g == nil {
+		return nil
+	}
+	clone := *g
+
+	return &clone
+}
+
+func (g *genObj) GetObjectMeta() *metav1.ObjectMeta { return &g.ObjectMeta }
+
+// SpecChanged implements GenerationTracker.
+func (g *genObj) SpecChanged(old runtime.Object) bool {
+	o, ok := old.(*genObj)
+
+	return !ok || g.Spec != o.Spec
+}
+
+var _ = Describe("DefaultStrategy generation semantics", func() {
+	// Upstream BeforeCreate never sets a generation and BeforeUpdate only copies the stored one
+	// forward, so incrementing it is each strategy's job. Without this a resource sits at
+	// generation 0 forever and every status.observedGeneration == metadata.generation test a
+	// controller makes is trivially true.
+	It("starts an opted-in resource at generation 1 on create", func() {
+		obj := &genObj{Spec: "a"}
+		DefaultStrategy{}.PrepareForCreate(context.Background(), obj)
+		Expect(obj.GetGeneration()).To(Equal(int64(1)))
+	})
+
+	It("leaves a resource that has not opted in at generation 0", func() {
+		obj := &testObj{}
+		DefaultStrategy{}.PrepareForCreate(context.Background(), obj)
+		Expect(obj.GetGeneration()).To(Equal(int64(0)))
+	})
+
+	It("increments the generation when the spec changes", func() {
+		old := &genObj{Spec: "a"}
+		old.SetGeneration(7)
+		// BeforeUpdate has already copied the stored generation onto the incoming object.
+		obj := &genObj{Spec: "b"}
+		obj.SetGeneration(7)
+		DefaultStrategy{}.PrepareForUpdate(context.Background(), obj, old)
+		Expect(obj.GetGeneration()).To(Equal(int64(8)))
+	})
+
+	// A label or annotation edit must not make every controller re-reconcile.
+	It("leaves the generation alone when the spec is unchanged", func() {
+		old := &genObj{Spec: "a"}
+		old.SetGeneration(7)
+		obj := &genObj{Spec: "a"}
+		obj.SetGeneration(7)
+		obj.SetLabels(map[string]string{"k": "v"})
+		DefaultStrategy{}.PrepareForUpdate(context.Background(), obj, old)
+		Expect(obj.GetGeneration()).To(Equal(int64(7)))
+	})
+
+	It("leaves a resource that has not opted in alone on update", func() {
+		obj := &testObj{Status: "new"}
+		old := &testObj{Status: "old"}
+		old.SetGeneration(3)
+		obj.SetGeneration(3)
+		DefaultStrategy{}.PrepareForUpdate(context.Background(), obj, old)
+		Expect(obj.GetGeneration()).To(Equal(int64(3)))
+	})
+})