Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ca-scribner opened this issue Mar 17, 2023 · 2 comments
Labels
bug Something isn't working Kubeflow 1.7

Comments

@ca-scribner
Copy link
Contributor

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.

@ca-scribner ca-scribner added bug Something isn't working Kubeflow 1.7 labels Mar 17, 2023
@DnPlas
Copy link
Contributor

DnPlas commented Mar 24, 2023

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
Copy link
Contributor

DnPlas commented Mar 29, 2023

Fixed in #122

@DnPlas DnPlas closed this as completed Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Kubeflow 1.7
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants