frontend: Warn before deleting protected namespaces - #6106
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Anjali-Chauhan1 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
mahesh-09-12
left a comment
There was a problem hiding this comment.
@Anjali-Chauhan1 took a look at the namespace safeguards and the changes around deletion behavior. The checks added here appear consistent with the expected restrictions, and the confirmation prompt is applied appropriately across the affected actions. I didn't spot any issues in the code paths modified by this update.
|
@illume , plz review the pr. |
There was a problem hiding this comment.
Pull request overview
This PR adds a frontend safety guard to reduce accidental deletion of Kubernetes system namespaces by introducing a “protected namespace” concept and requiring an explicit type-to-confirm step in the delete confirmation UI.
Changes:
- Add
Namespace.PROTECTED_NAMESPACESandNamespace.isProtected()helper for identifying system namespaces. - Update single and multi-delete dialogs to warn on protected namespaces and disable Confirm until the required name(s) are typed.
- Add i18n keys for the new UI strings plus unit tests and Storybook stories/snapshots.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/lib/k8s/namespace.ts | Adds protected namespace list and isProtected() helper. |
| frontend/src/lib/k8s/index.test.ts | Adds unit tests covering Namespace.isProtected(). |
| frontend/src/components/common/ConfirmDialog.tsx | Adds confirmButtonDisabled prop to support disabled confirm state. |
| frontend/src/components/common/Resource/DeleteButton.tsx | Adds protected-namespace warning and type-to-confirm gating for single delete. |
| frontend/src/components/common/Resource/DeleteMultipleButton.tsx | Adds protected-namespace warning and type-to-confirm gating for multi delete. |
| frontend/src/components/common/Resource/DeleteButton.stories.tsx | Adds protected/non-protected namespace stories. |
| frontend/src/components/common/Resource/snapshots/DeleteButton.ProtectedNamespace.stories.storyshot | Adds snapshot for the new protected namespace story. |
| frontend/src/components/common/Resource/snapshots/DeleteButton.NonProtectedNamespace.stories.storyshot | Adds snapshot for the new non-protected namespace story. |
| frontend/src/i18n/locales/en/translation.json | Adds English strings for protected-namespace warnings and prompts. |
| frontend/src/i18n/locales/ar/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/bn/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/de/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/es/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/fr/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/he/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/hi/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/it/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/ja/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/ko/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/pt/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/ru/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/ta/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/ur/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/zh/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
| frontend/src/i18n/locales/zh-tw/translation.json | Adds empty-string placeholders for new keys (translation fallback behavior). |
9022d0b to
222ea7b
Compare
|
@illume, I have did the required changes that are suggested by copilot :) |
222ea7b to
e53d819
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.
e53d819 to
b066c36
Compare
illume
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
Could you take a look at the commit messages in this PR? We follow a Linux kernel style for git commits — see the contributing guide and git log for examples.
Commits that need attention
frontend: Remove comment from DeleteButton stories— Only one file changed insidefrontend/; add a sub-area so it's clear what was touched (e.g.frontend: ComponentName: description).
Commit guidelines
- Use atomic commits focused on a single change.
- Use the title format
<area>: <Description of changes>— description must start with a capital letter. - Keep the title under 72 characters (soft requirement).
- Explain the intention and why the change is needed.
- Make commit titles meaningful and describe what changed.
- Do not add code that a later commit rewrites; squash or reorder commits instead.
- Do not include
Fixes #NNin commit messages.
Good examples:
frontend: HomeButton: Fix so it navigates to homebackend: config: Add enable-dynamic-clusters flag
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.
Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com> frontend: Update namespace deletion confirmation frontend: Add DeleteButton tests and refine protected namespace confirmation Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com> frontend: Protect namespace deletion checks Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com> frontend: DeleteButton.stories: Remove unnecessary comments Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com>
Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com>
Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com> refactor: Simplify protected namespace checks Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com>
Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com>
4129c01 to
302c8c2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
frontend/src/components/common/Resource/DeleteButton.tsx:102
isProtectedNamespacecurrently relies on a cast andnamespaceNameuses??. If a Namespace object is not an actualNamespaceinstance (or if thekubernetes.io/metadata.namelabel exists but is an empty string), this can throw or unintentionally enable deletion without typing anything. UseNamespace.isClassOf(item)for a safe type guard and use||so empty-string labels don’t bypass the type-to-confirm step.
// System namespaces require an extra type-to-confirm step before they can be deleted.
const isProtectedNamespace = item.kind === 'Namespace' && (item as Namespace).isProtected();
// Use the same label-or-name value that isProtected() checks so the confirmation prompt matches.
const namespaceName = item.metadata.labels?.['kubernetes.io/metadata.name'] ?? item.metadata.name;
frontend/src/components/common/Resource/DeleteMultipleButton.tsx:84
protectedNamespacesuses a kind-string check plus a cast to callisProtected(). This is less safe than the established pattern in the codebase (e.g.Deployment.isClassOf(item)inScaleMultipleButton.tsx) and can crash if an item has kindNamespacebut isn’t an actualNamespaceinstance. PreferNamespace.isClassOf(item)as the type guard before callingitem.isProtected().
// Protected namespaces included in the current selection, if any.
const protectedNamespaces = (items ?? []).filter(
(item): item is Namespace => item.kind === 'Namespace' && (item as Namespace).isProtected()
);
Signed-off-by: Anjali-Chauhan1 <anjalichauhan1036@gmail.com>
illume
left a comment
There was a problem hiding this comment.
Thanks for these changes.
Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Anjali-Chauhan1, vyncent-t The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
This PR adds a UI safeguard against accidentally deleting Kubernetes system namespaces. Headlamp previously showed Delete and Force Delete for every namespace — including
default,kube-system,kube-node-lease, andkube-public— so a user with a cluster-admin token could break a cluster with a single accidental click. These namespaces are now treated as protected: the delete dialog shows a warning and requires typing the namespace name to confirm.Related Issue
Fixes #6064
Changes
Namespace.PROTECTED_NAMESPACESand anisProtected()helper inlib/k8s/namespace.ts, matched against thekubernetes.io/metadata.namelabel (falling back to the object name).DeleteButtonto show a warning Alert and a type-the-name confirmation field, keeping the Delete button disabled until the name matches, for protected namespaces.DeleteMultipleButtonto flag any protected namespaces in a multi-select deletion.ConfirmDialogchange to support the disabled-confirm state.Steps to Test
kube-system). Observe the warning "This is a system namespace. Deleting it may break your cluster." and that Delete stays disabled until you type the namespace name.my-app). Observe the standard single-confirmation flow with no warning.Screenshots
Notes for the Reviewer
warning.mainon the Alert) rather than changing the shared theme, becausepalette.warning.lightis intentionally reused as a translucent background tint elsewhere (PluginSettingsDetails).