tenant-controller: add namespace/rbac sync on update of tenant #12
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: easeway The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
size/L
|
|
||
| func (c *Controller) ensureNamespaceExists(tenant *tenantsapi.Tenant, nsName string) error { | ||
| // TODO Add later ... sanity checks to ensure namespaces being requested are valid and not already assigned to another tenant | ||
| if err := newKubeCtl().addObjects(&corev1.Namespace{ |
There was a problem hiding this comment.
Minor comment: In the same file we now have calls using k8sclient as well as newKubeCtl() ... we should discuss whether to make it consistent and use the same approach everywhere or whether it is okay to use different approaches. The separate k8sclient approach was taken to natch the example in the sample-controller repo.
| // TODO sync namespace | ||
| // TODO sync RBAC inside namespace | ||
| func (c *Controller) createNamespaceForTenant(tenant *tenantsapi.Tenant, nsReq *tenantsapi.TenantNamespace) error { | ||
| if err := c.ensureNamespaceExists(tenant, nsReq.Name); err != nil { |
There was a problem hiding this comment.
Minor comment: The names of the functions ensureNamespaceExists and syncNamespaceForTenant are slightly misleading. The first one is not ensuring but actually creating the namespace (maybe a name like createNamespaceForTenant is something to consider ?). The second one is attaching rbac for this namespace (so maybe a name like syncRbacForTenant or syncPropertiesForTenant could be considered ?). Anyway its a minor comment.
Refactor the code and add the functionality to: