fix(rbac): reconcile Role when ObjectStore spec changes#823
Open
fix(rbac): reconcile Role when ObjectStore spec changes#823
Conversation
dosubot
bot
added
size:XL
armru
force-pushed
the
dev/trigger-reconciliation
branch
2 times, most recently
from
064864b to
bd2b992
Compare
dosubot
bot
added
size:XXL
armru
force-pushed
the
dev/trigger-reconciliation
branch
2 times, most recently
from
8fc4251 to
840fbc8
Compare
mnencia
force-pushed
the
dev/trigger-reconciliation
branch
from
840fbc8 to
3593c18
Compare
Member
|
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 ( 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. |
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
force-pushed
the
dev/trigger-reconciliation
branch
from
ad4f3d2 to
fe84b51
Compare
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
force-pushed
the
dev/trigger-reconciliation
branch
from
fe84b51 to
b761925
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.