search

airshipit / sip

4 stars 1 forks source link

Remove namespace create/delete premissions in SIP #5

Closed seaneagan closed 3 years ago

seaneagan commented 3 years ago

Problem description (if applicable) As of https://review.opendev.org/c/airship/sip/+/778584 we have come closer to guaranteeing uniqueness of the namespace for the infra services, by using the metadata.name of the SIP CR. However, there could be SIP CRs with the same name in different namespaces. We also are still creating the namespace in the SIP code, in which case we should probably eventually be deleting the namespace when the SIP CR is deleted as well. It would be nice if SIP didn't need such powerful permissions to mess with namespaces, and just relied on pre-existing namespaces only.

Proposed change

Create infra services in a pre-existing namespace, which could be either of:

1) runtime namespace of SIP controller e.g. sipcluster-system, better separates the RBAC SIP admins and SIP users 2) same namespace as SIP CR 3) metadata.namespace-metadata.name of SIP CR, similar to current approach by having a custom namespace, but uniqueness is guaranteed and SIP code expects this to already exist.

with 2) we can use simply use the metadata.name of the SIP CR to generate the SIP service objects. with 1) we would need to include both the metadata.namespace and metadata.name of the SIP CR

Potential impacts No longer need RBAC for namespaces in SIP

seaneagan commented 3 years ago
  1. seems best to me
seaneagan commented 3 years ago

@ian-howell if we go with 1. see https://review.opendev.org/c/airship/vino/+/774549 for how this is accomplished in vino

lb4368 commented 3 years ago

With 1. we end up with all of our infra services in the same namespace, right? In past design discussions there was a desire for infra services to not all be in the same namespace. Number 2 might be better for that.

seaneagan commented 3 years ago

yeah 2) does allow for separate namespaces for each SIP CR, by putting each SIP CR in it's own namespace, but 3) is the only one that guarantees it even for SIP CRs in the same namespace. 2) is probably a good balance for namespace granularity, whereas 1) may be overly coarse and 3) may be overly fine. also with 2) one could simply place their SIP CRs in the same namespace as the SIP controller runtime, to accomplish something similar to 1). if we implemented something similar to https://github.com/fluxcd/flux2/issues/224 then this would allow to have something even more similar to 1). so i'll change my vote to 2)

ian-howell commented 3 years ago

Sounds good, I'll go with option 2