canonical/seldon-core-operator

Charm missing kubeflow role aggregation rule that gives users access to `SeldonDeployments`

Closed this issue · 2 comments

As presently implemented, the combination of kubeflow-roles and Seldon that are planned for the Kubeflow 1.7 release do not grant users access to SeldonDeployments in their namespaces. This has the effect of making Seldon unusable for Kubeflow users.

In pod spec versions of the seldon charm, Kubeflow users were granted permission to create/edit/* SeldonDeployments in their namespace via this aggregated ClusterRole (for a description of how kubeflow aggregates roles to users, see this readme). This ClusterRole was implemented in the kubeflow roles charm because pod spec did not allow us to create arbitrary ClusterRoles.

Now that the charm was migrated to sidecar, this ClusterRole could be deployed by this charm. canonical/kubeflow-roles-operator#38 removed the ClusterRole from the central role deployment (possibly because it was thought that this role was now implemented in the seldon charm directly?). The result is that users are not granted the required permissions for SeldonDeployments.

To fix this, we should:

  • do one of:
    • restore the aggregation ClusterRole in kubeflow-roles
    • implement the aggregation ClusterRole here in Seldon
  • add tests that would catch this in future

Regarding adding tests, it is unclear where the best place should be for them. Because this was the transference of responsibility from one charm to another, I think it could only be caught at the bundle level? Although it feels unsatisfying that we can delete a file in kubeflow-roles and not have a check at the repo level to say that was important.

DnPlas commented

Seems like the roles got removed in upstream Kubeflow 1.7. This aggregation rule comes exclusively from the manifests in the upstream Kubeflow project, so I have asked why this role got removed. In the meantime, my proposed solution is to revert back this role removal from kubeflow-roles-operator.

DnPlas commented

Fixed in #122