Skip to content

fix(rbac): reconcile Role when ObjectStore spec changes#823

Merged
leonardoce merged 6 commits intomainfrom
dev/trigger-reconciliation
Apr 13, 2026
Merged

fix(rbac): reconcile Role when ObjectStore spec changes#823
leonardoce merged 6 commits intomainfrom
dev/trigger-reconciliation

Conversation

@armru
Copy link
Copy Markdown
Member

@armru armru commented Mar 26, 2026 •
edited by mnencia
Loading

When an ObjectStore's credentials change (e.g., secret rename), the
RBAC Role granting the Cluster's ServiceAccount access to those
secrets was not updated because nothing triggered a Cluster
reconciliation.

Implement the ObjectStore controller's Reconcile to detect affected
Roles and update their rules directly, without needing access to
Cluster objects. The controller lists Roles by a label set by the
Pre hook, inspects their rules to find which ObjectStores they
reference, fetches those ObjectStores, and patches the rules to
match the current specs.

Replace the custom setOwnerReference helper with
controllerutil.SetControllerReference. Add dynamic CNPG scheme
registration (internal/scheme) to the operator, instance, and
restore managers.

@armru armru requested a review from a team as a code owner March 26, 2026 15:15
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. go Pull requests that update go code labels Mar 26, 2026
@armru armru force-pushed the dev/trigger-reconciliation branch 2 times, most recently from 064864b to bd2b992 Compare March 27, 2026 11:31
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Mar 27, 2026
@armru armru force-pushed the dev/trigger-reconciliation branch 2 times, most recently from 8fc4251 to 840fbc8 Compare March 27, 2026 15:46
@armru armru added this to the v0.12.0 milestone Apr 8, 2026
@mnencia mnencia force-pushed the dev/trigger-reconciliation branch from 840fbc8 to 3593c18 Compare April 9, 2026 12:43
@mnencia
Copy link
Copy Markdown
Member

mnencia commented Apr 9, 2026 •
edited
Loading

I added a second commit that changes the ObjectStore controller to discover affected Roles directly instead of listing Clusters. The plugin should not need get/list/watch on Clusters.

The controller now lists Roles by a label (barmancloud.cnpg.io/cluster) set by the Pre hook, inspects their rules to find which ObjectStores they reference, then fetches those ObjectStores and rebuilds the rules. Conflict handling uses retry.RetryOnConflict to match the existing project pattern, and partial failures across Roles are aggregated with errors.Join instead of failing on the first one.

Pre-existing Roles without the label won't be found by the ObjectStore controller until the Pre hook adds it on the next Cluster reconciliation. Same staleness window as the current main branch.

PR description edited accordingly.

@mnencia mnencia force-pushed the dev/trigger-reconciliation branch 5 times, most recently from 1d638f9 to 134b45f Compare April 10, 2026 15:18
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 10, 2026
@mnencia mnencia mentioned this pull request Apr 10, 2026
Closed
@mnencia mnencia force-pushed the dev/trigger-reconciliation branch 4 times, most recently from 7b703aa to 6ecbd9c Compare April 13, 2026 09:31
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Apr 13, 2026
armru and others added 3 commits April 13, 2026 13:22
@armru @leonardoce
fix(rbac): reconcile Role when ObjectStore spec changes
fe445b1 
When an ObjectStore's credentials change (e.g., secret rename), the
RBAC Role granting the Cluster's ServiceAccount access to those secrets
was not updated because nothing triggered a Cluster reconciliation.

Implement the ObjectStore controller's Reconcile to detect referencing
Clusters and update their Roles directly. Extract ensureRole into a
shared rbac.EnsureRole function used by both the Pre hook and the
ObjectStore controller.

Handle concurrent modifications between the Pre hook and ObjectStore
controller gracefully: AlreadyExists on Create and Conflict on Patch
are retried once to avoid propagating transient errors as gRPC failures
to CNPG.

Replace the custom setOwnerReference helper (ownership.go) with
controllerutil.SetControllerReference for both Role and RoleBinding.
The old helper read the GVK from the object's metadata and replaced
all owner references unconditionally. The new function reads the GVK
from the scheme and appends to existing owner references, refusing to
overwrite if another controller already owns the object. Both produce
identical results for our use case since the Role is always freshly
built. The GVK is now resolved from the scheme configured via
CUSTOM_CNPG_GROUP/CUSTOM_CNPG_VERSION, which must match the actual
CNPG API group (same requirement as the instance sidecar).

Add dynamic CNPG scheme registration (internal/scheme) to the operator,
instance, and restore managers, replacing hardcoded cnpgv1.AddToScheme
calls. Add RBAC permission for the plugin to list/watch Clusters.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@mnencia @leonardoce
fix: discover affected Roles by label instead of listing Clusters
c7ddf03 
The ObjectStore controller now lists Roles by a label
(barmancloud.cnpg.io/cluster) set by the Pre hook, inspects their
rules to find which ObjectStores they reference, then fetches those
ObjectStores and rebuilds the rules. This removes the clusters
get/list/watch permission. Conflict handling uses RetryOnConflict to
match the existing project pattern, and partial failures across Roles
are aggregated with errors.Join instead of failing on the first one.

Pre-existing Roles without the label won't be found by the ObjectStore
controller until the Pre hook adds it on the next Cluster
reconciliation. Same staleness window as the current main branch.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia @leonardoce
fix: read owner GVK from object metadata instead of scheme
4dfd20b 
The operator does not know the CNPG API group at runtime (it is
not a sidecar injected by the CNPG operator, so CUSTOM_CNPG_GROUP
and CUSTOM_CNPG_VERSION are not available). Move SetControllerReference
to the specs package and read the GVK from the decoded Cluster's
TypeMeta rather than looking it up in the scheme.

