feat: implement serviceaccount primitive by sourcehawk · Pull Request #33 · sourcehawk/operator-component-framework

sourcehawk · 2026-03-22T03:11:54Z

Implements the serviceaccount Kubernetes resource primitive following the pattern established by the existing ConfigMap and Deployment primitives.

Summary

Adds serviceaccount primitive package under pkg/primitives/serviceaccount/
Implements required lifecycle interfaces
Includes editors, mutator, flavors, and builder
Adds serviceaccount entry to the built-in primitives table in docs/primitives.md

Checklist

Compiles cleanly
Tests pass
Follows naming conventions in CONTEXT.md
Shared file modifications limited to documentation index (docs/primitives.md)

Copilot

Pull request overview

Adds a new serviceaccount primitive to the Operator Component Framework, mirroring the existing configmap/deployment patterns for managing Kubernetes ServiceAccount resources via a static desired-state resource, builder configuration, mutation planning, and field-application flavors.

Changes:

Introduces pkg/primitives/serviceaccount/ with Resource, Builder, Mutator, and field-application flavors for ServiceAccount.
Adds unit tests for the new builder, mutator behavior (including ordering), and flavors.
Adds a runnable example (examples/serviceaccount-primitive/) plus dedicated primitive documentation (docs/primitives/serviceaccount.md).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/primitives/serviceaccount/resource.go	Implements the static `ServiceAccount` primitive `Resource` wrapper over `internal/generic.StaticResource`.
pkg/primitives/serviceaccount/builder.go	Provides fluent builder API for configuring mutations, flavors, and data extractors for `ServiceAccount`.
pkg/primitives/serviceaccount/mutator.go	Adds plan-and-apply `Mutator` with ordered edit categories for metadata, image pull secrets, and automount token.
pkg/primitives/serviceaccount/flavors.go	Adds typed flavors to preserve current labels/annotations via shared `pkg/flavors` helpers.
pkg/primitives/serviceaccount/builder_test.go	Tests builder validation and builder configuration methods.
pkg/primitives/serviceaccount/mutator_test.go	Tests mutator operations, idempotency, ordering, and feature-boundary behavior.
pkg/primitives/serviceaccount/flavors_test.go	Tests label/annotation preservation and flavor ordering/error propagation through `Mutate`.
examples/serviceaccount-primitive/app/controller.go	Example controller wiring a component with the serviceaccount resource.
examples/serviceaccount-primitive/features/mutations.go	Example feature-gated mutations composing SA behavior.
examples/serviceaccount-primitive/resources/serviceaccount.go	Example resource factory building the SA primitive with mutations/flavors/extraction.
examples/serviceaccount-primitive/main.go	Runnable fake-client demo for multiple reconciliation cycles.
examples/serviceaccount-primitive/README.md	Documentation for running/understanding the new example.
docs/primitives/serviceaccount.md	New primitive documentation and usage guidance.

examples/serviceaccount-primitive/resources/serviceaccount.go

docs/primitives/serviceaccount.md

examples/serviceaccount-primitive/main.go

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

sourcehawk · 2026-03-22T21:51:02Z

approved

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

pkg/primitives/serviceaccount/mutator_test.go

docs/primitives/serviceaccount.md

pkg/primitives/serviceaccount/mutator.go

sourcehawk · 2026-03-23T02:57:46Z

Claude Review Cycle 1 Complete

Addressed:
None — all three comments were evaluated and determined to not require changes.

Intentionally ignored:

mutator_test.go:183 (tests use unexported beginFeature() directly): This is the established codebase convention. Both configmap/mutator_test.go and deployment/mutator_test.go use beginFeature() directly in the same way. Changing only serviceaccount tests would introduce inconsistency.
docs/primitives/serviceaccount.md:12 (double pipes in table): The table uses standard single-pipe markdown format, identical to docs/primitives/configmap.md. No double pipes are present.
mutator.go:48 (unexported beginFeature prevents FeatureMutator satisfaction): Technically correct — Go interfaces with unexported methods can only be satisfied by types in the same package. However, this equally affects configmap.Mutator and deployment.Mutator, which follow the exact same pattern. This is a framework-level design concern, not a serviceaccount-specific issue. Simplifying only the serviceaccount mutator would make it inconsistent with the other primitives. The proper fix (exporting BeginFeature in the framework interface) is out of scope for this PR.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

examples/serviceaccount-primitive/features/mutations.go

