From 8a6c262211a383059907707ec6b72824d5b127f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:43:08 +0000 Subject: [PATCH 01/27] Add NetworkPolicy primitive with editor, builder, mutator, and flavors Implements the NetworkPolicy static primitive following the configmap reference pattern. Includes NetworkPolicySpecEditor tests, typed mutator with plan-and-apply pattern, and PreserveCurrentLabels/Annotations flavors. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../editors/networkpolicyspec_test.go | 137 +++++++++++ pkg/primitives/networkpolicy/builder.go | 111 +++++++++ pkg/primitives/networkpolicy/builder_test.go | 154 ++++++++++++ pkg/primitives/networkpolicy/flavors.go | 25 ++ pkg/primitives/networkpolicy/flavors_test.go | 119 +++++++++ pkg/primitives/networkpolicy/mutator.go | 109 +++++++++ pkg/primitives/networkpolicy/mutator_test.go | 230 ++++++++++++++++++ pkg/primitives/networkpolicy/resource.go | 70 ++++++ 8 files changed, 955 insertions(+) create mode 100644 pkg/mutation/editors/networkpolicyspec_test.go create mode 100644 pkg/primitives/networkpolicy/builder.go create mode 100644 pkg/primitives/networkpolicy/builder_test.go create mode 100644 pkg/primitives/networkpolicy/flavors.go create mode 100644 pkg/primitives/networkpolicy/flavors_test.go create mode 100644 pkg/primitives/networkpolicy/mutator.go create mode 100644 pkg/primitives/networkpolicy/mutator_test.go create mode 100644 pkg/primitives/networkpolicy/resource.go diff --git a/pkg/mutation/editors/networkpolicyspec_test.go b/pkg/mutation/editors/networkpolicyspec_test.go new file mode 100644 index 00000000..46939790 --- /dev/null +++ b/pkg/mutation/editors/networkpolicyspec_test.go @@ -0,0 +1,137 @@ +package editors + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func TestNetworkPolicySpecEditor_Raw(t *testing.T) { + spec := &networkingv1.NetworkPolicySpec{} + e := NewNetworkPolicySpecEditor(spec) + assert.Same(t, spec, e.Raw()) +} + +func TestNetworkPolicySpecEditor_SetPodSelector(t *testing.T) { + spec := &networkingv1.NetworkPolicySpec{} + e := NewNetworkPolicySpecEditor(spec) + selector := metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + } + e.SetPodSelector(selector) + assert.Equal(t, selector, spec.PodSelector) +} + +func TestNetworkPolicySpecEditor_EnsureIngressRule_Appends(t *testing.T) { + spec := &networkingv1.NetworkPolicySpec{} + e := NewNetworkPolicySpecEditor(spec) + + port80 := intstr.FromInt32(80) + port443 := intstr.FromInt32(443) + tcp := corev1.ProtocolTCP + + rule1 := networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, + } + rule2 := networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, + } + + e.EnsureIngressRule(rule1) + e.EnsureIngressRule(rule2) + + assert.Len(t, spec.Ingress, 2) + assert.Equal(t, rule1, spec.Ingress[0]) + assert.Equal(t, rule2, spec.Ingress[1]) +} + +func TestNetworkPolicySpecEditor_RemoveIngressRules(t *testing.T) { + port80 := intstr.FromInt32(80) + tcp := corev1.ProtocolTCP + + spec := &networkingv1.NetworkPolicySpec{ + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}}, + }, + } + e := NewNetworkPolicySpecEditor(spec) + e.RemoveIngressRules() + assert.Nil(t, spec.Ingress) +} + +func TestNetworkPolicySpecEditor_EnsureEgressRule_Appends(t *testing.T) { + spec := &networkingv1.NetworkPolicySpec{} + e := NewNetworkPolicySpecEditor(spec) + + port53 := intstr.FromInt32(53) + port443 := intstr.FromInt32(443) + udp := corev1.ProtocolUDP + tcp := corev1.ProtocolTCP + + rule1 := networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &udp, Port: &port53}}, + } + rule2 := networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, + } + + e.EnsureEgressRule(rule1) + e.EnsureEgressRule(rule2) + + assert.Len(t, spec.Egress, 2) + assert.Equal(t, rule1, spec.Egress[0]) + assert.Equal(t, rule2, spec.Egress[1]) +} + +func TestNetworkPolicySpecEditor_RemoveEgressRules(t *testing.T) { + port53 := intstr.FromInt32(53) + udp := corev1.ProtocolUDP + + spec := &networkingv1.NetworkPolicySpec{ + Egress: []networkingv1.NetworkPolicyEgressRule{ + {Ports: []networkingv1.NetworkPolicyPort{{Protocol: &udp, Port: &port53}}}, + }, + } + e := NewNetworkPolicySpecEditor(spec) + e.RemoveEgressRules() + assert.Nil(t, spec.Egress) +} + +func TestNetworkPolicySpecEditor_SetPolicyTypes(t *testing.T) { + spec := &networkingv1.NetworkPolicySpec{} + e := NewNetworkPolicySpecEditor(spec) + types := []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + } + e.SetPolicyTypes(types) + assert.Equal(t, types, spec.PolicyTypes) +} + +func TestNetworkPolicySpecEditor_ReplaceIngressAtomically(t *testing.T) { + port80 := intstr.FromInt32(80) + port443 := intstr.FromInt32(443) + port8080 := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + + spec := &networkingv1.NetworkPolicySpec{ + Ingress: []networkingv1.NetworkPolicyIngressRule{ + {Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}}, + {Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}}, + }, + } + e := NewNetworkPolicySpecEditor(spec) + + e.RemoveIngressRules() + newRule := networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port8080}}, + } + e.EnsureIngressRule(newRule) + + assert.Len(t, spec.Ingress, 1) + assert.Equal(t, newRule, spec.Ingress[0]) +} diff --git a/pkg/primitives/networkpolicy/builder.go b/pkg/primitives/networkpolicy/builder.go new file mode 100644 index 00000000..2db41849 --- /dev/null +++ b/pkg/primitives/networkpolicy/builder.go @@ -0,0 +1,111 @@ +package networkpolicy + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/internal/generic" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + networkingv1 "k8s.io/api/networking/v1" +) + +// Builder is a configuration helper for creating and customizing a NetworkPolicy +// 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[*networkingv1.NetworkPolicy, *Mutator] +} + +// NewBuilder initializes a new Builder with the provided NetworkPolicy object. +// +// The NetworkPolicy 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 NetworkPolicy must have both Name and Namespace set, which is +// validated during the Build() call. +func NewBuilder(np *networkingv1.NetworkPolicy) *Builder { + identityFunc := func(n *networkingv1.NetworkPolicy) string { + return fmt.Sprintf("networking.k8s.io/v1/NetworkPolicy/%s/%s", n.Namespace, n.Name) + } + + return &Builder{ + base: generic.NewStaticBuilder[*networkingv1.NetworkPolicy, *Mutator]( + np, + identityFunc, + DefaultFieldApplicator, + NewMutator, + ), + } +} + +// WithMutation registers a mutation for the NetworkPolicy. +// +// 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 NetworkPolicy 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 *networkingv1.NetworkPolicy) 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[*networkingv1.NetworkPolicy](flavor)) + return b +} + +// WithDataExtractor registers a function to read values from the NetworkPolicy +// after it has been successfully reconciled. +// +// The extractor receives a value copy of the reconciled NetworkPolicy. This is +// useful for surfacing the applied policy rules to other components or resources. +// +// A nil extractor is ignored. +func (b *Builder) WithDataExtractor(extractor func(networkingv1.NetworkPolicy) error) *Builder { + if extractor != nil { + b.base.WithDataExtractor(func(np *networkingv1.NetworkPolicy) error { + return extractor(*np) + }) + } + return b +} + +// Build validates the configuration and returns the initialized Resource. +// +// It returns an error if: +// - No NetworkPolicy object was provided. +// - The NetworkPolicy 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/networkpolicy/builder_test.go b/pkg/primitives/networkpolicy/builder_test.go new file mode 100644 index 00000000..81d309c8 --- /dev/null +++ b/pkg/primitives/networkpolicy/builder_test.go @@ -0,0 +1,154 @@ +package networkpolicy + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestBuilder_Build_Validation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + np *networkingv1.NetworkPolicy + expectedErr string + }{ + { + name: "nil networkpolicy", + np: nil, + expectedErr: "object cannot be nil", + }, + { + name: "empty name", + np: &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Namespace: "test-ns"}, + }, + expectedErr: "object name cannot be empty", + }, + { + name: "empty namespace", + np: &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np"}, + }, + expectedErr: "object namespace cannot be empty", + }, + { + name: "valid networkpolicy", + np: &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res, err := NewBuilder(tt.np).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, "networking.k8s.io/v1/NetworkPolicy/test-ns/test-np", res.Identity()) + } + }) + } +} + +func TestBuilder_WithMutation(t *testing.T) { + t.Parallel() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + res, err := NewBuilder(np). + 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() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + called := false + applicator := func(_, _ *networkingv1.NetworkPolicy) error { + called = true + return nil + } + res, err := NewBuilder(np). + 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() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + res, err := NewBuilder(np). + 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() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + called := false + extractor := func(_ networkingv1.NetworkPolicy) error { + called = true + return nil + } + res, err := NewBuilder(np). + WithDataExtractor(extractor). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 1) + require.NoError(t, res.base.DataExtractors[0](&networkingv1.NetworkPolicy{})) + assert.True(t, called) +} + +func TestBuilder_WithDataExtractor_Nil(t *testing.T) { + t.Parallel() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + res, err := NewBuilder(np). + WithDataExtractor(nil). + Build() + require.NoError(t, err) + assert.Len(t, res.base.DataExtractors, 0) +} + +func TestBuilder_WithDataExtractor_ErrorPropagated(t *testing.T) { + t.Parallel() + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, + } + res, err := NewBuilder(np). + WithDataExtractor(func(_ networkingv1.NetworkPolicy) error { + return errors.New("extractor error") + }). + Build() + require.NoError(t, err) + err = res.base.DataExtractors[0](&networkingv1.NetworkPolicy{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "extractor error") +} diff --git a/pkg/primitives/networkpolicy/flavors.go b/pkg/primitives/networkpolicy/flavors.go new file mode 100644 index 00000000..4c8d49b7 --- /dev/null +++ b/pkg/primitives/networkpolicy/flavors.go @@ -0,0 +1,25 @@ +package networkpolicy + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/flavors" + networkingv1 "k8s.io/api/networking/v1" +) + +// FieldApplicationFlavor defines a function signature for applying flavors to a +// NetworkPolicy 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[*networkingv1.NetworkPolicy] + +// PreserveCurrentLabels ensures that any labels present on the current live +// NetworkPolicy but missing from the applied (desired) object are preserved. +// If a label exists in both, the applied value wins. +func PreserveCurrentLabels(applied, current, desired *networkingv1.NetworkPolicy) error { + return flavors.PreserveCurrentLabels[*networkingv1.NetworkPolicy]()(applied, current, desired) +} + +// PreserveCurrentAnnotations ensures that any annotations present on the current +// live NetworkPolicy but missing from the applied (desired) object are preserved. +// If an annotation exists in both, the applied value wins. +func PreserveCurrentAnnotations(applied, current, desired *networkingv1.NetworkPolicy) error { + return flavors.PreserveCurrentAnnotations[*networkingv1.NetworkPolicy]()(applied, current, desired) +} diff --git a/pkg/primitives/networkpolicy/flavors_test.go b/pkg/primitives/networkpolicy/flavors_test.go new file mode 100644 index 00000000..b8bd1c61 --- /dev/null +++ b/pkg/primitives/networkpolicy/flavors_test.go @@ -0,0 +1,119 @@ +package networkpolicy + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} + current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} + current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{} + current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} + current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} + current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + 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 := &networkingv1.NetworkPolicy{ + 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(_, _, _ *networkingv1.NetworkPolicy) error { + order = append(order, "flavor1") + return nil + } + flavor2 := func(_, _, _ *networkingv1.NetworkPolicy) 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(&networkingv1.NetworkPolicy{})) + 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(_, _, _ *networkingv1.NetworkPolicy) error { + return flavorErr + }). + Build() + require.NoError(t, err) + + err = res.Mutate(&networkingv1.NetworkPolicy{}) + require.Error(t, err) + assert.True(t, errors.Is(err, flavorErr)) + }) +} diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go new file mode 100644 index 00000000..bc730eee --- /dev/null +++ b/pkg/primitives/networkpolicy/mutator.go @@ -0,0 +1,109 @@ +// Package networkpolicy provides a builder and resource for managing Kubernetes NetworkPolicies. +package networkpolicy + +import ( + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" + networkingv1 "k8s.io/api/networking/v1" +) + +// Mutation defines a mutation that is applied to a networkpolicy Mutator +// only if its associated feature gate is enabled. +type Mutation feature.Mutation[*Mutator] + +type featurePlan struct { + metadataEdits []func(*editors.ObjectMetaEditor) error + specEdits []func(*editors.NetworkPolicySpecEditor) error +} + +// Mutator is a high-level helper for modifying a Kubernetes NetworkPolicy. +// +// It uses a "plan-and-apply" pattern: mutations are recorded first, then +// applied to the NetworkPolicy 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 { + np *networkingv1.NetworkPolicy + + plans []featurePlan + active *featurePlan +} + +// NewMutator creates a new Mutator for the given NetworkPolicy. +func NewMutator(np *networkingv1.NetworkPolicy) *Mutator { + m := &Mutator{np: np} + 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 NetworkPolicy's own metadata. +// +// Metadata edits are applied before spec edits within the same feature. +// A nil edit function is ignored. +func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) { + if edit == nil { + return + } + m.active.metadataEdits = append(m.active.metadataEdits, edit) +} + +// EditNetworkPolicySpec records a mutation for the NetworkPolicy's spec via a +// NetworkPolicySpecEditor. +// +// The editor provides structured operations (SetPodSelector, EnsureIngressRule, +// RemoveIngressRules, EnsureEgressRule, RemoveEgressRules, SetPolicyTypes) as +// well as Raw() for free-form access. Spec edits are applied after metadata +// edits within the same feature, in registration order. +// +// A nil edit function is ignored. +func (m *Mutator) EditNetworkPolicySpec(edit func(*editors.NetworkPolicySpecEditor) error) { + if edit == nil { + return + } + m.active.specEdits = append(m.active.specEdits, edit) +} + +// Apply executes all recorded mutation intents on the underlying NetworkPolicy. +// +// Execution order across all registered features: +// +// 1. Metadata edits (in registration order within each feature) +// 2. Spec edits — EditNetworkPolicySpec (in registration order within each feature) +// +// Features are applied in the order they were registered. Later features observe +// the NetworkPolicy 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.np.ObjectMeta) + for _, edit := range plan.metadataEdits { + if err := edit(editor); err != nil { + return err + } + } + } + + // 2. Spec edits + if len(plan.specEdits) > 0 { + editor := editors.NewNetworkPolicySpecEditor(&m.np.Spec) + for _, edit := range plan.specEdits { + if err := edit(editor); err != nil { + return err + } + } + } + } + + return nil +} diff --git a/pkg/primitives/networkpolicy/mutator_test.go b/pkg/primitives/networkpolicy/mutator_test.go new file mode 100644 index 00000000..e22f9d9d --- /dev/null +++ b/pkg/primitives/networkpolicy/mutator_test.go @@ -0,0 +1,230 @@ +package networkpolicy + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" +) + +func newTestNP() *networkingv1.NetworkPolicy { + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-np", + Namespace: "default", + }, + } +} + +// --- EditObjectMetadata --- + +func TestMutator_EditObjectMetadata(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app", "myapp") + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, "myapp", np.Labels["app"]) +} + +func TestMutator_EditObjectMetadata_Nil(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditObjectMetadata(nil) + assert.NoError(t, m.Apply()) +} + +// --- EditNetworkPolicySpec --- + +func TestMutator_EditNetworkPolicySpec_SetPodSelector(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.SetPodSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + }) + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, map[string]string{"app": "web"}, np.Spec.PodSelector.MatchLabels) +} + +func TestMutator_EditNetworkPolicySpec_Nil(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditNetworkPolicySpec(nil) + assert.NoError(t, m.Apply()) +} + +func TestMutator_EditNetworkPolicySpec_IngressRules(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + + port80 := intstr.FromInt32(80) + tcp := corev1.ProtocolTCP + + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, + }) + return nil + }) + require.NoError(t, m.Apply()) + assert.Len(t, np.Spec.Ingress, 1) +} + +func TestMutator_EditNetworkPolicySpec_EgressRules(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + + port443 := intstr.FromInt32(443) + tcp := corev1.ProtocolTCP + + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, + }) + return nil + }) + require.NoError(t, m.Apply()) + assert.Len(t, np.Spec.Egress, 1) +} + +func TestMutator_EditNetworkPolicySpec_ReplaceIngressAtomically(t *testing.T) { + port80 := intstr.FromInt32(80) + port8080 := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + + np := newTestNP() + np.Spec.Ingress = []networkingv1.NetworkPolicyIngressRule{ + {Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}}, + } + + m := NewMutator(np) + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.RemoveIngressRules() + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port8080}}, + }) + return nil + }) + require.NoError(t, m.Apply()) + require.Len(t, np.Spec.Ingress, 1) + assert.Equal(t, &port8080, np.Spec.Ingress[0].Ports[0].Port) +} + +func TestMutator_EditNetworkPolicySpec_SetPolicyTypes(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.SetPolicyTypes([]networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }) + return nil + }) + require.NoError(t, m.Apply()) + assert.Len(t, np.Spec.PolicyTypes, 2) +} + +func TestMutator_EditNetworkPolicySpec_Raw(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + raw := e.Raw() + raw.PodSelector = metav1.LabelSelector{ + MatchLabels: map[string]string{"role": "db"}, + } + return nil + }) + require.NoError(t, m.Apply()) + assert.Equal(t, map[string]string{"role": "db"}, np.Spec.PodSelector.MatchLabels) +} + +// --- Execution order --- + +func TestMutator_OperationOrder(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + + port80 := intstr.FromInt32(80) + tcp := corev1.ProtocolTCP + + // Register in reverse logical order to confirm Apply() enforces category ordering. + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, + }) + return nil + }) + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("order", "tested") + return nil + }) + require.NoError(t, m.Apply()) + + assert.Equal(t, "tested", np.Labels["order"]) + assert.Len(t, np.Spec.Ingress, 1) +} + +func TestMutator_MultipleFeatures(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + + port80 := intstr.FromInt32(80) + port443 := intstr.FromInt32(443) + tcp := corev1.ProtocolTCP + + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, + }) + return nil + }) + m.beginFeature() + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, + }) + return nil + }) + require.NoError(t, m.Apply()) + + assert.Len(t, np.Spec.Ingress, 1) + assert.Len(t, np.Spec.Egress, 1) +} + +// --- Error propagation --- + +func TestMutator_MetadataEditError(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { + return errors.New("metadata error") + }) + assert.Error(t, m.Apply()) +} + +func TestMutator_SpecEditError(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + m.EditNetworkPolicySpec(func(_ *editors.NetworkPolicySpecEditor) error { + return errors.New("spec error") + }) + assert.Error(t, m.Apply()) +} + +// --- ObjectMutator interface --- + +func TestMutator_ImplementsObjectMutator(_ *testing.T) { + var _ editors.ObjectMutator = (*Mutator)(nil) +} diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go new file mode 100644 index 00000000..4f1a2f05 --- /dev/null +++ b/pkg/primitives/networkpolicy/resource.go @@ -0,0 +1,70 @@ +package networkpolicy + +import ( + "github.com/sourcehawk/operator-component-framework/internal/generic" + networkingv1 "k8s.io/api/networking/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 NetworkPolicy +// resources. Use a custom field applicator via Builder.WithCustomFieldApplicator +// if you need to preserve fields that other controllers manage. +func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error { + *current = *desired.DeepCopy() + return nil +} + +// Resource is a high-level abstraction for managing a Kubernetes NetworkPolicy +// 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. +// +// NetworkPolicy resources are static: they do not model convergence health, grace +// periods, or suspension. Use a workload or integration primitive for resources +// that require those concepts. +type Resource struct { + base *generic.StaticResource[*networkingv1.NetworkPolicy, *Mutator] +} + +// Identity returns a unique identifier for the NetworkPolicy in the format +// "networking.k8s.io/v1/NetworkPolicy//". +func (r *Resource) Identity() string { + return r.base.Identity() +} + +// Object returns a deep copy of the underlying Kubernetes NetworkPolicy 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 NetworkPolicy 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 NetworkPolicy. +// +// This is called by the framework after successful reconciliation, allowing the +// component to read generated or updated values from the NetworkPolicy. +func (r *Resource) ExtractData() error { + return r.base.ExtractData() +} From 849a657520e3cee5735222a80f10d9c8df14d4b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:47:57 +0000 Subject: [PATCH 02/27] Add NetworkPolicy primitive example with feature-gated ingress rules Demonstrates HTTP ingress, boolean-gated metrics ingress, DNS egress, version labels, and PreserveCurrentLabels flavor. The DefaultFieldApplicator preserves ResourceVersion across reconcile cycles. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../networkpolicy-primitive/app/controller.go | 54 +++++++++ .../features/mutations.go | 94 +++++++++++++++ examples/networkpolicy-primitive/main.go | 110 ++++++++++++++++++ .../resources/networkpolicy.go | 63 ++++++++++ pkg/primitives/networkpolicy/resource.go | 2 + 5 files changed, 323 insertions(+) create mode 100644 examples/networkpolicy-primitive/app/controller.go create mode 100644 examples/networkpolicy-primitive/features/mutations.go create mode 100644 examples/networkpolicy-primitive/main.go create mode 100644 examples/networkpolicy-primitive/resources/networkpolicy.go diff --git a/examples/networkpolicy-primitive/app/controller.go b/examples/networkpolicy-primitive/app/controller.go new file mode 100644 index 00000000..8bff6323 --- /dev/null +++ b/examples/networkpolicy-primitive/app/controller.go @@ -0,0 +1,54 @@ +// Package app provides a sample controller using the networkpolicy 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 + + // NewNetworkPolicyResource is a factory function to create the networkpolicy resource. + // This allows us to inject the resource construction logic. + NewNetworkPolicyResource 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 networkpolicy resource for this owner. + npResource, err := r.NewNetworkPolicyResource(owner) + if err != nil { + return err + } + + // 2. Build the component that manages the networkpolicy. + comp, err := component.NewComponentBuilder(). + WithName("example-app"). + WithConditionType("AppReady"). + WithResource(npResource, 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/networkpolicy-primitive/features/mutations.go b/examples/networkpolicy-primitive/features/mutations.go new file mode 100644 index 00000000..ded94071 --- /dev/null +++ b/examples/networkpolicy-primitive/features/mutations.go @@ -0,0 +1,94 @@ +// Package features provides sample mutations for the networkpolicy primitive example. +package features + +import ( + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "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/networkpolicy" +) + +// VersionLabelMutation sets the app.kubernetes.io/version label on the +// NetworkPolicy. It is always enabled. +func VersionLabelMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "version-label", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + return nil + }) + return nil + }, + } +} + +// HTTPIngressMutation adds an ingress rule allowing TCP traffic on port 8080. +// It is always enabled. +func HTTPIngressMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "http-ingress", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} + +// MetricsIngressMutation adds an ingress rule allowing TCP traffic on port 9090 +// for Prometheus scraping. It is enabled when enableMetrics is true. +func MetricsIngressMutation(version string, enableMetrics bool) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "metrics-ingress", + Feature: feature.NewResourceFeature(version, nil).When(enableMetrics), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(9090) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} + +// DNSEgressMutation adds an egress rule allowing UDP traffic on port 53 for DNS +// resolution. It is always enabled. +func DNSEgressMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "dns-egress", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(53) + udp := corev1.ProtocolUDP + e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &udp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} diff --git a/examples/networkpolicy-primitive/main.go b/examples/networkpolicy-primitive/main.go new file mode 100644 index 00000000..09b584bb --- /dev/null +++ b/examples/networkpolicy-primitive/main.go @@ -0,0 +1,110 @@ +// Package main is the entry point for the networkpolicy 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/networkpolicy-primitive/app" + "github.com/sourcehawk/operator-component-framework/examples/networkpolicy-primitive/resources" + sharedapp "github.com/sourcehawk/operator-component-framework/examples/shared/app" + networkingv1 "k8s.io/api/networking/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 := networkingv1.AddToScheme(scheme); err != nil { + fmt.Fprintf(os.Stderr, "failed to add networking/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, + }, + NewNetworkPolicyResource: resources.NewNetworkPolicyResource, + } + + // 4. Run reconciliation with multiple spec versions to demonstrate how + // feature-gated ingress rules compose from independent mutations. + specs := []sharedapp.ExampleAppSpec{ + { + Version: "1.2.3", + EnableTracing: true, + EnableMetrics: true, + }, + { + Version: "1.2.4", // Version upgrade + EnableTracing: true, + EnableMetrics: true, + }, + { + Version: "1.2.4", + EnableTracing: true, + EnableMetrics: false, // Disable metrics ingress + }, + } + + ctx := context.Background() + + for i, spec := range specs { + fmt.Printf("\n--- Step %d: Version=%s, Metrics=%v ---\n", + i+1, spec.Version, spec.EnableMetrics) + + owner.Spec = spec + if err := fakeClient.Update(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "failed to update owner: %v\n", err) + os.Exit(1) + } + + fmt.Println("Running reconciliation...") + if err := controller.Reconcile(ctx, owner); err != nil { + fmt.Fprintf(os.Stderr, "reconciliation failed: %v\n", err) + os.Exit(1) + } + + for _, cond := range owner.Status.Conditions { + fmt.Printf("Condition: %s, Status: %s, Reason: %s\n", + cond.Type, cond.Status, cond.Reason) + } + } + + fmt.Println("\nReconciliation sequence completed successfully!") +} diff --git a/examples/networkpolicy-primitive/resources/networkpolicy.go b/examples/networkpolicy-primitive/resources/networkpolicy.go new file mode 100644 index 00000000..88314e20 --- /dev/null +++ b/examples/networkpolicy-primitive/resources/networkpolicy.go @@ -0,0 +1,63 @@ +// Package resources provides resource implementations for the networkpolicy primitive example. +package resources + +import ( + "fmt" + + "github.com/sourcehawk/operator-component-framework/examples/networkpolicy-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/networkpolicy" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NewNetworkPolicyResource constructs a networkpolicy primitive resource with all the features. +func NewNetworkPolicyResource(owner *sharedapp.ExampleApp) (component.Resource, error) { + // 1. Create the base NetworkPolicy object. + base := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: owner.Name + "-netpol", + Namespace: owner.Namespace, + Labels: map[string]string{ + "app": owner.Name, + }, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": owner.Name, + }, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, + } + + // 2. Initialize the networkpolicy builder. + builder := networkpolicy.NewBuilder(base) + + // 3. Register mutations in dependency order. + builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) + builder.WithMutation(features.HTTPIngressMutation(owner.Spec.Version)) + builder.WithMutation(features.MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) + builder.WithMutation(features.DNSEgressMutation(owner.Spec.Version)) + + // 4. Preserve labels added by external controllers. + builder.WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels) + + // 5. Extract data from the reconciled NetworkPolicy. + builder.WithDataExtractor(func(np networkingv1.NetworkPolicy) error { + fmt.Printf("Reconciled NetworkPolicy: %s\n", np.Name) + fmt.Printf(" PodSelector: %v\n", np.Spec.PodSelector.MatchLabels) + fmt.Printf(" PolicyTypes: %v\n", np.Spec.PolicyTypes) + fmt.Printf(" IngressRules: %d\n", len(np.Spec.Ingress)) + fmt.Printf(" EgressRules: %d\n", len(np.Spec.Egress)) + return nil + }) + + // 6. Build the final resource. + return builder.Build() +} diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index 4f1a2f05..cf284a2a 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -12,7 +12,9 @@ import ( // resources. Use a custom field applicator via Builder.WithCustomFieldApplicator // if you need to preserve fields that other controllers manage. func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error { + rv := current.ResourceVersion *current = *desired.DeepCopy() + current.ResourceVersion = rv return nil } From 6e59ab6e2bd6d0ae364c6ad3e48b36c907d108ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 02:49:06 +0000 Subject: [PATCH 03/27] Add NetworkPolicy primitive documentation and example README Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 308 +++++++++++++++++++++ examples/networkpolicy-primitive/README.md | 31 +++ 2 files changed, 339 insertions(+) create mode 100644 docs/primitives/networkpolicy.md create mode 100644 examples/networkpolicy-primitive/README.md diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md new file mode 100644 index 00000000..83e0c875 --- /dev/null +++ b/docs/primitives/networkpolicy.md @@ -0,0 +1,308 @@ +# NetworkPolicy Primitive + +The `networkpolicy` primitive is the framework's built-in static abstraction for managing Kubernetes `NetworkPolicy` resources. It integrates with the component lifecycle and provides a structured mutation API for managing pod selectors, ingress rules, egress rules, and policy types. + +## Capabilities + +| Capability | Detail | +|-----------------------|------------------------------------------------------------------------------------------------------| +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| **Mutation pipeline** | Typed editors for NetworkPolicy spec and object metadata, with a raw escape hatch for free-form access | +| **Append semantics** | Ingress and egress rules have no unique key — `EnsureIngressRule`/`EnsureEgressRule` append unconditionally | +| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | +| **Data extraction** | Reads generated or updated values back from the reconciled NetworkPolicy after each sync cycle | + +## Building a NetworkPolicy Primitive + +```go +import "github.com/sourcehawk/operator-component-framework/pkg/primitives/networkpolicy" + +base := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-netpol", + Namespace: owner.Namespace, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": owner.Name}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + }, +} + +resource, err := networkpolicy.NewBuilder(base). + WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). + WithMutation(HTTPIngressMutation(owner.Spec.Version)). + Build() +``` + +## Default Field Application + +`DefaultFieldApplicator` replaces the current NetworkPolicy with a deep copy of the desired object, preserving the `ResourceVersion` from the current 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 := networkpolicy.NewBuilder(base). + WithCustomFieldApplicator(func(current, desired *networkingv1.NetworkPolicy) error { + // Only synchronise owned spec fields; leave other fields untouched. + current.Spec.Ingress = desired.Spec.Ingress + return nil + }). + Build() +``` + +## Mutations + +Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through typed editors. + +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: + +```go +func HTTPIngressMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "http-ingress", + Feature: feature.NewResourceFeature(version, nil), // always enabled + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + 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 MetricsIngressMutation(version string, enableMetrics bool) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "metrics-ingress", + Feature: feature.NewResourceFeature(version, nil).When(enableMetrics), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(9090) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} +``` + +### Version-gated mutations + +```go +var legacyConstraint = mustSemverConstraint("< 2.0.0") + +func LegacyNetworkPolicyMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "legacy-policy", + Feature: feature.NewResourceFeature( + version, + []feature.VersionConstraint{legacyConstraint}, + ), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.SetPolicyTypes([]networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }) + return nil + }) + return nil + }, + } +} +``` + +All version constraints and `When()` conditions must be satisfied for a mutation to apply. + +## Internal Mutation Ordering + +Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are recorded: + +| Step | Category | What it affects | +|------|-------------------|------------------------------------------------------------------| +| 1 | Metadata edits | Labels and annotations on the `NetworkPolicy` | +| 2 | Spec edits | Pod selector, ingress rules, egress rules, policy types via Raw | + +Within each category, edits are applied in their registration order. Later features observe the NetworkPolicy as modified by all previous features. + +## Editors + +### NetworkPolicySpecEditor + +The primary API for modifying the NetworkPolicy spec. Use `m.EditNetworkPolicySpec` for full control: + +```go +m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.SetPodSelector(metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + }) + port := intstr.FromInt32(80) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil +}) +``` + +#### SetPodSelector + +Sets the pod selector that determines which pods the policy applies to within the namespace. An empty `LabelSelector` matches all pods. + +#### EnsureIngressRule and EnsureEgressRule + +Append a rule unconditionally. Ingress and egress rules have no unique key, so these methods always append. To replace the full set of rules atomically, call `RemoveIngressRules` or `RemoveEgressRules` first: + +```go +m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + // Replace all ingress rules atomically. + e.RemoveIngressRules() + e.EnsureIngressRule(newRule1) + e.EnsureIngressRule(newRule2) + return nil +}) +``` + +#### RemoveIngressRules and RemoveEgressRules + +Clear all ingress or egress rules respectively. Use before `EnsureIngressRule`/`EnsureEgressRule` to replace the full set atomically. + +#### SetPolicyTypes + +Sets the policy types. Valid values are `networkingv1.PolicyTypeIngress` and `networkingv1.PolicyTypeEgress`. When `Egress` is included, egress rules must be set explicitly to permit traffic; an empty list denies all egress. + +#### Raw Escape Hatch + +`Raw()` returns the underlying `*networkingv1.NetworkPolicySpec` for free-form editing when none of the structured methods are sufficient: + +```go +m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + raw := e.Raw() + raw.PodSelector.MatchLabels["role"] = "db" + return nil +}) +``` + +### ObjectMetaEditor + +Modifies labels and annotations via `m.EditObjectMetadata`. + +Available methods: `EnsureLabel`, `RemoveLabel`, `EnsureAnnotation`, `RemoveAnnotation`, `Raw`. + +```go +m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("app.kubernetes.io/version", version) + e.EnsureAnnotation("policy/managed-by", "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 := networkpolicy.NewBuilder(base). + WithFieldApplicationFlavor(networkpolicy.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 := networkpolicy.NewBuilder(base). + WithFieldApplicationFlavor(networkpolicy.PreserveCurrentAnnotations). + Build() +``` + +Multiple flavors can be registered and run in registration order. + +## Full Example: Feature-Composed Network Policy + +```go +func HTTPIngressMutation(version string) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "http-ingress", + Feature: feature.NewResourceFeature(version, nil), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} + +func MetricsIngressMutation(version string, enabled bool) networkpolicy.Mutation { + return networkpolicy.Mutation{ + Name: "metrics-ingress", + Feature: feature.NewResourceFeature(version, nil).When(enabled), + Mutate: func(m *networkpolicy.Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + port := intstr.FromInt32(9090) + tcp := corev1.ProtocolTCP + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + } +} + +resource, err := networkpolicy.NewBuilder(base). + WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). + WithMutation(HTTPIngressMutation(owner.Spec.Version)). + WithMutation(MetricsIngressMutation(owner.Spec.Version, owner.Spec.MetricsEnabled)). + Build() +``` + +When `MetricsEnabled` is true, the final NetworkPolicy will have both HTTP and metrics ingress rules. When false, only the HTTP rule is present. 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 `RemoveIngressRules`/`RemoveEgressRules` for atomic replacement.** Since rules have no unique key, there is no upsert-by-key operation. To replace the full set of rules, call `Remove*Rules` first and then add the desired rules. Alternatively, use `Raw()` for fine-grained manipulation. + +**Register mutations in dependency order.** If mutation B relies on a rule added by mutation A, register A first. Since `EnsureIngressRule`/`EnsureEgressRule` append unconditionally, the order of registration determines the order of rules in the resulting spec. diff --git a/examples/networkpolicy-primitive/README.md b/examples/networkpolicy-primitive/README.md new file mode 100644 index 00000000..20b59413 --- /dev/null +++ b/examples/networkpolicy-primitive/README.md @@ -0,0 +1,31 @@ +# NetworkPolicy Primitive Example + +This example demonstrates the usage of the `networkpolicy` primitive within the operator component framework. +It shows how to manage a Kubernetes NetworkPolicy as a component of a larger application, utilising features like: + +- **Base Construction**: Initializing a NetworkPolicy with pod selector and policy types. +- **Feature Mutations**: Composing ingress and egress rules from independent, feature-gated mutations. +- **Boolean-Gated Rules**: Conditionally adding metrics ingress rules based on a spec flag. +- **Metadata Mutations**: Setting version labels on the NetworkPolicy via `EditObjectMetadata`. +- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. +- **Data Extraction**: Reading the applied policy configuration after each reconcile cycle. + +## Directory Structure + +- `app/`: Defines the controller that uses the component framework. The `ExampleApp` CRD is shared from `examples/shared/app`. +- `features/`: Contains modular feature definitions: + - `mutations.go`: HTTP ingress, boolean-gated metrics ingress, DNS egress, and version labelling. +- `resources/`: Contains the central `NewNetworkPolicyResource` factory that assembles all features using `networkpolicy.Builder`. +- `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. + +## Running the Example + +```bash +go run examples/networkpolicy-primitive/main.go +``` + +This will: +1. Initialize a fake Kubernetes client. +2. Create an `ExampleApp` owner object. +3. Reconcile through three spec variations, printing the applied policy details after each cycle. +4. Print the resulting status conditions. From cdab1da8c4a2be7c4dbc64e97664ea0c5f8fb3a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 03:26:53 +0000 Subject: [PATCH 04/27] Add missing NetworkPolicySpecEditor source file The NetworkPolicySpecEditor was referenced by tests and the networkpolicy primitive but was never committed, causing build failures in CI. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mutation/editors/networkpolicyspec.go | 81 +++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 pkg/mutation/editors/networkpolicyspec.go diff --git a/pkg/mutation/editors/networkpolicyspec.go b/pkg/mutation/editors/networkpolicyspec.go new file mode 100644 index 00000000..7bef7fd8 --- /dev/null +++ b/pkg/mutation/editors/networkpolicyspec.go @@ -0,0 +1,81 @@ +package editors + +import ( + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// NetworkPolicySpecEditor provides a typed API for mutating a Kubernetes +// NetworkPolicySpec. +// +// It exposes structured operations for managing the pod selector, ingress rules, +// egress rules, and policy types, as well as Raw() for free-form access when +// none of the structured methods are sufficient. +// +// Note: ingress and egress rules have no unique key. EnsureIngressRule and +// EnsureEgressRule append unconditionally. Use RemoveIngressRules or +// RemoveEgressRules to replace the full set atomically, or use Raw() for +// fine-grained manipulation. +type NetworkPolicySpecEditor struct { + spec *networkingv1.NetworkPolicySpec +} + +// NewNetworkPolicySpecEditor creates a new NetworkPolicySpecEditor wrapping the +// given NetworkPolicySpec pointer. +func NewNetworkPolicySpecEditor(spec *networkingv1.NetworkPolicySpec) *NetworkPolicySpecEditor { + return &NetworkPolicySpecEditor{spec: spec} +} + +// Raw returns the underlying *networkingv1.NetworkPolicySpec. +func (e *NetworkPolicySpecEditor) Raw() *networkingv1.NetworkPolicySpec { + return e.spec +} + +// SetPodSelector sets the pod selector for the NetworkPolicy. +// +// The selector determines which pods this policy applies to within the +// namespace. An empty LabelSelector matches all pods in the namespace. +func (e *NetworkPolicySpecEditor) SetPodSelector(selector metav1.LabelSelector) { + e.spec.PodSelector = selector +} + +// EnsureIngressRule appends an ingress rule to the NetworkPolicy. +// +// Rules have no unique key, so this method always appends. To replace the +// full set of ingress rules atomically, call RemoveIngressRules first and +// then add the desired rules, or use Raw() for fine-grained manipulation. +func (e *NetworkPolicySpecEditor) EnsureIngressRule(rule networkingv1.NetworkPolicyIngressRule) { + e.spec.Ingress = append(e.spec.Ingress, rule) +} + +// RemoveIngressRules clears all ingress rules from the NetworkPolicy. +// +// Use this before calling EnsureIngressRule to replace the full set atomically. +func (e *NetworkPolicySpecEditor) RemoveIngressRules() { + e.spec.Ingress = nil +} + +// EnsureEgressRule appends an egress rule to the NetworkPolicy. +// +// Rules have no unique key, so this method always appends. To replace the +// full set of egress rules atomically, call RemoveEgressRules first and +// then add the desired rules, or use Raw() for fine-grained manipulation. +func (e *NetworkPolicySpecEditor) EnsureEgressRule(rule networkingv1.NetworkPolicyEgressRule) { + e.spec.Egress = append(e.spec.Egress, rule) +} + +// RemoveEgressRules clears all egress rules from the NetworkPolicy. +// +// Use this before calling EnsureEgressRule to replace the full set atomically. +func (e *NetworkPolicySpecEditor) RemoveEgressRules() { + e.spec.Egress = nil +} + +// SetPolicyTypes sets the policy types for the NetworkPolicy. +// +// Valid values are networkingv1.PolicyTypeIngress and networkingv1.PolicyTypeEgress. +// When Egress is included in PolicyTypes, EgressRules must be set explicitly +// to permit traffic; an empty list denies all egress. +func (e *NetworkPolicySpecEditor) SetPolicyTypes(types []networkingv1.PolicyType) { + e.spec.PolicyTypes = types +} From 03a4232b083b28c07f1259301a23ed54b914365b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 04:30:46 +0000 Subject: [PATCH 05/27] Address Copilot review comments on NetworkPolicy primitive - Fix nil map panic in Raw() doc example by initializing MatchLabels before assigning into it - Remove ResourceVersion preservation from DefaultFieldApplicator to align with ConfigMap and Deployment primitives Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 3 +++ pkg/primitives/networkpolicy/resource.go | 2 -- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index 83e0c875..a6dcdc9f 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -202,6 +202,9 @@ Sets the policy types. Valid values are `networkingv1.PolicyTypeIngress` and `ne ```go m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { raw := e.Raw() + if raw.PodSelector.MatchLabels == nil { + raw.PodSelector.MatchLabels = make(map[string]string) + } raw.PodSelector.MatchLabels["role"] = "db" return nil }) diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index cf284a2a..4f1a2f05 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -12,9 +12,7 @@ import ( // resources. Use a custom field applicator via Builder.WithCustomFieldApplicator // if you need to preserve fields that other controllers manage. func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error { - rv := current.ResourceVersion *current = *desired.DeepCopy() - current.ResourceVersion = rv return nil } From 97515325c9c3cae165c20075475176fd06670df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 13:26:12 +0000 Subject: [PATCH 06/27] Fix inaccurate DefaultFieldApplicator docs for NetworkPolicy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation incorrectly claimed that DefaultFieldApplicator preserves the ResourceVersion from the current object. In reality, it performs a full deep copy replacement — consistent with the configmap and deployment primitives. Align the docs with the actual behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index a6dcdc9f..e2b63047 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -41,7 +41,7 @@ resource, err := networkpolicy.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current NetworkPolicy with a deep copy of the desired object, preserving the `ResourceVersion` from the current object. This ensures every reconciliation cycle produces a clean, predictable state and avoids any drift from the desired baseline. +`DefaultFieldApplicator` replaces the current NetworkPolicy 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: From 02c3a3895dbe75a8e90710b1e3f87fedcd1fa375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Sun, 22 Mar 2026 19:52:36 +0000 Subject: [PATCH 07/27] preserve server-managed metadata in default field applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/resource.go | 11 ++-- pkg/primitives/networkpolicy/resource_test.go | 55 +++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 pkg/primitives/networkpolicy/resource_test.go diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index 4f1a2f05..06cf167c 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/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 NetworkPolicy -// 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 *networkingv1.NetworkPolicy) error { + original := current.DeepCopy() *current = *desired.DeepCopy() + generic.PreserveServerManagedFields(current, original) return nil } diff --git a/pkg/primitives/networkpolicy/resource_test.go b/pkg/primitives/networkpolicy/resource_test.go new file mode 100644 index 00000000..db726934 --- /dev/null +++ b/pkg/primitives/networkpolicy/resource_test.go @@ -0,0 +1,55 @@ +package networkpolicy + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { + current := &networkingv1.NetworkPolicy{ + 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 := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + } + + err := DefaultFieldApplicator(current, desired) + require.NoError(t, err) + + // Desired spec and labels are applied + assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) + 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 7940f54484dea0771b27f2bfc9289b012fb83189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni?= Date: Sun, 22 Mar 2026 21:59:13 +0000 Subject: [PATCH 08/27] Apply suggestion from @sourcehawk --- docs/primitives/networkpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index e2b63047..788bf509 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -296,7 +296,7 @@ func MetricsIngressMutation(version string, enabled bool) networkpolicy.Mutation resource, err := networkpolicy.NewBuilder(base). WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). WithMutation(HTTPIngressMutation(owner.Spec.Version)). - WithMutation(MetricsIngressMutation(owner.Spec.Version, owner.Spec.MetricsEnabled)). + WithMutation(MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)). Build() ``` From b0f0e789a806eb07291aaa50b5065cd269921d44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 01:06:42 +0000 Subject: [PATCH 09/27] address comments --- docs/primitives/networkpolicy.md | 17 +- .../features/mutations.go | 19 +- .../resources/networkpolicy.go | 4 +- pkg/primitives/networkpolicy/mutator.go | 6 +- pkg/primitives/networkpolicy/resource.go | 2 +- pkg/primitives/networkpolicy/resource_test.go | 307 ++++++++++++++++++ 6 files changed, 328 insertions(+), 27 deletions(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index 788bf509..80ea77f7 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -35,7 +35,7 @@ base := &networkingv1.NetworkPolicy{ resource, err := networkpolicy.NewBuilder(base). WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). - WithMutation(HTTPIngressMutation(owner.Spec.Version)). + WithMutation(HTTPIngressMutation()). Build() ``` @@ -59,13 +59,13 @@ resource, err := networkpolicy.NewBuilder(base). Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through typed editors. -The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally. A feature with no version constraints and no `When()` conditions is also always enabled: +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally — prefer this for mutations that should always run, as it avoids unnecessary version parsing: ```go -func HTTPIngressMutation(version string) networkpolicy.Mutation { +func HTTPIngressMutation() networkpolicy.Mutation { return networkpolicy.Mutation{ - Name: "http-ingress", - Feature: feature.NewResourceFeature(version, nil), // always enabled + Name: "http-ingress", + // Feature is nil — mutation is applied unconditionally. Mutate: func(m *networkpolicy.Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) @@ -253,10 +253,9 @@ Multiple flavors can be registered and run in registration order. ## Full Example: Feature-Composed Network Policy ```go -func HTTPIngressMutation(version string) networkpolicy.Mutation { +func HTTPIngressMutation() networkpolicy.Mutation { return networkpolicy.Mutation{ - Name: "http-ingress", - Feature: feature.NewResourceFeature(version, nil), + Name: "http-ingress", Mutate: func(m *networkpolicy.Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) @@ -295,7 +294,7 @@ func MetricsIngressMutation(version string, enabled bool) networkpolicy.Mutation resource, err := networkpolicy.NewBuilder(base). WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). - WithMutation(HTTPIngressMutation(owner.Spec.Version)). + WithMutation(HTTPIngressMutation()). WithMutation(MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)). Build() ``` diff --git a/examples/networkpolicy-primitive/features/mutations.go b/examples/networkpolicy-primitive/features/mutations.go index ded94071..6c80f4f4 100644 --- a/examples/networkpolicy-primitive/features/mutations.go +++ b/examples/networkpolicy-primitive/features/mutations.go @@ -12,11 +12,10 @@ import ( ) // VersionLabelMutation sets the app.kubernetes.io/version label on the -// NetworkPolicy. It is always enabled. +// NetworkPolicy. It is always enabled (nil Feature gate). func VersionLabelMutation(version string) networkpolicy.Mutation { return networkpolicy.Mutation{ - Name: "version-label", - Feature: feature.NewResourceFeature(version, nil), + Name: "version-label", Mutate: func(m *networkpolicy.Mutator) error { m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app.kubernetes.io/version", version) @@ -28,11 +27,10 @@ func VersionLabelMutation(version string) networkpolicy.Mutation { } // HTTPIngressMutation adds an ingress rule allowing TCP traffic on port 8080. -// It is always enabled. -func HTTPIngressMutation(version string) networkpolicy.Mutation { +// It is always enabled (nil Feature gate). +func HTTPIngressMutation() networkpolicy.Mutation { return networkpolicy.Mutation{ - Name: "http-ingress", - Feature: feature.NewResourceFeature(version, nil), + Name: "http-ingress", Mutate: func(m *networkpolicy.Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) @@ -72,11 +70,10 @@ func MetricsIngressMutation(version string, enableMetrics bool) networkpolicy.Mu } // DNSEgressMutation adds an egress rule allowing UDP traffic on port 53 for DNS -// resolution. It is always enabled. -func DNSEgressMutation(version string) networkpolicy.Mutation { +// resolution. It is always enabled (nil Feature gate). +func DNSEgressMutation() networkpolicy.Mutation { return networkpolicy.Mutation{ - Name: "dns-egress", - Feature: feature.NewResourceFeature(version, nil), + Name: "dns-egress", Mutate: func(m *networkpolicy.Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(53) diff --git a/examples/networkpolicy-primitive/resources/networkpolicy.go b/examples/networkpolicy-primitive/resources/networkpolicy.go index 88314e20..495f83ed 100644 --- a/examples/networkpolicy-primitive/resources/networkpolicy.go +++ b/examples/networkpolicy-primitive/resources/networkpolicy.go @@ -41,9 +41,9 @@ func NewNetworkPolicyResource(owner *sharedapp.ExampleApp) (component.Resource, // 3. Register mutations in dependency order. builder.WithMutation(features.VersionLabelMutation(owner.Spec.Version)) - builder.WithMutation(features.HTTPIngressMutation(owner.Spec.Version)) + builder.WithMutation(features.HTTPIngressMutation()) builder.WithMutation(features.MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) - builder.WithMutation(features.DNSEgressMutation(owner.Spec.Version)) + builder.WithMutation(features.DNSEgressMutation()) // 4. Preserve labels added by external controllers. builder.WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index bc730eee..4f569920 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -21,10 +21,8 @@ type featurePlan struct { // It uses a "plan-and-apply" pattern: mutations are recorded first, then // applied to the NetworkPolicy 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. +// Within a single plan, edits are applied in category order: metadata edits +// first, then spec edits. All edits within a category run in registration order. type Mutator struct { np *networkingv1.NetworkPolicy diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index 06cf167c..d22d2661 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -22,7 +22,7 @@ func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error // // It implements the following component interfaces: // - component.Resource: for basic identity and mutation behaviour. -// - component.DataExtractable: for exporting values after successful reconciliation. +// - concepts.DataExtractable: for exporting values after successful reconciliation. // // NetworkPolicy resources are static: they do not model convergence health, grace // periods, or suspension. Use a workload or integration primitive for resources diff --git a/pkg/primitives/networkpolicy/resource_test.go b/pkg/primitives/networkpolicy/resource_test.go index db726934..eccf4385 100644 --- a/pkg/primitives/networkpolicy/resource_test.go +++ b/pkg/primitives/networkpolicy/resource_test.go @@ -1,14 +1,321 @@ package networkpolicy import ( + "errors" "testing" + "github.com/sourcehawk/operator-component-framework/pkg/feature" + "github.com/sourcehawk/operator-component-framework/pkg/mutation/editors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) +func TestResource_Identity(t *testing.T) { + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-netpol", + Namespace: "test-ns", + }, + } + res, _ := NewBuilder(np).Build() + + assert.Equal(t, "networking.k8s.io/v1/NetworkPolicy/test-ns/test-netpol", res.Identity()) +} + +func TestResource_Object(t *testing.T) { + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-netpol", + Namespace: "test-ns", + }, + } + res, _ := NewBuilder(np).Build() + + obj, err := res.Object() + require.NoError(t, err) + + got, ok := obj.(*networkingv1.NetworkPolicy) + require.True(t, ok) + assert.Equal(t, np.Name, got.Name) + assert.Equal(t, np.Namespace, got.Namespace) + + // Ensure it's a deep copy + got.Name = "changed" + assert.Equal(t, "test-netpol", np.Name) +} + +func TestResource_Mutate(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + } + + port := intstr.FromInt32(8080) + tcp := corev1.ProtocolTCP + + res, _ := NewBuilder(desired). + WithMutation(Mutation{ + Name: "test-mutation", + Feature: feature.NewResourceFeature("v1", nil).When(true), + Mutate: func(m *Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port}, + }, + }) + return nil + }) + return nil + }, + }). + Build() + + current := &networkingv1.NetworkPolicy{} + err := res.Mutate(current) + require.NoError(t, err) + + assert.Equal(t, "test", current.Labels["app"]) + assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) + require.Len(t, current.Spec.Ingress, 1) + require.Len(t, current.Spec.Ingress[0].Ports, 1) + assert.Equal(t, int32(8080), current.Spec.Ingress[0].Ports[0].Port.IntVal) +} + +func TestResource_Mutate_DisabledFeature(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + } + + res, _ := NewBuilder(desired). + WithMutation(Mutation{ + Name: "disabled-mutation", + Feature: feature.NewResourceFeature("v1", nil).When(false), + Mutate: func(m *Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("should-not", "appear") + return nil + }) + return nil + }, + }). + Build() + + current := &networkingv1.NetworkPolicy{} + err := res.Mutate(current) + require.NoError(t, err) + + assert.NotContains(t, current.Labels, "should-not") +} + +func TestResource_Mutate_NilFeatureIsUnconditional(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + } + + res, _ := NewBuilder(desired). + WithMutation(Mutation{ + Name: "unconditional", + Mutate: func(m *Mutator) error { + m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { + e.EnsureLabel("always", "present") + return nil + }) + return nil + }, + }). + Build() + + current := &networkingv1.NetworkPolicy{} + err := res.Mutate(current) + require.NoError(t, err) + + assert.Equal(t, "present", current.Labels["always"]) +} + +func TestResource_Mutate_MutationOrdering(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + } + + port80 := intstr.FromInt32(80) + port443 := intstr.FromInt32(443) + tcp := corev1.ProtocolTCP + + res, _ := NewBuilder(desired). + WithMutation(Mutation{ + Name: "http", + Mutate: func(m *Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port80}, + }, + }) + return nil + }) + return nil + }, + }). + WithMutation(Mutation{ + Name: "https", + Mutate: func(m *Mutator) error { + m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { + e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcp, Port: &port443}, + }, + }) + return nil + }) + return nil + }, + }). + Build() + + current := &networkingv1.NetworkPolicy{} + err := res.Mutate(current) + require.NoError(t, err) + + require.Len(t, current.Spec.Ingress, 2) + assert.Equal(t, int32(80), current.Spec.Ingress[0].Ports[0].Port.IntVal) + assert.Equal(t, int32(443), current.Spec.Ingress[1].Ports[0].Port.IntVal) +} + +func TestResource_Mutate_WithFlavor(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + } + + res, _ := NewBuilder(desired). + WithFieldApplicationFlavor(PreserveCurrentLabels). + Build() + + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"external": "label"}, + }, + } + err := res.Mutate(current) + require.NoError(t, err) + + assert.Equal(t, "test", current.Labels["app"]) + assert.Equal(t, "label", current.Labels["external"]) +} + +func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { + desired := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Labels: map[string]string{"app": "test"}, + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + } + + applicatorCalled := false + res, _ := NewBuilder(desired). + WithCustomFieldApplicator(func(current, desired *networkingv1.NetworkPolicy) error { + applicatorCalled = true + current.Name = desired.Name + current.Namespace = desired.Namespace + current.Spec.PodSelector = desired.Spec.PodSelector + return nil + }). + Build() + + current := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"external": "label"}, + }, + } + err := res.Mutate(current) + require.NoError(t, err) + + assert.True(t, applicatorCalled) + assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) + assert.Equal(t, "label", current.Labels["external"]) + assert.NotContains(t, current.Labels, "app") + + t.Run("returns error", func(t *testing.T) { + res, _ := NewBuilder(desired). + WithCustomFieldApplicator(func(_, _ *networkingv1.NetworkPolicy) error { + return errors.New("applicator error") + }). + Build() + + err := res.Mutate(&networkingv1.NetworkPolicy{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "applicator error") + }) +} + +func TestResource_ExtractData(t *testing.T) { + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + }, + } + + extractedSelector := "" + res, err := NewBuilder(np). + WithDataExtractor(func(np networkingv1.NetworkPolicy) error { + extractedSelector = np.Spec.PodSelector.MatchLabels["app"] + return nil + }). + Build() + require.NoError(t, err) + + err = res.ExtractData() + require.NoError(t, err) + assert.Equal(t, "test", extractedSelector) +} + func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { current := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ From 235f9cb6c7d80c112c71a19369acd484d02cbb2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 03:23:05 +0000 Subject: [PATCH 10/27] fix: align doc comments per Copilot review feedback - resource.go: use "workload or task primitive" wording consistent with configmap primitive instead of "integration primitive" - networkpolicyspec.go: clarify SetPolicyTypes comment to reference .Egress field instead of non-existent "EgressRules" Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mutation/editors/networkpolicyspec.go | 4 ++-- pkg/primitives/networkpolicy/resource.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/mutation/editors/networkpolicyspec.go b/pkg/mutation/editors/networkpolicyspec.go index 7bef7fd8..37a05e35 100644 --- a/pkg/mutation/editors/networkpolicyspec.go +++ b/pkg/mutation/editors/networkpolicyspec.go @@ -74,8 +74,8 @@ func (e *NetworkPolicySpecEditor) RemoveEgressRules() { // SetPolicyTypes sets the policy types for the NetworkPolicy. // // Valid values are networkingv1.PolicyTypeIngress and networkingv1.PolicyTypeEgress. -// When Egress is included in PolicyTypes, EgressRules must be set explicitly -// to permit traffic; an empty list denies all egress. +// When networkingv1.PolicyTypeEgress is included in PolicyTypes, the .Egress field +// (egress rules) must be set explicitly to permit traffic; an empty list denies all egress. func (e *NetworkPolicySpecEditor) SetPolicyTypes(types []networkingv1.PolicyType) { e.spec.PolicyTypes = types } diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index d22d2661..4b008f23 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -25,8 +25,8 @@ func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error // - concepts.DataExtractable: for exporting values after successful reconciliation. // // NetworkPolicy resources are static: they do not model convergence health, grace -// periods, or suspension. Use a workload or integration primitive for resources -// that require those concepts. +// periods, or suspension. Use a workload or task primitive for resources that +// require those concepts. type Resource struct { base *generic.StaticResource[*networkingv1.NetworkPolicy, *Mutator] } From b940579d215787238bbc7fe640f0659cbf9bc6cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:08:32 +0000 Subject: [PATCH 11/27] docs: document server-managed field preservation in NetworkPolicy default applicator Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index 80ea77f7..62e48628 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -41,7 +41,7 @@ resource, err := networkpolicy.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current NetworkPolicy 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 NetworkPolicy 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. NetworkPolicy has no Status subresource, so no status preservation is needed. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: From ace11f84073e5d0ffc72d6c28237bda29b099b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:42:55 +0000 Subject: [PATCH 12/27] fix: avoid duplicate beginFeature call in NetworkPolicy mutator constructor Initialize plans slice and active pointer directly in the constructor instead of calling beginFeature(), matching the fix applied to deployment and configmap primitives. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index 4f569920..3522b0aa 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -32,8 +32,11 @@ type Mutator struct { // NewMutator creates a new Mutator for the given NetworkPolicy. func NewMutator(np *networkingv1.NetworkPolicy) *Mutator { - m := &Mutator{np: np} - m.beginFeature() + m := &Mutator{ + np: np, + plans: []featurePlan{{}}, + } + m.active = &m.plans[0] return m } From 2dd76e0c1c7827eb8aac19ac06729481e4f667eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 16:58:13 +0000 Subject: [PATCH 13/27] docs: improve NetworkPolicy mutator GoDoc and add primitive to docs index Align the Mutator GoDoc with configmap/deployment conventions by documenting feature boundaries and the ObjectMutator interface. Add networkpolicy to the Built-in Primitives and Mutation Editors tables in docs/primitives.md so the new primitive is discoverable. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 6 ++++-- pkg/primitives/networkpolicy/mutator.go | 10 ++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index 13fde6a6..62c4b363 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -107,6 +107,7 @@ Editors provide scoped, typed APIs for modifying specific parts of a resource: | `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | | `DeploymentSpecEditor` | Replicas, update strategy, label selectors | | `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | +| `NetworkPolicySpecEditor` | Pod selector, ingress/egress rules, policy types | | `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | Every editor exposes a `.Raw()` method for cases where the typed API is insufficient, giving direct access to the underlying Kubernetes struct while keeping the mutation scoped to that editor's target. @@ -128,8 +129,9 @@ Selectors are evaluated against the container list *after* any presence operatio | Primitive | Category | Documentation | |--------------------------------------|------------|-----------------------------------------| -| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | -| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) | +| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) | +| `pkg/primitives/networkpolicy` | Static | [networkpolicy.md](primitives/networkpolicy.md) | ## Usage Examples diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index 3522b0aa..4ecf7437 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -21,8 +21,14 @@ type featurePlan struct { // It uses a "plan-and-apply" pattern: mutations are recorded first, then // applied to the NetworkPolicy in a single controlled pass when Apply() is called. // -// Within a single plan, edits are applied in category order: metadata edits -// first, then spec edits. All edits within a category run in registration order. +// The Mutator maintains feature boundaries: each feature's mutations are planned +// together and applied in the order the features were registered. +// +// Within a single feature plan, edits are applied in category order: metadata +// edits first, then spec edits. All edits within a category run in registration +// order. +// +// Mutator implements editors.ObjectMutator. type Mutator struct { np *networkingv1.NetworkPolicy From 3971a7487046a790bf8c41b3e17e6d2b855053dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Mon, 23 Mar 2026 21:33:31 +0000 Subject: [PATCH 14/27] fix: export BeginFeature to match FeatureMutator interface Align NetworkPolicy mutator with the exported BeginFeature() method now required by the FeatureMutator interface in resource_workload.go. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator.go | 4 ++-- pkg/primitives/networkpolicy/mutator_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index 4ecf7437..c78bb8f3 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -46,9 +46,9 @@ func NewMutator(np *networkingv1.NetworkPolicy) *Mutator { return m } -// beginFeature starts a new feature planning scope. All subsequent mutation +// BeginFeature starts a new feature planning scope. All subsequent mutation // registrations will be grouped into this feature's plan. -func (m *Mutator) beginFeature() { +func (m *Mutator) BeginFeature() { m.plans = append(m.plans, featurePlan{}) m.active = &m.plans[len(m.plans)-1] } diff --git a/pkg/primitives/networkpolicy/mutator_test.go b/pkg/primitives/networkpolicy/mutator_test.go index e22f9d9d..1b48cc35 100644 --- a/pkg/primitives/networkpolicy/mutator_test.go +++ b/pkg/primitives/networkpolicy/mutator_test.go @@ -190,7 +190,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { }) return nil }) - m.beginFeature() + m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, From f1edbd2fc0c74e1ffd38ab06ebf070423b9180ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:29:31 +0000 Subject: [PATCH 15/27] style: apply prettier markdown formatting Run `make fmt-md` to normalize markdown formatting across docs and examples added by the networkpolicy primitive. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives.md | 16 ++--- docs/primitives/networkpolicy.md | 79 ++++++++++++++-------- examples/networkpolicy-primitive/README.md | 13 ++-- 3 files changed, 67 insertions(+), 41 deletions(-) diff --git a/docs/primitives.md b/docs/primitives.md index 987a8455..3f1ce43c 100644 --- a/docs/primitives.md +++ b/docs/primitives.md @@ -124,14 +124,14 @@ This design: Editors provide scoped, typed APIs for modifying specific parts of a resource: -| Editor | Scope | -| ---------------------- | ----------------------------------------------------------------------- | -| `ContainerEditor` | Environment variables, arguments, resource limits, ports | -| `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | -| `DeploymentSpecEditor` | Replicas, update strategy, label selectors | -| `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | -| `NetworkPolicySpecEditor` | Pod selector, ingress/egress rules, policy types | -| `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | +| Editor | Scope | +| ------------------------- | ----------------------------------------------------------------------- | +| `ContainerEditor` | Environment variables, arguments, resource limits, ports | +| `PodSpecEditor` | Volumes, tolerations, node selectors, service account, security context | +| `DeploymentSpecEditor` | Replicas, update strategy, label selectors | +| `ConfigMapDataEditor` | `.data` entries — set, remove, deep-merge YAML patches, raw access | +| `NetworkPolicySpecEditor` | Pod selector, ingress/egress rules, policy types | +| `ObjectMetaEditor` | Labels and annotations on any Kubernetes object | Every editor exposes a `.Raw()` method for cases where the typed API is insufficient, giving direct access to the underlying Kubernetes struct while keeping the mutation scoped to that editor's target. diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index 62e48628..e87cdc55 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -1,16 +1,18 @@ # NetworkPolicy Primitive -The `networkpolicy` primitive is the framework's built-in static abstraction for managing Kubernetes `NetworkPolicy` resources. It integrates with the component lifecycle and provides a structured mutation API for managing pod selectors, ingress rules, egress rules, and policy types. +The `networkpolicy` primitive is the framework's built-in static abstraction for managing Kubernetes `NetworkPolicy` +resources. It integrates with the component lifecycle and provides a structured mutation API for managing pod selectors, +ingress rules, egress rules, and policy types. ## Capabilities -| Capability | Detail | -|-----------------------|------------------------------------------------------------------------------------------------------| -| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | -| **Mutation pipeline** | Typed editors for NetworkPolicy spec and object metadata, with a raw escape hatch for free-form access | +| Capability | Detail | +| --------------------- | ----------------------------------------------------------------------------------------------------------- | +| **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | +| **Mutation pipeline** | Typed editors for NetworkPolicy spec and object metadata, with a raw escape hatch for free-form access | | **Append semantics** | Ingress and egress rules have no unique key — `EnsureIngressRule`/`EnsureEgressRule` append unconditionally | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | -| **Data extraction** | Reads generated or updated values back from the reconciled NetworkPolicy 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 NetworkPolicy after each sync cycle | ## Building a NetworkPolicy Primitive @@ -41,7 +43,9 @@ resource, err := networkpolicy.NewBuilder(base). ## Default Field Application -`DefaultFieldApplicator` replaces the current NetworkPolicy 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. NetworkPolicy has no Status subresource, so no status preservation is needed. +`DefaultFieldApplicator` replaces the current NetworkPolicy 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. NetworkPolicy has no Status subresource, so no status preservation is needed. Use `WithCustomFieldApplicator` when other controllers manage fields that should not be overwritten: @@ -57,9 +61,11 @@ resource, err := networkpolicy.NewBuilder(base). ## Mutations -Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its baseline. Each mutation is a named function that receives a `*Mutator` and records edit intent through typed editors. +Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its baseline. Each mutation is a named +function that receives a `*Mutator` and records edit intent through typed editors. -The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally — prefer this for mutations that should always run, as it avoids unnecessary version parsing: +The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally — prefer this +for mutations that should always run, as it avoids unnecessary version parsing: ```go func HTTPIngressMutation() networkpolicy.Mutation { @@ -83,7 +89,8 @@ func HTTPIngressMutation() networkpolicy.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 @@ -138,14 +145,16 @@ All version constraints and `When()` conditions must be satisfied for a mutation ## Internal Mutation Ordering -Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are recorded: +Within a single mutation, edit operations are applied in a fixed category order regardless of the order they are +recorded: -| Step | Category | What it affects | -|------|-------------------|------------------------------------------------------------------| -| 1 | Metadata edits | Labels and annotations on the `NetworkPolicy` | -| 2 | Spec edits | Pod selector, ingress rules, egress rules, policy types via Raw | +| Step | Category | What it affects | +| ---- | -------------- | --------------------------------------------------------------- | +| 1 | Metadata edits | Labels and annotations on the `NetworkPolicy` | +| 2 | Spec edits | Pod selector, ingress rules, egress rules, policy types via Raw | -Within each category, edits are applied in their registration order. Later features observe the NetworkPolicy as modified by all previous features. +Within each category, edits are applied in their registration order. Later features observe the NetworkPolicy as +modified by all previous features. ## Editors @@ -171,11 +180,13 @@ m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { #### SetPodSelector -Sets the pod selector that determines which pods the policy applies to within the namespace. An empty `LabelSelector` matches all pods. +Sets the pod selector that determines which pods the policy applies to within the namespace. An empty `LabelSelector` +matches all pods. #### EnsureIngressRule and EnsureEgressRule -Append a rule unconditionally. Ingress and egress rules have no unique key, so these methods always append. To replace the full set of rules atomically, call `RemoveIngressRules` or `RemoveEgressRules` first: +Append a rule unconditionally. Ingress and egress rules have no unique key, so these methods always append. To replace +the full set of rules atomically, call `RemoveIngressRules` or `RemoveEgressRules` first: ```go m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { @@ -189,15 +200,18 @@ m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { #### RemoveIngressRules and RemoveEgressRules -Clear all ingress or egress rules respectively. Use before `EnsureIngressRule`/`EnsureEgressRule` to replace the full set atomically. +Clear all ingress or egress rules respectively. Use before `EnsureIngressRule`/`EnsureEgressRule` to replace the full +set atomically. #### SetPolicyTypes -Sets the policy types. Valid values are `networkingv1.PolicyTypeIngress` and `networkingv1.PolicyTypeEgress`. When `Egress` is included, egress rules must be set explicitly to permit traffic; an empty list denies all egress. +Sets the policy types. Valid values are `networkingv1.PolicyTypeIngress` and `networkingv1.PolicyTypeEgress`. When +`Egress` is included, egress rules must be set explicitly to permit traffic; an empty list denies all egress. #### Raw Escape Hatch -`Raw()` returns the underlying `*networkingv1.NetworkPolicySpec` for free-form editing when none of the structured methods are sufficient: +`Raw()` returns the underlying `*networkingv1.NetworkPolicySpec` for free-form editing when none of the structured +methods are sufficient: ```go m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { @@ -226,7 +240,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 @@ -240,7 +255,8 @@ resource, err := networkpolicy.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 := networkpolicy.NewBuilder(base). @@ -299,12 +315,19 @@ resource, err := networkpolicy.NewBuilder(base). Build() ``` -When `MetricsEnabled` is true, the final NetworkPolicy will have both HTTP and metrics ingress rules. When false, only the HTTP rule is present. Neither mutation needs to know about the other. +When `MetricsEnabled` is true, the final NetworkPolicy will have both HTTP and metrics ingress rules. When false, only +the HTTP rule is present. 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 `RemoveIngressRules`/`RemoveEgressRules` for atomic replacement.** Since rules have no unique key, there is no upsert-by-key operation. To replace the full set of rules, call `Remove*Rules` first and then add the desired rules. Alternatively, use `Raw()` for fine-grained manipulation. +**Use `RemoveIngressRules`/`RemoveEgressRules` for atomic replacement.** Since rules have no unique key, there is no +upsert-by-key operation. To replace the full set of rules, call `Remove*Rules` first and then add the desired rules. +Alternatively, use `Raw()` for fine-grained manipulation. -**Register mutations in dependency order.** If mutation B relies on a rule added by mutation A, register A first. Since `EnsureIngressRule`/`EnsureEgressRule` append unconditionally, the order of registration determines the order of rules in the resulting spec. +**Register mutations in dependency order.** If mutation B relies on a rule added by mutation A, register A first. Since +`EnsureIngressRule`/`EnsureEgressRule` append unconditionally, the order of registration determines the order of rules +in the resulting spec. diff --git a/examples/networkpolicy-primitive/README.md b/examples/networkpolicy-primitive/README.md index 20b59413..2f157e12 100644 --- a/examples/networkpolicy-primitive/README.md +++ b/examples/networkpolicy-primitive/README.md @@ -1,7 +1,7 @@ # NetworkPolicy Primitive Example -This example demonstrates the usage of the `networkpolicy` primitive within the operator component framework. -It shows how to manage a Kubernetes NetworkPolicy as a component of a larger application, utilising features like: +This example demonstrates the usage of the `networkpolicy` primitive within the operator component framework. It shows +how to manage a Kubernetes NetworkPolicy as a component of a larger application, utilising features like: - **Base Construction**: Initializing a NetworkPolicy with pod selector and policy types. - **Feature Mutations**: Composing ingress and egress rules from independent, feature-gated mutations. @@ -12,10 +12,12 @@ It shows how to manage a Kubernetes NetworkPolicy as a component of a larger app ## 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`: HTTP ingress, boolean-gated metrics ingress, DNS egress, and version labelling. -- `resources/`: Contains the central `NewNetworkPolicyResource` factory that assembles all features using `networkpolicy.Builder`. + - `mutations.go`: HTTP ingress, boolean-gated metrics ingress, DNS egress, and version labelling. +- `resources/`: Contains the central `NewNetworkPolicyResource` factory that assembles all features using + `networkpolicy.Builder`. - `main.go`: A standalone entry point that demonstrates multiple reconciliation cycles with a fake client. ## Running the Example @@ -25,6 +27,7 @@ go run examples/networkpolicy-primitive/main.go ``` This will: + 1. Initialize a fake Kubernetes client. 2. Create an `ExampleApp` owner object. 3. Reconcile through three spec variations, printing the applied policy details after each cycle. From 816c263636aae64903484262a5d0acdc49fa4fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 00:34:08 +0000 Subject: [PATCH 16/27] fix: assert Build() errors in networkpolicy resource tests Replace ignored Build() errors (res, _ := ...) with require.NoError assertions across all test functions to prevent confusing nil-pointer panics if builder validation ever changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/resource_test.go | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/pkg/primitives/networkpolicy/resource_test.go b/pkg/primitives/networkpolicy/resource_test.go index eccf4385..bb46d373 100644 --- a/pkg/primitives/networkpolicy/resource_test.go +++ b/pkg/primitives/networkpolicy/resource_test.go @@ -21,7 +21,8 @@ func TestResource_Identity(t *testing.T) { Namespace: "test-ns", }, } - res, _ := NewBuilder(np).Build() + res, err := NewBuilder(np).Build() + require.NoError(t, err) assert.Equal(t, "networking.k8s.io/v1/NetworkPolicy/test-ns/test-netpol", res.Identity()) } @@ -33,7 +34,8 @@ func TestResource_Object(t *testing.T) { Namespace: "test-ns", }, } - res, _ := NewBuilder(np).Build() + res, err := NewBuilder(np).Build() + require.NoError(t, err) obj, err := res.Object() require.NoError(t, err) @@ -65,7 +67,7 @@ func TestResource_Mutate(t *testing.T) { port := intstr.FromInt32(8080) tcp := corev1.ProtocolTCP - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithMutation(Mutation{ Name: "test-mutation", Feature: feature.NewResourceFeature("v1", nil).When(true), @@ -82,9 +84,10 @@ func TestResource_Mutate(t *testing.T) { }, }). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{} - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) assert.Equal(t, "test", current.Labels["app"]) @@ -107,7 +110,7 @@ func TestResource_Mutate_DisabledFeature(t *testing.T) { }, } - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithMutation(Mutation{ Name: "disabled-mutation", Feature: feature.NewResourceFeature("v1", nil).When(false), @@ -120,9 +123,10 @@ func TestResource_Mutate_DisabledFeature(t *testing.T) { }, }). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{} - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) assert.NotContains(t, current.Labels, "should-not") @@ -136,7 +140,7 @@ func TestResource_Mutate_NilFeatureIsUnconditional(t *testing.T) { }, } - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithMutation(Mutation{ Name: "unconditional", Mutate: func(m *Mutator) error { @@ -148,9 +152,10 @@ func TestResource_Mutate_NilFeatureIsUnconditional(t *testing.T) { }, }). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{} - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) assert.Equal(t, "present", current.Labels["always"]) @@ -173,7 +178,7 @@ func TestResource_Mutate_MutationOrdering(t *testing.T) { port443 := intstr.FromInt32(443) tcp := corev1.ProtocolTCP - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithMutation(Mutation{ Name: "http", Mutate: func(m *Mutator) error { @@ -203,9 +208,10 @@ func TestResource_Mutate_MutationOrdering(t *testing.T) { }, }). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{} - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) require.Len(t, current.Spec.Ingress, 2) @@ -222,16 +228,17 @@ func TestResource_Mutate_WithFlavor(t *testing.T) { }, } - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithFieldApplicationFlavor(PreserveCurrentLabels). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"external": "label"}, }, } - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) assert.Equal(t, "test", current.Labels["app"]) @@ -253,7 +260,7 @@ func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { } applicatorCalled := false - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithCustomFieldApplicator(func(current, desired *networkingv1.NetworkPolicy) error { applicatorCalled = true current.Name = desired.Name @@ -262,13 +269,14 @@ func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { return nil }). Build() + require.NoError(t, err) current := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"external": "label"}, }, } - err := res.Mutate(current) + err = res.Mutate(current) require.NoError(t, err) assert.True(t, applicatorCalled) @@ -277,13 +285,14 @@ func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { assert.NotContains(t, current.Labels, "app") t.Run("returns error", func(t *testing.T) { - res, _ := NewBuilder(desired). + res, err := NewBuilder(desired). WithCustomFieldApplicator(func(_, _ *networkingv1.NetworkPolicy) error { return errors.New("applicator error") }). Build() + require.NoError(t, err) - err := res.Mutate(&networkingv1.NetworkPolicy{}) + err = res.Mutate(&networkingv1.NetworkPolicy{}) require.Error(t, err) assert.Contains(t, err.Error(), "applicator error") }) From 3ddb19fdad72afce6469e52d76b94b448f0d0928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 01:12:06 +0000 Subject: [PATCH 17/27] fix: align MetricsEnabled reference with actual field name EnableMetrics Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index e87cdc55..b099c50d 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -315,7 +315,7 @@ resource, err := networkpolicy.NewBuilder(base). Build() ``` -When `MetricsEnabled` is true, the final NetworkPolicy will have both HTTP and metrics ingress rules. When false, only +When `EnableMetrics` is true, the final NetworkPolicy will have both HTTP and metrics ingress rules. When false, only the HTTP rule is present. Neither mutation needs to know about the other. ## Guidance From cda45609395f82fee5e3dc803812ef9754ab9f8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 02:13:23 +0000 Subject: [PATCH 18/27] refactor: rename EnsureIngressRule/EnsureEgressRule to AppendIngressRule/AppendEgressRule The Ensure* prefix implies idempotent upsert semantics (as used by EnsureEnvVar, EnsureToleration, etc.), but network policy rules have no unique key and these methods append unconditionally. Rename to Append* to accurately reflect the behavior and avoid misleading API consumers. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 22 +++++++++---------- .../features/mutations.go | 6 ++--- pkg/mutation/editors/networkpolicyspec.go | 16 +++++++------- .../editors/networkpolicyspec_test.go | 14 ++++++------ pkg/primitives/networkpolicy/mutator.go | 4 ++-- pkg/primitives/networkpolicy/mutator_test.go | 12 +++++----- pkg/primitives/networkpolicy/resource_test.go | 6 ++--- 7 files changed, 40 insertions(+), 40 deletions(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index b099c50d..a331d6f1 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -10,7 +10,7 @@ ingress rules, egress rules, and policy types. | --------------------- | ----------------------------------------------------------------------------------------------------------- | | **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | **Mutation pipeline** | Typed editors for NetworkPolicy spec and object metadata, with a raw escape hatch for free-form access | -| **Append semantics** | Ingress and egress rules have no unique key — `EnsureIngressRule`/`EnsureEgressRule` append unconditionally | +| **Append semantics** | Ingress and egress rules have no unique key — `AppendIngressRule`/`AppendEgressRule` append unconditionally | | **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | | **Data extraction** | Reads generated or updated values back from the reconciled NetworkPolicy after each sync cycle | @@ -76,7 +76,7 @@ func HTTPIngressMutation() networkpolicy.Mutation { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -103,7 +103,7 @@ func MetricsIngressMutation(version string, enableMetrics bool) networkpolicy.Mu m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(9090) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -169,7 +169,7 @@ m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { }) port := intstr.FromInt32(80) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -183,7 +183,7 @@ m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { Sets the pod selector that determines which pods the policy applies to within the namespace. An empty `LabelSelector` matches all pods. -#### EnsureIngressRule and EnsureEgressRule +#### AppendIngressRule and AppendEgressRule Append a rule unconditionally. Ingress and egress rules have no unique key, so these methods always append. To replace the full set of rules atomically, call `RemoveIngressRules` or `RemoveEgressRules` first: @@ -192,15 +192,15 @@ the full set of rules atomically, call `RemoveIngressRules` or `RemoveEgressRule m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { // Replace all ingress rules atomically. e.RemoveIngressRules() - e.EnsureIngressRule(newRule1) - e.EnsureIngressRule(newRule2) + e.AppendIngressRule(newRule1) + e.AppendIngressRule(newRule2) return nil }) ``` #### RemoveIngressRules and RemoveEgressRules -Clear all ingress or egress rules respectively. Use before `EnsureIngressRule`/`EnsureEgressRule` to replace the full +Clear all ingress or egress rules respectively. Use before `AppendIngressRule`/`AppendEgressRule` to replace the full set atomically. #### SetPolicyTypes @@ -276,7 +276,7 @@ func HTTPIngressMutation() networkpolicy.Mutation { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -296,7 +296,7 @@ func MetricsIngressMutation(version string, enabled bool) networkpolicy.Mutation m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(9090) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -329,5 +329,5 @@ upsert-by-key operation. To replace the full set of rules, call `Remove*Rules` f Alternatively, use `Raw()` for fine-grained manipulation. **Register mutations in dependency order.** If mutation B relies on a rule added by mutation A, register A first. Since -`EnsureIngressRule`/`EnsureEgressRule` append unconditionally, the order of registration determines the order of rules +`AppendIngressRule`/`AppendEgressRule` append unconditionally, the order of registration determines the order of rules in the resulting spec. diff --git a/examples/networkpolicy-primitive/features/mutations.go b/examples/networkpolicy-primitive/features/mutations.go index 6c80f4f4..92bb3edc 100644 --- a/examples/networkpolicy-primitive/features/mutations.go +++ b/examples/networkpolicy-primitive/features/mutations.go @@ -35,7 +35,7 @@ func HTTPIngressMutation() networkpolicy.Mutation { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(8080) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -57,7 +57,7 @@ func MetricsIngressMutation(version string, enableMetrics bool) networkpolicy.Mu m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(9090) tcp := corev1.ProtocolTCP - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -78,7 +78,7 @@ func DNSEgressMutation() networkpolicy.Mutation { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { port := intstr.FromInt32(53) udp := corev1.ProtocolUDP - e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + e.AppendEgressRule(networkingv1.NetworkPolicyEgressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &udp, Port: &port}, }, diff --git a/pkg/mutation/editors/networkpolicyspec.go b/pkg/mutation/editors/networkpolicyspec.go index 37a05e35..9afb600a 100644 --- a/pkg/mutation/editors/networkpolicyspec.go +++ b/pkg/mutation/editors/networkpolicyspec.go @@ -12,8 +12,8 @@ import ( // egress rules, and policy types, as well as Raw() for free-form access when // none of the structured methods are sufficient. // -// Note: ingress and egress rules have no unique key. EnsureIngressRule and -// EnsureEgressRule append unconditionally. Use RemoveIngressRules or +// Note: ingress and egress rules have no unique key. AppendIngressRule and +// AppendEgressRule append unconditionally. Use RemoveIngressRules or // RemoveEgressRules to replace the full set atomically, or use Raw() for // fine-grained manipulation. type NetworkPolicySpecEditor struct { @@ -39,34 +39,34 @@ func (e *NetworkPolicySpecEditor) SetPodSelector(selector metav1.LabelSelector) e.spec.PodSelector = selector } -// EnsureIngressRule appends an ingress rule to the NetworkPolicy. +// AppendIngressRule appends an ingress rule to the NetworkPolicy. // // Rules have no unique key, so this method always appends. To replace the // full set of ingress rules atomically, call RemoveIngressRules first and // then add the desired rules, or use Raw() for fine-grained manipulation. -func (e *NetworkPolicySpecEditor) EnsureIngressRule(rule networkingv1.NetworkPolicyIngressRule) { +func (e *NetworkPolicySpecEditor) AppendIngressRule(rule networkingv1.NetworkPolicyIngressRule) { e.spec.Ingress = append(e.spec.Ingress, rule) } // RemoveIngressRules clears all ingress rules from the NetworkPolicy. // -// Use this before calling EnsureIngressRule to replace the full set atomically. +// Use this before calling AppendIngressRule to replace the full set atomically. func (e *NetworkPolicySpecEditor) RemoveIngressRules() { e.spec.Ingress = nil } -// EnsureEgressRule appends an egress rule to the NetworkPolicy. +// AppendEgressRule appends an egress rule to the NetworkPolicy. // // Rules have no unique key, so this method always appends. To replace the // full set of egress rules atomically, call RemoveEgressRules first and // then add the desired rules, or use Raw() for fine-grained manipulation. -func (e *NetworkPolicySpecEditor) EnsureEgressRule(rule networkingv1.NetworkPolicyEgressRule) { +func (e *NetworkPolicySpecEditor) AppendEgressRule(rule networkingv1.NetworkPolicyEgressRule) { e.spec.Egress = append(e.spec.Egress, rule) } // RemoveEgressRules clears all egress rules from the NetworkPolicy. // -// Use this before calling EnsureEgressRule to replace the full set atomically. +// Use this before calling AppendEgressRule to replace the full set atomically. func (e *NetworkPolicySpecEditor) RemoveEgressRules() { e.spec.Egress = nil } diff --git a/pkg/mutation/editors/networkpolicyspec_test.go b/pkg/mutation/editors/networkpolicyspec_test.go index 46939790..04c4250b 100644 --- a/pkg/mutation/editors/networkpolicyspec_test.go +++ b/pkg/mutation/editors/networkpolicyspec_test.go @@ -26,7 +26,7 @@ func TestNetworkPolicySpecEditor_SetPodSelector(t *testing.T) { assert.Equal(t, selector, spec.PodSelector) } -func TestNetworkPolicySpecEditor_EnsureIngressRule_Appends(t *testing.T) { +func TestNetworkPolicySpecEditor_AppendIngressRule_Appends(t *testing.T) { spec := &networkingv1.NetworkPolicySpec{} e := NewNetworkPolicySpecEditor(spec) @@ -41,8 +41,8 @@ func TestNetworkPolicySpecEditor_EnsureIngressRule_Appends(t *testing.T) { Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, } - e.EnsureIngressRule(rule1) - e.EnsureIngressRule(rule2) + e.AppendIngressRule(rule1) + e.AppendIngressRule(rule2) assert.Len(t, spec.Ingress, 2) assert.Equal(t, rule1, spec.Ingress[0]) @@ -63,7 +63,7 @@ func TestNetworkPolicySpecEditor_RemoveIngressRules(t *testing.T) { assert.Nil(t, spec.Ingress) } -func TestNetworkPolicySpecEditor_EnsureEgressRule_Appends(t *testing.T) { +func TestNetworkPolicySpecEditor_AppendEgressRule_Appends(t *testing.T) { spec := &networkingv1.NetworkPolicySpec{} e := NewNetworkPolicySpecEditor(spec) @@ -79,8 +79,8 @@ func TestNetworkPolicySpecEditor_EnsureEgressRule_Appends(t *testing.T) { Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, } - e.EnsureEgressRule(rule1) - e.EnsureEgressRule(rule2) + e.AppendEgressRule(rule1) + e.AppendEgressRule(rule2) assert.Len(t, spec.Egress, 2) assert.Equal(t, rule1, spec.Egress[0]) @@ -130,7 +130,7 @@ func TestNetworkPolicySpecEditor_ReplaceIngressAtomically(t *testing.T) { newRule := networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port8080}}, } - e.EnsureIngressRule(newRule) + e.AppendIngressRule(newRule) assert.Len(t, spec.Ingress, 1) assert.Equal(t, newRule, spec.Ingress[0]) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index c78bb8f3..55cd5dfd 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -67,8 +67,8 @@ func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) // EditNetworkPolicySpec records a mutation for the NetworkPolicy's spec via a // NetworkPolicySpecEditor. // -// The editor provides structured operations (SetPodSelector, EnsureIngressRule, -// RemoveIngressRules, EnsureEgressRule, RemoveEgressRules, SetPolicyTypes) as +// The editor provides structured operations (SetPodSelector, AppendIngressRule, +// RemoveIngressRules, AppendEgressRule, RemoveEgressRules, SetPolicyTypes) as // well as Raw() for free-form access. Spec edits are applied after metadata // edits within the same feature, in registration order. // diff --git a/pkg/primitives/networkpolicy/mutator_test.go b/pkg/primitives/networkpolicy/mutator_test.go index 1b48cc35..9103fd1e 100644 --- a/pkg/primitives/networkpolicy/mutator_test.go +++ b/pkg/primitives/networkpolicy/mutator_test.go @@ -73,7 +73,7 @@ func TestMutator_EditNetworkPolicySpec_IngressRules(t *testing.T) { tcp := corev1.ProtocolTCP m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, }) return nil @@ -90,7 +90,7 @@ func TestMutator_EditNetworkPolicySpec_EgressRules(t *testing.T) { tcp := corev1.ProtocolTCP m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + e.AppendEgressRule(networkingv1.NetworkPolicyEgressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, }) return nil @@ -112,7 +112,7 @@ func TestMutator_EditNetworkPolicySpec_ReplaceIngressAtomically(t *testing.T) { m := NewMutator(np) m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { e.RemoveIngressRules() - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port8080}}, }) return nil @@ -161,7 +161,7 @@ func TestMutator_OperationOrder(t *testing.T) { // Register in reverse logical order to confirm Apply() enforces category ordering. m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, }) return nil @@ -185,14 +185,14 @@ func TestMutator_MultipleFeatures(t *testing.T) { tcp := corev1.ProtocolTCP m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port80}}, }) return nil }) m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureEgressRule(networkingv1.NetworkPolicyEgressRule{ + e.AppendEgressRule(networkingv1.NetworkPolicyEgressRule{ Ports: []networkingv1.NetworkPolicyPort{{Protocol: &tcp, Port: &port443}}, }) return nil diff --git a/pkg/primitives/networkpolicy/resource_test.go b/pkg/primitives/networkpolicy/resource_test.go index bb46d373..3814cbca 100644 --- a/pkg/primitives/networkpolicy/resource_test.go +++ b/pkg/primitives/networkpolicy/resource_test.go @@ -73,7 +73,7 @@ func TestResource_Mutate(t *testing.T) { Feature: feature.NewResourceFeature("v1", nil).When(true), Mutate: func(m *Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port}, }, @@ -183,7 +183,7 @@ func TestResource_Mutate_MutationOrdering(t *testing.T) { Name: "http", Mutate: func(m *Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port80}, }, @@ -197,7 +197,7 @@ func TestResource_Mutate_MutationOrdering(t *testing.T) { Name: "https", Mutate: func(m *Mutator) error { m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { - e.EnsureIngressRule(networkingv1.NetworkPolicyIngressRule{ + e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ Ports: []networkingv1.NetworkPolicyPort{ {Protocol: &tcp, Port: &port443}, }, From 5a8800fc737daff3de051f1c43d8d50aebbf85d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 02:20:36 +0000 Subject: [PATCH 19/27] docs: fix inaccurate description of nil Feature behavior Replace "avoids unnecessary version parsing" with "do not need feature-gate evaluation" since version parsing does not occur simply from using a non-nil feature. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index a331d6f1..788a6dbc 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -65,7 +65,7 @@ Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its b function that receives a `*Mutator` and records edit intent through typed editors. The `Feature` field controls when a mutation applies. Leaving it nil applies the mutation unconditionally — prefer this -for mutations that should always run, as it avoids unnecessary version parsing: +for mutations that should always run and do not need feature-gate evaluation: ```go func HTTPIngressMutation() networkpolicy.Mutation { From c4014f38c3ee52c530e5cf113404a3b3549200ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= Date: Tue, 24 Mar 2026 18:01:44 +0000 Subject: [PATCH 20/27] fix: do not initialize an empty plan on NetworkPolicy mutator construction Align with the configmap and deployment mutators: NewMutator no longer creates a default feature plan. BeginFeature must be called before registering any mutations. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator.go | 11 +++--- pkg/primitives/networkpolicy/mutator_test.go | 35 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index 55cd5dfd..c91b7c13 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -37,13 +37,14 @@ type Mutator struct { } // NewMutator creates a new Mutator for the given NetworkPolicy. +// +// It is typically used within a Feature's Mutation logic to express desired +// changes to the NetworkPolicy. BeginFeature must be called before registering +// any mutations. func NewMutator(np *networkingv1.NetworkPolicy) *Mutator { - m := &Mutator{ - np: np, - plans: []featurePlan{{}}, + return &Mutator{ + np: np, } - m.active = &m.plans[0] - return m } // BeginFeature starts a new feature planning scope. All subsequent mutation diff --git a/pkg/primitives/networkpolicy/mutator_test.go b/pkg/primitives/networkpolicy/mutator_test.go index 9103fd1e..6eb60f54 100644 --- a/pkg/primitives/networkpolicy/mutator_test.go +++ b/pkg/primitives/networkpolicy/mutator_test.go @@ -25,9 +25,32 @@ func newTestNP() *networkingv1.NetworkPolicy { // --- EditObjectMetadata --- +func TestNewMutator(t *testing.T) { + np := newTestNP() + m := NewMutator(np) + assert.NotNil(t, m) + assert.Equal(t, np, m.np) + 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) { + np := newTestNP() + m := NewMutator(np) + + 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 TestMutator_EditObjectMetadata(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditObjectMetadata(func(e *editors.ObjectMetaEditor) error { e.EnsureLabel("app", "myapp") return nil @@ -39,6 +62,7 @@ func TestMutator_EditObjectMetadata(t *testing.T) { func TestMutator_EditObjectMetadata_Nil(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditObjectMetadata(nil) assert.NoError(t, m.Apply()) } @@ -48,6 +72,7 @@ func TestMutator_EditObjectMetadata_Nil(t *testing.T) { func TestMutator_EditNetworkPolicySpec_SetPodSelector(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { e.SetPodSelector(metav1.LabelSelector{ MatchLabels: map[string]string{"app": "web"}, @@ -61,6 +86,7 @@ func TestMutator_EditNetworkPolicySpec_SetPodSelector(t *testing.T) { func TestMutator_EditNetworkPolicySpec_Nil(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(nil) assert.NoError(t, m.Apply()) } @@ -68,6 +94,7 @@ func TestMutator_EditNetworkPolicySpec_Nil(t *testing.T) { func TestMutator_EditNetworkPolicySpec_IngressRules(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() port80 := intstr.FromInt32(80) tcp := corev1.ProtocolTCP @@ -85,6 +112,7 @@ func TestMutator_EditNetworkPolicySpec_IngressRules(t *testing.T) { func TestMutator_EditNetworkPolicySpec_EgressRules(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() port443 := intstr.FromInt32(443) tcp := corev1.ProtocolTCP @@ -110,6 +138,7 @@ func TestMutator_EditNetworkPolicySpec_ReplaceIngressAtomically(t *testing.T) { } m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { e.RemoveIngressRules() e.AppendIngressRule(networkingv1.NetworkPolicyIngressRule{ @@ -125,6 +154,7 @@ func TestMutator_EditNetworkPolicySpec_ReplaceIngressAtomically(t *testing.T) { func TestMutator_EditNetworkPolicySpec_SetPolicyTypes(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { e.SetPolicyTypes([]networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -139,6 +169,7 @@ func TestMutator_EditNetworkPolicySpec_SetPolicyTypes(t *testing.T) { func TestMutator_EditNetworkPolicySpec_Raw(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(func(e *editors.NetworkPolicySpecEditor) error { raw := e.Raw() raw.PodSelector = metav1.LabelSelector{ @@ -155,6 +186,7 @@ func TestMutator_EditNetworkPolicySpec_Raw(t *testing.T) { func TestMutator_OperationOrder(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() port80 := intstr.FromInt32(80) tcp := corev1.ProtocolTCP @@ -179,6 +211,7 @@ func TestMutator_OperationOrder(t *testing.T) { func TestMutator_MultipleFeatures(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() port80 := intstr.FromInt32(80) port443 := intstr.FromInt32(443) @@ -208,6 +241,7 @@ func TestMutator_MultipleFeatures(t *testing.T) { func TestMutator_MetadataEditError(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditObjectMetadata(func(_ *editors.ObjectMetaEditor) error { return errors.New("metadata error") }) @@ -217,6 +251,7 @@ func TestMutator_MetadataEditError(t *testing.T) { func TestMutator_SpecEditError(t *testing.T) { np := newTestNP() m := NewMutator(np) + m.BeginFeature() m.EditNetworkPolicySpec(func(_ *editors.NetworkPolicySpecEditor) error { return errors.New("spec error") }) From eab4e60bda6ff805d7ffc1cc7f69b6f09a851db3 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:38 +0000 Subject: [PATCH 21/27] refactor: remove field applicators and flavors from networkpolicy primitive Align with the framework's switch to Server-Side Apply (SSA). Remove DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor, FieldApplicationFlavor, flavors.go, and flavors_test.go. Update builder to drop the defaultApplicator parameter, update tests to use Object() output instead of empty structs for Mutate(), and strip field applicator and flavor sections from primitive docs. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/primitives/networkpolicy.md | 49 ------ pkg/primitives/networkpolicy/builder.go | 41 +---- pkg/primitives/networkpolicy/builder_test.go | 32 ---- pkg/primitives/networkpolicy/flavors.go | 25 --- pkg/primitives/networkpolicy/flavors_test.go | 119 ------------- pkg/primitives/networkpolicy/resource.go | 19 +- pkg/primitives/networkpolicy/resource_test.go | 165 +++--------------- 7 files changed, 29 insertions(+), 421 deletions(-) delete mode 100644 pkg/primitives/networkpolicy/flavors.go delete mode 100644 pkg/primitives/networkpolicy/flavors_test.go diff --git a/docs/primitives/networkpolicy.md b/docs/primitives/networkpolicy.md index 788a6dbc..66784802 100644 --- a/docs/primitives/networkpolicy.md +++ b/docs/primitives/networkpolicy.md @@ -11,7 +11,6 @@ ingress rules, egress rules, and policy types. | **Static lifecycle** | No health tracking, grace periods, or suspension — the resource is reconciled to desired state | | **Mutation pipeline** | Typed editors for NetworkPolicy spec and object metadata, with a raw escape hatch for free-form access | | **Append semantics** | Ingress and egress rules have no unique key — `AppendIngressRule`/`AppendEgressRule` append unconditionally | -| **Flavors** | Preserves externally-managed fields — labels and annotations not owned by the operator | | **Data extraction** | Reads generated or updated values back from the reconciled NetworkPolicy after each sync cycle | ## Building a NetworkPolicy Primitive @@ -36,29 +35,10 @@ base := &networkingv1.NetworkPolicy{ } resource, err := networkpolicy.NewBuilder(base). - WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). WithMutation(HTTPIngressMutation()). Build() ``` -## Default Field Application - -`DefaultFieldApplicator` replaces the current NetworkPolicy 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. NetworkPolicy 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 := networkpolicy.NewBuilder(base). - WithCustomFieldApplicator(func(current, desired *networkingv1.NetworkPolicy) error { - // Only synchronise owned spec fields; leave other fields untouched. - current.Spec.Ingress = desired.Spec.Ingress - return nil - }). - Build() -``` - ## Mutations Mutations are the primary mechanism for modifying a `NetworkPolicy` beyond its baseline. Each mutation is a named @@ -238,34 +218,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 := networkpolicy.NewBuilder(base). - WithFieldApplicationFlavor(networkpolicy.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 := networkpolicy.NewBuilder(base). - WithFieldApplicationFlavor(networkpolicy.PreserveCurrentAnnotations). - Build() -``` - -Multiple flavors can be registered and run in registration order. - ## Full Example: Feature-Composed Network Policy ```go @@ -309,7 +261,6 @@ func MetricsIngressMutation(version string, enabled bool) networkpolicy.Mutation } resource, err := networkpolicy.NewBuilder(base). - WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels). WithMutation(HTTPIngressMutation()). WithMutation(MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)). Build() diff --git a/pkg/primitives/networkpolicy/builder.go b/pkg/primitives/networkpolicy/builder.go index 2db41849..cc9102b0 100644 --- a/pkg/primitives/networkpolicy/builder.go +++ b/pkg/primitives/networkpolicy/builder.go @@ -11,9 +11,9 @@ import ( // Builder is a configuration helper for creating and customizing a NetworkPolicy // 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[*networkingv1.NetworkPolicy, *Mutator] } @@ -22,7 +22,7 @@ type Builder struct { // // The NetworkPolicy 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 NetworkPolicy must have both Name and Namespace set, which is // validated during the Build() call. @@ -35,7 +35,6 @@ func NewBuilder(np *networkingv1.NetworkPolicy) *Builder { base: generic.NewStaticBuilder[*networkingv1.NetworkPolicy, *Mutator]( np, identityFunc, - DefaultFieldApplicator, NewMutator, ), } @@ -43,8 +42,7 @@ func NewBuilder(np *networkingv1.NetworkPolicy) *Builder { // WithMutation registers a mutation for the NetworkPolicy. // -// 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 { @@ -52,35 +50,6 @@ func (b *Builder) WithMutation(m Mutation) *Builder { return b } -// WithCustomFieldApplicator sets a custom strategy for applying the desired -// state to the existing NetworkPolicy 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 *networkingv1.NetworkPolicy) 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[*networkingv1.NetworkPolicy](flavor)) - return b -} - // WithDataExtractor registers a function to read values from the NetworkPolicy // after it has been successfully reconciled. // diff --git a/pkg/primitives/networkpolicy/builder_test.go b/pkg/primitives/networkpolicy/builder_test.go index 81d309c8..68bd89ed 100644 --- a/pkg/primitives/networkpolicy/builder_test.go +++ b/pkg/primitives/networkpolicy/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() - np := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, - } - called := false - applicator := func(_, _ *networkingv1.NetworkPolicy) error { - called = true - return nil - } - res, err := NewBuilder(np). - 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() - np := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{Name: "test-np", Namespace: "test-ns"}, - } - res, err := NewBuilder(np). - 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() np := &networkingv1.NetworkPolicy{ diff --git a/pkg/primitives/networkpolicy/flavors.go b/pkg/primitives/networkpolicy/flavors.go deleted file mode 100644 index 4c8d49b7..00000000 --- a/pkg/primitives/networkpolicy/flavors.go +++ /dev/null @@ -1,25 +0,0 @@ -package networkpolicy - -import ( - "github.com/sourcehawk/operator-component-framework/pkg/flavors" - networkingv1 "k8s.io/api/networking/v1" -) - -// FieldApplicationFlavor defines a function signature for applying flavors to a -// NetworkPolicy 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[*networkingv1.NetworkPolicy] - -// PreserveCurrentLabels ensures that any labels present on the current live -// NetworkPolicy but missing from the applied (desired) object are preserved. -// If a label exists in both, the applied value wins. -func PreserveCurrentLabels(applied, current, desired *networkingv1.NetworkPolicy) error { - return flavors.PreserveCurrentLabels[*networkingv1.NetworkPolicy]()(applied, current, desired) -} - -// PreserveCurrentAnnotations ensures that any annotations present on the current -// live NetworkPolicy but missing from the applied (desired) object are preserved. -// If an annotation exists in both, the applied value wins. -func PreserveCurrentAnnotations(applied, current, desired *networkingv1.NetworkPolicy) error { - return flavors.PreserveCurrentAnnotations[*networkingv1.NetworkPolicy]()(applied, current, desired) -} diff --git a/pkg/primitives/networkpolicy/flavors_test.go b/pkg/primitives/networkpolicy/flavors_test.go deleted file mode 100644 index b8bd1c61..00000000 --- a/pkg/primitives/networkpolicy/flavors_test.go +++ /dev/null @@ -1,119 +0,0 @@ -package networkpolicy - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - networkingv1 "k8s.io/api/networking/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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"keep": "applied"}}} - current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"key": "applied"}}} - current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{} - current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"keep": "applied"}}} - current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{"key": "applied"}}} - current := &networkingv1.NetworkPolicy{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 := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-np", - 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 := &networkingv1.NetworkPolicy{ - 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(_, _, _ *networkingv1.NetworkPolicy) error { - order = append(order, "flavor1") - return nil - } - flavor2 := func(_, _, _ *networkingv1.NetworkPolicy) 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(&networkingv1.NetworkPolicy{})) - 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(_, _, _ *networkingv1.NetworkPolicy) error { - return flavorErr - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&networkingv1.NetworkPolicy{}) - require.Error(t, err) - assert.True(t, errors.Is(err, flavorErr)) - }) -} diff --git a/pkg/primitives/networkpolicy/resource.go b/pkg/primitives/networkpolicy/resource.go index 4b008f23..d09b4048 100644 --- a/pkg/primitives/networkpolicy/resource.go +++ b/pkg/primitives/networkpolicy/resource.go @@ -6,17 +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.) -// and shared-controller fields (OwnerReferences, Finalizers) from the original -// current object. -func DefaultFieldApplicator(current, desired *networkingv1.NetworkPolicy) error { - original := current.DeepCopy() - *current = *desired.DeepCopy() - generic.PreserveServerManagedFields(current, original) - return nil -} - // Resource is a high-level abstraction for managing a Kubernetes NetworkPolicy // within a controller's reconciliation loop. // @@ -49,12 +38,8 @@ func (r *Resource) Object() (client.Object, error) { // desired state. // // The mutation process follows this order: -// 1. Field application: the current object is updated to reflect the desired base -// state, using either DefaultFieldApplicator or a custom applicator if one is -// configured. -// 2. Field application flavors: any registered flavors are applied in registration -// order. -// 3. Feature mutations: all registered feature-gated mutations are applied in order. +// 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/networkpolicy/resource_test.go b/pkg/primitives/networkpolicy/resource_test.go index 3814cbca..142b31cb 100644 --- a/pkg/primitives/networkpolicy/resource_test.go +++ b/pkg/primitives/networkpolicy/resource_test.go @@ -1,7 +1,6 @@ package networkpolicy import ( - "errors" "testing" "github.com/sourcehawk/operator-component-framework/pkg/feature" @@ -86,15 +85,16 @@ func TestResource_Mutate(t *testing.T) { Build() require.NoError(t, err) - current := &networkingv1.NetworkPolicy{} - err = res.Mutate(current) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.Equal(t, "test", current.Labels["app"]) - assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) - require.Len(t, current.Spec.Ingress, 1) - require.Len(t, current.Spec.Ingress[0].Ports, 1) - assert.Equal(t, int32(8080), current.Spec.Ingress[0].Ports[0].Port.IntVal) + got := obj.(*networkingv1.NetworkPolicy) + assert.Equal(t, "test", got.Labels["app"]) + assert.Equal(t, "test", got.Spec.PodSelector.MatchLabels["app"]) + require.Len(t, got.Spec.Ingress, 1) + require.Len(t, got.Spec.Ingress[0].Ports, 1) + assert.Equal(t, int32(8080), got.Spec.Ingress[0].Ports[0].Port.IntVal) } func TestResource_Mutate_DisabledFeature(t *testing.T) { @@ -125,11 +125,12 @@ func TestResource_Mutate_DisabledFeature(t *testing.T) { Build() require.NoError(t, err) - current := &networkingv1.NetworkPolicy{} - err = res.Mutate(current) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.NotContains(t, current.Labels, "should-not") + got := obj.(*networkingv1.NetworkPolicy) + assert.NotContains(t, got.Labels, "should-not") } func TestResource_Mutate_NilFeatureIsUnconditional(t *testing.T) { @@ -154,11 +155,12 @@ func TestResource_Mutate_NilFeatureIsUnconditional(t *testing.T) { Build() require.NoError(t, err) - current := &networkingv1.NetworkPolicy{} - err = res.Mutate(current) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.Equal(t, "present", current.Labels["always"]) + got := obj.(*networkingv1.NetworkPolicy) + assert.Equal(t, "present", got.Labels["always"]) } func TestResource_Mutate_MutationOrdering(t *testing.T) { @@ -210,92 +212,14 @@ func TestResource_Mutate_MutationOrdering(t *testing.T) { Build() require.NoError(t, err) - current := &networkingv1.NetworkPolicy{} - err = res.Mutate(current) - require.NoError(t, err) - - require.Len(t, current.Spec.Ingress, 2) - assert.Equal(t, int32(80), current.Spec.Ingress[0].Ports[0].Port.IntVal) - assert.Equal(t, int32(443), current.Spec.Ingress[1].Ports[0].Port.IntVal) -} - -func TestResource_Mutate_WithFlavor(t *testing.T) { - desired := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, - } - - res, err := NewBuilder(desired). - WithFieldApplicationFlavor(PreserveCurrentLabels). - Build() - require.NoError(t, err) - - current := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"external": "label"}, - }, - } - err = res.Mutate(current) - require.NoError(t, err) - - assert.Equal(t, "test", current.Labels["app"]) - assert.Equal(t, "label", current.Labels["external"]) -} - -func TestResource_Mutate_CustomFieldApplicator(t *testing.T) { - desired := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, - Spec: networkingv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "test"}, - }, - }, - } - - applicatorCalled := false - res, err := NewBuilder(desired). - WithCustomFieldApplicator(func(current, desired *networkingv1.NetworkPolicy) error { - applicatorCalled = true - current.Name = desired.Name - current.Namespace = desired.Namespace - current.Spec.PodSelector = desired.Spec.PodSelector - return nil - }). - Build() - require.NoError(t, err) - - current := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"external": "label"}, - }, - } - err = res.Mutate(current) + obj, err := res.Object() require.NoError(t, err) + require.NoError(t, res.Mutate(obj)) - assert.True(t, applicatorCalled) - assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) - assert.Equal(t, "label", current.Labels["external"]) - assert.NotContains(t, current.Labels, "app") - - t.Run("returns error", func(t *testing.T) { - res, err := NewBuilder(desired). - WithCustomFieldApplicator(func(_, _ *networkingv1.NetworkPolicy) error { - return errors.New("applicator error") - }). - Build() - require.NoError(t, err) - - err = res.Mutate(&networkingv1.NetworkPolicy{}) - require.Error(t, err) - assert.Contains(t, err.Error(), "applicator error") - }) + got := obj.(*networkingv1.NetworkPolicy) + require.Len(t, got.Spec.Ingress, 2) + assert.Equal(t, int32(80), got.Spec.Ingress[0].Ports[0].Port.IntVal) + assert.Equal(t, int32(443), got.Spec.Ingress[1].Ports[0].Port.IntVal) } func TestResource_ExtractData(t *testing.T) { @@ -324,48 +248,3 @@ func TestResource_ExtractData(t *testing.T) { require.NoError(t, err) assert.Equal(t, "test", extractedSelector) } - -func TestDefaultFieldApplicator_PreservesServerManagedFields(t *testing.T) { - current := &networkingv1.NetworkPolicy{ - 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 := &networkingv1.NetworkPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: "default", - Labels: map[string]string{"app": "test"}, - }, - Spec: networkingv1.NetworkPolicySpec{ - PodSelector: metav1.LabelSelector{ - MatchLabels: map[string]string{"app": "test"}, - }, - }, - } - - err := DefaultFieldApplicator(current, desired) - require.NoError(t, err) - - // Desired spec and labels are applied - assert.Equal(t, "test", current.Spec.PodSelector.MatchLabels["app"]) - 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 73b4abf9c9b670bd75b6911c346ea7cca467e914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:46:32 +0000 Subject: [PATCH 22/27] fix: remove references to deleted field applicators in example The networkpolicy example still referenced WithFieldApplicationFlavor and PreserveCurrentLabels which were removed in the prior refactor. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../networkpolicy-primitive/resources/networkpolicy.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/networkpolicy-primitive/resources/networkpolicy.go b/examples/networkpolicy-primitive/resources/networkpolicy.go index 495f83ed..0825847d 100644 --- a/examples/networkpolicy-primitive/resources/networkpolicy.go +++ b/examples/networkpolicy-primitive/resources/networkpolicy.go @@ -45,10 +45,7 @@ func NewNetworkPolicyResource(owner *sharedapp.ExampleApp) (component.Resource, builder.WithMutation(features.MetricsIngressMutation(owner.Spec.Version, owner.Spec.EnableMetrics)) builder.WithMutation(features.DNSEgressMutation()) - // 4. Preserve labels added by external controllers. - builder.WithFieldApplicationFlavor(networkpolicy.PreserveCurrentLabels) - - // 5. Extract data from the reconciled NetworkPolicy. + // 4. Extract data from the reconciled NetworkPolicy. builder.WithDataExtractor(func(np networkingv1.NetworkPolicy) error { fmt.Printf("Reconciled NetworkPolicy: %s\n", np.Name) fmt.Printf(" PodSelector: %v\n", np.Spec.PodSelector.MatchLabels) @@ -58,6 +55,6 @@ func NewNetworkPolicyResource(owner *sharedapp.ExampleApp) (component.Resource, return nil }) - // 6. Build the final resource. + // 5. Build the final resource. return builder.Build() } From 0968d4f160f43db4875c8bc4bc663d9ddac91075 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:34 +0000 Subject: [PATCH 23/27] docs: update README to remove references to deleted field flavors API The README still referenced PreserveCurrentLabels and field flavors which were removed in a prior refactor. Updated to match the current example code. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/networkpolicy-primitive/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/networkpolicy-primitive/README.md b/examples/networkpolicy-primitive/README.md index 2f157e12..38719801 100644 --- a/examples/networkpolicy-primitive/README.md +++ b/examples/networkpolicy-primitive/README.md @@ -6,8 +6,8 @@ how to manage a Kubernetes NetworkPolicy as a component of a larger application, - **Base Construction**: Initializing a NetworkPolicy with pod selector and policy types. - **Feature Mutations**: Composing ingress and egress rules from independent, feature-gated mutations. - **Boolean-Gated Rules**: Conditionally adding metrics ingress rules based on a spec flag. -- **Metadata Mutations**: Setting version labels on the NetworkPolicy via `EditObjectMetadata`. -- **Field Flavors**: Preserving labels managed by external controllers using `PreserveCurrentLabels`. +- **Metadata Mutations**: Setting version labels on the NetworkPolicy via metadata editors. +- **Label Coexistence**: Demonstrating how label updates from this component can coexist with labels managed by other controllers. - **Data Extraction**: Reading the applied policy configuration after each reconcile cycle. ## Directory Structure From 08868291107da839ee45ae98f51371b90ff9d569 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:10:49 +0000 Subject: [PATCH 24/27] fix lint --- examples/networkpolicy-primitive/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/networkpolicy-primitive/README.md b/examples/networkpolicy-primitive/README.md index 38719801..5d1737ee 100644 --- a/examples/networkpolicy-primitive/README.md +++ b/examples/networkpolicy-primitive/README.md @@ -7,7 +7,8 @@ how to manage a Kubernetes NetworkPolicy as a component of a larger application, - **Feature Mutations**: Composing ingress and egress rules from independent, feature-gated mutations. - **Boolean-Gated Rules**: Conditionally adding metrics ingress rules based on a spec flag. - **Metadata Mutations**: Setting version labels on the NetworkPolicy via metadata editors. -- **Label Coexistence**: Demonstrating how label updates from this component can coexist with labels managed by other controllers. +- **Label Coexistence**: Demonstrating how label updates from this component can coexist with labels managed by other + controllers. - **Data Extraction**: Reading the applied policy configuration after each reconcile cycle. ## Directory Structure From b2eccef7702f2352b210073bb3de46d6447c455f 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:12:30 +0000 Subject: [PATCH 25/27] fix: guard against nil active plan in mutator edit methods Add ensureActive() helper that auto-starts a feature plan when EditObjectMetadata or EditNetworkPolicySpec is called without a prior BeginFeature() call, preventing a nil-pointer dereference. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index c91b7c13..dbd02f69 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -62,6 +62,7 @@ func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) if edit == nil { return } + m.ensureActive() m.active.metadataEdits = append(m.active.metadataEdits, edit) } @@ -78,9 +79,17 @@ func (m *Mutator) EditNetworkPolicySpec(edit func(*editors.NetworkPolicySpecEdit if edit == nil { return } + m.ensureActive() m.active.specEdits = append(m.active.specEdits, edit) } +// ensureActive starts a feature plan if one has not been started yet. +func (m *Mutator) ensureActive() { + if m.active == nil { + m.BeginFeature() + } +} + // Apply executes all recorded mutation intents on the underlying NetworkPolicy. // // Execution order across all registered features: From 305bac3a98f2e6dd3658d6764dafcd55b5a1e3c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86gir=20M=C3=A1ni=20Hauksson?= <54936225+sourcehawk@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:26:10 +0000 Subject: [PATCH 26/27] docs: update NewMutator doc to reflect implicit feature plan behavior The doc comment incorrectly stated that BeginFeature must be called before registering mutations, but ensureActive() auto-creates a plan. Updated to accurately describe the implicit behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/primitives/networkpolicy/mutator.go b/pkg/primitives/networkpolicy/mutator.go index dbd02f69..53ae2378 100644 --- a/pkg/primitives/networkpolicy/mutator.go +++ b/pkg/primitives/networkpolicy/mutator.go @@ -39,8 +39,8 @@ type Mutator struct { // NewMutator creates a new Mutator for the given NetworkPolicy. // // It is typically used within a Feature's Mutation logic to express desired -// changes to the NetworkPolicy. BeginFeature must be called before registering -// any mutations. +// changes to the NetworkPolicy. If mutations are registered without an explicit +// BeginFeature call, a default feature plan is created automatically. func NewMutator(np *networkingv1.NetworkPolicy) *Mutator { return &Mutator{ np: np, From 777a4293a6d82b018d73d0f01aaeb8ab37adfe58 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:58:18 +0000 Subject: [PATCH 27/27] fix: use value assertion instead of pointer equality in networkpolicy test Compare port IntVal instead of pointer identity to make the test resilient to implementation changes like deep-copying. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/primitives/networkpolicy/mutator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/primitives/networkpolicy/mutator_test.go b/pkg/primitives/networkpolicy/mutator_test.go index 6eb60f54..5a929f51 100644 --- a/pkg/primitives/networkpolicy/mutator_test.go +++ b/pkg/primitives/networkpolicy/mutator_test.go @@ -148,7 +148,7 @@ func TestMutator_EditNetworkPolicySpec_ReplaceIngressAtomically(t *testing.T) { }) require.NoError(t, m.Apply()) require.Len(t, np.Spec.Ingress, 1) - assert.Equal(t, &port8080, np.Spec.Ingress[0].Ports[0].Port) + assert.Equal(t, port8080.IntVal, np.Spec.Ingress[0].Ports[0].Port.IntVal) } func TestMutator_EditNetworkPolicySpec_SetPolicyTypes(t *testing.T) {