From f42fe262f94f22bc10eb52fd18dce64b91369056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:40:54 +0000 Subject: [PATCH 01/21] Add serviceaccount primitive package Implement the ServiceAccount static primitive with builder, resource, mutator, and flavors. The mutator exposes direct methods for imagePullSecrets and automountServiceAccountToken since ServiceAccount has no spec field. Apply order: metadata, image pull secrets, automount. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/builder.go | 110 ++++++++++ pkg/primitives/serviceaccount/builder_test.go | 154 ++++++++++++++ pkg/primitives/serviceaccount/flavors.go | 25 +++ pkg/primitives/serviceaccount/flavors_test.go | 119 +++++++++++ pkg/primitives/serviceaccount/mutator.go | 130 ++++++++++++ pkg/primitives/serviceaccount/mutator_test.go | 189 ++++++++++++++++++ pkg/primitives/serviceaccount/resource.go | 66 ++++++ 7 files changed, 793 insertions(+) create mode 100644 pkg/primitives/serviceaccount/builder.go create mode 100644 pkg/primitives/serviceaccount/builder_test.go create mode 100644 pkg/primitives/serviceaccount/flavors.go create mode 100644 pkg/primitives/serviceaccount/flavors_test.go create mode 100644 pkg/primitives/serviceaccount/mutator.go create mode 100644 pkg/primitives/serviceaccount/mutator_test.go create mode 100644 pkg/primitives/serviceaccount/resource.go diff --git a/pkg/primitives/serviceaccount/builder.go b/pkg/primitives/serviceaccount/builder.go new file mode 100644 index 00000000..2f591f7b --- /dev/null +++ b/pkg/primitives/serviceaccount/builder.go @@ -0,0 +1,110 @@ +package serviceaccount + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/internal/generic" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + corev1 "k8s.io/api/core/v1" +) + +// Builder is a configuration helper for creating and customizing a ServiceAccount Resource. +// +// It provides a fluent API for registering mutations, field application flavors, +// and data extractors. Build() validates the configuration and returns an +// initialized Resource ready for use in a reconciliation loop. +type Builder struct { + base *generic.StaticBuilder[*corev1.ServiceAccount, *Mutator] +} + +// NewBuilder initializes a new Builder with the provided ServiceAccount object. +// +// The ServiceAccount 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 ServiceAccount must have both Name and Namespace set, which is validated +// during the Build() call. +func NewBuilder(sa *corev1.ServiceAccount) *Builder { + identityFunc := func(s *corev1.ServiceAccount) string { + return fmt.Sprintf("v1/ServiceAccount/%s/%s", s.Namespace, s.Name) + } + + return &Builder{ + base: generic.NewStaticBuilder[*corev1.ServiceAccount, *Mutator]( + sa, + identityFunc, + DefaultFieldApplicator, + NewMutator, + ), + } +} + +// WithMutation registers a mutation for the ServiceAccount. +// +// 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 ServiceAccount in the cluster. +// +// The default applicator (DefaultFieldApplicator) replaces the current object +// with a deep copy of the desired object. Use a custom applicator when other +// controllers manage fields you need to preserve. +// +// The applicator receives the current object from the API server and the desired +// object from the Resource, and is responsible for merging the desired changes +// into the current object. +func (b *Builder) WithCustomFieldApplicator( + applicator func(current, desired *corev1.ServiceAccount) 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.ServiceAccount](flavor)) + return b +} + +// WithDataExtractor registers a function to read values from the ServiceAccount after +// it has been successfully reconciled. +// +// The extractor receives a value copy of the reconciled ServiceAccount. This is useful +// for surfacing generated or updated entries to other components or resources. +// +// A nil extractor is ignored. +func (b *Builder) WithDataExtractor(extractor func(corev1.ServiceAccount) error) *Builder { + if extractor != nil { + b.base.WithDataExtractor(func(sa *corev1.ServiceAccount) error { + return extractor(*sa) + }) + } + return b +} + +// Build validates the configuration and returns the initialized Resource. +// +// It returns an error if: +// - No ServiceAccount object was provided. +// - The ServiceAccount 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/serviceaccount/builder_test.go b/pkg/primitives/serviceaccount/builder_test.go new file mode 100644 index 00000000..b34d60e6 --- /dev/null +++ b/pkg/primitives/serviceaccount/builder_test.go @@ -0,0 +1,154 @@ +package serviceaccount + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestBuilder_Build_Validation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + sa *corev1.ServiceAccount + expectedErr string + }{ + { + name: "nil serviceaccount", + sa: nil, + expectedErr: "object cannot be nil", + }, + { + name: "empty name", + sa: &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test-ns"}, + }, + expectedErr: "object name cannot be empty", + }, + { + name: "empty namespace", + sa: &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa"}, + }, + expectedErr: "object namespace cannot be empty", + }, + { + name: "valid serviceaccount", + sa: &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := NewBuilder(tt.sa).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/ServiceAccount/test-ns/test-sa", res.Identity()) + } + }) + } +} + +func TestBuilder_WithMutation(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + res, err := NewBuilder(sa). + WithMutation(Mutation{Name: "test-mutation"}). + Build() + require.NoError(t, err) + assert.Len(t, res.base.Mutations, 1) + assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) +} + +func TestBuilder_WithCustomFieldApplicator(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + called := false + applicator := func(_, _ *corev1.ServiceAccount) error { + called = true + return nil + } + res, err := NewBuilder(sa). + WithCustomFieldApplicator(applicator). + Build() + require.NoError(t, err) + require.NotNil(t, res.base.CustomFieldApplicator) + _ = res.base.CustomFieldApplicator(nil, nil) + assert.True(t, called) +} + +func TestBuilder_WithFieldApplicationFlavor(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + res, err := NewBuilder(sa). + WithFieldApplicationFlavor(PreserveCurrentLabels). + WithFieldApplicationFlavor(nil). // nil must be ignored + Build() + require.NoError(t, err) + assert.Len(t, res.base.FieldFlavors, 1) +} + +func TestBuilder_WithDataExtractor(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + called := false + extractor := func(_ corev1.ServiceAccount) error { + called = true + return nil + } + res, err := NewBuilder(sa). + WithDataExtractor(extractor). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 1) + require.NoError(t, res.base.DataExtractors[0](&corev1.ServiceAccount{})) + assert.True(t, called) +} + +func TestBuilder_WithDataExtractor_Nil(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + res, err := NewBuilder(sa). + WithDataExtractor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 0) +} + +func TestBuilder_WithDataExtractor_ErrorPropagated(t *testing.T) { + t.Parallel() + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + } + res, err := NewBuilder(sa). + WithDataExtractor(func(_ corev1.ServiceAccount) error { + return errors.New("extractor error") + }). + Build() + require.NoError(t, err) + err = res.base.DataExtractors[0](&corev1.ServiceAccount{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "extractor error") +} diff --git a/pkg/primitives/serviceaccount/flavors.go b/pkg/primitives/serviceaccount/flavors.go new file mode 100644 index 00000000..8aa0e5f4 --- /dev/null +++ b/pkg/primitives/serviceaccount/flavors.go @@ -0,0 +1,25 @@ +package serviceaccount + +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 +// ServiceAccount 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.ServiceAccount] + +// PreserveCurrentLabels ensures that any labels present on the current live +// ServiceAccount 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.ServiceAccount) error { + return flavors.PreserveCurrentLabels[*corev1.ServiceAccount]()(applied, current, desired) +} + +// PreserveCurrentAnnotations ensures that any annotations present on the current +// live ServiceAccount 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.ServiceAccount) error { + return flavors.PreserveCurrentAnnotations[*corev1.ServiceAccount]()(applied, current, desired) +} diff --git a/pkg/primitives/serviceaccount/flavors_test.go b/pkg/primitives/serviceaccount/flavors_test.go new file mode 100644 index 00000000..fb2f0aaa --- /dev/null +++ b/pkg/primitives/serviceaccount/flavors_test.go @@ -0,0 +1,119 @@ +package serviceaccount + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestPreserveCurrentLabels(t *testing.T) { + t.Run("adds missing labels from current", func(t *testing.T) { + applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} + current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "applied", applied.Labels["keep"]) + assert.Equal(t, "current", applied.Labels["extra"]) + }) + + t.Run("applied value wins on overlap", func(t *testing.T) { + applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} + current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "applied", applied.Labels["key"]) + }) + + t.Run("handles nil applied labels", func(t *testing.T) { + applied := &corev1.ServiceAccount{} + current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentLabels(applied, current, nil)) + assert.Equal(t, "current", applied.Labels["extra"]) + }) +} + +func TestPreserveCurrentAnnotations(t *testing.T) { + t.Run("adds missing annotations from current", func(t *testing.T) { + applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} + current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"extra": "current"}}} + + require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) + assert.Equal(t, "applied", applied.Annotations["keep"]) + assert.Equal(t, "current", applied.Annotations["extra"]) + }) + + t.Run("applied value wins on overlap", func(t *testing.T) { + applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} + current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "current"}}} + + require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) + assert.Equal(t, "applied", applied.Annotations["key"]) + }) +} + +func TestFlavors_Integration(t *testing.T) { + desired := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "default", + Labels: map[string]string{"app": "desired"}, + }, + } + + t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + require.NoError(t, err) + + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"external": "keep", "app": "old"}, + }, + } + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, "desired", current.Labels["app"]) + assert.Equal(t, "keep", current.Labels["external"]) + }) + + t.Run("flavors run in registration order", func(t *testing.T) { + var order []string + flavor1 := func(_, _, _ *corev1.ServiceAccount) error { + order = append(order, "flavor1") + return nil + } + flavor2 := func(_, _, _ *corev1.ServiceAccount) error { + order = append(order, "flavor2") + return nil + } + + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(flavor1). + WithFieldApplicationFlavor(flavor2). + Build() + require.NoError(t, err) + + require.NoError(t, res.Mutate(&corev1.ServiceAccount{})) + assert.Equal(t, []string{"flavor1", "flavor2"}, order) + }) + + t.Run("flavor error is returned", func(t *testing.T) { + flavorErr := errors.New("flavor boom") + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(func(_, _, _ *corev1.ServiceAccount) error { + return flavorErr + }). + Build() + require.NoError(t, err) + + err = res.Mutate(&corev1.ServiceAccount{}) + require.Error(t, err) + assert.True(t, errors.Is(err, flavorErr)) + }) +} diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go new file mode 100644 index 00000000..262390b1 --- /dev/null +++ b/pkg/primitives/serviceaccount/mutator.go @@ -0,0 +1,130 @@ +// Package serviceaccount provides a builder and resource for managing Kubernetes ServiceAccounts. +package serviceaccount + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + corev1 "k8s.io/api/core/v1" +) + +// Mutation defines a mutation that is applied to a serviceaccount Mutator +// only if its associated feature gate is enabled. +type Mutation feature.Mutation[*Mutator] + +type featurePlan struct { + metadataEdits []func(*editors.ObjectMetaEditor) error + imagePullSecretEdits []func(*corev1.ServiceAccount) + automountEdits []func(*corev1.ServiceAccount) +} + +// Mutator is a high-level helper for modifying a Kubernetes ServiceAccount. +// +// It uses a "plan-and-apply" pattern: mutations are recorded first, then +// applied to the ServiceAccount 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 { + sa *corev1.ServiceAccount + + plans []featurePlan + active *featurePlan +} + +// NewMutator creates a new Mutator for the given ServiceAccount. +func NewMutator(sa *corev1.ServiceAccount) *Mutator { + m := &Mutator{sa: sa} + 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 ServiceAccount's own metadata. +// +// Metadata edits are applied before image pull secret and automount 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) +} + +// EnsureImagePullSecret records that the named image pull secret should be present +// in .imagePullSecrets. If a secret with the same name already exists, it is a no-op. +func (m *Mutator) EnsureImagePullSecret(name string) { + m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) { + for _, ref := range sa.ImagePullSecrets { + if ref.Name == name { + return + } + } + sa.ImagePullSecrets = append(sa.ImagePullSecrets, corev1.LocalObjectReference{Name: name}) + }) +} + +// RemoveImagePullSecret records that the named image pull secret should be removed +// from .imagePullSecrets. It is a no-op if the secret is not present. +func (m *Mutator) RemoveImagePullSecret(name string) { + m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) { + filtered := sa.ImagePullSecrets[:0] + for _, ref := range sa.ImagePullSecrets { + if ref.Name != name { + filtered = append(filtered, ref) + } + } + sa.ImagePullSecrets = filtered + }) +} + +// SetAutomountServiceAccountToken records that .automountServiceAccountToken should +// be set to the provided value. Pass nil to unset the field. +func (m *Mutator) SetAutomountServiceAccountToken(v *bool) { + m.active.automountEdits = append(m.active.automountEdits, func(sa *corev1.ServiceAccount) { + sa.AutomountServiceAccountToken = v + }) +} + +// Apply executes all recorded mutation intents on the underlying ServiceAccount. +// +// Execution order across all registered features: +// +// 1. Metadata edits (in registration order within each feature) +// 2. Image pull secret edits — EnsureImagePullSecret, RemoveImagePullSecret (in registration order within each feature) +// 3. Automount edits — SetAutomountServiceAccountToken (in registration order within each feature) +// +// Features are applied in the order they were registered. Later features observe +// the ServiceAccount 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.sa.ObjectMeta) + for _, edit := range plan.metadataEdits { + if err := edit(editor); err != nil { + return err + } + } + } + + // 2. Image pull secret edits + for _, edit := range plan.imagePullSecretEdits { + edit(m.sa) + } + + // 3. Automount edits + for _, edit := range plan.automountEdits { + edit(m.sa) + } + } + + return nil +} diff --git a/pkg/primitives/serviceaccount/mutator_test.go b/pkg/primitives/serviceaccount/mutator_test.go new file mode 100644 index 00000000..cdfe1bae --- /dev/null +++ b/pkg/primitives/serviceaccount/mutator_test.go @@ -0,0 +1,189 @@ +package serviceaccount + +import ( + "testing" + + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func newTestSA() *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "default", + }, + } +} + +// --- EditObjectMetadata --- + +func TestMutator_EditObjectMetadata(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", sa.Labels["app"]) +} + +func TestMutator_EditObjectMetadata_Nil(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.EditObjectMetadata(nil) + assert.NoError(t, m.Apply()) +} + +// --- EnsureImagePullSecret --- + +func TestMutator_EnsureImagePullSecret(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.EnsureImagePullSecret("my-registry") + require.NoError(t, m.Apply()) + require.Len(t, sa.ImagePullSecrets, 1) + assert.Equal(t, "my-registry", sa.ImagePullSecrets[0].Name) +} + +func TestMutator_EnsureImagePullSecret_Idempotent(t *testing.T) { + sa := newTestSA() + sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "my-registry"}} + m := NewMutator(sa) + m.EnsureImagePullSecret("my-registry") + require.NoError(t, m.Apply()) + assert.Len(t, sa.ImagePullSecrets, 1) +} + +func TestMutator_EnsureImagePullSecret_Multiple(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.EnsureImagePullSecret("registry-a") + m.EnsureImagePullSecret("registry-b") + require.NoError(t, m.Apply()) + require.Len(t, sa.ImagePullSecrets, 2) + assert.Equal(t, "registry-a", sa.ImagePullSecrets[0].Name) + assert.Equal(t, "registry-b", sa.ImagePullSecrets[1].Name) +} + +// --- RemoveImagePullSecret --- + +func TestMutator_RemoveImagePullSecret(t *testing.T) { + sa := newTestSA() + sa.ImagePullSecrets = []corev1.LocalObjectReference{ + {Name: "keep"}, + {Name: "remove"}, + } + m := NewMutator(sa) + m.RemoveImagePullSecret("remove") + require.NoError(t, m.Apply()) + require.Len(t, sa.ImagePullSecrets, 1) + assert.Equal(t, "keep", sa.ImagePullSecrets[0].Name) +} + +func TestMutator_RemoveImagePullSecret_NotPresent(t *testing.T) { + sa := newTestSA() + sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "keep"}} + m := NewMutator(sa) + m.RemoveImagePullSecret("missing") + require.NoError(t, m.Apply()) + assert.Len(t, sa.ImagePullSecrets, 1) +} + +func TestMutator_RemoveImagePullSecret_Empty(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.RemoveImagePullSecret("missing") + require.NoError(t, m.Apply()) + assert.Empty(t, sa.ImagePullSecrets) +} + +// --- SetAutomountServiceAccountToken --- + +func TestMutator_SetAutomountServiceAccountToken(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + v := true + m.SetAutomountServiceAccountToken(&v) + require.NoError(t, m.Apply()) + require.NotNil(t, sa.AutomountServiceAccountToken) + assert.True(t, *sa.AutomountServiceAccountToken) +} + +func TestMutator_SetAutomountServiceAccountToken_False(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + v := false + m.SetAutomountServiceAccountToken(&v) + require.NoError(t, m.Apply()) + require.NotNil(t, sa.AutomountServiceAccountToken) + assert.False(t, *sa.AutomountServiceAccountToken) +} + +func TestMutator_SetAutomountServiceAccountToken_Nil(t *testing.T) { + v := true + sa := newTestSA() + sa.AutomountServiceAccountToken = &v + m := NewMutator(sa) + m.SetAutomountServiceAccountToken(nil) + require.NoError(t, m.Apply()) + assert.Nil(t, sa.AutomountServiceAccountToken) +} + +// --- Execution order --- + +func TestMutator_OperationOrder(t *testing.T) { + // Within a feature: metadata → image pull secrets → automount. + sa := newTestSA() + m := NewMutator(sa) + // Register in reverse logical order to confirm Apply() enforces category ordering. + v := false + m.SetAutomountServiceAccountToken(&v) + m.EnsureImagePullSecret("my-registry") + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "tested") + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "tested", sa.Labels["order"]) + require.Len(t, sa.ImagePullSecrets, 1) + assert.Equal(t, "my-registry", sa.ImagePullSecrets[0].Name) + require.NotNil(t, sa.AutomountServiceAccountToken) + assert.False(t, *sa.AutomountServiceAccountToken) +} + +func TestMutator_MultipleFeatures(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.EnsureImagePullSecret("feature1-registry") + m.beginFeature() + m.EnsureImagePullSecret("feature2-registry") + require.NoError(t, m.Apply()) + + require.Len(t, sa.ImagePullSecrets, 2) + assert.Equal(t, "feature1-registry", sa.ImagePullSecrets[0].Name) + assert.Equal(t, "feature2-registry", sa.ImagePullSecrets[1].Name) +} + +func TestMutator_MultipleFeatures_LaterObservesPrior(t *testing.T) { + // Feature 2 removes a secret added by feature 1. + sa := newTestSA() + m := NewMutator(sa) + m.EnsureImagePullSecret("temp-registry") + m.beginFeature() + m.RemoveImagePullSecret("temp-registry") + require.NoError(t, m.Apply()) + + assert.Empty(t, sa.ImagePullSecrets) +} + +// --- ObjectMutator interface --- + +func TestMutator_ImplementsObjectMutator(_ *testing.T) { + var _ editors.ObjectMutator = (*Mutator)(nil) +} diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go new file mode 100644 index 00000000..6dbbac51 --- /dev/null +++ b/pkg/primitives/serviceaccount/resource.go @@ -0,0 +1,66 @@ +package serviceaccount + +import ( + "github.com/sourcehawk/operator-component-framework/internal/generic" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// DefaultFieldApplicator replaces current with a deep copy of desired. +// +// This is the default baseline field application strategy for ServiceAccount resources. +// Use a custom field applicator via Builder.WithCustomFieldApplicator if you need +// to preserve fields that other controllers manage. +func DefaultFieldApplicator(current, desired *corev1.ServiceAccount) error { + *current = *desired.DeepCopy() + return nil +} + +// Resource is a high-level abstraction for managing a Kubernetes ServiceAccount within +// a controller's reconciliation loop. +// +// It implements the following component interfaces: +// - component.Resource: for basic identity and mutation behaviour. +// - component.DataExtractable: for exporting values after successful reconciliation. +// +// ServiceAccount resources are static: they do not model convergence health, grace periods, +// or suspension. Use a workload or task primitive for resources that require those concepts. +type Resource struct { + base *generic.StaticResource[*corev1.ServiceAccount, *Mutator] +} + +// Identity returns a unique identifier for the ServiceAccount in the format +// "v1/ServiceAccount//". +func (r *Resource) Identity() string { + return r.base.Identity() +} + +// Object returns a deep copy of the underlying Kubernetes ServiceAccount 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 ServiceAccount into the desired state. +// +// The mutation process follows this order: +// 1. Field application: the current object is updated to reflect the desired base state, +// using either DefaultFieldApplicator or a custom applicator if one is configured. +// 2. Field application flavors: any registered flavors are applied in registration order. +// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// +// This method is invoked by the framework during the Update phase of reconciliation. +func (r *Resource) Mutate(current client.Object) error { + return r.base.Mutate(current) +} + +// ExtractData executes all registered data extractor functions against a deep copy +// of the reconciled ServiceAccount. +// +// This is called by the framework after successful reconciliation, allowing the +// component to read generated or updated values from the ServiceAccount. +func (r *Resource) ExtractData() error { + return r.base.ExtractData() +} From 36fa7904e040ee9f5bac01053249c9ffc10ddeae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:41:44 +0000 Subject: [PATCH 02/21] Add serviceaccount primitive documentation Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/serviceaccount.md | 227 ++++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) create mode 100644 docs/primitives/serviceaccount.md diff --git a/docs/primitives/serviceaccount.md b/docs/primitives/serviceaccount.md new file mode 100644 index 00000000..4fc8269c --- /dev/null +++ b/docs/primitives/serviceaccount.md @@ -0,0 +1,227 @@ +# ServiceAccount Primitive + +The `serviceaccount` primitive is the framework's built-in static abstraction for managing Kubernetes `ServiceAccount` resources. It integrates with the component lifecycle and provides a structured mutation API for managing image pull secrets, the automount token flag, and object metadata. + +## Capabilities + +| Capability | Detail | +|-----------------------|----------------------------------------------------------------------------------------------------------| +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| **Mutation pipeline** | Direct mutator methods for `.imagePullSecrets` and `.automountServiceAccountToken`, plus metadata editors | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled ServiceAccount after each sync cycle | + +## Building a ServiceAccount Primitive + +```go +import "github.com/sourcehawk/operator-component-framework/pkg/primitives/serviceaccount" + +base := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-sa", + Namespace: owner.Namespace, + }, +} + +resource, err := serviceaccount.NewBuilder(base). + WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels). + WithMutation(MyFeatureMutation(owner.Spec.Version)). + Build() +``` + +## Default Field Application + +`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object. This ensures every reconciliation cycle produces a clean, predictable state and avoids any drift from the desired baseline. + +Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: + +```go +resource, err := serviceaccount.NewBuilder(base). + WithCustomFieldApplicator(func(current, desired *corev1.ServiceAccount) error { + // Only synchronise owned fields; leave other fields untouched. + current.ImagePullSecrets = desired.ImagePullSecrets + return nil + }). + Build() +``` + +## Mutations + +Mutations are the primary mechanism for modifying a `ServiceAccount` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through direct methods. + +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: + +```go +func MyFeatureMutation(version string) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "my-feature", + Feature: feature.NewResourceFeature(version, nil), // always enabled + Mutate: func(m *serviceaccount.Mutator) error { + m.EnsureImagePullSecret("my-registry") + 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 PrivateRegistryMutation(version string, usePrivateRegistry bool) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "private-registry", + Feature: feature.NewResourceFeature(version, nil).When(usePrivateRegistry), + Mutate: func(m *serviceaccount.Mutator) error { + m.EnsureImagePullSecret("private-registry-creds") + return nil + }, + } +} +``` + +### Version-gated mutations + +```go +var legacyConstraint = mustSemverConstraint("< 2.0.0") + +func LegacyTokenMutation(version string) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "legacy-token", + Feature: feature.NewResourceFeature( + version, + []feature.VersionConstraint{legacyConstraint}, + ), + Mutate: func(m *serviceaccount.Mutator) error { + v := true + m.SetAutomountServiceAccountToken(&v) + 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 `ServiceAccount` | +| 2 | Image pull secret edits | `.imagePullSecrets` — EnsureImagePullSecret, RemoveImagePullSecret | +| 3 | Automount edits | `.automountServiceAccountToken` — SetAutomountServiceAccountToken | + +Within each category, edits are applied in their registration order. Later features observe the ServiceAccount as modified by all previous features. + +## Mutator Methods + +### EnsureImagePullSecret + +Adds a named image pull secret to `.imagePullSecrets` if not already present. Idempotent — calling it with an already-present name is a no-op. + +```go +m.EnsureImagePullSecret("my-registry-creds") +``` + +### RemoveImagePullSecret + +Removes a named image pull secret from `.imagePullSecrets`. It is a no-op if the secret is not present. + +```go +m.RemoveImagePullSecret("old-registry-creds") +``` + +### SetAutomountServiceAccountToken + +Sets `.automountServiceAccountToken` to the provided value. Pass `nil` to unset the field. + +```go +v := false +m.SetAutomountServiceAccountToken(&v) +``` + +### EditObjectMetadata + +Modifies labels and annotations via `editors.ObjectMetaEditor`. + +Available methods: `EnsureLabel`, `RemoveLabel`, `EnsureAnnotation`, `RemoveAnnotation`, `Raw`. + +```go +m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + e.EnsureAnnotation("managed-by", "my-operator") + return nil +}) +``` + +## Flavors + +Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external controllers or other tools. + +### PreserveCurrentLabels + +Preserves labels present on the live object but absent from the applied desired state. Applied labels win on overlap. + +```go +resource, err := serviceaccount.NewBuilder(base). + WithFieldApplicationFlavor(serviceaccount.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 := serviceaccount.NewBuilder(base). + WithFieldApplicationFlavor(serviceaccount.PreserveCurrentAnnotations). + Build() +``` + +Multiple flavors can be registered and run in registration order. + +## Full Example: Feature-Composed ServiceAccount + +```go +func BaseImagePullSecretMutation(version string) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "base-pull-secret", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *serviceaccount.Mutator) error { + m.EnsureImagePullSecret("default-registry") + return nil + }, + } +} + +func DisableAutomountMutation(version string, disableAutomount bool) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "disable-automount", + Feature: feature.NewResourceFeature(version, nil).When(disableAutomount), + Mutate: func(m *serviceaccount.Mutator) error { + v := false + m.SetAutomountServiceAccountToken(&v) + return nil + }, + } +} + +resource, err := serviceaccount.NewBuilder(base). + WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels). + WithMutation(BaseImagePullSecretMutation(owner.Spec.Version)). + WithMutation(DisableAutomountMutation(owner.Spec.Version, owner.Spec.DisableAutomount)). + Build() +``` + +When `DisableAutomount` is true, `.automountServiceAccountToken` is set to `false`. When the condition is not met, the field is left at its baseline value. Neither mutation needs to know about the other. + +## Guidance + +**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use `feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for boolean conditions. + +**Use `EnsureImagePullSecret` for idempotent secret registration.** Multiple features can independently ensure their required pull secrets without conflicting with each other. + +**Register mutations in dependency order.** If mutation B relies on a secret added by mutation A, register A first. From dececa1e4aded558a5d083d7898c41c3d45b2558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:43:16 +0000 Subject: [PATCH 03/21] Add serviceaccount primitive example Demonstrates feature-gated image pull secret management, automount token control, version labelling, label preservation flavors, and data extraction across multiple reconciliation cycles. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/serviceaccount-primitive/README.md | 30 +++++ .../app/controller.go | 54 ++++++++ .../features/mutations.go | 64 ++++++++++ examples/serviceaccount-primitive/main.go | 115 ++++++++++++++++++ .../resources/serviceaccount.go | 61 ++++++++++ 5 files changed, 324 insertions(+) create mode 100644 examples/serviceaccount-primitive/README.md create mode 100644 examples/serviceaccount-primitive/app/controller.go create mode 100644 examples/serviceaccount-primitive/features/mutations.go create mode 100644 examples/serviceaccount-primitive/main.go create mode 100644 examples/serviceaccount-primitive/resources/serviceaccount.go diff --git a/examples/serviceaccount-primitive/README.md b/examples/serviceaccount-primitive/README.md new file mode 100644 index 00000000..b251aee2 --- /dev/null +++ b/examples/serviceaccount-primitive/README.md @@ -0,0 +1,30 @@ +# ServiceAccount Primitive Example + +This example demonstrates the usage of the `serviceaccount` primitive within the operator component framework. +It shows how to manage a Kubernetes ServiceAccount as a component of a larger application, utilising features like: + +- **Base Construction**: Initializing a ServiceAccount with basic metadata. +- **Feature Mutations**: Composing image pull secrets and automount settings from independent, feature-gated mutations. +- **Metadata Mutations**: Setting version labels on the ServiceAccount via `EditObjectMetadata`. +- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. +- **Data Extraction**: Harvesting ServiceAccount fields 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, image pull secrets, private registry, and automount control. +- `resources/`: Contains the central `NewServiceAccountResource` factory that assembles all features using `serviceaccount.Builder`. +- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. + +## Running the Example + +```bash +go run examples/serviceaccount-primitive/main.go +``` + +This will: +1. Initialize a fake Kubernetes client. +2. Create an `ExampleApp` owner object. +3. Reconcile through four spec variations, printing the ServiceAccount's image pull secrets and automount settings after each cycle. +4. Print the resulting status conditions. diff --git a/examples/serviceaccount-primitive/app/controller.go b/examples/serviceaccount-primitive/app/controller.go new file mode 100644 index 00000000..ed3980d4 --- /dev/null +++ b/examples/serviceaccount-primitive/app/controller.go @@ -0,0 +1,54 @@ +// Package app provides a sample controller using the serviceaccount primitive. +package app + +import ( + "context" + + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/component" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// ExampleController reconciles an ExampleApp object using the component framework. +type ExampleController struct { + client.Client + Scheme *runtime.Scheme + Recorder record.EventRecorder + Metrics component.Recorder + + // NewServiceAccountResource is a factory function to create the serviceaccount resource. + // This allows us to inject the resource construction logic. + NewServiceAccountResource func(*sharedapp.ExampleApp) (component.Resource, error) +} + +// Reconcile performs the reconciliation for a single ExampleApp. +func (r *ExampleController) Reconcile(ctx context.Context, owner *sharedapp.ExampleApp) error { + // 1. Build the serviceaccount resource for this owner. + saResource, err := r.NewServiceAccountResource(owner) + if err != nil { + return err + } + + // 2. Build the component that manages the serviceaccount. + comp, err := component.NewComponentBuilder(). + WithName("example-app"). + WithConditionType("AppReady"). + WithResource(saResource, component.ResourceOptions{}). + Build() + if err != nil { + return err + } + + // 3. Execute the component reconciliation. + resCtx := component.ReconcileContext{ + Client: r.Client, + Scheme: r.Scheme, + Recorder: r.Recorder, + Metrics: r.Metrics, + Owner: owner, + } + + return comp.Reconcile(ctx, resCtx) +} diff --git a/examples/serviceaccount-primitive/features/mutations.go b/examples/serviceaccount-primitive/features/mutations.go new file mode 100644 index 00000000..ea6da393 --- /dev/null +++ b/examples/serviceaccount-primitive/features/mutations.go @@ -0,0 +1,64 @@ +// Package features provides sample mutations for the serviceaccount 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/serviceaccount" +) + +// VersionLabelMutation sets the app.kubernetes.io/version label on the ServiceAccount. +// It is always enabled. +func VersionLabelMutation(version string) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "version-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *serviceaccount.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +// ImagePullSecretMutation ensures the default registry pull secret is attached +// to the ServiceAccount. It is always enabled. +func ImagePullSecretMutation(version string) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "image-pull-secret", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *serviceaccount.Mutator) error { + m.EnsureImagePullSecret("default-registry-creds") + return nil + }, + } +} + +// PrivateRegistryMutation adds a private registry pull secret to the ServiceAccount. +// It is enabled when usePrivateRegistry is true. +func PrivateRegistryMutation(version string, usePrivateRegistry bool) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "private-registry", + Feature: feature.NewResourceFeature(version, nil).When(usePrivateRegistry), + Mutate: func(m *serviceaccount.Mutator) error { + m.EnsureImagePullSecret("private-registry-creds") + return nil + }, + } +} + +// DisableAutomountMutation disables automatic mounting of the service account token. +// It is enabled when disableAutomount is true. +func DisableAutomountMutation(version string, disableAutomount bool) serviceaccount.Mutation { + return serviceaccount.Mutation{ + Name: "disable-automount", + Feature: feature.NewResourceFeature(version, nil).When(disableAutomount), + Mutate: func(m *serviceaccount.Mutator) error { + v := false + m.SetAutomountServiceAccountToken(&v) + return nil + }, + } +} diff --git a/examples/serviceaccount-primitive/main.go b/examples/serviceaccount-primitive/main.go new file mode 100644 index 00000000..9f4ec9b4 --- /dev/null +++ b/examples/serviceaccount-primitive/main.go @@ -0,0 +1,115 @@ +// Package main is the entry point for the serviceaccount 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/serviceaccount-primitive/app" + "github.com/sourcehawk/operator-component-framework/examples/serviceaccount-primitive/resources" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + 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 := sharedapp.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(&sharedapp.ExampleApp{}). + Build() + + // 2. Create an example Owner object. + owner := &sharedapp.ExampleApp{ + Spec: sharedapp.ExampleAppSpec{ + Version: "1.2.3", + EnableTracing: true, + EnableMetrics: true, + }, + } + 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, + }, + NewServiceAccountResource: resources.NewServiceAccountResource, + } + + // 4. Run reconciliation with multiple spec versions to demonstrate how + // feature-gated mutations compose image pull secrets and automount settings. + specs := []sharedapp.ExampleAppSpec{ + { + Version: "1.2.3", + EnableTracing: true, // adds private registry secret + EnableMetrics: true, // disables automount + }, + { + Version: "1.2.4", // Version upgrade + EnableTracing: true, + EnableMetrics: true, + }, + { + Version: "1.2.4", + EnableTracing: false, // Remove private registry + EnableMetrics: true, + }, + { + Version: "1.2.4", + EnableTracing: false, + EnableMetrics: false, // Re-enable automount (default) + }, + } + + ctx := context.Background() + + for i, spec := range specs { + fmt.Printf("\n--- Step %d: Version=%s, PrivateRegistry=%v, DisableAutomount=%v ---\n", + i+1, spec.Version, spec.EnableTracing, spec.EnableMetrics) + + owner.Spec = spec + if err := fakeClient.Update(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to update owner: %v\n", err) + os.Exit(1) + } + + fmt.Println("Running reconciliation...") + if err := controller.Reconcile(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "reconciliation failed: %v\n", err) + os.Exit(1) + } + + for _, cond := range owner.Status.Conditions { + fmt.Printf("Condition: %s, Status: %s, Reason: %s\n", + cond.Type, cond.Status, cond.Reason) + } + } + + fmt.Println("\nReconciliation sequence completed successfully!") +} diff --git a/examples/serviceaccount-primitive/resources/serviceaccount.go b/examples/serviceaccount-primitive/resources/serviceaccount.go new file mode 100644 index 00000000..bc07dc71 --- /dev/null +++ b/examples/serviceaccount-primitive/resources/serviceaccount.go @@ -0,0 +1,61 @@ +// Package resources provides resource implementations for the serviceaccount primitive example. +package resources + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/examples/serviceaccount-primitive/features" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + "github.com/sourcehawk/operator-component-framework/pkg/component" + "github.com/sourcehawk/operator-component-framework/pkg/primitives/serviceaccount" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NewServiceAccountResource constructs a serviceaccount primitive resource with all the features. +func NewServiceAccountResource(owner *sharedapp.ExampleApp) (component.Resource, error) { + // 1. Create the base ServiceAccount object. + base := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-sa", + Namespace: owner.Namespace, + Labels: map[string]string{ + "app": owner.Name, + }, + }, + } + + // 2. Initialize the serviceaccount builder. + builder := serviceaccount.NewBuilder(base) + + // 3. Register mutations in dependency order. + builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) + builder.WithMutation(features.ImagePullSecretMutation(owner.Spec.Version)) + builder.WithMutation(features.PrivateRegistryMutation(owner.Spec.Version, owner.Spec.EnableTracing)) + builder.WithMutation(features.DisableAutomountMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + + // 4. Preserve labels added by external controllers. + builder.WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels) + + // 5. Extract data from the reconciled ServiceAccount. + builder.WithDataExtractor(func(sa corev1.ServiceAccount) error { + fmt.Printf("Reconciled ServiceAccount: %s\n", sa.Name) + fmt.Printf(" ImagePullSecrets: ") + for i, ref := range sa.ImagePullSecrets { + if i > 0 { + fmt.Print(", ") + } + fmt.Print(ref.Name) + } + fmt.Println() + if sa.AutomountServiceAccountToken != nil { + fmt.Printf(" AutomountServiceAccountToken: %v\n", *sa.AutomountServiceAccountToken) + } else { + fmt.Println(" AutomountServiceAccountToken: ") + } + return nil + }) + + // 6. Build the final resource. + return builder.Build() +} From 64d398fc4eeaa7196fb4f728148654c2812b320d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:51:29 +0000 Subject: [PATCH 04/21] Fix gofmt formatting in serviceaccount example Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/serviceaccount-primitive/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/serviceaccount-primitive/main.go b/examples/serviceaccount-primitive/main.go index 9f4ec9b4..9ced2472 100644 --- a/examples/serviceaccount-primitive/main.go +++ b/examples/serviceaccount-primitive/main.go @@ -67,8 +67,8 @@ func main() { specs := []sharedapp.ExampleAppSpec{ { Version: "1.2.3", - EnableTracing: true, // adds private registry secret - EnableMetrics: true, // disables automount + EnableTracing: true, // adds private registry secret + EnableMetrics: true, // disables automount }, { Version: "1.2.4", // Version upgrade From 8c2c6db62623110fdd0a33302e75c4a6af295325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 04:30:44 +0000 Subject: [PATCH 05/21] Address Copilot review comments on serviceaccount example - Add clarifying comment in resources/serviceaccount.go explaining that EnableMetrics is reused to drive DisableAutomountMutation for demo purposes, and extract to a named variable for readability. - Fix misleading printf labels in main.go: PrivateRegistry/DisableAutomount renamed to EnableTracing/EnableMetrics to match actual spec field names. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/serviceaccount-primitive/main.go | 2 +- .../serviceaccount-primitive/resources/serviceaccount.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/examples/serviceaccount-primitive/main.go b/examples/serviceaccount-primitive/main.go index 9ced2472..98f28d9a 100644 --- a/examples/serviceaccount-primitive/main.go +++ b/examples/serviceaccount-primitive/main.go @@ -90,7 +90,7 @@ func main() { ctx := context.Background() for i, spec := range specs { - fmt.Printf("\n--- Step %d: Version=%s, PrivateRegistry=%v, DisableAutomount=%v ---\n", + fmt.Printf("\n--- Step %d: Version=%s, EnableTracing=%v, EnableMetrics=%v ---\n", i+1, spec.Version, spec.EnableTracing, spec.EnableMetrics) owner.Spec = spec diff --git a/examples/serviceaccount-primitive/resources/serviceaccount.go b/examples/serviceaccount-primitive/resources/serviceaccount.go index bc07dc71..7344eada 100644 --- a/examples/serviceaccount-primitive/resources/serviceaccount.go +++ b/examples/serviceaccount-primitive/resources/serviceaccount.go @@ -32,7 +32,11 @@ func NewServiceAccountResource(owner *sharedapp.ExampleApp) (component.Resource, builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) builder.WithMutation(features.ImagePullSecretMutation(owner.Spec.Version)) builder.WithMutation(features.PrivateRegistryMutation(owner.Spec.Version, owner.Spec.EnableTracing)) - builder.WithMutation(features.DisableAutomountMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + // In this example, we reuse EnableMetrics to drive the disable-automount feature + // purely to demonstrate the mutation. A real application would typically use a + // dedicated DisableAutomount boolean in its spec instead. + disableAutomount := owner.Spec.EnableMetrics + builder.WithMutation(features.DisableAutomountMutation(owner.Spec.Version, disableAutomount)) // 4. Preserve labels added by external controllers. builder.WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels) From 32098f43e02aca8eb9fe77a7fc60a50c665a4bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 19:51:45 +0000 Subject: [PATCH 06/21] Preserve server-managed metadata in default field applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/resource.go | 11 +++-- .../serviceaccount/resource_test.go | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 pkg/primitives/serviceaccount/resource_test.go diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go index 6dbbac51..a2383e13 100644 --- a/pkg/primitives/serviceaccount/resource.go +++ b/pkg/primitives/serviceaccount/resource.go @@ -6,13 +6,14 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator replaces current with a deep copy of desired. -// -// This is the default baseline field application strategy for ServiceAccount resources. -// Use a custom field applicator via Builder.WithCustomFieldApplicator if you need -// to preserve fields that other controllers manage. +// DefaultFieldApplicator replaces current with a deep copy of desired while +// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.) +// and shared-controller fields (OwnerReferences, Finalizers) from the original +// current object. func DefaultFieldApplicator(current, desired *corev1.ServiceAccount) error { + original := current.DeepCopy() *current = *desired.DeepCopy() + generic.PreserveServerManagedFields(current, original) return nil } diff --git a/pkg/primitives/serviceaccount/resource_test.go b/pkg/primitives/serviceaccount/resource_test.go new file mode 100644 index 00000000..c0a8fd75 --- /dev/null +++ b/pkg/primitives/serviceaccount/resource_test.go @@ -0,0 +1,49 @@ +package serviceaccount + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + ResourceVersion: "12345", + UID: "abc-def", + Generation: 3, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, + }, + Finalizers: []string{"finalizer.example.com"}, + }, + } + desired := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired labels are applied + assert.Equal(t, "test", current.Labels["app"]) + + // Server-managed fields are preserved + assert.Equal(t, "12345", current.ResourceVersion) + assert.Equal(t, "abc-def", string(current.UID)) + assert.Equal(t, int64(3), current.Generation) + + // Shared-controller fields are preserved + assert.Len(t, current.OwnerReferences, 1) + assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) + assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) +} From 95ed3b30ec43034879404c63157cb509457a12b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:43:09 +0000 Subject: [PATCH 07/21] fix duplicate calls to beginFeature in serviceaccount mutator constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align with the fix applied to deployment and configmap mutators — initialize the first feature plan inline instead of calling beginFeature(), which would create an empty extra plan when the mutator helper also calls beginFeature(). Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/mutator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index 262390b1..832c6023 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -35,8 +35,11 @@ type Mutator struct { // NewMutator creates a new Mutator for the given ServiceAccount. func NewMutator(sa *corev1.ServiceAccount) *Mutator { - m := &Mutator{sa: sa} - m.beginFeature() + m := &Mutator{ + sa: sa, + plans: []featurePlan{{}}, + } + m.active = &m.plans[0] return m } From af10313c69af7fca46f046c5db580de753940ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 17:00:15 +0000 Subject: [PATCH 08/21] export BeginFeature and use nil Feature for unconditional mutations Address PR review feedback: - Set Feature to nil for always-enabled mutations in examples instead of using feature.NewResourceFeature(version, nil), matching documented semantics and avoiding unnecessary version parsing. - Export BeginFeature() on the FeatureMutator interface and all primitive mutators (serviceaccount, configmap, deployment) so that external packages can satisfy the interface and participate in per-feature ordering semantics via the generic builder's type assertion. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/serviceaccount-primitive/features/mutations.go | 4 ++-- pkg/primitives/serviceaccount/mutator.go | 7 +++++-- pkg/primitives/serviceaccount/mutator_test.go | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/examples/serviceaccount-primitive/features/mutations.go b/examples/serviceaccount-primitive/features/mutations.go index ea6da393..77553f99 100644 --- a/examples/serviceaccount-primitive/features/mutations.go +++ b/examples/serviceaccount-primitive/features/mutations.go @@ -12,7 +12,7 @@ import ( func VersionLabelMutation(version string) serviceaccount.Mutation { return serviceaccount.Mutation{ Name: "version-label", - Feature: feature.NewResourceFeature(version, nil), + Feature: nil, Mutate: func(m *serviceaccount.Mutator) error { m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app.kubernetes.io/version", version) @@ -28,7 +28,7 @@ func VersionLabelMutation(version string) serviceaccount.Mutation { func ImagePullSecretMutation(version string) serviceaccount.Mutation { return serviceaccount.Mutation{ Name: "image-pull-secret", - Feature: feature.NewResourceFeature(version, nil), + Feature: nil, Mutate: func(m *serviceaccount.Mutator) error { m.EnsureImagePullSecret("default-registry-creds") return nil diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index 832c6023..809e2e32 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -43,9 +43,12 @@ func NewMutator(sa *corev1.ServiceAccount) *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() { +// +// This method satisfies the generic.FeatureMutator interface, allowing the +// generic builder to maintain per-feature ordering semantics for external mutators. +func (m *Mutator) BeginFeature() { m.plans = append(m.plans, featurePlan{}) m.active = &m.plans[len(m.plans)-1] } diff --git a/pkg/primitives/serviceaccount/mutator_test.go b/pkg/primitives/serviceaccount/mutator_test.go index cdfe1bae..85c64361 100644 --- a/pkg/primitives/serviceaccount/mutator_test.go +++ b/pkg/primitives/serviceaccount/mutator_test.go @@ -161,7 +161,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { sa := newTestSA() m := NewMutator(sa) m.EnsureImagePullSecret("feature1-registry") - m.beginFeature() + m.BeginFeature() m.EnsureImagePullSecret("feature2-registry") require.NoError(t, m.Apply()) @@ -175,7 +175,7 @@ func TestMutator_MultipleFeatures_LaterObservesPrior(t *testing.T) { sa := newTestSA() m := NewMutator(sa) m.EnsureImagePullSecret("temp-registry") - m.beginFeature() + m.BeginFeature() m.RemoveImagePullSecret("temp-registry") require.NoError(t, m.Apply()) From 9328bb27cd42a074b313fc77799025abdd47f6af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 21:33:25 +0000 Subject: [PATCH 09/21] fix lint error and update serviceaccount docs after rebase Rename unused version parameter to _ in ImagePullSecretMutation to fix revive lint error. Update docs to reflect server-managed metadata preservation in DefaultFieldApplicator. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/serviceaccount.md | 2 +- examples/serviceaccount-primitive/features/mutations.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/primitives/serviceaccount.md b/docs/primitives/serviceaccount.md index 4fc8269c..7ee23ece 100644 --- a/docs/primitives/serviceaccount.md +++ b/docs/primitives/serviceaccount.md @@ -31,7 +31,7 @@ resource, err := serviceaccount.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object. This ensures every reconciliation cycle produces a clean, predictable state and avoids any drift from the desired baseline. +`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.) and shared-controller fields (OwnerReferences, Finalizers) from the original live object. ServiceAccount has no Status subresource, so no status preservation is needed. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: diff --git a/examples/serviceaccount-primitive/features/mutations.go b/examples/serviceaccount-primitive/features/mutations.go index 77553f99..f8148b38 100644 --- a/examples/serviceaccount-primitive/features/mutations.go +++ b/examples/serviceaccount-primitive/features/mutations.go @@ -25,7 +25,7 @@ func VersionLabelMutation(version string) serviceaccount.Mutation { // ImagePullSecretMutation ensures the default registry pull secret is attached // to the ServiceAccount. It is always enabled. -func ImagePullSecretMutation(version string) serviceaccount.Mutation { +func ImagePullSecretMutation(_ string) serviceaccount.Mutation { return serviceaccount.Mutation{ Name: "image-pull-secret", Feature: nil, From 236bfbae8fea9d87c10354baaab4a5ee03eb6727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:29:09 +0000 Subject: [PATCH 10/21] format serviceaccount markdown files with prettier Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/serviceaccount.md | 62 +++++++++++++-------- examples/serviceaccount-primitive/README.md | 16 ++++-- 2 files changed, 49 insertions(+), 29 deletions(-) diff --git a/docs/primitives/serviceaccount.md b/docs/primitives/serviceaccount.md index 7ee23ece..6e13d1c7 100644 --- a/docs/primitives/serviceaccount.md +++ b/docs/primitives/serviceaccount.md @@ -1,15 +1,17 @@ # ServiceAccount Primitive -The `serviceaccount` primitive is the framework's built-in static abstraction for managing Kubernetes `ServiceAccount` resources. It integrates with the component lifecycle and provides a structured mutation API for managing image pull secrets, the automount token flag, and object metadata. +The `serviceaccount` primitive is the framework's built-in static abstraction for managing Kubernetes `ServiceAccount` +resources. It integrates with the component lifecycle and provides a structured mutation API for managing image pull +secrets, the automount token flag, and object metadata. ## Capabilities -| Capability | Detail | -|-----------------------|----------------------------------------------------------------------------------------------------------| -| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| Capability | Detail | +| --------------------- | --------------------------------------------------------------------------------------------------------- | +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | **Mutation pipeline** | Direct mutator methods for `.imagePullSecrets` and `.automountServiceAccountToken`, plus metadata editors | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads generated or updated values back from the reconciled ServiceAccount after each sync cycle | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled ServiceAccount after each sync cycle | ## Building a ServiceAccount Primitive @@ -31,7 +33,9 @@ resource, err := serviceaccount.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object, then restores server-managed metadata (ResourceVersion, UID, etc.) and shared-controller fields (OwnerReferences, Finalizers) from the original live object. ServiceAccount has no Status subresource, so no status preservation is needed. +`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object, then restores +server-managed metadata (ResourceVersion, UID, etc.) and shared-controller fields (OwnerReferences, Finalizers) from the +original live object. ServiceAccount has no Status subresource, so no status preservation is needed. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: @@ -47,9 +51,11 @@ resource, err := serviceaccount.NewBuilder(base). ## Mutations -Mutations are the primary mechanism for modifying a `ServiceAccount` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through direct methods. +Mutations are the primary mechanism for modifying a `ServiceAccount` beyond its baseline. Each mutation is a named +function that receives a `*Mutator` and records edit intent through direct methods. -The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature +with no version constraints and no `When()` conditions is also always enabled: ```go func MyFeatureMutation(version string) serviceaccount.Mutation { @@ -64,7 +70,8 @@ func MyFeatureMutation(version string) serviceaccount.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 @@ -106,21 +113,24 @@ 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 `ServiceAccount` | -| 2 | Image pull secret edits | `.imagePullSecrets` — EnsureImagePullSecret, RemoveImagePullSecret | -| 3 | Automount edits | `.automountServiceAccountToken` — SetAutomountServiceAccountToken | +| Step | Category | What it affects | +| ---- | ----------------------- | ------------------------------------------------------------------ | +| 1 | Metadata edits | Labels and annotations on the `ServiceAccount` | +| 2 | Image pull secret edits | `.imagePullSecrets` — EnsureImagePullSecret, RemoveImagePullSecret | +| 3 | Automount edits | `.automountServiceAccountToken` — SetAutomountServiceAccountToken | -Within each category, edits are applied in their registration order. Later features observe the ServiceAccount as modified by all previous features. +Within each category, edits are applied in their registration order. Later features observe the ServiceAccount as +modified by all previous features. ## Mutator Methods ### EnsureImagePullSecret -Adds a named image pull secret to `.imagePullSecrets` if not already present. Idempotent — calling it with an already-present name is a no-op. +Adds a named image pull secret to `.imagePullSecrets` if not already present. Idempotent — calling it with an +already-present name is a no-op. ```go m.EnsureImagePullSecret("my-registry-creds") @@ -159,7 +169,8 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { ## Flavors -Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external controllers or other tools. +Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external +controllers or other tools. ### PreserveCurrentLabels @@ -173,7 +184,8 @@ resource, err := serviceaccount.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 := serviceaccount.NewBuilder(base). @@ -216,12 +228,16 @@ resource, err := serviceaccount.NewBuilder(base). Build() ``` -When `DisableAutomount` is true, `.automountServiceAccountToken` is set to `false`. When the condition is not met, the field is left at its baseline value. Neither mutation needs to know about the other. +When `DisableAutomount` is true, `.automountServiceAccountToken` is set to `false`. When the condition is not met, the +field is left at its baseline value. Neither mutation needs to know about the other. ## Guidance -**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use `feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for boolean conditions. +**`Feature: nil` applies unconditionally.** Omit `Feature` (leave it nil) for mutations that should always run. Use +`feature.NewResourceFeature(version, constraints)` when version-based gating is needed, and chain `.When(bool)` for +boolean conditions. -**Use `EnsureImagePullSecret` for idempotent secret registration.** Multiple features can independently ensure their required pull secrets without conflicting with each other. +**Use `EnsureImagePullSecret` for idempotent secret registration.** Multiple features can independently ensure their +required pull secrets without conflicting with each other. **Register mutations in dependency order.** If mutation B relies on a secret added by mutation A, register A first. diff --git a/examples/serviceaccount-primitive/README.md b/examples/serviceaccount-primitive/README.md index b251aee2..c1a6ba18 100644 --- a/examples/serviceaccount-primitive/README.md +++ b/examples/serviceaccount-primitive/README.md @@ -1,7 +1,7 @@ # ServiceAccount Primitive Example -This example demonstrates the usage of the `serviceaccount` primitive within the operator component framework. -It shows how to manage a Kubernetes ServiceAccount as a component of a larger application, utilising features like: +This example demonstrates the usage of the `serviceaccount` primitive within the operator component framework. It shows +how to manage a Kubernetes ServiceAccount as a component of a larger application, utilising features like: - **Base Construction**: Initializing a ServiceAccount with basic metadata. - **Feature Mutations**: Composing image pull secrets and automount settings from independent, feature-gated mutations. @@ -11,10 +11,12 @@ It shows how to manage a Kubernetes ServiceAccount as a component of a larger ap ## 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, image pull secrets, private registry, and automount control. -- `resources/`: Contains the central `NewServiceAccountResource` factory that assembles all features using `serviceaccount.Builder`. + - `mutations.go`: version labelling, image pull secrets, private registry, and automount control. +- `resources/`: Contains the central `NewServiceAccountResource` factory that assembles all features using + `serviceaccount.Builder`. - `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. ## Running the Example @@ -24,7 +26,9 @@ go run examples/serviceaccount-primitive/main.go ``` This will: + 1. Initialize a fake Kubernetes client. 2. Create an `ExampleApp` owner object. -3. Reconcile through four spec variations, printing the ServiceAccount's image pull secrets and automount settings after each cycle. +3. Reconcile through four spec variations, printing the ServiceAccount's image pull secrets and automount settings after + each cycle. 4. Print the resulting status conditions. From ea07303e8bd7b682d09225437e6137ec25c2e364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:33:46 +0000 Subject: [PATCH 11/21] add serviceaccount to built-in primitives table in docs Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index dc2d4c2a..449e76cb 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -151,10 +151,11 @@ have been applied. This means a single mutation can safely add a container and t ## Built-in Primitives -| Primitive | Category | Documentation | -| --------------------------- | -------- | ----------------------------------------- | -| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | -| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| Primitive | Category | Documentation | +| ------------------------------- | -------- | ----------------------------------------------- | +| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | +| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| `pkg/primitives/serviceaccount` | Static | [serviceaccount.md](primitives/serviceaccount.md) | ## Usage Examples From 5925f2752c82ceb37e9a4fa4ce81bd72c9cd45e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:59:17 +0000 Subject: [PATCH 12/21] format docs/primitives.md with prettier to fix lint Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index 449e76cb..82a765c6 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -151,10 +151,10 @@ have been applied. This means a single mutation can safely add a container and t ## Built-in Primitives -| Primitive | Category | Documentation | -| ------------------------------- | -------- | ----------------------------------------------- | -| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | -| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| Primitive | Category | Documentation | +| ------------------------------- | -------- | ------------------------------------------------- | +| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | +| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | | `pkg/primitives/serviceaccount` | Static | [serviceaccount.md](primitives/serviceaccount.md) | ## Usage Examples From c8ec91476540745c8d92706e0aff64c6bf1d81fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:12:11 +0000 Subject: [PATCH 13/21] address copilot review: preserve SA secrets, snapshot automount bool - DefaultFieldApplicator now preserves current.Secrets to avoid overwriting controller-managed legacy token secret entries - SetAutomountServiceAccountToken now snapshots the *bool value at registration time so Apply() is deterministic Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/mutator.go | 11 ++++++++++- pkg/primitives/serviceaccount/resource.go | 8 +++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index 809e2e32..2156db7b 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -93,9 +93,18 @@ func (m *Mutator) RemoveImagePullSecret(name string) { // SetAutomountServiceAccountToken records that .automountServiceAccountToken should // be set to the provided value. Pass nil to unset the field. +// +// The pointed-to value is snapshotted at registration time so that Apply() +// is deterministic regardless of later caller-side mutations. func (m *Mutator) SetAutomountServiceAccountToken(v *bool) { + var snapshot *bool + if v != nil { + val := *v + snapshot = &val + } + m.active.automountEdits = append(m.active.automountEdits, func(sa *corev1.ServiceAccount) { - sa.AutomountServiceAccountToken = v + sa.AutomountServiceAccountToken = snapshot }) } diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go index a2383e13..61990100 100644 --- a/pkg/primitives/serviceaccount/resource.go +++ b/pkg/primitives/serviceaccount/resource.go @@ -7,13 +7,15 @@ import ( ) // DefaultFieldApplicator replaces current with a deep copy of desired while -// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.) -// and shared-controller fields (OwnerReferences, Finalizers) from the original -// current object. +// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), +// shared-controller fields (OwnerReferences, Finalizers), and controller-managed +// ServiceAccount spec fields such as .Secrets from the original current object. func DefaultFieldApplicator(current, desired *corev1.ServiceAccount) error { original := current.DeepCopy() + originalSecrets := current.Secrets *current = *desired.DeepCopy() generic.PreserveServerManagedFields(current, original) + current.Secrets = originalSecrets return nil } From 4d18ac9f3d3dc9825565fe73550ad795f4113e53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:25:16 +0000 Subject: [PATCH 14/21] address copilot review: document .secrets ownership, expand test coverage Update docs to explain that DefaultFieldApplicator preserves the live .secrets field and discards any desired .secrets value. Add comprehensive tests for Resource.Identity, Object deep-copy, Mutate (desired state, mutations, flavors, custom applicator, error), ExtractData, and the .secrets preservation contract. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/serviceaccount.md | 6 +- .../serviceaccount/resource_test.go | 257 +++++++++++++++++- 2 files changed, 260 insertions(+), 3 deletions(-) diff --git a/docs/primitives/serviceaccount.md b/docs/primitives/serviceaccount.md index 6e13d1c7..ba3f23d5 100644 --- a/docs/primitives/serviceaccount.md +++ b/docs/primitives/serviceaccount.md @@ -34,8 +34,10 @@ resource, err := serviceaccount.NewBuilder(base). ## Default Field Application `DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object, then restores -server-managed metadata (ResourceVersion, UID, etc.) and shared-controller fields (OwnerReferences, Finalizers) from the -original live object. ServiceAccount has no Status subresource, so no status preservation is needed. +server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the +live `.secrets` field from the original object. The `.secrets` field is populated by the token controller and is not +owned by the primitive — any `.secrets` value on the desired object is discarded. ServiceAccount has no Status +subresource, so no status preservation is needed. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: diff --git a/pkg/primitives/serviceaccount/resource_test.go b/pkg/primitives/serviceaccount/resource_test.go index c0a8fd75..4e9bb97c 100644 --- a/pkg/primitives/serviceaccount/resource_test.go +++ b/pkg/primitives/serviceaccount/resource_test.go @@ -1,14 +1,29 @@ package serviceaccount import ( + "errors" "testing" + "github.com/sourcehawk/operator-component-framework/pkg/feature" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) +func newValidSA() *corev1.ServiceAccount { + return &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: map[string]string{"app": "test"}, + }, + } +} + +// --- DefaultFieldApplicator tests --- + func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { current := &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ @@ -39,7 +54,7 @@ func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { // Server-managed fields are preserved assert.Equal(t, "12345", current.ResourceVersion) - assert.Equal(t, "abc-def", string(current.UID)) + assert.Equal(t, types.UID("abc-def"), current.UID) assert.Equal(t, int64(3), current.Generation) // Shared-controller fields are preserved @@ -47,3 +62,243 @@ func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { assert.Equal(t, "other-owner", current.OwnerReferences[0].Name) assert.Equal(t, []string{"finalizer.example.com"}, current.Finalizers) } + +func TestDefaultFieldApplicator_PreservesSecrets(t *testing.T) { + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []corev1.ObjectReference{ + {Name: "token-abc", Namespace: "default"}, + {Name: "token-xyz", Namespace: "default"}, + }, + } + desired := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Secrets from the live object are preserved + require.Len(t, current.Secrets, 2) + assert.Equal(t, "token-abc", current.Secrets[0].Name) + assert.Equal(t, "token-xyz", current.Secrets[1].Name) +} + +func TestDefaultFieldApplicator_DesiredSecretsAreDiscarded(t *testing.T) { + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []corev1.ObjectReference{ + {Name: "live-token", Namespace: "default"}, + }, + } + desired := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Secrets: []corev1.ObjectReference{ + {Name: "desired-token", Namespace: "default"}, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // The live secrets are preserved; desired secrets are discarded + require.Len(t, current.Secrets, 1) + assert.Equal(t, "live-token", current.Secrets[0].Name) +} + +// --- Resource.Identity tests --- + +func TestResource_Identity(t *testing.T) { + res, err := NewBuilder(newValidSA()).Build() + require.NoError(t, err) + + assert.Equal(t, "v1/ServiceAccount/test-ns/test-sa", res.Identity()) +} + +// --- Resource.Object tests --- + +func TestResource_Object_ReturnsDeepCopy(t *testing.T) { + sa := newValidSA() + res, err := NewBuilder(sa).Build() + require.NoError(t, err) + + got, err := res.Object() + require.NoError(t, err) + + casted, ok := got.(*corev1.ServiceAccount) + require.True(t, ok) + assert.Equal(t, "test-sa", casted.Name) + + // Mutating the returned copy must not affect the original + casted.Name = "changed" + assert.Equal(t, "test-sa", sa.Name) + + // A second call should also be independent + got2, err := res.Object() + require.NoError(t, err) + assert.Equal(t, "test-sa", got2.GetName()) +} + +// --- Resource.Mutate tests --- + +func TestResource_Mutate_AppliesDesiredState(t *testing.T) { + desired := newValidSA() + desired.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "registry"}} + + res, err := NewBuilder(desired).Build() + require.NoError(t, err) + + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + ResourceVersion: "999", + }, + } + require.NoError(t, res.Mutate(current)) + + assert.Equal(t, "test", current.Labels["app"]) + require.Len(t, current.ImagePullSecrets, 1) + assert.Equal(t, "registry", current.ImagePullSecrets[0].Name) + // Server-managed field preserved + assert.Equal(t, "999", current.ResourceVersion) +} + +func TestResource_Mutate_WithMutation(t *testing.T) { + desired := newValidSA() + res, err := NewBuilder(desired). + WithMutation(Mutation{ + Name: "add-pull-secret", + Feature: feature.NewResourceFeature("1.0.0", nil), + Mutate: func(m *Mutator) error { + m.EnsureImagePullSecret("my-registry") + return nil + }, + }). + Build() + require.NoError(t, err) + + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + }, + } + require.NoError(t, res.Mutate(current)) + + require.Len(t, current.ImagePullSecrets, 1) + assert.Equal(t, "my-registry", current.ImagePullSecrets[0].Name) +} + +func TestResource_Mutate_WithFlavor(t *testing.T) { + desired := newValidSA() + desired.Labels = map[string]string{"app": "test"} + + res, err := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + require.NoError(t, err) + + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: map[string]string{"external": "preserved", "app": "old"}, + }, + } + require.NoError(t, res.Mutate(current)) + + // Desired label wins on overlap + assert.Equal(t, "test", current.Labels["app"]) + // External label is preserved + assert.Equal(t, "preserved", current.Labels["external"]) +} + +func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { + desired := newValidSA() + desired.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "desired-secret"}} + + applicatorCalled := false + res, err := NewBuilder(desired). + WithCustomFieldApplicator(func(current, d *corev1.ServiceAccount) error { + applicatorCalled = true + current.ImagePullSecrets = d.ImagePullSecrets + return nil + }). + Build() + require.NoError(t, err) + + current := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "test-ns", + Labels: map[string]string{"external": "should-stay"}, + }, + } + require.NoError(t, res.Mutate(current)) + + assert.True(t, applicatorCalled) + require.Len(t, current.ImagePullSecrets, 1) + assert.Equal(t, "desired-secret", current.ImagePullSecrets[0].Name) + // Custom applicator didn't touch labels, so they stay + assert.Equal(t, "should-stay", current.Labels["external"]) +} + +func TestResource_Mutate_CustomFieldApplicator_Error(t *testing.T) { + res, err := NewBuilder(newValidSA()). + WithCustomFieldApplicator(func(_, _ *corev1.ServiceAccount) error { + return errors.New("applicator error") + }). + Build() + require.NoError(t, err) + + err = res.Mutate(&corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "applicator error") +} + +// --- Resource.ExtractData tests --- + +func TestResource_ExtractData(t *testing.T) { + sa := newValidSA() + sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "reg"}} + + var extractedName string + res, err := NewBuilder(sa). + WithDataExtractor(func(s corev1.ServiceAccount) error { + extractedName = s.ImagePullSecrets[0].Name + return nil + }). + Build() + require.NoError(t, err) + + require.NoError(t, res.ExtractData()) + assert.Equal(t, "reg", extractedName) +} + +func TestResource_ExtractData_Error(t *testing.T) { + res, err := NewBuilder(newValidSA()). + WithDataExtractor(func(_ corev1.ServiceAccount) error { + return errors.New("extract error") + }). + Build() + require.NoError(t, err) + + err = res.ExtractData() + require.Error(t, err) + assert.Contains(t, err.Error(), "extract error") +} From 228fc596f0720464b5bf8b5a30f1e4622cf469e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 17:24:07 +0000 Subject: [PATCH 15/21] fix doc comment: ServiceAccount has no Spec, .Secrets is top-level Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go index 61990100..3d995d6e 100644 --- a/pkg/primitives/serviceaccount/resource.go +++ b/pkg/primitives/serviceaccount/resource.go @@ -9,7 +9,7 @@ import ( // DefaultFieldApplicator replaces current with a deep copy of desired while // preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), // shared-controller fields (OwnerReferences, Finalizers), and controller-managed -// ServiceAccount spec fields such as .Secrets from the original current object. +// ServiceAccount fields such as .Secrets from the original current object. func DefaultFieldApplicator(current, desired *corev1.ServiceAccount) error { original := current.DeepCopy() originalSecrets := current.Secrets From 0c05584807d492c509b24faae03536e7b4fef3a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 17:55:27 +0000 Subject: [PATCH 16/21] do not initialize an empty plan on serviceaccount mutator construction Align with configmap/deployment: NewMutator no longer creates an initial feature plan. BeginFeature must be called before registering mutations. Update all tests to call BeginFeature and add constructor/plan invariant tests matching the other primitives. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/mutator.go | 8 +-- pkg/primitives/serviceaccount/mutator_test.go | 67 +++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index 2156db7b..f601148d 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -34,13 +34,11 @@ type Mutator struct { } // NewMutator creates a new Mutator for the given ServiceAccount. +// BeginFeature must be called before registering any mutations. func NewMutator(sa *corev1.ServiceAccount) *Mutator { - m := &Mutator{ - sa: sa, - plans: []featurePlan{{}}, + return &Mutator{ + sa: sa, } - m.active = &m.plans[0] - return m } // BeginFeature starts a new feature planning scope. All subsequent mutation diff --git a/pkg/primitives/serviceaccount/mutator_test.go b/pkg/primitives/serviceaccount/mutator_test.go index 85c64361..13eb50bf 100644 --- a/pkg/primitives/serviceaccount/mutator_test.go +++ b/pkg/primitives/serviceaccount/mutator_test.go @@ -24,6 +24,7 @@ func newTestSA() *corev1.ServiceAccount { func TestMutator_EditObjectMetadata(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil @@ -35,6 +36,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { func TestMutator_EditObjectMetadata_Nil(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EditObjectMetadata(nil) assert.NoError(t, m.Apply()) } @@ -44,6 +46,7 @@ func TestMutator_EditObjectMetadata_Nil(t *testing.T) { func TestMutator_EnsureImagePullSecret(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EnsureImagePullSecret("my-registry") require.NoError(t, m.Apply()) require.Len(t, sa.ImagePullSecrets, 1) @@ -54,6 +57,7 @@ func TestMutator_EnsureImagePullSecret_Idempotent(t *testing.T) { sa := newTestSA() sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "my-registry"}} m := NewMutator(sa) + m.BeginFeature() m.EnsureImagePullSecret("my-registry") require.NoError(t, m.Apply()) assert.Len(t, sa.ImagePullSecrets, 1) @@ -62,6 +66,7 @@ func TestMutator_EnsureImagePullSecret_Idempotent(t *testing.T) { func TestMutator_EnsureImagePullSecret_Multiple(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EnsureImagePullSecret("registry-a") m.EnsureImagePullSecret("registry-b") require.NoError(t, m.Apply()) @@ -79,6 +84,7 @@ func TestMutator_RemoveImagePullSecret(t *testing.T) { {Name: "remove"}, } m := NewMutator(sa) + m.BeginFeature() m.RemoveImagePullSecret("remove") require.NoError(t, m.Apply()) require.Len(t, sa.ImagePullSecrets, 1) @@ -89,6 +95,7 @@ func TestMutator_RemoveImagePullSecret_NotPresent(t *testing.T) { sa := newTestSA() sa.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "keep"}} m := NewMutator(sa) + m.BeginFeature() m.RemoveImagePullSecret("missing") require.NoError(t, m.Apply()) assert.Len(t, sa.ImagePullSecrets, 1) @@ -97,6 +104,7 @@ func TestMutator_RemoveImagePullSecret_NotPresent(t *testing.T) { func TestMutator_RemoveImagePullSecret_Empty(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.RemoveImagePullSecret("missing") require.NoError(t, m.Apply()) assert.Empty(t, sa.ImagePullSecrets) @@ -107,6 +115,7 @@ func TestMutator_RemoveImagePullSecret_Empty(t *testing.T) { func TestMutator_SetAutomountServiceAccountToken(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() v := true m.SetAutomountServiceAccountToken(&v) require.NoError(t, m.Apply()) @@ -117,6 +126,7 @@ func TestMutator_SetAutomountServiceAccountToken(t *testing.T) { func TestMutator_SetAutomountServiceAccountToken_False(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() v := false m.SetAutomountServiceAccountToken(&v) require.NoError(t, m.Apply()) @@ -129,6 +139,7 @@ func TestMutator_SetAutomountServiceAccountToken_Nil(t *testing.T) { sa := newTestSA() sa.AutomountServiceAccountToken = &v m := NewMutator(sa) + m.BeginFeature() m.SetAutomountServiceAccountToken(nil) require.NoError(t, m.Apply()) assert.Nil(t, sa.AutomountServiceAccountToken) @@ -140,6 +151,7 @@ func TestMutator_OperationOrder(t *testing.T) { // Within a feature: metadata → image pull secrets → automount. sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() // Register in reverse logical order to confirm Apply() enforces category ordering. v := false m.SetAutomountServiceAccountToken(&v) @@ -160,6 +172,7 @@ func TestMutator_OperationOrder(t *testing.T) { func TestMutator_MultipleFeatures(t *testing.T) { sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EnsureImagePullSecret("feature1-registry") m.BeginFeature() m.EnsureImagePullSecret("feature2-registry") @@ -174,6 +187,7 @@ func TestMutator_MultipleFeatures_LaterObservesPrior(t *testing.T) { // Feature 2 removes a secret added by feature 1. sa := newTestSA() m := NewMutator(sa) + m.BeginFeature() m.EnsureImagePullSecret("temp-registry") m.BeginFeature() m.RemoveImagePullSecret("temp-registry") @@ -182,6 +196,59 @@ func TestMutator_MultipleFeatures_LaterObservesPrior(t *testing.T) { assert.Empty(t, sa.ImagePullSecrets) } +// --- Constructor and feature plan invariants --- + +func TestNewMutator_InitializesNoPlan(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + + 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) { + sa := newTestSA() + m := NewMutator(sa) + + 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) { + sa := newTestSA() + m := NewMutator(sa) + + // Record a mutation in the first feature plan + m.BeginFeature() + m.EnsureImagePullSecret("f0-registry") + + // Start a new feature and record a different mutation + m.BeginFeature() + m.EnsureImagePullSecret("f1-registry") + + // The initial plan should have exactly one image pull secret edit + assert.Len(t, m.plans[0].imagePullSecretEdits, 1, "initial plan should have one edit") + // The second plan should also have exactly one image pull secret edit + assert.Len(t, m.plans[1].imagePullSecretEdits, 1, "second plan should have one edit") +} + +func TestMutator_SingleFeature_PlanCount(t *testing.T) { + sa := newTestSA() + m := NewMutator(sa) + m.BeginFeature() + m.EnsureImagePullSecret("my-registry") + + require.NoError(t, m.Apply()) + assert.Len(t, m.plans, 1, "no extra plans should be created during Apply") + require.Len(t, sa.ImagePullSecrets, 1) + assert.Equal(t, "my-registry", sa.ImagePullSecrets[0].Name) +} + // --- ObjectMutator interface --- func TestMutator_ImplementsObjectMutator(_ *testing.T) { From cd2700439466940285e198c1fff5805b094b98ea 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:07:20 +0000 Subject: [PATCH 17/21] remove field applicators and flavors from serviceaccount primitive Align with framework SSA refactor: remove DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor, flavors.go, and all related tests. Update Mutate tests to use Object() output. Strip field applicator and flavor sections from primitive docs. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/serviceaccount.md | 51 ----- pkg/primitives/serviceaccount/builder.go | 41 +--- pkg/primitives/serviceaccount/builder_test.go | 32 --- pkg/primitives/serviceaccount/flavors.go | 25 --- pkg/primitives/serviceaccount/flavors_test.go | 119 ----------- pkg/primitives/serviceaccount/resource.go | 19 +- .../serviceaccount/resource_test.go | 199 ++---------------- 7 files changed, 19 insertions(+), 467 deletions(-) delete mode 100644 pkg/primitives/serviceaccount/flavors.go delete mode 100644 pkg/primitives/serviceaccount/flavors_test.go diff --git a/docs/primitives/serviceaccount.md b/docs/primitives/serviceaccount.md index ba3f23d5..af7688a8 100644 --- a/docs/primitives/serviceaccount.md +++ b/docs/primitives/serviceaccount.md @@ -10,7 +10,6 @@ secrets, the automount token flag, and object metadata. | --------------------- | --------------------------------------------------------------------------------------------------------- | | **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | **Mutation pipeline** | Direct mutator methods for `.imagePullSecrets` and `.automountServiceAccountToken`, plus metadata editors | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | | **Data extraction** | Reads generated or updated values back from the reconciled ServiceAccount after each sync cycle | ## Building a ServiceAccount Primitive @@ -26,31 +25,10 @@ base := &corev1.ServiceAccount{ } resource, err := serviceaccount.NewBuilder(base). - WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels). WithMutation(MyFeatureMutation(owner.Spec.Version)). Build() ``` -## Default Field Application - -`DefaultFieldApplicator` replaces the current ServiceAccount with a deep copy of the desired object, then restores -server-managed metadata (ResourceVersion, UID, etc.), shared-controller fields (OwnerReferences, Finalizers), and the -live `.secrets` field from the original object. The `.secrets` field is populated by the token controller and is not -owned by the primitive — any `.secrets` value on the desired object is discarded. ServiceAccount has no Status -subresource, so no status preservation is needed. - -Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: - -```go -resource, err := serviceaccount.NewBuilder(base). - WithCustomFieldApplicator(func(current, desired *corev1.ServiceAccount) error { - // Only synchronise owned fields; leave other fields untouched. - current.ImagePullSecrets = desired.ImagePullSecrets - return nil - }). - Build() -``` - ## Mutations Mutations are the primary mechanism for modifying a `ServiceAccount` beyond its baseline. Each mutation is a named @@ -169,34 +147,6 @@ m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { }) ``` -## Flavors - -Flavors run after the baseline applicator and before mutations. They are used to preserve fields managed by external -controllers or other tools. - -### PreserveCurrentLabels - -Preserves labels present on the live object but absent from the applied desired state. Applied labels win on overlap. - -```go -resource, err := serviceaccount.NewBuilder(base). - WithFieldApplicationFlavor(serviceaccount.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 := serviceaccount.NewBuilder(base). - WithFieldApplicationFlavor(serviceaccount.PreserveCurrentAnnotations). - Build() -``` - -Multiple flavors can be registered and run in registration order. - ## Full Example: Feature-Composed ServiceAccount ```go @@ -224,7 +174,6 @@ func DisableAutomountMutation(version string, disableAutomount bool) serviceacco } resource, err := serviceaccount.NewBuilder(base). - WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels). WithMutation(BaseImagePullSecretMutation(owner.Spec.Version)). WithMutation(DisableAutomountMutation(owner.Spec.Version, owner.Spec.DisableAutomount)). Build() diff --git a/pkg/primitives/serviceaccount/builder.go b/pkg/primitives/serviceaccount/builder.go index 2f591f7b..937b40be 100644 --- a/pkg/primitives/serviceaccount/builder.go +++ b/pkg/primitives/serviceaccount/builder.go @@ -10,9 +10,9 @@ import ( // Builder is a configuration helper for creating and customizing a ServiceAccount Resource. // -// It provides a fluent API for registering mutations, field application flavors, -// and data extractors. Build() validates the configuration and returns an -// initialized Resource ready for use in a reconciliation loop. +// It provides a fluent API for registering mutations and data extractors. +// Build() validates the configuration and returns an initialized Resource +// ready for use in a reconciliation loop. type Builder struct { base *generic.StaticBuilder[*corev1.ServiceAccount, *Mutator] } @@ -21,7 +21,7 @@ type Builder struct { // // The ServiceAccount object serves as the desired base state. During reconciliation // the Resource will make the cluster's state match this base, modified by any -// registered mutations and flavors. +// registered mutations. // // The provided ServiceAccount must have both Name and Namespace set, which is validated // during the Build() call. @@ -34,7 +34,6 @@ func NewBuilder(sa *corev1.ServiceAccount) *Builder { base: generic.NewStaticBuilder[*corev1.ServiceAccount, *Mutator]( sa, identityFunc, - DefaultFieldApplicator, NewMutator, ), } @@ -42,8 +41,7 @@ func NewBuilder(sa *corev1.ServiceAccount) *Builder { // WithMutation registers a mutation for the ServiceAccount. // -// Mutations are applied sequentially during the Mutate() phase of reconciliation, -// after the baseline field applicator and any registered flavors have run. +// Mutations are applied sequentially during the Mutate() phase of reconciliation. // A mutation with a nil Feature is applied unconditionally; one with a non-nil // Feature is applied only when that feature is enabled. func (b *Builder) WithMutation(m Mutation) *Builder { @@ -51,35 +49,6 @@ func (b *Builder) WithMutation(m Mutation) *Builder { return b } -// WithCustomFieldApplicator sets a custom strategy for applying the desired -// state to the existing ServiceAccount in the cluster. -// -// The default applicator (DefaultFieldApplicator) replaces the current object -// with a deep copy of the desired object. Use a custom applicator when other -// controllers manage fields you need to preserve. -// -// The applicator receives the current object from the API server and the desired -// object from the Resource, and is responsible for merging the desired changes -// into the current object. -func (b *Builder) WithCustomFieldApplicator( - applicator func(current, desired *corev1.ServiceAccount) 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.ServiceAccount](flavor)) - return b -} - // WithDataExtractor registers a function to read values from the ServiceAccount after // it has been successfully reconciled. // diff --git a/pkg/primitives/serviceaccount/builder_test.go b/pkg/primitives/serviceaccount/builder_test.go index b34d60e6..5d295960 100644 --- a/pkg/primitives/serviceaccount/builder_test.go +++ b/pkg/primitives/serviceaccount/builder_test.go @@ -74,38 +74,6 @@ func TestBuilder_WithMutation(t *testing.T) { assert.Equal(t, "test-mutation", res.base.Mutations[0].Name) } -func TestBuilder_WithCustomFieldApplicator(t *testing.T) { - t.Parallel() - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, - } - called := false - applicator := func(_, _ *corev1.ServiceAccount) error { - called = true - return nil - } - res, err := NewBuilder(sa). - WithCustomFieldApplicator(applicator). - Build() - require.NoError(t, err) - require.NotNil(t, res.base.CustomFieldApplicator) - _ = res.base.CustomFieldApplicator(nil, nil) - assert.True(t, called) -} - -func TestBuilder_WithFieldApplicationFlavor(t *testing.T) { - t.Parallel() - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, - } - res, err := NewBuilder(sa). - WithFieldApplicationFlavor(PreserveCurrentLabels). - WithFieldApplicationFlavor(nil). // nil must be ignored - Build() - require.NoError(t, err) - assert.Len(t, res.base.FieldFlavors, 1) -} - func TestBuilder_WithDataExtractor(t *testing.T) { t.Parallel() sa := &corev1.ServiceAccount{ diff --git a/pkg/primitives/serviceaccount/flavors.go b/pkg/primitives/serviceaccount/flavors.go deleted file mode 100644 index 8aa0e5f4..00000000 --- a/pkg/primitives/serviceaccount/flavors.go +++ /dev/null @@ -1,25 +0,0 @@ -package serviceaccount - -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 -// ServiceAccount 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.ServiceAccount] - -// PreserveCurrentLabels ensures that any labels present on the current live -// ServiceAccount 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.ServiceAccount) error { - return flavors.PreserveCurrentLabels[*corev1.ServiceAccount]()(applied, current, desired) -} - -// PreserveCurrentAnnotations ensures that any annotations present on the current -// live ServiceAccount 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.ServiceAccount) error { - return flavors.PreserveCurrentAnnotations[*corev1.ServiceAccount]()(applied, current, desired) -} diff --git a/pkg/primitives/serviceaccount/flavors_test.go b/pkg/primitives/serviceaccount/flavors_test.go deleted file mode 100644 index fb2f0aaa..00000000 --- a/pkg/primitives/serviceaccount/flavors_test.go +++ /dev/null @@ -1,119 +0,0 @@ -package serviceaccount - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestPreserveCurrentLabels(t *testing.T) { - t.Run("adds missing labels from current", func(t *testing.T) { - applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} - current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "applied", applied.Labels["keep"]) - assert.Equal(t, "current", applied.Labels["extra"]) - }) - - t.Run("applied value wins on overlap", func(t *testing.T) { - applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} - current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "applied", applied.Labels["key"]) - }) - - t.Run("handles nil applied labels", func(t *testing.T) { - applied := &corev1.ServiceAccount{} - current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentLabels(applied, current, nil)) - assert.Equal(t, "current", applied.Labels["extra"]) - }) -} - -func TestPreserveCurrentAnnotations(t *testing.T) { - t.Run("adds missing annotations from current", func(t *testing.T) { - applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} - current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"extra": "current"}}} - - require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) - assert.Equal(t, "applied", applied.Annotations["keep"]) - assert.Equal(t, "current", applied.Annotations["extra"]) - }) - - t.Run("applied value wins on overlap", func(t *testing.T) { - applied := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} - current := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "current"}}} - - require.NoError(t, PreserveCurrentAnnotations(applied, current, nil)) - assert.Equal(t, "applied", applied.Annotations["key"]) - }) -} - -func TestFlavors_Integration(t *testing.T) { - desired := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-sa", - Namespace: "default", - Labels: map[string]string{"app": "desired"}, - }, - } - - t.Run("PreserveCurrentLabels via Mutate", func(t *testing.T) { - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(PreserveCurrentLabels). - Build() - require.NoError(t, err) - - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"external": "keep", "app": "old"}, - }, - } - require.NoError(t, res.Mutate(current)) - - assert.Equal(t, "desired", current.Labels["app"]) - assert.Equal(t, "keep", current.Labels["external"]) - }) - - t.Run("flavors run in registration order", func(t *testing.T) { - var order []string - flavor1 := func(_, _, _ *corev1.ServiceAccount) error { - order = append(order, "flavor1") - return nil - } - flavor2 := func(_, _, _ *corev1.ServiceAccount) error { - order = append(order, "flavor2") - return nil - } - - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(flavor1). - WithFieldApplicationFlavor(flavor2). - Build() - require.NoError(t, err) - - require.NoError(t, res.Mutate(&corev1.ServiceAccount{})) - assert.Equal(t, []string{"flavor1", "flavor2"}, order) - }) - - t.Run("flavor error is returned", func(t *testing.T) { - flavorErr := errors.New("flavor boom") - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(func(_, _, _ *corev1.ServiceAccount) error { - return flavorErr - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&corev1.ServiceAccount{}) - require.Error(t, err) - assert.True(t, errors.Is(err, flavorErr)) - }) -} diff --git a/pkg/primitives/serviceaccount/resource.go b/pkg/primitives/serviceaccount/resource.go index 3d995d6e..0245b6e3 100644 --- a/pkg/primitives/serviceaccount/resource.go +++ b/pkg/primitives/serviceaccount/resource.go @@ -6,19 +6,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// DefaultFieldApplicator replaces current with a deep copy of desired while -// preserving server-managed metadata (ResourceVersion, UID, Generation, etc.), -// shared-controller fields (OwnerReferences, Finalizers), and controller-managed -// ServiceAccount fields such as .Secrets from the original current object. -func DefaultFieldApplicator(current, desired *corev1.ServiceAccount) error { - original := current.DeepCopy() - originalSecrets := current.Secrets - *current = *desired.DeepCopy() - generic.PreserveServerManagedFields(current, original) - current.Secrets = originalSecrets - return nil -} - // Resource is a high-level abstraction for managing a Kubernetes ServiceAccount within // a controller's reconciliation loop. // @@ -49,10 +36,8 @@ func (r *Resource) Object() (client.Object, error) { // Mutate transforms the current state of a Kubernetes ServiceAccount into the desired state. // // The mutation process follows this order: -// 1. Field application: the current object is updated to reflect the desired base state, -// using either DefaultFieldApplicator or a custom applicator if one is configured. -// 2. Field application flavors: any registered flavors are applied in registration order. -// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// 1. The desired base state is applied to the current object. +// 2. Feature mutations: all registered feature-gated mutations are applied in order. // // This method is invoked by the framework during the Update phase of reconciliation. func (r *Resource) Mutate(current client.Object) error { diff --git a/pkg/primitives/serviceaccount/resource_test.go b/pkg/primitives/serviceaccount/resource_test.go index 4e9bb97c..960118a2 100644 --- a/pkg/primitives/serviceaccount/resource_test.go +++ b/pkg/primitives/serviceaccount/resource_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) func newValidSA() *corev1.ServiceAccount { @@ -22,102 +21,6 @@ func newValidSA() *corev1.ServiceAccount { } } -// --- DefaultFieldApplicator tests --- - -func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - ResourceVersion: "12345", - UID: "abc-def", - Generation: 3, - OwnerReferences: []metav1.OwnerReference{ - {APIVersion: "v1", Kind: "Pod", Name: "other-owner", UID: "other-uid"}, - }, - Finalizers: []string{"finalizer.example.com"}, - }, - } - desired := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // Desired labels are applied - assert.Equal(t, "test", current.Labels["app"]) - - // Server-managed fields are preserved - assert.Equal(t, "12345", current.ResourceVersion) - assert.Equal(t, types.UID("abc-def"), 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) -} - -func TestDefaultFieldApplicator_PreservesSecrets(t *testing.T) { - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Secrets: []corev1.ObjectReference{ - {Name: "token-abc", Namespace: "default"}, - {Name: "token-xyz", Namespace: "default"}, - }, - } - desired := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // Secrets from the live object are preserved - require.Len(t, current.Secrets, 2) - assert.Equal(t, "token-abc", current.Secrets[0].Name) - assert.Equal(t, "token-xyz", current.Secrets[1].Name) -} - -func TestDefaultFieldApplicator_DesiredSecretsAreDiscarded(t *testing.T) { - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Secrets: []corev1.ObjectReference{ - {Name: "live-token", Namespace: "default"}, - }, - } - desired := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - }, - Secrets: []corev1.ObjectReference{ - {Name: "desired-token", Namespace: "default"}, - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // The live secrets are preserved; desired secrets are discarded - require.Len(t, current.Secrets, 1) - assert.Equal(t, "live-token", current.Secrets[0].Name) -} - // --- Resource.Identity tests --- func TestResource_Identity(t *testing.T) { @@ -160,20 +63,14 @@ func TestResource_Mutate_AppliesDesiredState(t *testing.T) { res, err := NewBuilder(desired).Build() require.NoError(t, err) - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-sa", - Namespace: "test-ns", - ResourceVersion: "999", - }, - } - require.NoError(t, res.Mutate(current)) + obj, err := res.Object() + require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.Equal(t, "test", current.Labels["app"]) - require.Len(t, current.ImagePullSecrets, 1) - assert.Equal(t, "registry", current.ImagePullSecrets[0].Name) - // Server-managed field preserved - assert.Equal(t, "999", current.ResourceVersion) + got := obj.(*corev1.ServiceAccount) + assert.Equal(t, "test", got.Labels["app"]) + require.Len(t, got.ImagePullSecrets, 1) + assert.Equal(t, "registry", got.ImagePullSecrets[0].Name) } func TestResource_Mutate_WithMutation(t *testing.T) { @@ -190,85 +87,13 @@ func TestResource_Mutate_WithMutation(t *testing.T) { Build() require.NoError(t, err) - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-sa", - Namespace: "test-ns", - }, - } - require.NoError(t, res.Mutate(current)) - - require.Len(t, current.ImagePullSecrets, 1) - assert.Equal(t, "my-registry", current.ImagePullSecrets[0].Name) -} - -func TestResource_Mutate_WithFlavor(t *testing.T) { - desired := newValidSA() - 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.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-sa", - Namespace: "test-ns", - Labels: map[string]string{"external": "preserved", "app": "old"}, - }, - } - require.NoError(t, res.Mutate(current)) - - // Desired label wins on overlap - assert.Equal(t, "test", current.Labels["app"]) - // External label is preserved - assert.Equal(t, "preserved", current.Labels["external"]) -} - -func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { - desired := newValidSA() - desired.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "desired-secret"}} - - applicatorCalled := false - res, err := NewBuilder(desired). - WithCustomFieldApplicator(func(current, d *corev1.ServiceAccount) error { - applicatorCalled = true - current.ImagePullSecrets = d.ImagePullSecrets - return nil - }). - Build() - require.NoError(t, err) - - current := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-sa", - Namespace: "test-ns", - Labels: map[string]string{"external": "should-stay"}, - }, - } - require.NoError(t, res.Mutate(current)) - - assert.True(t, applicatorCalled) - require.Len(t, current.ImagePullSecrets, 1) - assert.Equal(t, "desired-secret", current.ImagePullSecrets[0].Name) - // Custom applicator didn't touch labels, so they stay - assert.Equal(t, "should-stay", current.Labels["external"]) -} - -func TestResource_Mutate_CustomFieldApplicator_Error(t *testing.T) { - res, err := NewBuilder(newValidSA()). - WithCustomFieldApplicator(func(_, _ *corev1.ServiceAccount) error { - return errors.New("applicator error") - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{Name: "test-sa", Namespace: "test-ns"}, - }) - require.Error(t, err) - assert.Contains(t, err.Error(), "applicator error") + got := obj.(*corev1.ServiceAccount) + require.Len(t, got.ImagePullSecrets, 1) + assert.Equal(t, "my-registry", got.ImagePullSecrets[0].Name) } // --- Resource.ExtractData tests --- From 2ee1b8edbe5abaf709673f11d514f031dea98233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:52:52 +0000 Subject: [PATCH 18/21] remove references to deleted field applicators in serviceaccount example The WithFieldApplicationFlavor and PreserveCurrentLabels were removed from the serviceaccount primitive but the example still referenced them. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../serviceaccount-primitive/resources/serviceaccount.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/serviceaccount-primitive/resources/serviceaccount.go b/examples/serviceaccount-primitive/resources/serviceaccount.go index 7344eada..3cfa6a5f 100644 --- a/examples/serviceaccount-primitive/resources/serviceaccount.go +++ b/examples/serviceaccount-primitive/resources/serviceaccount.go @@ -38,10 +38,7 @@ func NewServiceAccountResource(owner *sharedapp.ExampleApp) (component.Resource, disableAutomount := owner.Spec.EnableMetrics builder.WithMutation(features.DisableAutomountMutation(owner.Spec.Version, disableAutomount)) - // 4. Preserve labels added by external controllers. - builder.WithFieldApplicationFlavor(serviceaccount.PreserveCurrentLabels) - - // 5. Extract data from the reconciled ServiceAccount. + // 4. Extract data from the reconciled ServiceAccount. builder.WithDataExtractor(func(sa corev1.ServiceAccount) error { fmt.Printf("Reconciled ServiceAccount: %s\n", sa.Name) fmt.Printf(" ImagePullSecrets: ") @@ -60,6 +57,6 @@ func NewServiceAccountResource(owner *sharedapp.ExampleApp) (component.Resource, return nil }) - // 6. Build the final resource. + // 5. Build the final resource. return builder.Build() } From ecb67617741af4fe7c7674ef03aaef3bd9863e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:02:57 +0000 Subject: [PATCH 19/21] address copilot review: remove stale flavor references, add SA example to run-examples - Remove "Field Flavors" / PreserveCurrentLabels mention from README (the API was removed in cd27004 but the docs were not fully updated) - Add serviceaccount-primitive to the Makefile run-examples target Co-Authored-By: Claude Opus 4.6 (1M context) --- Makefile | 1 + examples/serviceaccount-primitive/README.md | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 67dde702..f523e323 100644 --- a/Makefile +++ b/Makefile @@ -122,6 +122,7 @@ build-examples: ## Build all example binaries. run-examples: ## Run all examples to verify they execute without error. go run ./examples/deployment-primitive/. go run ./examples/configmap-primitive/. + go run ./examples/serviceaccount-primitive/. go run ./examples/custom-resource-implementation/. ##@ E2E Testing diff --git a/examples/serviceaccount-primitive/README.md b/examples/serviceaccount-primitive/README.md index c1a6ba18..6de01570 100644 --- a/examples/serviceaccount-primitive/README.md +++ b/examples/serviceaccount-primitive/README.md @@ -6,7 +6,6 @@ how to manage a Kubernetes ServiceAccount as a component of a larger application - **Base Construction**: Initializing a ServiceAccount with basic metadata. - **Feature Mutations**: Composing image pull secrets and automount settings from independent, feature-gated mutations. - **Metadata Mutations**: Setting version labels on the ServiceAccount via `EditObjectMetadata`. -- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. - **Data Extraction**: Harvesting ServiceAccount fields after each reconcile cycle. ## Directory Structure From 1f3469b10ca8f56675e51c4158d491ea3585bc68 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:25:19 +0000 Subject: [PATCH 20/21] add ensureActive guard to serviceaccount mutator methods Prevent nil pointer panic when mutator methods are called without a prior BeginFeature() call. Each public mutation method now calls ensureActive() which panics with a clear message instead of a cryptic nil dereference. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/mutator.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index f601148d..a7370de2 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -51,6 +51,12 @@ func (m *Mutator) BeginFeature() { m.active = &m.plans[len(m.plans)-1] } +func (m *Mutator) ensureActive() { + if m.active == nil { + panic("serviceaccount.Mutator: BeginFeature() must be called before registering mutations") + } +} + // EditObjectMetadata records a mutation for the ServiceAccount's own metadata. // // Metadata edits are applied before image pull secret and automount edits within @@ -59,12 +65,14 @@ func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) if edit == nil { return } + m.ensureActive() m.active.metadataEdits = append(m.active.metadataEdits, edit) } // EnsureImagePullSecret records that the named image pull secret should be present // in .imagePullSecrets. If a secret with the same name already exists, it is a no-op. func (m *Mutator) EnsureImagePullSecret(name string) { + m.ensureActive() m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) { for _, ref := range sa.ImagePullSecrets { if ref.Name == name { @@ -78,6 +86,7 @@ func (m *Mutator) EnsureImagePullSecret(name string) { // RemoveImagePullSecret records that the named image pull secret should be removed // from .imagePullSecrets. It is a no-op if the secret is not present. func (m *Mutator) RemoveImagePullSecret(name string) { + m.ensureActive() m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) { filtered := sa.ImagePullSecrets[:0] for _, ref := range sa.ImagePullSecrets { @@ -101,6 +110,7 @@ func (m *Mutator) SetAutomountServiceAccountToken(v *bool) { snapshot = &val } + m.ensureActive() m.active.automountEdits = append(m.active.automountEdits, func(sa *corev1.ServiceAccount) { sa.AutomountServiceAccountToken = snapshot }) From 87eceaa1c100f59251a57c7c6065e79ddf3f7f54 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 17:00:12 +0000 Subject: [PATCH 21/21] replace panic with lazy BeginFeature in serviceaccount ensureActive guard Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/serviceaccount/mutator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/primitives/serviceaccount/mutator.go b/pkg/primitives/serviceaccount/mutator.go index a7370de2..a062e271 100644 --- a/pkg/primitives/serviceaccount/mutator.go +++ b/pkg/primitives/serviceaccount/mutator.go @@ -53,7 +53,7 @@ func (m *Mutator) BeginFeature() { func (m *Mutator) ensureActive() { if m.active == nil { - panic("serviceaccount.Mutator: BeginFeature() must be called before registering mutations") + m.BeginFeature() } }