Remove CNPG types from the operator's scheme and the env var bindings
from cmd/operator since they are no longer needed.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
mnencia and others added 3 commits April 13, 2026 13:22
@mnencia @leonardoce
test: add e2e test for credential rotation
4503532 
Add an e2e test that verifies the ObjectStore controller updates the
RBAC Role when an ObjectStore's secret reference changes. The test
creates a Cluster with a MinIO ObjectStore, verifies the Role has the
cluster label and references the original secret, then updates the
ObjectStore to point to a new secret and waits for the Role to be
patched accordingly.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@gabriele-wolfox @mnencia @leonardoce
chore: add unit tests, improve log and code readability (#839)
61350c7 
- Add 'namespace' structured field to the error log in Reconcile when a
role reconciliation fails
- Rename misleading local variable 'role' to 'roleBinding' in
ensureRoleBinding to match the actual type
- Add EnsureRole tests: transient Role creation error is propagated;
pre-existing unrelated labels are preserved after patch
- Add SetControllerReference test: returns an error when the owner does
not implement runtime.Object
- Add ObjectStoreReconciler tests: Role list failure and ObjectStore Get
transient error both surface through the reconcile return value
- Add scheme tests: AddCNPGToScheme with default and custom
group/version

Assisted-by: Claude Opus 4.6

Signed-off-by: Gabriele Quaresima <gabriele.quaresima@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@mnencia @leonardoce
chore: add review comments and clean up whitespace
aae694f 
Document the upgrade gap for pre-existing Roles without the
ClusterLabelName label, the single-owner assumption in
SetControllerReference, and the race window between the Pre hook
and the ObjectStore controller.

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
@leonardoce leonardoce force-pushed the dev/trigger-reconciliation branch from 843012f to aae694f Compare April 13, 2026 11:22
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Apr 13, 2026
@leonardoce leonardoce self-requested a review April 13, 2026 11:31
@leonardoce leonardoce merged commit 8971a39 into main Apr 13, 2026
8 checks passed
@leonardoce leonardoce deleted the dev/trigger-reconciliation branch April 13, 2026 13:48
@cnpg-bot cnpg-bot mentioned this pull request Apr 13, 2026
Merged
mnencia pushed a commit that referenced this pull request Apr 14, 2026
@cnpg-bot
chore(main): release 0.12.0 (#756)
e694dd8 
🤖 I have created a release *beep* *boop*
---


##
[0.12.0](v0.11.0...v0.12.0)
(2026-04-13)


### Features

* **deps:** Update dependency barman to v3.18.0
([#813](#813))
([a8b446f](a8b446f))


### Bug Fixes

* **deps:** Update all non-major go dependencies
([#751](#751))
([5001fe7](5001fe7))
* **deps:** Update all non-major go dependencies
([#757](#757))
([d031c23](d031c23))
* **deps:** Update all non-major go dependencies
([#801](#801))
([6ae101f](6ae101f))
* **deps:** Update dependency @easyops-cn/docusaurus-search-local to
^0.55.0
([#753](#753))
([60d32cc](60d32cc))
* **deps:** Update documentation dependencies
([#833](#833))
([e1d4a6e](e1d4a6e))
* **deps:** Update k8s.io/utils digest to 28399d8
([#829](#829))
([3549e26](3549e26))
* **deps:** Update k8s.io/utils digest to b8788ab
([#784](#784))
([f64ff8e](f64ff8e))
* **deps:** Update kubernetes packages to v0.35.2
([#788](#788))
([a7e28f6](a7e28f6))
* **deps:** Update module github.com/cert-manager/cert-manager to
v1.19.3 [security]
([#775](#775))
([79238f5](79238f5))
* **deps:** Update module github.com/cert-manager/cert-manager to
v1.20.2
([#844](#844))
([441f43b](441f43b))
* **deps:** Update module github.com/cloudnative-pg/api to v1.29.0
([#837](#837))
([09181b0](09181b0))
* **deps:** Update module github.com/cloudnative-pg/machinery to v0.4.0
([#850](#850))
([18e3888](18e3888))
* **deps:** Update module google.golang.org/grpc to v1.79.3 [security]
([#819](#819))
([376e178](376e178))
* **deps:** Update module sigs.k8s.io/controller-runtime to v0.23.3
([#789](#789))
([3f726ea](3f726ea))
* **deps:** Update module sigs.k8s.io/kustomize/api to v0.21.1
([#790](#790))
([84a388e](84a388e))
* **metrics:** Announce sidecar injection capability
([#776](#776))
([4a94cb9](4a94cb9))
* **rbac:** Reconcile Role when ObjectStore spec changes
([#823](#823))
([8971a39](8971a39))
* **restore:** Race condition in parallel WAL restore spool
([#812](#812))
([25d72ce](25d72ce))
* **restore:** Use custom CNPG group and version for scheme registration
([#847](#847))
([b1f373d](b1f373d))
* **security:** Harden GitHub Actions workflows against expression
injection
([#773](#773))
([ce7b761](ce7b761))
* Skip maintenance cycle when plugin is not enabled for backups
([#826](#826))
([63a67cb](63a67cb)),
closes
[#774](#774)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: Peggie <info@cloudnative-pg.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mnencia mnencia mnencia approved these changes

@leonardoce leonardoce leonardoce approved these changes

+1 more reviewer

@gabriele-wolfox gabriele-wolfox gabriele-wolfox approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

go Pull requests that update go code lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

v0.12.0

Development

Successfully merging this pull request may close these issues.

4 participants

@armru @mnencia @leonardoce @gabriele-wolfox