WIP: [UC Backup] BackupStorageLocation UX improvements by SamuAlfageme · Pull Request #7972 · kubermatic/dashboard

SamuAlfageme · 2026-04-03T01:10:18Z

What this PR does / why we need it:

This introduces a few UX improvements on the handling of BackupStorageLocations (BSLs) for the UC Backup feature. These apply to both, the Cluster Creation Wizard and "Edit cluster" dialogue.

Filter only 'available' BSLs on the dropdown selector.
New '+ Create Backup Storage Location' option.

Edit dialog

Add field validations on the BSL form(s):
- Basic regexp validation for DNS on 'Region' and 'Endpoint URL' (+ http(s) for the latter).

Mandatory 'Endpoint' & 'Region' unless using 'custom config' (see behavior in gif below).

Allow to copy BSL error messages on click from the BackupStorageLocation list view

Fix: reorder BSL selector & Skip router reconciliation on the 'Edit cluster' modal: the checkbox to enable UC Backup and the BSL dropdown were split by the new 'Skip router reconciliation' on OpenStack DCs

Which issue(s) this PR fixes: N/A

What type of PR is this?
/kind design

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Cluster Creation/Edition: Improve BackupStorageLocation UX: display only available BSLs and allow to create new ones.

Documentation:

NONE

kubermatic-bot · 2026-04-03T01:10:23Z

Adding the "do-not-merge/test-issue-needed" label because no test-issue block was detected, please add a proper test-issue block to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

kubermatic-bot · 2026-04-03T01:10:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign khizerrehan for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

modules/web/src/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubermatic-bot · 2026-04-03T01:10:31Z

Hi @SamuAlfageme. Thanks for your PR.

I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

KhizerRehan · 2026-04-03T10:39:37Z

/ok-to-test

KhizerRehan · 2026-04-06T08:24:02Z

@SamuAlfageme can your run npm run fix command and push after linting fixes

- Filter by 'available' BackupStorageLocation. - New '+ Create Backup Storage Location' option. Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

... apply to BSL Creation and Edition: - DNS validation for 'Region' and 'Endpoint URL' (& http(s) for the latter) - Explicit "Optional" on 'Access Key ID' and 'Secret Access Key' for anonymous login - Allow to copy BSL error messages on click from the BackupStorageLocation view Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

You can expect the user knows what it's doing while using 'custom config' so, no need to enforce string validation on the custom YAML Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

... on the 'Edit cluster' modal: the checkbox to enable UC Backup and the BSL dropdown were split by the new 'Skip router reconciliation' on OpenStack DCs Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

Replicate the same behaviour from the cluster creation wizard: - '+ Create Backup Storage Location' (modal on top of modal) - Filter by 'available' BSLs on the dropdown Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

Dynamical imports for AddBackupStorageLocationDialogComponent on the wizard and the edit-cluster dialog to fix the CI tests. Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

ahmadhamzh · 2026-04-09T06:09:18Z

                <mat-option [value]="bsl.name">{{bsl.displayName}}
                </mat-option>
                }
+                <mat-option [value]="createBackupStorageLocationOptionValue">+ Create Backup Storage Location</mat-option>


I don’t think adding an option to create a new BSL inside the dropdown is the right approach. I’d prefer something similar to how we handle SSH keys.

I think something like this makes more sense:

Good point! I forgot this pattern exists already somewhere in the Dashboard. I'll adapt the PR to use it too 👍

@ahmadhamzh btw, for the "edit UC" view, opening the 'Create BSL' modal over the 'Edit Cluster' modal is something that is not done somewhere else in the Dashboard IIRC, does that work for you?

Good point — I think we shouldn’t have that at all. So for cluster editing, we shouldn’t include an option to add a storage location at least for now.

IMO, dialog over a dialog is not a problem because i have seen this sort of behaviour in other applicaiton when supporting inline option e.g add-bsl dialog but I think what Ahmed said we shouldn't provide this option for edit cluster?

But I am also thinking what if there are existing cluster and need to backup in different locaiton how those scenario would be handled?

I like suggestion what ahmed has suggested of adding "+ Add BSL Button" which is better instead of adding inside Dropdown option

More clear/explicit

And we can opt this change adding behaviour for wizard (if not going for edit dialog)

But I am also thinking what if there are existing cluster and need to backup in different locaiton how those scenario would be handled?

users can add them from the cluster backups > Storage Locations and then use them in the edit dialog

ahmadhamzh · 2026-04-09T07:13:40Z

      });
  }

