Skip to content

Conversation

@rhamilto
Copy link
Member

@rhamilto rhamilto commented Dec 16, 2025

Summary

This PR refactors the operator details display to dynamically update when changing channels in the OperatorHub. Previously, operator details would remain static even when switching channels. Now, most operator metadata fields (see noted exclusions below) respond to channel selection changes.

Changes

New Hook Created

  • useCurrentChannel - Centralized hook that returns the currently selected channel based on URL query parameters, falling back to the default channel or first available channel. This hook eliminates code duplication across all operator detail components.

New Components Created

The following components were extracted to dynamically read from the selected channel (all use useCurrentChannel hook):

  • OperatorCapability - Displays capability level based on current channel
  • OperatorContainerImage - Shows container image from current channel
  • OperatorCreatedAt - Displays creation timestamp for current channel with improved date validation using Number.isNaN(Date.parse(...))
  • OperatorInfrastructureFeatures - Lists infrastructure features from current channel CSV
  • OperatorRepository - Shows repository link from current channel
  • OperatorSupport - Displays support information with marketplace workflow support
  • OperatorValidSubscriptions - Lists valid subscriptions from current channel

Hook Updates

  • useCurrentCSVDescription - Updated to use useSearchParams instead of deprecated getQueryArgument, and now accepts optional selectedChannel parameter

Refactoring in useOperatorCatalogItems

  • Replaced inline rendering of operator details with new dedicated components
  • Details now automatically update when the channel parameter changes
  • Removed unused imports (PlainList, Timestamp, ExternalLink, CapabilityLevel, getSupportWorkflowUrl)

Infrastructure Features Calculation

  • Infrastructure features are now computed dynamically using useMemo based on current channel's CSV annotations in both:
    • OperatorDescription component - for description section
    • OperatorHubItemDetails component - for details display
  • Removed infraFeatures prop from OperatorDescription component

Testing

  • Verify operator details update when changing channels in OperatorHub
  • Confirm all metadata fields (capability, container image, created date, etc.) reflect selected channel
  • Test with operators having multiple channels with different metadata
  • Verify date validation works correctly for invalid date strings

Note

  • source, provider, displayName, and keywords operator metadata fields were intentionally excluded from this fix since they are less likely to change when the channel changes.

🤖 Generated with Claude Code

@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. 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 Dec 16, 2025
@openshift-ci-robot
Copy link
Contributor

@rhamilto: This pull request references Jira Issue OCPBUGS-69387, which is invalid:

  • expected the bug to target the "4.22.0" version, but no target version was set

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:

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
Copy link

coderabbitai bot commented Dec 16, 2025

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds seven new operator-hub React components, a new useCurrentChannel hook, updates useCurrentCSVDescription to accept selectedChannel and use useSearchParams, and moves infrastructure-feature computation into OperatorDescription and OperatorHubItemDetails internals.

Changes

Cohort / File(s) Summary
New operator-hub components
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx, frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx
Seven new exported React functional components. Each takes a packageManifest: PackageManifestKind, uses useCurrentChannel to read current CSV annotations, renders annotation-derived values (with fallbacks), and adds local prop types or exported prop types as noted. Some components call helpers (e.g., getInfrastructureFeatures, getValidSubscription, getSupportWorkflowUrl) and log onError.
New hook
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx
New useCurrentChannel(packageManifest: PackageManifestKind) hook that reads channel from URL via useSearchParams, falls back to packageManifest defaults, and returns the matching channel object or undefined.
Hook signature update
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx
useCurrentCSVDescription signature changed to accept optional selectedChannel; URL query reading switched from getQueryArgument to useSearchParams; resolution order is selectedChannel → URL channel → package defaultChannel.
Operator item details refactor
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx
Removed infraFeatures from OperatorDescriptionProps and OperatorDescription props. Added getInfrastructureFeatures import and moved infra feature computation into OperatorDescription and OperatorHubItemDetails via useMemo, using cluster flags and onError logging.
Integration update
frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx
Replaced inline UI usage with the seven new components (Capability, ContainerImage, CreatedAt, InfrastructureFeatures, Repository, Support, ValidSubscriptions). Removed passing infraFeatures downstream and updated item details/metadata wiring to use the new modular components.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to URL resolution and precedence in useCurrentChannel and updated useCurrentCSVDescription.
  • Verify useMemo dependency arrays and onError logging in operator-hub-item-details.tsx.
  • Confirm new components consistently access annotations, handle undefined values, and export types/imports correctly.
  • Check useOperatorCatalogItems.tsx integration for remaining references to removed props and correct imports/exports.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@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 Dec 16, 2025
@openshift-ci openshift-ci bot requested review from jhadvig and sg00dwin December 16, 2025 19:25
@openshift-ci openshift-ci bot added component/olm Related to OLM approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 16, 2025
Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

@rhamilto Other than de-duping some of the channel selection logic that we talked about, looks good! Let me know when you update and I can give it the final lgtm tag.

@rhamilto rhamilto changed the title [WIP] OCPBUGS-69387: update operator details when changing channels OCPBUGS-69387: update operator details when changing channels Dec 16, 2025
@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 Dec 16, 2025
@rhamilto
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

✅ Actions performed

Review triggered.

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 19b01f0 and 08657c3.

📒 Files selected for processing (11)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (2 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx
🧬 Code graph analysis (8)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (2)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/public/components/utils/tile-view-page.jsx (1)
  • searchParams (466-466)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (3)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)
  • CapabilityLevel (58-86)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (7)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (1)
  • OperatorCapability (6-11)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1)
  • OperatorInfrastructureFeatures (8-30)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1)
  • OperatorValidSubscriptions (8-24)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1)
  • OperatorRepository (6-11)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1)
  • OperatorContainerImage (5-12)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1)
  • OperatorCreatedAt (6-15)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1)
  • OperatorSupport (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (4)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/index.ts (1)
  • CSVAnnotations (139-166)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (1)
  • getValidSubscription (205-208)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (1)
  • getInfrastructureFeatures (216-275)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
🔇 Additional comments (12)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx (1)

1-20: LGTM! Well-structured refactor with proper fallback logic.

The hook correctly prioritizes channel selection (explicit parameter → URL query → default → first available) and handles edge cases with appropriate optional chaining.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1)

1-15: LGTM! Clean component with proper null handling.

The component correctly uses optional chaining to handle undefined values and follows the established pattern for operator metadata components.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1)

1-26: LGTM! Proper prioritization and null handling.

The component correctly prioritizes the support workflow URL over plain support text and handles all undefined cases appropriately.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1)

1-28: LGTM! Proper error handling and null safety.

The component correctly handles parsing errors with informative warnings and safely renders subscriptions or a fallback dash.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1)

1-16: LGTM! Simple and effective.

The component correctly extracts and displays the container image with appropriate fallback handling.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1)

1-37: LGTM! Consistent error handling and proper cluster flag integration.

The component follows the established pattern and correctly passes cluster capabilities for infrastructure feature computation.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (1)

1-15: LGTM! Clean delegation to CapabilityLevel component.

The component correctly extracts capabilities and delegates rendering to the existing CapabilityLevel component with proper fallback handling.

frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)

1-22: Clean implementation with proper edge case handling.

The hook correctly implements the channel selection logic with a well-defined fallback chain (URL param → defaultChannel → first channel). Optional chaining is used consistently to handle cases where packageManifest or its nested properties might be unavailable.

frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (2)

361-396: Good modular refactoring for dynamic channel-based updates.

The new components correctly receive packageManifest and leverage useCurrentChannel internally to read the selected channel from URL query parameters. This ensures the displayed metadata updates dynamically when users switch channels.

The cluster flags (clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF) are appropriately passed to OperatorInfrastructureFeatures where they're needed for computing infrastructure features.


167-181: Verify filter attribute consistency with displayed values.

getCurrentCSVDescription(pkg) at line 167 returns the default channel's CSV description regardless of the selected channel. The attributes object (used for filtering) will contain infrastructureFeatures and validSubscription based on the default channel, while the rendered details now show the URL-selected channel's values.

If users filter by infrastructure features or valid subscriptions, the filter behavior may not match what's displayed when a non-default channel is selected. This may be acceptable if filters are intended to use default channel data, but worth verifying against expected UX.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (2)

37-42: LGTM!

The imported utilities support the new infrastructure features computation logic.


276-281: LGTM!

The token authentication feature detection correctly uses the locally computed infraFeatures and properly memoizes the result.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)

368-371: Extract repository, support, and validSubscription from the selected channel's CSV annotations instead of the static item prop.

These fields should update when switching channels, consistent with how containerImage, createdAt, and capability are handled. The dedicated components (OperatorRepository, OperatorSupport, OperatorValidSubscriptions) already follow this pattern by extracting these values from the current channel's CSV annotations. Extract them from currentCSVDescription?.annotations similar to the other annotation-based fields to ensure all operator details reflect the selected channel.

🧹 Nitpick comments (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)

260-274: Consider extracting duplicated infraFeatures computation into a custom hook.

The infraFeatures useMemo logic is nearly identical in both OperatorDescription and OperatorHubItemDetails. This could be consolidated into a reusable hook:

// e.g., useInfrastructureFeatures.ts
export const useInfrastructureFeatures = (
  currentCSVDescription: CSVDescription,
  clusterIsAWSSTS: boolean,
  clusterIsAzureWIF: boolean,
  clusterIsGCPWIF: boolean,
  packageName?: string,
) => {
  return React.useMemo(() => {
    const annotations = currentCSVDescription?.annotations ?? {};
    return getInfrastructureFeatures(annotations, {
      clusterIsAWSSTS,
      clusterIsAzureWIF,
      clusterIsGCPWIF,
      onError: (error) => {
        console.warn(
          `Error parsing infrastructure features from PackageManifest "${packageName}":`,
          error,
        );
      },
    });
  }, [currentCSVDescription, clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF, packageName]);
};

Also applies to: 393-407

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 08657c3 and bc0b4ef.

📒 Files selected for processing (11)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (3 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx
🧬 Code graph analysis (6)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (2)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/public/components/utils/tile-view-page.jsx (1)
  • searchParams (466-466)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (4)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/index.ts (1)
  • CSVAnnotations (139-166)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (1)
  • getInfrastructureFeatures (216-275)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (3)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/console-shared/src/components/links/ExternalLink.tsx (1)
  • ExternalLink (29-61)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (4)
  • getInfrastructureFeatures (216-275)
  • isAWSSTSCluster (43-53)
  • isAzureWIFCluster (55-65)
  • isGCPWIFCluster (67-77)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
🔇 Additional comments (11)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1)

6-11: LGTM!

The component correctly uses useCurrentChannel to derive the current CSV description and safely extracts the repository annotation. The conditional rendering appropriately handles both present and absent repository values.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1)

6-15: LGTM!

The component correctly validates dates using !Number.isNaN(Date.parse(createdAt)), which properly handles epoch time (Unix time 0) that would be falsy but valid. The fallback logic appropriately renders either the raw createdAt value or a dash when the date is invalid or missing.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1)

8-30: LGTM!

The component correctly derives infrastructure features from the current channel's CSV annotations using getInfrastructureFeatures. The error handling appropriately logs warnings with the package name for debugging, and the rendering logic properly handles both present and absent feature lists.

frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx (1)

4-15: LGTM!

The hook successfully migrates from the deprecated getQueryArgument to useSearchParams from react-router-dom-v5-compat. The channel resolution logic correctly prioritizes the provided selectedChannel parameter, then the URL query parameter, and finally the package manifest's default channel, maintaining backward compatibility while adding new functionality.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1)

5-12: LGTM!

The component correctly uses useCurrentChannel to derive the current CSV description and safely extracts the container image annotation. The rendering logic appropriately handles both present and absent values.

frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)

8-22: LGTM!

The hook correctly implements channel resolution with a sensible fallback chain: URL query parameter → default channel → first available channel. The use of useSearchParams from react-router-dom-v5-compat is appropriate, and the implementation includes clear documentation describing its behavior.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1)

8-24: LGTM!

The component correctly derives valid subscriptions from the current channel's CSV annotations using getValidSubscription. The error handling appropriately logs warnings with the package name, and the rendering logic properly handles both present and absent subscription lists.

frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (3)

11-34: LGTM!

The imports correctly include all the new modular operator-hub components (OperatorCapability, OperatorContainerImage, OperatorCreatedAt, OperatorInfrastructureFeatures, OperatorRepository, OperatorSupport, OperatorValidSubscriptions) that are used later in the file to replace inline rendering logic.


