Skip to content

[release-4.16] OCPBUGS-88027: Fix JSON annotation parsing to prevent OperatorHub crash#16588

Open
cardil wants to merge 1 commit into
openshift:release-4.16from
cardil:OCPBUGS-88027
Open

[release-4.16] OCPBUGS-88027: Fix JSON annotation parsing to prevent OperatorHub crash#16588
cardil wants to merge 1 commit into
openshift:release-4.16from
cardil:OCPBUGS-88027

Conversation

@cardil

@cardil cardil commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Manual backport of #14260 (OCPBUGS-35744) to release-4.16.

Problem

OperatorHub page crashes with TypeError: V.reduce is not a function when a PackageManifest has malformed JSON in operators.openshift.io/infrastructure-features or operators.openshift.io/valid-subscription annotations. The parsed value is a non-array truthy value, and the ?? [] fallback doesn't catch it.

Fix (minimal, EUS-appropriate)

  • Add parseJSONArrayAnnotation() helper that validates the parsed result is actually a string[] before returning
  • Switch annotation parsing call sites to use the validated helper
  • Add optional chaining to installModesFor() for missing status.channels
  • Harden getClusterServiceVersionPlugins() and getInternalObjects() similarly
  • 15 unit tests covering all edge cases

This is a surgical minimal fix rather than a full port of the original refactor (which renamed types, reorganized code, etc.), appropriate for an EUS branch.

/assign cardil

Assisted-by: 🤖 Claude Opus/Sonnet 4.6

Summary by CodeRabbit

  • Bug Fixes

    • Prevented crashes when operator metadata/annotations are missing and improved handling of malformed annotation data, with clearer warnings.
  • Improvements

    • More robust parsing of list-style annotations so features, subscriptions, plugins, and internal object lists are handled consistently.
  • Tests

    • Added tests covering JSON and JSON-array annotation parsing, fallbacks, and error reporting.

Harden annotation parsing for OperatorHub page to prevent TypeError
when PackageManifest annotations contain malformed or non-array JSON.

- Add parseJSONArrayAnnotation() with type validation
- Guard .reduce()/.map() calls on parsed annotation data
- Add optional chaining to installModesFor() for missing status/channels
- Add unit tests for annotation parsing edge cases

Backport of fix from PR openshift#14260 (OCPBUGS-35744) adapted for 4.16.

Assisted-by: 🤖 Claude Opus/Sonnet 4.6
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@cardil: This pull request references Jira Issue OCPBUGS-88027, which is invalid:

  • expected the bug to target the "4.16.z" version, but no target version was set
  • release note text must be set and not match the template OR release note type must be set to "Release Note Not Required". For more information you can reference the OpenShift Bug Process.
  • expected dependent Jira Issue OCPBUGS-35744 to target a version in 4.17.0, 4.17.z, but it targets "4.18.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Manual backport of #14260 (OCPBUGS-35744) to release-4.16.

Problem

OperatorHub page crashes with TypeError: V.reduce is not a function when a PackageManifest has malformed JSON in operators.openshift.io/infrastructure-features or operators.openshift.io/valid-subscription annotations. The parsed value is a non-array truthy value, and the ?? [] fallback doesn't catch it.

Fix (minimal, EUS-appropriate)

  • Add parseJSONArrayAnnotation() helper that validates the parsed result is actually a string[] before returning
  • Switch annotation parsing call sites to use the validated helper
  • Add optional chaining to installModesFor() for missing status.channels
  • Harden getClusterServiceVersionPlugins() and getInternalObjects() similarly
  • 15 unit tests covering all edge cases

This is a surgical minimal fix rather than a full port of the original refactor (which renamed types, reorganized code, etc.), appropriate for an EUS branch.

/assign cardil

Assisted-by: 🤖 Claude Opus/Sonnet 4.6

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.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2026
@openshift-ci

openshift-ci Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds parseJSONArrayAnnotation (validates JSON arrays of strings), tests it, migrates several operator-lifecycle-manager call sites to use it, and applies a safe optional-chaining fix when reading install modes.

Changes

JSON Array Annotation Parsing

