Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: steveb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
wondering what's the architecture/need is to use a dedicated namespace for the console deployment. iiuc from checking the related pr, its gonna be one per ctlplane deployment namespace. so far we kept all deployments for a ctlplane of an env in a single namespace. |
|
/retest |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
(apologies for delay in reply @stuggi this work was put on hold until 19 development started up) The issue is that the ironic-conductor service will be managing the full lifecycle of the pods in this namespace, which requires rbac rules for full management of pods. If we did this in the openstack namespace that adds back the rbac vulnerability that has only just been removed as part of the glasswing work. By running these pods in a separate namespace, a compromised ironic-conductor won't be able to modify pods running in the ctlplane openstack namespace. |
How would that work on an env where you run multiple controlplanes in a different namespace? Do those share this single openstack-ironic-consoles namespace? |
|
@steveb hey, thanks for the pointer to the openstack-operator reference. I took a look and I think that is a remnant of an old 'initialization resource' requirement which I'm not sure we actually ever needed. I submitted a PR to try to clean that up: openstack-k8s-operators/openstack-operator#2163 |
|
@steveb couldn't we just have a model where the administrator pre-creates the namespace like we do for openstack-operator proper? And then pass that to ironic-operator. We could even rely on a convention so it doesn't need to be specified if the user follows the defaults. But with namespace isolation (when the administrator installs multiple openstack deployments on the same OCP cluster) we'd likely have to specify the alternate namespace for ironic. |
|
I think we could make it configurable, but we'll need to add an overriding template for the configuration to then be generated by the operator. On a plus side, everything is in theory disposable. Sharing a singular namespace which is configured then is a risk if not explicitly delineated then either, but the collision point would really be if someone did their own uuid management and shared UUIDs across deployments. |
|
Build succeeded (check pipeline). ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 57m 20s |
|
As currently proposed [1] the namespace is built by prefixing with the control plane namespace, so this will work fine in a multi openstack deployment OCP. I'm proposing this because I'd prefer to avoid adding manual steps for a relatively esoteric feature, and an operator having the ability to create (but not delete) namespaces is not a risk for exploitation. [1] 3bd5d8f#diff-d9cd0c71a3f8ece60bda3ad98ae72c6080850aabbbd4c0a0a21b2e3f03b9fe5bR206 |
The graphical consoles feature will create the namespace openstack-ironic-consoles which Ironic will use to create the graphical console pods. To do this, ironic-operator needs rbac rules to create (not delete) namespaces. This is proposed as a standalone change because it needs to be packaged in the openstack-operator bundle before ironic-operator can use it. Jira: OSPRH-20211
The ironic-conductor controller creates namespace-scoped Roles that grant pods management permissions (create, watch, update, patch, delete) to the ironic service account. This is needed for graphical console support, where the conductor manages console pods in a separate namespace. Kubernetes RBAC escalation prevention requires that the operator's own ClusterRole holds all permissions it grants in any Role it creates. The current ClusterRole only grants get and list on pods, causing Role creation to be rejected with: roles.rbac.authorization.k8s.io is forbidden: user is attempting to grant RBAC permissions not currently held Expand the pods verbs in the kubebuilder RBAC marker from get;list to the full set needed by the created Roles.
|
I've added a commit to this PR which lets ironic-conductor manage pods in this namespace. Here is the commit message for context: The ironic-conductor controller creates namespace-scoped Roles that Kubernetes RBAC escalation prevention requires that the operator's roles.rbac.authorization.k8s.io is forbidden: user is attempting Expand the pods verbs in the kubebuilder RBAC marker from get;list |
|
Build succeeded (check pipeline). ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 55m 33s |
4 participants
Describe your changes
The graphical consoles feature will create the namespace openstack-ironic-consoles which Ironic will use to create the graphical console pods.
To do this, ironic-operator needs rbac rules to create and manage (not delete) namespaces. This is proposed as a standalone change because it needs to be packaged in the openstack-operator bundle before ironic-operator can use it.
For precedence of this change, openstack-operator has the ability to manage every aspect of namespaces: https://github.com/openstack-k8s-operators/openstack-operator/blob/main/config/rbac/role.yaml#L27-L32
Also, the ironic-conductor controller creates namespace-scoped Roles that
grant pods management permissions (create, watch, update, patch,
delete) to the ironic service account. This is needed for graphical
console support, where the conductor manages console pods in a
separate namespace.
Kubernetes RBAC escalation prevention requires that the operator's
own ClusterRole holds all permissions it grants in any Role it
creates. The current ClusterRole only grants get and list on pods,
causing Role creation to be rejected with:
roles.rbac.authorization.k8s.io is forbidden: user is attempting
to grant RBAC permissions not currently held
Expand the pods verbs in the kubebuilder RBAC marker from get;list
to the full set needed by the created Roles.
Jira: OSPRH-20211
Checklist before requesting a review
pre-commit run --all