362-396: LGTM!

The details properties have been successfully refactored to use modular operator-hub components. Each component receives packageManifest={pkg} and will dynamically update when the channel changes, achieving the PR's objective. The OperatorInfrastructureFeatures component correctly receives the additional cluster capability flags (clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF) required for feature determination.


401-413: LGTM!

The OperatorDescription component call correctly removes the infraFeatures prop, aligning with the PR's objective to internalize infrastructure features computation. The component now receives packageManifest and cluster capability flags, allowing it to derive infrastructure features internally based on the current channel's CSV annotations.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)

491-503: LGTM on the OperatorDescription integration.

The component now correctly receives cluster capability flags (clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF) and computes infrastructure features internally. The prop interface is cleaner with the infraFeatures prop removed.

@rhamilto
Copy link
Member Author

/retest

@rhamilto
Copy link
Member Author

/cherrypick release-4.21

@openshift-cherrypick-robot

@rhamilto: once the present PR merges, I will cherry-pick it on top of release-4.21 in a new PR and assign it to you.

Details

In response to this:

/cherrypick release-4.21

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.

@rhamilto
Copy link
Member Author

@rhamilto Other than de-duping some of the channel selection logic that we talked about, looks good! Let me know when you update and I can give it the final lgtm tag.

Done. And I think Claude, CodeRabbit, and I are all in agreement now. ;-)

@rhamilto
Copy link
Member Author

/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Dec 17, 2025
@rhamilto
Copy link
Member 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 Dec 17, 2025
@openshift-ci-robot
Copy link
Contributor

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

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.22.0) matches configured target version for branch (4.22.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @XiyunZhao

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.

@openshift-ci openshift-ci bot requested a review from XiyunZhao December 17, 2025 13:21
Copy link
Member

@TheRealJon TheRealJon left a comment

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 Dec 17, 2025
@rhamilto
Copy link
Member Author

@XiyunZhao, can you please verify?
/test okd-scos-images

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 19, 2025
Copy link
Member

@TheRealJon TheRealJon left a comment

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 Dec 19, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (2)

258-274: Consider removing packageManifest from the dependency array.

The packageManifest is included in the useMemo dependencies but is only used in the error handler (line 269). This could cause unnecessary recomputations when packageManifest changes. Consider using packageManifest?.metadata?.name in the dependency array or accepting that it's only used for logging.

However, this is a minor optimization as packageManifest changes are likely infrequent.

🔎 Optional optimization
-  }, [currentCSVDescription, clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF, packageManifest]);
+  }, [currentCSVDescription, clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF]);

258-274: Consider extracting the duplicated infraFeatures computation into a shared hook.

The infraFeatures computation logic (lines 258-274 and 392-407) is duplicated between OperatorDescription and OperatorHubItemDetails. This violates the DRY principle and increases maintenance burden when the logic needs to change.

Consider creating a custom hook like useInfrastructureFeatures that accepts the current CSV description and cluster flags, returning the computed infrastructure features.

🔎 Example refactor

Create a new hook in a shared location:

export const useInfrastructureFeatures = (
  currentCSVDescription,
  clusterIsAWSSTS,
  clusterIsAzureWIF,
  clusterIsGCPWIF,
  packageManifestName,
) => {
  return React.useMemo(() => {
    const currentCSVAnnotations = currentCSVDescription?.annotations ?? {};
    return getInfrastructureFeatures(currentCSVAnnotations, {
      clusterIsAWSSTS,
      clusterIsAzureWIF,
      clusterIsGCPWIF,
      onError: (error) => {
        console.warn(
          `Error parsing infrastructure features from PackageManifest "${packageManifestName}":`,
          error,
        );
      },
    });
  }, [currentCSVDescription, clusterIsAWSSTS, clusterIsAzureWIF, clusterIsGCPWIF]);
};

Then use it in both components:

const infraFeatures = useInfrastructureFeatures(
  currentCSVDescription,
  clusterIsAWSSTS,
  clusterIsAzureWIF,
  clusterIsGCPWIF,
  packageManifest?.metadata?.name,
);

Also applies to: 392-407

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between bc0b4ef and 0effcdc.

