NO-JIRA: do not fall back to the raw resource field for empty children

[XIANLIBO]

What this fixes

DetailsItem resolved its rendered value with:

const value: ReactNode = children || _.get(obj, path, defaultValue);

An empty child falls through to the raw resource field. For paths such as
metadata.annotations that field is a plain object, and React rejects a plain
object as a child, so the whole details page crashes:

Minified React error #31; object with keys
{argocd.argoproj.io/tracking-id, kubectl.kubernetes.io/last-applied-configuration}

How it is reachable today

Route details page, Japanese locale:

1. DetailsSectionLeftColumn renders the Annotations item as
   canUpdate ? <Button>{label}</Button> : label, where
   label = t('{{count}} annotation', { count }).
2. useAccessReview initialises isAllowed to false while the
   SelfSubjectAccessReview is in flight, so the first render always takes the
   plain-string branch — including for cluster admins.
3. In the ja resource, {{count}} annotation_other is an empty string, so
   children is ''.
4. '' || _.get(route, 'metadata.annotations') yields the annotations object,
   React throws, and the error boundary replaces the page.

Routes with two or more annotations are affected. Routes managed by Argo CD
always are, since they carry both argocd.argoproj.io/tracking-id and
kubectl.kubernetes.io/last-applied-configuration. Routes with exactly one
annotation render fine, because {{count}} annotation_one is absent from the
ja resource and the i18next fallback returns a non-empty string.

The locale resource as actually served

Captured from /locales/resource.json?ns=plugin__networking-console-plugin on a
4.21 cluster:

lng	{{count}} annotation_one	{{count}} annotation_other
en	"{{count}} annotation_one"	"{{count}} annotation_other"
ja	absent	""

That is the whole asymmetry. Under lng=en the value is the key name itself, so
the Annotations row renders the literal string 2 annotation_other — wrong
text, but non-empty, so the || fallback never fires. Under lng=ja the value
is "", the fallback fires, and the page crashes.

The English rendering is worth noting on its own: a details page showing a raw
i18next plural key to the user is already a visible defect, independent of the
crash.

Relation to 00e08ca

00e08ca ("Backport translations from 4.22") repairs those strings and removes
the trigger. It does not remove the fallback that turns a falsy child
into a rendered object, so any future empty or missing translation reintroduces
the same crash, in DetailsItem for every caller — not just Annotations.

The change

-  const value: ReactNode = children || _.get(obj, path, defaultValue);
+  const value: ReactNode = children ?? _.get(obj, path, defaultValue);

?? restores the intended contract: use the children when the caller provided
them, and read the resource field only when the caller provided none. A caller
passing a falsy child now renders that falsy child, which React handles, rather
than silently swapping in a value of an unrelated type.

I audited all 68 <DetailsItem> call sites — 49 with children, 19 self-closing.
None relies on a falsy child falling through to the raw field:

• The {cond && ...} patterns in this repo wrap the DetailsItem element
  itself, or sit alongside a sibling element so that children is an array
  (truthy either way).
• UDNLayer2Details / UDNLayer3Details receive subnets / joinSubnets from
  UDNLayerDetails.getList, which always returns an element — <List> when
  there are entries, <MutedText> otherwise.
• IngressStatus passes {ingress?.host} and {ingress?.wildcardPolicy}.
  Both are undefined when absent, which ?? still treats as "no children",
  so those items are unchanged.
• No call site passes a numeric or boolean child.

?? still falls through for the 19 self-closing call sites, so I checked those
too: every one points at a scalar path (metadata.name, spec.host,
spec.tls.termination, .mtu, .role, subnets[i].cidr, and so on). The
object-valued paths in this repo — metadata.labels, metadata.annotations,
metadata.ownerReferences, spec.selector, spec.ports — are all rendered
through element children, which ?? never discards. So after this change there
is no remaining path in the repo by which the raw field can reach React as a
plain object.

Verification

• npm run lint — 0 errors. (The one warning is pre-existing, in
  SyncedEditor.tsx.) Since .eslintrc.json sets prettier/prettier to
  error, this also confirms the formatting is correct.
• npx tsc --noEmit -p tsconfig.json — 32 errors before and after the change,
  unchanged; none in the touched file.
• No translatable strings are added or removed, so npm run i18n produces no
  change to locales/.

On the missing Jira reference

CONTRIBUTING.md asks for OCPBUGS-NNNNN: in the title. I do not have
issue-creation permission in the Red Hat Jira, so I could not file one — hence
NO-JIRA:. If you would like this tracked and backported to the active release
branches, please attach or file an OCPBUGS and I will retitle the PR.

Observed on

OpenShift 4.21.27, networking-console-plugin at release payload 4.21.27,
1629 affected Routes on the cluster where this was found.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@XIANLIBO: This pull request explicitly references no jira issue.

Details

In response to this:

What this fixes

DetailsItem resolved its rendered value with:

const value: ReactNode = children || _.get(obj, path, defaultValue);

An empty child falls through to the raw resource field. For paths such as
metadata.annotations that field is a plain object, and React rejects a plain
object as a child, so the whole details page crashes:

Minified React error #31; object with keys
{argocd.argoproj.io/tracking-id, kubectl.kubernetes.io/last-applied-configuration}

How it is reachable today

Route details page, Japanese locale:

1. DetailsSectionLeftColumn renders the Annotations item as
   canUpdate ? <Button>{label}</Button> : label, where
   label = t('{{count}} annotation', { count }).
2. useAccessReview initialises isAllowed to false while the
   SelfSubjectAccessReview is in flight, so the first render always takes the
   plain-string branch — including for cluster admins.
3. In the ja resource, {{count}} annotation_other is an empty string, so
   children is ''.
4. '' || _.get(route, 'metadata.annotations') yields the annotations object,
   React throws, and the error boundary replaces the page.

Routes with two or more annotations are affected. Routes managed by Argo CD
always are, since they carry both argocd.argoproj.io/tracking-id and
kubectl.kubernetes.io/last-applied-configuration. Routes with exactly one
annotation render fine, because {{count}} annotation_one is absent from the
ja resource and the i18next fallback returns a non-empty string.

The locale resource as actually served

Captured from /locales/resource.json?ns=plugin__networking-console-plugin on a
4.21 cluster:

lng	{{count}} annotation_one	{{count}} annotation_other
en	"{{count}} annotation_one"	"{{count}} annotation_other"
ja	absent	""

That is the whole asymmetry. Under lng=en the value is the key name itself, so
the Annotations row renders the literal string 2 annotation_other — wrong
text, but non-empty, so the || fallback never fires. Under lng=ja the value
is "", the fallback fires, and the page crashes.

The English rendering is worth noting on its own: a details page showing a raw
i18next plural key to the user is already a visible defect, independent of the
crash.

Relation to 00e08ca

00e08ca ("Backport translations from 4.22") repairs those strings and removes
the trigger. It does not remove the fallback that turns a falsy child
into a rendered object, so any future empty or missing translation reintroduces
the same crash, in DetailsItem for every caller — not just Annotations.

The change

-  const value: ReactNode = children || _.get(obj, path, defaultValue);
+  const value: ReactNode = children ?? _.get(obj, path, defaultValue);

?? restores the intended contract: use the children when the caller provided
them, and read the resource field only when the caller provided none. A caller
passing a falsy child now renders that falsy child, which React handles, rather
than silently swapping in a value of an unrelated type.

I audited all 68 <DetailsItem> call sites — 49 with children, 19 self-closing.
None relies on a falsy child falling through to the raw field:

• The {cond && ...} patterns in this repo wrap the DetailsItem element
  itself, or sit alongside a sibling element so that children is an array
  (truthy either way).
• UDNLayer2Details / UDNLayer3Details receive subnets / joinSubnets from
  UDNLayerDetails.getList, which always returns an element — <List> when
  there are entries, <MutedText> otherwise.
• IngressStatus passes {ingress?.host} and {ingress?.wildcardPolicy}.
  Both are undefined when absent, which ?? still treats as "no children",
  so those items are unchanged.
• No call site passes a numeric or boolean child.

?? still falls through for the 19 self-closing call sites, so I checked those
too: every one points at a scalar path (metadata.name, spec.host,
spec.tls.termination, .mtu, .role, subnets[i].cidr, and so on). The
object-valued paths in this repo — metadata.labels, metadata.annotations,
metadata.ownerReferences, spec.selector, spec.ports — are all rendered
through element children, which ?? never discards. So after this change there
is no remaining path in the repo by which the raw field can reach React as a
plain object.

Verification

• npm run lint — 0 errors. (The one warning is pre-existing, in
  SyncedEditor.tsx.) Since .eslintrc.json sets prettier/prettier to
  error, this also confirms the formatting is correct.
• npx tsc --noEmit -p tsconfig.json — 32 errors before and after the change,
  unchanged; none in the touched file.
• No translatable strings are added or removed, so npm run i18n produces no
  change to locales/.

On the missing Jira reference

CONTRIBUTING.md asks for OCPBUGS-NNNNN: in the title. I do not have
issue-creation permission in the Red Hat Jira, so I could not file one — hence
NO-JIRA:. If you would like this tracked and backported to the active release
branches, please attach or file an OCPBUGS and I will retitle the PR.

Observed on

OpenShift 4.21.27, networking-console-plugin at release payload 4.21.27,
1629 affected Routes on the cluster where this was found.

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 openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 70b66d61-2a1c-4b70-b377-e2f19dd84e70

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: XIANLIBO
Once this PR has been reviewed and has the lgtm label, please assign ronlavi2412 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:

• OWNERS

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

[openshift-ci]

Hi @XIANLIBO. Thanks for your PR.

I'm waiting for a openshift 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.