From e3486d61d94192f3161f7491495cf486b6ce94a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:38:05 +0000 Subject: [PATCH 01/22] Add PVSpecEditor for PersistentVolume spec mutations Introduces a typed editor for corev1.PersistentVolumeSpec with methods for capacity, access modes, reclaim policy, storage class, mount options, volume mode, and node affinity. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mutation/editors/pvspec.go | 64 +++++++++++++++++ pkg/mutation/editors/pvspec_test.go | 105 ++++++++++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 pkg/mutation/editors/pvspec.go create mode 100644 pkg/mutation/editors/pvspec_test.go diff --git a/pkg/mutation/editors/pvspec.go b/pkg/mutation/editors/pvspec.go new file mode 100644 index 00000000..e0e76890 --- /dev/null +++ b/pkg/mutation/editors/pvspec.go @@ -0,0 +1,64 @@ +package editors + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +// PVSpecEditor provides a typed API for mutating a Kubernetes PersistentVolumeSpec. +// +// It exposes structured operations for the most common PV spec fields. Use Raw() +// for free-form access when none of the structured methods are sufficient. +type PVSpecEditor struct { + spec *corev1.PersistentVolumeSpec +} + +// NewPVSpecEditor creates a new PVSpecEditor for the given PersistentVolumeSpec. +func NewPVSpecEditor(spec *corev1.PersistentVolumeSpec) *PVSpecEditor { + return &PVSpecEditor{spec: spec} +} + +// Raw returns the underlying *corev1.PersistentVolumeSpec. +// +// This is an escape hatch for cases where the typed API is insufficient. +func (e *PVSpecEditor) Raw() *corev1.PersistentVolumeSpec { + return e.spec +} + +// SetCapacity sets the storage capacity of the PersistentVolume. +func (e *PVSpecEditor) SetCapacity(storage resource.Quantity) { + if e.spec.Capacity == nil { + e.spec.Capacity = make(corev1.ResourceList) + } + e.spec.Capacity[corev1.ResourceStorage] = storage +} + +// SetAccessModes replaces the access modes of the PersistentVolume. +func (e *PVSpecEditor) SetAccessModes(modes []corev1.PersistentVolumeAccessMode) { + e.spec.AccessModes = modes +} + +// SetPersistentVolumeReclaimPolicy sets the reclaim policy for the PersistentVolume. +func (e *PVSpecEditor) SetPersistentVolumeReclaimPolicy(policy corev1.PersistentVolumeReclaimPolicy) { + e.spec.PersistentVolumeReclaimPolicy = policy +} + +// SetStorageClassName sets the storage class name for the PersistentVolume. +func (e *PVSpecEditor) SetStorageClassName(name string) { + e.spec.StorageClassName = name +} + +// SetMountOptions replaces the mount options for the PersistentVolume. +func (e *PVSpecEditor) SetMountOptions(options []string) { + e.spec.MountOptions = options +} + +// SetVolumeMode sets the volume mode for the PersistentVolume. +func (e *PVSpecEditor) SetVolumeMode(mode corev1.PersistentVolumeMode) { + e.spec.VolumeMode = &mode +} + +// SetNodeAffinity sets the node affinity for the PersistentVolume. +func (e *PVSpecEditor) SetNodeAffinity(affinity *corev1.VolumeNodeAffinity) { + e.spec.NodeAffinity = affinity +} diff --git a/pkg/mutation/editors/pvspec_test.go b/pkg/mutation/editors/pvspec_test.go new file mode 100644 index 00000000..fb6b51c7 --- /dev/null +++ b/pkg/mutation/editors/pvspec_test.go @@ -0,0 +1,105 @@ +package editors + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestPVSpecEditor_Raw(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + assert.Same(t, spec, editor.Raw()) +} + +func TestPVSpecEditor_SetCapacity(t *testing.T) { + t.Run("sets storage on nil capacity", func(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + qty := resource.MustParse("10Gi") + editor.SetCapacity(qty) + + require.NotNil(t, spec.Capacity) + assert.True(t, spec.Capacity[corev1.ResourceStorage].Equal(qty)) + }) + + t.Run("overwrites existing capacity", func(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("5Gi"), + }, + } + editor := NewPVSpecEditor(spec) + qty := resource.MustParse("20Gi") + editor.SetCapacity(qty) + + assert.True(t, spec.Capacity[corev1.ResourceStorage].Equal(qty)) + }) +} + +func TestPVSpecEditor_SetAccessModes(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + modes := []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + corev1.ReadOnlyMany, + } + editor.SetAccessModes(modes) + assert.Equal(t, modes, spec.AccessModes) +} + +func TestPVSpecEditor_SetPersistentVolumeReclaimPolicy(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + editor.SetPersistentVolumeReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + assert.Equal(t, corev1.PersistentVolumeReclaimRetain, spec.PersistentVolumeReclaimPolicy) +} + +func TestPVSpecEditor_SetStorageClassName(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + editor.SetStorageClassName("fast-ssd") + assert.Equal(t, "fast-ssd", spec.StorageClassName) +} + +func TestPVSpecEditor_SetMountOptions(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + opts := []string{"hard", "nfsvers=4.1"} + editor.SetMountOptions(opts) + assert.Equal(t, opts, spec.MountOptions) +} + +func TestPVSpecEditor_SetVolumeMode(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + mode := corev1.PersistentVolumeBlock + editor.SetVolumeMode(mode) + require.NotNil(t, spec.VolumeMode) + assert.Equal(t, mode, *spec.VolumeMode) +} + +func TestPVSpecEditor_SetNodeAffinity(t *testing.T) { + spec := &corev1.PersistentVolumeSpec{} + editor := NewPVSpecEditor(spec) + affinity := &corev1.VolumeNodeAffinity{ + Required: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "topology.kubernetes.io/zone", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"us-east-1a"}, + }, + }, + }, + }, + }, + } + editor.SetNodeAffinity(affinity) + assert.Equal(t, affinity, spec.NodeAffinity) +} From b46717d9a4cc1927a44e2d44fb23b2e0d1678c73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:41:43 +0000 Subject: [PATCH 02/22] Add PersistentVolume primitive with Integration lifecycle Implements the PV primitive as a cluster-scoped Integration resource: - Builder validates Name only (no namespace for cluster-scoped resources) - DefaultFieldApplicator preserves immutable fields on existing PVs (volume source, volume mode, claim ref) - DefaultOperationalStatusHandler reports status based on PV phase - Mutator with plan-and-apply pattern for metadata and spec edits - Flavors for preserving external labels and annotations Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/builder.go | 160 ++++++++++++++++++++++++++ pkg/primitives/pv/builder_test.go | 173 +++++++++++++++++++++++++++++ pkg/primitives/pv/flavors.go | 25 +++++ pkg/primitives/pv/flavors_test.go | 128 +++++++++++++++++++++ pkg/primitives/pv/handlers.go | 60 ++++++++++ pkg/primitives/pv/handlers_test.go | 76 +++++++++++++ pkg/primitives/pv/mutator.go | 138 +++++++++++++++++++++++ pkg/primitives/pv/mutator_test.go | 157 ++++++++++++++++++++++++++ pkg/primitives/pv/resource.go | 102 +++++++++++++++++ 9 files changed, 1019 insertions(+) create mode 100644 pkg/primitives/pv/builder.go create mode 100644 pkg/primitives/pv/builder_test.go create mode 100644 pkg/primitives/pv/flavors.go create mode 100644 pkg/primitives/pv/flavors_test.go create mode 100644 pkg/primitives/pv/handlers.go create mode 100644 pkg/primitives/pv/handlers_test.go create mode 100644 pkg/primitives/pv/mutator.go create mode 100644 pkg/primitives/pv/mutator_test.go create mode 100644 pkg/primitives/pv/resource.go diff --git a/pkg/primitives/pv/builder.go b/pkg/primitives/pv/builder.go new file mode 100644 index 00000000..acf377a9 --- /dev/null +++ b/pkg/primitives/pv/builder.go @@ -0,0 +1,160 @@ +// Package pv provides a builder and resource for managing Kubernetes PersistentVolumes. +package pv + +import ( + "errors" + "fmt" + + "github.com/sourcehawk/operator-component-framework/internal/generic" + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + corev1 "k8s.io/api/core/v1" +) + +// Builder is a configuration helper for creating and customizing a PersistentVolume Resource. +// +// It provides a fluent API for registering mutations, field application flavors, +// operational status handlers, and data extractors. Build() validates the +// configuration and returns an initialized Resource ready for use in a +// reconciliation loop. +type Builder struct { + base *generic.IntegrationBuilder[*corev1.PersistentVolume, *Mutator] + operationalStatusHandler func(concepts.ConvergingOperation, *corev1.PersistentVolume) (concepts.OperationalStatusWithReason, error) +} + +// NewBuilder initializes a new Builder with the provided PersistentVolume object. +// +// The PersistentVolume 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. +// +// PersistentVolumes are cluster-scoped; the provided object must have a Name set +// but must not have a Namespace. This is validated during the Build() call. +func NewBuilder(pv *corev1.PersistentVolume) *Builder { + identityFunc := func(p *corev1.PersistentVolume) string { + return fmt.Sprintf("v1/PersistentVolume/%s", p.Name) + } + + base := generic.NewIntegrationBuilder[*corev1.PersistentVolume, *Mutator]( + pv, + identityFunc, + DefaultFieldApplicator, + NewMutator, + ) + + b := &Builder{ + base: base, + operationalStatusHandler: DefaultOperationalStatusHandler, + } + + return b +} + +// WithMutation registers a mutation for the PersistentVolume. +// +// 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 PersistentVolume in the cluster. +// +// The default applicator (DefaultFieldApplicator) preserves immutable fields +// (volume source, volume mode, claim ref) on existing PVs while replacing all +// other fields from the desired state. Use a custom applicator when you need +// different preservation semantics. +// +// 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 *corev1.PersistentVolume) 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[*corev1.PersistentVolume](flavor)) + return b +} + +// WithCustomOperationalStatus overrides the default logic for determining if the +// PersistentVolume is operationally ready. +// +// The default behavior uses DefaultOperationalStatusHandler, which considers a PV +// operational when its phase is Available or Bound. Use this method if your PV +// requires more complex readiness checks. +func (b *Builder) WithCustomOperationalStatus( + handler func(concepts.ConvergingOperation, *corev1.PersistentVolume) (concepts.OperationalStatusWithReason, error), +) *Builder { + b.operationalStatusHandler = handler + return b +} + +// WithDataExtractor registers a function to read values from the PersistentVolume +// after it has been successfully reconciled. +// +// The extractor receives a value copy of the reconciled PersistentVolume. This is +// useful for surfacing generated or updated fields to other components or resources. +// +// A nil extractor is ignored. +func (b *Builder) WithDataExtractor(extractor func(corev1.PersistentVolume) error) *Builder { + if extractor != nil { + b.base.WithDataExtractor(func(pv *corev1.PersistentVolume) error { + return extractor(*pv) + }) + } + return b +} + +// Build validates the configuration and returns the initialized Resource. +// +// It returns an error if: +// - No PersistentVolume object was provided. +// - The PersistentVolume is missing a Name. +// - The PersistentVolume has a Namespace set (PVs are cluster-scoped). +func (b *Builder) Build() (*Resource, error) { + if err := b.validate(); err != nil { + return nil, err + } + + res := &generic.IntegrationResource[*corev1.PersistentVolume, *Mutator]{ + BaseResource: *b.base.BaseRes, + OperationalStatusHandler: b.operationalStatusHandler, + } + + return &Resource{base: res}, nil +} + +// validate checks PV-specific builder constraints. +// +// PersistentVolumes are cluster-scoped: Name is required, Namespace must be empty. +func (b *Builder) validate() error { + obj := b.base.BaseRes.DesiredObject + if obj == nil { + return errors.New("object cannot be nil") + } + + if obj.GetName() == "" { + return errors.New("object name cannot be empty") + } + + if obj.GetNamespace() != "" { + return errors.New("PersistentVolume is cluster-scoped: namespace must not be set") + } + + return nil +} diff --git a/pkg/primitives/pv/builder_test.go b/pkg/primitives/pv/builder_test.go new file mode 100644 index 00000000..0c7c4263 --- /dev/null +++ b/pkg/primitives/pv/builder_test.go @@ -0,0 +1,173 @@ +package pv + +import ( + "errors" + "testing" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestBuilder_Build_Validation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pv *corev1.PersistentVolume + expectedErr string + }{ + { + name: "nil persistent volume", + pv: nil, + expectedErr: "object cannot be nil", + }, + { + name: "empty name", + pv: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{}, + }, + expectedErr: "object name cannot be empty", + }, + { + name: "namespace set on cluster-scoped resource", + pv: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv", Namespace: "should-not-be-set"}, + }, + expectedErr: "PersistentVolume is cluster-scoped: namespace must not be set", + }, + { + name: "valid persistent volume", + pv: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := NewBuilder(tt.pv).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, "v1/PersistentVolume/test-pv", res.Identity()) + } + }) + } +} + +func TestBuilder_WithMutation(t *testing.T) { + t.Parallel() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + res, err := NewBuilder(pv). + 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() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + called := false + applicator := func(_, _ *corev1.PersistentVolume) error { + called = true + return nil + } + res, err := NewBuilder(pv). + 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() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + res, err := NewBuilder(pv). + WithFieldApplicationFlavor(PreserveCurrentLabels). + WithFieldApplicationFlavor(nil). // nil must be ignored + Build() + require.NoError(t, err) + assert.Len(t, res.base.FieldFlavors, 1) +} + +func TestBuilder_WithCustomOperationalStatus(t *testing.T) { + t.Parallel() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + handler := func(_ concepts.ConvergingOperation, _ *corev1.PersistentVolume) (concepts.OperationalStatusWithReason, error) { + return concepts.OperationalStatusWithReason{Status: concepts.OperationalStatusOperational}, nil + } + res, err := NewBuilder(pv). + WithCustomOperationalStatus(handler). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.OperationalStatusHandler) + status, err := res.base.OperationalStatusHandler(concepts.ConvergingOperationNone, nil) + require.NoError(t, err) + assert.Equal(t, concepts.OperationalStatusOperational, status.Status) +} + +func TestBuilder_WithDataExtractor(t *testing.T) { + t.Parallel() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + called := false + extractor := func(_ corev1.PersistentVolume) error { + called = true + return nil + } + res, err := NewBuilder(pv). + WithDataExtractor(extractor). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 1) + require.NoError(t, res.base.DataExtractors[0](&corev1.PersistentVolume{})) + assert.True(t, called) +} + +func TestBuilder_WithDataExtractor_Nil(t *testing.T) { + t.Parallel() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + res, err := NewBuilder(pv). + WithDataExtractor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 0) +} + +func TestBuilder_WithDataExtractor_ErrorPropagated(t *testing.T) { + t.Parallel() + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, + } + res, err := NewBuilder(pv). + WithDataExtractor(func(_ corev1.PersistentVolume) error { + return errors.New("extractor error") + }). + Build() + require.NoError(t, err) + err = res.base.DataExtractors[0](&corev1.PersistentVolume{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "extractor error") +} diff --git a/pkg/primitives/pv/flavors.go b/pkg/primitives/pv/flavors.go new file mode 100644 index 00000000..10238507 --- /dev/null +++ b/pkg/primitives/pv/flavors.go @@ -0,0 +1,25 @@ +package pv + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/flavors" + corev1 "k8s.io/api/core/v1" +) + +// FieldApplicationFlavor defines a function signature for applying flavors to a +// PersistentVolume 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[*corev1.PersistentVolume] + +// PreserveCurrentLabels ensures that any labels present on the current live +// PersistentVolume but missing from the applied (desired) object are preserved. +// If a label exists in both, the applied value wins. +func PreserveCurrentLabels(applied, current, desired *corev1.PersistentVolume) error { + return flavors.PreserveCurrentLabels[*corev1.PersistentVolume]()(applied, current, desired) +} + +// PreserveCurrentAnnotations ensures that any annotations present on the current +// live PersistentVolume but missing from the applied (desired) object are preserved. +// If an annotation exists in both, the applied value wins. +func PreserveCurrentAnnotations(applied, current, desired *corev1.PersistentVolume) error { + return flavors.PreserveCurrentAnnotations[*corev1.PersistentVolume]()(applied, current, desired) +} diff --git a/pkg/primitives/pv/flavors_test.go b/pkg/primitives/pv/flavors_test.go new file mode 100644 index 00000000..f2c11706 --- /dev/null +++ b/pkg/primitives/pv/flavors_test.go @@ -0,0 +1,128 @@ +package pv + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPreserveCurrentLabels(t *testing.T) { + t.Run("adds missing labels from current", func(t *testing.T) { + applied := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} + current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} + current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{} + current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} + current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} + current := &corev1.PersistentVolume{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) { + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + Labels: map[string]string{"app": "desired"}, + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{Path: "/data"}, + }, + }, + } + + t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + require.NoError(t, err) + + current := &corev1.PersistentVolume{ + 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(_, _, _ *corev1.PersistentVolume) error { + order = append(order, "flavor1") + return nil + } + flavor2 := func(_, _, _ *corev1.PersistentVolume) 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(&corev1.PersistentVolume{})) + 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(_, _, _ *corev1.PersistentVolume) error { + return flavorErr + }). + Build() + require.NoError(t, err) + + err = res.Mutate(&corev1.PersistentVolume{}) + require.Error(t, err) + assert.True(t, errors.Is(err, flavorErr)) + }) +} diff --git a/pkg/primitives/pv/handlers.go b/pkg/primitives/pv/handlers.go new file mode 100644 index 00000000..f08b7269 --- /dev/null +++ b/pkg/primitives/pv/handlers.go @@ -0,0 +1,60 @@ +package pv + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + corev1 "k8s.io/api/core/v1" +) + +// DefaultOperationalStatusHandler is the default logic for determining if a PersistentVolume +// is operationally ready. +// +// It considers a PV operational when its Status.Phase is Available or Bound. +// A PV in the Pending phase is reported as OperationPending. A PV in the +// Released or Failed phase is reported as OperationFailing. +// +// This function is used as the default handler by the Resource if no custom handler +// is registered via Builder.WithCustomOperationalStatus. It can be reused within +// custom handlers to augment the default behavior. +func DefaultOperationalStatusHandler( + _ concepts.ConvergingOperation, pv *corev1.PersistentVolume, +) (concepts.OperationalStatusWithReason, error) { + switch pv.Status.Phase { + case corev1.VolumeAvailable: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusOperational, + Reason: "PersistentVolume is available", + }, nil + + case corev1.VolumeBound: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusOperational, + Reason: "PersistentVolume is bound to a claim", + }, nil + + case corev1.VolumePending: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusPending, + Reason: "PersistentVolume is pending", + }, nil + + case corev1.VolumeReleased: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusFailing, + Reason: "PersistentVolume has been released and is not yet reclaimed", + }, nil + + case corev1.VolumeFailed: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusFailing, + Reason: "PersistentVolume reclamation has failed", + }, nil + + default: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusPending, + Reason: fmt.Sprintf("PersistentVolume phase is %q", pv.Status.Phase), + }, nil + } +} diff --git a/pkg/primitives/pv/handlers_test.go b/pkg/primitives/pv/handlers_test.go new file mode 100644 index 00000000..bd5252a3 --- /dev/null +++ b/pkg/primitives/pv/handlers_test.go @@ -0,0 +1,76 @@ +package pv + +import ( + "testing" + + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" +) + +func TestDefaultOperationalStatusHandler(t *testing.T) { + tests := []struct { + name string + phase corev1.PersistentVolumePhase + wantStatus concepts.OperationalStatus + wantReason string + }{ + { + name: "available", + phase: corev1.VolumeAvailable, + wantStatus: concepts.OperationalStatusOperational, + wantReason: "PersistentVolume is available", + }, + { + name: "bound", + phase: corev1.VolumeBound, + wantStatus: concepts.OperationalStatusOperational, + wantReason: "PersistentVolume is bound to a claim", + }, + { + name: "pending", + phase: corev1.VolumePending, + wantStatus: concepts.OperationalStatusPending, + wantReason: "PersistentVolume is pending", + }, + { + name: "released", + phase: corev1.VolumeReleased, + wantStatus: concepts.OperationalStatusFailing, + wantReason: "PersistentVolume has been released and is not yet reclaimed", + }, + { + name: "failed", + phase: corev1.VolumeFailed, + wantStatus: concepts.OperationalStatusFailing, + wantReason: "PersistentVolume reclamation has failed", + }, + { + name: "unknown phase", + phase: corev1.PersistentVolumePhase("Unknown"), + wantStatus: concepts.OperationalStatusPending, + wantReason: `PersistentVolume phase is "Unknown"`, + }, + { + name: "empty phase", + phase: "", + wantStatus: concepts.OperationalStatusPending, + wantReason: `PersistentVolume phase is ""`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pv := &corev1.PersistentVolume{ + Status: corev1.PersistentVolumeStatus{ + Phase: tt.phase, + }, + } + got, err := DefaultOperationalStatusHandler(concepts.ConvergingOperationNone, pv) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, got.Status) + assert.Equal(t, tt.wantReason, got.Reason) + }) + } +} diff --git a/pkg/primitives/pv/mutator.go b/pkg/primitives/pv/mutator.go new file mode 100644 index 00000000..bbdac303 --- /dev/null +++ b/pkg/primitives/pv/mutator.go @@ -0,0 +1,138 @@ +package pv + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + corev1 "k8s.io/api/core/v1" +) + +// Mutation defines a mutation that is applied to a pv 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.PVSpecEditor) error +} + +// Mutator is a high-level helper for modifying a Kubernetes PersistentVolume. +// +// It uses a "plan-and-apply" pattern: mutations are recorded first, then +// applied to the PersistentVolume 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 { + pv *corev1.PersistentVolume + + plans []featurePlan + active *featurePlan +} + +// NewMutator creates a new Mutator for the given PersistentVolume. +func NewMutator(pv *corev1.PersistentVolume) *Mutator { + m := &Mutator{pv: pv} + 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 PersistentVolume'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) +} + +// EditPVSpec records a mutation for the PersistentVolume's spec via a PVSpecEditor. +// +// The editor provides structured operations (SetCapacity, SetAccessModes, +// SetPersistentVolumeReclaimPolicy, 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) EditPVSpec(edit func(*editors.PVSpecEditor) error) { + if edit == nil { + return + } + m.active.specEdits = append(m.active.specEdits, edit) +} + +// SetStorageClassName records that the PV's storage class should be set to the given name. +// +// Convenience wrapper over EditPVSpec. +func (m *Mutator) SetStorageClassName(name string) { + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.SetStorageClassName(name) + return nil + }) +} + +// SetReclaimPolicy records that the PV's reclaim policy should be set to the given value. +// +// Convenience wrapper over EditPVSpec. +func (m *Mutator) SetReclaimPolicy(policy corev1.PersistentVolumeReclaimPolicy) { + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.SetPersistentVolumeReclaimPolicy(policy) + return nil + }) +} + +// SetMountOptions records that the PV's mount options should be set to the given values. +// +// Convenience wrapper over EditPVSpec. +func (m *Mutator) SetMountOptions(options []string) { + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.SetMountOptions(options) + return nil + }) +} + +// Apply executes all recorded mutation intents on the underlying PersistentVolume. +// +// Execution order across all registered features: +// +// 1. Metadata edits (in registration order within each feature) +// 2. Spec edits — EditPVSpec, SetStorageClassName, SetReclaimPolicy, SetMountOptions +// (in registration order within each feature) +// +// Features are applied in the order they were registered. Later features observe +// the PersistentVolume 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.pv.ObjectMeta) + for _, edit := range plan.metadataEdits { + if err := edit(editor); err != nil { + return err + } + } + } + + // 2. Spec edits + if len(plan.specEdits) > 0 { + editor := editors.NewPVSpecEditor(&m.pv.Spec) + for _, edit := range plan.specEdits { + if err := edit(editor); err != nil { + return err + } + } + } + } + + return nil +} diff --git a/pkg/primitives/pv/mutator_test.go b/pkg/primitives/pv/mutator_test.go new file mode 100644 index 00000000..4cec65e7 --- /dev/null +++ b/pkg/primitives/pv/mutator_test.go @@ -0,0 +1,157 @@ +package pv + +import ( + "testing" + + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func newTestPV() *corev1.PersistentVolume { + return &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + }, + } +} + +// --- EditObjectMetadata --- + +func TestMutator_EditObjectMetadata(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", pv.Labels["app"]) +} + +func TestMutator_EditObjectMetadata_Nil(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.EditObjectMetadata(nil) + assert.NoError(t, m.Apply()) +} + +// --- EditPVSpec --- + +func TestMutator_EditPVSpec(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.SetStorageClassName("fast-ssd") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "fast-ssd", pv.Spec.StorageClassName) +} + +func TestMutator_EditPVSpec_Nil(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.EditPVSpec(nil) + assert.NoError(t, m.Apply()) +} + +func TestMutator_EditPVSpec_RawAccess(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.Raw().PersistentVolumeReclaimPolicy = corev1.PersistentVolumeReclaimRetain + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, corev1.PersistentVolumeReclaimRetain, pv.Spec.PersistentVolumeReclaimPolicy) +} + +// --- Convenience methods --- + +func TestMutator_SetStorageClassName(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.SetStorageClassName("premium") + require.NoError(t, m.Apply()) + assert.Equal(t, "premium", pv.Spec.StorageClassName) +} + +func TestMutator_SetReclaimPolicy(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimDelete) + require.NoError(t, m.Apply()) + assert.Equal(t, corev1.PersistentVolumeReclaimDelete, pv.Spec.PersistentVolumeReclaimPolicy) +} + +func TestMutator_SetMountOptions(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + opts := []string{"hard", "nfsvers=4.1"} + m.SetMountOptions(opts) + require.NoError(t, m.Apply()) + assert.Equal(t, opts, pv.Spec.MountOptions) +} + +// --- Execution order --- + +func TestMutator_OperationOrder(t *testing.T) { + // Within a feature: metadata edits run before spec edits. + pv := newTestPV() + m := NewMutator(pv) + // Register in reverse logical order to confirm Apply() enforces category ordering. + m.SetStorageClassName("standard") + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "tested") + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "tested", pv.Labels["order"]) + assert.Equal(t, "standard", pv.Spec.StorageClassName) +} + +func TestMutator_MultipleFeatures(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.SetStorageClassName("feature1-class") + m.beginFeature() + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + require.NoError(t, m.Apply()) + + assert.Equal(t, "feature1-class", pv.Spec.StorageClassName) + assert.Equal(t, corev1.PersistentVolumeReclaimRetain, pv.Spec.PersistentVolumeReclaimPolicy) +} + +func TestMutator_LaterFeatureObservesPrior(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.SetStorageClassName("first") + m.beginFeature() + // Second feature should see the storage class set by the first. + m.EditPVSpec(func(e *editors.PVSpecEditor) error { + if e.Raw().StorageClassName == "first" { + e.SetStorageClassName("first-seen") + } + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "first-seen", pv.Spec.StorageClassName) +} + +// --- ObjectMutator interface --- + +func TestMutator_ImplementsObjectMutator(_ *testing.T) { + var _ editors.ObjectMutator = (*Mutator)(nil) +} diff --git a/pkg/primitives/pv/resource.go b/pkg/primitives/pv/resource.go new file mode 100644 index 00000000..a7b2b8db --- /dev/null +++ b/pkg/primitives/pv/resource.go @@ -0,0 +1,102 @@ +package pv + +import ( + "github.com/sourcehawk/operator-component-framework/internal/generic" + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// DefaultFieldApplicator preserves immutable fields from the existing PersistentVolume +// while applying the desired state. +// +// PersistentVolumes have immutable fields that cannot be changed after creation +// (e.g. the volume source). This applicator deep-copies the desired state onto +// the current object but preserves the existing Spec.PersistentVolumeSource, +// Spec.VolumeMode, and Spec.ClaimRef so that updates do not attempt to change +// server-enforced immutable fields. +// +// On a fresh PV (empty ResourceVersion, meaning it has not yet been persisted), +// the full desired state is applied without preservation. +func DefaultFieldApplicator(current, desired *corev1.PersistentVolume) error { + if current.ResourceVersion == "" { + // First creation — apply everything from desired. + *current = *desired.DeepCopy() + return nil + } + + // Snapshot immutable fields before overwriting. + savedSource := current.Spec.PersistentVolumeSource + savedVolumeMode := current.Spec.VolumeMode + savedClaimRef := current.Spec.ClaimRef + + *current = *desired.DeepCopy() + + // Restore immutable fields from the live object. + current.Spec.PersistentVolumeSource = savedSource + current.Spec.VolumeMode = savedVolumeMode + current.Spec.ClaimRef = savedClaimRef + + return nil +} + +// Resource is a high-level abstraction for managing a Kubernetes PersistentVolume +// within a controller's reconciliation loop. +// +// It implements the following component interfaces: +// - component.Resource: for basic identity and mutation behaviour. +// - component.Operational: for tracking whether the PV is operationally ready. +// - component.DataExtractable: for exporting values after successful reconciliation. +// +// PersistentVolume resources use the Integration lifecycle: they report an +// OperationalStatus rather than an Alive/Grace status, and do not support suspension. +type Resource struct { + base *generic.IntegrationResource[*corev1.PersistentVolume, *Mutator] +} + +// Identity returns a unique identifier for the PersistentVolume in the format +// "v1/PersistentVolume/". +// +// PersistentVolumes are cluster-scoped and do not include a namespace in their identity. +func (r *Resource) Identity() string { + return r.base.Identity() +} + +// Object returns a deep copy of the underlying Kubernetes PersistentVolume 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 PersistentVolume 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. +// The default applicator preserves immutable fields on existing PVs. +// 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) +} + +// ConvergingStatus evaluates if the PersistentVolume is operationally ready. +// +// By default, it uses DefaultOperationalStatusHandler, which considers a PV +// operational when its phase is Available or Bound. +func (r *Resource) ConvergingStatus(op concepts.ConvergingOperation) (concepts.OperationalStatusWithReason, error) { + return r.base.ConvergingStatus(op) +} + +// ExtractData executes all registered data extractor functions against a deep copy +// of the reconciled PersistentVolume. +// +// This is called by the framework after successful reconciliation, allowing the +// component to read generated or updated values from the PV. +func (r *Resource) ExtractData() error { + return r.base.ExtractData() +} From 650b118bb87a726b835fc2351ec91916b9b7be4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:42:55 +0000 Subject: [PATCH 03/22] Add PersistentVolume primitive documentation Documents the PV primitive's integration lifecycle, cluster-scoped identity, immutable field preservation, mutation pipeline, editors, flavors, operational status mapping, and usage guidance. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 288 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 288 insertions(+) create mode 100644 docs/primitives/pv.md diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md new file mode 100644 index 00000000..2f001f02 --- /dev/null +++ b/docs/primitives/pv.md @@ -0,0 +1,288 @@ +# PersistentVolume Primitive + +The `pv` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolume` resources. It integrates with the component lifecycle and provides a structured mutation API for managing PV spec fields and object metadata. + +## Capabilities + +| Capability | Detail | +|---------------------------|-----------------------------------------------------------------------------------------------------------------| +| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | +| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | +| **Immutable preservation**| Default field applicator preserves immutable fields (volume source, volume mode, claim ref) on existing PVs | +| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | + +## Building a PersistentVolume Primitive + +```go +import "github.com/sourcehawk/operator-component-framework/pkg/primitives/pv" + +base := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "data-volume", + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("100Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CSI: &corev1.CSIPersistentVolumeSource{ + Driver: "ebs.csi.aws.com", + VolumeHandle: "vol-abc123", + }, + }, + }, +} + +resource, err := pv.NewBuilder(base). + WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). + WithMutation(MyFeatureMutation(owner.Spec.Version)). + Build() +``` + +PersistentVolumes are cluster-scoped. The builder validates that Name is set and that Namespace is empty. Setting a namespace on the PV object will cause `Build()` to return an error. + +## Default Field Application + +`DefaultFieldApplicator` preserves immutable fields when updating an existing PersistentVolume: + +- `Spec.PersistentVolumeSource` — the volume backend (NFS, CSI, HostPath, etc.) +- `Spec.VolumeMode` — Filesystem or Block +- `Spec.ClaimRef` — the binding to a PersistentVolumeClaim + +On initial creation (empty `ResourceVersion`), the entire desired state is applied without preservation. + +Use `WithCustomFieldApplicator` when you need different preservation semantics: + +```go +resource, err := pv.NewBuilder(base). + WithCustomFieldApplicator(func(current, desired *corev1.PersistentVolume) error { + // Only update specific fields. + current.Spec.StorageClassName = desired.Spec.StorageClassName + current.Spec.MountOptions = desired.Spec.MountOptions + return nil + }). + Build() +``` + +## Mutations + +Mutations are the primary mechanism for modifying a `PersistentVolume` 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) pv.Mutation { + return pv.Mutation{ + Name: "my-feature", + Feature: feature.NewResourceFeature(version, nil), // always enabled + Mutate: func(m *pv.Mutator) error { + m.SetStorageClassName("fast-ssd") + 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 RetainPolicyMutation(version string, retainEnabled bool) pv.Mutation { + return pv.Mutation{ + Name: "retain-policy", + Feature: feature.NewResourceFeature(version, nil).When(retainEnabled), + Mutate: func(m *pv.Mutator) error { + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + return nil + }, + } +} +``` + +### Version-gated mutations + +```go +var legacyConstraint = mustSemverConstraint("< 2.0.0") + +func LegacyStorageClassMutation(version string) pv.Mutation { + return pv.Mutation{ + Name: "legacy-storage-class", + Feature: feature.NewResourceFeature( + version, + []feature.VersionConstraint{legacyConstraint}, + ), + Mutate: func(m *pv.Mutator) error { + m.SetStorageClassName("legacy-hdd") + 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 `PersistentVolume` | +| 2 | Spec edits | PV spec fields — storage class, reclaim policy, mount options, etc. | + +Within each category, edits are applied in their registration order. Later features observe the PersistentVolume as modified by all previous features. + +## Editors + +### PVSpecEditor + +The primary API for modifying PersistentVolume spec fields. Use `m.EditPVSpec` for full control: + +```go +m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.SetCapacity(resource.MustParse("200Gi")) + e.SetAccessModes([]corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}) + e.SetPersistentVolumeReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + return nil +}) +``` + +#### Available methods + +| Method | What it sets | +|--------------------------------------|--------------------------------------------| +| `SetCapacity(resource.Quantity)` | `.spec.capacity[storage]` | +| `SetAccessModes([]AccessMode)` | `.spec.accessModes` | +| `SetPersistentVolumeReclaimPolicy` | `.spec.persistentVolumeReclaimPolicy` | +| `SetStorageClassName(string)` | `.spec.storageClassName` | +| `SetMountOptions([]string)` | `.spec.mountOptions` | +| `SetVolumeMode(PersistentVolumeMode)`| `.spec.volumeMode` | +| `SetNodeAffinity(*VolumeNodeAffinity)` | `.spec.nodeAffinity` | +| `Raw()` | Returns `*corev1.PersistentVolumeSpec` | + +#### Raw escape hatch + +`Raw()` returns the underlying `*corev1.PersistentVolumeSpec` for free-form editing when none of the structured methods are sufficient: + +```go +m.EditPVSpec(func(e *editors.PVSpecEditor) error { + e.Raw().PersistentVolumeReclaimPolicy = corev1.PersistentVolumeReclaimDelete + 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("storage-tier", "premium") + e.EnsureAnnotation("provisioned-by", "my-operator") + return nil +}) +``` + +## Convenience Methods + +The `Mutator` exposes convenience wrappers for the most common PV spec operations: + +| Method | Equivalent to | +|------------------------------|--------------------------------------------------------| +| `SetStorageClassName(name)` | `EditPVSpec` → `e.SetStorageClassName(name)` | +| `SetReclaimPolicy(policy)` | `EditPVSpec` → `e.SetPersistentVolumeReclaimPolicy(p)` | +| `SetMountOptions(opts)` | `EditPVSpec` → `e.SetMountOptions(opts)` | + +Use these for simple, single-operation mutations. Use `EditPVSpec` when you need multiple operations or raw access in a single edit block. + +## Operational Status + +The PV primitive uses the Integration lifecycle. The default operational status handler maps PV phases to framework status: + +| PV Phase | Operational Status | Meaning | +|-------------|----------------------|---------------------------------------| +| Available | Operational | PV is ready for binding | +| Bound | Operational | PV is bound to a PersistentVolumeClaim| +| Pending | OperationPending | PV is waiting to become available | +| Released | OperationFailing | PV was released, not yet reclaimed | +| Failed | OperationFailing | PV reclamation has failed | + +Override with `WithCustomOperationalStatus` when your PV requires different readiness logic. + +## 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 := pv.NewBuilder(base). + WithFieldApplicationFlavor(pv.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 := pv.NewBuilder(base). + WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). + Build() +``` + +Multiple flavors can be registered and run in registration order. + +## Full Example: Storage-Tier PersistentVolume + +```go +func StorageClassMutation(version string) pv.Mutation { + return pv.Mutation{ + Name: "storage-class", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pv.Mutator) error { + m.SetStorageClassName("fast-ssd") + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + return nil + }, + } +} + +func TierLabelMutation(version, tier string) pv.Mutation { + return pv.Mutation{ + Name: "tier-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pv.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("storage-tier", tier) + return nil + }) + return nil + }, + } +} + +resource, err := pv.NewBuilder(base). + WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). + WithMutation(StorageClassMutation(owner.Spec.Version)). + WithMutation(TierLabelMutation(owner.Spec.Version, "premium")). + Build() +``` + +## Guidance + +**PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs with a clear error. + +**Immutable fields are preserved automatically.** The default field applicator detects existing PVs (by `ResourceVersion`) and preserves the volume source, volume mode, and claim ref. You do not need a custom applicator unless you require different preservation semantics. + +**Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. + +**Register mutations in dependency order.** If mutation B relies on a field set by mutation A, register A first. From 700431be1bf07b32333e2c778bf3eb17eb9210e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:46:37 +0000 Subject: [PATCH 04/22] Add PersistentVolume primitive example Demonstrates PV builder, mutations, flavors, and operational status: - Cluster-scoped identity (no namespace) - Immutable field preservation on existing PVs - Boolean-gated mutations for reclaim policy and mount options - PreserveCurrentAnnotations flavor for external fields - Operational status mapping from PV phases Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pv-primitive/README.md | 29 +++++ examples/pv-primitive/app/controller.go | 10 ++ examples/pv-primitive/features/mutations.go | 61 ++++++++++ examples/pv-primitive/main.go | 118 ++++++++++++++++++++ examples/pv-primitive/resources/pv.go | 53 +++++++++ 5 files changed, 271 insertions(+) create mode 100644 examples/pv-primitive/README.md create mode 100644 examples/pv-primitive/app/controller.go create mode 100644 examples/pv-primitive/features/mutations.go create mode 100644 examples/pv-primitive/main.go create mode 100644 examples/pv-primitive/resources/pv.go diff --git a/examples/pv-primitive/README.md b/examples/pv-primitive/README.md new file mode 100644 index 00000000..de973d47 --- /dev/null +++ b/examples/pv-primitive/README.md @@ -0,0 +1,29 @@ +# PersistentVolume Primitive Example + +This example demonstrates the usage of the `pv` primitive within the operator component framework. +It shows how to manage a Kubernetes PersistentVolume as a component of a larger application, utilising features like: + +- **Base Construction**: Initializing a cluster-scoped PersistentVolume with storage configuration. +- **Feature Mutations**: Applying feature-gated changes to reclaim policy, mount options, and metadata. +- **Field Flavors**: Preserving annotations managed by external controllers using `PreserveCurrentAnnotations`. +- **Data Extraction**: Inspecting PV configuration after each reconcile 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, boolean-gated retain policy, and mount options mutations. +- `resources/`: Contains the central `NewPVResource` factory that assembles all features using `pv.Builder`. +- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. + +## Running the Example + +```bash +go run examples/pv-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 PV configuration after each cycle. +4. Print the resulting status conditions. diff --git a/examples/pv-primitive/app/controller.go b/examples/pv-primitive/app/controller.go new file mode 100644 index 00000000..73126a50 --- /dev/null +++ b/examples/pv-primitive/app/controller.go @@ -0,0 +1,10 @@ +// Package app provides a sample controller using the pv primitive. +// +// PersistentVolumes are cluster-scoped resources. In a real operator, the owner +// would typically be a cluster-scoped CRD (e.g. a ClusterStorageConfig), or the +// PV resource would be registered with SkipOwnerRef to avoid the controller-runtime +// restriction that cluster-scoped resources cannot have namespace-scoped owners. +// +// This example demonstrates the PV primitive's builder, mutation, and status APIs +// without full component reconciliation to keep the focus on the primitive itself. +package app diff --git a/examples/pv-primitive/features/mutations.go b/examples/pv-primitive/features/mutations.go new file mode 100644 index 00000000..7bf07584 --- /dev/null +++ b/examples/pv-primitive/features/mutations.go @@ -0,0 +1,61 @@ +// Package features provides sample mutations for the pv primitive example. +package features + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "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/pv" + corev1 "k8s.io/api/core/v1" +) + +// VersionLabelMutation sets the app.kubernetes.io/version label on the PersistentVolume. +// It is always enabled. +func VersionLabelMutation(version string) pv.Mutation { + return pv.Mutation{ + Name: "version-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pv.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +// RetainPolicyMutation sets the reclaim policy to Retain when enabled. +// This is gated by a boolean condition. +func RetainPolicyMutation(version string, enabled bool) pv.Mutation { + return pv.Mutation{ + Name: "retain-policy", + Feature: feature.NewResourceFeature(version, nil).When(enabled), + Mutate: func(m *pv.Mutator) error { + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + return nil + }, + } +} + +// MountOptionsMutation adds NFS mount options when enabled. +// This is gated by a boolean condition. +func MountOptionsMutation(version string, enabled bool) pv.Mutation { + return pv.Mutation{ + Name: "mount-options", + Feature: feature.NewResourceFeature(version, nil).When(enabled), + Mutate: func(m *pv.Mutator) error { + m.SetMountOptions([]string{"hard", "nfsvers=4.1"}) + return nil + }, + } +} + +// ExampleOperationalStatus demonstrates the default operational status handler +// by returning the status for a given PV phase. +func ExampleOperationalStatus(phase corev1.PersistentVolumePhase) (concepts.OperationalStatusWithReason, error) { + p := &corev1.PersistentVolume{ + Status: corev1.PersistentVolumeStatus{Phase: phase}, + } + return pv.DefaultOperationalStatusHandler(concepts.ConvergingOperationNone, p) +} diff --git a/examples/pv-primitive/main.go b/examples/pv-primitive/main.go new file mode 100644 index 00000000..9e9a5054 --- /dev/null +++ b/examples/pv-primitive/main.go @@ -0,0 +1,118 @@ +// Package main is the entry point for the pv primitive example. +package main + +import ( + "fmt" + "os" + + "github.com/sourcehawk/operator-component-framework/examples/pv-primitive/features" + "github.com/sourcehawk/operator-component-framework/examples/pv-primitive/resources" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/yaml" +) + +func main() { + // 1. Create an example Owner object. + owner := &sharedapp.ExampleApp{ + Spec: sharedapp.ExampleAppSpec{ + Version: "1.0.0", + EnableTracing: false, + EnableMetrics: false, + }, + } + owner.Name = "my-storage-app" + owner.Namespace = "default" + + // 2. Run through multiple spec variations to demonstrate how mutations compose. + // EnableTracing gates the retain-policy mutation; EnableMetrics gates mount options. + specs := []sharedapp.ExampleAppSpec{ + { + Version: "1.0.0", + EnableTracing: false, + EnableMetrics: false, + }, + { + Version: "2.0.0", // Version upgrade + EnableTracing: false, + EnableMetrics: false, + }, + { + Version: "2.0.0", + EnableTracing: true, // Enable retain policy + EnableMetrics: false, + }, + { + Version: "2.0.0", + EnableTracing: true, + EnableMetrics: true, // Enable mount options + }, + } + + for i, spec := range specs { + fmt.Printf("\n--- Step %d: Version=%s, RetainPolicy=%v, MountOpts=%v ---\n", + i+1, spec.Version, spec.EnableTracing, spec.EnableMetrics) + + owner.Spec = spec + + // Build the PV resource for this spec. + pvResource, err := resources.NewPVResource(owner) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to build PV resource: %v\n", err) + os.Exit(1) + } + + fmt.Printf("Identity: %s\n", pvResource.Identity()) + + // Simulate a current PV from the cluster (what CreateOrUpdate would provide). + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-data", + ResourceVersion: "12345", // non-empty to simulate existing object + Annotations: map[string]string{ + "external-controller/managed": "true", // should be preserved by flavor + }, + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("50Gi"), + }, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{Path: "/mnt/old-data"}, + }, + }, + } + + // Apply mutations to the current object. + if err := pvResource.Mutate(current); err != nil { + fmt.Fprintf(os.Stderr, "mutation failed: %v\n", err) + os.Exit(1) + } + + // Print the result. + y, err := yaml.Marshal(current) + if err != nil { + fmt.Fprintf(os.Stderr, "failed to marshal PV: %v\n", err) + os.Exit(1) + } + fmt.Printf("Resulting PV:\n%s", string(y)) + + // Show that immutable fields were preserved from current. + fmt.Printf("Preserved volume source path: %s (original: /mnt/old-data)\n", + current.Spec.PersistentVolumeSource.HostPath.Path) + } + + // 3. Demonstrate the operational status handler. + fmt.Println("\n--- Operational Status Examples ---") + for _, phase := range []corev1.PersistentVolumePhase{ + corev1.VolumeAvailable, corev1.VolumeBound, corev1.VolumePending, + corev1.VolumeReleased, corev1.VolumeFailed, + } { + status, _ := features.ExampleOperationalStatus(phase) + fmt.Printf("Phase %-10s → Status: %s, Reason: %s\n", phase, status.Status, status.Reason) + } + + fmt.Println("\nExample completed successfully!") +} diff --git a/examples/pv-primitive/resources/pv.go b/examples/pv-primitive/resources/pv.go new file mode 100644 index 00000000..04fd9d58 --- /dev/null +++ b/examples/pv-primitive/resources/pv.go @@ -0,0 +1,53 @@ +// Package resources provides resource implementations for the pv primitive example. +package resources + +import ( + "github.com/sourcehawk/operator-component-framework/examples/pv-primitive/features" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/pv" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NewPVResource constructs a PersistentVolume primitive resource with all the features. +func NewPVResource(owner *sharedapp.ExampleApp) (*pv.Resource, error) { + // 1. Create the base PersistentVolume object. + // PVs are cluster-scoped — no namespace is set. + base := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-data", + Labels: map[string]string{ + "app": owner.Name, + }, + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("100Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/mnt/data", + }, + }, + StorageClassName: "standard", + }, + } + + // 2. Initialize the PV builder. + builder := pv.NewBuilder(base) + + // 3. Register mutations in dependency order. + builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) + builder.WithMutation(features.RetainPolicyMutation(owner.Spec.Version, owner.Spec.EnableTracing)) + builder.WithMutation(features.MountOptionsMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + + // 4. Preserve annotations added by external controllers. + builder.WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations) + + // 5. Build the final resource. + return builder.Build() +} From 8f995fe7bff978e4dd09f6cb39b0f345ecdd9df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:54:04 +0000 Subject: [PATCH 05/22] Fix staticcheck lint: remove redundant embedded field selector Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pv-primitive/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/pv-primitive/main.go b/examples/pv-primitive/main.go index 9e9a5054..443b332b 100644 --- a/examples/pv-primitive/main.go +++ b/examples/pv-primitive/main.go @@ -101,7 +101,7 @@ func main() { // Show that immutable fields were preserved from current. fmt.Printf("Preserved volume source path: %s (original: /mnt/old-data)\n", - current.Spec.PersistentVolumeSource.HostPath.Path) + current.Spec.HostPath.Path) } // 3. Demonstrate the operational status handler. From c117fa24e6bb3148cf18cfa339a475d8cf25e5d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 04:30:50 +0000 Subject: [PATCH 06/22] Address Copilot review comments on PV primitive PR - Fix compile error: use Spec.PersistentVolumeSource.HostPath instead of non-existent Spec.HostPath field, with nil check - Update README to accurately describe what the example does (in-memory mutations, not fake client; operational status, not status conditions) - Fix controller.go comment to remove reference to non-existent SkipOwnerRef option Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pv-primitive/README.md | 9 ++++----- examples/pv-primitive/app/controller.go | 6 ++++-- examples/pv-primitive/main.go | 7 +++++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/examples/pv-primitive/README.md b/examples/pv-primitive/README.md index de973d47..6ffd057f 100644 --- a/examples/pv-primitive/README.md +++ b/examples/pv-primitive/README.md @@ -14,7 +14,7 @@ It shows how to manage a Kubernetes PersistentVolume as a component of a larger - `features/`: Contains modular feature definitions: - `mutations.go`: version labelling, boolean-gated retain policy, and mount options mutations. - `resources/`: Contains the central `NewPVResource` factory that assembles all features using `pv.Builder`. -- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. +- `main.go`: A standalone entry point that demonstrates in-memory mutation across multiple spec variations. ## Running the Example @@ -23,7 +23,6 @@ go run examples/pv-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 PV configuration after each cycle. -4. Print the resulting status conditions. +1. Create an `ExampleApp` owner object. +2. Apply mutations across four spec variations, printing the resulting PV YAML after each cycle. +3. Print operational status examples for each PV phase. diff --git a/examples/pv-primitive/app/controller.go b/examples/pv-primitive/app/controller.go index 73126a50..6186ca64 100644 --- a/examples/pv-primitive/app/controller.go +++ b/examples/pv-primitive/app/controller.go @@ -2,8 +2,10 @@ // // PersistentVolumes are cluster-scoped resources. In a real operator, the owner // would typically be a cluster-scoped CRD (e.g. a ClusterStorageConfig), or the -// PV resource would be registered with SkipOwnerRef to avoid the controller-runtime -// restriction that cluster-scoped resources cannot have namespace-scoped owners. +// PV resource would be created and managed without using a component create +// pipeline that unconditionally sets controller references, to avoid the +// controller-runtime restriction that cluster-scoped resources cannot have +// namespace-scoped owners. // // This example demonstrates the PV primitive's builder, mutation, and status APIs // without full component reconciliation to keep the focus on the primitive itself. diff --git a/examples/pv-primitive/main.go b/examples/pv-primitive/main.go index 443b332b..974708ce 100644 --- a/examples/pv-primitive/main.go +++ b/examples/pv-primitive/main.go @@ -100,8 +100,11 @@ func main() { fmt.Printf("Resulting PV:\n%s", string(y)) // Show that immutable fields were preserved from current. - fmt.Printf("Preserved volume source path: %s (original: /mnt/old-data)\n", - current.Spec.HostPath.Path) + var preservedPath string + if hp := current.Spec.PersistentVolumeSource.HostPath; hp != nil { + preservedPath = hp.Path + } + fmt.Printf("Preserved volume source path: %s (original: /mnt/old-data)\n", preservedPath) } // 3. Demonstrate the operational status handler. From 597e7de65abded3d1994a87b4ecf90e76d162a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 04:42:06 +0000 Subject: [PATCH 07/22] fix lint --- examples/pv-primitive/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/pv-primitive/main.go b/examples/pv-primitive/main.go index 974708ce..a6c84006 100644 --- a/examples/pv-primitive/main.go +++ b/examples/pv-primitive/main.go @@ -101,7 +101,7 @@ func main() { // Show that immutable fields were preserved from current. var preservedPath string - if hp := current.Spec.PersistentVolumeSource.HostPath; hp != nil { + if hp := current.Spec.HostPath; hp != nil { preservedPath = hp.Path } fmt.Printf("Preserved volume source path: %s (original: /mnt/old-data)\n", preservedPath) From f06d21ca1901437082f4972187f858d49863dcfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 13:27:59 +0000 Subject: [PATCH 08/22] Add unit tests for DefaultFieldApplicator Cover create-vs-update behavior, immutable field preservation (PersistentVolumeSource, VolumeMode, ClaimRef), nil immutable field handling, and verify the desired object is not mutated. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/resource_test.go | 211 +++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 pkg/primitives/pv/resource_test.go diff --git a/pkg/primitives/pv/resource_test.go b/pkg/primitives/pv/resource_test.go new file mode 100644 index 00000000..821b7757 --- /dev/null +++ b/pkg/primitives/pv/resource_test.go @@ -0,0 +1,211 @@ +package pv + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestDefaultFieldApplicator_Create(t *testing.T) { + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + } + volumeMode := corev1.PersistentVolumeFilesystem + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + Labels: map[string]string{"app": "myapp"}, + }, + Spec: corev1.PersistentVolumeSpec{ + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "nfs.example.com", + Path: "/exports/data", + }, + }, + VolumeMode: &volumeMode, + StorageClassName: "fast-ssd", + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // On create (empty ResourceVersion), all fields should be applied. + assert.Equal(t, "myapp", current.Labels["app"]) + assert.Equal(t, resource.MustParse("10Gi"), current.Spec.Capacity[corev1.ResourceStorage]) + assert.Equal(t, "nfs.example.com", current.Spec.PersistentVolumeSource.NFS.Server) + assert.Equal(t, &volumeMode, current.Spec.VolumeMode) + assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) + assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy) +} + +func TestDefaultFieldApplicator_Update_PreservesImmutableFields(t *testing.T) { + existingVolumeMode := corev1.PersistentVolumeBlock + claimRef := &corev1.ObjectReference{ + Namespace: "default", + Name: "my-pvc", + } + + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + ResourceVersion: "12345", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "nfs.example.com", + Path: "/exports/data", + }, + }, + VolumeMode: &existingVolumeMode, + ClaimRef: claimRef, + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + StorageClassName: "old-class", + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + }, + } + + desiredVolumeMode := corev1.PersistentVolumeFilesystem + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + Labels: map[string]string{"updated": "true"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/mnt/data", + }, + }, + VolumeMode: &desiredVolumeMode, + ClaimRef: &corev1.ObjectReference{ + Namespace: "other", + Name: "other-pvc", + }, + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("20Gi"), + }, + StorageClassName: "new-class", + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Immutable fields should be preserved from the existing object. + assert.Equal(t, "nfs.example.com", current.Spec.PersistentVolumeSource.NFS.Server, + "PersistentVolumeSource should be preserved on update") + assert.Nil(t, current.Spec.PersistentVolumeSource.HostPath, + "desired volume source should not overwrite existing") + assert.Equal(t, &existingVolumeMode, current.Spec.VolumeMode, + "VolumeMode should be preserved on update") + assert.Equal(t, "my-pvc", current.Spec.ClaimRef.Name, + "ClaimRef should be preserved on update") + + // Mutable fields should be updated from desired. + assert.Equal(t, "true", current.Labels["updated"], + "labels should be updated from desired") + assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Capacity[corev1.ResourceStorage], + "capacity should be updated from desired") + assert.Equal(t, "new-class", current.Spec.StorageClassName, + "storage class should be updated from desired") + assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy, + "reclaim policy should be updated from desired") +} + +func TestDefaultFieldApplicator_Update_NilImmutableFields(t *testing.T) { + // When existing object has nil immutable fields, they should stay nil + // (not be overwritten by desired values). + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + ResourceVersion: "1", + }, + Spec: corev1.PersistentVolumeSpec{}, + } + + volumeMode := corev1.PersistentVolumeFilesystem + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "nfs.example.com", + Path: "/data", + }, + }, + VolumeMode: &volumeMode, + ClaimRef: &corev1.ObjectReference{ + Name: "some-pvc", + }, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Immutable fields from current (all zero/nil) should be restored. + assert.Nil(t, current.Spec.PersistentVolumeSource.NFS, + "nil volume source should be preserved") + assert.Nil(t, current.Spec.VolumeMode, + "nil volume mode should be preserved") + assert.Nil(t, current.Spec.ClaimRef, + "nil claim ref should be preserved") +} + +func TestDefaultFieldApplicator_DoesNotMutateDesired(t *testing.T) { + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + ResourceVersion: "1", + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "existing.example.com", + Path: "/old", + }, + }, + }, + } + + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1.PersistentVolumeSpec{ + StorageClassName: "desired-class", + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "desired.example.com", + Path: "/new", + }, + }, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired object must not be mutated. + assert.Equal(t, "desired.example.com", desired.Spec.PersistentVolumeSource.NFS.Server) + assert.Equal(t, "desired-class", desired.Spec.StorageClassName) +} From 4eb153cbaec1fc6c149f592eaa7db3f42143b22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 19:55:27 +0000 Subject: [PATCH 09/22] preserve server-managed metadata in default field applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/resource.go | 17 ++++---- pkg/primitives/pv/resource_test.go | 67 +++++++++++++++++++++++++++--- 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/pkg/primitives/pv/resource.go b/pkg/primitives/pv/resource.go index a7b2b8db..4e30b49b 100644 --- a/pkg/primitives/pv/resource.go +++ b/pkg/primitives/pv/resource.go @@ -7,14 +7,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator preserves immutable fields from the existing PersistentVolume -// while applying the desired state. -// -// PersistentVolumes have immutable fields that cannot be changed after creation -// (e.g. the volume source). This applicator deep-copies the desired state onto -// the current object but preserves the existing Spec.PersistentVolumeSource, -// Spec.VolumeMode, and Spec.ClaimRef so that updates do not attempt to change -// server-enforced immutable fields. +// DefaultFieldApplicator replaces current with a deep copy of desired while +// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), +// shared-controller fields (OwnerReferences, Finalizers), and PV-specific +// immutable fields (PersistentVolumeSource, VolumeMode, ClaimRef) from the +// original current object. // // On a fresh PV (empty ResourceVersion, meaning it has not yet been persisted), // the full desired state is applied without preservation. @@ -30,9 +27,11 @@ func DefaultFieldApplicator(current, desired *corev1.PersistentVolume) error { savedVolumeMode := current.Spec.VolumeMode savedClaimRef := current.Spec.ClaimRef + original := current.DeepCopy() *current = *desired.DeepCopy() + generic.PreserveServerManagedFields(current, original) - // Restore immutable fields from the live object. + // Restore PV-specific immutable fields from the live object. current.Spec.PersistentVolumeSource = savedSource current.Spec.VolumeMode = savedVolumeMode current.Spec.ClaimRef = savedClaimRef diff --git a/pkg/primitives/pv/resource_test.go b/pkg/primitives/pv/resource_test.go index 821b7757..f347f1c1 100644 --- a/pkg/primitives/pv/resource_test.go +++ b/pkg/primitives/pv/resource_test.go @@ -8,6 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) func TestDefaultFieldApplicator_Create(t *testing.T) { @@ -45,7 +46,7 @@ func TestDefaultFieldApplicator_Create(t *testing.T) { // On create (empty ResourceVersion), all fields should be applied. assert.Equal(t, "myapp", current.Labels["app"]) assert.Equal(t, resource.MustParse("10Gi"), current.Spec.Capacity[corev1.ResourceStorage]) - assert.Equal(t, "nfs.example.com", current.Spec.PersistentVolumeSource.NFS.Server) + assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server) assert.Equal(t, &volumeMode, current.Spec.VolumeMode) assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy) @@ -109,9 +110,9 @@ func TestDefaultFieldApplicator_Update_PreservesImmutableFields(t *testing.T) { require.NoError(t, err) // Immutable fields should be preserved from the existing object. - assert.Equal(t, "nfs.example.com", current.Spec.PersistentVolumeSource.NFS.Server, + assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server, "PersistentVolumeSource should be preserved on update") - assert.Nil(t, current.Spec.PersistentVolumeSource.HostPath, + assert.Nil(t, current.Spec.HostPath, "desired volume source should not overwrite existing") assert.Equal(t, &existingVolumeMode, current.Spec.VolumeMode, "VolumeMode should be preserved on update") @@ -163,7 +164,7 @@ func TestDefaultFieldApplicator_Update_NilImmutableFields(t *testing.T) { require.NoError(t, err) // Immutable fields from current (all zero/nil) should be restored. - assert.Nil(t, current.Spec.PersistentVolumeSource.NFS, + assert.Nil(t, current.Spec.NFS, "nil volume source should be preserved") assert.Nil(t, current.Spec.VolumeMode, "nil volume mode should be preserved") @@ -206,6 +207,62 @@ func TestDefaultFieldApplicator_DoesNotMutateDesired(t *testing.T) { require.NoError(t, err) // Desired object must not be mutated. - assert.Equal(t, "desired.example.com", desired.Spec.PersistentVolumeSource.NFS.Server) + assert.Equal(t, "desired.example.com", desired.Spec.NFS.Server) assert.Equal(t, "desired-class", desired.Spec.StorageClassName) } + +func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { + existingVolumeMode := corev1.PersistentVolumeBlock + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + ResourceVersion: "12345", + UID: types.UID("abc-def"), + Generation: 3, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, + }, + Finalizers: []string{"finalizer.example.com"}, + }, + Spec: corev1.PersistentVolumeSpec{ + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: "nfs.example.com", + Path: "/exports/data", + }, + }, + VolumeMode: &existingVolumeMode, + }, + } + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PersistentVolumeSpec{ + StorageClassName: "fast-ssd", + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec and labels are applied + assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) + assert.Equal(t, "test", current.Labels["app"]) + + // 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) + + // PV-specific immutable fields are preserved + assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server) + assert.Equal(t, &existingVolumeMode, current.Spec.VolumeMode) +} From 46fb62b37f601dc2c3a88db441881852a8e71371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 21:42:31 +0000 Subject: [PATCH 10/22] docs: address PR review feedback for PV primitive - Add cluster-scoped owner constraint guidance to PV docs - Add PV primitive and PVSpecEditor to primitives index Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 2 ++ docs/primitives/pv.md | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/docs/primitives.md b/docs/primitives.md index 13fde6a6..caf6c5b1 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 | +| `PVSpecEditor` | PV spec fields — capacity, access modes, reclaim policy, storage class | | `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/pv` | Integration| [pv.md](primitives/pv.md) | ## Usage Examples diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index 2f001f02..a2e2b183 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -285,4 +285,9 @@ resource, err := pv.NewBuilder(base). **Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. +**Controller references require a cluster-scoped owner.** The component reconciliation pipeline sets a controller reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be cluster-scoped. Using a namespace-scoped component (the common case) as the owner will cause the API server to reject the PV. If you need to manage PVs from a namespace-scoped component, either: + +- Model the PV as owned by a dedicated cluster-scoped controller/component, or +- Manage the PV outside the default component create/update pipeline (for example, via a custom controller that does **not** set `ownerReferences` on the PV). + **Register mutations in dependency order.** If mutation B relies on a field set by mutation A, register A first. From 195ad33ccb1d4c4453e92290772998297762ccef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 03:25:29 +0000 Subject: [PATCH 11/22] refactor: delegate PV builder to generic IntegrationBuilder.Build() Use MarkClusterScoped() + WithCustomOperationalStatus() on the generic IntegrationBuilder instead of reimplementing validation and resource construction. This ensures ValidateBase() runs the full shared validation (identity/applicator/mutator-factory checks) and keeps the PV primitive consistent with other primitives. Also fix table alignment in docs/primitives.md for the PV row. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 2 +- pkg/primitives/pv/builder.go | 46 ++++++------------------------- pkg/primitives/pv/builder_test.go | 2 +- 3 files changed, 11 insertions(+), 39 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index caf6c5b1..ae8a15de 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -131,7 +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/pv` | Integration| [pv.md](primitives/pv.md) | +| `pkg/primitives/pv` | Integration | [pv.md](primitives/pv.md) | ## Usage Examples diff --git a/pkg/primitives/pv/builder.go b/pkg/primitives/pv/builder.go index acf377a9..af4e7e82 100644 --- a/pkg/primitives/pv/builder.go +++ b/pkg/primitives/pv/builder.go @@ -2,7 +2,6 @@ package pv import ( - "errors" "fmt" "github.com/sourcehawk/operator-component-framework/internal/generic" @@ -18,8 +17,7 @@ import ( // configuration and returns an initialized Resource ready for use in a // reconciliation loop. type Builder struct { - base *generic.IntegrationBuilder[*corev1.PersistentVolume, *Mutator] - operationalStatusHandler func(concepts.ConvergingOperation, *corev1.PersistentVolume) (concepts.OperationalStatusWithReason, error) + base *generic.IntegrationBuilder[*corev1.PersistentVolume, *Mutator] } // NewBuilder initializes a new Builder with the provided PersistentVolume object. @@ -41,13 +39,10 @@ func NewBuilder(pv *corev1.PersistentVolume) *Builder { DefaultFieldApplicator, NewMutator, ) + base.MarkClusterScoped() + base.WithCustomOperationalStatus(DefaultOperationalStatusHandler) - b := &Builder{ - base: base, - operationalStatusHandler: DefaultOperationalStatusHandler, - } - - return b + return &Builder{base: base} } // WithMutation registers a mutation for the PersistentVolume. @@ -100,7 +95,7 @@ func (b *Builder) WithFieldApplicationFlavor(flavor FieldApplicationFlavor) *Bui func (b *Builder) WithCustomOperationalStatus( handler func(concepts.ConvergingOperation, *corev1.PersistentVolume) (concepts.OperationalStatusWithReason, error), ) *Builder { - b.operationalStatusHandler = handler + b.base.WithCustomOperationalStatus(handler) return b } @@ -126,35 +121,12 @@ func (b *Builder) WithDataExtractor(extractor func(corev1.PersistentVolume) erro // - No PersistentVolume object was provided. // - The PersistentVolume is missing a Name. // - The PersistentVolume has a Namespace set (PVs are cluster-scoped). +// - Identity function, field applicator, or mutator factory is nil. func (b *Builder) Build() (*Resource, error) { - if err := b.validate(); err != nil { + genericRes, err := b.base.Build() + if err != nil { return nil, err } - res := &generic.IntegrationResource[*corev1.PersistentVolume, *Mutator]{ - BaseResource: *b.base.BaseRes, - OperationalStatusHandler: b.operationalStatusHandler, - } - - return &Resource{base: res}, nil -} - -// validate checks PV-specific builder constraints. -// -// PersistentVolumes are cluster-scoped: Name is required, Namespace must be empty. -func (b *Builder) validate() error { - obj := b.base.BaseRes.DesiredObject - if obj == nil { - return errors.New("object cannot be nil") - } - - if obj.GetName() == "" { - return errors.New("object name cannot be empty") - } - - if obj.GetNamespace() != "" { - return errors.New("PersistentVolume is cluster-scoped: namespace must not be set") - } - - return nil + return &Resource{base: genericRes}, nil } diff --git a/pkg/primitives/pv/builder_test.go b/pkg/primitives/pv/builder_test.go index 0c7c4263..294cc0a3 100644 --- a/pkg/primitives/pv/builder_test.go +++ b/pkg/primitives/pv/builder_test.go @@ -36,7 +36,7 @@ func TestBuilder_Build_Validation(t *testing.T) { pv: &corev1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "test-pv", Namespace: "should-not-be-set"}, }, - expectedErr: "PersistentVolume is cluster-scoped: namespace must not be set", + expectedErr: "cluster-scoped object must not have a namespace", }, { name: "valid persistent volume", From 35bc9b171d1c254647ea0ed840d0498f6b845f81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:43:18 +0000 Subject: [PATCH 12/22] fix: remove beginFeature from PV mutator constructor Align with the fix applied to the deployment primitive on main. The constructor should not call beginFeature() because the generic mutator_helper.go already calls it, which would create an empty leading feature plan. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/mutator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/pv/mutator.go b/pkg/primitives/pv/mutator.go index bbdac303..dc14c6b1 100644 --- a/pkg/primitives/pv/mutator.go +++ b/pkg/primitives/pv/mutator.go @@ -33,8 +33,11 @@ type Mutator struct { // NewMutator creates a new Mutator for the given PersistentVolume. func NewMutator(pv *corev1.PersistentVolume) *Mutator { - m := &Mutator{pv: pv} - m.beginFeature() + m := &Mutator{ + pv: pv, + plans: []featurePlan{{}}, + } + m.active = &m.plans[0] return m } From 29fadc10c5b5d83b3e1c50c8a6d9f8df034f5538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:59:01 +0000 Subject: [PATCH 13/22] fix: address PR review feedback for PV primitive - Use Quantity.Equal for resource comparisons in tests instead of assert.Equal to avoid brittle internal-representation mismatches - Correct docs and example comments to reflect framework behavior: ownerReferences are conditionally skipped (not rejected) for cluster-scoped resources with namespace-scoped owners - Fix README bullet: replace "Data Extraction" with "Result Inspection" to match actual example code Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 10 +++--- examples/pv-primitive/README.md | 2 +- examples/pv-primitive/app/controller.go | 11 ++++--- pkg/primitives/pv/resource.go | 7 ++-- pkg/primitives/pv/resource_test.go | 43 +++++++++++++++++++++++-- 5 files changed, 57 insertions(+), 16 deletions(-) diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index a2e2b183..76b7c9f4 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -46,7 +46,9 @@ PersistentVolumes are cluster-scoped. The builder validates that Name is set and ## Default Field Application -`DefaultFieldApplicator` preserves immutable fields when updating an existing PersistentVolume: +`DefaultFieldApplicator` replaces the current PersistentVolume with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), the Status subresource, and PV-specific immutable fields from the original live object. This prevents spec-level reconciliation from clearing status data written by the API server or other controllers. + +Preserved immutable fields: - `Spec.PersistentVolumeSource` — the volume backend (NFS, CSI, HostPath, etc.) - `Spec.VolumeMode` — Filesystem or Block @@ -285,9 +287,9 @@ resource, err := pv.NewBuilder(base). **Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. -**Controller references require a cluster-scoped owner.** The component reconciliation pipeline sets a controller reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be cluster-scoped. Using a namespace-scoped component (the common case) as the owner will cause the API server to reject the PV. If you need to manage PVs from a namespace-scoped component, either: +**Controller references and garbage collection.** The component reconciliation pipeline attempts to set a controller reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be cluster-scoped. When the owner is namespace-scoped and the PV is cluster-scoped, the framework detects this mismatch and **skips setting `ownerReferences`** (logging an informational message) instead of letting the API server reject the request. As a result, such PVs will **not** be garbage collected automatically when the owning component is deleted. If you need garbage collection for PVs, either: -- Model the PV as owned by a dedicated cluster-scoped controller/component, or -- Manage the PV outside the default component create/update pipeline (for example, via a custom controller that does **not** set `ownerReferences` on the PV). +- Model the PV as owned by a dedicated **cluster-scoped** controller/component so a valid controller reference can be set, or +- Accept that PVs managed from a **namespace-scoped** component will not have `ownerReferences` and handle their lifecycle explicitly (for example, by deleting them in custom logic when appropriate). **Register mutations in dependency order.** If mutation B relies on a field set by mutation A, register A first. diff --git a/examples/pv-primitive/README.md b/examples/pv-primitive/README.md index 6ffd057f..282b6f16 100644 --- a/examples/pv-primitive/README.md +++ b/examples/pv-primitive/README.md @@ -6,7 +6,7 @@ It shows how to manage a Kubernetes PersistentVolume as a component of a larger - **Base Construction**: Initializing a cluster-scoped PersistentVolume with storage configuration. - **Feature Mutations**: Applying feature-gated changes to reclaim policy, mount options, and metadata. - **Field Flavors**: Preserving annotations managed by external controllers using `PreserveCurrentAnnotations`. -- **Data Extraction**: Inspecting PV configuration after each reconcile cycle. +- **Result Inspection**: Printing PV configuration after each reconcile cycle. ## Directory Structure diff --git a/examples/pv-primitive/app/controller.go b/examples/pv-primitive/app/controller.go index 6186ca64..e24403c9 100644 --- a/examples/pv-primitive/app/controller.go +++ b/examples/pv-primitive/app/controller.go @@ -1,11 +1,12 @@ // Package app provides a sample controller using the pv primitive. // // PersistentVolumes are cluster-scoped resources. In a real operator, the owner -// would typically be a cluster-scoped CRD (e.g. a ClusterStorageConfig), or the -// PV resource would be created and managed without using a component create -// pipeline that unconditionally sets controller references, to avoid the -// controller-runtime restriction that cluster-scoped resources cannot have -// namespace-scoped owners. +// would typically be a cluster-scoped CRD (e.g. a ClusterStorageConfig). When a +// component create pipeline is used, it will only set controller references where +// the owner/owned scopes are compatible, and will skip ownerReferences for +// cluster-scoped resources that would otherwise have namespace-scoped owners. In +// those skipped cases, the cluster-scoped resource will not be garbage-collected +// with the namespaced owner. // // This example demonstrates the PV primitive's builder, mutation, and status APIs // without full component reconciliation to keep the focus on the primitive itself. diff --git a/pkg/primitives/pv/resource.go b/pkg/primitives/pv/resource.go index 4e30b49b..e0531075 100644 --- a/pkg/primitives/pv/resource.go +++ b/pkg/primitives/pv/resource.go @@ -9,9 +9,9 @@ import ( // DefaultFieldApplicator replaces current with a deep copy of desired while // preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), -// shared-controller fields (OwnerReferences, Finalizers), and PV-specific -// immutable fields (PersistentVolumeSource, VolumeMode, ClaimRef) from the -// original current object. +// shared-controller fields (OwnerReferences, Finalizers), the Status +// subresource, and PV-specific immutable fields (PersistentVolumeSource, +// VolumeMode, ClaimRef) from the original current object. // // On a fresh PV (empty ResourceVersion, meaning it has not yet been persisted), // the full desired state is applied without preservation. @@ -30,6 +30,7 @@ func DefaultFieldApplicator(current, desired *corev1.PersistentVolume) error { original := current.DeepCopy() *current = *desired.DeepCopy() generic.PreserveServerManagedFields(current, original) + generic.PreserveStatus(current, original) // Restore PV-specific immutable fields from the live object. current.Spec.PersistentVolumeSource = savedSource diff --git a/pkg/primitives/pv/resource_test.go b/pkg/primitives/pv/resource_test.go index f347f1c1..4534cd34 100644 --- a/pkg/primitives/pv/resource_test.go +++ b/pkg/primitives/pv/resource_test.go @@ -45,7 +45,9 @@ func TestDefaultFieldApplicator_Create(t *testing.T) { // On create (empty ResourceVersion), all fields should be applied. assert.Equal(t, "myapp", current.Labels["app"]) - assert.Equal(t, resource.MustParse("10Gi"), current.Spec.Capacity[corev1.ResourceStorage]) + expectedCapacity := resource.MustParse("10Gi") + actualCapacity := current.Spec.Capacity[corev1.ResourceStorage] + assert.True(t, expectedCapacity.Equal(actualCapacity), "expected storage capacity %s, got %s", expectedCapacity.String(), actualCapacity.String()) assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server) assert.Equal(t, &volumeMode, current.Spec.VolumeMode) assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) @@ -122,8 +124,10 @@ func TestDefaultFieldApplicator_Update_PreservesImmutableFields(t *testing.T) { // Mutable fields should be updated from desired. assert.Equal(t, "true", current.Labels["updated"], "labels should be updated from desired") - assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Capacity[corev1.ResourceStorage], - "capacity should be updated from desired") + expectedUpdatedCapacity := resource.MustParse("20Gi") + actualUpdatedCapacity := current.Spec.Capacity[corev1.ResourceStorage] + assert.True(t, expectedUpdatedCapacity.Equal(actualUpdatedCapacity), + "capacity should be updated from desired: expected %s, got %s", expectedUpdatedCapacity.String(), actualUpdatedCapacity.String()) assert.Equal(t, "new-class", current.Spec.StorageClassName, "storage class should be updated from desired") assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy, @@ -211,6 +215,39 @@ func TestDefaultFieldApplicator_DoesNotMutateDesired(t *testing.T) { assert.Equal(t, "desired-class", desired.Spec.StorageClassName) } +func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { + current := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + ResourceVersion: "12345", + }, + Status: corev1.PersistentVolumeStatus{ + Phase: corev1.VolumeBound, + Message: "bound to claim default/data", + Reason: "Bound", + }, + } + desired := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Spec: corev1.PersistentVolumeSpec{ + StorageClassName: "fast-ssd", + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec is applied + assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) + + // Status from the live object is preserved + assert.Equal(t, corev1.VolumeBound, current.Status.Phase) + assert.Equal(t, "bound to claim default/data", current.Status.Message) + assert.Equal(t, "Bound", current.Status.Reason) +} + func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { existingVolumeMode := corev1.PersistentVolumeBlock current := &corev1.PersistentVolume{ From 96f290d5e18395b079317384ee962539d7d39854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 21:33:27 +0000 Subject: [PATCH 14/22] fix: export BeginFeature() on PV mutator to match FeatureMutator interface Align with the canonical fix in main (Fix/architectural issues #43) which exported BeginFeature() on the FeatureMutator interface. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/mutator.go | 4 ++-- pkg/primitives/pv/mutator_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/primitives/pv/mutator.go b/pkg/primitives/pv/mutator.go index dc14c6b1..1104d328 100644 --- a/pkg/primitives/pv/mutator.go +++ b/pkg/primitives/pv/mutator.go @@ -41,9 +41,9 @@ func NewMutator(pv *corev1.PersistentVolume) *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/pv/mutator_test.go b/pkg/primitives/pv/mutator_test.go index 4cec65e7..c82f4d51 100644 --- a/pkg/primitives/pv/mutator_test.go +++ b/pkg/primitives/pv/mutator_test.go @@ -125,7 +125,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { pv := newTestPV() m := NewMutator(pv) m.SetStorageClassName("feature1-class") - m.beginFeature() + m.BeginFeature() m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) require.NoError(t, m.Apply()) @@ -137,7 +137,7 @@ func TestMutator_LaterFeatureObservesPrior(t *testing.T) { pv := newTestPV() m := NewMutator(pv) m.SetStorageClassName("first") - m.beginFeature() + m.BeginFeature() // Second feature should see the storage class set by the first. m.EditPVSpec(func(e *editors.PVSpecEditor) error { if e.Raw().StorageClassName == "first" { From 045722c85b40d1082e2ec45851a7806249135d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:29:31 +0000 Subject: [PATCH 15/22] style: format markdown files with prettier Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 131 +++++++++++++++++++------------- examples/pv-primitive/README.md | 10 ++- 2 files changed, 85 insertions(+), 56 deletions(-) diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index 76b7c9f4..e0dd99ff 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -1,17 +1,19 @@ # PersistentVolume Primitive -The `pv` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolume` resources. It integrates with the component lifecycle and provides a structured mutation API for managing PV spec fields and object metadata. +The `pv` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolume` +resources. It integrates with the component lifecycle and provides a structured mutation API for managing PV spec fields +and object metadata. ## Capabilities -| Capability | Detail | -|---------------------------|-----------------------------------------------------------------------------------------------------------------| -| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | -| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | -| **Immutable preservation**| Default field applicator preserves immutable fields (volume source, volume mode, claim ref) on existing PVs | -| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed labels and annotations not owned by the operator | -| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | +| Capability | Detail | +| -------------------------- | ----------------------------------------------------------------------------------------------------------- | +| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | +| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | +| **Immutable preservation** | Default field applicator preserves immutable fields (volume source, volume mode, claim ref) on existing PVs | +| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | ## Building a PersistentVolume Primitive @@ -42,11 +44,15 @@ resource, err := pv.NewBuilder(base). Build() ``` -PersistentVolumes are cluster-scoped. The builder validates that Name is set and that Namespace is empty. Setting a namespace on the PV object will cause `Build()` to return an error. +PersistentVolumes are cluster-scoped. The builder validates that Name is set and that Namespace is empty. Setting a +namespace on the PV object will cause `Build()` to return an error. ## Default Field Application -`DefaultFieldApplicator` replaces the current PersistentVolume with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), the Status subresource, and PV-specific immutable fields 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 PersistentVolume with a deep copy of the desired object, then restores +server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), the Status +subresource, and PV-specific immutable fields from the original live object. This prevents spec-level reconciliation +from clearing status data written by the API server or other controllers. Preserved immutable fields: @@ -71,9 +77,11 @@ resource, err := pv.NewBuilder(base). ## Mutations -Mutations are the primary mechanism for modifying a `PersistentVolume` 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 `PersistentVolume` 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) pv.Mutation { @@ -88,7 +96,8 @@ func MyFeatureMutation(version string) pv.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 @@ -129,14 +138,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 `PersistentVolume` | +| Step | Category | What it affects | +| ---- | -------------- | ------------------------------------------------------------------- | +| 1 | Metadata edits | Labels and annotations on the `PersistentVolume` | | 2 | Spec edits | PV spec fields — storage class, reclaim policy, mount options, etc. | -Within each category, edits are applied in their registration order. Later features observe the PersistentVolume as modified by all previous features. +Within each category, edits are applied in their registration order. Later features observe the PersistentVolume as +modified by all previous features. ## Editors @@ -155,20 +166,21 @@ m.EditPVSpec(func(e *editors.PVSpecEditor) error { #### Available methods -| Method | What it sets | -|--------------------------------------|--------------------------------------------| -| `SetCapacity(resource.Quantity)` | `.spec.capacity[storage]` | -| `SetAccessModes([]AccessMode)` | `.spec.accessModes` | -| `SetPersistentVolumeReclaimPolicy` | `.spec.persistentVolumeReclaimPolicy` | -| `SetStorageClassName(string)` | `.spec.storageClassName` | -| `SetMountOptions([]string)` | `.spec.mountOptions` | -| `SetVolumeMode(PersistentVolumeMode)`| `.spec.volumeMode` | -| `SetNodeAffinity(*VolumeNodeAffinity)` | `.spec.nodeAffinity` | -| `Raw()` | Returns `*corev1.PersistentVolumeSpec` | +| Method | What it sets | +| -------------------------------------- | -------------------------------------- | +| `SetCapacity(resource.Quantity)` | `.spec.capacity[storage]` | +| `SetAccessModes([]AccessMode)` | `.spec.accessModes` | +| `SetPersistentVolumeReclaimPolicy` | `.spec.persistentVolumeReclaimPolicy` | +| `SetStorageClassName(string)` | `.spec.storageClassName` | +| `SetMountOptions([]string)` | `.spec.mountOptions` | +| `SetVolumeMode(PersistentVolumeMode)` | `.spec.volumeMode` | +| `SetNodeAffinity(*VolumeNodeAffinity)` | `.spec.nodeAffinity` | +| `Raw()` | Returns `*corev1.PersistentVolumeSpec` | #### Raw escape hatch -`Raw()` returns the underlying `*corev1.PersistentVolumeSpec` for free-form editing when none of the structured methods are sufficient: +`Raw()` returns the underlying `*corev1.PersistentVolumeSpec` for free-form editing when none of the structured methods +are sufficient: ```go m.EditPVSpec(func(e *editors.PVSpecEditor) error { @@ -195,31 +207,34 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { The `Mutator` exposes convenience wrappers for the most common PV spec operations: -| Method | Equivalent to | -|------------------------------|--------------------------------------------------------| -| `SetStorageClassName(name)` | `EditPVSpec` → `e.SetStorageClassName(name)` | -| `SetReclaimPolicy(policy)` | `EditPVSpec` → `e.SetPersistentVolumeReclaimPolicy(p)` | -| `SetMountOptions(opts)` | `EditPVSpec` → `e.SetMountOptions(opts)` | +| Method | Equivalent to | +| --------------------------- | ------------------------------------------------------ | +| `SetStorageClassName(name)` | `EditPVSpec` → `e.SetStorageClassName(name)` | +| `SetReclaimPolicy(policy)` | `EditPVSpec` → `e.SetPersistentVolumeReclaimPolicy(p)` | +| `SetMountOptions(opts)` | `EditPVSpec` → `e.SetMountOptions(opts)` | -Use these for simple, single-operation mutations. Use `EditPVSpec` when you need multiple operations or raw access in a single edit block. +Use these for simple, single-operation mutations. Use `EditPVSpec` when you need multiple operations or raw access in a +single edit block. ## Operational Status -The PV primitive uses the Integration lifecycle. The default operational status handler maps PV phases to framework status: +The PV primitive uses the Integration lifecycle. The default operational status handler maps PV phases to framework +status: -| PV Phase | Operational Status | Meaning | -|-------------|----------------------|---------------------------------------| -| Available | Operational | PV is ready for binding | -| Bound | Operational | PV is bound to a PersistentVolumeClaim| -| Pending | OperationPending | PV is waiting to become available | -| Released | OperationFailing | PV was released, not yet reclaimed | -| Failed | OperationFailing | PV reclamation has failed | +| PV Phase | Operational Status | Meaning | +| --------- | ------------------ | -------------------------------------- | +| Available | Operational | PV is ready for binding | +| Bound | Operational | PV is bound to a PersistentVolumeClaim | +| Pending | OperationPending | PV is waiting to become available | +| Released | OperationFailing | PV was released, not yet reclaimed | +| Failed | OperationFailing | PV reclamation has failed | Override with `WithCustomOperationalStatus` when your PV requires different readiness logic. ## 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 @@ -233,7 +248,8 @@ resource, err := pv.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 := pv.NewBuilder(base). @@ -281,15 +297,26 @@ resource, err := pv.NewBuilder(base). ## Guidance -**PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs with a clear error. +**PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs +with a clear error. -**Immutable fields are preserved automatically.** The default field applicator detects existing PVs (by `ResourceVersion`) and preserves the volume source, volume mode, and claim ref. You do not need a custom applicator unless you require different preservation semantics. +**Immutable fields are preserved automatically.** The default field applicator detects existing PVs (by +`ResourceVersion`) and preserves the volume source, volume mode, and claim ref. You do not need a custom applicator +unless you require different preservation semantics. -**Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. +**Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based +on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. -**Controller references and garbage collection.** The component reconciliation pipeline attempts to set a controller reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be cluster-scoped. When the owner is namespace-scoped and the PV is cluster-scoped, the framework detects this mismatch and **skips setting `ownerReferences`** (logging an informational message) instead of letting the API server reject the request. As a result, such PVs will **not** be garbage collected automatically when the owning component is deleted. If you need garbage collection for PVs, either: +**Controller references and garbage collection.** The component reconciliation pipeline attempts to set a controller +reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be +cluster-scoped. When the owner is namespace-scoped and the PV is cluster-scoped, the framework detects this mismatch and +**skips setting `ownerReferences`** (logging an informational message) instead of letting the API server reject the +request. As a result, such PVs will **not** be garbage collected automatically when the owning component is deleted. If +you need garbage collection for PVs, either: -- Model the PV as owned by a dedicated **cluster-scoped** controller/component so a valid controller reference can be set, or -- Accept that PVs managed from a **namespace-scoped** component will not have `ownerReferences` and handle their lifecycle explicitly (for example, by deleting them in custom logic when appropriate). +- Model the PV as owned by a dedicated **cluster-scoped** controller/component so a valid controller reference can be + set, or +- Accept that PVs managed from a **namespace-scoped** component will not have `ownerReferences` and handle their + lifecycle explicitly (for example, by deleting them in custom logic when appropriate). **Register mutations in dependency order.** If mutation B relies on a field set by mutation A, register A first. diff --git a/examples/pv-primitive/README.md b/examples/pv-primitive/README.md index 282b6f16..7e0d589e 100644 --- a/examples/pv-primitive/README.md +++ b/examples/pv-primitive/README.md @@ -1,7 +1,7 @@ # PersistentVolume Primitive Example -This example demonstrates the usage of the `pv` primitive within the operator component framework. -It shows how to manage a Kubernetes PersistentVolume as a component of a larger application, utilising features like: +This example demonstrates the usage of the `pv` primitive within the operator component framework. It shows how to +manage a Kubernetes PersistentVolume as a component of a larger application, utilising features like: - **Base Construction**: Initializing a cluster-scoped PersistentVolume with storage configuration. - **Feature Mutations**: Applying feature-gated changes to reclaim policy, mount options, and metadata. @@ -10,9 +10,10 @@ It shows how to manage a Kubernetes PersistentVolume as a component of a larger ## 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, boolean-gated retain policy, and mount options mutations. + - `mutations.go`: version labelling, boolean-gated retain policy, and mount options mutations. - `resources/`: Contains the central `NewPVResource` factory that assembles all features using `pv.Builder`. - `main.go`: A standalone entry point that demonstrates in-memory mutation across multiple spec variations. @@ -23,6 +24,7 @@ go run examples/pv-primitive/main.go ``` This will: + 1. Create an `ExampleApp` owner object. 2. Apply mutations across four spec variations, printing the resulting PV YAML after each cycle. 3. Print operational status examples for each PV phase. From b64e1dc3a2059a07aebc6ebab2998fcad37f487c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 18:00:21 +0000 Subject: [PATCH 16/22] fix: do not initialize empty feature plan in PV mutator constructor Align PV mutator with configmap/deployment: NewMutator no longer creates an initial feature plan. BeginFeature must be called before registering mutations. Adds constructor and feature plan invariant tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pv/mutator.go | 8 ++-- pkg/primitives/pv/mutator_test.go | 63 +++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/pkg/primitives/pv/mutator.go b/pkg/primitives/pv/mutator.go index 1104d328..1962fb01 100644 --- a/pkg/primitives/pv/mutator.go +++ b/pkg/primitives/pv/mutator.go @@ -32,13 +32,11 @@ type Mutator struct { } // NewMutator creates a new Mutator for the given PersistentVolume. +// BeginFeature must be called before registering any mutations. func NewMutator(pv *corev1.PersistentVolume) *Mutator { - m := &Mutator{ - pv: pv, - plans: []featurePlan{{}}, + return &Mutator{ + pv: pv, } - m.active = &m.plans[0] - return m } // BeginFeature starts a new feature planning scope. All subsequent mutation diff --git a/pkg/primitives/pv/mutator_test.go b/pkg/primitives/pv/mutator_test.go index c82f4d51..e408fada 100644 --- a/pkg/primitives/pv/mutator_test.go +++ b/pkg/primitives/pv/mutator_test.go @@ -30,6 +30,7 @@ func newTestPV() *corev1.PersistentVolume { func TestMutator_EditObjectMetadata(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil @@ -41,6 +42,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { func TestMutator_EditObjectMetadata_Nil(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.EditObjectMetadata(nil) assert.NoError(t, m.Apply()) } @@ -50,6 +52,7 @@ func TestMutator_EditObjectMetadata_Nil(t *testing.T) { func TestMutator_EditPVSpec(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.EditPVSpec(func(e *editors.PVSpecEditor) error { e.SetStorageClassName("fast-ssd") return nil @@ -61,6 +64,7 @@ func TestMutator_EditPVSpec(t *testing.T) { func TestMutator_EditPVSpec_Nil(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.EditPVSpec(nil) assert.NoError(t, m.Apply()) } @@ -68,6 +72,7 @@ func TestMutator_EditPVSpec_Nil(t *testing.T) { func TestMutator_EditPVSpec_RawAccess(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.EditPVSpec(func(e *editors.PVSpecEditor) error { e.Raw().PersistentVolumeReclaimPolicy = corev1.PersistentVolumeReclaimRetain return nil @@ -81,6 +86,7 @@ func TestMutator_EditPVSpec_RawAccess(t *testing.T) { func TestMutator_SetStorageClassName(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.SetStorageClassName("premium") require.NoError(t, m.Apply()) assert.Equal(t, "premium", pv.Spec.StorageClassName) @@ -89,6 +95,7 @@ func TestMutator_SetStorageClassName(t *testing.T) { func TestMutator_SetReclaimPolicy(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.SetReclaimPolicy(corev1.PersistentVolumeReclaimDelete) require.NoError(t, m.Apply()) assert.Equal(t, corev1.PersistentVolumeReclaimDelete, pv.Spec.PersistentVolumeReclaimPolicy) @@ -97,6 +104,7 @@ func TestMutator_SetReclaimPolicy(t *testing.T) { func TestMutator_SetMountOptions(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() opts := []string{"hard", "nfsvers=4.1"} m.SetMountOptions(opts) require.NoError(t, m.Apply()) @@ -109,6 +117,7 @@ func TestMutator_OperationOrder(t *testing.T) { // Within a feature: metadata edits run before spec edits. pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() // Register in reverse logical order to confirm Apply() enforces category ordering. m.SetStorageClassName("standard") m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { @@ -124,6 +133,7 @@ func TestMutator_OperationOrder(t *testing.T) { func TestMutator_MultipleFeatures(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.SetStorageClassName("feature1-class") m.BeginFeature() m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) @@ -136,6 +146,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { func TestMutator_LaterFeatureObservesPrior(t *testing.T) { pv := newTestPV() m := NewMutator(pv) + m.BeginFeature() m.SetStorageClassName("first") m.BeginFeature() // Second feature should see the storage class set by the first. @@ -150,6 +161,58 @@ func TestMutator_LaterFeatureObservesPrior(t *testing.T) { assert.Equal(t, "first-seen", pv.Spec.StorageClassName) } +// --- Constructor and feature plan invariants --- + +func TestNewMutator_InitializesNoPlan(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + + 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) { + pv := newTestPV() + m := NewMutator(pv) + + 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) { + pv := newTestPV() + m := NewMutator(pv) + + // Record a mutation in the first feature plan + m.BeginFeature() + m.SetStorageClassName("class0") + + // Start a new feature and record a different mutation + m.BeginFeature() + m.SetReclaimPolicy(corev1.PersistentVolumeReclaimRetain) + + // The initial plan should have exactly one spec edit + assert.Len(t, m.plans[0].specEdits, 1, "initial plan should have one edit") + // The second plan should also have exactly one spec edit + assert.Len(t, m.plans[1].specEdits, 1, "second plan should have one edit") +} + +func TestMutator_SingleFeature_PlanCount(t *testing.T) { + pv := newTestPV() + m := NewMutator(pv) + m.BeginFeature() + m.SetStorageClassName("standard") + + require.NoError(t, m.Apply()) + assert.Len(t, m.plans, 1, "no extra plans should be created during Apply") + assert.Equal(t, "standard", pv.Spec.StorageClassName) +} + // --- ObjectMutator interface --- func TestMutator_ImplementsObjectMutator(_ *testing.T) { From 34983d4e9199795b8c273cac76c9f9ffaeb1c32e 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:34 +0000 Subject: [PATCH 17/22] refactor: remove field applicators and flavors from PV 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 NewIntegrationBuilder signature (no applicator parameter). Rewrite resource tests to use the Object()-then-Mutate pattern. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 76 +------ pkg/primitives/pv/builder.go | 42 +--- pkg/primitives/pv/builder_test.go | 32 --- pkg/primitives/pv/flavors.go | 25 -- pkg/primitives/pv/flavors_test.go | 128 ----------- pkg/primitives/pv/resource.go | 40 +--- pkg/primitives/pv/resource_test.go | 351 +++++++++-------------------- 7 files changed, 117 insertions(+), 577 deletions(-) delete mode 100644 pkg/primitives/pv/flavors.go delete mode 100644 pkg/primitives/pv/flavors_test.go diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index e0dd99ff..e58708c6 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -6,14 +6,12 @@ and object metadata. ## Capabilities -| Capability | Detail | -| -------------------------- | ----------------------------------------------------------------------------------------------------------- | -| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | -| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | -| **Immutable preservation** | Default field applicator preserves immutable fields (volume source, volume mode, claim ref) on existing PVs | -| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed labels and annotations not owned by the operator | -| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | +| Capability | Detail | +| ------------------------- | -------------------------------------------------------------------------------------------------------- | +| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | +| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | +| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | +| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | ## Building a PersistentVolume Primitive @@ -39,7 +37,6 @@ base := &corev1.PersistentVolume{ } resource, err := pv.NewBuilder(base). - WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). WithMutation(MyFeatureMutation(owner.Spec.Version)). Build() ``` @@ -47,34 +44,6 @@ resource, err := pv.NewBuilder(base). PersistentVolumes are cluster-scoped. The builder validates that Name is set and that Namespace is empty. Setting a namespace on the PV object will cause `Build()` to return an error. -## Default Field Application - -`DefaultFieldApplicator` replaces the current PersistentVolume with a deep copy of the desired object, then restores -server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), the Status -subresource, and PV-specific immutable fields from the original live object. This prevents spec-level reconciliation -from clearing status data written by the API server or other controllers. - -Preserved immutable fields: - -- `Spec.PersistentVolumeSource` — the volume backend (NFS, CSI, HostPath, etc.) -- `Spec.VolumeMode` — Filesystem or Block -- `Spec.ClaimRef` — the binding to a PersistentVolumeClaim - -On initial creation (empty `ResourceVersion`), the entire desired state is applied without preservation. - -Use `WithCustomFieldApplicator` when you need different preservation semantics: - -```go -resource, err := pv.NewBuilder(base). - WithCustomFieldApplicator(func(current, desired *corev1.PersistentVolume) error { - // Only update specific fields. - current.Spec.StorageClassName = desired.Spec.StorageClassName - current.Spec.MountOptions = desired.Spec.MountOptions - return nil - }). - Build() -``` - ## Mutations Mutations are the primary mechanism for modifying a `PersistentVolume` beyond its baseline. Each mutation is a named @@ -231,34 +200,6 @@ status: Override with `WithCustomOperationalStatus` when your PV requires different readiness logic. -## 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 := pv.NewBuilder(base). - WithFieldApplicationFlavor(pv.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 := pv.NewBuilder(base). - WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). - Build() -``` - -Multiple flavors can be registered and run in registration order. - ## Full Example: Storage-Tier PersistentVolume ```go @@ -289,7 +230,6 @@ func TierLabelMutation(version, tier string) pv.Mutation { } resource, err := pv.NewBuilder(base). - WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations). WithMutation(StorageClassMutation(owner.Spec.Version)). WithMutation(TierLabelMutation(owner.Spec.Version, "premium")). Build() @@ -300,10 +240,6 @@ resource, err := pv.NewBuilder(base). **PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs with a clear error. -**Immutable fields are preserved automatically.** The default field applicator detects existing PVs (by -`ResourceVersion`) and preserves the volume source, volume mode, and claim ref. You do not need a custom applicator -unless you require different preservation semantics. - **Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. diff --git a/pkg/primitives/pv/builder.go b/pkg/primitives/pv/builder.go index af4e7e82..c22e7d8e 100644 --- a/pkg/primitives/pv/builder.go +++ b/pkg/primitives/pv/builder.go @@ -12,10 +12,9 @@ import ( // Builder is a configuration helper for creating and customizing a PersistentVolume Resource. // -// It provides a fluent API for registering mutations, field application flavors, -// operational status handlers, 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, operational status handlers, +// and data extractors. Build() validates the configuration and returns an +// initialized Resource ready for use in a reconciliation loop. type Builder struct { base *generic.IntegrationBuilder[*corev1.PersistentVolume, *Mutator] } @@ -24,7 +23,7 @@ type Builder struct { // // The PersistentVolume 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. // // PersistentVolumes are cluster-scoped; the provided object must have a Name set // but must not have a Namespace. This is validated during the Build() call. @@ -36,7 +35,6 @@ func NewBuilder(pv *corev1.PersistentVolume) *Builder { base := generic.NewIntegrationBuilder[*corev1.PersistentVolume, *Mutator]( pv, identityFunc, - DefaultFieldApplicator, NewMutator, ) base.MarkClusterScoped() @@ -56,36 +54,6 @@ func (b *Builder) WithMutation(m Mutation) *Builder { return b } -// WithCustomFieldApplicator sets a custom strategy for applying the desired -// state to the existing PersistentVolume in the cluster. -// -// The default applicator (DefaultFieldApplicator) preserves immutable fields -// (volume source, volume mode, claim ref) on existing PVs while replacing all -// other fields from the desired state. Use a custom applicator when you need -// different preservation semantics. -// -// 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 *corev1.PersistentVolume) 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[*corev1.PersistentVolume](flavor)) - return b -} - // WithCustomOperationalStatus overrides the default logic for determining if the // PersistentVolume is operationally ready. // @@ -121,7 +89,7 @@ func (b *Builder) WithDataExtractor(extractor func(corev1.PersistentVolume) erro // - No PersistentVolume object was provided. // - The PersistentVolume is missing a Name. // - The PersistentVolume has a Namespace set (PVs are cluster-scoped). -// - Identity function, field applicator, or mutator factory is nil. +// - Identity function or mutator factory is nil. func (b *Builder) Build() (*Resource, error) { genericRes, err := b.base.Build() if err != nil { diff --git a/pkg/primitives/pv/builder_test.go b/pkg/primitives/pv/builder_test.go index 294cc0a3..01cec72a 100644 --- a/pkg/primitives/pv/builder_test.go +++ b/pkg/primitives/pv/builder_test.go @@ -75,38 +75,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() - pv := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, - } - called := false - applicator := func(_, _ *corev1.PersistentVolume) error { - called = true - return nil - } - res, err := NewBuilder(pv). - 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() - pv := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{Name: "test-pv"}, - } - res, err := NewBuilder(pv). - WithFieldApplicationFlavor(PreserveCurrentLabels). - WithFieldApplicationFlavor(nil). // nil must be ignored - Build() - require.NoError(t, err) - assert.Len(t, res.base.FieldFlavors, 1) -} - func TestBuilder_WithCustomOperationalStatus(t *testing.T) { t.Parallel() pv := &corev1.PersistentVolume{ diff --git a/pkg/primitives/pv/flavors.go b/pkg/primitives/pv/flavors.go deleted file mode 100644 index 10238507..00000000 --- a/pkg/primitives/pv/flavors.go +++ /dev/null @@ -1,25 +0,0 @@ -package pv - -import ( - "github.com/sourcehawk/operator-component-framework/pkg/flavors" - corev1 "k8s.io/api/core/v1" -) - -// FieldApplicationFlavor defines a function signature for applying flavors to a -// PersistentVolume 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[*corev1.PersistentVolume] - -// PreserveCurrentLabels ensures that any labels present on the current live -// PersistentVolume but missing from the applied (desired) object are preserved. -// If a label exists in both, the applied value wins. -func PreserveCurrentLabels(applied, current, desired *corev1.PersistentVolume) error { - return flavors.PreserveCurrentLabels[*corev1.PersistentVolume]()(applied, current, desired) -} - -// PreserveCurrentAnnotations ensures that any annotations present on the current -// live PersistentVolume but missing from the applied (desired) object are preserved. -// If an annotation exists in both, the applied value wins. -func PreserveCurrentAnnotations(applied, current, desired *corev1.PersistentVolume) error { - return flavors.PreserveCurrentAnnotations[*corev1.PersistentVolume]()(applied, current, desired) -} diff --git a/pkg/primitives/pv/flavors_test.go b/pkg/primitives/pv/flavors_test.go deleted file mode 100644 index f2c11706..00000000 --- a/pkg/primitives/pv/flavors_test.go +++ /dev/null @@ -1,128 +0,0 @@ -package pv - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestPreserveCurrentLabels(t *testing.T) { - t.Run("adds missing labels from current", func(t *testing.T) { - applied := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} - current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} - current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{} - current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} - current := &corev1.PersistentVolume{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 := &corev1.PersistentVolume{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} - current := &corev1.PersistentVolume{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) { - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - Labels: map[string]string{"app": "desired"}, - }, - Spec: corev1.PersistentVolumeSpec{ - Capacity: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - PersistentVolumeSource: corev1.PersistentVolumeSource{ - HostPath: &corev1.HostPathVolumeSource{Path: "/data"}, - }, - }, - } - - t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(PreserveCurrentLabels). - Build() - require.NoError(t, err) - - current := &corev1.PersistentVolume{ - 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(_, _, _ *corev1.PersistentVolume) error { - order = append(order, "flavor1") - return nil - } - flavor2 := func(_, _, _ *corev1.PersistentVolume) 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(&corev1.PersistentVolume{})) - 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(_, _, _ *corev1.PersistentVolume) error { - return flavorErr - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&corev1.PersistentVolume{}) - require.Error(t, err) - assert.True(t, errors.Is(err, flavorErr)) - }) -} diff --git a/pkg/primitives/pv/resource.go b/pkg/primitives/pv/resource.go index e0531075..0a760f96 100644 --- a/pkg/primitives/pv/resource.go +++ b/pkg/primitives/pv/resource.go @@ -7,39 +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), the Status -// subresource, and PV-specific immutable fields (PersistentVolumeSource, -// VolumeMode, ClaimRef) from the original current object. -// -// On a fresh PV (empty ResourceVersion, meaning it has not yet been persisted), -// the full desired state is applied without preservation. -func DefaultFieldApplicator(current, desired *corev1.PersistentVolume) error { - if current.ResourceVersion == "" { - // First creation — apply everything from desired. - *current = *desired.DeepCopy() - return nil - } - - // Snapshot immutable fields before overwriting. - savedSource := current.Spec.PersistentVolumeSource - savedVolumeMode := current.Spec.VolumeMode - savedClaimRef := current.Spec.ClaimRef - - original := current.DeepCopy() - *current = *desired.DeepCopy() - generic.PreserveServerManagedFields(current, original) - generic.PreserveStatus(current, original) - - // Restore PV-specific immutable fields from the live object. - current.Spec.PersistentVolumeSource = savedSource - current.Spec.VolumeMode = savedVolumeMode - current.Spec.ClaimRef = savedClaimRef - - return nil -} - // Resource is a high-level abstraction for managing a Kubernetes PersistentVolume // within a controller's reconciliation loop. // @@ -73,11 +40,8 @@ func (r *Resource) Object() (client.Object, error) { // Mutate transforms the current state of a Kubernetes PersistentVolume 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. -// The default applicator preserves immutable fields on existing PVs. -// 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/pv/resource_test.go b/pkg/primitives/pv/resource_test.go index 4534cd34..c26fcb70 100644 --- a/pkg/primitives/pv/resource_test.go +++ b/pkg/primitives/pv/resource_test.go @@ -1,305 +1,162 @@ package pv import ( + "errors" "testing" + "github.com/sourcehawk/operator-component-framework/pkg/component/concepts" + "github.com/sourcehawk/operator-component-framework/pkg/feature" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) -func TestDefaultFieldApplicator_Create(t *testing.T) { - current := &corev1.PersistentVolume{ +func newValidPV() *corev1.PersistentVolume { + return &corev1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "test-pv", }, - } - volumeMode := corev1.PersistentVolumeFilesystem - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - Labels: map[string]string{"app": "myapp"}, - }, Spec: corev1.PersistentVolumeSpec{ Capacity: corev1.ResourceList{ corev1.ResourceStorage: resource.MustParse("10Gi"), }, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "nfs.example.com", - Path: "/exports/data", - }, + HostPath: &corev1.HostPathVolumeSource{Path: "/data"}, }, - VolumeMode: &volumeMode, - StorageClassName: "fast-ssd", - PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, }, } +} - err := DefaultFieldApplicator(current, desired) +func TestResource_Identity(t *testing.T) { + res, err := NewBuilder(newValidPV()).Build() require.NoError(t, err) - - // On create (empty ResourceVersion), all fields should be applied. - assert.Equal(t, "myapp", current.Labels["app"]) - expectedCapacity := resource.MustParse("10Gi") - actualCapacity := current.Spec.Capacity[corev1.ResourceStorage] - assert.True(t, expectedCapacity.Equal(actualCapacity), "expected storage capacity %s, got %s", expectedCapacity.String(), actualCapacity.String()) - assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server) - assert.Equal(t, &volumeMode, current.Spec.VolumeMode) - assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) - assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy) + assert.Equal(t, "v1/PersistentVolume/test-pv", res.Identity()) } -func TestDefaultFieldApplicator_Update_PreservesImmutableFields(t *testing.T) { - existingVolumeMode := corev1.PersistentVolumeBlock - claimRef := &corev1.ObjectReference{ - Namespace: "default", - Name: "my-pvc", - } +func TestResource_Object(t *testing.T) { + pv := newValidPV() + res, err := NewBuilder(pv).Build() + require.NoError(t, err) - current := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - ResourceVersion: "12345", - }, - Spec: corev1.PersistentVolumeSpec{ - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "nfs.example.com", - Path: "/exports/data", - }, - }, - VolumeMode: &existingVolumeMode, - ClaimRef: claimRef, - Capacity: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - StorageClassName: "old-class", - PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, - }, - } + obj, err := res.Object() + require.NoError(t, err) - desiredVolumeMode := corev1.PersistentVolumeFilesystem - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - Labels: map[string]string{"updated": "true"}, - }, - Spec: corev1.PersistentVolumeSpec{ - PersistentVolumeSource: corev1.PersistentVolumeSource{ - HostPath: &corev1.HostPathVolumeSource{ - Path: "/mnt/data", - }, - }, - VolumeMode: &desiredVolumeMode, - ClaimRef: &corev1.ObjectReference{ - Namespace: "other", - Name: "other-pvc", - }, - Capacity: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("20Gi"), - }, - StorageClassName: "new-class", - PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, - }, - } + got, ok := obj.(*corev1.PersistentVolume) + require.True(t, ok) + assert.Equal(t, pv.Name, got.Name) - err := DefaultFieldApplicator(current, desired) + // Must be a deep copy. + got.Name = "changed" + assert.Equal(t, "test-pv", pv.Name) +} + +func TestResource_Mutate(t *testing.T) { + desired := newValidPV() + res, err := NewBuilder(desired).Build() require.NoError(t, err) - // Immutable fields should be preserved from the existing object. - assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server, - "PersistentVolumeSource should be preserved on update") - assert.Nil(t, current.Spec.HostPath, - "desired volume source should not overwrite existing") - assert.Equal(t, &existingVolumeMode, current.Spec.VolumeMode, - "VolumeMode should be preserved on update") - assert.Equal(t, "my-pvc", current.Spec.ClaimRef.Name, - "ClaimRef should be preserved on update") + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - // Mutable fields should be updated from desired. - assert.Equal(t, "true", current.Labels["updated"], - "labels should be updated from desired") - expectedUpdatedCapacity := resource.MustParse("20Gi") - actualUpdatedCapacity := current.Spec.Capacity[corev1.ResourceStorage] - assert.True(t, expectedUpdatedCapacity.Equal(actualUpdatedCapacity), - "capacity should be updated from desired: expected %s, got %s", expectedUpdatedCapacity.String(), actualUpdatedCapacity.String()) - assert.Equal(t, "new-class", current.Spec.StorageClassName, - "storage class should be updated from desired") - assert.Equal(t, corev1.PersistentVolumeReclaimRetain, current.Spec.PersistentVolumeReclaimPolicy, - "reclaim policy should be updated from desired") + got := obj.(*corev1.PersistentVolume) + expectedCapacity := resource.MustParse("10Gi") + actualCapacity := got.Spec.Capacity[corev1.ResourceStorage] + assert.True(t, expectedCapacity.Equal(actualCapacity)) + assert.Equal(t, "/data", got.Spec.HostPath.Path) } -func TestDefaultFieldApplicator_Update_NilImmutableFields(t *testing.T) { - // When existing object has nil immutable fields, they should stay nil - // (not be overwritten by desired values). - current := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - ResourceVersion: "1", - }, - Spec: corev1.PersistentVolumeSpec{}, - } - - volumeMode := corev1.PersistentVolumeFilesystem - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - }, - Spec: corev1.PersistentVolumeSpec{ - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "nfs.example.com", - Path: "/data", - }, - }, - VolumeMode: &volumeMode, - ClaimRef: &corev1.ObjectReference{ - Name: "some-pvc", +func TestResource_Mutate_WithMutation(t *testing.T) { + desired := newValidPV() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "set-storage-class", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageClassName("fast-ssd") + return nil }, - }, - } + }). + Build() + require.NoError(t, err) - err := DefaultFieldApplicator(current, desired) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - // Immutable fields from current (all zero/nil) should be restored. - assert.Nil(t, current.Spec.NFS, - "nil volume source should be preserved") - assert.Nil(t, current.Spec.VolumeMode, - "nil volume mode should be preserved") - assert.Nil(t, current.Spec.ClaimRef, - "nil claim ref should be preserved") + got := obj.(*corev1.PersistentVolume) + assert.Equal(t, "fast-ssd", got.Spec.StorageClassName) + assert.Equal(t, "/data", got.Spec.HostPath.Path) } -func TestDefaultFieldApplicator_DoesNotMutateDesired(t *testing.T) { - current := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - ResourceVersion: "1", - }, - Spec: corev1.PersistentVolumeSpec{ - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "existing.example.com", - Path: "/old", - }, +func TestResource_Mutate_FeatureOrdering(t *testing.T) { + desired := newValidPV() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "feature-a", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageClassName("a") + return nil }, - }, - } - - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - }, - Spec: corev1.PersistentVolumeSpec{ - StorageClassName: "desired-class", - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "desired.example.com", - Path: "/new", - }, + }). + WithMutation(Mutation{ + Name: "feature-b", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageClassName("b") + return nil }, - }, - } + }). + Build() + require.NoError(t, err) - err := DefaultFieldApplicator(current, desired) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - // Desired object must not be mutated. - assert.Equal(t, "desired.example.com", desired.Spec.NFS.Server) - assert.Equal(t, "desired-class", desired.Spec.StorageClassName) + got := obj.(*corev1.PersistentVolume) + assert.Equal(t, "b", got.Spec.StorageClassName) } -func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { - current := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - ResourceVersion: "12345", - }, - Status: corev1.PersistentVolumeStatus{ - Phase: corev1.VolumeBound, - Message: "bound to claim default/data", - Reason: "Bound", - }, - } - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - }, - Spec: corev1.PersistentVolumeSpec{ - StorageClassName: "fast-ssd", - }, - } - - err := DefaultFieldApplicator(current, desired) +func TestResource_ConvergingStatus(t *testing.T) { + desired := newValidPV() + res, err := NewBuilder(desired).Build() require.NoError(t, err) - // Desired spec is applied - assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) - - // Status from the live object is preserved - assert.Equal(t, corev1.VolumeBound, current.Status.Phase) - assert.Equal(t, "bound to claim default/data", current.Status.Message) - assert.Equal(t, "Bound", current.Status.Reason) + status, err := res.ConvergingStatus(concepts.ConvergingOperationNone) + require.NoError(t, err) + // Default handler on a PV with no phase set returns OperationPending. + assert.Equal(t, concepts.OperationalStatusPending, status.Status) } -func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { - existingVolumeMode := corev1.PersistentVolumeBlock - current := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - ResourceVersion: "12345", - UID: types.UID("abc-def"), - Generation: 3, - OwnerReferences: []metav1.OwnerReference{ - {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, - }, - Finalizers: []string{"finalizer.example.com"}, - }, - Spec: corev1.PersistentVolumeSpec{ - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: "nfs.example.com", - Path: "/exports/data", - }, - }, - VolumeMode: &existingVolumeMode, - }, - } - desired := &corev1.PersistentVolume{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pv", - Labels: map[string]string{"app": "test"}, - }, - Spec: corev1.PersistentVolumeSpec{ - StorageClassName: "fast-ssd", - PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, - }, - } - - err := DefaultFieldApplicator(current, desired) +func TestResource_ExtractData(t *testing.T) { + pv := newValidPV() + var extracted string + res, err := NewBuilder(pv). + WithDataExtractor(func(p corev1.PersistentVolume) error { + extracted = p.Spec.HostPath.Path + return nil + }). + Build() require.NoError(t, err) - // Desired spec and labels are applied - assert.Equal(t, "fast-ssd", current.Spec.StorageClassName) - assert.Equal(t, "test", current.Labels["app"]) - - // 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) + require.NoError(t, res.ExtractData()) + assert.Equal(t, "/data", extracted) +} - // 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) +func TestResource_ExtractData_Error(t *testing.T) { + res, err := NewBuilder(newValidPV()). + WithDataExtractor(func(_ corev1.PersistentVolume) error { + return errors.New("extract error") + }). + Build() + require.NoError(t, err) - // PV-specific immutable fields are preserved - assert.Equal(t, "nfs.example.com", current.Spec.NFS.Server) - assert.Equal(t, &existingVolumeMode, current.Spec.VolumeMode) + err = res.ExtractData() + require.Error(t, err) + assert.Contains(t, err.Error(), "extract error") } From 8f143cf3bdb01427fc722418e18c764f0288b460 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:46:33 +0000 Subject: [PATCH 18/22] fix: remove references to deleted WithFieldApplicationFlavor and PreserveCurrentAnnotations The pv-primitive example still referenced these APIs which were removed in the field applicators refactor, causing the lint CI to fail. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pv-primitive/resources/pv.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/examples/pv-primitive/resources/pv.go b/examples/pv-primitive/resources/pv.go index 04fd9d58..e30bb41d 100644 --- a/examples/pv-primitive/resources/pv.go +++ b/examples/pv-primitive/resources/pv.go @@ -45,9 +45,6 @@ func NewPVResource(owner *sharedapp.ExampleApp) (*pv.Resource, error) { builder.WithMutation(features.RetainPolicyMutation(owner.Spec.Version, owner.Spec.EnableTracing)) builder.WithMutation(features.MountOptionsMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) - // 4. Preserve annotations added by external controllers. - builder.WithFieldApplicationFlavor(pv.PreserveCurrentAnnotations) - - // 5. Build the final resource. + // 4. Build the final resource. return builder.Build() } From 32d81f4729890460e4959fb7dad5291ee7a2f004 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:53:01 +0000 Subject: [PATCH 19/22] fix: format pv.md to pass prettier lint check Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index e58708c6..d768da7a 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -6,12 +6,12 @@ and object metadata. ## Capabilities -| Capability | Detail | -| ------------------------- | -------------------------------------------------------------------------------------------------------- | -| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | -| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | -| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | -| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | +| Capability | Detail | +| ------------------------- | -------------------------------------------------------------------------------------------------- | +| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | +| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | +| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | +| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | ## Building a PersistentVolume Primitive From db71071dd9829c35b0120b5618f53717de197b7d 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:04:39 +0000 Subject: [PATCH 20/22] fix: correct misleading comment about flavor-based annotation preservation The comment claimed annotations were preserved "by flavor", but the pv primitive has no flavor support. Annotations are preserved because the selective mutation pattern only modifies explicitly targeted fields. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pv-primitive/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/pv-primitive/main.go b/examples/pv-primitive/main.go index a6c84006..f1099971 100644 --- a/examples/pv-primitive/main.go +++ b/examples/pv-primitive/main.go @@ -72,7 +72,7 @@ func main() { Name: owner.Name + "-data", ResourceVersion: "12345", // non-empty to simulate existing object Annotations: map[string]string{ - "external-controller/managed": "true", // should be preserved by flavor + "external-controller/managed": "true", // preserved: mutations only touch fields they explicitly target }, }, Spec: corev1.PersistentVolumeSpec{ From 175fd91368e375d624a3486ece228d4e50c3dc74 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:14:33 +0000 Subject: [PATCH 21/22] fix: align operational status names in docs and comments with code Update documentation and handler comments to use the actual concepts.OperationalStatusPending / concepts.OperationalStatusFailing / concepts.OperationalStatusOperational identifiers instead of the shorthand OperationPending / OperationFailing / Operational names. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pv.md | 14 +++++++------- pkg/primitives/pv/handlers.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index d768da7a..1a957d66 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -8,7 +8,7 @@ and object metadata. | Capability | Detail | | ------------------------- | -------------------------------------------------------------------------------------------------- | -| **Integration lifecycle** | Reports `Operational`, `OperationPending`, or `OperationFailing` based on the PV's phase | +| **Integration lifecycle** | Reports `concepts.OperationalStatusOperational`, `concepts.OperationalStatusPending`, or `concepts.OperationalStatusFailing` based on the PV's phase | | **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | | **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | | **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | @@ -192,11 +192,11 @@ status: | PV Phase | Operational Status | Meaning | | --------- | ------------------ | -------------------------------------- | -| Available | Operational | PV is ready for binding | -| Bound | Operational | PV is bound to a PersistentVolumeClaim | -| Pending | OperationPending | PV is waiting to become available | -| Released | OperationFailing | PV was released, not yet reclaimed | -| Failed | OperationFailing | PV reclamation has failed | +| Available | OperationalStatusOperational | PV is ready for binding | +| Bound | OperationalStatusOperational | PV is bound to a PersistentVolumeClaim | +| Pending | OperationalStatusPending | PV is waiting to become available | +| Released | OperationalStatusFailing | PV was released, not yet reclaimed | +| Failed | OperationalStatusFailing | PV reclamation has failed | Override with `WithCustomOperationalStatus` when your PV requires different readiness logic. @@ -240,7 +240,7 @@ resource, err := pv.NewBuilder(base). **PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs with a clear error. -**Use the Integration lifecycle for status.** PVs report `Operational`, `OperationPending`, or `OperationFailing` based +**Use the Integration lifecycle for status.** PVs report `OperationalStatusOperational`, `OperationalStatusPending`, or `OperationalStatusFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. **Controller references and garbage collection.** The component reconciliation pipeline attempts to set a controller diff --git a/pkg/primitives/pv/handlers.go b/pkg/primitives/pv/handlers.go index f08b7269..a078a2cc 100644 --- a/pkg/primitives/pv/handlers.go +++ b/pkg/primitives/pv/handlers.go @@ -11,8 +11,8 @@ import ( // is operationally ready. // // It considers a PV operational when its Status.Phase is Available or Bound. -// A PV in the Pending phase is reported as OperationPending. A PV in the -// Released or Failed phase is reported as OperationFailing. +// A PV in the Pending phase is reported as concepts.OperationalStatusPending. A PV in the +// Released or Failed phase is reported as concepts.OperationalStatusFailing. // // This function is used as the default handler by the Resource if no custom handler // is registered via Builder.WithCustomOperationalStatus. It can be reused within From 7c6589c59595c9492faab06097b659fededd1a4c 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:47:58 +0000 Subject: [PATCH 22/22] fix docs --- docs/primitives/pv.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/primitives/pv.md b/docs/primitives/pv.md index 1a957d66..2b2984b4 100644 --- a/docs/primitives/pv.md +++ b/docs/primitives/pv.md @@ -6,12 +6,12 @@ and object metadata. ## Capabilities -| Capability | Detail | -| ------------------------- | -------------------------------------------------------------------------------------------------- | +| Capability | Detail | +| ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- | | **Integration lifecycle** | Reports `concepts.OperationalStatusOperational`, `concepts.OperationalStatusPending`, or `concepts.OperationalStatusFailing` based on the PV's phase | -| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | -| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | -| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | +| **Cluster-scoped** | No namespace in the identity or builder — PersistentVolumes are cluster-scoped resources | +| **Mutation pipeline** | Typed editors for PV spec fields and object metadata, with a raw escape hatch for free-form access | +| **Data extraction** | Reads generated or updated values back from the reconciled PersistentVolume after each sync cycle | ## Building a PersistentVolume Primitive @@ -190,13 +190,13 @@ single edit block. The PV primitive uses the Integration lifecycle. The default operational status handler maps PV phases to framework status: -| PV Phase | Operational Status | Meaning | -| --------- | ------------------ | -------------------------------------- | +| PV Phase | Operational Status | Meaning | +| --------- | ---------------------------- | -------------------------------------- | | Available | OperationalStatusOperational | PV is ready for binding | | Bound | OperationalStatusOperational | PV is bound to a PersistentVolumeClaim | -| Pending | OperationalStatusPending | PV is waiting to become available | -| Released | OperationalStatusFailing | PV was released, not yet reclaimed | -| Failed | OperationalStatusFailing | PV reclamation has failed | +| Pending | OperationalStatusPending | PV is waiting to become available | +| Released | OperationalStatusFailing | PV was released, not yet reclaimed | +| Failed | OperationalStatusFailing | PV reclamation has failed | Override with `WithCustomOperationalStatus` when your PV requires different readiness logic. @@ -240,8 +240,9 @@ resource, err := pv.NewBuilder(base). **PersistentVolumes are cluster-scoped.** Do not set a namespace on the PV object. The builder rejects namespaced PVs with a clear error. -**Use the Integration lifecycle for status.** PVs report `OperationalStatusOperational`, `OperationalStatusPending`, or `OperationalStatusFailing` based -on their phase. Override with `WithCustomOperationalStatus` only when phase-based readiness is insufficient. +**Use the Integration lifecycle for status.** PVs report `OperationalStatusOperational`, `OperationalStatusPending`, or +`OperationalStatusFailing` based on their phase. Override with `WithCustomOperationalStatus` only when phase-based +readiness is insufficient. **Controller references and garbage collection.** The component reconciliation pipeline attempts to set a controller reference on created/updated resources. Because `PersistentVolume` is cluster-scoped, its controller owner must also be