+  async onBackupStorageLocationSelectionChange(event: MatSelectChange<string>): Promise<void> {


Async/await is working fine, but since the rest of the component uses RxJS pipelines, I think it’s better to stay consistent with that approach.

ahmadhamzh · 2026-04-09T07:16:14Z

+
+    const AddBackupStorageLocationDialogComponent = await DynamicModule.AddBackupStorageLocationDialogComponent;
+    if (!AddBackupStorageLocationDialogComponent) {
+      return;
+    }
+    this._matDialog
+      .open(AddBackupStorageLocationDialogComponent, {
+        data: {projectID: this.projectID},
+      })
+      .afterClosed()
+      .pipe(take(1), filter(Boolean))


Maybe something like this would be more consistent with the rest of the code.

Suggested change

const AddBackupStorageLocationDialogComponent = await DynamicModule.AddBackupStorageLocationDialogComponent;

if (!AddBackupStorageLocationDialogComponent) {

return;

}

this._matDialog

.open(AddBackupStorageLocationDialogComponent, {

data: {projectID: this.projectID},

})

.afterClosed()

.pipe(take(1), filter(Boolean))

from(DynamicModule.AddBackupStorageLocationDialogComponent)

.pipe(

filter(Boolean),

switchMap(component =>

this._matDialog.open(component, {data: {projectID: this.projectID}}).afterClosed()

),

take(1),

filter(Boolean),

)

ahmadhamzh · 2026-04-09T07:25:03Z

+  }
+
+  private _isBackupStorageLocationAvailable(bsl: BackupStorageLocation): boolean {
+    const status = bsl.status?.phase || bsl.spec?.status || '';


i don't think we need to check for status cause it will never be available

Suggested change

const status = bsl.status?.phase || bsl.spec?.status || '';

return bsl.status?.phase.toLowerCase() === 'available'

ahmadhamzh · 2026-04-09T07:40:39Z

    return bsl;
  }
+
+  private _dnsNameValidator(): ValidatorFn {


We usually add validators in src/app/shared/validators

We do have a file shared/validators/others.ts where have defined all regex in case needed in other place IMO, regex that is reusable can be put into this file

let me wdyt?

well after another look i don't think we need this method just to repeat the same regex check.
we can update the regex value to include the .

KhizerRehan

Feedback(PTAL)

KhizerRehan · 2026-04-09T08:52:37Z

    return bsl;
  }
+
+  private _dnsNameValidator(): ValidatorFn {


We do have a file shared/validators/others.ts where have defined all regex in case needed in other place IMO, regex that is reusable can be put into this file

let me wdyt?

KhizerRehan · 2026-04-09T09:23:24Z

+      }
+
+      const labels = value.split('.');
+      return labels.every(label => this._dnsLabelRegex.test(label)) ? null : {invalidDnsName: true};


just to double confirm?

values like

us-east-1 ✅

us east 1 ❌

us-east-1- ❌

_us-east-1 ❌

are not valid DNS

KhizerRehan · 2026-04-09T09:26:11Z

