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

Deploy package form: service account selection should be optional #5522

Closed
gfichtenholt opened this issue Oct 20, 2022 · 3 comments · Fixed by #5535
Closed

Deploy package form: service account selection should be optional #5522

gfichtenholt opened this issue Oct 20, 2022 · 3 comments · Fixed by #5535
Assignees
Labels
component/apprepository Issue related to kubeapps apprepository component/ui Issue related to kubeapps UI kind/bug An issue that reports a defect in an existing feature

Comments

@gfichtenholt
Copy link
Contributor

Filing bug per @antgamdia request. When installing a flux package I am forced to select a service account in the dashboard ? service account name is optional on the backend, why is the UI forcing me to pick one?
Screen Shot 2022-10-20 at 12 43 19 AM

@gfichtenholt gfichtenholt added the kind/bug An issue that reports a defect in an existing feature label Oct 20, 2022
@ppbaena ppbaena added component/ui Issue related to kubeapps UI component/apprepository Issue related to kubeapps apprepository labels Oct 20, 2022
@ppbaena ppbaena added the next-iteration Issues to be discussed in planning session label Oct 20, 2022
@antgamdia antgamdia self-assigned this Oct 20, 2022
@absoludity
Copy link
Contributor

Flux needs some credential with which to install...I believe ServiceAccountName is optional in the Flux backend because you can instead specify the KubeConfig, or run flux with the --default-service-account flag when installing flux itself (see the doc comment for the KubeConfig flag on the same page Greg linked to, which says: "If the --default-service-account flag is set, its value will be used as a controller level fallback for when HelmReleaseSpec.ServiceAccountName is empty.").

I'm not 100%, but I'd think that if you run flux without the --default-service-account flag then an install without the ServiceAccountName (and without a KubeConfig which we explicitly don't use) will fail? ie. it's a required option if running flux without that flag.

If that's true, then I think either:

  • we cater for the case where people may run flux with a --default-service-account set and so only make the ServiceAccountName field optional if we determine flux is running with --default-service-account set, or
  • we leave it required so that, like other plugins, users must provide the credentials with which an install is carried out (whether it's their own as in the Helm plugin or a service account as with the carvel plugin).

I don't know that just making it optional is going to be a good user experience, unless I'm missing something.

@gfichtenholt
Copy link
Contributor Author

gfichtenholt commented Oct 21, 2022

Disagree. Maybe flux does need some account down deep but not at the point of my interaction with it (creating/editing a HelmRelease CR). Whatever the default behavior is, at the end of the day I am able to deploy 99.99999% of my flux HelmRelease CRs without having to specify the service account. Most of the time I just don't care/want to worry about it. In the one case that I do care, I will look into the spec and see what's what. I would like the same experience from UX.

@absoludity
Copy link
Contributor

Yep - my mistake. It looks like Flux uses a cluster-admin role when reconciling by default, which is why you never have to worry about it - as long as you can create a HelmRelease CR, flux reconciles it for you using a cluster-admin role (even if you would not have been able to create the reconciled resources yourself). The --default-service-account option is new because people realised that without it flux doesn't easily support a multi-tenant cluster, as anyone who can create a HelmRelease in (only) one namespace, basically has cluster-admin (or can get it quite easily).

But that's the way Helm is today - so of course +1 to use the same UX :)

antgamdia added a commit that referenced this issue Oct 26, 2022
### Description of the change

This PR removed the `required=true` UI validation if using the Flux
plugin for installing a package.

### Benefits

UI consistent with the Flux backend.

### Possible drawbacks

N/A

### Applicable issues

- fixes #5522

### Additional information


![image](https://user-images.githubusercontent.com/11535726/197036382-3aaf4196-4724-46ad-bfa9-68f5eb26b32d.png)

Signed-off-by: Antonio Gamez Diaz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/apprepository Issue related to kubeapps apprepository component/ui Issue related to kubeapps UI kind/bug An issue that reports a defect in an existing feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants