CONSOLE-4106: Add ClusterExtension creation page #15741
Conversation
Implements an application-level TECH_PREVIEW feature flag in the console-app package that is set based on the SERVER_FLAGS.techPreview value from the backend. Frontend changes: - Add FLAG_TECH_PREVIEW constant to console-app/src/consts.ts - Create useTechPreviewFlagProvider hook to set flag from server - Register hook provider in console-app console-extensions.json - Export provider in console-app package.json - Add techPreview type to SERVER_FLAGS interface Backend changes: - Add TechPreview field to Server struct - Add techPreview to jsGlobals struct - Pass techPreview value to frontend in index handler 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Adds a new extension point allowing plugins to contribute toolbar items to the Catalog view for specific catalog types. This enables plugins to provide custom UI controls (like toggles, filters, or actions) that appear in the catalog toolbar alongside the existing search, sort, and grouping controls. Changes include: - New CatalogToolbarItem extension type in console-dynamic-plugin-sdk - Extension consumption in console-shared catalog components - Unit tests for useCatalogExtensions hook covering toolbar item filtering - React Testing Library tests for CatalogToolbar component rendering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Adds a toolbar toggle component that allows users to enable/disable OLMv1 operators in the developer catalog. The toggle uses user settings to persist state and includes a tech preview label with informational popover. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This commit adds a user-controlled feature flag for OLMv1 catalog that: - Creates OLMV1_ENABLED flag synchronized with console.olmv1.enabled user setting - Adds User Preferences tab under "OLMV1 Catalog" group for toggling OLMv1 - Integrates OLMv1 into dev-catalog instead of standalone /olmv1/catalog page - Disables legacy operator catalog extensions when OLMV1_ENABLED is true - Fixes loading state bug in useCatalogItems hook - Fixes type mismatch (operator vs OLMv1CatalogItem) in catalog items - Adds defensive null checks in catalog shared components Both the toolbar toggle and user preferences checkbox control the same setting, ensuring a synchronized experience across the console. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This extension allows plugins to contribute alerts to the Catalog view that appear just under the catalog toolbar. The extension has the same properties as CatalogToolbarItem (catalogId and type) and follows the same filtering behavior. Changes: - Add CatalogAlert extension type definition and type guard - Update useCatalogExtensions hook to resolve alert extensions - Pass alert extensions through CatalogService and CatalogController - Render alerts in CatalogController between PageHeading and catalog content - Add comprehensive test coverage for alert extension filtering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Implements an example usage of the new CatalogAlert extension point by creating an OLMv1Alert component that displays information about OLM version 1 in the operator catalog. Changes: - Add OLMv1Alert component displaying OLMv1 information and documentation link - Register OLMv1Alert as console.catalog/alert extension for operator catalog - Expose OLMv1Alert module in plugin configuration - Extension is gated by TECH_PREVIEW and OLMV1_ENABLED feature flags 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace dynamic extension points with hard-coded components gated by feature flags for OLMv1 tech preview. This simplifies the implementation while maintaining the same functionality until OLMv1 reaches GA. Changes: - Remove CatalogToolbarItem and CatalogAlert extension type definitions - Remove toolbar and alert extension resolution from useCatalogExtensions - Hard-code OLMv1ToolbarToggle in CatalogToolbar (lazy loaded) - Hard-code OLMv1Alert in CatalogController - Gate both components with TECH_PREVIEW flag and operator catalog type - OLMv1Alert also requires OLMV1_ENABLED flag - Add TODO comments to remove when OLMv1 GAs - Remove extension registrations from OLMv1 package - Update all test files to remove extension-related tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Changes based on code review feedback: - Rename OLMv1ToolbarToggle to OLMv1Switch for better naming clarity - Simplify CatalogToolbar lazy loading import - Update catalog hooks and utilities based on feedback - Refine flag provider implementation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
openshift-ci-robot
added
the
jira/valid-reference
|
@TheRealJon: This pull request references CONSOLE-4106 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. 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. |
WalkthroughThis change introduces tech preview flag infrastructure, replaces the extension catalog system (IndexedDB-based) with OLMv1 catalog (API-based), adds OLMv1-specific UI components and feature flags, updates console extensions and localization, and refactors related catalog utilities across backend and frontend packages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TheRealJon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
component/core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
frontend/packages/console-shared/src/components/catalog/utils/category-utils.ts (1)
12-13: Optional chaining improves defensive handling.The optional chaining safely handles potentially null/undefined
categoryvalues at runtime. However, the function signature declarescategory: CatalogCategory(non-nullable). Ifcategorycan truly be null/undefined, consider updating the type signature tocategory: CatalogCategory | null | undefinedfor consistency.frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogToolbar.tsx (1)
106-113: Consider a loading skeleton for better UX.The
Suspensefallback is set tonull, which could cause a brief layout shift when theOLMv1Switchcomponent loads. Consider using a loading skeleton or placeholder to maintain consistent spacing, especially if the component bundle is large.Apply this diff to add a minimal placeholder:
{showOLMv1Toggle && ( <FlexItem> - <Suspense fallback={null}> + <Suspense fallback={<div style={{ width: '150px' }} />}> <OLMv1Switch /> </Suspense> </FlexItem>frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx (1)
36-36: Type safety concern with props spreading.The
as anycast bypasses TypeScript's type checking. While this is a common pattern with theCreateYAMLcomponent, consider properly typing the props or creating a wrapper type to maintain type safety.frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx (1)
38-42: Avoid repeated array spreading in infrastructure feature normalization
normalizedInfrastructureFeaturesbuilds a new array on every iteration ([...acc, ...]), which is unnecessary overhead in a hot normalization path. You can keep the same behavior but reduce allocations by mutating the accumulator:- const normalizedInfrastructureFeatures = infrastructureFeatures?.reduce( - (acc, feature) => - infrastructureFeatureMap[feature] ? [...acc, infrastructureFeatureMap[feature]] : acc, - [], - ); + const normalizedInfrastructureFeatures = infrastructureFeatures?.reduce<string[]>( + (acc, feature) => { + const mapped = infrastructureFeatureMap[feature]; + if (mapped) { + acc.push(mapped); + } + return acc; + }, + [], + );This keeps the API identical but scales better with larger catalogs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (58)
cmd/bridge/main.go(1 hunks)frontend/@types/console/index.d.ts(1 hunks)frontend/packages/console-app/console-extensions.json(2 hunks)frontend/packages/console-app/package.json(1 hunks)frontend/packages/console-app/src/consts.ts(1 hunks)frontend/packages/console-app/src/hooks/useTechPreviewFlagProvider.ts(1 hunks)frontend/packages/console-shared/src/components/catalog/CatalogController.tsx(3 hunks)frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogCategories.tsx(1 hunks)frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogToolbar.tsx(4 hunks)frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogView.tsx(3 hunks)frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx(3 hunks)frontend/packages/console-shared/src/components/catalog/utils/category-utils.ts(1 hunks)frontend/packages/console-shared/src/components/query-browser/QueryBrowser.tsx(1 hunks)frontend/packages/console-shared/src/hooks/usePoll.ts(1 hunks)frontend/packages/helm-plugin/src/providers/helm-detection-provider.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/README.md(1 hunks)frontend/packages/operator-lifecycle-manager-v1/console-extensions.json(3 hunks)frontend/packages/operator-lifecycle-manager-v1/locales/en/olm-v1.json(1 hunks)frontend/packages/operator-lifecycle-manager-v1/package.json(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/components/ExtensionCatalog.tsx(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Alert.tsx(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Switch.tsx(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/const.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/contexts/ExtensionCatalogDatabaseContext.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/contexts/types.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/contexts/useExtensionCatalogDatabaseContextValues.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/database/indexeddb.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/database/injest.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/database/jsonl.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/database/types.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/bundles.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/channels.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/metadata.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/packages.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/type-guards.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/types.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/fbc/util.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogCategories.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogCategories.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogItems.ts(0 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useOLMv1FlagProvider.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/models.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/types.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx(3 hunks)frontend/packages/operator-lifecycle-manager-v1/src/utils/filters.ts(0 hunks)frontend/packages/operator-lifecycle-manager/console-extensions.json(2 hunks)frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts(3 hunks)frontend/packages/operator-lifecycle-manager/src/const.ts(1 hunks)frontend/packages/operator-lifecycle-manager/src/utils/filters.ts(1 hunks)frontend/public/co-fetch.ts(1 hunks)frontend/public/components/poll-console-updates.tsx(1 hunks)frontend/public/components/utils/index.tsx(0 hunks)frontend/public/components/utils/url-poll-hook.ts(1 hunks)frontend/public/locales/en/public.json(1 hunks)frontend/public/module/k8s/k8s.ts(1 hunks)pkg/server/server.go(3 hunks)
💤 Files with no reviewable changes (19)
- frontend/packages/operator-lifecycle-manager-v1/src/database/injest.ts
- frontend/packages/operator-lifecycle-manager-v1/src/contexts/types.ts
- frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogCategories.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/type-guards.ts
- frontend/packages/operator-lifecycle-manager-v1/src/components/ExtensionCatalog.tsx
- frontend/public/components/utils/index.tsx
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/metadata.ts
- frontend/packages/operator-lifecycle-manager-v1/src/contexts/useExtensionCatalogDatabaseContextValues.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/util.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/packages.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/channels.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/types.ts
- frontend/packages/operator-lifecycle-manager-v1/src/database/jsonl.ts
- frontend/packages/operator-lifecycle-manager-v1/src/fbc/bundles.ts
- frontend/packages/operator-lifecycle-manager-v1/src/database/types.ts
- frontend/packages/operator-lifecycle-manager-v1/src/contexts/ExtensionCatalogDatabaseContext.ts
- frontend/packages/operator-lifecycle-manager-v1/src/utils/filters.ts
- frontend/packages/operator-lifecycle-manager-v1/src/database/indexeddb.ts
- frontend/packages/operator-lifecycle-manager-v1/src/hooks/useExtensionCatalogItems.ts
🧰 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/console-shared/src/components/catalog/utils/category-utils.tsfrontend/@types/console/index.d.tsfrontend/packages/console-shared/src/components/query-browser/QueryBrowser.tsxfrontend/packages/operator-lifecycle-manager/src/utils/filters.tsfrontend/packages/helm-plugin/src/providers/helm-detection-provider.tsfrontend/public/components/utils/url-poll-hook.tsfrontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Alert.tsxfrontend/packages/operator-lifecycle-manager-v1/src/const.tsfrontend/packages/console-app/package.jsonfrontend/packages/operator-lifecycle-manager-v1/src/models.tsfrontend/packages/operator-lifecycle-manager-v1/README.mdfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsxfrontend/packages/operator-lifecycle-manager-v1/src/hooks/useOLMv1FlagProvider.tsfrontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Switch.tsxfrontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogCategories.tscmd/bridge/main.gofrontend/packages/console-shared/src/components/catalog/catalog-view/CatalogView.tsxfrontend/packages/console-shared/src/components/catalog/CatalogController.tsxfrontend/public/co-fetch.tsfrontend/packages/console-shared/src/components/catalog/catalog-view/CatalogCategories.tsxfrontend/packages/console-app/src/consts.tsfrontend/packages/operator-lifecycle-manager-v1/src/types.tsfrontend/packages/operator-lifecycle-manager-v1/package.jsonpkg/server/server.gofrontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.tsfrontend/public/components/poll-console-updates.tsxfrontend/packages/console-app/src/hooks/useTechPreviewFlagProvider.tsfrontend/packages/operator-lifecycle-manager/console-extensions.jsonfrontend/packages/console-shared/src/components/catalog/catalog-view/CatalogToolbar.tsxfrontend/packages/operator-lifecycle-manager-v1/console-extensions.jsonfrontend/packages/operator-lifecycle-manager-v1/locales/en/olm-v1.jsonfrontend/packages/operator-lifecycle-manager/src/const.tsfrontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.tsfrontend/packages/console-app/console-extensions.jsonfrontend/public/module/k8s/k8s.tsfrontend/public/locales/en/public.jsonfrontend/packages/console-shared/src/hooks/usePoll.tsfrontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx
🪛 Biome (2.1.2)
frontend/packages/console-app/console-extensions.json
[error] 2533-2538: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 2538-2538: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (42)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx (3)
106-113: Good optimization to prevent unnecessary re-renders.The deep equality check ensures state only updates when categories actually change, preventing unnecessary component re-renders.
143-145: Good defensive coding.The check prevents crashes when catalog items have types not registered in
catalogProviderExtensions, improving robustness.
168-171: Review category deduplication approach for data consistency.The code silently deduplicates categories by ID across multiple providers, keeping only the first occurrence. However, verification reveals that multiple category providers can target the same catalog (e.g., OLM and Dev Console both serve "dev-catalog"), and there is no enforced contract that categories with identical IDs have identical properties (label, tags, subcategories).
If different providers contribute categories with the same ID but differing properties, only the first provider's category will be retained—silently discarding data from subsequent providers without warning or merge attempt. For example, two providers generating
{id: "operators", label: "..."}with different tag sets would lose one provider's tags with no indication.Recommendation: Either:
- Document and enforce the architectural constraint that identical category IDs must have identical properties across all providers, or
- Implement conflict detection/merging logic instead of silent deduplication, or
- Namespace categories per provider to avoid global ID collisions.
frontend/public/co-fetch.ts (1)
48-49: LGTM: Correct handling of HTTP 304 Not Modified.Treating 304 as a successful response is correct per HTTP semantics—it indicates the cached resource is still valid. This allows the caller to use the cached version without triggering error handling.
frontend/packages/helm-plugin/src/providers/helm-detection-provider.ts (1)
7-7: LGTM: Import consolidation to shared utilities.Moving
usePollto the shared package improves code organization and reusability across the codebase.frontend/packages/console-app/src/consts.ts (1)
11-11: LGTM: Tech preview flag constant added.The new
FLAG_TECH_PREVIEWconstant establishes the foundation for gating experimental features. This aligns with the broader tech preview infrastructure introduced in this PR.frontend/public/module/k8s/k8s.ts (1)
30-34: LGTM: Improved null safety.The addition of optional chaining (
ref?.split) and nullish coalescing (?? []) properly handles undefined/null ref values, preventing potential runtime errors. The function correctly returns undefined for invalid inputs.cmd/bridge/main.go (1)
361-361: LGTM: Tech preview flag wired to server configuration.The
TechPreviewfield is properly initialized from the command-line flag, completing the backend portion of the tech preview infrastructure.pkg/server/server.go (1)
144-144: LGTM: Complete tech preview flag propagation.The
TechPreviewfield is properly added to both theServerandjsGlobalsstructs, and correctly propagated in theindexHandler. This completes the backend-to-frontend flag exposure, allowing the frontend to gate tech preview features viawindow.SERVER_FLAGS.techPreview.Also applies to: 175-175, 777-777
frontend/packages/operator-lifecycle-manager/src/const.ts (1)
21-27: Enum rename from PackageSource to OperatorSource has been successfully completed.Verification confirms that all references to the
PackageSourceenum have been updated toOperatorSource. The remaining occurrences of the string "PackageSource" in the codebase are function and constant names (getPackageSource,defaultPackageSourceMap), which are acceptable naming conventions and do not represent uses of the old enum type.The enum definition in
frontend/packages/operator-lifecycle-manager/src/const.tscorrectly exportsOperatorSource, and all imports have been properly updated across the codebase.frontend/packages/operator-lifecycle-manager-v1/src/models.ts (1)
1-31: LGTM!The K8sKind model definitions are well-structured and follow the expected conventions for OpenShift Console resource models. Both ClusterExtension and ClusterCatalog models are correctly configured as non-namespaced CRDs with appropriate metadata.
frontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-utils.ts (2)
39-41: LGTM on the new helper function.The
getClusterCatalogSourcefunction provides a clean way to map cluster catalog values to OperatorSource with an appropriate fallback toOperatorSource.Custom.
20-32: Code changes verified—all enum values exist and mappings are correct.The OperatorSource enum is properly defined with all expected values (RedHatOperators, RedHatMarketplace, CertifiedOperators, CommunityOperators, and Custom). Both mappings reference valid enum members from DefaultCatalogSource, DefaultClusterCatalog, and OperatorSource. The refactoring is sound.
frontend/packages/operator-lifecycle-manager/src/utils/filters.ts (1)
1-22: LGTM!The comparator functions provide clean wrappers around the existing sort utilities. The implementation is straightforward and delegates correctly to the underlying sort functions.
frontend/packages/console-shared/src/components/query-browser/QueryBrowser.tsx (1)
68-68: LGTM!The import path update consolidates the
usePollhook to a shared location, improving code organization and reusability.frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogCategories.tsx (1)
31-31: LGTM!The null check prevents potential runtime errors when
categoryis undefined or whencategorizedIds[category.id]is missing. This is a good defensive programming practice.frontend/@types/console/index.d.ts (1)
74-74: LGTM!The addition of the
techPreviewboolean flag toWindow.SERVER_FLAGSis correctly typed and consistent with the existing flag structure. This enables frontend consumption of the tech preview feature flag.frontend/packages/console-shared/src/hooks/usePoll.ts (1)
24-24: LGTM!Adding an explicit no-op cleanup function ensures the
useEffectalways returns a function, which is a React best practice. This prevents potential issues whendelayis falsy.frontend/public/components/poll-console-updates.tsx (1)
5-6: LGTM!The import path updates consolidate utilities and hooks to shared locations, improving code organization and maintainability. No functional changes are introduced.
frontend/public/components/utils/url-poll-hook.ts (1)
4-4: LGTM! Improved code organization.Moving the
usePollimport to a shared location promotes reusability and reduces code duplication across the codebase.frontend/packages/operator-lifecycle-manager-v1/README.md (1)
1-47: LGTM! Clear and comprehensive documentation.The README provides well-structured documentation with practical usage examples for both programmatic consumption (via
useFlag) and declarative usage (via console extensions). The synchronization behavior between the user setting and UI controls is clearly explained.frontend/packages/console-shared/src/components/catalog/catalog-view/CatalogView.tsx (3)
121-123: LGTM! Improved type safety.Adding explicit
stringtype annotation for thecategoryIdparameter improves type safety and makes the function signature more explicit.
157-159: LGTM! Defensive null checks and graceful fallbacks.The added null check for categories and nullish coalescing for labels prevent runtime errors when dealing with potentially missing category data. This is a solid defensive programming practice.
344-344: LGTM! Enables catalog-type-specific behavior.Passing
catalogTypetoCatalogToolbarenables the toolbar to adjust its behavior based on the catalog type, which aligns with the OLMv1 feature flag infrastructure introduced in this PR.frontend/packages/console-app/package.json (1)
80-80: LGTM! Consistent module exposure.The new exposed module follows the established naming and path conventions, enabling other packages to consume the tech preview flag provider hook.
frontend/packages/operator-lifecycle-manager-v1/src/const.ts (1)
9-14: LGTM! Well-documented constants.The new constants follow established naming conventions and are clearly documented. The namespaced user setting key (
console.olmv1.enabled) is a good practice for avoiding conflicts.frontend/packages/operator-lifecycle-manager/console-extensions.json (2)
383-386: LGTM! Ensures mutual exclusivity between OLM versions.The
disallowedflag constraint ensures that the legacy OLM catalog item provider is disabled when OLMv1 is enabled, preventing conflicts between the two implementations.
444-447: LGTM! Consistent mutual exclusivity pattern.This constraint mirrors the item provider pattern, ensuring that legacy OLM categories are disabled when OLMv1 is active, maintaining consistency across the catalog provider infrastructure.
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Alert.tsx (1)
5-22: LGTM! Well-structured informational component.The component provides clear information about OLMv1 with proper internationalization and a helpful documentation link. The use of PatternFly's Alert component with the info variant is appropriate for this use case.
frontend/packages/console-app/src/hooks/useTechPreviewFlagProvider.ts (1)
1-9: LGTM! Clean flag provider implementation.The hook follows the standard pattern for feature flag providers, using boolean coercion to ensure a consistent flag value from the server configuration. The implementation is simple and focused.
frontend/packages/console-app/console-extensions.json (2)
1011-1014: LGTM!The disallowed flag correctly gates the legacy Installed Operators navigation when OLMv1 is enabled, ensuring a clean transition between the old and new operator management UIs.
2533-2538: The new entry is valid; the parse error is pre-existing.The new console.flag/hookProvider entry at lines 2533-2538 is syntactically correct JSON. The jq parse error originates from line 1684, where the file contains C-style comments (
//) that are not supported in strict JSON. This is a pre-existing file format condition unrelated to the PR changes. The file appears to intentionally use JSON5 or a similar format that allows comments.Likely an incorrect or invalid review comment.
frontend/packages/console-shared/src/components/catalog/CatalogController.tsx (2)
57-61: LGTM!The feature flag gating logic correctly checks both tech preview enablement and OLMv1 enablement before showing the alert, and appropriately scopes it to the operator catalog type only.
195-200: LGTM!The conditional rendering of the OLMv1Alert is properly gated by the computed flag and includes appropriate spacing via PatternFly utility classes.
frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogCategories.ts (1)
7-29: LGTM!The hook correctly derives unique, sorted categories from catalog items with proper memoization and error handling. The category normalization (trimming, deduplication, lowercase IDs) follows good practices for consistent catalog navigation.
frontend/packages/operator-lifecycle-manager-v1/src/hooks/useOLMv1FlagProvider.ts (1)
11-19: LGTM!The flag provider correctly implements the requirement that OLMv1 features are only enabled when both tech preview mode is active AND the user has enabled the OLMv1 toggle. The documentation is clear and the logic is straightforward.
frontend/packages/operator-lifecycle-manager-v1/src/types.ts (1)
1-25: LGTM!The type definitions are clear and comprehensive. Note that all fields in
OLMCatalogItemare required. Ensure the API always provides all these fields, or consider marking optional fields with?if the API contract allows missing values.frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts (1)
16-73: LGTM!The hook implements efficient catalog item fetching with proper conditional request handling (If-Modified-Since/304 Not Modified), cleanup via AbortController, and appropriate polling interval. The caching strategy balances freshness with server load.
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Switch.tsx (1)
14-28: OLMv1 switch implementation looks good; verify user setting key alignmentThe switch wiring (feature flag + persisted user setting + callback) looks correct and straightforward. One thing to double‑check:
OLMV1_ENABLED_USER_SETTING_KEYshould match theuserSettingsKeyused inconsole-extensions.json(currently"console.olmv1.enabled") so the toolbar toggle and the user preference item stay in sync.frontend/packages/operator-lifecycle-manager-v1/package.json (1)
9-12: Exposed modules align with console extension usageThe exposed module names and paths (
useCatalogItems,useCatalogCategories,useOLMv1FlagProvider,CreateClusterExtension) match the$codeRefentries inconsole-extensions.json, which should keep the plugin surface consistent.frontend/packages/operator-lifecycle-manager-v1/console-extensions.json (1)
36-103: Flag and user-preference wiring is coherentThe OLMv1 nav item, catalog item/categories providers, and user preference group/item are consistently gated (CLUSTER_*_API + TECH_PREVIEW + OLMV1_ENABLED where appropriate), and the IDs/labels (
"console.olmv1.enabled",%olm-v1~…%) line up with the new OLMv1 strings and user setting. This should give a predictable toggle flow for enabling the OLMv1 catalog.frontend/packages/operator-lifecycle-manager-v1/locales/en/olm-v1.json (1)
2-14: OLMv1 localization keys align with usagesThe OLMv1 strings here match the keys referenced in
console-extensions.jsonandOLMv1Switch.tsx, so the new catalog, toggle, and help text should all localize correctly.
| "type": "console.resource/create", | ||
| "properties": { | ||
| "model": { | ||
| "group": "olm.operatorframework.io", | ||
| "version": "v1", | ||
| "kind": "ClusterExtension" | ||
| }, | ||
| "component": { | ||
| "$codeRef": "CreateClusterExtension" | ||
| } | ||
| }, | ||
| "flags": { | ||
| "required": ["CLUSTER_EXTENSION_API"] | ||
| } |
There was a problem hiding this comment.
Align create-page flags with other OLMv1 tech preview surfaces
The console.resource/create entry for ClusterExtension is only gated by CLUSTER_EXTENSION_API, while the navigation item and catalog providers also require TECH_PREVIEW and OLMV1_ENABLED. If the intent is to keep all OLMv1 UX behind the tech preview + user toggle, consider aligning this to:
"flags": {
"required": ["CLUSTER_EXTENSION_API", "TECH_PREVIEW", "OLMV1_ENABLED"]
}Otherwise, users could still hit the OLMv1 create page via direct URL even when OLMv1 is disabled.
🤖 Prompt for AI Agents
frontend/packages/operator-lifecycle-manager-v1/console-extensions.json lines
106 to 119: the create-page entry for ClusterExtension is only gated by
CLUSTER_EXTENSION_API which allows direct URL access when OLMv1 or tech preview
are disabled; update the flags.required array to include CLUSTER_EXTENSION_API,
TECH_PREVIEW, and OLMV1_ENABLED so the create resource is consistently behind
the same feature and tech preview toggles as the navigation and catalog
providers.
| packageName: ${packageName || '<package-name>'}\n | ||
| version : ${version || '<version>'} |
There was a problem hiding this comment.
Fix YAML formatting typo.
Line 29 has an extra space before the colon in version : which is inconsistent with the surrounding YAML and likely unintentional.
Apply this diff:
packageName: ${packageName || '<package-name>'}\n
- version : ${version || '<version>'}
+ version: ${version || '<version>'}
selector:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| packageName: ${packageName || '<package-name>'}\n | |
| version : ${version || '<version>'} | |
| packageName: ${packageName || '<package-name>'}\n | |
| version: ${version || '<version>'} |
🤖 Prompt for AI Agents
frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx
around lines 28 to 29: fix the YAML formatting typo by removing the extra space
before the colon on the version line so it matches the surrounding YAML style
(change `version : ${version || '<version>'}` to `version: ${version ||
'<version>'}`), ensuring consistent key:value formatting.
| variant="info" | ||
| title={t('Operator Lifecycle Management version 1')} | ||
| actionLinks={ | ||
| <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL}/html/operators/olm-v1`}> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify documentationBaseURL availability.
The component assumes window.SERVER_FLAGS.documentationBaseURL is always defined. If this value is missing or undefined, the href will become "undefined/html/operators/olm-v1", resulting in a broken link.
Consider adding a fallback or conditional rendering:
- <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL}/html/operators/olm-v1`}>
+ <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL || ''}/html/operators/olm-v1`}>Or verify that documentationBaseURL is always set when this component renders:
🏁 Script executed:
#!/bin/bash
# Description: Check if documentationBaseURL is consistently defined across the codebase
# Search for SERVER_FLAGS.documentationBaseURL usage patterns
rg -nP 'SERVER_FLAGS\.documentationBaseURL' -A2 -B2
# Search for SERVER_FLAGS interface definition
ast-grep --pattern 'interface ServerFlags {
$$$
documentationBaseURL$_
$$$
}'Length of output: 2369
I need to check if SERVER_FLAGS.documentationBaseURL is guaranteed to be defined at runtime.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Search for SERVER_FLAGS type/interface definition
rg -nP 'SERVER_FLAGS\s*[:=].*{' -A 20 | head -100
# Search for window.SERVER_FLAGS initialization
rg -nP 'window\.SERVER_FLAGS\s*=' -A 5 -B 5
# Look for TypeScript interface/type definitions for SERVER_FLAGS
rg -nP '(interface|type)\s+.*SERVER_FLAGS' -A 20Length of output: 37579
Add fallback for potentially undefined documentationBaseURL.
The codebase shows inconsistent handling of window.SERVER_FLAGS.documentationBaseURL. While some components (e.g., documentation.tsx, GuidedTourText.tsx) use fallbacks, this component at line 12 does not. The type definition is incomplete, and other parts of the codebase demonstrate this property can be undefined, which would result in a broken link like "undefined/html/operators/olm-v1".
Apply the suggested fix:
- <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL}/html/operators/olm-v1`}>
+ <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL || ''}/html/operators/olm-v1`}>Or align with the pattern used elsewhere and provide a meaningful fallback URL.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL}/html/operators/olm-v1`}> | |
| <ExternalLink href={`${window.SERVER_FLAGS.documentationBaseURL || ''}/html/operators/olm-v1`}> |
🤖 Prompt for AI Agents
In frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Alert.tsx
around line 12, the ExternalLink href uses
window.SERVER_FLAGS.documentationBaseURL directly which can be undefined and
produce a broken "undefined/..." URL; update the expression to use the same
fallback pattern used elsewhere (e.g. default to an empty string or a known docs
base URL) so the href becomes `${window.SERVER_FLAGS.documentationBaseURL ||
'<fallback>'}/html/operators/olm-v1`, ensuring the type still matches and the
link remains valid when documentationBaseURL is not set.
Implements a custom create page for ClusterExtension resources that pre-fills the YAML editor with operator details from the catalog. When a user clicks "Install" on an operator in the Extension Catalog, they are directed to a creation page with a YAML template pre-populated with the selected operator's package name, version, and catalog source. Changes: - Add CreateClusterExtension component with query parameter support - Add ClusterExtension and ClusterCatalog model definitions - Update catalog item install URL to include query parameters - Add console.resource/create extension for custom create page - Expose CreateClusterExtension component in package exports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
TheRealJon
force-pushed
the
CONSOLE-4106
branch
from
c859efc to
7c9617e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx (1)
29-29: Fix YAML formatting typo.The extra space before the colon in
version :is inconsistent with YAML conventions and the surrounding code.Apply this diff:
- version : ${version || '<version>'} + version: ${version || '<version>'}frontend/packages/operator-lifecycle-manager-v1/console-extensions.json (1)
106-119: Align create-page flags with other OLMv1 surfaces.The
console.resource/createentry is only gated byCLUSTER_EXTENSION_API, while navigation and catalog providers require bothTECH_PREVIEWandOLMV1_ENABLED. This inconsistency allows direct URL access to the create page even when OLMv1 is disabled via user preferences.Apply this diff to align the flags:
"flags": { - "required": ["CLUSTER_EXTENSION_API"] + "required": ["CLUSTER_EXTENSION_API", "TECH_PREVIEW", "OLMV1_ENABLED"] }
🧹 Nitpick comments (3)
frontend/public/module/k8s/k8s.ts (1)
27-35: Safer handling of undefinedreflooks good; consider aligning the type signatureUsing
ref?.split('~') ?? []avoids runtime errors whenrefisnull/undefinedwhile preserving existing behavior for valid strings, which is a nice robustness improvement.If
refbeingundefinedis expected, you might optionally update the parameter type to reflect that (e.g.,ref?: GroupVersionKind | stringor includingundefinedin the union) so the TS contract matches the runtime behavior, but this isn’t strictly required.frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx (2)
28-28: Remove unnecessary escape sequence.The
\nat the end of line 28 is redundant since the template literal already contains natural line breaks.Apply this diff:
- packageName: ${packageName || '<package-name>'}\n + packageName: ${packageName || '<package-name>'}
36-36: Consider improving type safety.The
as anytype assertion bypasses TypeScript's type checking. IfCreateYAMLaccepts no additional props, consider removing the spread; otherwise, define an explicit props interface for the component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (6)
frontend/packages/operator-lifecycle-manager-v1/console-extensions.json(3 hunks)frontend/packages/operator-lifecycle-manager-v1/package.json(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsx(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/models.ts(1 hunks)frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx(3 hunks)frontend/public/module/k8s/k8s.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/operator-lifecycle-manager-v1/package.json
🧰 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/public/module/k8s/k8s.tsfrontend/packages/operator-lifecycle-manager-v1/src/components/cluster-extension/CreateClusterExtension.tsxfrontend/packages/operator-lifecycle-manager-v1/src/models.tsfrontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsxfrontend/packages/operator-lifecycle-manager-v1/console-extensions.json
🔇 Additional comments (1)
frontend/packages/operator-lifecycle-manager-v1/src/models.ts (1)
1-31: LGTM!Both model definitions are well-structured and consistent. The field values are appropriate for cluster-scoped CRDs in the
olm.operatorframework.ioAPI group.
frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx
Show resolved
Hide resolved
|
@TheRealJon: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
2 participants
Depends on #15715