CONSOLE-4729,CONSOLE-4823: Enable OLMv1 Tech Preview #15715
Conversation
openshift-ci
bot
added
component/backend
TheRealJon
left a comment
TheRealJon
left a comment
There was a problem hiding this comment.
Group review comments
frontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.md
Outdated
Show resolved
Hide resolved
frontend/packages/console-dynamic-plugin-sdk/src/extensions/catalog.ts
Outdated
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1ToolbarToggle.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1ToolbarToggle.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1ToolbarToggle.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1ToolbarToggle.tsx
Outdated
Show resolved
Hide resolved
frontend/packages/operator-lifecycle-manager-v1/src/utils/catalog-item.tsx
Outdated
Show resolved
Hide resolved
|
QE Approver Docs Approver: PX Approver: |
|
@TheRealJon: GitHub didn't allow me to assign the following users: jseseCCS. Note that only openshift members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. 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 kubernetes-sigs/prow repository. |
TheRealJon
force-pushed
the
CONSOLE-4729
branch
from
f5a1629 to
466d81f
Compare
| } | ||
| ] | ||
| "id": "olmv1-catalog", | ||
| "label": "%olm-v1~OLMV1 Catalog%" |
There was a problem hiding this comment.
| "label": "%olm-v1~OLMV1 Catalog%" | |
| "label": "%olm-v1~OLMv1 Catalog%" |
it would be good if OLMv1 word is consistent across all pages
openshift-merge-robot
added
the
needs-rebase
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]>
openshift-ci
bot
added
the
docs-approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx(3 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/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx
🔇 Additional comments (2)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx (2)
143-145: Well done fixing the critical logic error!The defensive initialization correctly addresses the past review concern where items were silently lost. Items with types not present in
catalogProviderExtensionswill now be properly collected.
168-171: Verify deduplication behavior when categories share IDs but differ in properties.
uniqBykeeps the first occurrence and discards duplicates. If multiple category providers return categories with the sameidbut different properties (e.g.,label,description), only the first will be retained. Confirm this is the intended behavior.
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx (1)
168-171: Category aggregation/deduping looks good; consider defensive filteringFlattening
categoryProviderMapanduniqBy(..., 'id')is a clean way to expose a single de‑duplicated category list to consumers.If plugin providers ever return
null/undefinedentries, you could defensively filter them out before dedupe to keepcategoriesstrictlyCatalogCategory[]at runtime:- const categories = React.useMemo( - () => _.uniqBy(_.flatten(Object.values(categoryProviderMap)), 'id'), - [categoryProviderMap], - ); + const categories = React.useMemo( + () => + _.uniqBy( + _.flatten(Object.values(categoryProviderMap)).filter(Boolean), + 'id', + ), + [categoryProviderMap], + );Not required if providers already guarantee well‑formed arrays, but it would make this layer more robust against bad plugin data.
📜 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 (2)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx(3 hunks)frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts(1 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/console-shared/src/components/catalog/service/CatalogServiceProvider.tsxfrontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts
🔇 Additional comments (4)
frontend/packages/console-shared/src/components/catalog/service/CatalogServiceProvider.tsx (2)
106-113: Category provider map update now avoids redundant state changesUsing
_.isEqualinsidesetCategoryProviderMapto short‑circuit whennewCategorieshasn’t actually changed is correct and prevents unnecessary re‑renders. The callback signature also now matches the resolver’s(value, id)shape, so categories from all providers will be preserved.
137-147: Type bucketing fix correctly preserves items for previously-unknown typesInitializing
result[item.type]on demand beforepushensures catalog items whosetypewasn’t preseeded fromcatalogProviderExtensionsare no longer dropped on the floor. The overallcatalogItemsMapconstruction remains straightforward and efficient for the expected data sizes.frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts (2)
62-66: LGTM: Cleanup prevents memory leaks.The cleanup effect correctly aborts in-flight requests when the component unmounts, preventing the setState warnings mentioned in the past review.
23-30: No action required—Cache-Control request header is intentionally expected by the backend.The backend (
pkg/olm/catalog_client.go:78) explicitly setsCache-Controlas a request header, confirming this is a deliberate design choice rather than a mistake. The frontend and backend are aligned on this pattern: both sendCache-Control: max-age=300when caching is needed.
| const fetchItems = useCallback(() => { | ||
| abortControllerRef.current?.abort(); | ||
| abortControllerRef.current = new AbortController(); | ||
|
|
||
| consoleFetch('/api/olm/catalog-items/', { headers, signal: abortControllerRef.current.signal }) | ||
| .then((response) => { | ||
| if (response.status === 304) { | ||
| return null; | ||
| } | ||
|
|
||
| // Only update state on successful 200 response | ||
| if (response.status === 200) { | ||
| setLastModified((current) => response.headers.get('Last-Modified') || current); | ||
| return response.json(); | ||
| } | ||
| throw new Error(`HTTP ${response.status}: ${response.statusText}`); | ||
| }) | ||
| .then((olmItems: OLMCatalogItem[] | null) => { | ||
| if (olmItems !== null) { | ||
| setOLMCatalogItems(olmItems); | ||
| } | ||
| setLoading(false); | ||
| }) | ||
| .catch((err) => { | ||
| if (err.name === 'AbortError') return; | ||
| setError(err.toString()); | ||
| setLoading(false); | ||
| }); | ||
| }, [headers]); |
There was a problem hiding this comment.
Clear error state on successful fetch.
The error state (line 57) is never cleared when subsequent requests succeed. This leads to stale error messages persisting after recovery.
Apply this diff to clear the error state on successful requests:
.then((olmItems: OLMCatalogItem[] | null) => {
if (olmItems !== null) {
setOLMCatalogItems(olmItems);
}
+ setError('');
setLoading(false);
})Additionally, consider clearing the error when starting a new fetch attempt:
const fetchItems = useCallback(() => {
abortControllerRef.current?.abort();
abortControllerRef.current = new AbortController();
+ setError('');
consoleFetch('/api/olm/catalog-items/', { headers, signal: abortControllerRef.current.signal })📝 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.
| const fetchItems = useCallback(() => { | |
| abortControllerRef.current?.abort(); | |
| abortControllerRef.current = new AbortController(); | |
| consoleFetch('/api/olm/catalog-items/', { headers, signal: abortControllerRef.current.signal }) | |
| .then((response) => { | |
| if (response.status === 304) { | |
| return null; | |
| } | |
| // Only update state on successful 200 response | |
| if (response.status === 200) { | |
| setLastModified((current) => response.headers.get('Last-Modified') || current); | |
| return response.json(); | |
| } | |
| throw new Error(`HTTP ${response.status}: ${response.statusText}`); | |
| }) | |
| .then((olmItems: OLMCatalogItem[] | null) => { | |
| if (olmItems !== null) { | |
| setOLMCatalogItems(olmItems); | |
| } | |
| setLoading(false); | |
| }) | |
| .catch((err) => { | |
| if (err.name === 'AbortError') return; | |
| setError(err.toString()); | |
| setLoading(false); | |
| }); | |
| }, [headers]); | |
| const fetchItems = useCallback(() => { | |
| abortControllerRef.current?.abort(); | |
| abortControllerRef.current = new AbortController(); | |
| setError(''); | |
| consoleFetch('/api/olm/catalog-items/', { headers, signal: abortControllerRef.current.signal }) | |
| .then((response) => { | |
| if (response.status === 304) { | |
| return null; | |
| } | |
| // Only update state on successful 200 response | |
| if (response.status === 200) { | |
| setLastModified((current) => response.headers.get('Last-Modified') || current); | |
| return response.json(); | |
| } | |
| throw new Error(`HTTP ${response.status}: ${response.statusText}`); | |
| }) | |
| .then((olmItems: OLMCatalogItem[] | null) => { | |
| if (olmItems !== null) { | |
| setOLMCatalogItems(olmItems); | |
| } | |
| setError(''); | |
| setLoading(false); | |
| }) | |
| .catch((err) => { | |
| if (err.name === 'AbortError') return; | |
| setError(err.toString()); | |
| setLoading(false); | |
| }); | |
| }, [headers]); |
🤖 Prompt for AI Agents
In frontend/packages/operator-lifecycle-manager-v1/src/hooks/useCatalogItems.ts
around lines 32 to 60, the error state is never cleared after a successful
fetch, causing stale error messages to persist; to fix, clear the error at the
start of a new fetch (before launching the request) and also clear it when a 200
response is processed (e.g., call setError(null) right before or after
setOLMCatalogItems when olmItems is not null), ensuring you still
setLoading(false) and preserve existing abort handling.
|
@TheRealJon: all tests passed! 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. |
|
/lgtm Since this is the first of several PRs and is behind a feature flag. There are known issues with the display of |
rhamilto
added
the
plugin-api-approved
|
@yapei Could you take a look when you get a chance? Would love to merge this before Friday. |
|
created a cluster-bot cluster, enable featuregate via and we can still see following error in console pod 
|
|
Hi @yapei, sorry, I should have mentioned, openshift/console-operator#1068 fixes the issue with console ClusterCatalog permissions. Could you test these two together? |
jseseCCS
left a comment
jseseCCS
left a comment
There was a problem hiding this comment.
Finished reviewing all user-facing content. Added a few small wording suggestions; everything else looks good. Let me know if you need anything else.
| }, []); | ||
|
|
||
| const catalogCategories = React.useMemo<CatalogCategory[]>(() => { | ||
| const allCategory = { id: ALL_CATEGORY, label: t('console-shared~All items') }; |
There was a problem hiding this comment.
@jseseCCS PatternFly recommends sentence case so for any inconsistencies, I'd change the other items to sentence case https://www.patternfly.org/ux-writing/capitalization
| } | ||
| > | ||
| {t( | ||
| "With OLMv1, you'll get a much simpler API that's easier to work with and understand. Plus, you have more direct control over updates. You can define update ranges and decide exactly how they are rolled out.", |
There was a problem hiding this comment.
we shouldn’t describe OLMv1 as “simpler,” “easier to work with,” “easier to understand” unless we provide concrete examples that map to those descriptions. Suggest removing those claims. For example --> "With OLMv1, you have more direct control over updates. You can define update ranges and decide how updates are rolled out."
give example of rollout definition options?
| const popoverContent = ( | ||
| <div> | ||
| {t( | ||
| 'olm-v1~Lets you use OLMv1 (Tech Preview), a streamlined redesign of OLMv0. OLMv1 simplifies operator management with declarative APIs, enhanced security, and direct, GitOps-friendly control over upgrades.', |
There was a problem hiding this comment.
suggested rewrite: “OLMv1, a technology preview, is the next iteration of OLM...."
also, we shouldn’t claim OLMv1 is “streamlined,” “simplifies,” “GitOps-friendly” unless we can support those with specific user-visible examples/differences.
| label={t('olm-v1~Enable OLMv1')} | ||
| isChecked={olmv1Enabled ?? false} | ||
| onChange={handleToggle} | ||
| aria-label={t('olm-v1~Toggle OLMv1 UI')} |
There was a problem hiding this comment.
suggestion --> aria-label={t('olm-v1-Toggle OLMv1 visibility')}
| <Popover aria-label={t('olm-v1~OLMv1 information')} bodyContent={popoverContent}> | ||
| <Button | ||
| icon={<OutlinedQuestionCircleIcon />} | ||
| aria-label={t('olm-v1~OLMv1 information')} |
There was a problem hiding this comment.
suggestion --> aria-label={t('olm-v1-More information about OLMv1')}
| { label: 'Source', value: source || '-' }, | ||
| { label: 'Provider', value: provider || '-' }, | ||
| { | ||
| label: 'Infrastructure features', |
There was a problem hiding this comment.
“Infrastructure Features” for consistency with other catalog labels that use headline case?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jseseCCS, rhamilto, 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 |
All of the user-facing text came from the UXD designs. @kevinhatchoua do you mind taking a look at Jocelyn's suggestions? |
|
tested the changes with openshift/console-operator#1068 now works well /verified by @yapei |
openshift-ci-robot
added
the
verified
|
@yapei: This PR has been marked as verified by 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. |
|
/jira refresh |
|
@TheRealJon: No Jira issue is referenced in the title of this pull request. 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. |
TheRealJon
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@TheRealJon: This pull request references CONSOLE-4729 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. This pull request references CONSOLE-4823 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. |
|
/jira refresh |
|
@TheRealJon: This pull request references CONSOLE-4729 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. This pull request references CONSOLE-4823 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. |
11 participants
This PR integrates OLM version 1 into the developer catalog
OLMv1 Feature Integration
Technical Changes
Frontend:
Backend:
Testing