📒 Files selected for processing (11)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (3 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1 hunks)
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-capability.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentCSVDescription.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx
  • frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx
  • frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx
🧬 Code graph analysis (7)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (4)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/index.ts (1)
  • CSVAnnotations (141-168)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (1)
  • getValidSubscription (205-208)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (4)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/index.ts (1)
  • CSVAnnotations (141-168)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (1)
  • getInfrastructureFeatures (216-275)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (3)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/console-shared/src/components/links/ExternalLink.tsx (1)
  • ExternalLink (29-61)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (2)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)
  • useCurrentChannel (8-22)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (2)
frontend/packages/operator-lifecycle-manager/src/types.ts (1)
  • PackageManifestKind (256-283)
frontend/public/components/utils/tile-view-page.jsx (1)
  • searchParams (466-466)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (1)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (4)
  • getInfrastructureFeatures (216-275)
  • isAWSSTSCluster (43-53)
  • isAzureWIFCluster (55-65)
  • isGCPWIFCluster (67-77)
🔇 Additional comments (12)
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-repository.tsx (1)

1-15: LGTM!

Clean, focused component that follows the established pattern for modular operator-hub components. The implementation correctly uses useCurrentChannel to derive channel-specific data.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-infrastructure-features.tsx (1)

1-37: LGTM!

The component correctly computes infrastructure features from channel annotations with appropriate error handling. The error callback includes the package name which aids debugging.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-created-at.tsx (1)

1-19: LGTM!

The date validation correctly handles edge cases including Unix epoch. The graceful fallback to display the raw string or dash provides good UX for invalid dates.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-support.tsx (1)

1-26: LGTM!

The component correctly extracts support workflow annotations and renders the appropriate link with proper internationalization.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-container-image.tsx (1)

1-16: LGTM!

Simple, focused component following the established pattern.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-valid-subscriptions.tsx (1)

1-28: LGTM!

Consistent implementation with proper error handling that follows the established pattern of the other modular components.

frontend/packages/operator-lifecycle-manager/src/hooks/useCurrentChannel.tsx (1)

1-22: LGTM!

Well-designed centralized hook with a clear fallback chain (URL param → default channel → first channel). The JSDoc comment clearly documents the behavior. Good use of optional chaining for null safety.

frontend/packages/operator-lifecycle-manager/src/hooks/useOperatorCatalogItems.tsx (1)

352-412: No action needed. The channel selection properly syncs to URL parameters.

The OperatorChannelSelect component includes a useEffect hook that calls setQueryArgument('channel', selectedChannel) whenever the selected channel changes (line 57-65 of operator-channel-version-select.tsx). This ensures the channel query parameter is always updated in the URL when users change the selection. The modular components using useCurrentChannel correctly read from these updated URL parameters via searchParams.get('channel'), so they will reflect channel changes as expected.

frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-item-details.tsx (4)

37-42: LGTM!

The new imports support the infrastructure feature computation logic and align with the centralized utility functions in operator-hub-utils.ts.


387-390: LGTM!

The cluster type detection logic correctly uses the imported utility functions to determine the cluster configuration for infrastructure feature computation.


409-409: LGTM! Previous issue resolved.

The infrastructure features are now correctly derived from the dynamically computed infraFeatures (based on the current channel's CSV annotations) rather than the static item.infraFeatures. This ensures the UI updates properly when users switch channels.

Based on past review comments, this addresses the concern about static infrastructure feature display.

Also applies to: 458-460


383-383: LGTM!

The date validation using Date.parse before rendering the Timestamp component is a good practice that prevents errors from invalid date strings, aligning with the PR's improved date validation objective.

@rhamilto
Copy link
Member Author

/retest

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2025
@sg00dwin
Copy link
Member

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 22, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhamilto, sg00dwin, TheRealJon

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

@rhamilto
Copy link
Member Author

/retest

@rhamilto
Copy link
Member Author

/test okd-scos-images

2 similar comments
@rhamilto
Copy link
Member Author

/test okd-scos-images

@rhamilto
Copy link
Member Author

/test okd-scos-images

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 23, 2025

@rhamilto: 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/okd-scos-images 120b7fb link true /test okd-scos-images

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

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. component/olm Related to OLM jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants