Skip to content

feat: implement rolebinding primitive#35

Merged
sourcehawk merged 26 commits intomainfrom
feature/rolebinding-primitive
Mar 25, 2026
Merged

feat: implement rolebinding primitive#35
sourcehawk merged 26 commits intomainfrom
feature/rolebinding-primitive

Conversation

@sourcehawk
Copy link
Owner

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

Summary

  • Adds rolebinding primitive package under pkg/primitives/rolebinding/
  • Implements required lifecycle interfaces
  • Includes editors, mutator, flavors, and builder

Checklist

  • Compiles cleanly
  • Tests pass
  • Follows naming conventions in CONTEXT.md
  • Does not modify shared files

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Kubernetes RoleBinding primitive to the Operator Component Framework, following the existing static primitive patterns (e.g., ConfigMap) and providing an example + documentation for consumers.

Changes:

  • Introduces pkg/primitives/rolebinding/ with builder/resource/mutator, field-application flavors, and tests.
  • Adds a new shared mutation editor (BindingSubjectsEditor) for RoleBinding/ClusterRoleBinding subjects management, with tests.
  • Adds a runnable example and a dedicated primitive doc page (docs/primitives/rolebinding.md).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pkg/primitives/rolebinding/resource.go Static RoleBinding resource wrapper + default field applicator (roleRef preservation intent)
pkg/primitives/rolebinding/mutator.go RoleBinding mutator with planned edits (metadata + subjects)
pkg/primitives/rolebinding/mutator_test.go Unit tests for RoleBinding mutator behavior and ordering
pkg/primitives/rolebinding/flavors.go RoleBinding field-application flavors (preserve labels/annotations)
pkg/primitives/rolebinding/flavors_test.go Unit/integration-style tests for flavors and builder wiring
pkg/primitives/rolebinding/builder.go Fluent builder API for RoleBinding primitive configuration
pkg/primitives/rolebinding/builder_test.go Builder validation and configuration tests
pkg/mutation/editors/bindingsubjects.go Shared editor for mutating binding subjects lists
pkg/mutation/editors/bindingsubjects_test.go Unit tests for BindingSubjectsEditor
examples/rolebinding-primitive/resources/rolebinding.go Example resource factory assembling a RoleBinding with mutations/flavors/extractors
examples/rolebinding-primitive/README.md Example documentation and run instructions
examples/rolebinding-primitive/main.go Runnable example using a fake client to demonstrate reconciliation cycles
examples/rolebinding-primitive/features/mutations.go Example feature mutations using RoleBinding mutator APIs
examples/rolebinding-primitive/app/controller.go Example controller wiring the component + RoleBinding resource factory
docs/primitives/rolebinding.md Primitive documentation (capabilities, usage, ordering, editors, flavors)

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • Added RoleRef validation in Build() — validates that APIGroup, Kind, and Name are non-empty, catching invalid RoleBindings at build time rather than at apply time. Added test cases for empty and partial RoleRef. Updated existing builder tests to include a valid RoleRef.
  • Added nil pointer guard in NewBindingSubjectsEditor — allocates an empty slice when a nil pointer is passed, consistent with the NewConfigMapDataEditor pattern. Added test for nil pointer constructor case.

Intentionally ignored:
None

<!-- claude-review-cycle -->

Copilot AI review requested due to automatic review settings March 22, 2026 19:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@sourcehawk
Copy link
Owner Author

approved

Copilot AI review requested due to automatic review settings March 23, 2026 00:49
@sourcehawk sourcehawk force-pushed the feature/rolebinding-primitive branch from 9dc626b to 89bce3f Compare March 23, 2026 00:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • pkg/mutation/editors/bindingsubjects.go:52 — Zero trailing slice elements in RemoveSubject: This fix was already applied in a prior commit (ec93e9f). Resolved merge conflict to preserve the fix when merging with remote branch.

Intentionally ignored:

  • None

<!-- claude-review-cycle -->

Copilot AI review requested due to automatic review settings March 23, 2026 16:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings March 23, 2026 16:58
@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:
None — no code changes were required.

Intentionally ignored:

  • bindingsubjects.go shared editor location (pkg/mutation/editors/bindingsubjects.go, line 7): Copilot flagged that the PR checklist says "Does not modify shared files" but a new editor was added to pkg/mutation/editors/. This is the established architecture pattern — every primitive (configmap, deployment, rolebinding) adds its domain-specific editor to this shared package. The file is a new addition, not a modification to existing shared code. The PR description checklist item should be updated to clarify that new shared editors are being added as part of the primitive (consistent with how configmapdata.go, deploymentspec.go, etc. were added for their respective primitives).

