diff --git a/packages/dashboard-frontend/adr/adr-001-auto-generate-resource-name-on-restore.md b/packages/dashboard-frontend/adr/adr-001-auto-generate-resource-name-on-restore.md new file mode 100644 index 0000000000..9a8a0894e9 --- /dev/null +++ b/packages/dashboard-frontend/adr/adr-001-auto-generate-resource-name-on-restore.md @@ -0,0 +1,127 @@ +# ADR-001: Auto-generate K8s resource name when restoring from backup + +- **Status:** Accepted +- **Date:** 2026-05-13 +- **PR:** [#1565](https://github.com/eclipse-che/che-dashboard/pull/1565) + +## Context + +Eclipse Che workspaces have two names: + +- **`metadata.name`** — the immutable Kubernetes resource identifier, set at creation time (e.g., `empty-1234`). Never shown to users in the dashboard UI. +- **Display name** — stored in the `kubernetes.io/metadata.name` label, changeable by the user (e.g., `not-empty-1234`). Shown everywhere in the UI: workspace list, sidebar, breadcrumbs, details page. + +The restore-from-backup flow currently sets `metadata.name` directly from the user's text input. This creates a UX problem after workspace rename: + +1. User creates workspace `empty-1234` (this becomes `metadata.name`) +2. User renames it to `not-empty-1234` (sets the display name label; `metadata.name` stays `empty-1234`) +3. User sees `not-empty-1234` everywhere — the name `empty-1234` is invisible +4. DWO backs up the workspace under `empty-1234` (uses `metadata.name`) +5. User tries to restore a backup with name `empty-1234` +6. Conflict error: "A workspace with this name already exists" — but the user cannot see any workspace named `empty-1234` + +This looks like a bug to the user. + +## Options considered + +### 1. Improve the error message + +Show the display name alongside the resource name in the conflict error: *"A workspace named 'not-empty-1234' (resource: empty-1234) already exists."* + +- **Pros:** Minimal change, transparent, shippable immediately +- **Cons:** Leaks K8s internals (`metadata.name`) to users who have no mental model for it. Every other UI surface hides this identifier — the restore form would be the only place exposing it. Fixes the confusion but preserves the broken interaction model. + +### 2. Validate display names only, handle K8s 409 at API level + +The frontend checks only display names for conflicts. If the K8s API rejects the create with a 409 (resource name collision), surface an error after submission. + +- **Pros:** UX is intuitive from the user's perspective — if they can't see the name, no conflict is shown +- **Cons:** Error surfaces late (after clicking Restore), not in the form. The K8s 409 error message is cryptic and hard to translate into a user-friendly message. Split validation model (frontend checks one thing, backend checks another) is fragile. + +### 3. Auto-generate `metadata.name` at restore time + +The user types a display name. The system generates a unique `metadata.name` using `generateWorkspaceName()` (appends a random 4-character suffix), and stores the user-typed name as the display name label. Conflict validation checks display names instead of resource names. + +- **Pros:** Eliminates K8s resource name conflicts entirely. Consistent with the factory workspace creation flow, which already uses `generateWorkspaceName()`. Users never encounter errors referencing invisible identifiers. Proven pattern in production. +- **Cons:** `metadata.name` of a restored workspace differs from the original backup's workspace name (e.g., `my-ws-x7q2` instead of `my-ws`). Users who `kubectl get devworkspaces` see the suffixed name. Display name collisions are possible (two workspaces with the same display name but different resource names). + +### 4. Try exact name, append suffix on conflict + +Use the user-typed name as `metadata.name`. If K8s rejects with a 409, retry with an auto-generated suffix. + +- **Pros:** Clean names when no conflict exists +- **Cons:** Inconsistent behavior — sometimes the user gets what they typed, sometimes they don't. Requires two API calls in the conflict case. Factory creation always generates a suffix; restore should be consistent. + +### 5. Fix at DWO level + +Make workspace rename update `metadata.name` by deleting and recreating the DevWorkspace resource with the new name. + +- **Pros:** Eliminates the dual-name problem at the source +- **Cons:** Requires DWO changes (different repository, different release cycle). Recreating a DevWorkspace loses UID, events, and status history. Complex and risky. + +## Decision + +**Option 3: Auto-generate `metadata.name` at restore time.** + +### Motivation + +The restore flow creates a new DevWorkspace resource — it does not patch an existing one. The sequence is: + +1. Build a restore devfile with `RESTORE_WORKSPACE=true` and `RESTORE_SOURCE_IMAGE` attributes +2. Call `fetchResources()` to generate DevWorkspace + DevWorkspaceTemplate resources +3. `DwtApi.createTemplate()` — creates new DevWorkspaceTemplate +4. `DwApi.createWorkspace()` — creates new DevWorkspace +5. Set ownerReference linking the two + +This is the same create-resource pattern that factory workspace creation uses. Factory creation already calls `generateWorkspaceName()` to produce suffixed names like `my-project-a1b2`. Applying the same pattern to restore is architecturally consistent. + +Every "negative" consequence of this approach (suffixed `metadata.name`, display name collisions, kubectl showing a different name) already exists in factory workspace creation and is accepted as a trade-off. Restore should not behave differently from creation. + +The other options either expose K8s internals to users (option 1), push errors to submission time (option 2), behave inconsistently (option 4), or require changes outside the dashboard (option 5). + +### What changes + +In `restoreWorkspaceFromBackup.ts`, instead of: + +```typescript +devWorkspaceResource.metadata.name = workspaceName; +``` + +Do: + +```typescript +devWorkspaceResource.metadata.name = generateWorkspaceName(workspaceName); +if (!devWorkspaceResource.metadata.labels) { + devWorkspaceResource.metadata.labels = {}; +} +devWorkspaceResource.metadata.labels[DEVWORKSPACE_LABEL_METADATA_NAME] = workspaceName; +``` + +The conflict validation in `RestoreFromBackup/index.tsx` shifts from checking K8s resource names to checking display names: + +```typescript +// Before: checked metadata.name (invisible to user) +const existingWorkspaceNames = new Set(allWorkspaces.map(ws => ws.resourceName)); + +// After: check display names (what the user sees) +const existingWorkspaceNames = new Set(allWorkspaces.map(ws => ws.name)); +``` + +## Consequences + +### Positive + +- K8s resource name conflicts become structurally impossible during restore +- Restore behavior is consistent with factory workspace creation +- Users never encounter errors referencing invisible identifiers +- The conflict check validates what the user actually sees + +### Negative + +- `metadata.name` of a restored workspace will differ from the original backup's workspace name. This is the same trade-off factory creation makes. +- Users who `kubectl get devworkspaces` will see the suffixed name, not the display name. This is also consistent with factory-created workspaces. +- Display name collisions are possible (two workspaces with the same display name but different resource names). This is already possible with factory-created workspaces and is not a new risk. + +### Migration + +No migration needed. This change only affects new restore operations. Existing workspaces are unaffected. diff --git a/packages/dashboard-frontend/src/containers/WorkspacesList/__tests__/index.spec.tsx b/packages/dashboard-frontend/src/containers/WorkspacesList/__tests__/index.spec.tsx index f491e4b950..1159b1f4a8 100644 --- a/packages/dashboard-frontend/src/containers/WorkspacesList/__tests__/index.spec.tsx +++ b/packages/dashboard-frontend/src/containers/WorkspacesList/__tests__/index.spec.tsx @@ -179,6 +179,53 @@ describe('Workspaces List Container', () => { ); }); + it('should use resource name (metadata.name) for renamed workspaces, not display name', async () => { + mockedBackupApi.getBackupConfig.mockResolvedValue({ + enabled: true, + schedule: '0 1 * * *', + registry: 'registry.example.com', + }); + mockedBackupApi.getWorkspaceBackupStatus.mockResolvedValue({ + status: BackupStatus.NEVER, + backupSchedule: '0 1 * * *', + }); + + const workspaces = [ + new DevWorkspaceBuilder() + .withName('original-k8s-name') + .withNamespace('user-che') + .withUID('uid-renamed') + .withMetadata({ + labels: { + 'kubernetes.io/metadata.name': 'user-display-name', + }, + }) + .build(), + ]; + const store = new MockStoreBuilder({ + backups: { + ...backupsUnloadedState, + backupConfig: { + enabled: true, + schedule: '0 1 * * *', + registry: 'registry.example.com', + }, + }, + }) + .withDevWorkspaces({ workspaces }, false) + .withWorkspaces({}, false) + .build(); + + renderComponent(store); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(mockedBackupApi.getWorkspaceBackupStatus).toHaveBeenCalledWith( + 'user-che', + 'original-k8s-name', + ); + }); + it('should not fetch backup statuses when backup config has no registry', () => { mockedBackupApi.getBackupConfig.mockResolvedValue({ enabled: false, diff --git a/packages/dashboard-frontend/src/containers/WorkspacesList/index.tsx b/packages/dashboard-frontend/src/containers/WorkspacesList/index.tsx index 34aaff61eb..089b7be36f 100644 --- a/packages/dashboard-frontend/src/containers/WorkspacesList/index.tsx +++ b/packages/dashboard-frontend/src/containers/WorkspacesList/index.tsx @@ -69,7 +69,7 @@ export class WorkspacesListContainer extends React.PureComponent { this.props.fetchWorkspaceBackupStatus({ namespace: workspace.namespace, workspaceUID: workspace.uid, - workspaceName: workspace.name, + workspaceName: workspace.resourceName, }); } }); diff --git a/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/__tests__/__snapshots__/index.spec.tsx.snap b/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/__tests__/__snapshots__/index.spec.tsx.snap index c4c0c5d314..d5701b9fc8 100644 --- a/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/__tests__/__snapshots__/index.spec.tsx.snap +++ b/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/__tests__/__snapshots__/index.spec.tsx.snap @@ -9,7 +9,7 @@ exports[`BackupTab error state snapshot - error 1`] = ` >
{ expect(mockFetchBackupStatus).toHaveBeenCalledWith({ namespace: workspace.namespace, workspaceUID: workspace.uid, - workspaceName: workspace.name, + workspaceName: workspace.resourceName, }); }); @@ -72,7 +72,29 @@ describe('BackupTab', () => { expect(mockFetchBackupStatus).toHaveBeenLastCalledWith({ namespace: newWorkspace.namespace, workspaceUID: newWorkspace.uid, - workspaceName: newWorkspace.name, + workspaceName: newWorkspace.resourceName, + }); + }); + + test('should use resource name (metadata.name) for renamed workspaces, not display name', () => { + const renamedDevWorkspace = new DevWorkspaceBuilder() + .withName('original-k8s-name') + .withNamespace('user-namespace') + .withUID('uid-renamed') + .withMetadata({ + labels: { + 'kubernetes.io/metadata.name': 'user-display-name', + }, + }) + .build(); + const renamedWorkspace = constructWorkspace(renamedDevWorkspace); + + renderComponent(renamedWorkspace); + + expect(mockFetchBackupStatus).toHaveBeenCalledWith({ + namespace: 'user-namespace', + workspaceUID: 'uid-renamed', + workspaceName: 'original-k8s-name', }); }); diff --git a/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/index.tsx b/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/index.tsx index c22277ae30..20a73bf296 100644 --- a/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/index.tsx +++ b/packages/dashboard-frontend/src/pages/WorkspaceDetails/BackupTab/index.tsx @@ -58,7 +58,7 @@ export class BackupTab extends React.PureComponent { this.props.fetchWorkspaceBackupStatus({ namespace: workspace.namespace, workspaceUID: workspace.uid, - workspaceName: workspace.name, + workspaceName: workspace.resourceName, }); } diff --git a/packages/dashboard-frontend/src/services/workspace-adapter/__tests__/index.spec.ts b/packages/dashboard-frontend/src/services/workspace-adapter/__tests__/index.spec.ts index 4e98365eae..245ece6e76 100644 --- a/packages/dashboard-frontend/src/services/workspace-adapter/__tests__/index.spec.ts +++ b/packages/dashboard-frontend/src/services/workspace-adapter/__tests__/index.spec.ts @@ -180,6 +180,30 @@ describe('for DevWorkspace', () => { expect(workspace.name).toEqual('new-name'); }); + it('should return resourceName from metadata.name even when label overrides name', () => { + const metadataName = 'original-k8s-name'; + const displayName = 'renamed-display-name'; + const devWorkspace = new DevWorkspaceBuilder() + .withMetadata({ + labels: { + [DEVWORKSPACE_LABEL_METADATA_NAME]: displayName, + }, + name: metadataName, + }) + .build(); + const workspace = constructWorkspace(devWorkspace); + expect(workspace.name).toEqual(displayName); + expect(workspace.resourceName).toEqual(metadataName); + }); + + it('should return resourceName equal to name when no label is set', () => { + const name = 'wksp-1234'; + const devWorkspace = new DevWorkspaceBuilder().withName(name).build(); + const workspace = constructWorkspace(devWorkspace); + expect(workspace.resourceName).toEqual(name); + expect(workspace.name).toEqual(name); + }); + it('should return namespace', () => { const namespace = 'test-namespace'; const devWorkspace = new DevWorkspaceBuilder().withNamespace(namespace).build(); diff --git a/packages/dashboard-frontend/src/services/workspace-adapter/index.ts b/packages/dashboard-frontend/src/services/workspace-adapter/index.ts index 601961c104..8d34ba146a 100644 --- a/packages/dashboard-frontend/src/services/workspace-adapter/index.ts +++ b/packages/dashboard-frontend/src/services/workspace-adapter/index.ts @@ -35,6 +35,7 @@ export interface Workspace { readonly id: string; readonly uid: string; name: string; + readonly resourceName: string; readonly namespace: string; readonly infrastructureNamespace: string; readonly created: number; @@ -157,6 +158,10 @@ export class WorkspaceAdapter implements Work this.workspace.metadata.labels[DEVWORKSPACE_LABEL_METADATA_NAME] = name; } + get resourceName(): string { + return this.workspace.metadata.name; + } + get namespace(): string { return this.workspace.metadata.namespace; } diff --git a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/__tests__/restoreWorkspaceFromBackup.spec.ts b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/__tests__/restoreWorkspaceFromBackup.spec.ts index 892484e3ed..c0924d786a 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/__tests__/restoreWorkspaceFromBackup.spec.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/__tests__/restoreWorkspaceFromBackup.spec.ts @@ -198,7 +198,7 @@ describe('restoreWorkspaceFromBackup', () => { ); }); - test('should set the workspace name from the argument, not the devfile', async () => { + test('should auto-generate metadata.name and set display name label', async () => { await store.dispatch( restoreWorkspaceFromBackup( 'user-ns', @@ -210,7 +210,12 @@ describe('restoreWorkspaceFromBackup', () => { expect(DwApi.createWorkspace).toHaveBeenCalledWith( expect.objectContaining({ - metadata: expect.objectContaining({ name: 'my-custom-name' }), + metadata: expect.objectContaining({ + name: expect.stringMatching(/^my-custom-name-[a-z0-9]{4}$/), + labels: expect.objectContaining({ + 'kubernetes.io/metadata.name': 'my-custom-name', + }), + }), }), ); }); diff --git a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/restoreWorkspaceFromBackup.ts b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/restoreWorkspaceFromBackup.ts index 7036cce621..ab66121ca6 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/restoreWorkspaceFromBackup.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/actions/actionCreators/restoreWorkspaceFromBackup.ts @@ -21,9 +21,11 @@ import { fetchResources } from '@/services/backend-client/devworkspaceResourcesA import * as DwtApi from '@/services/backend-client/devWorkspaceTemplateApi'; import devfileApi from '@/services/devfileApi'; import { DEVWORKSPACE_CHE_EDITOR } from '@/services/devfileApi/devWorkspace/metadata'; +import { generateWorkspaceName } from '@/services/helpers/generateName'; import { loadResourcesContent } from '@/services/registry/resources'; import { COMPONENT_UPDATE_POLICY, + DEVWORKSPACE_LABEL_METADATA_NAME, REGISTRY_URL, } from '@/services/workspace-client/devworkspace/devWorkspaceClient'; import { AppThunk } from '@/store'; @@ -129,9 +131,14 @@ export const restoreWorkspaceFromBackup = throw new Error('Failed to find DevWorkspaceTemplate in fetched resources.'); } - // Ensure the workspace name matches user input (generator may produce a different name) - devWorkspaceResource.metadata.name = workspaceName; + // Auto-generate a unique K8s resource name and store the user-typed name as the + // display name label — consistent with factory workspace creation (see ADR-001). + devWorkspaceResource.metadata.name = generateWorkspaceName(workspaceName); devWorkspaceResource.metadata.namespace = namespace; + if (!devWorkspaceResource.metadata.labels) { + devWorkspaceResource.metadata.labels = {}; + } + devWorkspaceResource.metadata.labels[DEVWORKSPACE_LABEL_METADATA_NAME] = workspaceName; if (!devWorkspaceResource.metadata.annotations) { devWorkspaceResource.metadata.annotations = {};