+        return null;
+      }
+
+      if (!/^https?:\/\//i.test(value)) {


Just to double confirm if following values are not valid test value?

http://minio.local. ❌ (FQDN trailing dot rejected before)

https://[::1]:9000 (IPv6) ❌

https://s3_internal.lan:9000 (underscore host) ❌

KhizerRehan · 2026-04-09T11:26:46Z

+  private _isBackupStorageLocationAvailable(bsl: BackupStorageLocation): boolean {
+    const status = bsl.status?.phase || bsl.spec?.status || '';
+    return status.toLowerCase() === 'available';
+  }


FYI:
This comment should be taken into consideraiton incase we OPT to choose to changes in (Edit Cluster Dialog

Otherwise ignore this refactoring

I see this method duplicate can we move to src/app/shared/entity/backup.ts path wdyt?

I was checking codebase something we are doing

dashboard/modules/web/src/app/shared/entity/health.ts

Line 76 in bdafe71

export namespace HealthState {

like that using namespaces concept.

Ideally, i though it shouldn't be part of entity/ folder bcz IMO, we have kept all classes or interfaces but also it make sense to keep backup relateed things there even if there are shared utils as like defined here
src/app/shared/utils

something like we are doing here

dashboard/modules/web/src/app/shared/utils/cluster.ts

Line 42 in bdafe71

export function generateEncryptionKey(): string {

as this method is used in both place

wizard

edit cluster dialog

ahmadhamzh · 2026-04-13T10:54:56Z

 })
 export class AddBackupStorageLocationDialogComponent implements OnInit, OnDestroy {
  private readonly _unsubscribe = new Subject<void>();
+  private readonly _dnsLabelRegex = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/i;


this can be change to include the . and then we don't need to use _dnsNameValidator() method.
we can add
export const DNS_VALIDATOR = Validators.pattern(/^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)*$/i)

to the shared/validators/others.ts file

ahmadhamzh · 2026-04-13T10:56:24Z

-      [Controls.Region]: this._builder.control(this._config.bslObject?.spec.config?.region ?? ''),
-      [Controls.Endpoints]: this._builder.control(this._config.bslObject?.spec.config?.s3Url ?? ''),
+      [Controls.Region]: this._builder.control(this._config.bslObject?.spec.config?.region ?? '', [
+        this._dnsNameValidator(),


Suggested change

this._dnsNameValidator(),

DNS_VALIDATOR,

ahmadhamzh · 2026-04-13T11:25:51Z

+
+      try {
+        const url = new URL(value);
+        if (!['http:', 'https:'].includes(url.protocol)) {


i think if we have this check then we don't need the the previous condition if (!/^https?:\/\//i.test(value)) cause i think both do the same check, right ?

KhizerRehan · 2026-04-23T07:52:30Z

@SamuAlfageme thanks for the contribution!

Could you please review the feedback and let us know if anything is unclear? We’ve added a few comments for clarity and alignment.

If possible, please address the requested updates. Appreciate your effort.

KhizerRehan · 2026-04-23T09:46:27Z

/hold
/retitle WIP: [UC Backup] BackupStorageLocation UX improvements

SamuAlfageme · 2026-04-27T14:30:39Z

Hi @KhizerRehan, thank you for taking the time (together with @ahmadhamzh) to review this PR! I've been a bit busier than usual, but I should find some time to address your comments later this week.

kubermatic-bot added docs/none Denotes a PR that doesn't need documentation (changes). release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/design Categorizes issue or PR as related to design. labels Apr 3, 2026

kubermatic-bot added the do-not-merge/test-issue-needed Indicates that a PR should not merge because it's missing one of the test issue labels. label Apr 3, 2026

kubermatic-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 3, 2026

kubermatic-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 3, 2026

SamuAlfageme changed the title ~~BackupStorageLocation UX~~ [UC Backup] BackupStorageLocation UX improvements Apr 3, 2026

kubermatic-bot added the dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. label Apr 3, 2026

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from 231fbb2 to fc501a5 Compare April 3, 2026 01:14

kubermatic-bot added dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. and removed dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. labels Apr 3, 2026

kubermatic-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 3, 2026

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from fc501a5 to 85b3894 Compare April 7, 2026 14:43

kubermatic-bot added dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. and removed dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. labels Apr 7, 2026

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from 85b3894 to 3d3f94e Compare April 7, 2026 14:44

SamuAlfageme added 4 commits April 8, 2026 16:33

BSL improvements on Cluster Creation Wizard

199d31c

- Filter by 'available' BackupStorageLocation. - New '+ Create Backup Storage Location' option. Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

Endpoint/region not mandatory on 'custom config'

0fcd91f

You can expect the user knows what it's doing while using 'custom config' so, no need to enforce string validation on the custom YAML Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

Reorder BSL selector & Skip router reconciliation

d1335ea

... on the 'Edit cluster' modal: the checkbox to enable UC Backup and the BSL dropdown were split by the new 'Skip router reconciliation' on OpenStack DCs Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

SamuAlfageme added 2 commits April 8, 2026 16:33

Extend BSL-selection logic to 'Edit cluster'

30e2fa4

Replicate the same behaviour from the cluster creation wizard: - '+ Create Backup Storage Location' (modal on top of modal) - Filter by 'available' BSLs on the dropdown Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

html-beautify changes

97085f7

Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from 3b3b6f0 to d663049 Compare April 8, 2026 14:33

kubermatic-bot added dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. and removed dco-signoff: no Denotes that at least one commit in the pull request doesn't have a valid DCO signoff message. labels Apr 8, 2026

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from d663049 to d0e88a8 Compare April 8, 2026 14:45

Fix static imports for EE components

0f3bafe

Dynamical imports for AddBackupStorageLocationDialogComponent on the wizard and the edit-cluster dialog to fix the CI tests. Signed-off-by: Samuel Alfageme Sainz <samuel@alfage.me>

SamuAlfageme force-pushed the uc_backup_bsl_enhancements branch from d0e88a8 to 0f3bafe Compare April 8, 2026 18:04

ahmadhamzh self-requested a review April 9, 2026 07:01

ahmadhamzh requested changes Apr 9, 2026

View reviewed changes

kubermatic-bot assigned ahmadhamzh Apr 9, 2026

KhizerRehan reviewed Apr 9, 2026

View reviewed changes

ahmadhamzh reviewed Apr 13, 2026

View reviewed changes

kubermatic-bot changed the title ~~[UC Backup] BackupStorageLocation UX improvements~~ WIP: [UC Backup] BackupStorageLocation UX improvements Apr 23, 2026

kubermatic-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Apr 23, 2026

	const status = bsl.status?.phase \|\| bsl.spec?.status \|\| '';
	return bsl.status?.phase.toLowerCase() === 'available'

Conversation

SamuAlfageme commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubermatic-bot commented Apr 3, 2026

Uh oh!

kubermatic-bot commented Apr 3, 2026

Uh oh!

kubermatic-bot commented Apr 3, 2026

Uh oh!

KhizerRehan commented Apr 3, 2026

Uh oh!

KhizerRehan commented Apr 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahmadhamzh Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KhizerRehan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KhizerRehan commented Apr 23, 2026

Uh oh!

KhizerRehan commented Apr 23, 2026

Uh oh!

SamuAlfageme commented Apr 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SamuAlfageme commented Apr 3, 2026 •

edited

Loading

ahmadhamzh Apr 13, 2026 •

edited

Loading