Layer / File(s) Summary
parseJSONArrayAnnotation utility and tests
frontend/packages/console-shared/src/utils/annotations.ts, frontend/packages/console-shared/src/utils/__tests__/annotations.spec.ts
New parseJSONArrayAnnotation validates parsed JSON is an array of strings and returns [] for missing/invalid values; tests cover valid parsing, missing keys, invalid JSON, type/element mismatches, and onError invocation.
Operator hub parsing call-sites
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-page.tsx
Swaps parseJSONAnnotation -> parseJSONArrayAnnotation for infrastructureFeatures and validSubscription, supplying per-package onError fallbacks and removing local ?? [].
Operator utils migration
frontend/packages/operator-lifecycle-manager/src/utils.ts
getClusterServiceVersionPlugins and getInternalObjects now call parseJSONArrayAnnotation and return its result directly (removed ?? []).
Safe navigation for installModes retrieval
frontend/packages/operator-lifecycle-manager/src/components/index.tsx
installModesFor uses optional chaining and nullish coalescing when accessing channel currentCSVDesc.installModes.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete; it omits several required template sections such as Analysis/Root cause, Screenshots, Browser conformance, and Reviewers sections are not filled properly. Complete the missing template sections, especially Analysis/Root cause, Test cases, Browser conformance, and proper Reviewers/assignees formatting per the template requirements.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding JSON annotation parsing validation to prevent OperatorHub crashes, with the Jira issue prefix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed This PR contains no Ginkgo tests. The only test file added is a Jest test (annotations.spec.ts) with stable, deterministic test names containing no dynamic values, timestamps, UUIDs, or generated i...
Test Structure And Quality ✅ Passed This PR modifies only frontend TypeScript/Jest code. The custom check requires reviewing Ginkgo test code (Go testing framework), which is not present in this PR. Check is not applicable.
Microshift Test Compatibility ✅ Passed PR contains no Ginkgo e2e tests, only Jest unit tests and frontend code changes. MicroShift compatibility check applies only to Ginkgo e2e tests.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No Ginkgo e2e tests present in this PR. All changes are TypeScript/JavaScript with Jest unit tests for annotation parsing utilities, which do not apply to SNO compatibility concerns.
Topology-Aware Scheduling Compatibility ✅ Passed This PR modifies only frontend console code (React components and utilities) with no deployment manifests, operators, controllers, or scheduling constraints. Not applicable to topology-aware schedu...
Ote Binary Stdout Contract ✅ Passed OTE Binary Stdout Contract applies only to Go/Rust binaries with process-level code violations. This PR modifies only TypeScript/React frontend code with Jest tests—not applicable to check.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR adds no Ginkgo e2e tests. All changes are TypeScript/JavaScript frontend code (Jest unit tests and React components), which fall outside the scope of the IPv6/disconnected network compatibi...
No-Weak-Crypto ✅ Passed No weak cryptography (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), crypto imports, custom crypto implementations, or non-constant-time secret comparisons found in PR files.
Container-Privileges ✅ Passed This PR modifies only TypeScript/JavaScript frontend code (annotation parsing utilities and operator hub components) with no container manifests, Dockerfiles, or Kubernetes security configurations....
No-Sensitive-Data-In-Logs ✅ Passed All logging statements log only non-sensitive metadata: annotation keys, JSON parse error messages (which exclude input), and operator names. No passwords, tokens, PII, or sensitive data are expose...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci Bot added component/olm Related to OLM component/shared Related to console-shared labels Jun 10, 2026
@cardil

cardil commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@cardil: This pull request references Jira Issue OCPBUGS-88027, which is invalid:

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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.

@cardil

cardil commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 10, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@cardil: This pull request references Jira Issue OCPBUGS-88027, which is valid. The bug has been moved to the POST state.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.16.z) matches configured target version for branch (4.16.z)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note text is set and does not match the template
  • dependent bug Jira Issue OCPBUGS-51277 is in the state Closed (Done-Errata), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
  • dependent Jira Issue OCPBUGS-51277 targets the "4.17.z" version, which is one of the valid target versions: 4.17.0, 4.17.z
  • bug has dependents
Details

In response to this:

/jira refresh

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.

@cardil cardil marked this pull request as ready for review June 10, 2026 13:43
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 10, 2026
@openshift-ci openshift-ci Bot requested review from TheRealJon and spadgett June 10, 2026 13:44
@cardil

cardil commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@cardil: This pull request references Jira Issue OCPBUGS-88027, which is valid.

7 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.16.z) matches configured target version for branch (4.16.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
  • release note text is set and does not match the template
  • dependent bug Jira Issue OCPBUGS-51277 is in the state Closed (Done-Errata), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
  • dependent Jira Issue OCPBUGS-51277 targets the "4.17.z" version, which is one of the valid target versions: 4.17.0, 4.17.z
  • bug has dependents
Details

In response to this:

Manual backport of #14260 (OCPBUGS-35744) to release-4.16.

Problem

OperatorHub page crashes with TypeError: V.reduce is not a function when a PackageManifest has malformed JSON in operators.openshift.io/infrastructure-features or operators.openshift.io/valid-subscription annotations. The parsed value is a non-array truthy value, and the ?? [] fallback doesn't catch it.

Fix (minimal, EUS-appropriate)

  • Add parseJSONArrayAnnotation() helper that validates the parsed result is actually a string[] before returning
  • Switch annotation parsing call sites to use the validated helper
  • Add optional chaining to installModesFor() for missing status.channels
  • Harden getClusterServiceVersionPlugins() and getInternalObjects() similarly
  • 15 unit tests covering all edge cases

This is a surgical minimal fix rather than a full port of the original refactor (which renamed types, reorganized code, etc.), appropriate for an EUS branch.

/assign cardil

Assisted-by: 🤖 Claude Opus/Sonnet 4.6

Summary by CodeRabbit

  • Bug Fixes

  • Fixed potential crashes when operator metadata is incomplete or missing.

  • Improved error handling and logging for invalid operator configurations.

  • Tests

  • Added comprehensive test coverage for annotation parsing utilities.

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.

@cardil

cardil commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Reproduction and fix verification evidence has been added to the linked JIRA OCPBUGS-88027.

Summary: Tested on a 4.16 nightly cluster with a CatalogSource containing malformed CSV annotations. Stock console crashes with TypeError: V.reduce is not a function on the OperatorHub page. Patched console loads successfully with 0 errors — malformed annotations are logged as warnings and gracefully skipped.

cc @TheRealJon @jhadvig

@RadekManak

Copy link
Copy Markdown

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@cardil

cardil commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-gcp-console

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/packages/console-shared/src/utils/annotations.ts (1)

31-33: ⚡ Quick win

Consider improving the error message for array validation failures.

When parsed is an array with non-string elements (e.g., [1, 2, 3]), typeof parsed reports "object" rather than indicating it's an array with invalid element types. This could make debugging slightly harder.

📝 Suggested improvement for clarity
     const error = new Error(
-      `Expected annotation "${annotationKey}" to be an array of strings, got ${typeof parsed}`,
+      `Expected annotation "${annotationKey}" to be an array of strings, got ${
+        Array.isArray(parsed) ? 'array with non-string elements' : typeof parsed
+      }`,
     );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/packages/console-shared/src/utils/annotations.ts` around lines 31 -
33, The error thrown in annotations.ts when validating annotation values uses
typeof parsed which reports "object" for arrays and obscures cases like arrays
with non-string elements; update the Error message created where annotationKey
and parsed are used to detect and report "array with non-string elements" (or
include the element types/preview of parsed) so failures show that parsed is an
array but contains invalid element types (reference the variable names
annotationKey and parsed and the Error construction site).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@frontend/packages/console-shared/src/utils/annotations.ts`:
- Around line 31-33: The error thrown in annotations.ts when validating
annotation values uses typeof parsed which reports "object" for arrays and
obscures cases like arrays with non-string elements; update the Error message
created where annotationKey and parsed are used to detect and report "array with
non-string elements" (or include the element types/preview of parsed) so
failures show that parsed is an array but contains invalid element types
(reference the variable names annotationKey and parsed and the Error
construction site).

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 638407a6-2113-4be8-90a4-5d994e1f6e08

📥 Commits

Reviewing files that changed from the base of the PR and between 074380b and 3c095f0.

📒 Files selected for processing (5)
  • frontend/packages/console-shared/src/utils/__tests__/annotations.spec.ts
  • frontend/packages/console-shared/src/utils/annotations.ts
  • frontend/packages/operator-lifecycle-manager/src/components/index.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-page.tsx
  • frontend/packages/operator-lifecycle-manager/src/utils.ts

@fsgreco

fsgreco commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

/retest

@fsgreco fsgreco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
@openshift-ci

openshift-ci Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, fsgreco, logonoff

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 12, 2026
@fsgreco

fsgreco commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

/retest

@cardil

cardil commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

The e2e-gcp-console failure is unrelated to this PR's changes. Our fix works correctly — the OperatorHub page loads successfully with 376 operators and no crash (see failure screenshot).

Root cause of e2e failure: The edit-default-sources.cy.ts test disables and re-enables redhat-operators CatalogSource right before the operator-hub.cy.ts test runs. The catalog doesn't have time to fully re-populate, so the "Red Hat" source filter (catalogSourceDisplayName-red-hat) isn't available yet when the next test looks for it.

The same test fails on other unrelated release-4.16 PRs: #16580, #16599 — confirming it's a pre-existing flake, not caused by this change.

@cardil

cardil commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Root cause of e2e failure [..]

There is a PR to fix this flakiness: #16608

@cardil

cardil commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

/cherrypick release-4.14
/cherrypick release-4.12

@openshift-cherrypick-robot

Copy link
Copy Markdown

@cardil: once the present PR merges, I will cherry-pick it on top of release-4.12, release-4.14 in new PRs and assign them to you.

Details

In response to this:

/cherrypick release-4.14
/cherrypick release-4.12

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.

@cardil

cardil commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

/test e2e-gcp-console

1 similar comment
@cajieh

cajieh commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

/test e2e-gcp-console

@cajieh

cajieh commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

/label backport-risk-assessed
/verified by CI

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 15, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@cajieh: This PR has been marked as verified by CI.

Details

In response to this:

/label backport-risk-assessed
/verified by CI

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.

@openshift-ci openshift-ci Bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Jun 15, 2026
@cajieh

cajieh commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

/test e2e-gcp-console

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

/retest-required

Remaining retests: 0 against base HEAD 074380b and 2 for PR HEAD 3c095f0 in total

@cajieh

cajieh commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

/test e2e-gcp-console

@cardil

cardil commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

/retest-required

@openshift-ci

openshift-ci Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

@cardil: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gcp-console 3c095f0 link true /test e2e-gcp-console

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. component/olm Related to OLM component/shared Related to console-shared jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants