frontend: project: Fix ProjectList crash when a namespace has no labels#5723
Conversation
|
Welcome @govindup63! |
Reading n.metadata.labels![PROJECT_ID_LABEL] inside the groupBy iteratee
threw
TypeError: Cannot read properties of undefined (reading
'headlamp.dev/project-id')
whenever the items returned from Namespace.useList contained a namespace
without metadata.labels populated. The labelSelector on the request
filters at the API level, but the returned list can still transiently
include unlabelled items (multi-cluster fan-out, react-query cache
during a label removal), and an unguarded access there took down the
whole Projects page.
Move the grouping into a small exported helper so the null-safety guard
lives in one place and add a vitest regression covering the unlabelled
and partially-labelled cases.
Issue: kubernetes-sigs#5254
Signed-off-by: Govind Pandey <govindup63@gmail.com>
f89ddf2 to
c3b0b96
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the Projects page caused by dereferencing metadata.labels[PROJECT_ID_LABEL] when a Namespace item transiently lacks metadata.labels (e.g., during cache refresh or multi-cluster fan-out).
Changes:
- Added
groupNamespacesIntoProjectshelper to safely group namespaces byPROJECT_ID_LABEL, filtering out entries without that label beforegroupBy. - Updated
useProjectsto tolerateNamespace.useListreturningitems: nullby defaulting to an empty list. - Added a new Vitest suite covering normal grouping, multi-cluster behavior, and the two regression scenarios from #5254.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/src/components/project/ProjectList.tsx | Prevents Projects page crash by filtering namespaces lacking metadata.labels[PROJECT_ID_LABEL] before grouping; handles items: null. |
| frontend/src/components/project/ProjectList.test.tsx | Adds unit tests for the grouping helper, including regressions for missing labels / missing project-id label. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: govindup63, illume 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 fixes a TypeError that crashed the Projects page whenever the list returned from
Namespace.useListcontained a namespace withoutmetadata.labelspopulated. The Projects view simply went blank with this error in the console:The crash originated on
frontend/src/components/project/ProjectList.tsx, where thegroupByiteratee dereferencedn.metadata.labels![PROJECT_ID_LABEL]with a non-null assertion. ThelabelSelectoron the API request filters at the server side, but the array reachinggroupBycan still transiently include namespaces withoutmetadata.labelspopulated, for example during multi-cluster fan-out or while the react-query cache is being refreshed after a label removal.Related Issue
Fixes #5254
Changes
frontend/src/components/project/ProjectList.tsx: extract the namespaces-to-projects transformation into a small exported helpergroupNamespacesIntoProjectsand filter out items that do not carryPROJECT_ID_LABELbefore grouping. TheuseProjectshook now calls the helper and toleratesNamespace.useListreturningitems: null.frontend/src/components/project/ProjectList.test.tsx: new vitest suite covering normal grouping, multi-cluster collection, and the two regression cases from Crash Report: Cannot read properties of undefined (reading 'headlamp.dev/project-id') #5254 (a namespace whosemetadata.labelsisundefined, and a namespace whose labels object is present but does not includePROJECT_ID_LABEL).Steps to Test
npm installunderfrontend/if needed.npx vitest run src/components/project/ProjectList.test.tsx. All four tests should pass.groupNamespacesIntoProjectsreproduces the original crash from Crash Report: Cannot read properties of undefined (reading 'headlamp.dev/project-id') #5254 with the exact error message, which the new tests catch.npx tsc --noEmitandnpx eslint -c package.json src/components/project/ProjectList.tsx src/components/project/ProjectList.test.tsxare clean.Notes for the Reviewer
filteris a no-op. Its only job is to keep the inner iteratee safe against transient cache contents.useProjectsso it can be tested directly without an RTL harness. Following the cyclic-import workaround already used byprojectUtils.test.ts(the_dont_delete_me = Appimport) was the cheapest way to make the new test load.