From 04fbe14929e287fd4ce912bd51a73156daf977ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:39:43 +0000 Subject: [PATCH 01/20] Add PodDisruptionBudget primitive and PDB spec editor Implement the PDB primitive as a Static resource following the ConfigMap pattern. Includes builder, resource, mutator (plan-and-apply with metadata and spec edit categories), field application flavors, and the typed PodDisruptionBudgetSpecEditor for mutation surfaces. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mutation/editors/pdbspec.go | 68 ++++++++++ pkg/mutation/editors/pdbspec_test.go | 121 ++++++++++++++++++ pkg/primitives/pdb/builder.go | 110 ++++++++++++++++ pkg/primitives/pdb/builder_test.go | 154 +++++++++++++++++++++++ pkg/primitives/pdb/flavors.go | 25 ++++ pkg/primitives/pdb/flavors_test.go | 124 ++++++++++++++++++ pkg/primitives/pdb/mutator.go | 107 ++++++++++++++++ pkg/primitives/pdb/mutator_test.go | 182 +++++++++++++++++++++++++++ pkg/primitives/pdb/resource.go | 68 ++++++++++ 9 files changed, 959 insertions(+) create mode 100644 pkg/mutation/editors/pdbspec.go create mode 100644 pkg/mutation/editors/pdbspec_test.go create mode 100644 pkg/primitives/pdb/builder.go create mode 100644 pkg/primitives/pdb/builder_test.go create mode 100644 pkg/primitives/pdb/flavors.go create mode 100644 pkg/primitives/pdb/flavors_test.go create mode 100644 pkg/primitives/pdb/mutator.go create mode 100644 pkg/primitives/pdb/mutator_test.go create mode 100644 pkg/primitives/pdb/resource.go diff --git a/pkg/mutation/editors/pdbspec.go b/pkg/mutation/editors/pdbspec.go new file mode 100644 index 00000000..3e982647 --- /dev/null +++ b/pkg/mutation/editors/pdbspec.go @@ -0,0 +1,68 @@ +package editors + +import ( + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// PodDisruptionBudgetSpecEditor provides a typed API for mutating a Kubernetes +// PodDisruptionBudgetSpec. +type PodDisruptionBudgetSpecEditor struct { + spec *policyv1.PodDisruptionBudgetSpec +} + +// NewPodDisruptionBudgetSpecEditor creates a new PodDisruptionBudgetSpecEditor for +// the given PodDisruptionBudgetSpec. +func NewPodDisruptionBudgetSpecEditor(spec *policyv1.PodDisruptionBudgetSpec) *PodDisruptionBudgetSpecEditor { + return &PodDisruptionBudgetSpecEditor{spec: spec} +} + +// Raw returns the underlying *policyv1.PodDisruptionBudgetSpec. +// +// This is an escape hatch for cases where the typed API is insufficient. +func (e *PodDisruptionBudgetSpecEditor) Raw() *policyv1.PodDisruptionBudgetSpec { + return e.spec +} + +// SetMinAvailable sets the minimum number of pods that must be available after an eviction. +// Accepts either an integer or a percentage string (e.g. "50%"). +func (e *PodDisruptionBudgetSpecEditor) SetMinAvailable(val intstr.IntOrString) { + e.spec.MinAvailable = &val +} + +// SetMaxUnavailable sets the maximum number of pods that can be unavailable after an eviction. +// Accepts either an integer or a percentage string (e.g. "25%"). +func (e *PodDisruptionBudgetSpecEditor) SetMaxUnavailable(val intstr.IntOrString) { + e.spec.MaxUnavailable = &val +} + +// ClearMinAvailable removes the MinAvailable constraint. +func (e *PodDisruptionBudgetSpecEditor) ClearMinAvailable() { + e.spec.MinAvailable = nil +} + +// ClearMaxUnavailable removes the MaxUnavailable constraint. +func (e *PodDisruptionBudgetSpecEditor) ClearMaxUnavailable() { + e.spec.MaxUnavailable = nil +} + +// SetSelector replaces the pod selector used by the PodDisruptionBudget. +func (e *PodDisruptionBudgetSpecEditor) SetSelector(selector *metav1.LabelSelector) { + e.spec.Selector = selector +} + +// SetUnhealthyPodEvictionPolicy sets the unhealthy pod eviction policy. +// +// Valid values are policyv1.IfHealthyBudget and policyv1.AlwaysAllow. +func (e *PodDisruptionBudgetSpecEditor) SetUnhealthyPodEvictionPolicy( + policy policyv1.UnhealthyPodEvictionPolicyType, +) { + e.spec.UnhealthyPodEvictionPolicy = &policy +} + +// ClearUnhealthyPodEvictionPolicy removes the unhealthy pod eviction policy, +// reverting to the cluster default. +func (e *PodDisruptionBudgetSpecEditor) ClearUnhealthyPodEvictionPolicy() { + e.spec.UnhealthyPodEvictionPolicy = nil +} diff --git a/pkg/mutation/editors/pdbspec_test.go b/pkg/mutation/editors/pdbspec_test.go new file mode 100644 index 00000000..b9ae49db --- /dev/null +++ b/pkg/mutation/editors/pdbspec_test.go @@ -0,0 +1,121 @@ +package editors + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestPodDisruptionBudgetSpecEditor_SetMinAvailable(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.SetMinAvailable(intstr.FromInt32(2)) + + require.NotNil(t, spec.MinAvailable) + assert.Equal(t, intstr.FromInt32(2), *spec.MinAvailable) +} + +func TestPodDisruptionBudgetSpecEditor_SetMinAvailable_Percentage(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.SetMinAvailable(intstr.FromString("50%")) + + require.NotNil(t, spec.MinAvailable) + assert.Equal(t, intstr.FromString("50%"), *spec.MinAvailable) +} + +func TestPodDisruptionBudgetSpecEditor_SetMaxUnavailable(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.SetMaxUnavailable(intstr.FromInt32(1)) + + require.NotNil(t, spec.MaxUnavailable) + assert.Equal(t, intstr.FromInt32(1), *spec.MaxUnavailable) +} + +func TestPodDisruptionBudgetSpecEditor_SetMaxUnavailable_Percentage(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.SetMaxUnavailable(intstr.FromString("25%")) + + require.NotNil(t, spec.MaxUnavailable) + assert.Equal(t, intstr.FromString("25%"), *spec.MaxUnavailable) +} + +func TestPodDisruptionBudgetSpecEditor_ClearMinAvailable(t *testing.T) { + t.Parallel() + val := intstr.FromInt32(2) + spec := &policyv1.PodDisruptionBudgetSpec{MinAvailable: &val} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.ClearMinAvailable() + + assert.Nil(t, spec.MinAvailable) +} + +func TestPodDisruptionBudgetSpecEditor_ClearMaxUnavailable(t *testing.T) { + t.Parallel() + val := intstr.FromInt32(1) + spec := &policyv1.PodDisruptionBudgetSpec{MaxUnavailable: &val} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.ClearMaxUnavailable() + + assert.Nil(t, spec.MaxUnavailable) +} + +func TestPodDisruptionBudgetSpecEditor_SetSelector(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + selector := &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "my-app"}, + } + e.SetSelector(selector) + + require.NotNil(t, spec.Selector) + assert.Equal(t, selector, spec.Selector) +} + +func TestPodDisruptionBudgetSpecEditor_SetUnhealthyPodEvictionPolicy(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.SetUnhealthyPodEvictionPolicy(policyv1.AlwaysAllow) + + require.NotNil(t, spec.UnhealthyPodEvictionPolicy) + assert.Equal(t, policyv1.AlwaysAllow, *spec.UnhealthyPodEvictionPolicy) +} + +func TestPodDisruptionBudgetSpecEditor_ClearUnhealthyPodEvictionPolicy(t *testing.T) { + t.Parallel() + policy := policyv1.IfHealthyBudget + spec := &policyv1.PodDisruptionBudgetSpec{UnhealthyPodEvictionPolicy: &policy} + e := NewPodDisruptionBudgetSpecEditor(spec) + + e.ClearUnhealthyPodEvictionPolicy() + + assert.Nil(t, spec.UnhealthyPodEvictionPolicy) +} + +func TestPodDisruptionBudgetSpecEditor_Raw(t *testing.T) { + t.Parallel() + spec := &policyv1.PodDisruptionBudgetSpec{} + e := NewPodDisruptionBudgetSpecEditor(spec) + + assert.Same(t, spec, e.Raw()) +} diff --git a/pkg/primitives/pdb/builder.go b/pkg/primitives/pdb/builder.go new file mode 100644 index 00000000..cb35035c --- /dev/null +++ b/pkg/primitives/pdb/builder.go @@ -0,0 +1,110 @@ +package pdb + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/internal/generic" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + policyv1 "k8s.io/api/policy/v1" +) + +// Builder is a configuration helper for creating and customizing a PodDisruptionBudget Resource. +// +// It provides a fluent API for registering mutations, field application flavors, +// and data extractors. Build() validates the configuration and returns an +// initialized Resource ready for use in a reconciliation loop. +type Builder struct { + base *generic.StaticBuilder[*policyv1.PodDisruptionBudget, *Mutator] +} + +// NewBuilder initializes a new Builder with the provided PodDisruptionBudget object. +// +// The PodDisruptionBudget object serves as the desired base state. During reconciliation +// the Resource will make the cluster's state match this base, modified by any +// registered mutations and flavors. +// +// The provided PodDisruptionBudget must have both Name and Namespace set, which is +// validated during the Build() call. +func NewBuilder(p *policyv1.PodDisruptionBudget) *Builder { + identityFunc := func(p *policyv1.PodDisruptionBudget) string { + return fmt.Sprintf("policy/v1/PodDisruptionBudget/%s/%s", p.Namespace, p.Name) + } + + return &Builder{ + base: generic.NewStaticBuilder[*policyv1.PodDisruptionBudget, *Mutator]( + p, + identityFunc, + DefaultFieldApplicator, + NewMutator, + ), + } +} + +// WithMutation registers a mutation for the PodDisruptionBudget. +// +// Mutations are applied sequentially during the Mutate() phase of reconciliation, +// after the baseline field applicator and any registered flavors have run. +// A mutation with a nil Feature is applied unconditionally; one with a non-nil +// Feature is applied only when that feature is enabled. +func (b *Builder) WithMutation(m Mutation) *Builder { + b.base.WithMutation(feature.Mutation[*Mutator](m)) + return b +} + +// WithCustomFieldApplicator sets a custom strategy for applying the desired +// state to the existing PodDisruptionBudget in the cluster. +// +// The default applicator (DefaultFieldApplicator) replaces the current object +// with a deep copy of the desired object. Use a custom applicator when other +// controllers manage fields you need to preserve. +// +// The applicator receives the current object from the API server and the desired +// object from the Resource, and is responsible for merging the desired changes +// into the current object. +func (b *Builder) WithCustomFieldApplicator( + applicator func(current, desired *policyv1.PodDisruptionBudget) error, +) *Builder { + b.base.WithCustomFieldApplicator(applicator) + return b +} + +// WithFieldApplicationFlavor registers a post-baseline field application flavor. +// +// Flavors run after the baseline applicator (default or custom) in registration +// order. They are typically used to preserve fields from the live cluster object +// that should not be overwritten by the desired state. +// +// A nil flavor is ignored. +func (b *Builder) WithFieldApplicationFlavor(flavor FieldApplicationFlavor) *Builder { + b.base.WithFieldApplicationFlavor(generic.FieldApplicationFlavor[*policyv1.PodDisruptionBudget](flavor)) + return b +} + +// WithDataExtractor registers a function to read values from the PodDisruptionBudget +// after it has been successfully reconciled. +// +// The extractor receives a value copy of the reconciled PodDisruptionBudget. This is +// useful for surfacing generated or updated values to other components or resources. +// +// A nil extractor is ignored. +func (b *Builder) WithDataExtractor(extractor func(policyv1.PodDisruptionBudget) error) *Builder { + if extractor != nil { + b.base.WithDataExtractor(func(p *policyv1.PodDisruptionBudget) error { + return extractor(*p) + }) + } + return b +} + +// Build validates the configuration and returns the initialized Resource. +// +// It returns an error if: +// - No PodDisruptionBudget object was provided. +// - The PodDisruptionBudget is missing a Name or Namespace. +func (b *Builder) Build() (*Resource, error) { + genericRes, err := b.base.Build() + if err != nil { + return nil, err + } + return &Resource{base: genericRes}, nil +} diff --git a/pkg/primitives/pdb/builder_test.go b/pkg/primitives/pdb/builder_test.go new file mode 100644 index 00000000..45bef64d --- /dev/null +++ b/pkg/primitives/pdb/builder_test.go @@ -0,0 +1,154 @@ +package pdb + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestBuilder_Build_Validation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pdb *policyv1.PodDisruptionBudget + expectedErr string + }{ + { + name: "nil pdb", + pdb: nil, + expectedErr: "object cannot be nil", + }, + { + name: "empty name", + pdb: &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test-ns"}, + }, + expectedErr: "object name cannot be empty", + }, + { + name: "empty namespace", + pdb: &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb"}, + }, + expectedErr: "object namespace cannot be empty", + }, + { + name: "valid pdb", + pdb: &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := NewBuilder(tt.pdb).Build() + if tt.expectedErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedErr) + assert.Nil(t, res) + } else { + require.NoError(t, err) + require.NotNil(t, res) + assert.Equal(t, "policy/v1/PodDisruptionBudget/test-ns/test-pdb", res.Identity()) + } + }) + } +} + +func TestBuilder_WithMutation(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + res, err := NewBuilder(p). + WithMutation(Mutation{Name: "test-mutation"}). + Build() + require.NoError(t, err) + assert.Len(t, res.base.Mutations, 1) + assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) +} + +func TestBuilder_WithCustomFieldApplicator(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + called := false + applicator := func(_, _ *policyv1.PodDisruptionBudget) error { + called = true + return nil + } + res, err := NewBuilder(p). + WithCustomFieldApplicator(applicator). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.CustomFieldApplicator) + _ = res.base.CustomFieldApplicator(nil, nil) + assert.True(t, called) +} + +func TestBuilder_WithFieldApplicationFlavor(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + res, err := NewBuilder(p). + WithFieldApplicationFlavor(PreserveCurrentLabels). + WithFieldApplicationFlavor(nil). // nil must be ignored + Build() + require.NoError(t, err) + assert.Len(t, res.base.FieldFlavors, 1) +} + +func TestBuilder_WithDataExtractor(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + called := false + extractor := func(_ policyv1.PodDisruptionBudget) error { + called = true + return nil + } + res, err := NewBuilder(p). + WithDataExtractor(extractor). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 1) + require.NoError(t, res.base.DataExtractors[0](&policyv1.PodDisruptionBudget{})) + assert.True(t, called) +} + +func TestBuilder_WithDataExtractor_Nil(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + res, err := NewBuilder(p). + WithDataExtractor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 0) +} + +func TestBuilder_WithDataExtractor_ErrorPropagated(t *testing.T) { + t.Parallel() + p := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, + } + res, err := NewBuilder(p). + WithDataExtractor(func(_ policyv1.PodDisruptionBudget) error { + return errors.New("extractor error") + }). + Build() + require.NoError(t, err) + err = res.base.DataExtractors[0](&policyv1.PodDisruptionBudget{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "extractor error") +} diff --git a/pkg/primitives/pdb/flavors.go b/pkg/primitives/pdb/flavors.go new file mode 100644 index 00000000..51e066fd --- /dev/null +++ b/pkg/primitives/pdb/flavors.go @@ -0,0 +1,25 @@ +package pdb + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/flavors" + policyv1 "k8s.io/api/policy/v1" +) + +// FieldApplicationFlavor defines a function signature for applying flavors to a +// PodDisruptionBudget resource. A flavor is called after the baseline field applicator +// has run and can be used to preserve or merge fields from the live cluster object. +type FieldApplicationFlavor flavors.FieldApplicationFlavor[*policyv1.PodDisruptionBudget] + +// PreserveCurrentLabels ensures that any labels present on the current live +// PodDisruptionBudget but missing from the applied (desired) object are preserved. +// If a label exists in both, the applied value wins. +func PreserveCurrentLabels(applied, current, desired *policyv1.PodDisruptionBudget) error { + return flavors.PreserveCurrentLabels[*policyv1.PodDisruptionBudget]()(applied, current, desired) +} + +// PreserveCurrentAnnotations ensures that any annotations present on the current +// live PodDisruptionBudget but missing from the applied (desired) object are preserved. +// If an annotation exists in both, the applied value wins. +func PreserveCurrentAnnotations(applied, current, desired *policyv1.PodDisruptionBudget) error { + return flavors.PreserveCurrentAnnotations[*policyv1.PodDisruptionBudget]()(applied, current, desired) +} diff --git a/pkg/primitives/pdb/flavors_test.go b/pkg/primitives/pdb/flavors_test.go new file mode 100644 index 00000000..c709548c --- /dev/null +++ b/pkg/primitives/pdb/flavors_test.go @@ -0,0 +1,124 @@ +package pdb + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestPreserveCurrentLabels(t *testing.T) { + t.Run("adds missing labels from current", func(t *testing.T) { + applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} + current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "applied", applied.Labels["keep"]) + assert.Equal(t, "current", applied.Labels["extra"]) + }) + + t.Run("applied value wins on overlap", func(t *testing.T) { + applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} + current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "applied", applied.Labels["key"]) + }) + + t.Run("handles nil applied labels", func(t *testing.T) { + applied := &policyv1.PodDisruptionBudget{} + current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "current", applied.Labels["extra"]) + }) +} + +func TestPreserveCurrentAnnotations(t *testing.T) { + t.Run("adds missing annotations from current", func(t *testing.T) { + applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} + current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) + assert.Equal(t, "applied", applied.Annotations["keep"]) + assert.Equal(t, "current", applied.Annotations["extra"]) + }) + + t.Run("applied value wins on overlap", func(t *testing.T) { + applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} + current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "current"}}} + + require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) + assert.Equal(t, "applied", applied.Annotations["key"]) + }) +} + +func TestFlavors_Integration(t *testing.T) { + minAvail := intstr.FromInt32(1) + desired := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pdb", + Namespace: "default", + Labels: map[string]string{"app": "desired"}, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvail, + }, + } + + t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + require.NoError(t, err) + + current := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"external": "keep", "app": "old"}, + }, + } + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, "desired", current.Labels["app"]) + assert.Equal(t, "keep", current.Labels["external"]) + }) + + t.Run("flavors run in registration order", func(t *testing.T) { + var order []string + flavor1 := func(_, _, _ *policyv1.PodDisruptionBudget) error { + order = append(order, "flavor1") + return nil + } + flavor2 := func(_, _, _ *policyv1.PodDisruptionBudget) error { + order = append(order, "flavor2") + return nil + } + + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(flavor1). + WithFieldApplicationFlavor(flavor2). + Build() + require.NoError(t, err) + + require.NoError(t, res.Mutate(&policyv1.PodDisruptionBudget{})) + assert.Equal(t, []string{"flavor1", "flavor2"}, order) + }) + + t.Run("flavor error is returned", func(t *testing.T) { + flavorErr := errors.New("flavor boom") + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(func(_, _, _ *policyv1.PodDisruptionBudget) error { + return flavorErr + }). + Build() + require.NoError(t, err) + + err = res.Mutate(&policyv1.PodDisruptionBudget{}) + require.Error(t, err) + assert.True(t, errors.Is(err, flavorErr)) + }) +} diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go new file mode 100644 index 00000000..b5405676 --- /dev/null +++ b/pkg/primitives/pdb/mutator.go @@ -0,0 +1,107 @@ +package pdb + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + policyv1 "k8s.io/api/policy/v1" +) + +// Mutation defines a mutation that is applied to a pdb Mutator +// only if its associated feature gate is enabled. +type Mutation feature.Mutation[*Mutator] + +type featurePlan struct { + metadataEdits []func(*editors.ObjectMetaEditor) error + specEdits []func(*editors.PodDisruptionBudgetSpecEditor) error +} + +// Mutator is a high-level helper for modifying a Kubernetes PodDisruptionBudget. +// +// It uses a "plan-and-apply" pattern: mutations are recorded first, then +// applied to the PodDisruptionBudget in a single controlled pass when Apply() is called. +// +// The Mutator maintains feature boundaries: each feature's mutations are planned +// together and applied in the order the features were registered. +// +// Mutator implements editors.ObjectMutator. +type Mutator struct { + pdb *policyv1.PodDisruptionBudget + + plans []featurePlan + active *featurePlan +} + +// NewMutator creates a new Mutator for the given PodDisruptionBudget. +func NewMutator(pdb *policyv1.PodDisruptionBudget) *Mutator { + m := &Mutator{pdb: pdb} + m.beginFeature() + return m +} + +// beginFeature starts a new feature planning scope. All subsequent mutation +// registrations will be grouped into this feature's plan. +func (m *Mutator) beginFeature() { + m.plans = append(m.plans, featurePlan{}) + m.active = &m.plans[len(m.plans)-1] +} + +// EditObjectMetadata records a mutation for the PodDisruptionBudget's own metadata. +// +// Metadata edits are applied before spec edits within the same feature. +// A nil edit function is ignored. +func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) { + if edit == nil { + return + } + m.active.metadataEdits = append(m.active.metadataEdits, edit) +} + +// EditSpec records a mutation for the PodDisruptionBudget's spec via a +// PodDisruptionBudgetSpecEditor. +// +// The editor provides structured operations (SetMinAvailable, SetMaxUnavailable, +// SetSelector, etc.) as well as Raw() for free-form access. Spec edits are applied +// after metadata edits within the same feature, in registration order. +// +// A nil edit function is ignored. +func (m *Mutator) EditSpec(edit func(*editors.PodDisruptionBudgetSpecEditor) error) { + if edit == nil { + return + } + m.active.specEdits = append(m.active.specEdits, edit) +} + +// Apply executes all recorded mutation intents on the underlying PodDisruptionBudget. +// +// Execution order across all registered features: +// +// 1. Metadata edits (in registration order within each feature) +// 2. Spec edits — EditSpec (in registration order within each feature) +// +// Features are applied in the order they were registered. Later features observe +// the PodDisruptionBudget as modified by all previous features. +func (m *Mutator) Apply() error { + for _, plan := range m.plans { + // 1. Metadata edits + if len(plan.metadataEdits) > 0 { + editor := editors.NewObjectMetaEditor(&m.pdb.ObjectMeta) + for _, edit := range plan.metadataEdits { + if err := edit(editor); err != nil { + return err + } + } + } + + // 2. Spec edits + if len(plan.specEdits) > 0 { + editor := editors.NewPodDisruptionBudgetSpecEditor(&m.pdb.Spec) + for _, edit := range plan.specEdits { + if err := edit(editor); err != nil { + return err + } + } + } + } + + return nil +} diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go new file mode 100644 index 00000000..7f81f795 --- /dev/null +++ b/pkg/primitives/pdb/mutator_test.go @@ -0,0 +1,182 @@ +package pdb + +import ( + "testing" + + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func newTestPDB() *policyv1.PodDisruptionBudget { + return &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pdb", + Namespace: "default", + }, + } +} + +// --- EditObjectMetadata --- + +func TestMutator_EditObjectMetadata(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", p.Labels["app"]) +} + +func TestMutator_EditObjectMetadata_Nil(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditObjectMetadata(nil) + assert.NoError(t, m.Apply()) +} + +// --- EditSpec --- + +func TestMutator_EditSpec_SetMinAvailable(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromInt32(2)) + return nil + }) + require.NoError(t, m.Apply()) + require.NotNil(t, p.Spec.MinAvailable) + assert.Equal(t, intstr.FromInt32(2), *p.Spec.MinAvailable) +} + +func TestMutator_EditSpec_SetMaxUnavailable(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMaxUnavailable(intstr.FromString("25%")) + return nil + }) + require.NoError(t, m.Apply()) + require.NotNil(t, p.Spec.MaxUnavailable) + assert.Equal(t, intstr.FromString("25%"), *p.Spec.MaxUnavailable) +} + +func TestMutator_EditSpec_SetSelector(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + selector := &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + } + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetSelector(selector) + return nil + }) + require.NoError(t, m.Apply()) + require.NotNil(t, p.Spec.Selector) + assert.Equal(t, selector, p.Spec.Selector) +} + +func TestMutator_EditSpec_SetUnhealthyPodEvictionPolicy(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetUnhealthyPodEvictionPolicy(policyv1.AlwaysAllow) + return nil + }) + require.NoError(t, m.Apply()) + require.NotNil(t, p.Spec.UnhealthyPodEvictionPolicy) + assert.Equal(t, policyv1.AlwaysAllow, *p.Spec.UnhealthyPodEvictionPolicy) +} + +func TestMutator_EditSpec_RawAccess(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + val := intstr.FromInt32(3) + e.Raw().MinAvailable = &val + return nil + }) + require.NoError(t, m.Apply()) + require.NotNil(t, p.Spec.MinAvailable) + assert.Equal(t, intstr.FromInt32(3), *p.Spec.MinAvailable) +} + +func TestMutator_EditSpec_Nil(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(nil) + assert.NoError(t, m.Apply()) +} + +// --- Execution order --- + +func TestMutator_OperationOrder(t *testing.T) { + // Within a feature: metadata edits run before spec edits. + p := newTestPDB() + m := NewMutator(p) + // Register in reverse logical order to confirm Apply() enforces category ordering. + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromInt32(1)) + return nil + }) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "tested") + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "tested", p.Labels["order"]) + require.NotNil(t, p.Spec.MinAvailable) + assert.Equal(t, intstr.FromInt32(1), *p.Spec.MinAvailable) +} + +func TestMutator_MultipleFeatures(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("feature1", "on") + return nil + }) + m.beginFeature() + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("feature2", "on") + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "on", p.Labels["feature1"]) + assert.Equal(t, "on", p.Labels["feature2"]) +} + +func TestMutator_EditSpec_ErrorPropagated(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditSpec(func(_ *editors.PodDisruptionBudgetSpecEditor) error { + return assert.AnError + }) + err := m.Apply() + require.Error(t, err) + assert.ErrorIs(t, err, assert.AnError) +} + +func TestMutator_EditObjectMetadata_ErrorPropagated(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + return assert.AnError + }) + err := m.Apply() + require.Error(t, err) + assert.ErrorIs(t, err, assert.AnError) +} + +// --- ObjectMutator interface --- + +func TestMutator_ImplementsObjectMutator(_ *testing.T) { + var _ editors.ObjectMutator = (*Mutator)(nil) +} diff --git a/pkg/primitives/pdb/resource.go b/pkg/primitives/pdb/resource.go new file mode 100644 index 00000000..77641204 --- /dev/null +++ b/pkg/primitives/pdb/resource.go @@ -0,0 +1,68 @@ +// Package pdb provides a builder and resource for managing Kubernetes PodDisruptionBudgets. +package pdb + +import ( + "github.com/sourcehawk/operator-component-framework/internal/generic" + policyv1 "k8s.io/api/policy/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// DefaultFieldApplicator replaces current with a deep copy of desired. +// +// This is the default baseline field application strategy for PodDisruptionBudget +// resources. Use a custom field applicator via Builder.WithCustomFieldApplicator +// if you need to preserve fields that other controllers manage. +func DefaultFieldApplicator(current, desired *policyv1.PodDisruptionBudget) error { + *current = *desired.DeepCopy() + return nil +} + +// Resource is a high-level abstraction for managing a Kubernetes PodDisruptionBudget +// within a controller's reconciliation loop. +// +// It implements the following component interfaces: +// - component.Resource: for basic identity and mutation behaviour. +// - component.DataExtractable: for exporting values after successful reconciliation. +// +// PodDisruptionBudget resources are static: they do not model convergence health, +// grace periods, or suspension. Use a workload or task primitive for resources +// that require those concepts. +type Resource struct { + base *generic.StaticResource[*policyv1.PodDisruptionBudget, *Mutator] +} + +// Identity returns a unique identifier for the PodDisruptionBudget in the format +// "policy/v1/PodDisruptionBudget//". +func (r *Resource) Identity() string { + return r.base.Identity() +} + +// Object returns a deep copy of the underlying Kubernetes PodDisruptionBudget object. +// +// The returned object implements client.Object, making it compatible with +// controller-runtime's Client for Create, Update, and Patch operations. +func (r *Resource) Object() (client.Object, error) { + return r.base.Object() +} + +// Mutate transforms the current state of a Kubernetes PodDisruptionBudget into the desired state. +// +// The mutation process follows this order: +// 1. Field application: the current object is updated to reflect the desired base state, +// using either DefaultFieldApplicator or a custom applicator if one is configured. +// 2. Field application flavors: any registered flavors are applied in registration order. +// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// +// This method is invoked by the framework during the Update phase of reconciliation. +func (r *Resource) Mutate(current client.Object) error { + return r.base.Mutate(current) +} + +// ExtractData executes all registered data extractor functions against a deep copy +// of the reconciled PodDisruptionBudget. +// +// This is called by the framework after successful reconciliation, allowing the +// component to read generated or updated values from the PodDisruptionBudget. +func (r *Resource) ExtractData() error { + return r.base.ExtractData() +} From 918cf610b39122d4d50d5d580fe317040707447c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:41:09 +0000 Subject: [PATCH 02/20] Add PDB primitive example Demonstrates PodDisruptionBudget management with feature-gated mutations that switch between MinAvailable and MaxUnavailable based on runtime conditions, version labelling, and field application flavors. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pdb-primitive/README.md | 30 +++++ examples/pdb-primitive/app/controller.go | 54 +++++++++ examples/pdb-primitive/features/mutations.go | 43 ++++++++ examples/pdb-primitive/main.go | 110 +++++++++++++++++++ examples/pdb-primitive/resources/pdb.go | 62 +++++++++++ 5 files changed, 299 insertions(+) create mode 100644 examples/pdb-primitive/README.md create mode 100644 examples/pdb-primitive/app/controller.go create mode 100644 examples/pdb-primitive/features/mutations.go create mode 100644 examples/pdb-primitive/main.go create mode 100644 examples/pdb-primitive/resources/pdb.go diff --git a/examples/pdb-primitive/README.md b/examples/pdb-primitive/README.md new file mode 100644 index 00000000..4285dfd6 --- /dev/null +++ b/examples/pdb-primitive/README.md @@ -0,0 +1,30 @@ +# PodDisruptionBudget Primitive Example + +This example demonstrates the usage of the `pdb` primitive within the operator component framework. +It shows how to manage a Kubernetes PodDisruptionBudget as a component of a larger application, utilising features like: + +- **Base Construction**: Initializing a PDB with a percentage-based `MinAvailable` and a label selector. +- **Feature Mutations**: Switching between `MinAvailable` and `MaxUnavailable` based on a feature toggle via `EditSpec`. +- **Metadata Mutations**: Setting version labels on the PDB via `EditObjectMetadata`. +- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. +- **Data Extraction**: Inspecting the reconciled PDB's disruption policy after each sync cycle. + +## Directory Structure + +- `app/`: Defines the controller that uses the component framework. The `ExampleApp` CRD is shared from `examples/shared/app`. +- `features/`: Contains modular feature definitions: + - `mutations.go`: version labelling and feature-gated strict availability. +- `resources/`: Contains the central `NewPDBResource` factory that assembles all features using `pdb.Builder`. +- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. + +## Running the Example + +```bash +go run examples/pdb-primitive/main.go +``` + +This will: +1. Initialize a fake Kubernetes client. +2. Create an `ExampleApp` owner object. +3. Reconcile through four spec variations, printing the PDB disruption policy after each cycle. +4. Print the resulting status conditions. diff --git a/examples/pdb-primitive/app/controller.go b/examples/pdb-primitive/app/controller.go new file mode 100644 index 00000000..a65d68a0 --- /dev/null +++ b/examples/pdb-primitive/app/controller.go @@ -0,0 +1,54 @@ +// Package app provides a sample controller using the PDB primitive. +package app + +import ( + "context" + + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/component" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// ExampleController reconciles an ExampleApp object using the component framework. +type ExampleController struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder + Metrics component.Recorder + + // NewPDBResource is a factory function to create the PDB resource. + // This allows us to inject the resource construction logic. + NewPDBResource func(*sharedapp.ExampleApp) (component.Resource, error) +} + +// Reconcile performs the reconciliation for a single ExampleApp. +func (r *ExampleController) Reconcile(ctx context.Context, owner *sharedapp.ExampleApp) error { + // 1. Build the PDB resource for this owner. + pdbResource, err := r.NewPDBResource(owner) + if err != nil { + return err + } + + // 2. Build the component that manages the PDB. + comp, err := component.NewComponentBuilder(). + WithName("example-app"). + WithConditionType("AppReady"). + WithResource(pdbResource, component.ResourceOptions{}). + Build() + if err != nil { + return err + } + + // 3. Execute the component reconciliation. + resCtx := component.ReconcileContext{ + Client: r.Client, + Scheme: r.Scheme, + Recorder: r.Recorder, + Metrics: r.Metrics, + Owner: owner, + } + + return comp.Reconcile(ctx, resCtx) +} diff --git a/examples/pdb-primitive/features/mutations.go b/examples/pdb-primitive/features/mutations.go new file mode 100644 index 00000000..2e14c1f1 --- /dev/null +++ b/examples/pdb-primitive/features/mutations.go @@ -0,0 +1,43 @@ +// Package features provides sample mutations for the PDB primitive example. +package features + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/pdb" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// VersionLabelMutation sets the app.kubernetes.io/version label on the PDB. +// It is always enabled. +func VersionLabelMutation(version string) pdb.Mutation { + return pdb.Mutation{ + Name: "version-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pdb.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +// StrictAvailabilityMutation switches the PDB from percentage-based MinAvailable +// to an absolute MaxUnavailable of 1 when metrics are enabled, indicating that +// the service requires stricter disruption control. +func StrictAvailabilityMutation(version string, metricsEnabled bool) pdb.Mutation { + return pdb.Mutation{ + Name: "strict-availability", + Feature: feature.NewResourceFeature(version, nil).When(metricsEnabled), + Mutate: func(m *pdb.Mutator) error { + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.ClearMinAvailable() + e.SetMaxUnavailable(intstr.FromInt32(1)) + return nil + }) + return nil + }, + } +} diff --git a/examples/pdb-primitive/main.go b/examples/pdb-primitive/main.go new file mode 100644 index 00000000..abbc5060 --- /dev/null +++ b/examples/pdb-primitive/main.go @@ -0,0 +1,110 @@ +// Package main is the entry point for the PDB primitive example. +package main + +import ( + "context" + "fmt" + "os" + + ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + "github.com/sourcehawk/operator-component-framework/examples/pdb-primitive/app" + "github.com/sourcehawk/operator-component-framework/examples/pdb-primitive/resources" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func main() { + // 1. Setup scheme and fake client. + scheme := runtime.NewScheme() + if err := sharedapp.AddToScheme(scheme); err != nil { + fmt.Fprintf(os.Stderr, "failed to add to scheme: %v\n", err) + os.Exit(1) + } + if err := policyv1.AddToScheme(scheme); err != nil { + fmt.Fprintf(os.Stderr, "failed to add policy/v1 to scheme: %v\n", err) + os.Exit(1) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&sharedapp.ExampleApp{}). + Build() + + // 2. Create an example Owner object. + owner := &sharedapp.ExampleApp{ + Spec: sharedapp.ExampleAppSpec{ + Version: "1.0.0", + EnableMetrics: false, + }, + } + owner.Name = "my-example-app" + owner.Namespace = "default" + + if err := fakeClient.Create(context.Background(), owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to create owner: %v\n", err) + os.Exit(1) + } + + // 3. Initialize the controller. + gauge := ocm.NewOperatorConditionsGauge("example") + controller := &app.ExampleController{ + Client: fakeClient, + Scheme: scheme, + Recorder: record.NewFakeRecorder(100), + Metrics: &ocm.ConditionMetricRecorder{ + Controller: "example-controller", + OperatorConditionsGauge: gauge, + }, + NewPDBResource: resources.NewPDBResource, + } + + // 4. Run reconciliation with multiple spec versions to demonstrate how + // feature-gated mutations modify the PDB disruption policy. + specs := []sharedapp.ExampleAppSpec{ + { + Version: "1.0.0", + EnableMetrics: false, + }, + { + Version: "1.1.0", // Version upgrade + EnableMetrics: false, + }, + { + Version: "1.1.0", + EnableMetrics: true, // Enable metrics → stricter availability + }, + { + Version: "1.1.0", + EnableMetrics: false, // Disable metrics → back to default + }, + } + + ctx := context.Background() + + for i, spec := range specs { + fmt.Printf("\n--- Step %d: Version=%s, Metrics=%v ---\n", + i+1, spec.Version, spec.EnableMetrics) + + owner.Spec = spec + if err := fakeClient.Update(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to update owner: %v\n", err) + os.Exit(1) + } + + fmt.Println("Running reconciliation...") + if err := controller.Reconcile(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "reconciliation failed: %v\n", err) + os.Exit(1) + } + + for _, cond := range owner.Status.Conditions { + fmt.Printf("Condition: %s, Status: %s, Reason: %s\n", + cond.Type, cond.Status, cond.Reason) + } + } + + fmt.Println("\nReconciliation sequence completed successfully!") +} diff --git a/examples/pdb-primitive/resources/pdb.go b/examples/pdb-primitive/resources/pdb.go new file mode 100644 index 00000000..734dc1b6 --- /dev/null +++ b/examples/pdb-primitive/resources/pdb.go @@ -0,0 +1,62 @@ +// Package resources provides resource implementations for the PDB primitive example. +package resources + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/examples/pdb-primitive/features" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/component" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/pdb" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// NewPDBResource constructs a PDB primitive resource with all the features. +func NewPDBResource(owner *sharedapp.ExampleApp) (component.Resource, error) { + // 1. Create the base PodDisruptionBudget object. + minAvailable := intstr.FromString("50%") + base := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-pdb", + Namespace: owner.Namespace, + Labels: map[string]string{ + "app": owner.Name, + }, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": owner.Name, + }, + }, + }, + } + + // 2. Initialize the PDB builder. + builder := pdb.NewBuilder(base) + + // 3. Register mutations in dependency order. + builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) + builder.WithMutation(features.StrictAvailabilityMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + + // 4. Preserve labels added by external controllers. + builder.WithFieldApplicationFlavor(pdb.PreserveCurrentLabels) + + // 5. Extract data from the reconciled PDB. + builder.WithDataExtractor(func(p policyv1.PodDisruptionBudget) error { + fmt.Printf("Reconciled PDB: %s\n", p.Name) + if p.Spec.MinAvailable != nil { + fmt.Printf(" MinAvailable: %s\n", p.Spec.MinAvailable.String()) + } + if p.Spec.MaxUnavailable != nil { + fmt.Printf(" MaxUnavailable: %s\n", p.Spec.MaxUnavailable.String()) + } + return nil + }) + + // 6. Build the final resource. + return builder.Build() +} From ca8baf010a2484c3d8f0543ca780def2414247e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:42:25 +0000 Subject: [PATCH 03/20] Add PDB primitive documentation Add docs/primitives/pdb.md covering building, mutations, editors, flavors, and guidance. Update docs/primitives.md to list the PDB primitive in the built-in primitives table and the PodDisruptionBudgetSpecEditor in the editors table. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 2 + docs/primitives/pdb.md | 283 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 285 insertions(+) create mode 100644 docs/primitives/pdb.md diff --git a/docs/primitives.md b/docs/primitives.md index 13fde6a6..cf17688f 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -107,6 +107,7 @@ Editors provide scoped, typed APIs for modifying specific parts of a resource: | `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | | `DeploymentSpecEditor` | Replicas, update strategy, label selectors | | `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | +| `PodDisruptionBudgetSpecEditor` | MinAvailable, MaxUnavailable, selector, eviction policy | | `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | Every editor exposes a `.Raw()` method for cases where the typed API is insufficient, giving direct access to the underlying Kubernetes struct while keeping the mutation scoped to that editor's target. @@ -130,6 +131,7 @@ Selectors are evaluated against the container list *after* any presence operatio |--------------------------------------|------------|-----------------------------------------| | `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | | `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| `pkg/primitives/pdb` | Static | [pdb.md](primitives/pdb.md) | ## Usage Examples diff --git a/docs/primitives/pdb.md b/docs/primitives/pdb.md new file mode 100644 index 00000000..02a34a9d --- /dev/null +++ b/docs/primitives/pdb.md @@ -0,0 +1,283 @@ +# PodDisruptionBudget Primitive + +The `pdb` primitive is the framework's built-in static abstraction for managing Kubernetes `PodDisruptionBudget` resources. It integrates with the component lifecycle and provides a structured mutation API for managing disruption policies and object metadata. + +## Capabilities + +| Capability | Detail | +|-----------------------|-------------------------------------------------------------------------------------------------| +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| **Mutation pipeline** | Typed editors for PDB spec and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled PDB after each sync cycle | + +## Building a PDB Primitive + +```go +import "github.com/sourcehawk/operator-component-framework/pkg/primitives/pdb" + +minAvailable := intstr.FromString("50%") +base := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "web-server-pdb", + Namespace: owner.Namespace, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &minAvailable, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web-server"}, + }, + }, +} + +resource, err := pdb.NewBuilder(base). + WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). + WithMutation(MyFeatureMutation(owner.Spec.Version)). + Build() +``` + +## Default Field Application + +`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object. This ensures every reconciliation cycle produces a clean, predictable state and avoids any drift from the desired baseline. + +Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: + +```go +resource, err := pdb.NewBuilder(base). + WithCustomFieldApplicator(func(current, desired *policyv1.PodDisruptionBudget) error { + // Only synchronise the spec; leave metadata untouched. + current.Spec = *desired.Spec.DeepCopy() + return nil + }). + Build() +``` + +## Mutations + +Mutations are the primary mechanism for modifying a `PodDisruptionBudget` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through typed editors. + +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: + +```go +func MyFeatureMutation(version string) pdb.Mutation { + return pdb.Mutation{ + Name: "my-feature", + Feature: feature.NewResourceFeature(version, nil), // always enabled + Mutate: func(m *pdb.Mutator) error { + // record edits here + return nil + }, + } +} +``` + +Mutations are applied in the order they are registered with the builder. If one mutation depends on a change made by another, register the dependency first. + +### Boolean-gated mutations + +```go +func StrictAvailabilityMutation(version string, enabled bool) pdb.Mutation { + return pdb.Mutation{ + Name: "strict-availability", + Feature: feature.NewResourceFeature(version, nil).When(enabled), + Mutate: func(m *pdb.Mutator) error { + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.ClearMinAvailable() + e.SetMaxUnavailable(intstr.FromInt32(1)) + return nil + }) + return nil + }, + } +} +``` + +### Version-gated mutations + +```go +var legacyConstraint = mustSemverConstraint("< 2.0.0") + +func LegacyPDBMutation(version string) pdb.Mutation { + return pdb.Mutation{ + Name: "legacy-pdb", + Feature: feature.NewResourceFeature( + version, + []feature.VersionConstraint{legacyConstraint}, + ), + Mutate: func(m *pdb.Mutator) error { + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromInt32(1)) + return nil + }) + return nil + }, + } +} +``` + +All version constraints and `When()` conditions must be satisfied for a mutation to apply. + +## Internal Mutation Ordering + +Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are recorded: + +| Step | Category | What it affects | +|------|----------------|----------------------------------------------------| +| 1 | Metadata edits | Labels and annotations on the `PodDisruptionBudget` | +| 2 | Spec edits | MinAvailable, MaxUnavailable, selector, eviction policy | + +Within each category, edits are applied in their registration order. Later features observe the PodDisruptionBudget as modified by all previous features. + +## Editors + +### PodDisruptionBudgetSpecEditor + +The primary API for modifying the PDB spec. Use `m.EditSpec` for full control: + +```go +m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromString("50%")) + e.SetSelector(&metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + }) + return nil +}) +``` + +#### SetMinAvailable and SetMaxUnavailable + +`SetMinAvailable` sets the minimum number of pods that must remain available during a disruption. `SetMaxUnavailable` sets the maximum number of pods that can be unavailable. Both accept `intstr.IntOrString` — either an integer count or a percentage string (e.g. `"50%"`). + +These fields are mutually exclusive in the Kubernetes API. Use `ClearMinAvailable` or `ClearMaxUnavailable` to remove the opposing constraint when switching between them: + +```go +m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.ClearMinAvailable() + e.SetMaxUnavailable(intstr.FromInt32(1)) + return nil +}) +``` + +#### SetSelector + +`SetSelector` replaces the pod selector used by the PDB: + +```go +m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetSelector(&metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web", "tier": "frontend"}, + }) + return nil +}) +``` + +#### SetUnhealthyPodEvictionPolicy + +`SetUnhealthyPodEvictionPolicy` controls how unhealthy pods are handled during eviction. Valid values are `policyv1.IfHealthyBudget` and `policyv1.AlwaysAllow`. Use `ClearUnhealthyPodEvictionPolicy` to revert to the cluster default: + +```go +m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetUnhealthyPodEvictionPolicy(policyv1.AlwaysAllow) + return nil +}) +``` + +#### Raw Escape Hatch + +`Raw()` returns the underlying `*policyv1.PodDisruptionBudgetSpec` for direct access when the typed API is insufficient: + +```go +m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.Raw().MinAvailable = &customValue + return nil +}) +``` + +### ObjectMetaEditor + +Modifies labels and annotations via `m.EditObjectMetadata`. + +Available methods: `EnsureLabel`, `RemoveLabel`, `EnsureAnnotation`, `RemoveAnnotation`, `Raw`. + +```go +m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + e.EnsureAnnotation("pdb.example.io/policy", "strict") + return nil +}) +``` + +## Flavors + +Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external controllers or other tools. + +### PreserveCurrentLabels + +Preserves labels present on the live object but absent from the applied desired state. Applied labels win on overlap. + +```go +resource, err := pdb.NewBuilder(base). + WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). + Build() +``` + +### PreserveCurrentAnnotations + +Preserves annotations present on the live object but absent from the applied desired state. Applied annotations win on overlap. + +```go +resource, err := pdb.NewBuilder(base). + WithFieldApplicationFlavor(pdb.PreserveCurrentAnnotations). + Build() +``` + +Multiple flavors can be registered and run in registration order. + +## Full Example: Feature-Gated Disruption Policy + +```go +func BasePDBMutation(version string) pdb.Mutation { + return pdb.Mutation{ + Name: "base-pdb", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pdb.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +func StrictAvailabilityMutation(version string, enabled bool) pdb.Mutation { + return pdb.Mutation{ + Name: "strict-availability", + Feature: feature.NewResourceFeature(version, nil).When(enabled), + Mutate: func(m *pdb.Mutator) error { + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.ClearMinAvailable() + e.SetMaxUnavailable(intstr.FromInt32(1)) + return nil + }) + return nil + }, + } +} + +resource, err := pdb.NewBuilder(base). + WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). + WithMutation(BasePDBMutation(owner.Spec.Version)). + WithMutation(StrictAvailabilityMutation(owner.Spec.Version, owner.Spec.StrictMode)). + Build() +``` + +When `StrictMode` is true, the PDB switches from percentage-based `MinAvailable` to an absolute `MaxUnavailable` of 1. When false, only the base mutation runs and the original `MinAvailable` from the baseline is preserved. Neither mutation needs to know about the other. + +## Guidance + +**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use `feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for boolean conditions. + +**`MinAvailable` and `MaxUnavailable` are mutually exclusive.** When switching between them, always clear the opposing field first. The typed API makes this explicit with `ClearMinAvailable` and `ClearMaxUnavailable`. + +**Register mutations in dependency order.** If mutation B relies on state set by mutation A, register A first. From 6f9a13f17c7c5a150c2b450684dc58477a1e958f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 19:53:09 +0000 Subject: [PATCH 04/20] preserve server-managed metadata in default field applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/resource.go | 11 +++--- pkg/primitives/pdb/resource_test.go | 55 +++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 pkg/primitives/pdb/resource_test.go diff --git a/pkg/primitives/pdb/resource.go b/pkg/primitives/pdb/resource.go index 77641204..073bacf7 100644 --- a/pkg/primitives/pdb/resource.go +++ b/pkg/primitives/pdb/resource.go @@ -7,13 +7,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator replaces current with a deep copy of desired. -// -// This is the default baseline field application strategy for PodDisruptionBudget -// resources. Use a custom field applicator via Builder.WithCustomFieldApplicator -// if you need to preserve fields that other controllers manage. +// DefaultFieldApplicator replaces current with a deep copy of desired while +// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.) +// and shared-controller fields (OwnerReferences, Finalizers) from the original +// current object. func DefaultFieldApplicator(current, desired *policyv1.PodDisruptionBudget) error { + original := current.DeepCopy() *current = *desired.DeepCopy() + generic.PreserveServerManagedFields(current, original) return nil } diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go new file mode 100644 index 00000000..f28792df --- /dev/null +++ b/pkg/primitives/pdb/resource_test.go @@ -0,0 +1,55 @@ +package pdb + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" +) + +func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { + current := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + ResourceVersion: "12345", + UID: "abc-def", + Generation: 3, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, + }, + Finalizers: []string{"finalizer.example.com"}, + }, + } + desired := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: ptr.To(intstr.FromInt32(2)), + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec and labels are applied + assert.Equal(t, "test", current.Labels["app"]) + assert.Equal(t, intstr.FromInt32(2), *current.Spec.MinAvailable) + + // Server-managed fields are preserved + assert.Equal(t, "12345", current.ResourceVersion) + assert.Equal(t, "abc-def", string(current.UID)) + assert.Equal(t, int64(3), current.Generation) + + // Shared-controller fields are preserved + assert.Len(t, current.OwnerReferences, 1) + assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) + assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) +} From 20fc3e87415eee69e236db1e2e8e9de1a22e82a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 03:23:28 +0000 Subject: [PATCH 05/20] fix: rename shadowed variable in PDB builder identity func Rename inner parameter `p` to `pdb` in the identityFunc closure to avoid shadowing the outer NewBuilder parameter, improving readability. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/builder.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/pdb/builder.go b/pkg/primitives/pdb/builder.go index cb35035c..f2e24653 100644 --- a/pkg/primitives/pdb/builder.go +++ b/pkg/primitives/pdb/builder.go @@ -26,8 +26,8 @@ type Builder struct { // The provided PodDisruptionBudget must have both Name and Namespace set, which is // validated during the Build() call. func NewBuilder(p *policyv1.PodDisruptionBudget) *Builder { - identityFunc := func(p *policyv1.PodDisruptionBudget) string { - return fmt.Sprintf("policy/v1/PodDisruptionBudget/%s/%s", p.Namespace, p.Name) + identityFunc := func(pdb *policyv1.PodDisruptionBudget) string { + return fmt.Sprintf("policy/v1/PodDisruptionBudget/%s/%s", pdb.Namespace, pdb.Name) } return &Builder{ From 0d96de1000edacb385fb7ea0b8ad024c7123e439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:43:34 +0000 Subject: [PATCH 06/20] fix: pdb mutator constructor should not call beginFeature Matches the fix applied to deployment and configmap mutators - initialize the plans slice inline instead of calling beginFeature(), which would create a duplicate empty feature when the generic helper also calls it. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go index b5405676..d66975b1 100644 --- a/pkg/primitives/pdb/mutator.go +++ b/pkg/primitives/pdb/mutator.go @@ -33,8 +33,11 @@ type Mutator struct { // NewMutator creates a new Mutator for the given PodDisruptionBudget. func NewMutator(pdb *policyv1.PodDisruptionBudget) *Mutator { - m := &Mutator{pdb: pdb} - m.beginFeature() + m := &Mutator{ + pdb: pdb, + plans: []featurePlan{{}}, + } + m.active = &m.plans[0] return m } From fa0bdb9ef422ce81fd2c1ec794776ec6c006ecf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 17:00:06 +0000 Subject: [PATCH 07/20] docs: update pdb DefaultFieldApplicator comment and docs to reflect PreserveStatus Update the function comment and documentation to accurately describe that DefaultFieldApplicator preserves the Status subresource in addition to server-managed metadata and shared-controller fields. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pdb.md | 2 +- pkg/primitives/pdb/resource.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/primitives/pdb.md b/docs/primitives/pdb.md index 02a34a9d..d5104194 100644 --- a/docs/primitives/pdb.md +++ b/docs/primitives/pdb.md @@ -38,7 +38,7 @@ resource, err := pdb.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object. This ensures every reconciliation cycle produces a clean, predictable state and avoids any drift from the desired baseline. +`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by the API server or other controllers. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: diff --git a/pkg/primitives/pdb/resource.go b/pkg/primitives/pdb/resource.go index 073bacf7..31b18bc0 100644 --- a/pkg/primitives/pdb/resource.go +++ b/pkg/primitives/pdb/resource.go @@ -8,13 +8,14 @@ import ( ) // DefaultFieldApplicator replaces current with a deep copy of desired while -// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.) -// and shared-controller fields (OwnerReferences, Finalizers) from the original -// current object. +// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), +// shared-controller fields (OwnerReferences, Finalizers), and the Status +// subresource from the original current object. func DefaultFieldApplicator(current, desired *policyv1.PodDisruptionBudget) error { original := current.DeepCopy() *current = *desired.DeepCopy() generic.PreserveServerManagedFields(current, original) + generic.PreserveStatus(current, original) return nil } From 4ad9bacb48dc6e0712c6b565d822a3719dc4a0c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 21:33:38 +0000 Subject: [PATCH 08/20] fix: export BeginFeature to satisfy FeatureMutator interface Align with main branch change in #43 that exported BeginFeature() on the FeatureMutator interface. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator.go | 4 ++-- pkg/primitives/pdb/mutator_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go index d66975b1..8aed81ee 100644 --- a/pkg/primitives/pdb/mutator.go +++ b/pkg/primitives/pdb/mutator.go @@ -41,9 +41,9 @@ func NewMutator(pdb *policyv1.PodDisruptionBudget) *Mutator { return m } -// beginFeature starts a new feature planning scope. All subsequent mutation +// BeginFeature starts a new feature planning scope. All subsequent mutation // registrations will be grouped into this feature's plan. -func (m *Mutator) beginFeature() { +func (m *Mutator) BeginFeature() { m.plans = append(m.plans, featurePlan{}) m.active = &m.plans[len(m.plans)-1] } diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go index 7f81f795..69196519 100644 --- a/pkg/primitives/pdb/mutator_test.go +++ b/pkg/primitives/pdb/mutator_test.go @@ -142,7 +142,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { e.EnsureLabel("feature1", "on") return nil }) - m.beginFeature() + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("feature2", "on") return nil From e28f28304c0925698ae4b945b1810558d4c6cba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:29:23 +0000 Subject: [PATCH 09/20] style: format markdown files with prettier Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 16 ++++---- docs/primitives/pdb.md | 70 +++++++++++++++++++++----------- examples/pdb-primitive/README.md | 10 +++-- 3 files changed, 60 insertions(+), 36 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index 8966fffa..b40a5be9 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -124,14 +124,14 @@ This design: Editors provide scoped, typed APIs for modifying specific parts of a resource: -| Editor | Scope | -| ---------------------- | ----------------------------------------------------------------------- | -| `ContainerEditor` | Environment variables, arguments, resource limits, ports | -| `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | -| `DeploymentSpecEditor` | Replicas, update strategy, label selectors | -| `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | -| `PodDisruptionBudgetSpecEditor` | MinAvailable, MaxUnavailable, selector, eviction policy | -| `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | +| Editor | Scope | +| ------------------------------- | ----------------------------------------------------------------------- | +| `ContainerEditor` | Environment variables, arguments, resource limits, ports | +| `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | +| `DeploymentSpecEditor` | Replicas, update strategy, label selectors | +| `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | +| `PodDisruptionBudgetSpecEditor` | MinAvailable, MaxUnavailable, selector, eviction policy | +| `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | Every editor exposes a `.Raw()` method for cases where the typed API is insufficient, giving direct access to the underlying Kubernetes struct while keeping the mutation scoped to that editor's target. diff --git a/docs/primitives/pdb.md b/docs/primitives/pdb.md index d5104194..63aacc9c 100644 --- a/docs/primitives/pdb.md +++ b/docs/primitives/pdb.md @@ -1,15 +1,17 @@ # PodDisruptionBudget Primitive -The `pdb` primitive is the framework's built-in static abstraction for managing Kubernetes `PodDisruptionBudget` resources. It integrates with the component lifecycle and provides a structured mutation API for managing disruption policies and object metadata. +The `pdb` primitive is the framework's built-in static abstraction for managing Kubernetes `PodDisruptionBudget` +resources. It integrates with the component lifecycle and provides a structured mutation API for managing disruption +policies and object metadata. ## Capabilities -| Capability | Detail | -|-----------------------|-------------------------------------------------------------------------------------------------| -| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | -| **Mutation pipeline** | Typed editors for PDB spec and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads generated or updated values back from the reconciled PDB after each sync cycle | +| Capability | Detail | +| --------------------- | ---------------------------------------------------------------------------------------------- | +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| **Mutation pipeline** | Typed editors for PDB spec and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled PDB after each sync cycle | ## Building a PDB Primitive @@ -38,7 +40,10 @@ resource, err := pdb.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by the API server or other controllers. +`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object, then restores +server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the +Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data +written by the API server or other controllers. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: @@ -54,9 +59,11 @@ resource, err := pdb.NewBuilder(base). ## Mutations -Mutations are the primary mechanism for modifying a `PodDisruptionBudget` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through typed editors. +Mutations are the primary mechanism for modifying a `PodDisruptionBudget` beyond its baseline. Each mutation is a named +function that receives a `*Mutator` and records edit intent through typed editors. -The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature +with no version constraints and no `When()` conditions is also always enabled: ```go func MyFeatureMutation(version string) pdb.Mutation { @@ -71,7 +78,8 @@ func MyFeatureMutation(version string) pdb.Mutation { } ``` -Mutations are applied in the order they are registered with the builder. If one mutation depends on a change made by another, register the dependency first. +Mutations are applied in the order they are registered with the builder. If one mutation depends on a change made by +another, register the dependency first. ### Boolean-gated mutations @@ -119,14 +127,16 @@ All version constraints and `When()` conditions must be satisfied for a mutation ## Internal Mutation Ordering -Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are recorded: +Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are +recorded: -| Step | Category | What it affects | -|------|----------------|----------------------------------------------------| -| 1 | Metadata edits | Labels and annotations on the `PodDisruptionBudget` | +| Step | Category | What it affects | +| ---- | -------------- | ------------------------------------------------------- | +| 1 | Metadata edits | Labels and annotations on the `PodDisruptionBudget` | | 2 | Spec edits | MinAvailable, MaxUnavailable, selector, eviction policy | -Within each category, edits are applied in their registration order. Later features observe the PodDisruptionBudget as modified by all previous features. +Within each category, edits are applied in their registration order. Later features observe the PodDisruptionBudget as +modified by all previous features. ## Editors @@ -146,9 +156,12 @@ m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { #### SetMinAvailable and SetMaxUnavailable -`SetMinAvailable` sets the minimum number of pods that must remain available during a disruption. `SetMaxUnavailable` sets the maximum number of pods that can be unavailable. Both accept `intstr.IntOrString` — either an integer count or a percentage string (e.g. `"50%"`). +`SetMinAvailable` sets the minimum number of pods that must remain available during a disruption. `SetMaxUnavailable` +sets the maximum number of pods that can be unavailable. Both accept `intstr.IntOrString` — either an integer count or a +percentage string (e.g. `"50%"`). -These fields are mutually exclusive in the Kubernetes API. Use `ClearMinAvailable` or `ClearMaxUnavailable` to remove the opposing constraint when switching between them: +These fields are mutually exclusive in the Kubernetes API. Use `ClearMinAvailable` or `ClearMaxUnavailable` to remove +the opposing constraint when switching between them: ```go m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { @@ -173,7 +186,9 @@ m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { #### SetUnhealthyPodEvictionPolicy -`SetUnhealthyPodEvictionPolicy` controls how unhealthy pods are handled during eviction. Valid values are `policyv1.IfHealthyBudget` and `policyv1.AlwaysAllow`. Use `ClearUnhealthyPodEvictionPolicy` to revert to the cluster default: +`SetUnhealthyPodEvictionPolicy` controls how unhealthy pods are handled during eviction. Valid values are +`policyv1.IfHealthyBudget` and `policyv1.AlwaysAllow`. Use `ClearUnhealthyPodEvictionPolicy` to revert to the cluster +default: ```go m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { @@ -209,7 +224,8 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { ## Flavors -Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external controllers or other tools. +Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external +controllers or other tools. ### PreserveCurrentLabels @@ -223,7 +239,8 @@ resource, err := pdb.NewBuilder(base). ### PreserveCurrentAnnotations -Preserves annotations present on the live object but absent from the applied desired state. Applied annotations win on overlap. +Preserves annotations present on the live object but absent from the applied desired state. Applied annotations win on +overlap. ```go resource, err := pdb.NewBuilder(base). @@ -272,12 +289,17 @@ resource, err := pdb.NewBuilder(base). Build() ``` -When `StrictMode` is true, the PDB switches from percentage-based `MinAvailable` to an absolute `MaxUnavailable` of 1. When false, only the base mutation runs and the original `MinAvailable` from the baseline is preserved. Neither mutation needs to know about the other. +When `StrictMode` is true, the PDB switches from percentage-based `MinAvailable` to an absolute `MaxUnavailable` of 1. +When false, only the base mutation runs and the original `MinAvailable` from the baseline is preserved. Neither mutation +needs to know about the other. ## Guidance -**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use `feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for boolean conditions. +**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use +`feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for +boolean conditions. -**`MinAvailable` and `MaxUnavailable` are mutually exclusive.** When switching between them, always clear the opposing field first. The typed API makes this explicit with `ClearMinAvailable` and `ClearMaxUnavailable`. +**`MinAvailable` and `MaxUnavailable` are mutually exclusive.** When switching between them, always clear the opposing +field first. The typed API makes this explicit with `ClearMinAvailable` and `ClearMaxUnavailable`. **Register mutations in dependency order.** If mutation B relies on state set by mutation A, register A first. diff --git a/examples/pdb-primitive/README.md b/examples/pdb-primitive/README.md index 4285dfd6..b0255b18 100644 --- a/examples/pdb-primitive/README.md +++ b/examples/pdb-primitive/README.md @@ -1,7 +1,7 @@ # PodDisruptionBudget Primitive Example -This example demonstrates the usage of the `pdb` primitive within the operator component framework. -It shows how to manage a Kubernetes PodDisruptionBudget as a component of a larger application, utilising features like: +This example demonstrates the usage of the `pdb` primitive within the operator component framework. It shows how to +manage a Kubernetes PodDisruptionBudget as a component of a larger application, utilising features like: - **Base Construction**: Initializing a PDB with a percentage-based `MinAvailable` and a label selector. - **Feature Mutations**: Switching between `MinAvailable` and `MaxUnavailable` based on a feature toggle via `EditSpec`. @@ -11,9 +11,10 @@ It shows how to manage a Kubernetes PodDisruptionBudget as a component of a larg ## Directory Structure -- `app/`: Defines the controller that uses the component framework. The `ExampleApp` CRD is shared from `examples/shared/app`. +- `app/`: Defines the controller that uses the component framework. The `ExampleApp` CRD is shared from + `examples/shared/app`. - `features/`: Contains modular feature definitions: - - `mutations.go`: version labelling and feature-gated strict availability. + - `mutations.go`: version labelling and feature-gated strict availability. - `resources/`: Contains the central `NewPDBResource` factory that assembles all features using `pdb.Builder`. - `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. @@ -24,6 +25,7 @@ go run examples/pdb-primitive/main.go ``` This will: + 1. Initialize a fake Kubernetes client. 2. Create an `ExampleApp` owner object. 3. Reconcile through four spec variations, printing the PDB disruption policy after each cycle. From 785838f736c39a3b6cde0b996cb1b6990e3f42eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:13:01 +0000 Subject: [PATCH 10/20] test: strengthen PDB test coverage for status preservation and operation ordering Add TestDefaultFieldApplicator_PreservesStatus to verify that non-zero Status fields survive field application, matching the deployment primitive's coverage. Update TestMutator_OperationOrder so the spec edit reads a label set by the metadata edit, making the test fail if category ordering regresses. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator_test.go | 15 +++++++++--- pkg/primitives/pdb/resource_test.go | 36 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go index 69196519..e5358fdc 100644 --- a/pkg/primitives/pdb/mutator_test.go +++ b/pkg/primitives/pdb/mutator_test.go @@ -117,21 +117,30 @@ func TestMutator_EditSpec_Nil(t *testing.T) { func TestMutator_OperationOrder(t *testing.T) { // Within a feature: metadata edits run before spec edits. + // The spec edit reads a label set by the metadata edit to prove ordering. p := newTestPDB() m := NewMutator(p) // Register in reverse logical order to confirm Apply() enforces category ordering. m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { - e.SetMinAvailable(intstr.FromInt32(1)) + // Branch on the label set by the metadata edit: if metadata ran first + // the label exists and we set MinAvailable to 1; otherwise we set it to 99. + if p.Labels["order"] == "metadata-first" { + e.SetMinAvailable(intstr.FromInt32(1)) + } else { + e.SetMinAvailable(intstr.FromInt32(99)) + } return nil }) m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { - e.EnsureLabel("order", "tested") + e.EnsureLabel("order", "metadata-first") return nil }) require.NoError(t, m.Apply()) - assert.Equal(t, "tested", p.Labels["order"]) + assert.Equal(t, "metadata-first", p.Labels["order"]) require.NotNil(t, p.Spec.MinAvailable) + // MinAvailable == 1 proves metadata edits ran before spec edits. + // If ordering regressed, MinAvailable would be 99. assert.Equal(t, intstr.FromInt32(1), *p.Spec.MinAvailable) } diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index f28792df..32083b0a 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -53,3 +53,39 @@ func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) } + +func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { + current := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Status: policyv1.PodDisruptionBudgetStatus{ + CurrentHealthy: 3, + DesiredHealthy: 2, + DisruptionsAllowed: 1, + ExpectedPods: 3, + }, + } + desired := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: ptr.To(intstr.FromInt32(2)), + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec is applied + assert.Equal(t, intstr.FromInt32(2), *current.Spec.MinAvailable) + + // Status from the live object is preserved + assert.Equal(t, int32(3), current.Status.CurrentHealthy) + assert.Equal(t, int32(2), current.Status.DesiredHealthy) + assert.Equal(t, int32(1), current.Status.DisruptionsAllowed) + assert.Equal(t, int32(3), current.Status.ExpectedPods) +} From 5e04901b624e22f5667cf3579d8701583202b859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:20:21 +0000 Subject: [PATCH 11/20] style: fix gofmt alignment in PDB test Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/resource_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index 32083b0a..5f2ff1ba 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -61,10 +61,10 @@ func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { Namespace: "default", }, Status: policyv1.PodDisruptionBudgetStatus{ - CurrentHealthy: 3, - DesiredHealthy: 2, + CurrentHealthy: 3, + DesiredHealthy: 2, DisruptionsAllowed: 1, - ExpectedPods: 3, + ExpectedPods: 3, }, } desired := &policyv1.PodDisruptionBudget{ From 8c55158e41108657503aa9779aa26386a5ac8cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:25:21 +0000 Subject: [PATCH 12/20] build: add pdb-primitive example to run-examples Makefile target Co-Authored-By: Claude Opus 4.6 (1M context) --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 29424227..b4e128a7 100644 --- a/Makefile +++ b/Makefile @@ -116,6 +116,7 @@ run-examples: ## Run all examples to verify they execute without error. go run ./examples/deployment-primitive/. go run ./examples/configmap-primitive/. go run ./examples/custom-resource-implementation/. + go run ./examples/pdb-primitive/. # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist From aef4f433c493749f891fd763aa6c894b3037ee45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 18:00:18 +0000 Subject: [PATCH 13/20] fix: do not initialize an empty plan on PDB mutator construction Align PDB Mutator with deployment/configmap: NewMutator no longer creates an initial feature plan. BeginFeature must be called before registering any mutations. Tests updated accordingly and constructor invariant tests added. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator.go | 11 +++-- pkg/primitives/pdb/mutator_test.go | 76 ++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go index 8aed81ee..2118a388 100644 --- a/pkg/primitives/pdb/mutator.go +++ b/pkg/primitives/pdb/mutator.go @@ -32,13 +32,14 @@ type Mutator struct { } // NewMutator creates a new Mutator for the given PodDisruptionBudget. +// +// It is typically used within a Feature's Mutation logic to express desired +// changes to the PodDisruptionBudget. BeginFeature must be called before +// registering any mutations. func NewMutator(pdb *policyv1.PodDisruptionBudget) *Mutator { - m := &Mutator{ - pdb: pdb, - plans: []featurePlan{{}}, + return &Mutator{ + pdb: pdb, } - m.active = &m.plans[0] - return m } // BeginFeature starts a new feature planning scope. All subsequent mutation diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go index e5358fdc..e08431df 100644 --- a/pkg/primitives/pdb/mutator_test.go +++ b/pkg/primitives/pdb/mutator_test.go @@ -25,6 +25,7 @@ func newTestPDB() *policyv1.PodDisruptionBudget { func TestMutator_EditObjectMetadata(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil @@ -36,6 +37,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { func TestMutator_EditObjectMetadata_Nil(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditObjectMetadata(nil) assert.NoError(t, m.Apply()) } @@ -45,6 +47,7 @@ func TestMutator_EditObjectMetadata_Nil(t *testing.T) { func TestMutator_EditSpec_SetMinAvailable(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMinAvailable(intstr.FromInt32(2)) return nil @@ -57,6 +60,7 @@ func TestMutator_EditSpec_SetMinAvailable(t *testing.T) { func TestMutator_EditSpec_SetMaxUnavailable(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMaxUnavailable(intstr.FromString("25%")) return nil @@ -69,6 +73,7 @@ func TestMutator_EditSpec_SetMaxUnavailable(t *testing.T) { func TestMutator_EditSpec_SetSelector(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() selector := &metav1.LabelSelector{ MatchLabels: map[string]string{"app": "web"}, } @@ -84,6 +89,7 @@ func TestMutator_EditSpec_SetSelector(t *testing.T) { func TestMutator_EditSpec_SetUnhealthyPodEvictionPolicy(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetUnhealthyPodEvictionPolicy(policyv1.AlwaysAllow) return nil @@ -96,6 +102,7 @@ func TestMutator_EditSpec_SetUnhealthyPodEvictionPolicy(t *testing.T) { func TestMutator_EditSpec_RawAccess(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { val := intstr.FromInt32(3) e.Raw().MinAvailable = &val @@ -109,6 +116,7 @@ func TestMutator_EditSpec_RawAccess(t *testing.T) { func TestMutator_EditSpec_Nil(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(nil) assert.NoError(t, m.Apply()) } @@ -120,6 +128,7 @@ func TestMutator_OperationOrder(t *testing.T) { // The spec edit reads a label set by the metadata edit to prove ordering. p := newTestPDB() m := NewMutator(p) + m.BeginFeature() // Register in reverse logical order to confirm Apply() enforces category ordering. m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { // Branch on the label set by the metadata edit: if metadata ran first @@ -147,6 +156,7 @@ func TestMutator_OperationOrder(t *testing.T) { func TestMutator_MultipleFeatures(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("feature1", "on") return nil @@ -165,6 +175,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { func TestMutator_EditSpec_ErrorPropagated(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditSpec(func(_ *editors.PodDisruptionBudgetSpecEditor) error { return assert.AnError }) @@ -176,6 +187,7 @@ func TestMutator_EditSpec_ErrorPropagated(t *testing.T) { func TestMutator_EditObjectMetadata_ErrorPropagated(t *testing.T) { p := newTestPDB() m := NewMutator(p) + m.BeginFeature() m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { return assert.AnError }) @@ -184,6 +196,70 @@ func TestMutator_EditObjectMetadata_ErrorPropagated(t *testing.T) { assert.ErrorIs(t, err, assert.AnError) } +// --- Constructor and feature plan invariants --- + +func TestNewMutator_InitializesNoPlan(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + + assert.Empty(t, m.plans, "NewMutator must not create any plans") + assert.Nil(t, m.active, "active plan must not be set") +} + +func TestBeginFeature_AddsExactlyOnePlan(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + + m.BeginFeature() + require.Len(t, m.plans, 1, "BeginFeature must add exactly one plan") + assert.Equal(t, &m.plans[0], m.active, "active must point to the new plan") + + m.BeginFeature() + require.Len(t, m.plans, 2) + assert.Equal(t, &m.plans[1], m.active) +} + +func TestBeginFeature_IsolatesFeaturePlans(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + + // Record a mutation in the first feature plan + m.BeginFeature() + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromInt32(1)) + return nil + }) + + // Start a new feature and record a different mutation + m.BeginFeature() + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("f1", "on") + return nil + }) + + // The first plan should have exactly one spec edit and no metadata edits + assert.Len(t, m.plans[0].specEdits, 1, "first plan should have one spec edit") + assert.Empty(t, m.plans[0].metadataEdits, "first plan should have no metadata edits") + // The second plan should have exactly one metadata edit and no spec edits + assert.Len(t, m.plans[1].metadataEdits, 1, "second plan should have one metadata edit") + assert.Empty(t, m.plans[1].specEdits, "second plan should have no spec edits") +} + +func TestMutator_SingleFeature_PlanCount(t *testing.T) { + p := newTestPDB() + m := NewMutator(p) + m.BeginFeature() + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMinAvailable(intstr.FromInt32(1)) + return nil + }) + + require.NoError(t, m.Apply()) + assert.Len(t, m.plans, 1, "no extra plans should be created during Apply") + require.NotNil(t, p.Spec.MinAvailable) + assert.Equal(t, intstr.FromInt32(1), *p.Spec.MinAvailable) +} + // --- ObjectMutator interface --- func TestMutator_ImplementsObjectMutator(_ *testing.T) { From 0b8f8369c4737617cdd3847b239103023de95bec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 14:08:27 +0000 Subject: [PATCH 14/20] refactor: remove field applicators and flavors from PDB primitive Align with the framework's switch to Server-Side Apply. Remove DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor, flavors.go, and all associated tests. Update builder to use the new three-argument NewStaticBuilder constructor. Rewrite resource tests to use Object() output instead of empty structs for Mutate(). Strip SSA and flavor sections from primitive docs. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pdb.md | 50 -------- pkg/primitives/pdb/builder.go | 41 +------ pkg/primitives/pdb/builder_test.go | 32 ----- pkg/primitives/pdb/flavors.go | 25 ---- pkg/primitives/pdb/flavors_test.go | 124 ------------------- pkg/primitives/pdb/resource.go | 18 +-- pkg/primitives/pdb/resource_test.go | 184 +++++++++++++++++++--------- 7 files changed, 131 insertions(+), 343 deletions(-) delete mode 100644 pkg/primitives/pdb/flavors.go delete mode 100644 pkg/primitives/pdb/flavors_test.go diff --git a/docs/primitives/pdb.md b/docs/primitives/pdb.md index 63aacc9c..ccd9a7bc 100644 --- a/docs/primitives/pdb.md +++ b/docs/primitives/pdb.md @@ -10,7 +10,6 @@ policies and object metadata. | --------------------- | ---------------------------------------------------------------------------------------------- | | **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | **Mutation pipeline** | Typed editors for PDB spec and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | | **Data extraction** | Reads generated or updated values back from the reconciled PDB after each sync cycle | ## Building a PDB Primitive @@ -33,30 +32,10 @@ base := &policyv1.PodDisruptionBudget{ } resource, err := pdb.NewBuilder(base). - WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). WithMutation(MyFeatureMutation(owner.Spec.Version)). Build() ``` -## Default Field Application - -`DefaultFieldApplicator` replaces the current PodDisruptionBudget with a deep copy of the desired object, then restores -server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the -Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data -written by the API server or other controllers. - -Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: - -```go -resource, err := pdb.NewBuilder(base). - WithCustomFieldApplicator(func(current, desired *policyv1.PodDisruptionBudget) error { - // Only synchronise the spec; leave metadata untouched. - current.Spec = *desired.Spec.DeepCopy() - return nil - }). - Build() -``` - ## Mutations Mutations are the primary mechanism for modifying a `PodDisruptionBudget` beyond its baseline. Each mutation is a named @@ -222,34 +201,6 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { }) ``` -## Flavors - -Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external -controllers or other tools. - -### PreserveCurrentLabels - -Preserves labels present on the live object but absent from the applied desired state. Applied labels win on overlap. - -```go -resource, err := pdb.NewBuilder(base). - WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). - Build() -``` - -### PreserveCurrentAnnotations - -Preserves annotations present on the live object but absent from the applied desired state. Applied annotations win on -overlap. - -```go -resource, err := pdb.NewBuilder(base). - WithFieldApplicationFlavor(pdb.PreserveCurrentAnnotations). - Build() -``` - -Multiple flavors can be registered and run in registration order. - ## Full Example: Feature-Gated Disruption Policy ```go @@ -283,7 +234,6 @@ func StrictAvailabilityMutation(version string, enabled bool) pdb.Mutation { } resource, err := pdb.NewBuilder(base). - WithFieldApplicationFlavor(pdb.PreserveCurrentLabels). WithMutation(BasePDBMutation(owner.Spec.Version)). WithMutation(StrictAvailabilityMutation(owner.Spec.Version, owner.Spec.StrictMode)). Build() diff --git a/pkg/primitives/pdb/builder.go b/pkg/primitives/pdb/builder.go index f2e24653..008fa5eb 100644 --- a/pkg/primitives/pdb/builder.go +++ b/pkg/primitives/pdb/builder.go @@ -10,9 +10,9 @@ import ( // Builder is a configuration helper for creating and customizing a PodDisruptionBudget Resource. // -// It provides a fluent API for registering mutations, field application flavors, -// and data extractors. Build() validates the configuration and returns an -// initialized Resource ready for use in a reconciliation loop. +// It provides a fluent API for registering mutations and data extractors. +// Build() validates the configuration and returns an initialized Resource +// ready for use in a reconciliation loop. type Builder struct { base *generic.StaticBuilder[*policyv1.PodDisruptionBudget, *Mutator] } @@ -21,7 +21,7 @@ type Builder struct { // // The PodDisruptionBudget object serves as the desired base state. During reconciliation // the Resource will make the cluster's state match this base, modified by any -// registered mutations and flavors. +// registered mutations. // // The provided PodDisruptionBudget must have both Name and Namespace set, which is // validated during the Build() call. @@ -34,7 +34,6 @@ func NewBuilder(p *policyv1.PodDisruptionBudget) *Builder { base: generic.NewStaticBuilder[*policyv1.PodDisruptionBudget, *Mutator]( p, identityFunc, - DefaultFieldApplicator, NewMutator, ), } @@ -42,8 +41,7 @@ func NewBuilder(p *policyv1.PodDisruptionBudget) *Builder { // WithMutation registers a mutation for the PodDisruptionBudget. // -// Mutations are applied sequentially during the Mutate() phase of reconciliation, -// after the baseline field applicator and any registered flavors have run. +// Mutations are applied sequentially during the Mutate() phase of reconciliation. // A mutation with a nil Feature is applied unconditionally; one with a non-nil // Feature is applied only when that feature is enabled. func (b *Builder) WithMutation(m Mutation) *Builder { @@ -51,35 +49,6 @@ func (b *Builder) WithMutation(m Mutation) *Builder { return b } -// WithCustomFieldApplicator sets a custom strategy for applying the desired -// state to the existing PodDisruptionBudget in the cluster. -// -// The default applicator (DefaultFieldApplicator) replaces the current object -// with a deep copy of the desired object. Use a custom applicator when other -// controllers manage fields you need to preserve. -// -// The applicator receives the current object from the API server and the desired -// object from the Resource, and is responsible for merging the desired changes -// into the current object. -func (b *Builder) WithCustomFieldApplicator( - applicator func(current, desired *policyv1.PodDisruptionBudget) error, -) *Builder { - b.base.WithCustomFieldApplicator(applicator) - return b -} - -// WithFieldApplicationFlavor registers a post-baseline field application flavor. -// -// Flavors run after the baseline applicator (default or custom) in registration -// order. They are typically used to preserve fields from the live cluster object -// that should not be overwritten by the desired state. -// -// A nil flavor is ignored. -func (b *Builder) WithFieldApplicationFlavor(flavor FieldApplicationFlavor) *Builder { - b.base.WithFieldApplicationFlavor(generic.FieldApplicationFlavor[*policyv1.PodDisruptionBudget](flavor)) - return b -} - // WithDataExtractor registers a function to read values from the PodDisruptionBudget // after it has been successfully reconciled. // diff --git a/pkg/primitives/pdb/builder_test.go b/pkg/primitives/pdb/builder_test.go index 45bef64d..12d93089 100644 --- a/pkg/primitives/pdb/builder_test.go +++ b/pkg/primitives/pdb/builder_test.go @@ -74,38 +74,6 @@ func TestBuilder_WithMutation(t *testing.T) { assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) } -func TestBuilder_WithCustomFieldApplicator(t *testing.T) { - t.Parallel() - p := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, - } - called := false - applicator := func(_, _ *policyv1.PodDisruptionBudget) error { - called = true - return nil - } - res, err := NewBuilder(p). - WithCustomFieldApplicator(applicator). - Build() - require.NoError(t, err) - require.NotNil(t, res.base.CustomFieldApplicator) - _ = res.base.CustomFieldApplicator(nil, nil) - assert.True(t, called) -} - -func TestBuilder_WithFieldApplicationFlavor(t *testing.T) { - t.Parallel() - p := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{Name: "test-pdb", Namespace: "test-ns"}, - } - res, err := NewBuilder(p). - WithFieldApplicationFlavor(PreserveCurrentLabels). - WithFieldApplicationFlavor(nil). // nil must be ignored - Build() - require.NoError(t, err) - assert.Len(t, res.base.FieldFlavors, 1) -} - func TestBuilder_WithDataExtractor(t *testing.T) { t.Parallel() p := &policyv1.PodDisruptionBudget{ diff --git a/pkg/primitives/pdb/flavors.go b/pkg/primitives/pdb/flavors.go deleted file mode 100644 index 51e066fd..00000000 --- a/pkg/primitives/pdb/flavors.go +++ /dev/null @@ -1,25 +0,0 @@ -package pdb - -import ( - "github.com/sourcehawk/operator-component-framework/pkg/flavors" - policyv1 "k8s.io/api/policy/v1" -) - -// FieldApplicationFlavor defines a function signature for applying flavors to a -// PodDisruptionBudget resource. A flavor is called after the baseline field applicator -// has run and can be used to preserve or merge fields from the live cluster object. -type FieldApplicationFlavor flavors.FieldApplicationFlavor[*policyv1.PodDisruptionBudget] - -// PreserveCurrentLabels ensures that any labels present on the current live -// PodDisruptionBudget but missing from the applied (desired) object are preserved. -// If a label exists in both, the applied value wins. -func PreserveCurrentLabels(applied, current, desired *policyv1.PodDisruptionBudget) error { - return flavors.PreserveCurrentLabels[*policyv1.PodDisruptionBudget]()(applied, current, desired) -} - -// PreserveCurrentAnnotations ensures that any annotations present on the current -// live PodDisruptionBudget but missing from the applied (desired) object are preserved. -// If an annotation exists in both, the applied value wins. -func PreserveCurrentAnnotations(applied, current, desired *policyv1.PodDisruptionBudget) error { - return flavors.PreserveCurrentAnnotations[*policyv1.PodDisruptionBudget]()(applied, current, desired) -} diff --git a/pkg/primitives/pdb/flavors_test.go b/pkg/primitives/pdb/flavors_test.go deleted file mode 100644 index c709548c..00000000 --- a/pkg/primitives/pdb/flavors_test.go +++ /dev/null @@ -1,124 +0,0 @@ -package pdb - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - policyv1 "k8s.io/api/policy/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" -) - -func TestPreserveCurrentLabels(t *testing.T) { - t.Run("adds missing labels from current", func(t *testing.T) { - applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} - current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "applied", applied.Labels["keep"]) - assert.Equal(t, "current", applied.Labels["extra"]) - }) - - t.Run("applied value wins on overlap", func(t *testing.T) { - applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} - current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "applied", applied.Labels["key"]) - }) - - t.Run("handles nil applied labels", func(t *testing.T) { - applied := &policyv1.PodDisruptionBudget{} - current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "current", applied.Labels["extra"]) - }) -} - -func TestPreserveCurrentAnnotations(t *testing.T) { - t.Run("adds missing annotations from current", func(t *testing.T) { - applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} - current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) - assert.Equal(t, "applied", applied.Annotations["keep"]) - assert.Equal(t, "current", applied.Annotations["extra"]) - }) - - t.Run("applied value wins on overlap", func(t *testing.T) { - applied := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} - current := &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "current"}}} - - require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) - assert.Equal(t, "applied", applied.Annotations["key"]) - }) -} - -func TestFlavors_Integration(t *testing.T) { - minAvail := intstr.FromInt32(1) - desired := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pdb", - Namespace: "default", - Labels: map[string]string{"app": "desired"}, - }, - Spec: policyv1.PodDisruptionBudgetSpec{ - MinAvailable: &minAvail, - }, - } - - t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(PreserveCurrentLabels). - Build() - require.NoError(t, err) - - current := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"external": "keep", "app": "old"}, - }, - } - require.NoError(t, res.Mutate(current)) - - assert.Equal(t, "desired", current.Labels["app"]) - assert.Equal(t, "keep", current.Labels["external"]) - }) - - t.Run("flavors run in registration order", func(t *testing.T) { - var order []string - flavor1 := func(_, _, _ *policyv1.PodDisruptionBudget) error { - order = append(order, "flavor1") - return nil - } - flavor2 := func(_, _, _ *policyv1.PodDisruptionBudget) error { - order = append(order, "flavor2") - return nil - } - - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(flavor1). - WithFieldApplicationFlavor(flavor2). - Build() - require.NoError(t, err) - - require.NoError(t, res.Mutate(&policyv1.PodDisruptionBudget{})) - assert.Equal(t, []string{"flavor1", "flavor2"}, order) - }) - - t.Run("flavor error is returned", func(t *testing.T) { - flavorErr := errors.New("flavor boom") - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(func(_, _, _ *policyv1.PodDisruptionBudget) error { - return flavorErr - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&policyv1.PodDisruptionBudget{}) - require.Error(t, err) - assert.True(t, errors.Is(err, flavorErr)) - }) -} diff --git a/pkg/primitives/pdb/resource.go b/pkg/primitives/pdb/resource.go index 31b18bc0..7624f813 100644 --- a/pkg/primitives/pdb/resource.go +++ b/pkg/primitives/pdb/resource.go @@ -7,18 +7,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator replaces current with a deep copy of desired while -// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), -// shared-controller fields (OwnerReferences, Finalizers), and the Status -// subresource from the original current object. -func DefaultFieldApplicator(current, desired *policyv1.PodDisruptionBudget) error { - original := current.DeepCopy() - *current = *desired.DeepCopy() - generic.PreserveServerManagedFields(current, original) - generic.PreserveStatus(current, original) - return nil -} - // Resource is a high-level abstraction for managing a Kubernetes PodDisruptionBudget // within a controller's reconciliation loop. // @@ -50,10 +38,8 @@ func (r *Resource) Object() (client.Object, error) { // Mutate transforms the current state of a Kubernetes PodDisruptionBudget into the desired state. // // The mutation process follows this order: -// 1. Field application: the current object is updated to reflect the desired base state, -// using either DefaultFieldApplicator or a custom applicator if one is configured. -// 2. Field application flavors: any registered flavors are applied in registration order. -// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// 1. The desired base state is applied to the current object. +// 2. Feature mutations: all registered feature-gated mutations are applied in order. // // This method is invoked by the framework during the Update phase of reconciliation. func (r *Resource) Mutate(current client.Object) error { diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index 5f2ff1ba..2dea72b2 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -1,8 +1,11 @@ package pdb import ( + "errors" "testing" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" policyv1 "k8s.io/api/policy/v1" @@ -11,81 +14,142 @@ import ( "k8s.io/utils/ptr" ) -func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { - current := &policyv1.PodDisruptionBudget{ +func newValidPDB() *policyv1.PodDisruptionBudget { + return &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - ResourceVersion: "12345", - UID: "abc-def", - Generation: 3, - OwnerReferences: []metav1.OwnerReference{ - {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, - }, - Finalizers: []string{"finalizer.example.com"}, - }, - } - desired := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - Labels: map[string]string{"app": "test"}, + Name: "test-pdb", + Namespace: "test-ns", }, Spec: policyv1.PodDisruptionBudgetSpec{ MinAvailable: ptr.To(intstr.FromInt32(2)), }, } +} - err := DefaultFieldApplicator(current, desired) +func TestResource_Identity(t *testing.T) { + res, err := NewBuilder(newValidPDB()).Build() require.NoError(t, err) + assert.Equal(t, "policy/v1/PodDisruptionBudget/test-ns/test-pdb", res.Identity()) +} - // Desired spec and labels are applied - assert.Equal(t, "test", current.Labels["app"]) - assert.Equal(t, intstr.FromInt32(2), *current.Spec.MinAvailable) +func TestResource_Object(t *testing.T) { + p := newValidPDB() + res, err := NewBuilder(p).Build() + require.NoError(t, err) + + obj, err := res.Object() + require.NoError(t, err) - // Server-managed fields are preserved - assert.Equal(t, "12345", current.ResourceVersion) - assert.Equal(t, "abc-def", string(current.UID)) - assert.Equal(t, int64(3), current.Generation) + got, ok := obj.(*policyv1.PodDisruptionBudget) + require.True(t, ok) + assert.Equal(t, p.Name, got.Name) + assert.Equal(t, p.Namespace, got.Namespace) - // Shared-controller fields are preserved - assert.Len(t, current.OwnerReferences, 1) - assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) - assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) + // Must be a deep copy. + got.Name = "changed" + assert.Equal(t, "test-pdb", p.Name) } -func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { - current := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Status: policyv1.PodDisruptionBudgetStatus{ - CurrentHealthy: 3, - DesiredHealthy: 2, - DisruptionsAllowed: 1, - ExpectedPods: 3, - }, - } - desired := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Spec: policyv1.PodDisruptionBudgetSpec{ - MinAvailable: ptr.To(intstr.FromInt32(2)), - }, - } +func TestResource_Mutate(t *testing.T) { + desired := newValidPDB() + res, err := NewBuilder(desired).Build() + require.NoError(t, err) - err := DefaultFieldApplicator(current, desired) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) + + got := obj.(*policyv1.PodDisruptionBudget) + assert.Equal(t, intstr.FromInt32(2), *got.Spec.MinAvailable) +} - // Desired spec is applied - assert.Equal(t, intstr.FromInt32(2), *current.Spec.MinAvailable) +func TestResource_Mutate_WithMutation(t *testing.T) { + desired := newValidPDB() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "set-max-unavailable", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + e.SetMaxUnavailable(intstr.FromInt32(1)) + return nil + }) + return nil + }, + }). + Build() + require.NoError(t, err) + + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) + + got := obj.(*policyv1.PodDisruptionBudget) + require.NotNil(t, got.Spec.MaxUnavailable) + assert.Equal(t, intstr.FromInt32(1), *got.Spec.MaxUnavailable) +} + +func TestResource_Mutate_FeatureOrdering(t *testing.T) { + desired := newValidPDB() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "feature-a", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "a") + return nil + }) + return nil + }, + }). + WithMutation(Mutation{ + Name: "feature-b", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "b") + return nil + }) + return nil + }, + }). + Build() + require.NoError(t, err) + + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) + + got := obj.(*policyv1.PodDisruptionBudget) + assert.Equal(t, "b", got.Labels["order"]) +} + +func TestResource_ExtractData(t *testing.T) { + p := newValidPDB() + + var extracted int32 + res, err := NewBuilder(p). + WithDataExtractor(func(pdb policyv1.PodDisruptionBudget) error { + extracted = pdb.Spec.MinAvailable.IntVal + return nil + }). + Build() + require.NoError(t, err) + + require.NoError(t, res.ExtractData()) + assert.Equal(t, int32(2), extracted) +} + +func TestResource_ExtractData_Error(t *testing.T) { + res, err := NewBuilder(newValidPDB()). + WithDataExtractor(func(_ policyv1.PodDisruptionBudget) error { + return errors.New("extract error") + }). + Build() + require.NoError(t, err) - // Status from the live object is preserved - assert.Equal(t, int32(3), current.Status.CurrentHealthy) - assert.Equal(t, int32(2), current.Status.DesiredHealthy) - assert.Equal(t, int32(1), current.Status.DisruptionsAllowed) - assert.Equal(t, int32(3), current.Status.ExpectedPods) + err = res.ExtractData() + require.Error(t, err) + assert.Contains(t, err.Error(), "extract error") } From c2d4c7dd8103b3f44d2464799f6e10a0e15472c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:52:57 +0000 Subject: [PATCH 15/20] fix: remove references to deleted field application flavor API in example The previous refactor removed WithFieldApplicationFlavor and PreserveCurrentLabels from the PDB builder but the example still referenced them, breaking the lint CI. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pdb-primitive/resources/pdb.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/pdb-primitive/resources/pdb.go b/examples/pdb-primitive/resources/pdb.go index 734dc1b6..30f7fb95 100644 --- a/examples/pdb-primitive/resources/pdb.go +++ b/examples/pdb-primitive/resources/pdb.go @@ -42,10 +42,7 @@ func NewPDBResource(owner *sharedapp.ExampleApp) (component.Resource, error) { builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) builder.WithMutation(features.StrictAvailabilityMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) - // 4. Preserve labels added by external controllers. - builder.WithFieldApplicationFlavor(pdb.PreserveCurrentLabels) - - // 5. Extract data from the reconciled PDB. + // 4. Extract data from the reconciled PDB. builder.WithDataExtractor(func(p policyv1.PodDisruptionBudget) error { fmt.Printf("Reconciled PDB: %s\n", p.Name) if p.Spec.MinAvailable != nil { @@ -57,6 +54,6 @@ func NewPDBResource(owner *sharedapp.ExampleApp) (component.Resource, error) { return nil }) - // 6. Build the final resource. + // 5. Build the final resource. return builder.Build() } From 13c07f30d6541f8f96b34768eed5c7c15c8d0ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:02:56 +0000 Subject: [PATCH 16/20] docs: remove stale PreserveCurrentLabels reference from PDB example README The Field Flavors bullet point referenced PreserveCurrentLabels which was removed in the prior refactor. Update the README to match the actual API. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pdb-primitive/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/pdb-primitive/README.md b/examples/pdb-primitive/README.md index b0255b18..bb8b20c5 100644 --- a/examples/pdb-primitive/README.md +++ b/examples/pdb-primitive/README.md @@ -6,7 +6,6 @@ manage a Kubernetes PodDisruptionBudget as a component of a larger application, - **Base Construction**: Initializing a PDB with a percentage-based `MinAvailable` and a label selector. - **Feature Mutations**: Switching between `MinAvailable` and `MaxUnavailable` based on a feature toggle via `EditSpec`. - **Metadata Mutations**: Setting version labels on the PDB via `EditObjectMetadata`. -- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. - **Data Extraction**: Inspecting the reconciled PDB's disruption policy after each sync cycle. ## Directory Structure From b6a8aa1d8778aa8f9160fcc808561665d6243e49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:16:55 +0000 Subject: [PATCH 17/20] fix: add defensive nil-active checks and selector to PDB test helper Address Copilot review feedback: - EditObjectMetadata/EditSpec now return error and check for nil active feature scope (ErrNoActiveFeature) instead of panicking on nil pointer - Update Mutator doc comment to note it intentionally diverges from the ObjectMutator interface to provide safer error-returning methods - Replace interface assertion test with ErrNoActiveFeature coverage - Add Selector to newValidPDB helper for Kubernetes API validity - Propagate errors from EditObjectMetadata/EditSpec in example mutations Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pdb-primitive/features/mutations.go | 6 ++--- pkg/primitives/pdb/mutator.go | 26 ++++++++++++++++---- pkg/primitives/pdb/mutator_test.go | 18 +++++++++++--- pkg/primitives/pdb/resource_test.go | 5 ++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/examples/pdb-primitive/features/mutations.go b/examples/pdb-primitive/features/mutations.go index 2e14c1f1..1029b82a 100644 --- a/examples/pdb-primitive/features/mutations.go +++ b/examples/pdb-primitive/features/mutations.go @@ -15,11 +15,10 @@ func VersionLabelMutation(version string) pdb.Mutation { Name: "version-label", Feature: feature.NewResourceFeature(version, nil), Mutate: func(m *pdb.Mutator) error { - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + return m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app.kubernetes.io/version", version) return nil }) - return nil }, } } @@ -32,12 +31,11 @@ func StrictAvailabilityMutation(version string, metricsEnabled bool) pdb.Mutatio Name: "strict-availability", Feature: feature.NewResourceFeature(version, nil).When(metricsEnabled), Mutate: func(m *pdb.Mutator) error { - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + return m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.ClearMinAvailable() e.SetMaxUnavailable(intstr.FromInt32(1)) return nil }) - return nil }, } } diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go index 2118a388..a725e636 100644 --- a/pkg/primitives/pdb/mutator.go +++ b/pkg/primitives/pdb/mutator.go @@ -1,11 +1,17 @@ package pdb import ( + "errors" + "github.com/sourcehawk/operator-component-framework/pkg/feature" "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" policyv1 "k8s.io/api/policy/v1" ) +// ErrNoActiveFeature is returned when EditObjectMetadata or EditSpec is called +// before BeginFeature has started a feature scope. +var ErrNoActiveFeature = errors.New("pdb mutator: no active feature scope; call BeginFeature first") + // Mutation defines a mutation that is applied to a pdb Mutator // only if its associated feature gate is enabled. type Mutation feature.Mutation[*Mutator] @@ -23,7 +29,9 @@ type featurePlan struct { // The Mutator maintains feature boundaries: each feature's mutations are planned // together and applied in the order the features were registered. // -// Mutator implements editors.ObjectMutator. +// Unlike other primitive mutators, EditObjectMetadata and EditSpec return an +// error when called without an active feature scope, rather than silently +// succeeding. This means Mutator does not satisfy editors.ObjectMutator. type Mutator struct { pdb *policyv1.PodDisruptionBudget @@ -53,11 +61,15 @@ func (m *Mutator) BeginFeature() { // // Metadata edits are applied before spec edits within the same feature. // A nil edit function is ignored. -func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) { +func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) error { if edit == nil { - return + return nil + } + if m.active == nil { + return ErrNoActiveFeature } m.active.metadataEdits = append(m.active.metadataEdits, edit) + return nil } // EditSpec records a mutation for the PodDisruptionBudget's spec via a @@ -68,11 +80,15 @@ func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) // after metadata edits within the same feature, in registration order. // // A nil edit function is ignored. -func (m *Mutator) EditSpec(edit func(*editors.PodDisruptionBudgetSpecEditor) error) { +func (m *Mutator) EditSpec(edit func(*editors.PodDisruptionBudgetSpecEditor) error) error { if edit == nil { - return + return nil + } + if m.active == nil { + return ErrNoActiveFeature } m.active.specEdits = append(m.active.specEdits, edit) + return nil } // Apply executes all recorded mutation intents on the underlying PodDisruptionBudget. diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go index e08431df..105db405 100644 --- a/pkg/primitives/pdb/mutator_test.go +++ b/pkg/primitives/pdb/mutator_test.go @@ -260,8 +260,20 @@ func TestMutator_SingleFeature_PlanCount(t *testing.T) { assert.Equal(t, intstr.FromInt32(1), *p.Spec.MinAvailable) } -// --- ObjectMutator interface --- +// --- ErrNoActiveFeature --- -func TestMutator_ImplementsObjectMutator(_ *testing.T) { - var _ editors.ObjectMutator = (*Mutator)(nil) +func TestMutator_EditObjectMetadata_NoActiveFeature(t *testing.T) { + m := NewMutator(newTestPDB()) + err := m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + return nil + }) + require.ErrorIs(t, err, ErrNoActiveFeature) +} + +func TestMutator_EditSpec_NoActiveFeature(t *testing.T) { + m := NewMutator(newTestPDB()) + err := m.EditSpec(func(_ *editors.PodDisruptionBudgetSpecEditor) error { + return nil + }) + require.ErrorIs(t, err, ErrNoActiveFeature) } diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index 2dea72b2..e7426c78 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -22,6 +22,11 @@ func newValidPDB() *policyv1.PodDisruptionBudget { }, Spec: policyv1.PodDisruptionBudgetSpec{ MinAvailable: ptr.To(intstr.FromInt32(2)), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "test", + }, + }, }, } } From bb8c0e2a565c0ccc6edd7eaaf6ce041dd4d56a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:27:03 +0000 Subject: [PATCH 18/20] fix: propagate errors from EditSpec and EditObjectMetadata in tests and docs Return errors from m.EditSpec() and m.EditObjectMetadata() instead of silently discarding them, so test failures and editor errors are properly surfaced. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pdb.md | 12 ++++------- pkg/primitives/pdb/resource_test.go | 31 ++++------------------------- 2 files changed, 8 insertions(+), 35 deletions(-) diff --git a/docs/primitives/pdb.md b/docs/primitives/pdb.md index ccd9a7bc..65386d8f 100644 --- a/docs/primitives/pdb.md +++ b/docs/primitives/pdb.md @@ -68,12 +68,11 @@ func StrictAvailabilityMutation(version string, enabled bool) pdb.Mutation { Name: "strict-availability", Feature: feature.NewResourceFeature(version, nil).When(enabled), Mutate: func(m *pdb.Mutator) error { - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + return m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.ClearMinAvailable() e.SetMaxUnavailable(intstr.FromInt32(1)) return nil }) - return nil }, } } @@ -92,11 +91,10 @@ func LegacyPDBMutation(version string) pdb.Mutation { []feature.VersionConstraint{legacyConstraint}, ), Mutate: func(m *pdb.Mutator) error { - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + return m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMinAvailable(intstr.FromInt32(1)) return nil }) - return nil }, } } @@ -209,11 +207,10 @@ func BasePDBMutation(version string) pdb.Mutation { Name: "base-pdb", Feature: feature.NewResourceFeature(version, nil), Mutate: func(m *pdb.Mutator) error { - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + return m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app.kubernetes.io/version", version) return nil }) - return nil }, } } @@ -223,12 +220,11 @@ func StrictAvailabilityMutation(version string, enabled bool) pdb.Mutation { Name: "strict-availability", Feature: feature.NewResourceFeature(version, nil).When(enabled), Mutate: func(m *pdb.Mutator) error { - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + return m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.ClearMinAvailable() e.SetMaxUnavailable(intstr.FromInt32(1)) return nil }) - return nil }, } } diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index e7426c78..4db49573 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -2,8 +2,6 @@ package pdb import ( "errors" - "testing" - "github.com/sourcehawk/operator-component-framework/pkg/feature" "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" "github.com/stretchr/testify/assert" @@ -12,6 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" + "testing" ) func newValidPDB() *policyv1.PodDisruptionBudget { @@ -30,44 +29,35 @@ func newValidPDB() *policyv1.PodDisruptionBudget { }, } } - func TestResource_Identity(t *testing.T) { res, err := NewBuilder(newValidPDB()).Build() require.NoError(t, err) assert.Equal(t, "policy/v1/PodDisruptionBudget/test-ns/test-pdb", res.Identity()) } - func TestResource_Object(t *testing.T) { p := newValidPDB() res, err := NewBuilder(p).Build() require.NoError(t, err) - obj, err := res.Object() require.NoError(t, err) - got, ok := obj.(*policyv1.PodDisruptionBudget) require.True(t, ok) assert.Equal(t, p.Name, got.Name) assert.Equal(t, p.Namespace, got.Namespace) - // Must be a deep copy. got.Name = "changed" assert.Equal(t, "test-pdb", p.Name) } - func TestResource_Mutate(t *testing.T) { desired := newValidPDB() res, err := NewBuilder(desired).Build() require.NoError(t, err) - obj, err := res.Object() require.NoError(t, err) require.NoError(t, res.Mutate(obj)) - got := obj.(*policyv1.PodDisruptionBudget) assert.Equal(t, intstr.FromInt32(2), *got.Spec.MinAvailable) } - func TestResource_Mutate_WithMutation(t *testing.T) { desired := newValidPDB() res, err := NewBuilder(desired). @@ -75,25 +65,21 @@ func TestResource_Mutate_WithMutation(t *testing.T) { Name: "set-max-unavailable", Feature: feature.NewResourceFeature("v1", nil).When(true), Mutate: func(m *Mutator) error { - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + return m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMaxUnavailable(intstr.FromInt32(1)) return nil }) - return nil }, }). Build() require.NoError(t, err) - obj, err := res.Object() require.NoError(t, err) require.NoError(t, res.Mutate(obj)) - got := obj.(*policyv1.PodDisruptionBudget) require.NotNil(t, got.Spec.MaxUnavailable) assert.Equal(t, intstr.FromInt32(1), *got.Spec.MaxUnavailable) } - func TestResource_Mutate_FeatureOrdering(t *testing.T) { desired := newValidPDB() res, err := NewBuilder(desired). @@ -101,38 +87,32 @@ func TestResource_Mutate_FeatureOrdering(t *testing.T) { Name: "feature-a", Feature: feature.NewResourceFeature("v1", nil).When(true), Mutate: func(m *Mutator) error { - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + return m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("order", "a") return nil }) - return nil }, }). WithMutation(Mutation{ Name: "feature-b", Feature: feature.NewResourceFeature("v1", nil).When(true), Mutate: func(m *Mutator) error { - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + return m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("order", "b") return nil }) - return nil }, }). Build() require.NoError(t, err) - obj, err := res.Object() require.NoError(t, err) require.NoError(t, res.Mutate(obj)) - got := obj.(*policyv1.PodDisruptionBudget) assert.Equal(t, "b", got.Labels["order"]) } - func TestResource_ExtractData(t *testing.T) { p := newValidPDB() - var extracted int32 res, err := NewBuilder(p). WithDataExtractor(func(pdb policyv1.PodDisruptionBudget) error { @@ -141,11 +121,9 @@ func TestResource_ExtractData(t *testing.T) { }). Build() require.NoError(t, err) - require.NoError(t, res.ExtractData()) assert.Equal(t, int32(2), extracted) } - func TestResource_ExtractData_Error(t *testing.T) { res, err := NewBuilder(newValidPDB()). WithDataExtractor(func(_ policyv1.PodDisruptionBudget) error { @@ -153,7 +131,6 @@ func TestResource_ExtractData_Error(t *testing.T) { }). Build() require.NoError(t, err) - err = res.ExtractData() require.Error(t, err) assert.Contains(t, err.Error(), "extract error") From c5e51497f57aa6b4d71cc23ef064c8e5afa330a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:52:18 +0000 Subject: [PATCH 19/20] fix: check error returns and fix import grouping in PDB tests Satisfy errcheck linter by wrapping EditSpec/EditObjectMetadata calls with require.NoError, and fix goimports grouping in resource_test.go. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator_test.go | 64 ++++++++++++++--------------- pkg/primitives/pdb/resource_test.go | 3 +- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/pkg/primitives/pdb/mutator_test.go b/pkg/primitives/pdb/mutator_test.go index 105db405..b353e48e 100644 --- a/pkg/primitives/pdb/mutator_test.go +++ b/pkg/primitives/pdb/mutator_test.go @@ -26,10 +26,10 @@ func TestMutator_EditObjectMetadata(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + require.NoError(t, m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil - }) + })) require.NoError(t, m.Apply()) assert.Equal(t, "myapp", p.Labels["app"]) } @@ -38,7 +38,7 @@ func TestMutator_EditObjectMetadata_Nil(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditObjectMetadata(nil) + require.NoError(t, m.EditObjectMetadata(nil)) assert.NoError(t, m.Apply()) } @@ -48,10 +48,10 @@ func TestMutator_EditSpec_SetMinAvailable(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMinAvailable(intstr.FromInt32(2)) return nil - }) + })) require.NoError(t, m.Apply()) require.NotNil(t, p.Spec.MinAvailable) assert.Equal(t, intstr.FromInt32(2), *p.Spec.MinAvailable) @@ -61,10 +61,10 @@ func TestMutator_EditSpec_SetMaxUnavailable(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMaxUnavailable(intstr.FromString("25%")) return nil - }) + })) require.NoError(t, m.Apply()) require.NotNil(t, p.Spec.MaxUnavailable) assert.Equal(t, intstr.FromString("25%"), *p.Spec.MaxUnavailable) @@ -77,10 +77,10 @@ func TestMutator_EditSpec_SetSelector(t *testing.T) { selector := &metav1.LabelSelector{ MatchLabels: map[string]string{"app": "web"}, } - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetSelector(selector) return nil - }) + })) require.NoError(t, m.Apply()) require.NotNil(t, p.Spec.Selector) assert.Equal(t, selector, p.Spec.Selector) @@ -90,10 +90,10 @@ func TestMutator_EditSpec_SetUnhealthyPodEvictionPolicy(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetUnhealthyPodEvictionPolicy(policyv1.AlwaysAllow) return nil - }) + })) require.NoError(t, m.Apply()) require.NotNil(t, p.Spec.UnhealthyPodEvictionPolicy) assert.Equal(t, policyv1.AlwaysAllow, *p.Spec.UnhealthyPodEvictionPolicy) @@ -103,11 +103,11 @@ func TestMutator_EditSpec_RawAccess(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { val := intstr.FromInt32(3) e.Raw().MinAvailable = &val return nil - }) + })) require.NoError(t, m.Apply()) require.NotNil(t, p.Spec.MinAvailable) assert.Equal(t, intstr.FromInt32(3), *p.Spec.MinAvailable) @@ -117,7 +117,7 @@ func TestMutator_EditSpec_Nil(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(nil) + require.NoError(t, m.EditSpec(nil)) assert.NoError(t, m.Apply()) } @@ -130,7 +130,7 @@ func TestMutator_OperationOrder(t *testing.T) { m := NewMutator(p) m.BeginFeature() // Register in reverse logical order to confirm Apply() enforces category ordering. - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { // Branch on the label set by the metadata edit: if metadata ran first // the label exists and we set MinAvailable to 1; otherwise we set it to 99. if p.Labels["order"] == "metadata-first" { @@ -139,11 +139,11 @@ func TestMutator_OperationOrder(t *testing.T) { e.SetMinAvailable(intstr.FromInt32(99)) } return nil - }) - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + })) + require.NoError(t, m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("order", "metadata-first") return nil - }) + })) require.NoError(t, m.Apply()) assert.Equal(t, "metadata-first", p.Labels["order"]) @@ -157,15 +157,15 @@ func TestMutator_MultipleFeatures(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + require.NoError(t, m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("feature1", "on") return nil - }) + })) m.BeginFeature() - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + require.NoError(t, m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("feature2", "on") return nil - }) + })) require.NoError(t, m.Apply()) assert.Equal(t, "on", p.Labels["feature1"]) @@ -176,9 +176,9 @@ func TestMutator_EditSpec_ErrorPropagated(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(_ *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(_ *editors.PodDisruptionBudgetSpecEditor) error { return assert.AnError - }) + })) err := m.Apply() require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) @@ -188,9 +188,9 @@ func TestMutator_EditObjectMetadata_ErrorPropagated(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + require.NoError(t, m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { return assert.AnError - }) + })) err := m.Apply() require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) @@ -225,17 +225,17 @@ func TestBeginFeature_IsolatesFeaturePlans(t *testing.T) { // Record a mutation in the first feature plan m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMinAvailable(intstr.FromInt32(1)) return nil - }) + })) // Start a new feature and record a different mutation m.BeginFeature() - m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + require.NoError(t, m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("f1", "on") return nil - }) + })) // The first plan should have exactly one spec edit and no metadata edits assert.Len(t, m.plans[0].specEdits, 1, "first plan should have one spec edit") @@ -249,10 +249,10 @@ func TestMutator_SingleFeature_PlanCount(t *testing.T) { p := newTestPDB() m := NewMutator(p) m.BeginFeature() - m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { + require.NoError(t, m.EditSpec(func(e *editors.PodDisruptionBudgetSpecEditor) error { e.SetMinAvailable(intstr.FromInt32(1)) return nil - }) + })) require.NoError(t, m.Apply()) assert.Len(t, m.plans, 1, "no extra plans should be created during Apply") diff --git a/pkg/primitives/pdb/resource_test.go b/pkg/primitives/pdb/resource_test.go index 4db49573..f39122ce 100644 --- a/pkg/primitives/pdb/resource_test.go +++ b/pkg/primitives/pdb/resource_test.go @@ -2,6 +2,8 @@ package pdb import ( "errors" + "testing" + "github.com/sourcehawk/operator-component-framework/pkg/feature" "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" "github.com/stretchr/testify/assert" @@ -10,7 +12,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" - "testing" ) func newValidPDB() *policyv1.PodDisruptionBudget { From a0121925cd53aff18d7e2926b30268f12db4075b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 18:39:16 +0000 Subject: [PATCH 20/20] docs: clarify nil-edit behavior in PDB mutator type comment The comment on the Mutator type incorrectly implied that EditObjectMetadata and EditSpec always error without an active feature scope. In reality, nil edit functions are silently ignored regardless of scope state. Updated the comment to reflect this nuance. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pdb/mutator.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/pdb/mutator.go b/pkg/primitives/pdb/mutator.go index a725e636..cb4721ac 100644 --- a/pkg/primitives/pdb/mutator.go +++ b/pkg/primitives/pdb/mutator.go @@ -30,8 +30,9 @@ type featurePlan struct { // together and applied in the order the features were registered. // // Unlike other primitive mutators, EditObjectMetadata and EditSpec return an -// error when called without an active feature scope, rather than silently -// succeeding. This means Mutator does not satisfy editors.ObjectMutator. +// error when a non-nil edit function is provided without an active feature +// scope (i.e. before BeginFeature is called). Nil edit functions are always +// ignored and return nil. This means Mutator does not satisfy editors.ObjectMutator. type Mutator struct { pdb *policyv1.PodDisruptionBudget