From 979a02ec7d66660e9197ec26869db482c3458816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:42:08 +0000 Subject: [PATCH 01/21] Add PVCSpecEditor for PersistentVolumeClaim spec mutations Typed editor providing SetStorageRequest, SetAccessModes, SetStorageClassName, SetVolumeMode, SetVolumeName, and Raw() escape hatch. Follows the existing editor patterns (DeploymentSpecEditor, ContainerEditor, etc.). Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mutation/editors/pvcspec.go | 66 +++++++++++++++++++++++++ pkg/mutation/editors/pvcspec_test.go | 72 ++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 pkg/mutation/editors/pvcspec.go create mode 100644 pkg/mutation/editors/pvcspec_test.go diff --git a/pkg/mutation/editors/pvcspec.go b/pkg/mutation/editors/pvcspec.go new file mode 100644 index 00000000..6d8a763a --- /dev/null +++ b/pkg/mutation/editors/pvcspec.go @@ -0,0 +1,66 @@ +package editors + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +// PVCSpecEditor provides a typed API for mutating a Kubernetes PersistentVolumeClaimSpec. +type PVCSpecEditor struct { + spec *corev1.PersistentVolumeClaimSpec +} + +// NewPVCSpecEditor creates a new PVCSpecEditor for the given PersistentVolumeClaimSpec. +func NewPVCSpecEditor(spec *corev1.PersistentVolumeClaimSpec) *PVCSpecEditor { + return &PVCSpecEditor{spec: spec} +} + +// Raw returns the underlying *corev1.PersistentVolumeClaimSpec. +// +// This is an escape hatch for cases where the typed API is insufficient. +func (e *PVCSpecEditor) Raw() *corev1.PersistentVolumeClaimSpec { + return e.spec +} + +// SetStorageRequest sets the storage resource request for the PVC. +// +// On existing PVCs, Kubernetes only allows storage expansion (not shrinking), +// provided the StorageClass supports volume expansion. +func (e *PVCSpecEditor) SetStorageRequest(quantity resource.Quantity) { + if e.spec.Resources.Requests == nil { + e.spec.Resources.Requests = corev1.ResourceList{} + } + e.spec.Resources.Requests[corev1.ResourceStorage] = quantity +} + +// SetAccessModes sets the access modes for the PVC. +// +// Access modes are immutable on existing PVCs. This method is intended for +// initial construction; the DefaultFieldApplicator preserves existing values. +func (e *PVCSpecEditor) SetAccessModes(modes []corev1.PersistentVolumeAccessMode) { + e.spec.AccessModes = modes +} + +// SetStorageClassName sets the storage class name for the PVC. +// +// The storage class name is immutable on existing PVCs. This method is intended +// for initial construction; the DefaultFieldApplicator preserves existing values. +func (e *PVCSpecEditor) SetStorageClassName(name string) { + e.spec.StorageClassName = &name +} + +// SetVolumeMode sets the volume mode (Filesystem or Block) for the PVC. +// +// The volume mode is immutable on existing PVCs. This method is intended for +// initial construction; the DefaultFieldApplicator preserves existing values. +func (e *PVCSpecEditor) SetVolumeMode(mode corev1.PersistentVolumeMode) { + e.spec.VolumeMode = &mode +} + +// SetVolumeName binds the PVC to a specific PersistentVolume by name. +// +// The volume name is immutable on existing PVCs. This method is intended for +// initial construction; the DefaultFieldApplicator preserves existing values. +func (e *PVCSpecEditor) SetVolumeName(name string) { + e.spec.VolumeName = name +} diff --git a/pkg/mutation/editors/pvcspec_test.go b/pkg/mutation/editors/pvcspec_test.go new file mode 100644 index 00000000..c505fb99 --- /dev/null +++ b/pkg/mutation/editors/pvcspec_test.go @@ -0,0 +1,72 @@ +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 TestPVCSpecEditor_Raw(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + assert.Same(t, spec, editor.Raw()) +} + +func TestPVCSpecEditor_SetStorageRequest(t *testing.T) { + t.Run("nil requests map", func(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + qty := resource.MustParse("10Gi") + editor.SetStorageRequest(qty) + require.NotNil(t, spec.Resources.Requests) + assert.True(t, spec.Resources.Requests[corev1.ResourceStorage].Equal(qty)) + }) + + t.Run("existing requests map", func(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("5Gi"), + }, + }, + } + editor := NewPVCSpecEditor(spec) + qty := resource.MustParse("20Gi") + editor.SetStorageRequest(qty) + assert.True(t, spec.Resources.Requests[corev1.ResourceStorage].Equal(qty)) + }) +} + +func TestPVCSpecEditor_SetAccessModes(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + modes := []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce, corev1.ReadOnlyMany} + editor.SetAccessModes(modes) + assert.Equal(t, modes, spec.AccessModes) +} + +func TestPVCSpecEditor_SetStorageClassName(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + editor.SetStorageClassName("fast-ssd") + require.NotNil(t, spec.StorageClassName) + assert.Equal(t, "fast-ssd", *spec.StorageClassName) +} + +func TestPVCSpecEditor_SetVolumeMode(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + editor.SetVolumeMode(corev1.PersistentVolumeBlock) + require.NotNil(t, spec.VolumeMode) + assert.Equal(t, corev1.PersistentVolumeBlock, *spec.VolumeMode) +} + +func TestPVCSpecEditor_SetVolumeName(t *testing.T) { + spec := &corev1.PersistentVolumeClaimSpec{} + editor := NewPVCSpecEditor(spec) + editor.SetVolumeName("pv-001") + assert.Equal(t, "pv-001", spec.VolumeName) +} From 8342c75d2cf0d9ad17e3dde7d97c6093ea40a330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:42:16 +0000 Subject: [PATCH 02/21] Add PVC primitive package with Integration lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements PersistentVolumeClaim as an Integration primitive with: - Builder with fluent API for mutations, flavors, status handlers - Mutator with plan-and-apply pattern (metadata edits, spec edits) - DefaultFieldApplicator preserving immutable fields (accessModes, storageClassName, volumeMode, volumeName) on existing PVCs - DefaultOperationalStatusHandler (Bound/Pending/Lost → status mapping) - DefaultSuspendMutationHandler (no-op, PVCs have no runtime state) - DefaultSuspensionStatusHandler (always Suspended) - DefaultDeleteOnSuspendHandler (false, preserve data) - PreserveCurrentLabels and PreserveCurrentAnnotations flavors Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/builder.go | 165 ++++++++++++++++++ pkg/primitives/pvc/builder_test.go | 259 ++++++++++++++++++++++++++++ pkg/primitives/pvc/flavors.go | 25 +++ pkg/primitives/pvc/flavors_test.go | 144 ++++++++++++++++ pkg/primitives/pvc/handlers.go | 81 +++++++++ pkg/primitives/pvc/handlers_test.go | 89 ++++++++++ pkg/primitives/pvc/mutator.go | 118 +++++++++++++ pkg/primitives/pvc/mutator_test.go | 178 +++++++++++++++++++ pkg/primitives/pvc/resource.go | 126 ++++++++++++++ 9 files changed, 1185 insertions(+) create mode 100644 pkg/primitives/pvc/builder.go create mode 100644 pkg/primitives/pvc/builder_test.go create mode 100644 pkg/primitives/pvc/flavors.go create mode 100644 pkg/primitives/pvc/flavors_test.go create mode 100644 pkg/primitives/pvc/handlers.go create mode 100644 pkg/primitives/pvc/handlers_test.go create mode 100644 pkg/primitives/pvc/mutator.go create mode 100644 pkg/primitives/pvc/mutator_test.go create mode 100644 pkg/primitives/pvc/resource.go diff --git a/pkg/primitives/pvc/builder.go b/pkg/primitives/pvc/builder.go new file mode 100644 index 00000000..784570cc --- /dev/null +++ b/pkg/primitives/pvc/builder.go @@ -0,0 +1,165 @@ +package pvc + +import ( + "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 PVC Resource. +// +// It provides a fluent API for registering mutations, field application flavors, +// 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.PersistentVolumeClaim, *Mutator] +} + +// NewBuilder initializes a new Builder with the provided PersistentVolumeClaim object. +// +// The PVC object serves as the desired base state. During reconciliation the Resource +// will make the cluster's state match this base, modified by any registered mutations +// and flavors. +// +// The provided PVC must have both Name and Namespace set, which is validated during +// the Build() call. +func NewBuilder(pvc *corev1.PersistentVolumeClaim) *Builder { + identityFunc := func(p *corev1.PersistentVolumeClaim) string { + return fmt.Sprintf("v1/PersistentVolumeClaim/%s/%s", p.Namespace, p.Name) + } + + base := generic.NewIntegrationBuilder[*corev1.PersistentVolumeClaim, *Mutator]( + pvc, + identityFunc, + DefaultFieldApplicator, + NewMutator, + ) + + base. + WithCustomOperationalStatus(DefaultOperationalStatusHandler). + WithCustomSuspendStatus(DefaultSuspensionStatusHandler). + WithCustomSuspendMutation(DefaultSuspendMutationHandler). + WithCustomSuspendDeletionDecision(DefaultDeleteOnSuspendHandler) + + return &Builder{ + base: base, + } +} + +// WithMutation registers a mutation for the PVC. +// +// 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 PVC in the cluster. +// +// The default applicator (DefaultFieldApplicator) replaces the current object +// with a deep copy of the desired object while preserving immutable fields on +// existing PVCs. Use a custom applicator when you need different merge semantics. +func (b *Builder) WithCustomFieldApplicator( + applicator func(current, desired *corev1.PersistentVolumeClaim) 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.PersistentVolumeClaim](flavor)) + return b +} + +// WithCustomOperationalStatus overrides the default logic for determining if the +// PVC has reached its desired operational state. +// +// The default behavior uses DefaultOperationalStatusHandler, which checks whether +// the PVC is Bound. Use this method if you need additional checks, such as verifying +// specific annotations or conditions. +func (b *Builder) WithCustomOperationalStatus( + handler func(concepts.ConvergingOperation, *corev1.PersistentVolumeClaim) (concepts.OperationalStatusWithReason, error), +) *Builder { + b.base.WithCustomOperationalStatus(handler) + return b +} + +// WithCustomSuspendStatus overrides how the progress of suspension is reported. +// +// The default behavior uses DefaultSuspensionStatusHandler, which always reports +// Suspended since PVCs have no runtime state to wind down. +func (b *Builder) WithCustomSuspendStatus( + handler func(*corev1.PersistentVolumeClaim) (concepts.SuspensionStatusWithReason, error), +) *Builder { + b.base.WithCustomSuspendStatus(handler) + return b +} + +// WithCustomSuspendMutation defines how the PVC should be modified when the +// component is suspended. +// +// The default behavior uses DefaultSuspendMutationHandler, which is a no-op. +// Override this if you need to add annotations or labels when suspended. +func (b *Builder) WithCustomSuspendMutation( + handler func(*Mutator) error, +) *Builder { + b.base.WithCustomSuspendMutation(handler) + return b +} + +// WithCustomSuspendDeletionDecision overrides the decision of whether to delete +// the PVC when the component is suspended. +// +// The default behavior uses DefaultDeleteOnSuspendHandler, which does not delete +// PVCs during suspension to preserve data. Return true from this handler if you +// want the PVC to be completely removed from the cluster when suspended. +func (b *Builder) WithCustomSuspendDeletionDecision( + handler func(*corev1.PersistentVolumeClaim) bool, +) *Builder { + b.base.WithCustomSuspendDeletionDecision(handler) + return b +} + +// WithDataExtractor registers a function to read values from the PVC after +// it has been successfully reconciled. +// +// The extractor receives a value copy of the reconciled PVC. This is useful +// for surfacing the bound volume name, capacity, or other status fields to +// other components or resources. +// +// A nil extractor is ignored. +func (b *Builder) WithDataExtractor(extractor func(corev1.PersistentVolumeClaim) error) *Builder { + if extractor != nil { + b.base.WithDataExtractor(func(p *corev1.PersistentVolumeClaim) error { + return extractor(*p) + }) + } + return b +} + +// Build validates the configuration and returns the initialized Resource. +// +// It returns an error if: +// - No PVC object was provided. +// - The PVC is missing a Name or Namespace. +func (b *Builder) Build() (*Resource, error) { + genericRes, err := b.base.Build() + if err != nil { + return nil, err + } + return &Resource{base: genericRes}, nil +} diff --git a/pkg/primitives/pvc/builder_test.go b/pkg/primitives/pvc/builder_test.go new file mode 100644 index 00000000..a7d90b70 --- /dev/null +++ b/pkg/primitives/pvc/builder_test.go @@ -0,0 +1,259 @@ +package pvc + +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" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestBuilder(t *testing.T) { + t.Parallel() + + t.Run("Build validation", func(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pvc *corev1.PersistentVolumeClaim + expectedErr string + }{ + { + name: "nil pvc", + pvc: nil, + expectedErr: "object cannot be nil", + }, + { + name: "empty name", + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-ns", + }, + }, + expectedErr: "object name cannot be empty", + }, + { + name: "empty namespace", + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + }, + }, + expectedErr: "object namespace cannot be empty", + }, + { + name: "valid pvc", + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + }, + expectedErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := NewBuilder(tt.pvc).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/PersistentVolumeClaim/test-ns/test-pvc", res.Identity()) + } + }) + } + }) + + t.Run("WithMutation", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + m := Mutation{ + Name: "test-mutation", + } + res, err := NewBuilder(pvc). + WithMutation(m). + Build() + require.NoError(t, err) + assert.Len(t, res.base.Mutations, 1) + assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) + }) + + t.Run("WithCustomFieldApplicator", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + applied := false + applicator := func(_, _ *corev1.PersistentVolumeClaim) error { + applied = true + return nil + } + res, err := NewBuilder(pvc). + WithCustomFieldApplicator(applicator). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.CustomFieldApplicator) + _ = res.base.CustomFieldApplicator(nil, nil) + assert.True(t, applied) + }) + + t.Run("WithFieldApplicationFlavor", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + res, err := NewBuilder(pvc). + WithFieldApplicationFlavor(PreserveCurrentLabels). + WithFieldApplicationFlavor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.FieldFlavors, 1) + }) + + t.Run("WithCustomOperationalStatus", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + handler := func(_ concepts.ConvergingOperation, _ *corev1.PersistentVolumeClaim) (concepts.OperationalStatusWithReason, error) { + return concepts.OperationalStatusWithReason{Status: concepts.OperationalStatusPending}, nil + } + res, err := NewBuilder(pvc). + 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.OperationalStatusPending, status.Status) + }) + + t.Run("WithCustomSuspendStatus", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + handler := func(_ *corev1.PersistentVolumeClaim) (concepts.SuspensionStatusWithReason, error) { + return concepts.SuspensionStatusWithReason{Status: concepts.SuspensionStatusSuspended}, nil + } + res, err := NewBuilder(pvc). + WithCustomSuspendStatus(handler). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.SuspendStatusHandler) + status, err := res.base.SuspendStatusHandler(nil) + require.NoError(t, err) + assert.Equal(t, concepts.SuspensionStatusSuspended, status.Status) + }) + + t.Run("WithCustomSuspendMutation", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + handler := func(_ *Mutator) error { + return errors.New("suspend error") + } + res, err := NewBuilder(pvc). + WithCustomSuspendMutation(handler). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.SuspendMutationHandler) + err = res.base.SuspendMutationHandler(nil) + assert.EqualError(t, err, "suspend error") + }) + + t.Run("WithCustomSuspendDeletionDecision", func(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + handler := func(_ *corev1.PersistentVolumeClaim) bool { + return true + } + res, err := NewBuilder(pvc). + WithCustomSuspendDeletionDecision(handler). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.DeleteOnSuspendHandler) + assert.True(t, res.base.DeleteOnSuspendHandler(nil)) + }) + + t.Run("WithDataExtractor", func(t *testing.T) { + t.Parallel() + p := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + }, + } + called := false + extractor := func(_ corev1.PersistentVolumeClaim) error { + called = true + return nil + } + res, err := NewBuilder(p). + WithDataExtractor(extractor). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 1) + err = res.base.DataExtractors[0](&corev1.PersistentVolumeClaim{}) + require.NoError(t, err) + assert.True(t, called) + }) + + t.Run("WithDataExtractor nil", func(t *testing.T) { + t.Parallel() + p := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + } + res, err := NewBuilder(p). + WithDataExtractor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 0) + }) +} diff --git a/pkg/primitives/pvc/flavors.go b/pkg/primitives/pvc/flavors.go new file mode 100644 index 00000000..6ea8ab26 --- /dev/null +++ b/pkg/primitives/pvc/flavors.go @@ -0,0 +1,25 @@ +package pvc + +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 +// PVC 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.PersistentVolumeClaim] + +// PreserveCurrentLabels ensures that any labels present on the current live +// PVC 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.PersistentVolumeClaim) error { + return flavors.PreserveCurrentLabels[*corev1.PersistentVolumeClaim]()(applied, current, desired) +} + +// PreserveCurrentAnnotations ensures that any annotations present on the current +// live PVC 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.PersistentVolumeClaim) error { + return flavors.PreserveCurrentAnnotations[*corev1.PersistentVolumeClaim]()(applied, current, desired) +} diff --git a/pkg/primitives/pvc/flavors_test.go b/pkg/primitives/pvc/flavors_test.go new file mode 100644 index 00000000..319b2675 --- /dev/null +++ b/pkg/primitives/pvc/flavors_test.go @@ -0,0 +1,144 @@ +package pvc + +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 TestPreserveCurrentLabels(t *testing.T) { + t.Parallel() + + t.Run("preserves missing labels", func(t *testing.T) { + applied := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "myapp"}, + }, + } + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "myapp", "external": "value"}, + }, + } + desired := &corev1.PersistentVolumeClaim{} + + err := PreserveCurrentLabels(applied, current, desired) + require.NoError(t, err) + assert.Equal(t, "myapp", applied.Labels["app"]) + assert.Equal(t, "value", applied.Labels["external"]) + }) + + t.Run("applied wins on overlap", func(t *testing.T) { + applied := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "new"}, + }, + } + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "old"}, + }, + } + desired := &corev1.PersistentVolumeClaim{} + + err := PreserveCurrentLabels(applied, current, desired) + require.NoError(t, err) + assert.Equal(t, "new", applied.Labels["app"]) + }) + + t.Run("nil labels on current", func(t *testing.T) { + applied := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": "myapp"}, + }, + } + current := &corev1.PersistentVolumeClaim{} + desired := &corev1.PersistentVolumeClaim{} + + err := PreserveCurrentLabels(applied, current, desired) + require.NoError(t, err) + assert.Equal(t, "myapp", applied.Labels["app"]) + }) +} + +func TestPreserveCurrentAnnotations(t *testing.T) { + t.Parallel() + + t.Run("preserves missing annotations", func(t *testing.T) { + applied := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"managed": "true"}, + }, + } + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"managed": "true", "external": "injected"}, + }, + } + desired := &corev1.PersistentVolumeClaim{} + + err := PreserveCurrentAnnotations(applied, current, desired) + require.NoError(t, err) + assert.Equal(t, "true", applied.Annotations["managed"]) + assert.Equal(t, "injected", applied.Annotations["external"]) + }) + + t.Run("nil annotations on current", func(t *testing.T) { + applied := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{"managed": "true"}, + }, + } + current := &corev1.PersistentVolumeClaim{} + desired := &corev1.PersistentVolumeClaim{} + + err := PreserveCurrentAnnotations(applied, current, desired) + require.NoError(t, err) + assert.Equal(t, "true", applied.Annotations["managed"]) + }) +} + +func TestFlavorsViaBuilder(t *testing.T) { + t.Parallel() + + base := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "default", + Labels: map[string]string{ + "app": "myapp", + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("5Gi"), + }, + }, + }, + } + + res, err := NewBuilder(base). + WithFieldApplicationFlavor(PreserveCurrentLabels). + WithFieldApplicationFlavor(PreserveCurrentAnnotations). + Build() + require.NoError(t, err) + + current := base.DeepCopy() + current.Labels["external-label"] = "preserved" + current.Annotations = map[string]string{"external-annotation": "preserved"} + + err = res.Mutate(current) + require.NoError(t, err) + + obj, err := res.Object() + require.NoError(t, err) + pvc := obj.(*corev1.PersistentVolumeClaim) + assert.Equal(t, "preserved", pvc.Labels["external-label"]) + assert.Equal(t, "preserved", pvc.Annotations["external-annotation"]) +} diff --git a/pkg/primitives/pvc/handlers.go b/pkg/primitives/pvc/handlers.go new file mode 100644 index 00000000..005998de --- /dev/null +++ b/pkg/primitives/pvc/handlers.go @@ -0,0 +1,81 @@ +package pvc + +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 PVC +// has reached its desired operational state. +// +// It considers a PVC operational when its Status.Phase is Bound: +// - Bound → Operational +// - Pending → Pending +// - Lost → Failing +// +// 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, pvc *corev1.PersistentVolumeClaim, +) (concepts.OperationalStatusWithReason, error) { + switch pvc.Status.Phase { + case corev1.ClaimBound: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusOperational, + Reason: fmt.Sprintf("PVC is bound to volume %s", pvc.Spec.VolumeName), + }, nil + case corev1.ClaimLost: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusFailing, + Reason: "PVC has lost its bound volume", + }, nil + default: + return concepts.OperationalStatusWithReason{ + Status: concepts.OperationalStatusPending, + Reason: "Waiting for PVC to be bound", + }, nil + } +} + +// DefaultDeleteOnSuspendHandler provides the default decision of whether to delete +// the PVC when the parent component is suspended. +// +// It always returns false, meaning the PVC is kept in the cluster to preserve its +// underlying storage. Deleting a PVC can lead to permanent data loss if the reclaim +// policy is Delete. +// +// This function is used as the default handler by the Resource if no custom handler +// is registered via Builder.WithCustomSuspendDeletionDecision. +func DefaultDeleteOnSuspendHandler(_ *corev1.PersistentVolumeClaim) bool { + return false +} + +// DefaultSuspendMutationHandler provides the default mutation applied to a PVC when +// the component is suspended. +// +// It is a no-op: PVCs do not have runtime state (like replicas) that needs to be +// wound down. The consuming workload (e.g. a Deployment) is responsible for stopping +// its use of the PVC. +// +// This function is used as the default handler by the Resource if no custom handler +// is registered via Builder.WithCustomSuspendMutation. +func DefaultSuspendMutationHandler(_ *Mutator) error { + return nil +} + +// DefaultSuspensionStatusHandler monitors the progress of the suspension process. +// +// It always reports Suspended because PVCs themselves have no runtime state to wind +// down. Once Suspend() is called, the PVC is considered immediately suspended. +// +// This function is used as the default handler by the Resource if no custom handler +// is registered via Builder.WithCustomSuspendStatus. +func DefaultSuspensionStatusHandler(_ *corev1.PersistentVolumeClaim) (concepts.SuspensionStatusWithReason, error) { + return concepts.SuspensionStatusWithReason{ + Status: concepts.SuspensionStatusSuspended, + Reason: "PVC does not require suspension actions", + }, nil +} diff --git a/pkg/primitives/pvc/handlers_test.go b/pkg/primitives/pvc/handlers_test.go new file mode 100644 index 00000000..dd0c8c0e --- /dev/null +++ b/pkg/primitives/pvc/handlers_test.go @@ -0,0 +1,89 @@ +package pvc + +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 + pvc *corev1.PersistentVolumeClaim + wantStatus concepts.OperationalStatus + wantReason string + }{ + { + name: "bound", + pvc: &corev1.PersistentVolumeClaim{ + Spec: corev1.PersistentVolumeClaimSpec{ + VolumeName: "pv-001", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + }, + wantStatus: concepts.OperationalStatusOperational, + wantReason: "PVC is bound to volume pv-001", + }, + { + name: "pending", + pvc: &corev1.PersistentVolumeClaim{ + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimPending, + }, + }, + wantStatus: concepts.OperationalStatusPending, + wantReason: "Waiting for PVC to be bound", + }, + { + name: "lost", + pvc: &corev1.PersistentVolumeClaim{ + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimLost, + }, + }, + wantStatus: concepts.OperationalStatusFailing, + wantReason: "PVC has lost its bound volume", + }, + { + name: "empty phase", + pvc: &corev1.PersistentVolumeClaim{}, + wantStatus: concepts.OperationalStatusPending, + wantReason: "Waiting for PVC to be bound", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := DefaultOperationalStatusHandler(concepts.ConvergingOperationCreated, tt.pvc) + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, got.Status) + assert.Equal(t, tt.wantReason, got.Reason) + }) + } +} + +func TestDefaultDeleteOnSuspendHandler(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + assert.False(t, DefaultDeleteOnSuspendHandler(pvc)) +} + +func TestDefaultSuspendMutationHandler(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + mutator := NewMutator(pvc) + err := DefaultSuspendMutationHandler(mutator) + require.NoError(t, err) + require.NoError(t, mutator.Apply()) +} + +func TestDefaultSuspensionStatusHandler(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + got, err := DefaultSuspensionStatusHandler(pvc) + require.NoError(t, err) + assert.Equal(t, concepts.SuspensionStatusSuspended, got.Status) + assert.Equal(t, "PVC does not require suspension actions", got.Reason) +} diff --git a/pkg/primitives/pvc/mutator.go b/pkg/primitives/pvc/mutator.go new file mode 100644 index 00000000..cc02eb72 --- /dev/null +++ b/pkg/primitives/pvc/mutator.go @@ -0,0 +1,118 @@ +// Package pvc provides a builder and resource for managing Kubernetes PersistentVolumeClaims. +package pvc + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +// Mutation defines a mutation that is applied to a pvc 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.PVCSpecEditor) error +} + +// Mutator is a high-level helper for modifying a Kubernetes PersistentVolumeClaim. +// +// It uses a "plan-and-apply" pattern: mutations are recorded first, then +// applied to the PVC 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 { + pvc *corev1.PersistentVolumeClaim + + plans []featurePlan + active *featurePlan +} + +// NewMutator creates a new Mutator for the given PersistentVolumeClaim. +func NewMutator(pvc *corev1.PersistentVolumeClaim) *Mutator { + m := &Mutator{pvc: pvc} + 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 PVC'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) +} + +// EditPVCSpec records a mutation for the PVC's spec via a PVCSpecEditor. +// +// The editor provides structured operations (SetStorageRequest, SetAccessModes, 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) EditPVCSpec(edit func(*editors.PVCSpecEditor) error) { + if edit == nil { + return + } + m.active.specEdits = append(m.active.specEdits, edit) +} + +// SetStorageRequest records that the PVC's storage request should be set to quantity. +// +// Convenience wrapper over EditPVCSpec. +func (m *Mutator) SetStorageRequest(quantity resource.Quantity) { + m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(quantity) + return nil + }) +} + +// Apply executes all recorded mutation intents on the underlying PVC. +// +// Execution order across all registered features: +// +// 1. Metadata edits (in registration order within each feature) +// 2. Spec edits — EditPVCSpec, SetStorageRequest (in registration order within each feature) +// +// Features are applied in the order they were registered. Later features observe +// the PVC 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.pvc.ObjectMeta) + for _, edit := range plan.metadataEdits { + if err := edit(editor); err != nil { + return err + } + } + } + + // 2. Spec edits + if len(plan.specEdits) > 0 { + editor := editors.NewPVCSpecEditor(&m.pvc.Spec) + for _, edit := range plan.specEdits { + if err := edit(editor); err != nil { + return err + } + } + } + } + + return nil +} diff --git a/pkg/primitives/pvc/mutator_test.go b/pkg/primitives/pvc/mutator_test.go new file mode 100644 index 00000000..dc0f166d --- /dev/null +++ b/pkg/primitives/pvc/mutator_test.go @@ -0,0 +1,178 @@ +package pvc + +import ( + "errors" + "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 TestMutator_EditObjectMetadata(t *testing.T) { + t.Parallel() + + t.Run("adds label", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + m := NewMutator(pvc) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", pvc.Labels["app"]) + }) + + t.Run("nil edit ignored", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.EditObjectMetadata(nil) + require.NoError(t, m.Apply()) + }) + + t.Run("error propagated", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + return errors.New("metadata error") + }) + err := m.Apply() + require.Error(t, err) + assert.Contains(t, err.Error(), "metadata error") + }) +} + +func TestMutator_EditPVCSpec(t *testing.T) { + t.Parallel() + + t.Run("sets storage request", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(resource.MustParse("10Gi")) + return nil + }) + require.NoError(t, m.Apply()) + qty := pvc.Spec.Resources.Requests[corev1.ResourceStorage] + assert.True(t, qty.Equal(resource.MustParse("10Gi"))) + }) + + t.Run("nil edit ignored", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.EditPVCSpec(nil) + require.NoError(t, m.Apply()) + }) + + t.Run("error propagated", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.EditPVCSpec(func(_ *editors.PVCSpecEditor) error { + return errors.New("spec error") + }) + err := m.Apply() + require.Error(t, err) + assert.Contains(t, err.Error(), "spec error") + }) +} + +func TestMutator_SetStorageRequest(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.SetStorageRequest(resource.MustParse("20Gi")) + require.NoError(t, m.Apply()) + qty := pvc.Spec.Resources.Requests[corev1.ResourceStorage] + assert.True(t, qty.Equal(resource.MustParse("20Gi"))) +} + +func TestMutator_ExecutionOrder(t *testing.T) { + t.Parallel() + + t.Run("metadata before spec within same feature", func(t *testing.T) { + var order []string + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + m := NewMutator(pvc) + m.EditPVCSpec(func(_ *editors.PVCSpecEditor) error { + order = append(order, "spec") + return nil + }) + m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + order = append(order, "metadata") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, []string{"metadata", "spec"}, order) + }) + + t.Run("feature ordering", func(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + m := NewMutator(pvc) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("feature", "one") + return nil + }) + + // Simulate second feature + m.beginFeature() + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + // Later feature sees earlier mutation + e.EnsureLabel("feature", "two") + return nil + }) + + require.NoError(t, m.Apply()) + assert.Equal(t, "two", pvc.Labels["feature"]) + }) +} + +func TestMutator_MultiFeature(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + m := NewMutator(pvc) + + // Feature 1: set label and storage + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(resource.MustParse("5Gi")) + return nil + }) + + // Feature 2: override storage + m.beginFeature() + m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(resource.MustParse("10Gi")) + return nil + }) + + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", pvc.Labels["app"]) + qty := pvc.Spec.Resources.Requests[corev1.ResourceStorage] + assert.True(t, qty.Equal(resource.MustParse("10Gi"))) +} diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go new file mode 100644 index 00000000..74906551 --- /dev/null +++ b/pkg/primitives/pvc/resource.go @@ -0,0 +1,126 @@ +package pvc + +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 replaces current with a deep copy of desired, preserving +// immutable fields on existing PVCs. +// +// Kubernetes marks several PVC spec fields as immutable after creation: accessModes, +// storageClassName, volumeMode, and volumeName. If the current object has a non-empty +// ResourceVersion (indicating it already exists in the cluster), those fields are +// restored from the current object after the deep copy. +// +// Use a custom field applicator via Builder.WithCustomFieldApplicator if you need +// different merge semantics. +func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) error { + isExisting := current.ResourceVersion != "" + + savedAccessModes := current.Spec.AccessModes + savedStorageClassName := current.Spec.StorageClassName + savedVolumeMode := current.Spec.VolumeMode + savedVolumeName := current.Spec.VolumeName + + *current = *desired.DeepCopy() + + if isExisting { + current.Spec.AccessModes = savedAccessModes + current.Spec.StorageClassName = savedStorageClassName + current.Spec.VolumeMode = savedVolumeMode + current.Spec.VolumeName = savedVolumeName + } + + return nil +} + +// Resource is a high-level abstraction for managing a Kubernetes PersistentVolumeClaim +// 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 PVC is bound and operational. +// - component.Suspendable: for controlled suspension (e.g. retaining the PVC while suspending consumers). +// - component.DataExtractable: for exporting values after successful reconciliation. +// +// PVC resources follow the Integration lifecycle: they are operationally significant +// (a PVC must be Bound to be useful) and support suspension semantics. +type Resource struct { + base *generic.IntegrationResource[*corev1.PersistentVolumeClaim, *Mutator] +} + +// Identity returns a unique identifier for the PVC in the format +// "v1/PersistentVolumeClaim//". +func (r *Resource) Identity() string { + return r.base.Identity() +} + +// Object returns a deep copy of the underlying Kubernetes PersistentVolumeClaim 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 PersistentVolumeClaim into the +// desired state. +// +// The mutation process follows this order: +// 1. Field application: the current object is updated to reflect the desired base state, +// using either DefaultFieldApplicator or a custom applicator if one is configured. +// 2. Field application flavors: any registered flavors are applied in registration order. +// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// 4. Suspension: if the resource is in a suspending state, the suspension logic is applied. +// +// 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 whether the PVC has reached its desired operational state. +// +// By default, it uses DefaultOperationalStatusHandler, which checks the PVC's phase +// to determine if it is Bound (operational), Pending, or Lost (failing). +func (r *Resource) ConvergingStatus(op concepts.ConvergingOperation) (concepts.OperationalStatusWithReason, error) { + return r.base.ConvergingStatus(op) +} + +// DeleteOnSuspend determines whether the PVC should be deleted from the cluster +// when the parent component is suspended. +// +// By default, it uses DefaultDeleteOnSuspendHandler, which returns false to preserve +// the PVC and its underlying storage during suspension. +func (r *Resource) DeleteOnSuspend() bool { + return r.base.DeleteOnSuspend() +} + +// Suspend triggers the deactivation of the PVC. +// +// It registers a mutation that will be executed during the next Mutate call. +// The default behaviour uses DefaultSuspendMutationHandler, which is a no-op +// since PVCs do not require modification when suspended — the consuming workload +// is responsible for scaling down. +func (r *Resource) Suspend() error { + return r.base.Suspend() +} + +// SuspensionStatus monitors the progress of the suspension process. +// +// By default, it uses DefaultSuspensionStatusHandler, which always reports +// Suspended since PVCs themselves have no runtime state to wind down. +func (r *Resource) SuspensionStatus() (concepts.SuspensionStatusWithReason, error) { + return r.base.SuspensionStatus() +} + +// ExtractData executes all registered data extractor functions against a deep copy +// of the reconciled PVC. +// +// This is called by the framework after successful reconciliation, allowing the +// component to read generated or updated values from the PVC. +func (r *Resource) ExtractData() error { + return r.base.ExtractData() +} From 943fcbf8dcdca6a0fa226c6594d30c31daa80ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:43:15 +0000 Subject: [PATCH 03/21] Add PVC primitive documentation Covers capabilities, building, default field application (immutable field preservation), mutations (boolean-gated, version-gated), internal ordering, editors (PVCSpecEditor, ObjectMetaEditor), convenience methods, flavors, status handlers, and usage guidance. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 256 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 docs/primitives/pvc.md diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md new file mode 100644 index 00000000..4ffc78cf --- /dev/null +++ b/docs/primitives/pvc.md @@ -0,0 +1,256 @@ +# PersistentVolumeClaim Primitive + +The `pvc` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolumeClaim` resources. It integrates with the component lifecycle and provides a structured mutation API for managing storage requests and object metadata. + +## Capabilities + +| Capability | Detail | +|---------------------------|------------------------------------------------------------------------------------------------| +| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `Pending`, or `Failing` (Lost) | +| **Immutable field safety**| DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | +| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | +| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | + +## Building a PVC Primitive + +```go +import "github.com/sourcehawk/operator-component-framework/pkg/primitives/pvc" + +base := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: owner.Namespace, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + }, +} + +resource, err := pvc.NewBuilder(base). + WithFieldApplicationFlavor(pvc.PreserveCurrentLabels). + WithMutation(MyStorageMutation(owner.Spec.Version)). + Build() +``` + +## Default Field Application + +`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, but preserves immutable fields on existing PVCs. + +Kubernetes marks several PVC spec fields as immutable after creation. The default applicator detects existing PVCs (via `ResourceVersion`) and restores these fields from the cluster object: + +| Immutable Field | Preserved On Existing | +|----------------------|-----------------------| +| `accessModes` | Yes | +| `storageClassName` | Yes | +| `volumeMode` | Yes | +| `volumeName` | Yes | + +Use `WithCustomFieldApplicator` when you need different merge semantics: + +```go +resource, err := pvc.NewBuilder(base). + WithCustomFieldApplicator(func(current, desired *corev1.PersistentVolumeClaim) error { + // Only update the storage request; leave everything else untouched. + current.Spec.Resources.Requests = desired.Spec.Resources.Requests + return nil + }). + Build() +``` + +## Mutations + +Mutations are the primary mechanism for modifying a `PersistentVolumeClaim` 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 MyStorageMutation(version string) pvc.Mutation { + return pvc.Mutation{ + Name: "storage-expansion", + Feature: feature.NewResourceFeature(version, nil), // always enabled + Mutate: func(m *pvc.Mutator) error { + m.SetStorageRequest(resource.MustParse("20Gi")) + 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 LargeStorageMutation(version string, needsLargeStorage bool) pvc.Mutation { + return pvc.Mutation{ + Name: "large-storage", + Feature: feature.NewResourceFeature(version, nil).When(needsLargeStorage), + Mutate: func(m *pvc.Mutator) error { + m.SetStorageRequest(resource.MustParse("100Gi")) + return nil + }, + } +} +``` + +### Version-gated mutations + +```go +var v2Constraint = mustSemverConstraint(">= 2.0.0") + +func V2StorageMutation(version string) pvc.Mutation { + return pvc.Mutation{ + Name: "v2-storage", + Feature: feature.NewResourceFeature( + version, + []feature.VersionConstraint{v2Constraint}, + ), + Mutate: func(m *pvc.Mutator) error { + m.SetStorageRequest(resource.MustParse("50Gi")) + 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 `PersistentVolumeClaim` | +| 2 | Spec edits | PVC spec — storage requests, access modes, etc. | + +Within each category, edits are applied in their registration order. Later features observe the PVC as modified by all previous features. + +## Editors + +### PVCSpecEditor + +The primary API for modifying PVC spec fields. Use `m.EditPVCSpec` for full control: + +```go +m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(resource.MustParse("20Gi")) + return nil +}) +``` + +Available methods: + +| Method | What it does | +|-----------------------|---------------------------------------------------| +| `SetStorageRequest` | Sets `spec.resources.requests[storage]` | +| `SetAccessModes` | Sets `spec.accessModes` (immutable after creation) | +| `SetStorageClassName` | Sets `spec.storageClassName` (immutable after creation) | +| `SetVolumeMode` | Sets `spec.volumeMode` (immutable after creation) | +| `SetVolumeName` | Sets `spec.volumeName` (immutable after creation) | +| `Raw` | Returns `*corev1.PersistentVolumeClaimSpec` | + +#### Raw Escape Hatch + +`Raw()` returns the underlying `*corev1.PersistentVolumeClaimSpec` for free-form editing when none of the structured methods are sufficient: + +```go +m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + raw := e.Raw() + raw.Selector = &metav1.LabelSelector{ + MatchLabels: map[string]string{"type": "fast"}, + } + return nil +}) +``` + +### ObjectMetaEditor + +Modifies labels and annotations via `m.EditObjectMetadata`. + +Available methods: `EnsureLabel`, `RemoveLabel`, `EnsureAnnotation`, `RemoveAnnotation`, `Raw`. + +```go +m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + e.EnsureAnnotation("storage/class-hint", "fast-ssd") + return nil +}) +``` + +## Convenience Methods + +The `Mutator` exposes a convenience wrapper for the most common PVC operation: + +| Method | Equivalent to | +|--------------------------------|-----------------------------------------------| +| `SetStorageRequest(quantity)` | `EditPVCSpec` → `e.SetStorageRequest(quantity)` | + +Use this for simple, single-operation mutations. Use `EditPVCSpec` when you need multiple operations or raw access in a single edit block. + +## 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 := pvc.NewBuilder(base). + WithFieldApplicationFlavor(pvc.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 := pvc.NewBuilder(base). + WithFieldApplicationFlavor(pvc.PreserveCurrentAnnotations). + Build() +``` + +Multiple flavors can be registered and run in registration order. + +## Status Handlers + +### Operational Status + +The default handler (`DefaultOperationalStatusHandler`) maps PVC phase to operational status: + +| PVC Phase | Status | Reason | +|------------|---------------|----------------------------------| +| `Bound` | Operational | PVC is bound to volume \ | +| `Pending` | Pending | Waiting for PVC to be bound | +| `Lost` | Failing | PVC has lost its bound volume | + +Override with `WithCustomOperationalStatus` for additional checks (e.g. verifying specific annotations or volume attributes). + +### Suspension + +PVCs have no runtime state to wind down, so: + +- `DefaultSuspendMutationHandler` is a no-op. +- `DefaultSuspensionStatusHandler` always reports `Suspended`. +- `DefaultDeleteOnSuspendHandler` returns `false` to preserve data. + +Override these handlers if you need custom suspension behavior, such as adding annotations when suspended or deleting PVCs that use ephemeral storage. + +## Guidance + +**Use `DefaultFieldApplicator` unless you have a specific reason not to.** It handles the immutable field preservation that Kubernetes requires for PVCs, preventing rejected update requests. + +**Register mutations for storage expansion carefully.** Kubernetes only allows expanding PVC storage (not shrinking). Ensure your mutations respect this constraint. The `SetStorageRequest` method does not enforce this — the API server will reject invalid requests. + +**Prefer `WithCustomSuspendDeletionDecision` over deleting PVCs manually.** If you need PVCs to be cleaned up during suspension, register a deletion decision handler rather than deleting them in a mutation. + +**Use flavors for externally-managed metadata.** If admission webhooks, external controllers, or GitOps tools add labels or annotations to your PVCs, use `PreserveCurrentLabels` and `PreserveCurrentAnnotations` to prevent your operator from removing them. From 3a91a5db05234bd652bc86da63f6b820383b970e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:45:07 +0000 Subject: [PATCH 04/21] Add PVC primitive example Demonstrates PVC builder with version labels, storage annotations, conditional large-storage expansion, label/annotation flavors, data extraction, and suspension. Follows the configmap and deployment example patterns. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/pvc-primitive/README.md | 31 +++++ examples/pvc-primitive/app/controller.go | 54 +++++++++ examples/pvc-primitive/app/owner.go | 20 ++++ examples/pvc-primitive/features/mutations.go | 54 +++++++++ examples/pvc-primitive/main.go | 119 +++++++++++++++++++ examples/pvc-primitive/resources/pvc.go | 61 ++++++++++ 6 files changed, 339 insertions(+) create mode 100644 examples/pvc-primitive/README.md create mode 100644 examples/pvc-primitive/app/controller.go create mode 100644 examples/pvc-primitive/app/owner.go create mode 100644 examples/pvc-primitive/features/mutations.go create mode 100644 examples/pvc-primitive/main.go create mode 100644 examples/pvc-primitive/resources/pvc.go diff --git a/examples/pvc-primitive/README.md b/examples/pvc-primitive/README.md new file mode 100644 index 00000000..6bbacd18 --- /dev/null +++ b/examples/pvc-primitive/README.md @@ -0,0 +1,31 @@ +# PVC Primitive Example + +This example demonstrates the usage of the `pvc` primitive within the operator component framework. +It shows how to manage a Kubernetes PersistentVolumeClaim as a component of a larger application, utilising features like: + +- **Base Construction**: Initializing a PVC with access modes, storage request, and metadata. +- **Feature Mutations**: Applying conditional storage expansion and metadata updates using the `Mutator`. +- **Immutable Field Safety**: The `DefaultFieldApplicator` preserves immutable PVC fields on existing resources. +- **Field Flavors**: Preserving labels and annotations managed by external tools. +- **Suspension**: PVCs are immediately suspended with data preserved by default. +- **Data Extraction**: Harvesting PVC status 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, storage annotation, and conditional large-storage expansion. +- `resources/`: Contains the central `NewPVCResource` factory that assembles all features using `pvc.Builder`. +- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. + +## Running the Example + +```bash +go run examples/pvc-primitive/main.go +``` + +This will: +1. Initialize a fake Kubernetes client. +2. Create an `ExampleApp` owner object. +3. Reconcile through four spec variations, demonstrating version upgrades, storage expansion, and suspension. +4. Print the resulting status conditions. diff --git a/examples/pvc-primitive/app/controller.go b/examples/pvc-primitive/app/controller.go new file mode 100644 index 00000000..9d055e2a --- /dev/null +++ b/examples/pvc-primitive/app/controller.go @@ -0,0 +1,54 @@ +// Package app provides a sample controller using the PVC primitive. +package app + +import ( + "context" + + "github.com/sourcehawk/operator-component-framework/pkg/component" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// ExampleController reconciles an ExampleApp object using the component framework. +type ExampleController struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder + Metrics component.Recorder + + // NewPVCResource is a factory function to create the PVC resource. + // This allows us to inject the resource construction logic. + NewPVCResource func(*ExampleApp) (component.Resource, error) +} + +// Reconcile performs the reconciliation for a single ExampleApp. +func (r *ExampleController) Reconcile(ctx context.Context, owner *ExampleApp) error { + // 1. Build the PVC resource for this owner. + pvcResource, err := r.NewPVCResource(owner) + if err != nil { + return err + } + + // 2. Build the component that manages the PVC. + comp, err := component.NewComponentBuilder(). + WithName("example-app"). + WithConditionType("AppReady"). + WithResource(pvcResource, component.ResourceOptions{}). + Suspend(owner.Spec.Suspended). + Build() + if err != nil { + return err + } + + // 3. Execute the component reconciliation. + resCtx := component.ReconcileContext{ + Client: r.Client, + Scheme: r.Scheme, + Recorder: r.Recorder, + Metrics: r.Metrics, + Owner: owner, + } + + return comp.Reconcile(ctx, resCtx) +} diff --git a/examples/pvc-primitive/app/owner.go b/examples/pvc-primitive/app/owner.go new file mode 100644 index 00000000..6b611a02 --- /dev/null +++ b/examples/pvc-primitive/app/owner.go @@ -0,0 +1,20 @@ +package app + +import ( + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" +) + +// ExampleApp re-exports the shared CRD type so callers in this package need no import alias. +type ExampleApp = sharedapp.ExampleApp + +// ExampleAppSpec re-exports the shared spec type. +type ExampleAppSpec = sharedapp.ExampleAppSpec + +// ExampleAppStatus re-exports the shared status type. +type ExampleAppStatus = sharedapp.ExampleAppStatus + +// ExampleAppList re-exports the shared list type. +type ExampleAppList = sharedapp.ExampleAppList + +// AddToScheme registers the ExampleApp types with the given scheme. +var AddToScheme = sharedapp.AddToScheme diff --git a/examples/pvc-primitive/features/mutations.go b/examples/pvc-primitive/features/mutations.go new file mode 100644 index 00000000..3bd6b5fc --- /dev/null +++ b/examples/pvc-primitive/features/mutations.go @@ -0,0 +1,54 @@ +// Package features provides sample mutations for the PVC primitive example. +package features + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/pvc" + "k8s.io/apimachinery/pkg/api/resource" +) + +// VersionLabelMutation sets the app.kubernetes.io/version label on the PVC. +// It is always enabled. +func VersionLabelMutation(version string) pvc.Mutation { + return pvc.Mutation{ + Name: "version-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pvc.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +// LargeStorageMutation expands the PVC storage request to 50Gi. +// It is enabled when needsLargeStorage is true. +func LargeStorageMutation(version string, needsLargeStorage bool) pvc.Mutation { + return pvc.Mutation{ + Name: "large-storage", + Feature: feature.NewResourceFeature(version, nil).When(needsLargeStorage), + Mutate: func(m *pvc.Mutator) error { + m.SetStorageRequest(resource.MustParse("50Gi")) + return nil + }, + } +} + +// StorageAnnotationMutation adds a storage-class hint annotation to the PVC. +// It is always enabled. +func StorageAnnotationMutation(version string, storageClass string) pvc.Mutation { + return pvc.Mutation{ + Name: "storage-annotation", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *pvc.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureAnnotation("storage/class-hint", storageClass) + return nil + }) + return nil + }, + } +} diff --git a/examples/pvc-primitive/main.go b/examples/pvc-primitive/main.go new file mode 100644 index 00000000..ec5ff891 --- /dev/null +++ b/examples/pvc-primitive/main.go @@ -0,0 +1,119 @@ +// Package main is the entry point for the PVC primitive example. +package main + +import ( + "context" + "fmt" + "os" + + ocm "github.com/sourcehawk/go-crd-condition-metrics/pkg/crd-condition-metrics" + "github.com/sourcehawk/operator-component-framework/examples/pvc-primitive/app" + "github.com/sourcehawk/operator-component-framework/examples/pvc-primitive/resources" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func main() { + // 1. Setup scheme and fake client. + scheme := runtime.NewScheme() + if err := app.AddToScheme(scheme); err != nil { + fmt.Fprintf(os.Stderr, "failed to add to scheme: %v\n", err) + os.Exit(1) + } + if err := corev1.AddToScheme(scheme); err != nil { + fmt.Fprintf(os.Stderr, "failed to add core/v1 to scheme: %v\n", err) + os.Exit(1) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithStatusSubresource(&app.ExampleApp{}). + Build() + + // 2. Create an example Owner object. + owner := &app.ExampleApp{ + Spec: app.ExampleAppSpec{ + Version: "1.2.3", + EnableTracing: false, + EnableMetrics: false, + Suspended: false, + }, + } + owner.Name = "my-example-app" + owner.Namespace = "default" + + if err := fakeClient.Create(context.Background(), owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to create owner: %v\n", err) + os.Exit(1) + } + + // 3. Initialize the controller. + gauge := ocm.NewOperatorConditionsGauge("example") + controller := &app.ExampleController{ + Client: fakeClient, + Scheme: scheme, + Recorder: record.NewFakeRecorder(100), + Metrics: &ocm.ConditionMetricRecorder{ + Controller: "example-controller", + OperatorConditionsGauge: gauge, + }, + NewPVCResource: resources.NewPVCResource, + } + + // 4. Run reconciliation with multiple spec versions to demonstrate + // how PVC mutations compose across features. + specs := []app.ExampleAppSpec{ + { + Version: "1.2.3", + EnableTracing: false, + EnableMetrics: false, + Suspended: false, + }, + { + Version: "1.2.4", // Version upgrade + EnableTracing: false, + EnableMetrics: false, + Suspended: false, + }, + { + Version: "1.2.4", + EnableTracing: false, + EnableMetrics: true, // Triggers large storage mutation + Suspended: false, + }, + { + Version: "1.2.4", + EnableTracing: false, + EnableMetrics: true, + Suspended: true, // Suspend the app + }, + } + + ctx := context.Background() + + for i, spec := range specs { + fmt.Printf("\n--- Step %d: Version=%s, Metrics=%v, Suspended=%v ---\n", + i+1, spec.Version, spec.EnableMetrics, spec.Suspended) + + owner.Spec = spec + if err := fakeClient.Update(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to update owner: %v\n", err) + os.Exit(1) + } + + fmt.Println("Running reconciliation...") + if err := controller.Reconcile(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "reconciliation failed: %v\n", err) + os.Exit(1) + } + + for _, cond := range owner.Status.Conditions { + fmt.Printf("Condition: %s, Status: %s, Reason: %s\n", + cond.Type, cond.Status, cond.Reason) + } + } + + fmt.Println("\nReconciliation sequence completed successfully!") +} diff --git a/examples/pvc-primitive/resources/pvc.go b/examples/pvc-primitive/resources/pvc.go new file mode 100644 index 00000000..68581e4d --- /dev/null +++ b/examples/pvc-primitive/resources/pvc.go @@ -0,0 +1,61 @@ +// Package resources provides resource implementations for the PVC primitive example. +package resources + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/examples/pvc-primitive/app" + "github.com/sourcehawk/operator-component-framework/examples/pvc-primitive/features" + "github.com/sourcehawk/operator-component-framework/pkg/component" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/pvc" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NewPVCResource constructs a PVC primitive resource with all the features. +func NewPVCResource(owner *app.ExampleApp) (component.Resource, error) { + // 1. Create the base PVC object. + base := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-data", + Namespace: owner.Namespace, + Labels: map[string]string{ + "app": owner.Name, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + }, + } + + // 2. Initialize the PVC builder. + builder := pvc.NewBuilder(base) + + // 3. Register mutations in dependency order. + builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) + builder.WithMutation(features.StorageAnnotationMutation(owner.Spec.Version, "standard")) + builder.WithMutation(features.LargeStorageMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + + // 4. Preserve labels and annotations added by external controllers. + builder.WithFieldApplicationFlavor(pvc.PreserveCurrentLabels) + builder.WithFieldApplicationFlavor(pvc.PreserveCurrentAnnotations) + + // 5. Extract data from the reconciled PVC. + builder.WithDataExtractor(func(p corev1.PersistentVolumeClaim) error { + fmt.Printf("Reconciled PVC: %s\n", p.Name) + fmt.Printf(" Phase: %s\n", p.Status.Phase) + if storage, ok := p.Spec.Resources.Requests[corev1.ResourceStorage]; ok { + fmt.Printf(" Storage Request: %s\n", storage.String()) + } + return nil + }) + + // 6. Build the final resource. + return builder.Build() +} From d41665ce8595cae83f369ea361e3f606212cfdbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 04:31:24 +0000 Subject: [PATCH 05/21] Add unit tests for DefaultFieldApplicator Address Copilot review comment: DefaultFieldApplicator had no test coverage for its immutable field preservation logic. Add tests validating that: - New PVCs (empty ResourceVersion) use all fields from desired - Existing PVCs preserve immutable fields (accessModes, storageClassName, volumeMode, volumeName) from current - Mutable fields (e.g. storage requests) come from desired Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/resource_test.go | 136 ++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 pkg/primitives/pvc/resource_test.go diff --git a/pkg/primitives/pvc/resource_test.go b/pkg/primitives/pvc/resource_test.go new file mode 100644 index 00000000..10ba6ca1 --- /dev/null +++ b/pkg/primitives/pvc/resource_test.go @@ -0,0 +1,136 @@ +package pvc + +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(t *testing.T) { + storageClass := "fast-ssd" + volumeMode := corev1.PersistentVolumeFilesystem + + tests := []struct { + name string + current *corev1.PersistentVolumeClaim + desired *corev1.PersistentVolumeClaim + verify func(t *testing.T, result *corev1.PersistentVolumeClaim) + }{ + { + name: "new PVC uses desired immutable fields", + current: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + }, + desired: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + StorageClassName: &storageClass, + VolumeMode: &volumeMode, + VolumeName: "pv-001", + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + }, + }, + verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { + assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, result.Spec.AccessModes) + assert.Equal(t, &storageClass, result.Spec.StorageClassName) + assert.Equal(t, &volumeMode, result.Spec.VolumeMode) + assert.Equal(t, "pv-001", result.Spec.VolumeName) + assert.Equal(t, resource.MustParse("10Gi"), result.Spec.Resources.Requests[corev1.ResourceStorage]) + }, + }, + { + name: "existing PVC preserves immutable fields from current", + current: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + ResourceVersion: "12345", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, + StorageClassName: &storageClass, + VolumeMode: &volumeMode, + VolumeName: "pv-original", + }, + }, + desired: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, + StorageClassName: func() *string { s := "slow-hdd"; return &s }(), + VolumeMode: func() *corev1.PersistentVolumeMode { m := corev1.PersistentVolumeBlock; return &m }(), + VolumeName: "pv-new", + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("20Gi"), + }, + }, + }, + }, + verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { + // Immutable fields should be preserved from current. + assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, result.Spec.AccessModes) + assert.Equal(t, &storageClass, result.Spec.StorageClassName) + assert.Equal(t, &volumeMode, result.Spec.VolumeMode) + assert.Equal(t, "pv-original", result.Spec.VolumeName) + // Mutable fields should come from desired. + assert.Equal(t, resource.MustParse("20Gi"), result.Spec.Resources.Requests[corev1.ResourceStorage]) + }, + }, + { + name: "existing PVC with empty ResourceVersion treated as new", + current: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + ResourceVersion: "", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, + VolumeName: "pv-original", + }, + }, + desired: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, + VolumeName: "pv-new", + }, + }, + verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { + // No ResourceVersion means new — desired fields should be used. + assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, result.Spec.AccessModes) + assert.Equal(t, "pv-new", result.Spec.VolumeName) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := DefaultFieldApplicator(tt.current, tt.desired) + require.NoError(t, err) + tt.verify(t, tt.current) + }) + } +} From ffb1d90050da8f8d92eec274aedaf94f119b1c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 19:53:28 +0000 Subject: [PATCH 06/21] preserve server-managed metadata in default field applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/resource.go | 6 ++- pkg/primitives/pvc/resource_test.go | 64 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go index 74906551..e13cc9bc 100644 --- a/pkg/primitives/pvc/resource.go +++ b/pkg/primitives/pvc/resource.go @@ -8,7 +8,9 @@ import ( ) // DefaultFieldApplicator replaces current with a deep copy of desired, preserving -// immutable fields on existing PVCs. +// server-managed metadata (ResourceVersion, UID, Generation, etc.), shared-controller +// fields (OwnerReferences, Finalizers), and immutable PVC spec fields on existing +// resources. // // Kubernetes marks several PVC spec fields as immutable after creation: accessModes, // storageClassName, volumeMode, and volumeName. If the current object has a non-empty @@ -19,6 +21,7 @@ import ( // different merge semantics. func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) error { isExisting := current.ResourceVersion != "" + original := current.DeepCopy() savedAccessModes := current.Spec.AccessModes savedStorageClassName := current.Spec.StorageClassName @@ -26,6 +29,7 @@ func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) erro savedVolumeName := current.Spec.VolumeName *current = *desired.DeepCopy() + generic.PreserveServerManagedFields(current, original) if isExisting { current.Spec.AccessModes = savedAccessModes diff --git a/pkg/primitives/pvc/resource_test.go b/pkg/primitives/pvc/resource_test.go index 10ba6ca1..75419740 100644 --- a/pkg/primitives/pvc/resource_test.go +++ b/pkg/primitives/pvc/resource_test.go @@ -134,3 +134,67 @@ func TestDefaultFieldApplicator(t *testing.T) { }) } } + +func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { + storageClass := "fast-ssd" + volumeMode := corev1.PersistentVolumeFilesystem + + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + ResourceVersion: "12345", + UID: "abc-def", + Generation: 3, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, + }, + Finalizers: []string{"finalizer.example.com"}, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + StorageClassName: &storageClass, + VolumeMode: &volumeMode, + VolumeName: "pv-001", + }, + } + desired := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, + VolumeName: "pv-new", + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("20Gi"), + }, + }, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec (mutable fields) and labels are applied + assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) + assert.Equal(t, "test", current.Labels["app"]) + + // Immutable PVC fields are preserved + assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, current.Spec.AccessModes) + assert.Equal(t, &storageClass, current.Spec.StorageClassName) + assert.Equal(t, &volumeMode, current.Spec.VolumeMode) + assert.Equal(t, "pv-001", current.Spec.VolumeName) + + // Server-managed fields are preserved + assert.Equal(t, "12345", current.ResourceVersion) + assert.Equal(t, "abc-def", string(current.UID)) + assert.Equal(t, int64(3), current.Generation) + + // Shared-controller fields are preserved + assert.Len(t, current.OwnerReferences, 1) + assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) + assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) +} From 4e7b2bbd972768d0e3ddc2d2fec516c2260e5c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 02:56:47 +0000 Subject: [PATCH 07/21] add PVC primitive to built-in primitives documentation table Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/primitives.md b/docs/primitives.md index 13fde6a6..a7ca70b1 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -130,6 +130,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/pvc` | Integration | [pvc.md](primitives/pvc.md) | ## Usage Examples From 25906deadeeec803cf2dded7d5476beacf41f243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:42:54 +0000 Subject: [PATCH 08/21] fix(pvc): initialize first feature plan inline instead of calling beginFeature Aligns with the fix applied to the deployment and configmap primitives, preventing an empty leading feature plan when the mutator helper calls beginFeature(). Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/mutator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/pvc/mutator.go b/pkg/primitives/pvc/mutator.go index cc02eb72..74d50b8a 100644 --- a/pkg/primitives/pvc/mutator.go +++ b/pkg/primitives/pvc/mutator.go @@ -35,8 +35,11 @@ type Mutator struct { // NewMutator creates a new Mutator for the given PersistentVolumeClaim. func NewMutator(pvc *corev1.PersistentVolumeClaim) *Mutator { - m := &Mutator{pvc: pvc} - m.beginFeature() + m := &Mutator{ + pvc: pvc, + plans: []featurePlan{{}}, + } + m.active = &m.plans[0] return m } From 2c25d1f14760df448a75a73c1981f6b7bfca49ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:58:36 +0000 Subject: [PATCH 09/21] fix(pvc): address PR review - preserve status, add test, fix docs - Add generic.PreserveStatus call in DefaultFieldApplicator to prevent status clobbering on update - Add TestDefaultFieldApplicator_PreservesStatus regression test - Update pvc.md to accurately describe mutation ordering (no per-feature boundaries due to unexported FeatureMutator interface) - Update DefaultFieldApplicator doc comment to mention Status preservation Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 4 +-- pkg/primitives/pvc/resource.go | 5 ++-- pkg/primitives/pvc/resource_test.go | 38 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index 4ffc78cf..a58e4f79 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -41,7 +41,7 @@ resource, err := pvc.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, but preserves immutable fields on existing PVCs. +`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by the API server or other controllers. Kubernetes marks several PVC spec fields as immutable after creation. The default applicator detects existing PVCs (via `ResourceVersion`) and restores these fields from the cluster object: @@ -131,7 +131,7 @@ Within a single mutation, edit operations are applied in a fixed category order | 1 | Metadata edits | Labels and annotations on the `PersistentVolumeClaim` | | 2 | Spec edits | PVC spec — storage requests, access modes, etc. | -Within each category, edits are applied in their registration order. Later features observe the PVC as modified by all previous features. +Within each category, edits are applied in their registration order. The PVC primitive does not currently group mutations by feature boundary; all applicable edits are applied in a single deterministic sequence rather than guaranteeing that later features observe only fully-applied state from earlier features. ## Editors diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go index e13cc9bc..5c166fad 100644 --- a/pkg/primitives/pvc/resource.go +++ b/pkg/primitives/pvc/resource.go @@ -9,8 +9,8 @@ import ( // DefaultFieldApplicator replaces current with a deep copy of desired, preserving // server-managed metadata (ResourceVersion, UID, Generation, etc.), shared-controller -// fields (OwnerReferences, Finalizers), and immutable PVC spec fields on existing -// resources. +// fields (OwnerReferences, Finalizers), the Status subresource, and immutable PVC +// spec fields on existing resources. // // Kubernetes marks several PVC spec fields as immutable after creation: accessModes, // storageClassName, volumeMode, and volumeName. If the current object has a non-empty @@ -30,6 +30,7 @@ func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) erro *current = *desired.DeepCopy() generic.PreserveServerManagedFields(current, original) + generic.PreserveStatus(current, original) if isExisting { current.Spec.AccessModes = savedAccessModes diff --git a/pkg/primitives/pvc/resource_test.go b/pkg/primitives/pvc/resource_test.go index 75419740..9ea39b01 100644 --- a/pkg/primitives/pvc/resource_test.go +++ b/pkg/primitives/pvc/resource_test.go @@ -135,6 +135,44 @@ func TestDefaultFieldApplicator(t *testing.T) { } } +func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + } + desired := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-data", + Namespace: "default", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("20Gi"), + }, + }, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec is applied + assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) + + // Status from the live object is preserved + assert.Equal(t, corev1.ClaimBound, current.Status.Phase) + assert.Equal(t, resource.MustParse("10Gi"), current.Status.Capacity[corev1.ResourceStorage]) +} + func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { storageClass := "fast-ssd" volumeMode := corev1.PersistentVolumeFilesystem From dc9f30a9794430366012c679bea6eb8327c5c22b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 21:37:56 +0000 Subject: [PATCH 10/21] fix(pvc): export BeginFeature to match FeatureMutator interface Align with upstream change in main that exported BeginFeature() on the FeatureMutator interface. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/mutator.go | 4 ++-- pkg/primitives/pvc/mutator_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/primitives/pvc/mutator.go b/pkg/primitives/pvc/mutator.go index 74d50b8a..31108e27 100644 --- a/pkg/primitives/pvc/mutator.go +++ b/pkg/primitives/pvc/mutator.go @@ -43,9 +43,9 @@ func NewMutator(pvc *corev1.PersistentVolumeClaim) *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/pvc/mutator_test.go b/pkg/primitives/pvc/mutator_test.go index dc0f166d..88814720 100644 --- a/pkg/primitives/pvc/mutator_test.go +++ b/pkg/primitives/pvc/mutator_test.go @@ -132,7 +132,7 @@ func TestMutator_ExecutionOrder(t *testing.T) { }) // Simulate second feature - m.beginFeature() + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { // Later feature sees earlier mutation e.EnsureLabel("feature", "two") @@ -165,7 +165,7 @@ func TestMutator_MultiFeature(t *testing.T) { }) // Feature 2: override storage - m.beginFeature() + m.BeginFeature() m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { e.SetStorageRequest(resource.MustParse("10Gi")) return nil From b218c992170fa7256c386e9904583806514834b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:29:25 +0000 Subject: [PATCH 11/21] style: format markdown files with prettier Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 126 ++++++++++++++++++------------- examples/pvc-primitive/README.md | 10 ++- 2 files changed, 81 insertions(+), 55 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index a58e4f79..dd9a1d37 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -1,17 +1,19 @@ # PersistentVolumeClaim Primitive -The `pvc` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolumeClaim` resources. It integrates with the component lifecycle and provides a structured mutation API for managing storage requests and object metadata. +The `pvc` primitive is the framework's built-in integration abstraction for managing Kubernetes `PersistentVolumeClaim` +resources. It integrates with the component lifecycle and provides a structured mutation API for managing storage +requests and object metadata. ## Capabilities -| Capability | Detail | -|---------------------------|------------------------------------------------------------------------------------------------| -| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `Pending`, or `Failing` (Lost) | -| **Immutable field safety**| DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | -| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | -| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | +| Capability | Detail | +| -------------------------- | ---------------------------------------------------------------------------------------------- | +| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `Pending`, or `Failing` (Lost) | +| **Immutable field safety** | DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | +| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | +| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | ## Building a PVC Primitive @@ -41,16 +43,20 @@ resource, err := pvc.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by the API server or other controllers. +`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, then restores server-managed +metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status +subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by +the API server or other controllers. -Kubernetes marks several PVC spec fields as immutable after creation. The default applicator detects existing PVCs (via `ResourceVersion`) and restores these fields from the cluster object: +Kubernetes marks several PVC spec fields as immutable after creation. The default applicator detects existing PVCs (via +`ResourceVersion`) and restores these fields from the cluster object: -| Immutable Field | Preserved On Existing | -|----------------------|-----------------------| -| `accessModes` | Yes | -| `storageClassName` | Yes | -| `volumeMode` | Yes | -| `volumeName` | Yes | +| Immutable Field | Preserved On Existing | +| ------------------ | --------------------- | +| `accessModes` | Yes | +| `storageClassName` | Yes | +| `volumeMode` | Yes | +| `volumeName` | Yes | Use `WithCustomFieldApplicator` when you need different merge semantics: @@ -66,9 +72,11 @@ resource, err := pvc.NewBuilder(base). ## Mutations -Mutations are the primary mechanism for modifying a `PersistentVolumeClaim` 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 `PersistentVolumeClaim` 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 MyStorageMutation(version string) pvc.Mutation { @@ -83,7 +91,8 @@ func MyStorageMutation(version string) pvc.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 @@ -124,14 +133,17 @@ 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 `PersistentVolumeClaim` | -| 2 | Spec edits | PVC spec — storage requests, access modes, etc. | +| Step | Category | What it affects | +| ---- | -------------- | ----------------------------------------------------- | +| 1 | Metadata edits | Labels and annotations on the `PersistentVolumeClaim` | +| 2 | Spec edits | PVC spec — storage requests, access modes, etc. | -Within each category, edits are applied in their registration order. The PVC primitive does not currently group mutations by feature boundary; all applicable edits are applied in a single deterministic sequence rather than guaranteeing that later features observe only fully-applied state from earlier features. +Within each category, edits are applied in their registration order. The PVC primitive does not currently group +mutations by feature boundary; all applicable edits are applied in a single deterministic sequence rather than +guaranteeing that later features observe only fully-applied state from earlier features. ## Editors @@ -148,18 +160,19 @@ m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { Available methods: -| Method | What it does | -|-----------------------|---------------------------------------------------| -| `SetStorageRequest` | Sets `spec.resources.requests[storage]` | -| `SetAccessModes` | Sets `spec.accessModes` (immutable after creation) | +| Method | What it does | +| --------------------- | ------------------------------------------------------- | +| `SetStorageRequest` | Sets `spec.resources.requests[storage]` | +| `SetAccessModes` | Sets `spec.accessModes` (immutable after creation) | | `SetStorageClassName` | Sets `spec.storageClassName` (immutable after creation) | -| `SetVolumeMode` | Sets `spec.volumeMode` (immutable after creation) | -| `SetVolumeName` | Sets `spec.volumeName` (immutable after creation) | -| `Raw` | Returns `*corev1.PersistentVolumeClaimSpec` | +| `SetVolumeMode` | Sets `spec.volumeMode` (immutable after creation) | +| `SetVolumeName` | Sets `spec.volumeName` (immutable after creation) | +| `Raw` | Returns `*corev1.PersistentVolumeClaimSpec` | #### Raw Escape Hatch -`Raw()` returns the underlying `*corev1.PersistentVolumeClaimSpec` for free-form editing when none of the structured methods are sufficient: +`Raw()` returns the underlying `*corev1.PersistentVolumeClaimSpec` for free-form editing when none of the structured +methods are sufficient: ```go m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { @@ -189,15 +202,17 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { The `Mutator` exposes a convenience wrapper for the most common PVC operation: -| Method | Equivalent to | -|--------------------------------|-----------------------------------------------| -| `SetStorageRequest(quantity)` | `EditPVCSpec` → `e.SetStorageRequest(quantity)` | +| Method | Equivalent to | +| ----------------------------- | ----------------------------------------------- | +| `SetStorageRequest(quantity)` | `EditPVCSpec` → `e.SetStorageRequest(quantity)` | -Use this for simple, single-operation mutations. Use `EditPVCSpec` when you need multiple operations or raw access in a single edit block. +Use this for simple, single-operation mutations. Use `EditPVCSpec` when you need multiple operations or raw access in a +single edit block. ## 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 @@ -211,7 +226,8 @@ resource, err := pvc.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 := pvc.NewBuilder(base). @@ -227,13 +243,14 @@ Multiple flavors can be registered and run in registration order. The default handler (`DefaultOperationalStatusHandler`) maps PVC phase to operational status: -| PVC Phase | Status | Reason | -|------------|---------------|----------------------------------| -| `Bound` | Operational | PVC is bound to volume \ | -| `Pending` | Pending | Waiting for PVC to be bound | -| `Lost` | Failing | PVC has lost its bound volume | +| PVC Phase | Status | Reason | +| --------- | ----------- | ------------------------------- | +| `Bound` | Operational | PVC is bound to volume \ | +| `Pending` | Pending | Waiting for PVC to be bound | +| `Lost` | Failing | PVC has lost its bound volume | -Override with `WithCustomOperationalStatus` for additional checks (e.g. verifying specific annotations or volume attributes). +Override with `WithCustomOperationalStatus` for additional checks (e.g. verifying specific annotations or volume +attributes). ### Suspension @@ -243,14 +260,21 @@ PVCs have no runtime state to wind down, so: - `DefaultSuspensionStatusHandler` always reports `Suspended`. - `DefaultDeleteOnSuspendHandler` returns `false` to preserve data. -Override these handlers if you need custom suspension behavior, such as adding annotations when suspended or deleting PVCs that use ephemeral storage. +Override these handlers if you need custom suspension behavior, such as adding annotations when suspended or deleting +PVCs that use ephemeral storage. ## Guidance -**Use `DefaultFieldApplicator` unless you have a specific reason not to.** It handles the immutable field preservation that Kubernetes requires for PVCs, preventing rejected update requests. +**Use `DefaultFieldApplicator` unless you have a specific reason not to.** It handles the immutable field preservation +that Kubernetes requires for PVCs, preventing rejected update requests. -**Register mutations for storage expansion carefully.** Kubernetes only allows expanding PVC storage (not shrinking). Ensure your mutations respect this constraint. The `SetStorageRequest` method does not enforce this — the API server will reject invalid requests. +**Register mutations for storage expansion carefully.** Kubernetes only allows expanding PVC storage (not shrinking). +Ensure your mutations respect this constraint. The `SetStorageRequest` method does not enforce this — the API server +will reject invalid requests. -**Prefer `WithCustomSuspendDeletionDecision` over deleting PVCs manually.** If you need PVCs to be cleaned up during suspension, register a deletion decision handler rather than deleting them in a mutation. +**Prefer `WithCustomSuspendDeletionDecision` over deleting PVCs manually.** If you need PVCs to be cleaned up during +suspension, register a deletion decision handler rather than deleting them in a mutation. -**Use flavors for externally-managed metadata.** If admission webhooks, external controllers, or GitOps tools add labels or annotations to your PVCs, use `PreserveCurrentLabels` and `PreserveCurrentAnnotations` to prevent your operator from removing them. +**Use flavors for externally-managed metadata.** If admission webhooks, external controllers, or GitOps tools add labels +or annotations to your PVCs, use `PreserveCurrentLabels` and `PreserveCurrentAnnotations` to prevent your operator from +removing them. diff --git a/examples/pvc-primitive/README.md b/examples/pvc-primitive/README.md index 6bbacd18..610bef33 100644 --- a/examples/pvc-primitive/README.md +++ b/examples/pvc-primitive/README.md @@ -1,7 +1,7 @@ # PVC Primitive Example -This example demonstrates the usage of the `pvc` primitive within the operator component framework. -It shows how to manage a Kubernetes PersistentVolumeClaim as a component of a larger application, utilising features like: +This example demonstrates the usage of the `pvc` primitive within the operator component framework. It shows how to +manage a Kubernetes PersistentVolumeClaim as a component of a larger application, utilising features like: - **Base Construction**: Initializing a PVC with access modes, storage request, and metadata. - **Feature Mutations**: Applying conditional storage expansion and metadata updates using the `Mutator`. @@ -12,9 +12,10 @@ It shows how to manage a Kubernetes PersistentVolumeClaim as a component of a la ## 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, storage annotation, and conditional large-storage expansion. + - `mutations.go`: version labelling, storage annotation, and conditional large-storage expansion. - `resources/`: Contains the central `NewPVCResource` factory that assembles all features using `pvc.Builder`. - `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. @@ -25,6 +26,7 @@ go run examples/pvc-primitive/main.go ``` This will: + 1. Initialize a fake Kubernetes client. 2. Create an `ExampleApp` owner object. 3. Reconcile through four spec variations, demonstrating version upgrades, storage expansion, and suspension. From ab9b98256b749ce02ad1516f813f38228aad286f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:34:37 +0000 Subject: [PATCH 12/21] fix(pvc): correct interface and status names in comments and docs Update comments in resource.go to reference concepts.Operational, concepts.Suspendable, and concepts.DataExtractable instead of the incorrect component. prefix. Update handlers.go and pvc.md to use the actual OperationPending/OperationFailing status values instead of the shorthand Pending/Failing. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 2 +- pkg/primitives/pvc/handlers.go | 4 ++-- pkg/primitives/pvc/resource.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index dd9a1d37..ad7ad964 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -8,7 +8,7 @@ requests and object metadata. | Capability | Detail | | -------------------------- | ---------------------------------------------------------------------------------------------- | -| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `Pending`, or `Failing` (Lost) | +| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `OperationPending`, or `OperationFailing` (Lost) | | **Immutable field safety** | DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | | **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | | **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | diff --git a/pkg/primitives/pvc/handlers.go b/pkg/primitives/pvc/handlers.go index 005998de..d43b6c18 100644 --- a/pkg/primitives/pvc/handlers.go +++ b/pkg/primitives/pvc/handlers.go @@ -12,8 +12,8 @@ import ( // // It considers a PVC operational when its Status.Phase is Bound: // - Bound → Operational -// - Pending → Pending -// - Lost → Failing +// - Pending → OperationPending +// - Lost → 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 diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go index 5c166fad..5532a3a7 100644 --- a/pkg/primitives/pvc/resource.go +++ b/pkg/primitives/pvc/resource.go @@ -47,9 +47,9 @@ func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) erro // // It implements the following component interfaces: // - component.Resource: for basic identity and mutation behaviour. -// - component.Operational: for tracking whether the PVC is bound and operational. -// - component.Suspendable: for controlled suspension (e.g. retaining the PVC while suspending consumers). -// - component.DataExtractable: for exporting values after successful reconciliation. +// - concepts.Operational: for tracking whether the PVC is bound and operational. +// - concepts.Suspendable: for controlled suspension (e.g. retaining the PVC while suspending consumers). +// - concepts.DataExtractable: for exporting values after successful reconciliation. // // PVC resources follow the Integration lifecycle: they are operationally significant // (a PVC must be Bound to be useful) and support suspension semantics. From 0a752d3a1e9dd61e5e4aa086a5081d9444704e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:45:54 +0000 Subject: [PATCH 13/21] style: format pvc.md with prettier Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index ad7ad964..a483d49f 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -6,14 +6,14 @@ requests and object metadata. ## Capabilities -| Capability | Detail | -| -------------------------- | ---------------------------------------------------------------------------------------------- | +| Capability | Detail | +| -------------------------- | ---------------------------------------------------------------------------------------------------- | | **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `OperationPending`, or `OperationFailing` (Lost) | -| **Immutable field safety** | DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | -| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | -| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | +| **Immutable field safety** | DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | +| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | +| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | ## Building a PVC Primitive From a5835397204f6b9cd8db187dcd43655b150cb7ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:12:54 +0000 Subject: [PATCH 14/21] docs(pvc): fix mutation ordering docs to reflect feature boundary grouping The PVC primitive does group mutations by feature boundary via BeginFeature(), consistent with other primitives. Update the docs to accurately describe this behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index a483d49f..5ab98495 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -141,9 +141,9 @@ recorded: | 1 | Metadata edits | Labels and annotations on the `PersistentVolumeClaim` | | 2 | Spec edits | PVC spec — storage requests, access modes, etc. | -Within each category, edits are applied in their registration order. The PVC primitive does not currently group -mutations by feature boundary; all applicable edits are applied in a single deterministic sequence rather than -guaranteeing that later features observe only fully-applied state from earlier features. +Within each category, edits are applied in their registration order. The PVC primitive groups mutations by feature +boundary: for each applicable feature (after evaluating version constraints and any `When()` conditions), all of its +planned edits are applied in order, and later features and mutations observe the fully-applied state from earlier ones. ## Editors From 7438e0260e26251128588d9c99b19dc8dfb9c10f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:24:34 +0000 Subject: [PATCH 15/21] docs(pvc): use correct operational status names in status table Update the Operational Status table to use `OperationPending` and `OperationFailing` instead of shortened "Pending"/"Failing", matching the framework's actual status names and the capabilities table earlier in the document. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index 5ab98495..02f1d220 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -243,11 +243,11 @@ Multiple flavors can be registered and run in registration order. The default handler (`DefaultOperationalStatusHandler`) maps PVC phase to operational status: -| PVC Phase | Status | Reason | -| --------- | ----------- | ------------------------------- | -| `Bound` | Operational | PVC is bound to volume \ | -| `Pending` | Pending | Waiting for PVC to be bound | -| `Lost` | Failing | PVC has lost its bound volume | +| PVC Phase | Status | Reason | +| --------- | ------------------ | ------------------------------- | +| `Bound` | `Operational` | PVC is bound to volume \ | +| `Pending` | `OperationPending` | Waiting for PVC to be bound | +| `Lost` | `OperationFailing` | PVC has lost its bound volume | Override with `WithCustomOperationalStatus` for additional checks (e.g. verifying specific annotations or volume attributes). From f201f48818feaa7afde1865db36d07507252b161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 02:14:26 +0000 Subject: [PATCH 16/21] test(pvc): add resource-level behavioral tests Add integration tests for the Resource type covering Identity, Object deep copy, Mutate with mutations and field flavors, ConvergingStatus for Bound/Pending/Lost phases, default suspension behavior, and data extraction. These tests validate the generic.IntegrationResource plumbing end-to-end, matching coverage provided by other primitives. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/resource_test.go | 181 ++++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/pkg/primitives/pvc/resource_test.go b/pkg/primitives/pvc/resource_test.go index 9ea39b01..9539f6bc 100644 --- a/pkg/primitives/pvc/resource_test.go +++ b/pkg/primitives/pvc/resource_test.go @@ -3,6 +3,8 @@ package pvc import ( "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" @@ -10,6 +12,185 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func newValidPVC() *corev1.PersistentVolumeClaim { + return &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pvc", + Namespace: "test-ns", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + Resources: corev1.VolumeResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }, + }, + } +} + +func TestResource_Identity(t *testing.T) { + res, err := NewBuilder(newValidPVC()).Build() + require.NoError(t, err) + assert.Equal(t, "v1/PersistentVolumeClaim/test-ns/test-pvc", res.Identity()) +} + +func TestResource_Object(t *testing.T) { + pvc := newValidPVC() + res, err := NewBuilder(pvc).Build() + require.NoError(t, err) + + obj, err := res.Object() + require.NoError(t, err) + + got, ok := obj.(*corev1.PersistentVolumeClaim) + require.True(t, ok) + assert.Equal(t, pvc.Name, got.Name) + assert.Equal(t, pvc.Namespace, got.Namespace) + + // Must be a deep copy. + got.Name = "changed" + assert.Equal(t, "test-pvc", pvc.Name) +} + +func TestResource_Mutate(t *testing.T) { + desired := newValidPVC() + res, err := NewBuilder(desired).Build() + require.NoError(t, err) + + current := &corev1.PersistentVolumeClaim{} + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, resource.MustParse("10Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) +} + +func TestResource_Mutate_WithMutation(t *testing.T) { + desired := newValidPVC() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "increase-storage", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageRequest(resource.MustParse("50Gi")) + return nil + }, + }). + Build() + require.NoError(t, err) + + current := &corev1.PersistentVolumeClaim{} + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, resource.MustParse("50Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) +} + +func TestResource_Mutate_WithFieldApplicationFlavor(t *testing.T) { + desired := newValidPVC() + desired.Labels = map[string]string{"app": "test"} + + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + require.NoError(t, err) + + current := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"external": "preserved"}, + }, + } + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, "test", current.Labels["app"]) + assert.Equal(t, "preserved", current.Labels["external"]) +} + +func TestResource_Mutate_FeatureOrdering(t *testing.T) { + desired := newValidPVC() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "feature-a", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageRequest(resource.MustParse("20Gi")) + return nil + }, + }). + WithMutation(Mutation{ + Name: "feature-b", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.SetStorageRequest(resource.MustParse("30Gi")) + return nil + }, + }). + Build() + require.NoError(t, err) + + current := &corev1.PersistentVolumeClaim{} + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, resource.MustParse("30Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) +} + +func TestResource_ConvergingStatus(t *testing.T) { + tests := []struct { + name string + phase corev1.PersistentVolumeClaimPhase + expectedStatus concepts.OperationalStatus + }{ + {"Bound", corev1.ClaimBound, concepts.OperationalStatusOperational}, + {"Pending", corev1.ClaimPending, concepts.OperationalStatusPending}, + {"Lost", corev1.ClaimLost, concepts.OperationalStatusFailing}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pvc := newValidPVC() + pvc.Status.Phase = tt.phase + res, err := NewBuilder(pvc).Build() + require.NoError(t, err) + + status, err := res.ConvergingStatus(concepts.ConvergingOperationUpdated) + require.NoError(t, err) + assert.Equal(t, tt.expectedStatus, status.Status) + }) + } +} + +func TestResource_DeleteOnSuspend(t *testing.T) { + res, err := NewBuilder(newValidPVC()).Build() + require.NoError(t, err) + assert.False(t, res.DeleteOnSuspend()) +} + +func TestResource_Suspend_And_SuspensionStatus(t *testing.T) { + res, err := NewBuilder(newValidPVC()).Build() + require.NoError(t, err) + + err = res.Suspend() + require.NoError(t, err) + + status, err := res.SuspensionStatus() + require.NoError(t, err) + assert.Equal(t, concepts.SuspensionStatusSuspended, status.Status) +} + +func TestResource_ExtractData(t *testing.T) { + pvc := newValidPVC() + + var extracted resource.Quantity + res, err := NewBuilder(pvc). + WithDataExtractor(func(p corev1.PersistentVolumeClaim) error { + extracted = p.Spec.Resources.Requests[corev1.ResourceStorage] + return nil + }). + Build() + require.NoError(t, err) + + require.NoError(t, res.ExtractData()) + assert.Equal(t, resource.MustParse("10Gi"), extracted) +} + func TestDefaultFieldApplicator(t *testing.T) { storageClass := "fast-ssd" volumeMode := corev1.PersistentVolumeFilesystem From 0a6187a71f475d9a512c401f61b4cfe75f9f01cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 17:57:39 +0000 Subject: [PATCH 17/21] fix(pvc): do not initialize an empty plan on mutator construction Align PVC mutator with deployment/configmap: NewMutator no longer creates an initial feature plan. BeginFeature must be called before registering mutations. Updates all tests accordingly and adds constructor/feature-plan invariant tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/pvc/mutator.go | 8 ++-- pkg/primitives/pvc/mutator_test.go | 67 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/pkg/primitives/pvc/mutator.go b/pkg/primitives/pvc/mutator.go index 31108e27..19d6058d 100644 --- a/pkg/primitives/pvc/mutator.go +++ b/pkg/primitives/pvc/mutator.go @@ -34,13 +34,11 @@ type Mutator struct { } // NewMutator creates a new Mutator for the given PersistentVolumeClaim. +// BeginFeature must be called before registering any mutations. func NewMutator(pvc *corev1.PersistentVolumeClaim) *Mutator { - m := &Mutator{ - pvc: pvc, - plans: []featurePlan{{}}, + return &Mutator{ + pvc: pvc, } - m.active = &m.plans[0] - return m } // BeginFeature starts a new feature planning scope. All subsequent mutation diff --git a/pkg/primitives/pvc/mutator_test.go b/pkg/primitives/pvc/mutator_test.go index 88814720..fe1c8221 100644 --- a/pkg/primitives/pvc/mutator_test.go +++ b/pkg/primitives/pvc/mutator_test.go @@ -23,6 +23,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { }, } m := NewMutator(pvc) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil @@ -34,6 +35,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { t.Run("nil edit ignored", func(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.EditObjectMetadata(nil) require.NoError(t, m.Apply()) }) @@ -41,6 +43,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { t.Run("error propagated", func(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { return errors.New("metadata error") }) @@ -56,6 +59,7 @@ func TestMutator_EditPVCSpec(t *testing.T) { t.Run("sets storage request", func(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { e.SetStorageRequest(resource.MustParse("10Gi")) return nil @@ -68,6 +72,7 @@ func TestMutator_EditPVCSpec(t *testing.T) { t.Run("nil edit ignored", func(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.EditPVCSpec(nil) require.NoError(t, m.Apply()) }) @@ -75,6 +80,7 @@ func TestMutator_EditPVCSpec(t *testing.T) { t.Run("error propagated", func(t *testing.T) { pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.EditPVCSpec(func(_ *editors.PVCSpecEditor) error { return errors.New("spec error") }) @@ -88,6 +94,7 @@ func TestMutator_SetStorageRequest(t *testing.T) { t.Parallel() pvc := &corev1.PersistentVolumeClaim{} m := NewMutator(pvc) + m.BeginFeature() m.SetStorageRequest(resource.MustParse("20Gi")) require.NoError(t, m.Apply()) qty := pvc.Spec.Resources.Requests[corev1.ResourceStorage] @@ -106,6 +113,7 @@ func TestMutator_ExecutionOrder(t *testing.T) { }, } m := NewMutator(pvc) + m.BeginFeature() m.EditPVCSpec(func(_ *editors.PVCSpecEditor) error { order = append(order, "spec") return nil @@ -126,6 +134,7 @@ func TestMutator_ExecutionOrder(t *testing.T) { }, } m := NewMutator(pvc) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("feature", "one") return nil @@ -153,6 +162,7 @@ func TestMutator_MultiFeature(t *testing.T) { }, } m := NewMutator(pvc) + m.BeginFeature() // Feature 1: set label and storage m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { @@ -176,3 +186,60 @@ func TestMutator_MultiFeature(t *testing.T) { qty := pvc.Spec.Resources.Requests[corev1.ResourceStorage] assert.True(t, qty.Equal(resource.MustParse("10Gi"))) } + +// --- Constructor and feature plan invariants --- + +func TestNewMutator_InitializesNoPlan(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + + 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) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + + 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) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + + // Record a mutation in the first feature plan + m.BeginFeature() + m.EditPVCSpec(func(e *editors.PVCSpecEditor) error { + e.SetStorageRequest(resource.MustParse("5Gi")) + return nil + }) + + // Start a new feature and record a different mutation + m.BeginFeature() + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "test") + return nil + }) + + assert.Len(t, m.plans[0].specEdits, 1, "first plan should have one spec edit") + assert.Empty(t, m.plans[0].metadataEdits, "first plan should have no metadata edits") + assert.Len(t, m.plans[1].metadataEdits, 1, "second plan should have one metadata edit") + assert.Empty(t, m.plans[1].specEdits, "second plan should have no spec edits") +} + +func TestMutator_SingleFeature_PlanCount(t *testing.T) { + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + m.BeginFeature() + m.SetStorageRequest(resource.MustParse("1Gi")) + + require.NoError(t, m.Apply()) + assert.Len(t, m.plans, 1) +} From 4bee9e147a4d62e3056bb745655b8f239a4da55a 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:10:46 +0000 Subject: [PATCH 18/21] refactor(pvc): remove field applicators and flavors for SSA migration The framework now uses Server-Side Apply instead of ctrl.CreateOrUpdate, so field applicators and flavors are no longer needed. This removes DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor, the FieldApplicationFlavor type, flavors.go, flavors_test.go, and updates all tests to use the Object()-then-Mutate() pattern. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 79 +------ examples/pvc-primitive/README.md | 2 - examples/pvc-primitive/resources/pvc.go | 8 +- pkg/mutation/editors/pvcspec.go | 8 +- pkg/primitives/pvc/builder.go | 38 +--- pkg/primitives/pvc/builder_test.go | 38 ---- pkg/primitives/pvc/flavors.go | 25 --- pkg/primitives/pvc/flavors_test.go | 144 ------------- pkg/primitives/pvc/resource.go | 43 +--- pkg/primitives/pvc/resource_test.go | 269 ++---------------------- 10 files changed, 34 insertions(+), 620 deletions(-) delete mode 100644 pkg/primitives/pvc/flavors.go delete mode 100644 pkg/primitives/pvc/flavors_test.go diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index 02f1d220..03841bfb 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -6,14 +6,12 @@ requests and object metadata. ## Capabilities -| Capability | Detail | -| -------------------------- | ---------------------------------------------------------------------------------------------------- | -| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `OperationPending`, or `OperationFailing` (Lost) | -| **Immutable field safety** | DefaultFieldApplicator preserves `accessModes`, `storageClassName`, `volumeMode`, `volumeName` | -| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | -| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | +| Capability | Detail | +| ------------------------ | ---------------------------------------------------------------------------------------------------- | +| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `OperationPending`, or `OperationFailing` (Lost) | +| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | +| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | +| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | ## Building a PVC Primitive @@ -36,40 +34,10 @@ base := &corev1.PersistentVolumeClaim{ } resource, err := pvc.NewBuilder(base). - WithFieldApplicationFlavor(pvc.PreserveCurrentLabels). WithMutation(MyStorageMutation(owner.Spec.Version)). Build() ``` -## Default Field Application - -`DefaultFieldApplicator` replaces the current PVC with a deep copy of the desired object, then restores server-managed -metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the Status -subresource from the original live object. This prevents spec-level reconciliation from clearing status data written by -the API server or other controllers. - -Kubernetes marks several PVC spec fields as immutable after creation. The default applicator detects existing PVCs (via -`ResourceVersion`) and restores these fields from the cluster object: - -| Immutable Field | Preserved On Existing | -| ------------------ | --------------------- | -| `accessModes` | Yes | -| `storageClassName` | Yes | -| `volumeMode` | Yes | -| `volumeName` | Yes | - -Use `WithCustomFieldApplicator` when you need different merge semantics: - -```go -resource, err := pvc.NewBuilder(base). - WithCustomFieldApplicator(func(current, desired *corev1.PersistentVolumeClaim) error { - // Only update the storage request; leave everything else untouched. - current.Spec.Resources.Requests = desired.Spec.Resources.Requests - return nil - }). - Build() -``` - ## Mutations Mutations are the primary mechanism for modifying a `PersistentVolumeClaim` beyond its baseline. Each mutation is a @@ -209,34 +177,6 @@ The `Mutator` exposes a convenience wrapper for the most common PVC operation: Use this for simple, single-operation mutations. Use `EditPVCSpec` when you need multiple operations or raw access in a single edit block. -## 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 := pvc.NewBuilder(base). - WithFieldApplicationFlavor(pvc.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 := pvc.NewBuilder(base). - WithFieldApplicationFlavor(pvc.PreserveCurrentAnnotations). - Build() -``` - -Multiple flavors can be registered and run in registration order. - ## Status Handlers ### Operational Status @@ -265,16 +205,9 @@ PVCs that use ephemeral storage. ## Guidance -**Use `DefaultFieldApplicator` unless you have a specific reason not to.** It handles the immutable field preservation -that Kubernetes requires for PVCs, preventing rejected update requests. - **Register mutations for storage expansion carefully.** Kubernetes only allows expanding PVC storage (not shrinking). Ensure your mutations respect this constraint. The `SetStorageRequest` method does not enforce this — the API server will reject invalid requests. **Prefer `WithCustomSuspendDeletionDecision` over deleting PVCs manually.** If you need PVCs to be cleaned up during suspension, register a deletion decision handler rather than deleting them in a mutation. - -**Use flavors for externally-managed metadata.** If admission webhooks, external controllers, or GitOps tools add labels -or annotations to your PVCs, use `PreserveCurrentLabels` and `PreserveCurrentAnnotations` to prevent your operator from -removing them. diff --git a/examples/pvc-primitive/README.md b/examples/pvc-primitive/README.md index 610bef33..9f7e123b 100644 --- a/examples/pvc-primitive/README.md +++ b/examples/pvc-primitive/README.md @@ -5,8 +5,6 @@ manage a Kubernetes PersistentVolumeClaim as a component of a larger application - **Base Construction**: Initializing a PVC with access modes, storage request, and metadata. - **Feature Mutations**: Applying conditional storage expansion and metadata updates using the `Mutator`. -- **Immutable Field Safety**: The `DefaultFieldApplicator` preserves immutable PVC fields on existing resources. -- **Field Flavors**: Preserving labels and annotations managed by external tools. - **Suspension**: PVCs are immediately suspended with data preserved by default. - **Data Extraction**: Harvesting PVC status after each reconcile cycle. diff --git a/examples/pvc-primitive/resources/pvc.go b/examples/pvc-primitive/resources/pvc.go index 68581e4d..3689f3d0 100644 --- a/examples/pvc-primitive/resources/pvc.go +++ b/examples/pvc-primitive/resources/pvc.go @@ -42,11 +42,7 @@ func NewPVCResource(owner *app.ExampleApp) (component.Resource, error) { builder.WithMutation(features.StorageAnnotationMutation(owner.Spec.Version, "standard")) builder.WithMutation(features.LargeStorageMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) - // 4. Preserve labels and annotations added by external controllers. - builder.WithFieldApplicationFlavor(pvc.PreserveCurrentLabels) - builder.WithFieldApplicationFlavor(pvc.PreserveCurrentAnnotations) - - // 5. Extract data from the reconciled PVC. + // 4. Extract data from the reconciled PVC. builder.WithDataExtractor(func(p corev1.PersistentVolumeClaim) error { fmt.Printf("Reconciled PVC: %s\n", p.Name) fmt.Printf(" Phase: %s\n", p.Status.Phase) @@ -56,6 +52,6 @@ func NewPVCResource(owner *app.ExampleApp) (component.Resource, error) { return nil }) - // 6. Build the final resource. + // 5. Build the final resource. return builder.Build() } diff --git a/pkg/mutation/editors/pvcspec.go b/pkg/mutation/editors/pvcspec.go index 6d8a763a..1b78f4ac 100644 --- a/pkg/mutation/editors/pvcspec.go +++ b/pkg/mutation/editors/pvcspec.go @@ -36,7 +36,7 @@ func (e *PVCSpecEditor) SetStorageRequest(quantity resource.Quantity) { // SetAccessModes sets the access modes for the PVC. // // Access modes are immutable on existing PVCs. This method is intended for -// initial construction; the DefaultFieldApplicator preserves existing values. +// initial construction; immutable fields are preserved by Server-Side Apply. func (e *PVCSpecEditor) SetAccessModes(modes []corev1.PersistentVolumeAccessMode) { e.spec.AccessModes = modes } @@ -44,7 +44,7 @@ func (e *PVCSpecEditor) SetAccessModes(modes []corev1.PersistentVolumeAccessMode // SetStorageClassName sets the storage class name for the PVC. // // The storage class name is immutable on existing PVCs. This method is intended -// for initial construction; the DefaultFieldApplicator preserves existing values. +// for initial construction; immutable fields are preserved by Server-Side Apply. func (e *PVCSpecEditor) SetStorageClassName(name string) { e.spec.StorageClassName = &name } @@ -52,7 +52,7 @@ func (e *PVCSpecEditor) SetStorageClassName(name string) { // SetVolumeMode sets the volume mode (Filesystem or Block) for the PVC. // // The volume mode is immutable on existing PVCs. This method is intended for -// initial construction; the DefaultFieldApplicator preserves existing values. +// initial construction; immutable fields are preserved by Server-Side Apply. func (e *PVCSpecEditor) SetVolumeMode(mode corev1.PersistentVolumeMode) { e.spec.VolumeMode = &mode } @@ -60,7 +60,7 @@ func (e *PVCSpecEditor) SetVolumeMode(mode corev1.PersistentVolumeMode) { // SetVolumeName binds the PVC to a specific PersistentVolume by name. // // The volume name is immutable on existing PVCs. This method is intended for -// initial construction; the DefaultFieldApplicator preserves existing values. +// initial construction; immutable fields are preserved by Server-Side Apply. func (e *PVCSpecEditor) SetVolumeName(name string) { e.spec.VolumeName = name } diff --git a/pkg/primitives/pvc/builder.go b/pkg/primitives/pvc/builder.go index 784570cc..99fdb204 100644 --- a/pkg/primitives/pvc/builder.go +++ b/pkg/primitives/pvc/builder.go @@ -11,9 +11,9 @@ import ( // Builder is a configuration helper for creating and customizing a PVC Resource. // -// It provides a fluent API for registering mutations, field application flavors, -// 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, 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.PersistentVolumeClaim, *Mutator] } @@ -21,8 +21,7 @@ type Builder struct { // NewBuilder initializes a new Builder with the provided PersistentVolumeClaim object. // // The PVC 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. +// will make the cluster's state match this base, modified by any registered mutations. // // The provided PVC must have both Name and Namespace set, which is validated during // the Build() call. @@ -34,7 +33,6 @@ func NewBuilder(pvc *corev1.PersistentVolumeClaim) *Builder { base := generic.NewIntegrationBuilder[*corev1.PersistentVolumeClaim, *Mutator]( pvc, identityFunc, - DefaultFieldApplicator, NewMutator, ) @@ -51,8 +49,7 @@ func NewBuilder(pvc *corev1.PersistentVolumeClaim) *Builder { // WithMutation registers a mutation for the PVC. // -// Mutations are applied sequentially during the Mutate() phase of reconciliation, -// after the baseline field applicator and any registered flavors have run. +// Mutations are applied sequentially during the Mutate() phase of reconciliation. // A mutation with a nil Feature is applied unconditionally; one with a non-nil // Feature is applied only when that feature is enabled. func (b *Builder) WithMutation(m Mutation) *Builder { @@ -60,31 +57,6 @@ func (b *Builder) WithMutation(m Mutation) *Builder { return b } -// WithCustomFieldApplicator sets a custom strategy for applying the desired -// state to the existing PVC in the cluster. -// -// The default applicator (DefaultFieldApplicator) replaces the current object -// with a deep copy of the desired object while preserving immutable fields on -// existing PVCs. Use a custom applicator when you need different merge semantics. -func (b *Builder) WithCustomFieldApplicator( - applicator func(current, desired *corev1.PersistentVolumeClaim) 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.PersistentVolumeClaim](flavor)) - return b -} - // WithCustomOperationalStatus overrides the default logic for determining if the // PVC has reached its desired operational state. // diff --git a/pkg/primitives/pvc/builder_test.go b/pkg/primitives/pvc/builder_test.go index a7d90b70..34a990c7 100644 --- a/pkg/primitives/pvc/builder_test.go +++ b/pkg/primitives/pvc/builder_test.go @@ -93,44 +93,6 @@ func TestBuilder(t *testing.T) { assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) }) - t.Run("WithCustomFieldApplicator", func(t *testing.T) { - t.Parallel() - pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: "test-ns", - }, - } - applied := false - applicator := func(_, _ *corev1.PersistentVolumeClaim) error { - applied = true - return nil - } - res, err := NewBuilder(pvc). - WithCustomFieldApplicator(applicator). - Build() - require.NoError(t, err) - require.NotNil(t, res.base.CustomFieldApplicator) - _ = res.base.CustomFieldApplicator(nil, nil) - assert.True(t, applied) - }) - - t.Run("WithFieldApplicationFlavor", func(t *testing.T) { - t.Parallel() - pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: "test-ns", - }, - } - res, err := NewBuilder(pvc). - WithFieldApplicationFlavor(PreserveCurrentLabels). - WithFieldApplicationFlavor(nil). - Build() - require.NoError(t, err) - assert.Len(t, res.base.FieldFlavors, 1) - }) - t.Run("WithCustomOperationalStatus", func(t *testing.T) { t.Parallel() pvc := &corev1.PersistentVolumeClaim{ diff --git a/pkg/primitives/pvc/flavors.go b/pkg/primitives/pvc/flavors.go deleted file mode 100644 index 6ea8ab26..00000000 --- a/pkg/primitives/pvc/flavors.go +++ /dev/null @@ -1,25 +0,0 @@ -package pvc - -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 -// PVC 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.PersistentVolumeClaim] - -// PreserveCurrentLabels ensures that any labels present on the current live -// PVC 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.PersistentVolumeClaim) error { - return flavors.PreserveCurrentLabels[*corev1.PersistentVolumeClaim]()(applied, current, desired) -} - -// PreserveCurrentAnnotations ensures that any annotations present on the current -// live PVC 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.PersistentVolumeClaim) error { - return flavors.PreserveCurrentAnnotations[*corev1.PersistentVolumeClaim]()(applied, current, desired) -} diff --git a/pkg/primitives/pvc/flavors_test.go b/pkg/primitives/pvc/flavors_test.go deleted file mode 100644 index 319b2675..00000000 --- a/pkg/primitives/pvc/flavors_test.go +++ /dev/null @@ -1,144 +0,0 @@ -package pvc - -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 TestPreserveCurrentLabels(t *testing.T) { - t.Parallel() - - t.Run("preserves missing labels", func(t *testing.T) { - applied := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "myapp"}, - }, - } - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "myapp", "external": "value"}, - }, - } - desired := &corev1.PersistentVolumeClaim{} - - err := PreserveCurrentLabels(applied, current, desired) - require.NoError(t, err) - assert.Equal(t, "myapp", applied.Labels["app"]) - assert.Equal(t, "value", applied.Labels["external"]) - }) - - t.Run("applied wins on overlap", func(t *testing.T) { - applied := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "new"}, - }, - } - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "old"}, - }, - } - desired := &corev1.PersistentVolumeClaim{} - - err := PreserveCurrentLabels(applied, current, desired) - require.NoError(t, err) - assert.Equal(t, "new", applied.Labels["app"]) - }) - - t.Run("nil labels on current", func(t *testing.T) { - applied := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"app": "myapp"}, - }, - } - current := &corev1.PersistentVolumeClaim{} - desired := &corev1.PersistentVolumeClaim{} - - err := PreserveCurrentLabels(applied, current, desired) - require.NoError(t, err) - assert.Equal(t, "myapp", applied.Labels["app"]) - }) -} - -func TestPreserveCurrentAnnotations(t *testing.T) { - t.Parallel() - - t.Run("preserves missing annotations", func(t *testing.T) { - applied := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"managed": "true"}, - }, - } - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"managed": "true", "external": "injected"}, - }, - } - desired := &corev1.PersistentVolumeClaim{} - - err := PreserveCurrentAnnotations(applied, current, desired) - require.NoError(t, err) - assert.Equal(t, "true", applied.Annotations["managed"]) - assert.Equal(t, "injected", applied.Annotations["external"]) - }) - - t.Run("nil annotations on current", func(t *testing.T) { - applied := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"managed": "true"}, - }, - } - current := &corev1.PersistentVolumeClaim{} - desired := &corev1.PersistentVolumeClaim{} - - err := PreserveCurrentAnnotations(applied, current, desired) - require.NoError(t, err) - assert.Equal(t, "true", applied.Annotations["managed"]) - }) -} - -func TestFlavorsViaBuilder(t *testing.T) { - t.Parallel() - - base := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-pvc", - Namespace: "default", - Labels: map[string]string{ - "app": "myapp", - }, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("5Gi"), - }, - }, - }, - } - - res, err := NewBuilder(base). - WithFieldApplicationFlavor(PreserveCurrentLabels). - WithFieldApplicationFlavor(PreserveCurrentAnnotations). - Build() - require.NoError(t, err) - - current := base.DeepCopy() - current.Labels["external-label"] = "preserved" - current.Annotations = map[string]string{"external-annotation": "preserved"} - - err = res.Mutate(current) - require.NoError(t, err) - - obj, err := res.Object() - require.NoError(t, err) - pvc := obj.(*corev1.PersistentVolumeClaim) - assert.Equal(t, "preserved", pvc.Labels["external-label"]) - assert.Equal(t, "preserved", pvc.Annotations["external-annotation"]) -} diff --git a/pkg/primitives/pvc/resource.go b/pkg/primitives/pvc/resource.go index 5532a3a7..955d5bb8 100644 --- a/pkg/primitives/pvc/resource.go +++ b/pkg/primitives/pvc/resource.go @@ -7,41 +7,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator replaces current with a deep copy of desired, preserving -// server-managed metadata (ResourceVersion, UID, Generation, etc.), shared-controller -// fields (OwnerReferences, Finalizers), the Status subresource, and immutable PVC -// spec fields on existing resources. -// -// Kubernetes marks several PVC spec fields as immutable after creation: accessModes, -// storageClassName, volumeMode, and volumeName. If the current object has a non-empty -// ResourceVersion (indicating it already exists in the cluster), those fields are -// restored from the current object after the deep copy. -// -// Use a custom field applicator via Builder.WithCustomFieldApplicator if you need -// different merge semantics. -func DefaultFieldApplicator(current, desired *corev1.PersistentVolumeClaim) error { - isExisting := current.ResourceVersion != "" - original := current.DeepCopy() - - savedAccessModes := current.Spec.AccessModes - savedStorageClassName := current.Spec.StorageClassName - savedVolumeMode := current.Spec.VolumeMode - savedVolumeName := current.Spec.VolumeName - - *current = *desired.DeepCopy() - generic.PreserveServerManagedFields(current, original) - generic.PreserveStatus(current, original) - - if isExisting { - current.Spec.AccessModes = savedAccessModes - current.Spec.StorageClassName = savedStorageClassName - current.Spec.VolumeMode = savedVolumeMode - current.Spec.VolumeName = savedVolumeName - } - - return nil -} - // Resource is a high-level abstraction for managing a Kubernetes PersistentVolumeClaim // within a controller's reconciliation loop. // @@ -75,11 +40,9 @@ func (r *Resource) Object() (client.Object, error) { // desired state. // // The mutation process follows this order: -// 1. Field application: the current object is updated to reflect the desired base state, -// using either DefaultFieldApplicator or a custom applicator if one is configured. -// 2. Field application flavors: any registered flavors are applied in registration order. -// 3. Feature mutations: all registered feature-gated mutations are applied in order. -// 4. Suspension: if the resource is in a suspending state, the suspension logic is applied. +// 1. The desired base state is applied to the current object. +// 2. Feature mutations: all registered feature-gated mutations are applied in order. +// 3. Suspension: if the resource is in a suspending state, the suspension logic is applied. // // 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/pvc/resource_test.go b/pkg/primitives/pvc/resource_test.go index 9539f6bc..88275f00 100644 --- a/pkg/primitives/pvc/resource_test.go +++ b/pkg/primitives/pvc/resource_test.go @@ -58,10 +58,12 @@ func TestResource_Mutate(t *testing.T) { res, err := NewBuilder(desired).Build() require.NoError(t, err) - current := &corev1.PersistentVolumeClaim{} - require.NoError(t, res.Mutate(current)) + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.Equal(t, resource.MustParse("10Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) + got := obj.(*corev1.PersistentVolumeClaim) + assert.Equal(t, resource.MustParse("10Gi"), got.Spec.Resources.Requests[corev1.ResourceStorage]) } func TestResource_Mutate_WithMutation(t *testing.T) { @@ -78,30 +80,12 @@ func TestResource_Mutate_WithMutation(t *testing.T) { Build() require.NoError(t, err) - current := &corev1.PersistentVolumeClaim{} - require.NoError(t, res.Mutate(current)) - - assert.Equal(t, resource.MustParse("50Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) -} - -func TestResource_Mutate_WithFieldApplicationFlavor(t *testing.T) { - desired := newValidPVC() - desired.Labels = map[string]string{"app": "test"} - - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(PreserveCurrentLabels). - Build() + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"external": "preserved"}, - }, - } - require.NoError(t, res.Mutate(current)) - - assert.Equal(t, "test", current.Labels["app"]) - assert.Equal(t, "preserved", current.Labels["external"]) + got := obj.(*corev1.PersistentVolumeClaim) + assert.Equal(t, resource.MustParse("50Gi"), got.Spec.Resources.Requests[corev1.ResourceStorage]) } func TestResource_Mutate_FeatureOrdering(t *testing.T) { @@ -126,10 +110,12 @@ func TestResource_Mutate_FeatureOrdering(t *testing.T) { Build() require.NoError(t, err) - current := &corev1.PersistentVolumeClaim{} - require.NoError(t, res.Mutate(current)) + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.Equal(t, resource.MustParse("30Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) + got := obj.(*corev1.PersistentVolumeClaim) + assert.Equal(t, resource.MustParse("30Gi"), got.Spec.Resources.Requests[corev1.ResourceStorage]) } func TestResource_ConvergingStatus(t *testing.T) { @@ -190,230 +176,3 @@ func TestResource_ExtractData(t *testing.T) { require.NoError(t, res.ExtractData()) assert.Equal(t, resource.MustParse("10Gi"), extracted) } - -func TestDefaultFieldApplicator(t *testing.T) { - storageClass := "fast-ssd" - volumeMode := corev1.PersistentVolumeFilesystem - - tests := []struct { - name string - current *corev1.PersistentVolumeClaim - desired *corev1.PersistentVolumeClaim - verify func(t *testing.T, result *corev1.PersistentVolumeClaim) - }{ - { - name: "new PVC uses desired immutable fields", - current: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - }, - desired: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - StorageClassName: &storageClass, - VolumeMode: &volumeMode, - VolumeName: "pv-001", - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - }, - }, - }, - verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { - assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, result.Spec.AccessModes) - assert.Equal(t, &storageClass, result.Spec.StorageClassName) - assert.Equal(t, &volumeMode, result.Spec.VolumeMode) - assert.Equal(t, "pv-001", result.Spec.VolumeName) - assert.Equal(t, resource.MustParse("10Gi"), result.Spec.Resources.Requests[corev1.ResourceStorage]) - }, - }, - { - name: "existing PVC preserves immutable fields from current", - current: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - ResourceVersion: "12345", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, - StorageClassName: &storageClass, - VolumeMode: &volumeMode, - VolumeName: "pv-original", - }, - }, - desired: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, - StorageClassName: func() *string { s := "slow-hdd"; return &s }(), - VolumeMode: func() *corev1.PersistentVolumeMode { m := corev1.PersistentVolumeBlock; return &m }(), - VolumeName: "pv-new", - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("20Gi"), - }, - }, - }, - }, - verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { - // Immutable fields should be preserved from current. - assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, result.Spec.AccessModes) - assert.Equal(t, &storageClass, result.Spec.StorageClassName) - assert.Equal(t, &volumeMode, result.Spec.VolumeMode) - assert.Equal(t, "pv-original", result.Spec.VolumeName) - // Mutable fields should come from desired. - assert.Equal(t, resource.MustParse("20Gi"), result.Spec.Resources.Requests[corev1.ResourceStorage]) - }, - }, - { - name: "existing PVC with empty ResourceVersion treated as new", - current: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - ResourceVersion: "", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, - VolumeName: "pv-original", - }, - }, - desired: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, - VolumeName: "pv-new", - }, - }, - verify: func(t *testing.T, result *corev1.PersistentVolumeClaim) { - // No ResourceVersion means new — desired fields should be used. - assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, result.Spec.AccessModes) - assert.Equal(t, "pv-new", result.Spec.VolumeName) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := DefaultFieldApplicator(tt.current, tt.desired) - require.NoError(t, err) - tt.verify(t, tt.current) - }) - } -} - -func TestDefaultFieldApplicator_PreservesStatus(t *testing.T) { - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - Status: corev1.PersistentVolumeClaimStatus{ - Phase: corev1.ClaimBound, - Capacity: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - }, - } - desired := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("20Gi"), - }, - }, - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // Desired spec is applied - assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) - - // Status from the live object is preserved - assert.Equal(t, corev1.ClaimBound, current.Status.Phase) - assert.Equal(t, resource.MustParse("10Gi"), current.Status.Capacity[corev1.ResourceStorage]) -} - -func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { - storageClass := "fast-ssd" - volumeMode := corev1.PersistentVolumeFilesystem - - current := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - ResourceVersion: "12345", - UID: "abc-def", - Generation: 3, - OwnerReferences: []metav1.OwnerReference{ - {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, - }, - Finalizers: []string{"finalizer.example.com"}, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, - StorageClassName: &storageClass, - VolumeMode: &volumeMode, - VolumeName: "pv-001", - }, - } - desired := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "app-data", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}, - VolumeName: "pv-new", - Resources: corev1.VolumeResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("20Gi"), - }, - }, - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // Desired spec (mutable fields) and labels are applied - assert.Equal(t, resource.MustParse("20Gi"), current.Spec.Resources.Requests[corev1.ResourceStorage]) - assert.Equal(t, "test", current.Labels["app"]) - - // Immutable PVC fields are preserved - assert.Equal(t, []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, current.Spec.AccessModes) - assert.Equal(t, &storageClass, current.Spec.StorageClassName) - assert.Equal(t, &volumeMode, current.Spec.VolumeMode) - assert.Equal(t, "pv-001", current.Spec.VolumeName) - - // Server-managed fields are preserved - assert.Equal(t, "12345", current.ResourceVersion) - assert.Equal(t, "abc-def", string(current.UID)) - assert.Equal(t, int64(3), current.Generation) - - // Shared-controller fields are preserved - assert.Len(t, current.OwnerReferences, 1) - assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) - assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) -} From c6b4d4b74b12e03294fff0165b3fe13108e20ad9 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:26:12 +0000 Subject: [PATCH 19/21] fix(pvc): address Copilot review comments - Add panic guards with clear messages when EditObjectMetadata/EditPVCSpec are called before BeginFeature, preventing nil-pointer dereferences - Handle empty VolumeName in Bound status to produce a meaningful reason - Update docs to use Go constant names (OperationalStatusPending, etc.) - Add tests for new panic behavior and empty volume name case Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/pvc.md | 8 ++++---- pkg/primitives/pvc/handlers.go | 6 +++++- pkg/primitives/pvc/handlers_test.go | 12 +++++++++++- pkg/primitives/pvc/mutator.go | 10 ++++++++++ pkg/primitives/pvc/mutator_test.go | 20 ++++++++++++++++++++ 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index 03841bfb..2ee43aa9 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -8,7 +8,7 @@ requests and object metadata. | Capability | Detail | | ------------------------ | ---------------------------------------------------------------------------------------------------- | -| **Operational tracking** | Monitors PVC phase — reports `Operational` (Bound), `OperationPending`, or `OperationFailing` (Lost) | +| **Operational tracking** | Monitors PVC phase — reports `OperationalStatusOperational` (Bound), `OperationalStatusPending`, or `OperationalStatusFailing` (Lost) | | **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | | **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | | **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | @@ -185,9 +185,9 @@ The default handler (`DefaultOperationalStatusHandler`) maps PVC phase to operat | PVC Phase | Status | Reason | | --------- | ------------------ | ------------------------------- | -| `Bound` | `Operational` | PVC is bound to volume \ | -| `Pending` | `OperationPending` | Waiting for PVC to be bound | -| `Lost` | `OperationFailing` | PVC has lost its bound volume | +| `Bound` | `OperationalStatusOperational` | PVC is bound to volume \ | +| `Pending` | `OperationalStatusPending` | Waiting for PVC to be bound | +| `Lost` | `OperationalStatusFailing` | PVC has lost its bound volume | Override with `WithCustomOperationalStatus` for additional checks (e.g. verifying specific annotations or volume attributes). diff --git a/pkg/primitives/pvc/handlers.go b/pkg/primitives/pvc/handlers.go index d43b6c18..0f20b686 100644 --- a/pkg/primitives/pvc/handlers.go +++ b/pkg/primitives/pvc/handlers.go @@ -23,9 +23,13 @@ func DefaultOperationalStatusHandler( ) (concepts.OperationalStatusWithReason, error) { switch pvc.Status.Phase { case corev1.ClaimBound: + reason := "PVC is bound" + if pvc.Spec.VolumeName != "" { + reason = fmt.Sprintf("PVC is bound to volume %s", pvc.Spec.VolumeName) + } return concepts.OperationalStatusWithReason{ Status: concepts.OperationalStatusOperational, - Reason: fmt.Sprintf("PVC is bound to volume %s", pvc.Spec.VolumeName), + Reason: reason, }, nil case corev1.ClaimLost: return concepts.OperationalStatusWithReason{ diff --git a/pkg/primitives/pvc/handlers_test.go b/pkg/primitives/pvc/handlers_test.go index dd0c8c0e..591b3892 100644 --- a/pkg/primitives/pvc/handlers_test.go +++ b/pkg/primitives/pvc/handlers_test.go @@ -17,7 +17,7 @@ func TestDefaultOperationalStatusHandler(t *testing.T) { wantReason string }{ { - name: "bound", + name: "bound with volume name", pvc: &corev1.PersistentVolumeClaim{ Spec: corev1.PersistentVolumeClaimSpec{ VolumeName: "pv-001", @@ -29,6 +29,16 @@ func TestDefaultOperationalStatusHandler(t *testing.T) { wantStatus: concepts.OperationalStatusOperational, wantReason: "PVC is bound to volume pv-001", }, + { + name: "bound without volume name", + pvc: &corev1.PersistentVolumeClaim{ + Status: corev1.PersistentVolumeClaimStatus{ + Phase: corev1.ClaimBound, + }, + }, + wantStatus: concepts.OperationalStatusOperational, + wantReason: "PVC is bound", + }, { name: "pending", pvc: &corev1.PersistentVolumeClaim{ diff --git a/pkg/primitives/pvc/mutator.go b/pkg/primitives/pvc/mutator.go index 19d6058d..7495f5b2 100644 --- a/pkg/primitives/pvc/mutator.go +++ b/pkg/primitives/pvc/mutator.go @@ -52,10 +52,15 @@ func (m *Mutator) BeginFeature() { // // Metadata edits are applied before spec edits within the same feature. // A nil edit function is ignored. +// +// Panics if BeginFeature has not been called. func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) { if edit == nil { return } + if m.active == nil { + panic("pvc.Mutator: EditObjectMetadata called before BeginFeature") + } m.active.metadataEdits = append(m.active.metadataEdits, edit) } @@ -66,10 +71,15 @@ func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) // within the same feature, in registration order. // // A nil edit function is ignored. +// +// Panics if BeginFeature has not been called. func (m *Mutator) EditPVCSpec(edit func(*editors.PVCSpecEditor) error) { if edit == nil { return } + if m.active == nil { + panic("pvc.Mutator: EditPVCSpec called before BeginFeature") + } m.active.specEdits = append(m.active.specEdits, edit) } diff --git a/pkg/primitives/pvc/mutator_test.go b/pkg/primitives/pvc/mutator_test.go index fe1c8221..c55ac57d 100644 --- a/pkg/primitives/pvc/mutator_test.go +++ b/pkg/primitives/pvc/mutator_test.go @@ -187,6 +187,26 @@ func TestMutator_MultiFeature(t *testing.T) { assert.True(t, qty.Equal(resource.MustParse("10Gi"))) } +// --- Panic guards --- + +func TestMutator_EditObjectMetadata_PanicsWithoutBeginFeature(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + assert.PanicsWithValue(t, "pvc.Mutator: EditObjectMetadata called before BeginFeature", func() { + m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { return nil }) + }) +} + +func TestMutator_EditPVCSpec_PanicsWithoutBeginFeature(t *testing.T) { + t.Parallel() + pvc := &corev1.PersistentVolumeClaim{} + m := NewMutator(pvc) + assert.PanicsWithValue(t, "pvc.Mutator: EditPVCSpec called before BeginFeature", func() { + m.EditPVCSpec(func(_ *editors.PVCSpecEditor) error { return nil }) + }) +} + // --- Constructor and feature plan invariants --- func TestNewMutator_InitializesNoPlan(t *testing.T) { From 73bb7038f5ce233c3dba5d267adaa458addbf853 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:46:58 +0000 Subject: [PATCH 20/21] fix formatting --- docs/primitives/pvc.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/primitives/pvc.md b/docs/primitives/pvc.md index 2ee43aa9..b1c0687c 100644 --- a/docs/primitives/pvc.md +++ b/docs/primitives/pvc.md @@ -6,12 +6,12 @@ requests and object metadata. ## Capabilities -| Capability | Detail | -| ------------------------ | ---------------------------------------------------------------------------------------------------- | +| Capability | Detail | +| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------- | | **Operational tracking** | Monitors PVC phase — reports `OperationalStatusOperational` (Bound), `OperationalStatusPending`, or `OperationalStatusFailing` (Lost) | -| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | -| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | -| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | +| **Suspension** | PVCs are immediately suspended (no runtime state to wind down); data is preserved by default | +| **Mutation pipeline** | Typed editors for PVC spec and object metadata, with a raw escape hatch for free-form access | +| **Data extraction** | Reads bound volume name, capacity, or other status fields after each sync cycle | ## Building a PVC Primitive @@ -183,8 +183,8 @@ single edit block. The default handler (`DefaultOperationalStatusHandler`) maps PVC phase to operational status: -| PVC Phase | Status | Reason | -| --------- | ------------------ | ------------------------------- | +| PVC Phase | Status | Reason | +| --------- | ------------------------------ | ------------------------------- | | `Bound` | `OperationalStatusOperational` | PVC is bound to volume \ | | `Pending` | `OperationalStatusPending` | Waiting for PVC to be bound | | `Lost` | `OperationalStatusFailing` | PVC has lost its bound volume | From 9cd3be29fd690dab4f07de9e00324277e4650a0c 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:57:52 +0000 Subject: [PATCH 21/21] fix(pvc): clarify immutable field comments and add PVCSpecEditor to docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reword comments on immutable PVC field setters (accessModes, storageClassName, volumeMode, volumeName) to clarify that SSA does not preserve changed values — the API server rejects the patch. Also add PVCSpecEditor to the Mutation Editors table in primitives.md. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 1 + pkg/mutation/editors/pvcspec.go | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index bb04bd1c..8f424fc0 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -120,6 +120,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 | +| `PVCSpecEditor` | Access modes, storage class, volume mode, storage requests | | `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 diff --git a/pkg/mutation/editors/pvcspec.go b/pkg/mutation/editors/pvcspec.go index 1b78f4ac..f7907843 100644 --- a/pkg/mutation/editors/pvcspec.go +++ b/pkg/mutation/editors/pvcspec.go @@ -35,32 +35,40 @@ func (e *PVCSpecEditor) SetStorageRequest(quantity resource.Quantity) { // SetAccessModes sets the access modes for the PVC. // -// Access modes are immutable on existing PVCs. This method is intended for -// initial construction; immutable fields are preserved by Server-Side Apply. +// Access modes are immutable on existing PVCs. This method should be used for +// initial construction, or when reapplying the same value via Server-Side Apply. +// Attempting to change this field on an existing PVC will be rejected by the +// Kubernetes API server. func (e *PVCSpecEditor) SetAccessModes(modes []corev1.PersistentVolumeAccessMode) { e.spec.AccessModes = modes } // SetStorageClassName sets the storage class name for the PVC. // -// The storage class name is immutable on existing PVCs. This method is intended -// for initial construction; immutable fields are preserved by Server-Side Apply. +// The storage class name is immutable on existing PVCs. This method should be +// used for initial construction, or when reapplying the same value via +// Server-Side Apply. Attempting to change this field on an existing PVC will be +// rejected by the Kubernetes API server. func (e *PVCSpecEditor) SetStorageClassName(name string) { e.spec.StorageClassName = &name } // SetVolumeMode sets the volume mode (Filesystem or Block) for the PVC. // -// The volume mode is immutable on existing PVCs. This method is intended for -// initial construction; immutable fields are preserved by Server-Side Apply. +// The volume mode is immutable on existing PVCs. This method should be used +// for initial construction, or when reapplying the same value via Server-Side +// Apply. Attempting to change this field on an existing PVC will be rejected +// by the Kubernetes API server. func (e *PVCSpecEditor) SetVolumeMode(mode corev1.PersistentVolumeMode) { e.spec.VolumeMode = &mode } // SetVolumeName binds the PVC to a specific PersistentVolume by name. // -// The volume name is immutable on existing PVCs. This method is intended for -// initial construction; immutable fields are preserved by Server-Side Apply. +// The volume name is immutable on existing PVCs. This method should be used +// for initial construction, or when reapplying the same value via Server-Side +// Apply. Attempting to change this field on an existing PVC will be rejected +// by the Kubernetes API server. func (e *PVCSpecEditor) SetVolumeName(name string) { e.spec.VolumeName = name }