hierarchical-namespaces icon indicating copy to clipboard operation
hierarchical-namespaces copied to clipboard

Allow RoleBindings to support AllowPropagate mode

Open gbmeuk opened this issue 2 years ago • 8 comments

Our use-case for this is where other controllers (Config Sync's namespace reconciler RepoSync) are creating role bindings in a namespace, which has no uniqueness in its name (name: configsync.gke.io:ns-reconciler). This causes the binding to be inherited to children and then no RepoSyncs can be created in the descendants. The current approach of adding a propagate.hnc.x-k8s.io/none: "true" annotation also is challenging as we're not adding the role.

Trying to set the AllowPropagate like:

spec:
  resources:
    - resource: rolebindings
      group: rbac.authorization.k8s.io
      mode: AllowPropagate

Returns:

failed to apply HNCConfiguration.hnc.x-k8s.io, /config: admission webhook "hncconfigurations.hnc.x-k8s.io" denied the request: HNCConfiguration.hnc.x-k8s.io "config" is invalid: spec.resources[1]: Invalid value: rolebindings.rbac.authorization.k8s.io: always uses the 'Propagate' mode and cannot be configured

It feels that now that AllowPropagate feature is available that this can be relaxed?

NB. We have actually managed to work around the issue with RepoSync by pre-defining the role binding in a Helm chart in exactly the same way as Config Sync does.

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  annotations:
    propagate.hnc.x-k8s.io/none: "true"
  name: configsync.gke.io:ns-reconciler
  namespace: {{ .Release.namespace" }}
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: configsync.gke.io:ns-reconciler
subjects:
- kind: ServiceAccount
  name:  ns-reconciler-{{ .Release.namespace" }}-root-sync-9
  namespace: 

However, it's a fragile solution because if something changes with Config Sync's approach, it will fail again. Also, Config Sync doesn't seem to care that it already exists but this may not work for other controllers/scenarios.

On a side note, I can partially work around the constraint by dropping group from the resourced object:

spec:
  resources:
    - resource: rolebindings
      mode: AllowPropagate

So I suspect there may be a bit of a bug in the matching algorithm. It only partially works though and causes some other component of the system to revert the change frequently.

{"level":"info","ts":1698313059.863884,"logger":"hncconfig.reconcile","msg":"Changing sync mode of the object reconciler","gvk":"rbac.authorization.k8s.io/v1, Kind=RoleBinding","oldMode":"AllowPropagate","newMode":"Propagate"}
{"level":"info","ts":1698313059.8659315,"logger":"hncconfig.reconcile","msg":"Changing sync mode of the object reconciler","gvk":"rbac.authorization.k8s.io/v1, Kind=RoleBinding","oldMode":"Propagate","newMode":"AllowPropagate"}

There is also this. Albeit, not exactly a problem but does suggest that the matching needs a review. When I set role bindings to Propagate as the value is needs to be, there is still an admission webhook error:

spec:
  resources:
    - resource: rolebindings
      mode: Propagate
admission webhook "hncconfigurations.hnc.x-k8s.io" denied the request: HNCConfiguration.hnc.x-k8s.io "config" is invalid: spec.resources[1]: Invalid value: rolebindings.rbac.authorization.k8s.io: always uses the 'Propagate'

gbmeuk avatar Oct 27 '23 10:10 gbmeuk