pkg/primitives/serviceaccount/mutator.go

examples/serviceaccount-primitive/features/mutations.go

sourcehawk · 2026-03-23T17:00:29Z

Claude Review Cycle 1 Complete

Addressed:

Set Feature: nil for always-enabled mutations (VersionLabelMutation, ImagePullSecretMutation) in examples, matching documented semantics and avoiding unnecessary version parsing (comments 1 & 3)
Exported BeginFeature() on the FeatureMutator interface and all primitive mutators (serviceaccount, configmap, deployment) so external packages can satisfy the interface and participate in per-feature ordering semantics (comment 2). This was a broader fix than suggested — the unexported beginFeature() method meant no external primitive could satisfy the interface, so the type assertion in ApplyMutations would always fail.

Intentionally ignored:
None

Copilot

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

docs/primitives/serviceaccount.md

internal/generic/resource_workload.go

examples/serviceaccount-primitive/main.go

Implement the ServiceAccount static primitive with builder, resource, mutator, and flavors. The mutator exposes direct methods for imagePullSecrets and automountServiceAccountToken since ServiceAccount has no spec field. Apply order: metadata, image pull secrets, automount. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Demonstrates feature-gated image pull secret management, automount token control, version labelling, label preservation flavors, and data extraction across multiple reconciliation cycles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add clarifying comment in resources/serviceaccount.go explaining that EnableMetrics is reused to drive DisableAutomountMutation for demo purposes, and extract to a named variable for readability. - Fix misleading printf labels in main.go: PrivateRegistry/DisableAutomount renamed to EnableTracing/EnableMetrics to match actual spec field names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ctor Align with the fix applied to deployment and configmap mutators — initialize the first feature plan inline instead of calling beginFeature(), which would create an empty extra plan when the mutator helper also calls beginFeature(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Address PR review feedback: - Set Feature to nil for always-enabled mutations in examples instead of using feature.NewResourceFeature(version, nil), matching documented semantics and avoiding unnecessary version parsing. - Export BeginFeature() on the FeatureMutator interface and all primitive mutators (serviceaccount, configmap, deployment) so that external packages can satisfy the interface and participate in per-feature ordering semantics via the generic builder's type assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Rename unused version parameter to _ in ImagePullSecretMutation to fix revive lint error. Update docs to reflect server-managed metadata preservation in DefaultFieldApplicator. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…e to run-examples - Remove "Field Flavors" / PreserveCurrentLabels mention from README (the API was removed in cd27004 but the docs were not fully updated) - Add serviceaccount-primitive to the Makefile run-examples target Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sourcehawk · 2026-03-25T16:03:14Z

Claude Review Cycle 1 Complete

Addressed:

README.md line 9: Removed stale "Field Flavors" / PreserveCurrentLabels bullet from the feature list, since neither the API nor the builder method exists.
Makefile run-examples target: Added go run ./examples/serviceaccount-primitive/. so the new example is exercised alongside the existing ones.

Intentionally ignored:

builder.go line 66 (Builder missing WithFieldApplicationFlavor): The example code no longer calls this method — it was already removed in commit cd27004/2ee1b8e. The comment is stale; no code change needed.
resources/serviceaccount.go line 44 (example won't compile): The example compiles and runs successfully — WithFieldApplicationFlavor is not called in the current code. The comment was based on an outdated view of the file.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Copilot · 2026-03-25T16:11:46Z

pkg/primitives/serviceaccount/mutator.go

+func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) {
+	if edit == nil {
+		return
+	}
+	m.active.metadataEdits = append(m.active.metadataEdits, edit)
+}


These mutation registration methods dereference m.active without ensuring BeginFeature() has been called. If a caller invokes any of these before BeginFeature(), this will panic with a nil pointer dereference. Since this is a public mutator API, consider making it non-panicking by (mandatory) adding a guard that initializes an active plan when m.active == nil (e.g., lazily calling BeginFeature()), or alternatively returning a structured error (would require API changes) rather than crashing at runtime.

Copilot · 2026-03-25T16:11:47Z

pkg/primitives/serviceaccount/mutator.go

+func (m *Mutator) EnsureImagePullSecret(name string) {
+	m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) {
+		for _, ref := range sa.ImagePullSecrets {
+			if ref.Name == name {
+				return
+			}
+		}
+		sa.ImagePullSecrets = append(sa.ImagePullSecrets, corev1.LocalObjectReference{Name: name})
+	})
+}


These mutation registration methods dereference m.active without ensuring BeginFeature() has been called. If a caller invokes any of these before BeginFeature(), this will panic with a nil pointer dereference. Since this is a public mutator API, consider making it non-panicking by (mandatory) adding a guard that initializes an active plan when m.active == nil (e.g., lazily calling BeginFeature()), or alternatively returning a structured error (would require API changes) rather than crashing at runtime.

Copilot · 2026-03-25T16:11:47Z

pkg/primitives/serviceaccount/mutator.go

+func (m *Mutator) RemoveImagePullSecret(name string) {
+	m.active.imagePullSecretEdits = append(m.active.imagePullSecretEdits, func(sa *corev1.ServiceAccount) {
+		filtered := sa.ImagePullSecrets[:0]
+		for _, ref := range sa.ImagePullSecrets {
+			if ref.Name != name {
+				filtered = append(filtered, ref)
+			}
+		}
+		sa.ImagePullSecrets = filtered
+	})
+}


These mutation registration methods dereference m.active without ensuring BeginFeature() has been called. If a caller invokes any of these before BeginFeature(), this will panic with a nil pointer dereference. Since this is a public mutator API, consider making it non-panicking by (mandatory) adding a guard that initializes an active plan when m.active == nil (e.g., lazily calling BeginFeature()), or alternatively returning a structured error (would require API changes) rather than crashing at runtime.

Copilot · 2026-03-25T16:11:47Z

pkg/primitives/serviceaccount/mutator.go

+func (m *Mutator) SetAutomountServiceAccountToken(v *bool) {
+	var snapshot *bool
+	if v != nil {
+		val := *v
+		snapshot = &val
+	}
+
+	m.active.automountEdits = append(m.active.automountEdits, func(sa *corev1.ServiceAccount) {
+		sa.AutomountServiceAccountToken = snapshot
+	})
+}


These mutation registration methods dereference m.active without ensuring BeginFeature() has been called. If a caller invokes any of these before BeginFeature(), this will panic with a nil pointer dereference. Since this is a public mutator API, consider making it non-panicking by (mandatory) adding a guard that initializes an active plan when m.active == nil (e.g., lazily calling BeginFeature()), or alternatively returning a structured error (would require API changes) rather than crashing at runtime.

sourcehawk · 2026-03-25T16:14:24Z

Claude Review Cycle 1 Complete

Addressed:
None

Intentionally ignored:

mutator.go lines 63, 76, 90, 107 — nil guard for m.active before BeginFeature() is called: This is a valid defensive programming concern in isolation, but the same pattern (no nil guard on m.active) is used consistently across all other primitive mutators in the codebase (configmap, deployment). The precondition is already documented on NewMutator: "BeginFeature must be called before registering any mutations." Adding a lazy-init guard only to the serviceaccount mutator would be inconsistent with the established codebase convention. If this is to be addressed, it should be done uniformly across all primitives in a separate change.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

pkg/primitives/serviceaccount/mutator.go

pkg/primitives/serviceaccount/builder_test.go

Prevent nil pointer panic when mutator methods are called without a prior BeginFeature() call. Each public mutation method now calls ensureActive() which panics with a clear message instead of a cryptic nil dereference. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sourcehawk · 2026-03-25T16:25:32Z

Claude Review Cycle 1 Complete

Addressed:

mutator.go: Added ensureActive() guard to all public mutator methods (EditObjectMetadata, EnsureImagePullSecret, RemoveImagePullSecret, SetAutomountServiceAccountToken). If BeginFeature() has not been called, these methods now panic with a clear error message instead of a nil pointer dereference.

Intentionally ignored:

builder_test.go loop variable capture (tt := tt): This project uses Go 1.25.6, which scopes loop variables per iteration (changed in Go 1.22). The tt := tt rebind is unnecessary and no other test in the codebase uses this pattern.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

pkg/primitives/serviceaccount/mutator.go

pkg/primitives/serviceaccount/resource_test.go

…uard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sourcehawk · 2026-03-25T17:01:26Z

Claude Review Cycle 1 Complete

Addressed:

mutator.go:58 — Replaced panic in ensureActive() with a lazy BeginFeature() call so that misconfiguration results in controlled behavior (auto-initializing a default plan) rather than crashing the controller process.

Intentionally ignored:

resource_test.go:74 — The suggestion to construct a divergent current ServiceAccount and pass it to Mutate() is based on a misunderstanding of the architecture. Mutate() applies only feature mutations to the current object; it does not overwrite base state fields (labels, ImagePullSecrets, etc.). This is consistent with how all other primitives (configmap, deployment) work and are tested. The existing test correctly uses res.Object() (which returns the desired state) as the starting point for mutation, matching the framework's reconciliation pattern.

sourcehawk requested a review from Copilot March 22, 2026 03:42

Copilot started reviewing on behalf of sourcehawk March 22, 2026 03:43 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

examples/serviceaccount-primitive/resources/serviceaccount.go Outdated Show resolved Hide resolved

docs/primitives/serviceaccount.md Outdated Show resolved Hide resolved

examples/serviceaccount-primitive/main.go Outdated Show resolved Hide resolved

sourcehawk requested a review from Copilot March 22, 2026 13:19

Copilot started reviewing on behalf of sourcehawk March 22, 2026 13:20 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

sourcehawk force-pushed the feature/serviceaccount-primitive branch from 2f416f3 to 7078efd Compare March 23, 2026 00:50

Copilot AI review requested due to automatic review settings March 23, 2026 00:50

Copilot started reviewing on behalf of sourcehawk March 23, 2026 00:50 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

pkg/primitives/serviceaccount/mutator_test.go Show resolved Hide resolved

docs/primitives/serviceaccount.md Outdated Show resolved Hide resolved

pkg/primitives/serviceaccount/mutator.go Show resolved Hide resolved

Copilot AI review requested due to automatic review settings March 23, 2026 16:08

Copilot started reviewing on behalf of sourcehawk March 23, 2026 16:08 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

examples/serviceaccount-primitive/features/mutations.go Outdated Show resolved Hide resolved

pkg/primitives/serviceaccount/mutator.go Show resolved Hide resolved

examples/serviceaccount-primitive/features/mutations.go Show resolved Hide resolved

Copilot AI review requested due to automatic review settings March 23, 2026 17:00

Copilot started reviewing on behalf of sourcehawk March 23, 2026 17:00 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

docs/primitives/serviceaccount.md Show resolved Hide resolved

internal/generic/resource_workload.go Show resolved Hide resolved

internal/generic/resource_workload.go Show resolved Hide resolved

examples/serviceaccount-primitive/main.go Show resolved Hide resolved

sourcehawk and others added 9 commits March 23, 2026 20:16

Add serviceaccount primitive documentation

36fa790

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fix gofmt formatting in serviceaccount example

64d398f

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Preserve server-managed metadata in default field applicator

32098f4

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

sourcehawk force-pushed the feature/serviceaccount-primitive branch from 443df47 to 9328bb2 Compare March 23, 2026 21:37

sourcehawk requested a review from Copilot March 23, 2026 21:51

Copilot AI review requested due to automatic review settings March 25, 2026 16:03

Copilot started reviewing on behalf of sourcehawk March 25, 2026 16:03 View session

sourcehawk requested review from Copilot and removed request for Copilot March 25, 2026 16:04

Copilot started reviewing on behalf of sourcehawk March 25, 2026 16:08 View session

sourcehawk requested review from Copilot and removed request for Copilot March 25, 2026 16:09

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Copilot started reviewing on behalf of sourcehawk March 25, 2026 16:14 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/primitives/serviceaccount/mutator.go Show resolved Hide resolved

pkg/primitives/serviceaccount/builder_test.go Show resolved Hide resolved

Copilot AI reviewed Mar 25, 2026

View reviewed changes

pkg/primitives/serviceaccount/mutator.go Show resolved Hide resolved

pkg/primitives/serviceaccount/resource_test.go Show resolved Hide resolved

replace panic with lazy BeginFeature in serviceaccount ensureActive g…

87eceaa

…uard Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Conversation

sourcehawk commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

sourcehawk commented Mar 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sourcehawk commented Mar 23, 2026

Claude Review Cycle 1 Complete

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sourcehawk commented Mar 23, 2026

Claude Review Cycle 1 Complete

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sourcehawk commented Mar 25, 2026

Claude Review Cycle 1 Complete

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sourcehawk commented Mar 25, 2026

Claude Review Cycle 1 Complete

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

sourcehawk commented Mar 25, 2026

Claude Review Cycle 1 Complete

Uh oh!

Copilot AI left a comment

sourcehawk commented Mar 22, 2026 •

edited

Loading