<!-- claude-review-cycle -->

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

sourcehawk and others added 4 commits March 23, 2026 20:17
Implements a typed editor for mutating the subjects list of RBAC bindings,
with EnsureSubject (upsert by Kind+Name+Namespace), RemoveSubject, and Raw
access. Shared by both RoleBinding and ClusterRoleBinding primitives.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements builder, resource, mutator, and flavors for the RoleBinding
primitive. DefaultFieldApplicator preserves the immutable roleRef from
the live object. Mutator supports EditObjectMetadata and EditSubjects
with plan-and-apply pattern.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers building, default field application (immutable roleRef), mutations,
editors (BindingSubjectsEditor, ObjectMetaEditor), flavors, and guidance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates building a RoleBinding with immutable roleRef, feature-gated
subject mutations, version labels, field flavors, and data extraction
across multiple reconciliation cycles.

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

Claude Review Cycle 1 Complete

Addressed:

  • resource.go:59 — Clarified Mutate() docstring to specify that roleRef preservation is guaranteed by DefaultFieldApplicator, and that custom field applicators must also preserve the immutable roleRef to avoid update failures.

Intentionally ignored:
None

<!-- claude-review-cycle -->

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 24, 2026 18:28
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 2 Complete

Addressed:

  • resource.go roleRef preservation comment: Already fixed in commit 6d776bf. The docstring now clarifies that preserving roleRef is the responsibility of the applicator, that DefaultFieldApplicator does this automatically, and that custom applicators must do the same.
  • docs/primitives.md table formatting: Already fixed in commit 858062a. The BindingSubjectsEditor row now has consistent spacing matching the rest of the table.

Intentionally ignored:

  • Makefile / PR description checklist: Copilot noted the PR description says "Does not modify shared files" but the PR modifies the Makefile and docs. This is a PR description accuracy issue, not a code issue. The Makefile and docs changes are intentional and necessary for the rolebinding primitive. The PR description checklist should be updated by the author.

<!-- claude-review-cycle -->

@sourcehawk sourcehawk requested a review from Copilot March 24, 2026 18:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:
None — all comments were already resolved.

Intentionally ignored:

  • docs/primitives.md line 134 (table formatting): The markdown table row for BindingSubjectsEditor already has correct spacing after | and is properly aligned with surrounding rows. This was fixed in commit 858062a ("Fix markdown table alignment in Mutation Editors section"). The Copilot comment appears to be based on a prior revision.

@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 24, 2026 18:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

sourcehawk and others added 2 commits March 25, 2026 14:19
The framework now uses SSA instead of ctrl.CreateOrUpdate, eliminating the
need to merge desired state onto current state. This removes
DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor,
FieldApplicationFlavor, the pkg/flavors package, and all flavors.go files
across the generic internals and all primitives (deployment, configmap,
rolebinding). Tests are updated to pass Object() output to Mutate() instead
of empty structs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Resolved all conflicts using main's version. Removed field_applicator
files deleted in main.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 25, 2026 16:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

…semantics

Correct the deployment suspend example comment to clarify that the desired
object is rebuilt from scratch each reconcile, so annotations are not
automatically preserved from the live cluster. Update rolebinding docs to
state that roleRef must be set on the base object and cannot be changed
(requires delete/recreate), removing the incorrect "preserved from live
cluster" implication.

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

Claude Review Cycle 1 Complete

Addressed:

  • Fixed misleading comment in examples/deployment-primitive/features/status.go:58-60 — clarified that the desired object is rebuilt from scratch each reconcile, so annotations are not automatically preserved from the live cluster via SSA. Added guidance to store the timestamp in owner status or read from the live object.
  • Fixed incorrect capability description in docs/primitives/rolebinding.md:13 — replaced "preserved from the live cluster object after initial creation" with "must be set on the base object and cannot be changed after creation (requires delete/recreate)".

Intentionally ignored:

  • Makefile comment (line 126): This is a meta-comment about the PR description checklist, not a code issue. The Makefile change (adding go run ./examples/rolebinding-primitive/.) is a necessary part of the rolebinding primitive addition, not an unrelated cross-cutting change.

<!-- claude-review-cycle -->

@sourcehawk sourcehawk merged commit 19a07ba into main Mar 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants