kubermatic / dashboard

Dashboard For The Kubermatic Kubernetes Platform
https://www.kubermatic.com
Other
42 stars 62 forks source link

Make preset customizable #6756

Open csengerszabo opened 3 months ago

csengerszabo commented 3 months ago

Description of the feature you would like to add / User story

As a KKP User cluster owner I want that if a preset doesn't specify a security group, network or subnet, those fields should be loaded using the credentials and be available to be selected from a drop down list in order to let the users configure a preset with using pre-existing security group, network or subnet.

Solution details

Alternative approaches

Use cases

Additional information

### Providers
- [x] OpenStack
- [ ] All the others (3 points of effort / provider)
csengerszabo commented 3 months ago

/label customer-request /label sig/cluster-management /label sig/ui /label sig/api

csengerszabo commented 3 months ago

/label -sig/cluster-management

kubermatic-bot commented 3 months ago

@csengerszabo: The label(s) /label -sig/cluster-management cannot be applied. These labels are supported: blocked by backend, merge-type/merge, merge-type/rebase, needs details, service accounts, Epic, MVP, customer-request, design, feature, proposal, ready-to-challenge, redesign, sig/api, sig/app-management, sig/cluster-management, sig/community, sig/infra, sig/networking, sig/ui, sig/virtualization, sprint, team/marketing, team/ps, lifecycle/frozen, backport-needed, backport-complete, ee, needs-release-testing, test/require-vsphere, test/require-kubevirt, test/require-vmwareclouddirector, test/require-nutanix. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to [this](https://github.com/kubermatic/dashboard/issues/6756): >/label -sig/cluster-management Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
csengerszabo commented 3 months ago

/remove-label sig/cluster-management

csengerszabo commented 3 months ago

/assign @ahmadhamzh

csengerszabo commented 2 months ago

/transfer dashboard

ahmedwaleedmalik commented 2 months ago

As discussed, having this feature is more problematic than beneficial since it goes against the essence of how presets are designed to work on the UI. They are meant to obscure confidential information from the end users in its entirety in a way that the back end is responsible for managing all that data instead of any front-facing entity. That's one of the reasons why you can't edit the form or change individual fields within the credentials form once you have selected a preset. API calls will not use credentials explicitly and just use the preset name, which also enhances the security spectrum.

Having such a feature would mean that we need to add specialized cases per field to mark them as "not optional but editable" which adds a lot of toil. It would also mean that the source of credentials is now shared between "preset" and "manual input" in a non-deterministic way. We can't determine whether a field was left empty because of the preset or the user, which is similar to where a certain value is coming from.

An alternative approach could be to single out the most crucial fields, which are username and password(keywords will vary based on a cloud provider). They would be uneditable in a preset. And there would be a flag, let's say isEditable, that would signify that all the fields other than username and password can be edited from the UI during cluster creation. This can introduce a more generic approach while fulfilling the need to set these individual fields, which is also understandable.

Eventually, UI would still rely on the credentials from the preset, but everything else will be picked from the user's input. Also, this needs to be looked at, but in such a case, we shouldn't link the cluster with preset whatsoever and just consider it to be static credentials provided by the user.

judge-red commented 2 months ago

As this issue is based on a support ticket that I opened, I'll weigh in on what Waleed commented.

Having such a feature would mean that we need to add specialized cases per field to mark them as "not optional but editable" which adds a lot of toil.

Indeed, and we're not looking for that. My initial suggestion was to make all the fields editable that have no value / an empty string set (e.g. in the OpenStack provider, if the Network isn't defined, KKP will create one, so this is a valid value). However, I already anticipated that this might not be desired for the original use case of the presets which you've now shared with us, thanks.

An alternative approach could be to single out the most crucial fields, [...]. And there would be a flag, let's say isEditable, that would signify that all the fields [...] can be edited from the UI during cluster creation

Yes, that was exactly my suggestion, in case the above wasn't acceptable, as well.

csengerszabo commented 2 months ago

Let's move forward with @ahmedwaleedmalik's alternative approach with this issue if we happen to implement this.

ahmedwaleedmalik commented 1 month ago

Update: https://github.com/kubermatic/dashboard/pull/6807 implements this feature for OpenStack in KKP. This feature will be shipped in KKP 2.26. Moving this ticket out of KKP 2.26 milestone as OpenStack was the requirement for 2.26 and that has been covered. cc @csengerszabo

We'll follow suit for rest of the cloud providers in KKP 2.27.