CONSOLE-3769: Update OpenShift Dynamic Plugin SDK + webpack-related deps#15892
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@logonoff: This pull request references CONSOLE-3769 which is a valid jira issue. DetailsIn 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. |
WalkthroughRefactors a manifest/type import to type-only and switches Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (2)frontend/packages/shipwright-plugin/src/components/logs/tekton-results.ts (1)
frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (1)
🔇 Additional comments (7)
Comment |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
5-31: Complete the migration guide referenced in CHANGELOG before release.The type changes from
ExtensionDeclaration<...>toExtension<...>are correctly implemented and no old type references remain in the codebase. However, the CHANGELOG for version 4.22.0-prerelease.1 referencesupgrade-sdk.mdfor migration details, but the upgrade guide section for 4.22 is incomplete (marked TODO). This migration guidance must be completed before the release to help plugin authors adapt to the breaking changes.
🤖 Fix all issues with AI Agents
In
@frontend/packages/console-app/src/components/console-operator/ConsoleOperatorConfig.tsx:
- Around line 338-349: The CSP violations lookup is inconsistent: change the
lookup key to use the plugin manifest name (the same identification used to find
pluginInfo) rather than plugin?.metadata?.name; specifically, in the block
handling loadedPluginInfo/notLoadedPluginInfo replace
cspViolations[plugin?.metadata?.name] with cspViolations[plugin?.manifest?.name]
(or use loadedPluginInfo?.manifest?.name / notLoadedPluginInfo?.manifest?.name
as appropriate) so pluginName, pluginInfo, loadedPluginInfo, notLoadedPluginInfo
and cspViolations all use manifest.name consistently.
In @frontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.ts:
- Line 5: The SDK store is missing the FLAGS reducer so featureFlagMiddleware
(which calls s.getState().FLAGS.toObject()) will throw; fix by adding the FLAGS
reducer to SDKReducers alongside sdkCore and k8s (import and include the
existing FLAGS reducer implementation) or, alternatively, only apply
featureFlagMiddleware in useReduxStore when the created store's state contains
FLAGS (e.g., check s.getState().FLAGS !== undefined before using/applying
middleware) so featureFlagMiddleware never runs against an undefined FLAGS.
In @frontend/packages/console-dynamic-plugin-sdk/upgrade-sdk.md:
- Around line 3-4: The 4.22 upgrade notes are still a TODO and must be completed
or deferred; update the "## 4.22" section in upgrade-sdk.md to list the breaking
changes introduced (e.g., removal of legacy plugin support and any refactors),
provide concrete migration steps for plugin authors (how to replace legacy APIs,
updated entry points, config keys, and examples), and include links to any
renamed functions/classes and sample diffs, or alternatively remove the
placeholder and move the full migration guide to a follow-up docs PR so this
branch does not ship incomplete guidance.
In @frontend/packages/console-plugin-sdk/src/codegen/active-plugins.ts:
- Line 15: The generated module from getActivePluginsModule is missing the
required version field for ActivePlugin; update the activePlugins.push(...) call
inside getActivePluginsModule to include version: '${pkg.version}' alongside
name and extensions so the produced objects match the ActivePlugin type and
mirror loadActivePluginsForTestPurposes (which already uses version).
In @frontend/public/reducers/features.ts:
- Around line 75-80: The code weakens type safety by casting
pluginStore.getExtensions().filter(isModelFeatureFlag) to any and also uses
referenceForModel incorrectly for ExtensionK8sModel; update the logic to
preserve types and use the correct reference function: remove the `as any` cast
by narrowing the array with the isModelFeatureFlag type guard so the items are
typed as ExtensionK8sModel, then call
addToCRDs(referenceForExtensionModel(ff.properties.model), ff.properties.flag)
instead of referenceForModel; if immediate SDK migration isn't possible, add a
runtime type check/assert around ff.properties.model to ensure it matches
ExtensionK8sModel shape before calling referenceForExtensionModel to avoid
runtime errors.
🧹 Nitpick comments (6)
frontend/packages/console-plugin-sdk/src/api/pluginSubscriptionService.ts (1)
44-88: Remove unused feature flag tracking code.Feature flags are retrieved and tracked (lines 44, 46, 67-68, 76, 78, 83) but are no longer used after the removal of flag-based filtering. This is dead code that can be cleaned up.
🔎 Suggested cleanup
const getExtensions = () => pluginStore.getExtensions(); - const getFlags = () => reduxStore.getState().FLAGS; - - type FeatureFlags = ReturnType<typeof getFlags>; const invokeExtensionListener = (sub: ExtensionSubscription, currentExtensions: Extension[]) => { // Narrow extensions according to type guards const matchedExtensions = _.flatMap(sub.typeGuards.map((tg) => currentExtensions.filter(tg))); // Invoke listener only if the extension list has changed if (!_.isEqual(matchedExtensions, sub.listenerLastArgs)) { sub.listenerLastArgs = matchedExtensions; sub.listener(matchedExtensions); } }; onExtensionSubscriptionAdded = (sub) => { invokeExtensionListener(sub, getExtensions()); }; onDynamicPluginListenerAdded = (listener) => { listener(pluginStore.getPluginInfo()); }; let lastExtensions: Extension[] = null; - let lastFlags: FeatureFlags = null; const invokeAllExtensionListeners = () => { if (extensionSubscriptions.length === 0) { return; } const nextExtensions = getExtensions(); - const nextFlags = getFlags(); - if (_.isEqual(nextExtensions, lastExtensions) && nextFlags === lastFlags) { + if (_.isEqual(nextExtensions, lastExtensions)) { return; } lastExtensions = nextExtensions; - lastFlags = nextFlags; extensionSubscriptions.forEach((sub) => { invokeExtensionListener(sub, nextExtensions); }); };frontend/packages/console-plugin-sdk/src/store.ts (1)
51-60: Consider clarifying the testing-only nature of this implementation.The JSDoc on line 51-53 states this is "intended for testing purposes," but the class is exported publicly. The
optionsparameter (line 57) is typed as{}but unused beyond passing tosuper(). Consider adding a more explicit type or removing if not needed.frontend/packages/console-app/src/components/console-operator/ConsolePluginDescriptionDetail.tsx (1)
16-20: Consider stronger typing for console custom properties.The
as anycast at Line 17 suppresses type checking when accessingmanifest.customProperties?.console. This could mask type mismatches or missing properties.🔎 Proposed fix for type safety
Define a proper interface for the console custom properties:
interface ConsoleCustomProperties { console?: { description?: string; }; } // Then use it with proper typing: const description = (pluginInfo.manifest.customProperties as ConsoleCustomProperties)?.console?.description || DASH;frontend/packages/console-dynamic-plugin-sdk/src/extensions/guided-tour.ts (1)
4-4: Import path inconsistency with peer filesThis file uses an absolute import path
@console/dynamic-plugin-sdk/src/types, while other files in the sameextensions/directory use the relative path../types. Consider using the relative import for consistency with the rest of the codebase.🔎 Proposed fix
-import type { Extension, CodeRef } from '@console/dynamic-plugin-sdk/src/types'; +import type { Extension, CodeRef } from '../types';frontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.ts (1)
41-47: Remove unnecessary& {}intersections.The
& {}intersections onResourceListPageandResourceDetailsPageare redundant since intersecting with an empty object type has no effect.🔎 Suggested simplification
-export type ResourceListPage = Extension<'console.page/resource/list', ResourcePageProperties & {}>; +export type ResourceListPage = Extension<'console.page/resource/list', ResourcePageProperties>; /** Adds new resource details page to Console router. */ -export type ResourceDetailsPage = Extension< - 'console.page/resource/details', - ResourcePageProperties & {} ->; +export type ResourceDetailsPage = Extension<'console.page/resource/details', ResourcePageProperties>;frontend/public/plugins.ts (1)
37-46: Consider stronger typing for the middleware.The middleware currently uses
anytypes fornextandactionparameters, reducing type safety. Consider using Redux'sMiddlewaretype for better type checking:import type { Middleware } from 'redux'; export const featureFlagMiddleware: Middleware<{}, RootState> = (store) => (next) => (action) => { const result = next(action); pluginStore.setFeatureFlags(store.getState().FLAGS.toObject()); return result; };This middleware runs on every dispatched action. While likely necessary for synchronization, be mindful of performance if the flag set is large or actions are frequent.
📜 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
📒 Files selected for processing (111)
frontend/packages/console-app/src/__tests__/plugin-test-utils.tsfrontend/packages/console-app/src/components/console-operator/ConsoleOperatorConfig.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginDescriptionDetail.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginEnabledStatusDetail.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginManifestPage.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginStatusDetail.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginVersionDetail.tsxfrontend/packages/console-app/src/components/dashboards-page/dynamic-plugins-health-resource/DynamicPluginsPopover.tsxfrontend/packages/console-app/src/components/dashboards-page/dynamic-plugins-health-resource/NotLoadedDynamicPlugins.tsxfrontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsxfrontend/packages/console-app/src/components/nav/NavSection.tsxfrontend/packages/console-app/src/components/nav/PluginNavItem.tsxfrontend/packages/console-app/src/components/nav/useNavExtensionForPerspective.tsfrontend/packages/console-app/src/components/nav/useNavExtensionsForSection.tsfrontend/packages/console-app/src/components/nav/utils.tsfrontend/packages/console-app/src/components/user-preferences/perspective/PreferredPerspectiveSelect.tsxfrontend/packages/console-app/src/components/user-preferences/perspective/__tests__/perspective.data.tsfrontend/packages/console-app/src/hooks/useCSPViolationDetector.tsxfrontend/packages/console-app/src/hooks/usePluginRoutes.tsxfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/README.mdfrontend/packages/console-dynamic-plugin-sdk/scripts/generate-schema.tsfrontend/packages/console-dynamic-plugin-sdk/scripts/parsers/ExtensionParser.tsfrontend/packages/console-dynamic-plugin-sdk/scripts/utils/type-resolver.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/useResolvedExtensions.tsfrontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.tsfrontend/packages/console-dynamic-plugin-sdk/src/build-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/coderefs/__tests__/coderef-resolver.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/coderefs/coderef-resolver.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/actions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/add-actions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/alerts.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/breadcrumbs.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/catalog.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-configuration.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-overview.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-settings.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/context-providers.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/create-project-modal.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/create-resource.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/custom.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboard-data-source.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboards.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/details-item.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/file-upload.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/guided-tour.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/horizontal-nav-tabs.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/import-environments.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/navigation.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/node.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/notification-alert.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/perspectives.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/project-overview.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/pvc.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/redux.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/resource-metadata.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/storage-class-provisioner.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/storage-provider.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/telemetry.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-details.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/topology.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/user-preferences.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/yaml-templates.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-dependencies.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-loader.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-dependencies.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-utils.tsfrontend/packages/console-dynamic-plugin-sdk/src/schema/plugin-manifest.tsfrontend/packages/console-dynamic-plugin-sdk/src/types.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/k8s-utils.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/test-utils.tsfrontend/packages/console-dynamic-plugin-sdk/src/validation/__tests__/ExtensionValidator.spec.tsfrontend/packages/console-dynamic-plugin-sdk/upgrade-sdk.mdfrontend/packages/console-plugin-sdk/src/__tests__/store.spec.tsfrontend/packages/console-plugin-sdk/src/api/pluginSubscriptionService.tsfrontend/packages/console-plugin-sdk/src/api/useExtensions.tsfrontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.tsfrontend/packages/console-plugin-sdk/src/codegen/active-plugins.tsfrontend/packages/console-plugin-sdk/src/index.tsfrontend/packages/console-plugin-sdk/src/store.tsfrontend/packages/console-plugin-sdk/src/typings/base.tsfrontend/packages/console-plugin-sdk/src/typings/index.tsfrontend/packages/console-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-shared/src/components/actions/__tests__/utils-test-data.tsfrontend/packages/console-shared/src/components/actions/utils.tsfrontend/packages/console-shared/src/components/dashboard/status-card/OperatorStatusBody.tsxfrontend/packages/console-shared/src/hooks/create-resource-hook.tsfrontend/packages/console-shared/src/hooks/perspective-utils.tsfrontend/packages/console-shared/src/utils/sample-utils.tsfrontend/packages/dev-console/src/components/add/AddCardSection.tsxfrontend/packages/dev-console/src/components/add/__tests__/add-page-test-data.tsxfrontend/packages/dev-console/src/components/import/import-types.tsfrontend/packages/topology/src/components/workload/utils.tsfrontend/public/actions/flags.tsfrontend/public/components/about-modal.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/health-item.tsxfrontend/public/components/notification-drawer.tsxfrontend/public/components/utils/__tests__/nav.spec.tsfrontend/public/components/utils/async.tsxfrontend/public/module/k8s/__tests__/get-resources.spec.tsfrontend/public/module/k8s/get-resources.tsfrontend/public/module/k8s/k8s-models.tsfrontend/public/plugins.tsfrontend/public/reducers/features.tsfrontend/public/redux.ts
💤 Files with no reviewable changes (7)
- frontend/packages/console-dynamic-plugin-sdk/src/utils/k8s/k8s-utils.ts
- frontend/packages/console-plugin-sdk/src/typings/index.ts
- frontend/packages/console-plugin-sdk/src/index.ts
- frontend/packages/console-dynamic-plugin-sdk/src/runtime/tests/plugin-loader.spec.ts
- frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.ts
- frontend/packages/console-dynamic-plugin-sdk/src/runtime/tests/plugin-dependencies.spec.ts
- frontend/packages/console-plugin-sdk/src/tests/store.spec.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-plugin-sdk/src/utils/__tests__/extension-i18n.spec.tsfrontend/public/actions/flags.tsfrontend/packages/console-dynamic-plugin-sdk/src/validation/__tests__/ExtensionValidator.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/schema/plugin-manifest.tsfrontend/packages/console-app/src/components/detect-perspective/__tests__/PerspectiveDetector.spec.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginEnabledStatusDetail.tsxfrontend/public/module/k8s/get-resources.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/file-upload.tsfrontend/packages/console-app/src/components/console-operator/ConsolePluginStatusDetail.tsxfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-dependencies.tsfrontend/packages/console-dynamic-plugin-sdk/upgrade-sdk.mdfrontend/packages/console-dynamic-plugin-sdk/src/extensions/notification-alert.tsfrontend/packages/console-app/src/components/nav/useNavExtensionsForSection.tsfrontend/packages/console-dynamic-plugin-sdk/src/utils/test-utils.tsfrontend/public/components/about-modal.tsxfrontend/packages/console-shared/src/hooks/create-resource-hook.tsfrontend/packages/console-app/src/components/console-operator/ConsolePluginManifestPage.tsxfrontend/packages/console-shared/src/components/dashboard/status-card/OperatorStatusBody.tsxfrontend/packages/console-app/src/components/console-operator/ConsolePluginVersionDetail.tsxfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-utils.tsfrontend/packages/console-app/src/hooks/usePluginRoutes.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/horizontal-nav-tabs.tsfrontend/packages/console-plugin-sdk/src/utils/extension-i18n.tsfrontend/packages/console-dynamic-plugin-sdk/scripts/utils/type-resolver.tsfrontend/packages/console-shared/src/components/actions/utils.tsfrontend/public/reducers/features.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/guided-tour.tsfrontend/packages/console-shared/src/components/actions/__tests__/utils-test-data.tsfrontend/packages/console-app/src/components/nav/utils.tsfrontend/public/module/k8s/k8s-models.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/storage-class-provisioner.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/custom.tsfrontend/packages/console-app/src/components/nav/useNavExtensionForPerspective.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/redux.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboard-data-source.tsfrontend/packages/console-app/src/hooks/useCSPViolationDetector.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/perspectives.tsfrontend/packages/console-dynamic-plugin-sdk/scripts/generate-schema.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/breadcrumbs.tsfrontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.tsfrontend/packages/console-dynamic-plugin-sdk/README.mdfrontend/packages/console-dynamic-plugin-sdk/scripts/parsers/ExtensionParser.tsfrontend/public/module/k8s/__tests__/get-resources.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/user-preferences.tsfrontend/packages/console-app/src/__tests__/plugin-test-utils.tsfrontend/packages/topology/src/components/workload/utils.tsfrontend/public/components/utils/async.tsxfrontend/packages/console-app/src/components/dashboards-page/dynamic-plugins-health-resource/DynamicPluginsPopover.tsxfrontend/packages/console-shared/src/utils/sample-utils.tsfrontend/packages/console-app/src/components/nav/PluginNavItem.tsxfrontend/packages/console-dynamic-plugin-sdk/src/coderefs/coderef-resolver.tsfrontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.tsfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/src/api/useResolvedExtensions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/details-item.tsfrontend/packages/console-app/src/components/user-preferences/perspective/__tests__/perspective.data.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/add-actions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/yaml-templates.tsfrontend/packages/dev-console/src/components/import/import-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/build-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/create-resource.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/create-project-modal.tsfrontend/packages/console-app/src/components/dashboards-page/dynamic-plugins-health-resource/NotLoadedDynamicPlugins.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.tsfrontend/public/redux.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/project-overview.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/context-providers.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.tsfrontend/packages/console-shared/src/hooks/perspective-utils.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-configuration.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/resource-metadata.tsfrontend/public/plugins.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/telemetry.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-plugin-sdk/src/typings/base.tsfrontend/packages/dev-console/src/components/add/AddCardSection.tsxfrontend/packages/console-app/src/components/nav/NavSection.tsxfrontend/packages/console-plugin-sdk/src/api/pluginSubscriptionService.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-details.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/navigation.tsfrontend/packages/console-plugin-sdk/src/codegen/active-plugins.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.tsfrontend/packages/console-plugin-sdk/src/api/useExtensions.tsfrontend/public/components/utils/__tests__/nav.spec.tsfrontend/packages/console-app/src/components/console-operator/ConsolePluginDescriptionDetail.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboards.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/import-environments.tsfrontend/public/components/notification-drawer.tsxfrontend/packages/dev-console/src/components/add/__tests__/add-page-test-data.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/node.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-overview.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-settings.tsfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/health-item.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/storage-provider.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/alerts.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/actions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/pvc.tsfrontend/packages/console-dynamic-plugin-sdk/src/types.tsfrontend/packages/console-app/src/components/user-preferences/perspective/PreferredPerspectiveSelect.tsxfrontend/packages/console-app/src/components/console-operator/ConsoleOperatorConfig.tsxfrontend/packages/console-dynamic-plugin-sdk/src/extensions/topology.tsfrontend/packages/console-dynamic-plugin-sdk/src/coderefs/__tests__/coderef-resolver.spec.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/catalog.tsfrontend/packages/console-plugin-sdk/src/store.ts
🧬 Code graph analysis (33)
frontend/packages/console-dynamic-plugin-sdk/src/schema/plugin-manifest.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/build-types.ts (1)
ConsolePluginManifest(43-49)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/file-upload.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-dependencies.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/build-types.ts (1)
ConsolePluginManifest(43-49)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/notification-alert.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/utils/test-utils.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/build-types.ts (1)
ConsolePluginManifest(43-49)
frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-utils.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/build-types.ts (1)
ConsolePluginManifest(43-49)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/horizontal-nav-tabs.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/scripts/utils/type-resolver.ts (1)
frontend/packages/console-dynamic-plugin-sdk/scripts/utils/typescript.ts (1)
getTypeReferenceNode(55-60)
frontend/public/reducers/features.ts (2)
frontend/public/plugins.ts (1)
pluginStore(30-30)frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
isModelFeatureFlag(37-38)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/storage-class-provisioner.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboard-data-source.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/perspectives.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/scripts/generate-schema.ts (1)
frontend/packages/console-dynamic-plugin-sdk/scripts/parsers/ExtensionParser.ts (1)
ExtensionParser(10-38)
frontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.ts (1)
frontend/public/plugins.ts (1)
featureFlagMiddleware(40-46)
frontend/public/components/utils/async.tsx (2)
frontend/public/components/factory/table.tsx (1)
ComponentProps(733-738)frontend/packages/console-shared/src/components/loading/LoadingBox.tsx (1)
LoadingBox(20-31)
frontend/packages/console-dynamic-plugin-sdk/src/coderefs/coderef-resolver.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (2)
Extension(12-12)ExtensionProperties(47-47)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/create-project-modal.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
K8sResourceCommon(67-71)frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-app/src/components/dashboards-page/dynamic-plugins-health-resource/NotLoadedDynamicPlugins.tsx (1)
frontend/public/models/index.ts (1)
ConsolePluginModel(1296-1311)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/feature-flags.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/public/redux.ts (1)
frontend/public/plugins.ts (1)
featureFlagMiddleware(40-46)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/project-overview.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/resource-metadata.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/public/plugins.ts (2)
frontend/packages/console-plugin-sdk/src/store.ts (1)
PluginStore(54-81)frontend/public/redux.ts (1)
RootState(20-28)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/navigation.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/dashboards.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
K8sResourceCommon(67-71)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/import-environments.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/public/components/notification-drawer.tsx (2)
frontend/public/components/utils/resource-link.tsx (1)
resourcePath(54-66)frontend/public/models/index.ts (1)
ConsolePluginModel(1296-1311)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/node.ts (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
ResourcesObject(200-200)frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-overview.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/cluster-settings.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/catalog.ts (1)
frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1)
Extension(12-12)
frontend/packages/console-plugin-sdk/src/store.ts (1)
frontend/packages/console-plugin-sdk/src/typings/base.ts (1)
ActivePlugin(7-11)
| .find((i) => i?.manifest?.name === pluginName); | ||
| const notLoadedPluginInfo = pluginInfo | ||
| .filter((p) => p.status !== 'loaded') | ||
| .find((i) => i?.pluginName === pluginName); | ||
| .find((i) => i?.manifest?.name === pluginName); | ||
| if (loadedPluginInfo) { | ||
| return { | ||
| name: plugin?.metadata?.name, | ||
| version: loadedPluginInfo?.metadata?.version, | ||
| description: loadedPluginInfo?.metadata?.customProperties?.console?.description, | ||
| version: loadedPluginInfo?.manifest?.version, | ||
| description: (loadedPluginInfo?.manifest?.customProperties?.console as any)?.description, | ||
| enabled, | ||
| status: loadedPluginInfo?.status, | ||
| hasCSPViolations: cspViolations[plugin.metadata.name] ?? false, | ||
| hasCSPViolations: cspViolations[plugin?.metadata?.name] ?? false, |
There was a problem hiding this comment.
Inconsistent CSP violations lookup on Line 349.
Line 349 uses plugin?.metadata?.name for the CSP violations lookup, while DevPluginsPage at Line 310 uses plugin.manifest.name. This inconsistency contradicts the stated goal of migrating to manifest-based identification.
Since pluginName (from plugin?.metadata?.name at Line 334) is used to find matching pluginInfo by comparing manifest.name (Lines 338, 341), the lookup should also use manifest.name for consistency.
🔎 Proposed fix
if (loadedPluginInfo) {
return {
name: plugin?.metadata?.name,
version: loadedPluginInfo?.manifest?.version,
description: (loadedPluginInfo?.manifest?.customProperties?.console as any)?.description,
enabled,
status: loadedPluginInfo?.status,
- hasCSPViolations: cspViolations[plugin?.metadata?.name] ?? false,
+ hasCSPViolations: cspViolations[loadedPluginInfo?.manifest?.name] ?? false,
};
}📝 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.
| .find((i) => i?.manifest?.name === pluginName); | |
| const notLoadedPluginInfo = pluginInfo | |
| .filter((p) => p.status !== 'loaded') | |
| .find((i) => i?.pluginName === pluginName); | |
| .find((i) => i?.manifest?.name === pluginName); | |
| if (loadedPluginInfo) { | |
| return { | |
| name: plugin?.metadata?.name, | |
| version: loadedPluginInfo?.metadata?.version, | |
| description: loadedPluginInfo?.metadata?.customProperties?.console?.description, | |
| version: loadedPluginInfo?.manifest?.version, | |
| description: (loadedPluginInfo?.manifest?.customProperties?.console as any)?.description, | |
| enabled, | |
| status: loadedPluginInfo?.status, | |
| hasCSPViolations: cspViolations[plugin.metadata.name] ?? false, | |
| hasCSPViolations: cspViolations[plugin?.metadata?.name] ?? false, | |
| .find((i) => i?.manifest?.name === pluginName); | |
| const notLoadedPluginInfo = pluginInfo | |
| .filter((p) => p.status !== 'loaded') | |
| .find((i) => i?.manifest?.name === pluginName); | |
| if (loadedPluginInfo) { | |
| return { | |
| name: plugin?.metadata?.name, | |
| version: loadedPluginInfo?.manifest?.version, | |
| description: (loadedPluginInfo?.manifest?.customProperties?.console as any)?.description, | |
| enabled, | |
| status: loadedPluginInfo?.status, | |
| hasCSPViolations: cspViolations[loadedPluginInfo?.manifest?.name] ?? false, |
🤖 Prompt for AI Agents
In
@frontend/packages/console-app/src/components/console-operator/ConsoleOperatorConfig.tsx
around lines 338 - 349, The CSP violations lookup is inconsistent: change the
lookup key to use the plugin manifest name (the same identification used to find
pluginInfo) rather than plugin?.metadata?.name; specifically, in the block
handling loadedPluginInfo/notLoadedPluginInfo replace
cspViolations[plugin?.metadata?.name] with cspViolations[plugin?.manifest?.name]
(or use loadedPluginInfo?.manifest?.name / notLoadedPluginInfo?.manifest?.name
as appropriate) so pluginName, pluginInfo, loadedPluginInfo, notLoadedPluginInfo
and cspViolations all use manifest.name consistently.
| import { useStore } from 'react-redux'; | ||
| import { applyMiddleware, combineReducers, createStore, compose, Store } from 'redux'; | ||
| import thunk from 'redux-thunk'; | ||
| import { featureFlagMiddleware } from '@console/internal/plugins'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify FLAGS reducer is included in SDKReducers and check pluginStore initialization
# Check if FLAGS is included in SDKReducers
echo "=== Checking SDKReducers for FLAGS ==="
rg -nP --type=ts -A10 'export const SDKReducers' frontend/packages/console-dynamic-plugin-sdk/src/app/redux.ts
# Check pluginStore initialization relative to middleware usage
echo -e "\n=== Checking pluginStore initialization ==="
rg -nP --type=ts 'pluginStore.*=' frontend/packages/console-dynamic-plugin-sdk/ frontend/public/Repository: openshift/console
Length of output: 1149
🏁 Script executed:
# Check the full useReduxStore.ts file to see the context
echo "=== Full useReduxStore.ts file ==="
cat -n frontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.tsRepository: openshift/console
Length of output: 2093
🏁 Script executed:
# Check featureFlagMiddleware implementation
echo "=== Checking featureFlagMiddleware implementation ==="
rg -nP --type=ts -B5 -A10 'featureFlagMiddleware' frontend/packages/console-dynamic-plugin-sdk/src/Repository: openshift/console
Length of output: 3313
🏁 Script executed:
# Check if FLAGS is defined in SDKReducers
echo "=== Checking for FLAGS definition ==="
rg -nP --type=ts 'FLAGS' frontend/packages/console-dynamic-plugin-sdk/src/app/redux.tsRepository: openshift/console
Length of output: 98
🏁 Script executed:
# Find where featureFlagMiddleware is defined
echo "=== Locating featureFlagMiddleware implementation ==="
fd -t f "plugins" frontend/packages/console-internal/ | head -20
# Search for featureFlagMiddleware in the entire codebase
echo -e "\n=== Searching for featureFlagMiddleware definition ==="
rg -nP --type=ts -B5 -A15 'export.*featureFlagMiddleware' frontend/packages/Repository: openshift/console
Length of output: 310
🏁 Script executed:
# Check what featureFlagMiddleware accesses
echo "=== Checking featureFlagMiddleware for FLAGS access ==="
rg -nP --type=ts 'FLAGS' frontend/packages/console-internal/ --max-count=20Repository: openshift/console
Length of output: 209
🏁 Script executed:
# Search for featureFlagMiddleware definition across entire repo
echo "=== Searching for featureFlagMiddleware definition ==="
rg -nP --type=ts 'export.*featureFlagMiddleware|const featureFlagMiddleware'
# Check tsconfig for path aliases
echo -e "\n=== Checking path aliases ==="
rg -nP '@console/internal' frontend/ --max-count=5Repository: openshift/console
Length of output: 50374
🏁 Script executed:
# Look for plugins directory
echo "=== Finding plugins directory ==="
fd -type d "plugins" frontend/
# Search for featureFlagMiddleware more broadly
echo -e "\n=== Broad search for featureFlagMiddleware ==="
rg -nP 'featureFlagMiddleware' --max-count=20Repository: openshift/console
Length of output: 922
🏁 Script executed:
# Get the full featureFlagMiddleware implementation
cat -n frontend/public/plugins.ts | sed -n '35,65p'Repository: openshift/console
Length of output: 960
🏁 Script executed:
# Also check the main store setup in redux.ts for comparison
echo "=== Frontend redux.ts store setup ==="
cat -n frontend/public/redux.ts | sed -n '1,50p'Repository: openshift/console
Length of output: 2050
🏁 Script executed:
# Check the featureReducerName value
echo "=== Checking featureReducerName ==="
rg -n 'featureReducerName|FLAGS' frontend/public/reducers/features.ts | head -20Repository: openshift/console
Length of output: 1348
🏁 Script executed:
# Check SDKReducers definition
echo "=== SDK Reducers definition ==="
cat -n frontend/packages/console-dynamic-plugin-sdk/src/app/redux.tsRepository: openshift/console
Length of output: 619
FLAGS reducer is missing from SDKReducers, causing featureFlagMiddleware to fail at runtime.
The middleware calls s.getState().FLAGS.toObject() after each action, but the SDK store is created with only SDKReducers (which contains only sdkCore and k8s). When featureFlagMiddleware executes in standalone SDK mode, it will attempt to access an undefined FLAGS state and throw an error.
Add the FLAGS reducer to SDKReducers or conditionally apply the middleware only when FLAGS is available in the store state.
🤖 Prompt for AI Agents
In @frontend/packages/console-dynamic-plugin-sdk/src/app/useReduxStore.ts at
line 5, The SDK store is missing the FLAGS reducer so featureFlagMiddleware
(which calls s.getState().FLAGS.toObject()) will throw; fix by adding the FLAGS
reducer to SDKReducers alongside sdkCore and k8s (import and include the
existing FLAGS reducer implementation) or, alternatively, only apply
featureFlagMiddleware in useReduxStore when the created store's state contains
FLAGS (e.g., check s.getState().FLAGS !== undefined before using/applying
middleware) so featureFlagMiddleware never runs against an undefined FLAGS.
| ## 4.22 | ||
| TODO |
There was a problem hiding this comment.
Complete the upgrade documentation before merging.
The PR introduces breaking changes (removal of legacy plugin support and other refactoring), but the 4.22 upgrade notes section contains only a placeholder. Plugin developers will need clear, actionable migration guidance for a breaking release.
Either populate the upgrade guide with the actual changes and migration steps, or defer this file to a separate documentation PR to avoid shipping incomplete guidance.
🤖 Prompt for AI Agents
In @frontend/packages/console-dynamic-plugin-sdk/upgrade-sdk.md around lines 3 -
4, The 4.22 upgrade notes are still a TODO and must be completed or deferred;
update the "## 4.22" section in upgrade-sdk.md to list the breaking changes
introduced (e.g., removal of legacy plugin support and any refactors), provide
concrete migration steps for plugin authors (how to replace legacy APIs, updated
entry points, config keys, and examples), and include links to any renamed
functions/classes and sample diffs, or alternatively remove the placeholder and
move the full migration guide to a follow-up docs PR so this branch does not
ship incomplete guidance.
| import { ValidationResult } from '@console/dynamic-plugin-sdk/src/validation/ValidationResult'; | ||
| import { validateConsoleExtensionsFileSchema } from '@console/dynamic-plugin-sdk/src/webpack/ConsoleRemotePlugin'; | ||
| import { Extension, ActivePlugin } from '../typings'; | ||
| import { ActivePlugin } from '../typings/base'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify ActivePlugin type definition includes version field and check for usage patterns.
# Check ActivePlugin type definition in typings/base
echo "=== ActivePlugin type definition ==="
rg -n -A 5 'type ActivePlugin\s*=' frontend/packages/console-plugin-sdk/src/typings/
# Check for consumers of ActivePlugin that may need updates
echo -e "\n=== ActivePlugin usage patterns ==="
rg -n -C 3 'ActivePlugin' frontend/packages/console-plugin-sdk/src/ -g '!*.spec.ts' -g '!*.test.ts'Repository: openshift/console
Length of output: 10075
🏁 Script executed:
sed -n '82,95p' frontend/packages/console-plugin-sdk/src/codegen/active-plugins.tsRepository: openshift/console
Length of output: 301
🏁 Script executed:
sed -n '45,71p' frontend/packages/console-plugin-sdk/src/codegen/active-plugins.tsRepository: openshift/console
Length of output: 625
Fix the generated activePlugins code to include the version field.
The ActivePlugin type now requires a version field, but getActivePluginsModule (lines 45-71) generates code that omits it. Update the generated activePlugins.push() call to include version: '${pkg.version}' alongside name and extensions, keeping it in sync with loadActivePluginsForTestPurposes (line 85-87).
🤖 Prompt for AI Agents
In @frontend/packages/console-plugin-sdk/src/codegen/active-plugins.ts at line
15, The generated module from getActivePluginsModule is missing the required
version field for ActivePlugin; update the activePlugins.push(...) call inside
getActivePluginsModule to include version: '${pkg.version}' alongside name and
extensions so the produced objects match the ActivePlugin type and mirror
loadActivePluginsForTestPurposes (which already uses version).
| // TODO: When migrating to @openshift/dynamic-plugin-sdk, use the type parameter from | ||
| // pluginStore.getExtensions<...>() to avoid `as any` cast. | ||
| (pluginStore.getExtensions().filter(isModelFeatureFlag) as any).forEach((ff) => { | ||
| // This is incorrect (for `ExtensionK8sModel` we should use `referenceForExtensionModel`). | ||
| addToCRDs(referenceForModel(ff.properties.model), ff.properties.flag); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if referenceForExtensionModel exists and identify its usage patterns
# Search for referenceForExtensionModel definition
echo "=== Searching for referenceForExtensionModel definition ==="
rg -n "referenceForExtensionModel" --type ts -C 3
# Search for ExtensionK8sModel usage to understand the type
echo -e "\n=== Searching for ExtensionK8sModel usage ==="
rg -n "ExtensionK8sModel" --type ts -C 2
# Find other usages of referenceForModel with plugin extensions
echo -e "\n=== Finding other extension model references ==="
rg -n "referenceForModel.*\.properties\.model" --type ts -C 2
# Search for isModelFeatureFlag type guard definition
echo -e "\n=== Searching for isModelFeatureFlag definition ==="
rg -n "isModelFeatureFlag" --type ts -C 3Repository: openshift/console
Length of output: 40971
Address type-safety degradation and acknowledged incorrect usage.
The as any cast defeats TypeScript's type checking and nullifies the benefit of the isModelFeatureFlag type guard. Additionally, the inline comment explicitly states that using referenceForModel is incorrect for ExtensionK8sModel types and should use referenceForExtensionModel instead.
While the TODOs indicate this is temporary migration work, both issues create maintenance risk and potential runtime correctness issues if the data or types evolve. The correct function referenceForExtensionModel exists and is used throughout the codebase for handling extension models.
Consider prioritizing the migration to the type-safe SDK approach or, at minimum, wrapping this logic with runtime validation to catch any type mismatches until the proper migration is complete.
🤖 Prompt for AI Agents
In @frontend/public/reducers/features.ts around lines 75 - 80, The code weakens
type safety by casting pluginStore.getExtensions().filter(isModelFeatureFlag) to
any and also uses referenceForModel incorrectly for ExtensionK8sModel; update
the logic to preserve types and use the correct reference function: remove the
`as any` cast by narrowing the array with the isModelFeatureFlag type guard so
the items are typed as ExtensionK8sModel, then call
addToCRDs(referenceForExtensionModel(ff.properties.model), ff.properties.flag)
instead of referenceForModel; if immediate SDK migration isn't possible, add a
runtime type check/assert around ff.properties.model to ensure it matches
ExtensionK8sModel shape before calling referenceForExtensionModel to avoid
runtime errors.
logonoff
force-pushed
the
CONSOLE-3769-s3e2-the-convention
branch
from
6341d35 to
8284285
Compare
|
@logonoff: This pull request references CONSOLE-3769 which is a valid jira issue. DetailsIn 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. |
|
@logonoff: This pull request references CONSOLE-3769 which is a valid jira issue. DetailsIn 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. |
logonoff
changed the title
|
@logonoff: This pull request references CONSOLE-3769 which is a valid jira issue. DetailsIn 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. |
|
@logonoff: This pull request references CONSOLE-3769 which is a valid jira issue. DetailsIn 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. |
logonoff
force-pushed
the
CONSOLE-3769-s3e2-the-convention
branch
3 times, most recently
from
0e69f0d to
4622ac2
Compare
|
/label px-approved |
openshift-ci
bot
added
px-approved
logonoff
force-pushed
the
CONSOLE-3769-s3e2-the-convention
branch
from
4622ac2 to
362d7ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/shipwright-plugin/src/components/logs/tekton-results.ts (1)
440-446: Add explicit type parameter to match declared return type.
getTaskRunLogdeclaresPromise<string>but returns the result offetchTaskRunLogs({...})without a type parameter. WhenTis not specified, it defaults tounknown, making the actual return typePromise<unknown | string>, which is incompatible with the declaredPromise<string>in strict TypeScript.Call
fetchTaskRunLogs<string>({...})to explicitly set the type parameter and ensure type safety.
🤖 Fix all issues with AI agents
In @frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md:
- Line 15: Add the missing PR reference [#15945] to the references section of
CHANGELOG-webpack.md: insert the reference entry for [#15945] in the PR
references list between the existing [#15802] entry and the issue references
block so the in-text citation on line 15 resolves; ensure the new entry follows
the same link format and ordering as the other PR reference lines.
In @frontend/packages/console-dynamic-plugin-sdk/package.json:
- Around line 20-21: The package.json references non-existent versions for
@openshift/dynamic-plugin-sdk (^6.0.0) and @openshift/dynamic-plugin-sdk-webpack
(^5.0.0); confirm the intended releases and update these entries to published
versions (e.g., @openshift/dynamic-plugin-sdk to 5.0.1 and
@openshift/dynamic-plugin-sdk-webpack to a published 4.x version) or change them
to the correct prerelease tags/registry entries used in your build environment;
ensure the dependencies fields for "@openshift/dynamic-plugin-sdk" and
"@openshift/dynamic-plugin-sdk-webpack" are updated accordingly and validate by
running npm/yarn install before merging.
🧹 Nitpick comments (2)
frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (1)
75-78: Consider refining the type assertion for better type safety.The type assertion
resources as WatchK8sResourcesGenericon line 76 works but could be more precise. Sinceresourcesat this point could beundefined(when the factory doesn't specify resources), consider:properties: { ...rest, resources: resources as WatchK8sResourcesGeneric | undefined },Or simply omit the assertion if TypeScript can infer the type correctly after the function check:
properties: { ...rest, resources },This maintains type safety while still creating the uniform structure for both async and static resolution paths.
♻️ Optional refinement
return { - properties: { ...rest, resources: resources as WatchK8sResourcesGeneric }, + properties: { ...rest, resources }, pluginID: factory.pluginID, };frontend/packages/shipwright-plugin/src/components/logs/tekton-results.ts (1)
417-438: Consider removing or constraining the generic type parameter.The function signature declares
<T>but always returnsPromise<T | string>, which makes the generic type parameter less meaningful. Since the function now handles both JSON and non-JSON responses, consider one of these approaches:
- Remove the generic and return
Promise<unknown | string>orPromise<any | string>- Constrain the generic to ensure type safety:
<T extends object = object>- Add type narrowing logic or return a discriminated union
Currently,
getTaskRunLogcalls this function without specifyingT, meaning it defaults tounknown, and the function returns the result directly without type narrowing, which could lead to type safety issues.♻️ Suggested refactor options
Option 1: Remove generic and return explicit union
-const fetchTaskRunLogs = async <T>(taskRunLogRequest: TaskRunLogRequest): Promise<T | string> => { +const fetchTaskRunLogs = async (taskRunLogRequest: TaskRunLogRequest): Promise<unknown | string> => {Option 2: Return discriminated union for better type safety
+type TaskRunLogResult<T> = + | { type: 'json'; data: T } + | { type: 'string'; data: string }; + -const fetchTaskRunLogs = async <T>(taskRunLogRequest: TaskRunLogRequest): Promise<T | string> => { +const fetchTaskRunLogs = async <T>(taskRunLogRequest: TaskRunLogRequest): Promise<TaskRunLogResult<T>> => { // ... existing code ... - return isJSONString(taskRunLogResponse.body) - ? JSON.parse(taskRunLogResponse.body) - : taskRunLogResponse.body; + return isJSONString(taskRunLogResponse.body) + ? { type: 'json', data: JSON.parse(taskRunLogResponse.body) } + : { type: 'string', data: taskRunLogResponse.body }; };
📜 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
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
frontend/package.jsonfrontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsxfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/console-dynamic-plugin-sdk/package.jsonfrontend/packages/console-dynamic-plugin-sdk/src/build-types.tsfrontend/packages/console-shared/src/hooks/useUserSettings.tsfrontend/packages/shipwright-plugin/src/components/logs/tekton-results.tsfrontend/packages/topology/src/data-transforms/DataModelProvider.tsxfrontend/public/i18n.jsfrontend/webpack.config.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/public/i18n.jsfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdfrontend/packages/shipwright-plugin/src/components/logs/tekton-results.tsfrontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsxfrontend/packages/topology/src/data-transforms/DataModelProvider.tsxfrontend/packages/console-dynamic-plugin-sdk/package.jsonfrontend/packages/console-shared/src/hooks/useUserSettings.tsfrontend/webpack.config.tsfrontend/package.jsonfrontend/packages/console-dynamic-plugin-sdk/src/build-types.ts
🧬 Code graph analysis (2)
frontend/packages/shipwright-plugin/src/components/logs/tekton-results.ts (1)
pkg/devconsole/tekton-results/types.go (1)
TaskRunLogRequest(13-15)
frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (2)
frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology.ts (1)
TopologyDataModelFactory(36-54)frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
WatchK8sResourcesGeneric(216-221)
🔇 Additional comments (10)
frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (1)
31-39: LGTM! Type refinement improves type safety.The use of
Omitto excluderesourcesbefore adding it back with a specific type is a sound approach. This prevents type conflicts that could arise from the original union type (WatchK8sResourcesGeneric | CodeRef<...>) and ensures the resolved factories always have a consistent, predictable type.frontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsx (1)
9-9: LGTM!The import path update to use the ES module variant of
i18next-pseudois consistent with the broader i18n updates in this PR.frontend/public/i18n.js (1)
5-5: LGTM!The import path update aligns with the ES module variant changes throughout the codebase.
frontend/webpack.config.ts (2)
239-239: LGTM! Improved type safety.The type signature refinement from
{ resource?: string }towebpack.NormalModuleprovides better type safety while maintaining the same functionality.
325-325: Default ReactRefreshWebpackPlugin configuration is safe and appropriate.The simplification is correct. Hot reload configuration is properly handled by the devServer settings (
hot,webSocketServer: 'sockjs'), and ReactRefreshWebpackPlugin v0.6.2 uses sensible defaults that integrate seamlessly. No port conflicts exist, and no custom socket configuration is required.frontend/packages/console-shared/src/hooks/useUserSettings.ts (1)
1-1: The crypto module setup is properly configured for browser use.The
cryptoimport is correctly polyfilled byNodePolyfillPlugin, which is installed and configured inwebpack.config.ts. The plugin handles thecryptomodule by default—theadditionalAliases: ['process']configuration adds additional aliases beyond the default Node.js modules, not a whitelist excluding crypto. The removal ofcrypto-browserifyin favor of Node's built-incryptomodule with webpack polyfilling is the correct approach and requires no further changes.frontend/packages/console-dynamic-plugin-sdk/src/build-types.ts (2)
1-5: LGTM: Type-only imports correctly applied.The switch to type-only imports (
import type) is appropriate since these symbols are used exclusively in type positions. This helps optimize tree-shaking and makes the intent clearer.
46-52: Remove unsupported claims about breaking changes and legacy manifest support.The review comment suggests verifying compatibility with legacy plugin manifests (0.x), but no evidence of legacy manifest support exists in the codebase. No
PluginManifesttype is defined; all code consistently usesConsolePluginManifestwhich extendsRemotePluginManifest. All consuming code in plugin loading logic, dependencies resolution, and the plugin store already work with the current structure without any compatibility issues.Likely an incorrect or invalid review comment.
frontend/package.json (2)
226-314: Webpack ecosystem updates are compatible with current configuration.The webpack configuration in
frontend/webpack.config.tsis compatible with the updated versions:
- webpack 5.100.0 configuration follows standard patterns
- devServer settings are compatible with webpack-dev-server 5.2.2
- Plugin configurations (HtmlWebpackPlugin, MiniCssExtractPlugin, ForkTsCheckerWebpackPlugin, CopyWebpackPlugin, etc.) use standard setups matching their versions
- Loaders (esbuild-loader, thread-loader, babel-loader, sass-loader) are properly configured for their respective versions
- The minimal
node: { global: true }configuration does not conflict with webpack 5No deprecated webpack 5 options or breaking changes detected. The build process should work correctly with these versions.
193-193: Crypto functionality is adequately provided by the updated polyfill.The
node-polyfill-webpack-plugin@~4.0.0is properly configured in the webpack build and includescrypto-browserify@^3.12.0as a transitive dependency, ensuring all crypto functionality is available. The codebase correctly usesimport { createHash } from 'crypto'in browser code (e.g.,useUserSettings.ts), which gets polyfilled by webpack, while Node.js-based code (e.g., cypress configs) uses the nativecryptomodule. The PR's decision to remove the directcrypto-browserifydependency and rely on the plugin's polyfill is sound and properly implemented.
| "@openshift/dynamic-plugin-sdk": "^6.0.0", | ||
| "@openshift/dynamic-plugin-sdk-webpack": "^5.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify usage of dynamic-plugin-sdk APIs across the codebase
echo "=== Searching for dynamic-plugin-sdk imports and usage ==="
rg -n --type=ts --type=tsx -C2 "@openshift/dynamic-plugin-sdk" -g '!**/node_modules/**' -g '!**/dist/**' | head -100
echo -e "\n=== Searching for ConsolePluginManifest usage ==="
rg -n --type=ts --type=tsx -C2 "ConsolePluginManifest|PluginManifest|RemotePluginManifest" -g '!**/node_modules/**' -g '!**/dist/**' | head -100Repository: openshift/console
Length of output: 243
🏁 Script executed:
# Check if there are any changelog or release notes references
find . -name "CHANGELOG*" -o -name "RELEASE*" -o -name "HISTORY*" 2>/dev/null | head -20Repository: openshift/console
Length of output: 193
🏁 Script executed:
# Search for webpack configuration files that might need updates
fd -e "webpack.*\.(ts|js|mjs|json)" --type f | grep -v node_modules | head -20Repository: openshift/console
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Read the CHANGELOG files to understand breaking changes
echo "=== CHANGELOG-core.md (first 200 lines) ==="
head -200 ./frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md
echo -e "\n=== CHANGELOG-webpack.md (first 200 lines) ==="
head -200 ./frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.mdRepository: openshift/console
Length of output: 17255
🏁 Script executed:
#!/bin/bash
# Search for SDK imports with corrected file types
echo "=== Searching for dynamic-plugin-sdk imports (TS files) ==="
rg -n --type=ts -C2 "@openshift/dynamic-plugin-sdk" -g '!**/node_modules/**' -g '!**/dist/**' 2>/dev/null | head -150
echo -e "\n=== Searching for ConsolePluginManifest/PluginManifest ==="
rg -n --type=ts -C2 "ConsolePluginManifest|RemotePluginManifest" -g '!**/node_modules/**' -g '!**/dist/**' 2>/dev/null | head -150Repository: openshift/console
Length of output: 21464
🏁 Script executed:
#!/bin/bash
# Find and check build-types.ts
echo "=== Looking for build-types.ts ==="
fd "build-types.ts" --type f 2>/dev/null
echo -e "\n=== Looking for manifest-related files ==="
fd "manifest" --type f -e ts -e tsx 2>/dev/null | grep -v node_modules | head -20Repository: openshift/console
Length of output: 1282
🌐 Web query:
@openshift/dynamic-plugin-sdk v6.0.0 release notes breaking changes
💡 Result:
I couldn't find a published @openshift/dynamic-plugin-sdk v6.0.0 or release notes for it. The repository is at github.com/openshift/dynamic-plugin-sdk and the latest npm release for @openshift/dynamic-plugin-sdk appears to be v5.0.1. [1][2]
If you meant a different package (for example @openshift-console/dynamic-plugin-sdk) or a prerelease, tell me which package/name and I’ll look up that package's changelog/release notes and breaking changes.
Sources:
- GitHub repository overview. [1]
- npm package page for @openshift/dynamic-plugin-sdk (shows latest published v5.0.1). [2]
🌐 Web query:
@openshift/dynamic-plugin-sdk-webpack v5.0.0 release notes breaking changes
💡 Result:
Do you want me to search the web and show the v5.0.0 release notes and list the breaking changes for @openshift/dynamic-plugin-sdk-webpack?
🏁 Script executed:
#!/bin/bash
# Check the actual package.json content for the SDK versions
echo "=== Package.json SDK dependencies ==="
sed -n '15,30p' ./frontend/packages/console-dynamic-plugin-sdk/package.json
echo -e "\n=== Full dependencies section ==="
jq '.dependencies' ./frontend/packages/console-dynamic-plugin-sdk/package.json 2>/dev/null | head -30Repository: openshift/console
Length of output: 588
🏁 Script executed:
#!/bin/bash
# Search for any version constraints or range specifications
echo "=== Looking for SDK version info anywhere in build config ==="
grep -r "@openshift/dynamic-plugin-sdk" ./frontend/packages/console-dynamic-plugin-sdk --include="*.json" --include="*.ts" | grep -E "version|dependencies" | head -20Repository: openshift/console
Length of output: 122
🌐 Web query:
npm package @openshift/dynamic-plugin-sdk versions available releases
💡 Result:
Latest published version (npm): 5.0.1. [1]
To see the full list of published versions locally run:
- npm view @openshift/dynamic-plugin-sdk versions --json
- or yarn info @openshift/dynamic-plugin-sdk versions
Sources:
[1] npm package page for @openshift/dynamic-plugin-sdk.
[2] GitHub repository for openshift/dynamic-plugin-sdk.
The specified SDK versions do not appear to exist on npm—package installation will fail.
The package.json references @openshift/dynamic-plugin-sdk: ^6.0.0 and @openshift/dynamic-plugin-sdk-webpack: ^5.0.0, but the latest published versions on npm are @openshift/dynamic-plugin-sdk: 5.0.1 and likely @openshift/dynamic-plugin-sdk-webpack: 4.x.z respectively. Confirm that these version numbers are correct or update them to available releases. If these are intended as prerelease/development versions, ensure they are published or available in your build environment before merging.
🤖 Prompt for AI Agents
In @frontend/packages/console-dynamic-plugin-sdk/package.json around lines 20 -
21, The package.json references non-existent versions for
@openshift/dynamic-plugin-sdk (^6.0.0) and @openshift/dynamic-plugin-sdk-webpack
(^5.0.0); confirm the intended releases and update these entries to published
versions (e.g., @openshift/dynamic-plugin-sdk to 5.0.1 and
@openshift/dynamic-plugin-sdk-webpack to a published 4.x version) or change them
to the correct prerelease tags/registry entries used in your build environment;
ensure the dependencies fields for "@openshift/dynamic-plugin-sdk" and
"@openshift/dynamic-plugin-sdk-webpack" are updated accordingly and validate by
running npm/yarn install before merging.
362d7ad to
620f452
Compare
Remove `hoist-non-react-statics` due to non usage. Did not bump `css-loader` due to incompatibility with `thread-loader`. Removed `crypto-browserify` and used the webpack node polyfill instead.
logonoff
force-pushed
the
CONSOLE-3769-s3e2-the-convention
branch
from
620f452 to
905cbbb
Compare
| getTaskStatusKey, | ||
| } from '@patternfly/quickstarts'; | ||
| import Pseudo from 'i18next-pseudo'; | ||
| import Pseudo from 'i18next-pseudo/es'; |
There was a problem hiding this comment.
fixes "synthetic import needs to be enabled" errors
| @@ -1,6 +1,6 @@ | |||
| import { createHash } from 'crypto'; | |||
There was a problem hiding this comment.
not needed but this helps us not ship two copies of crypto-browserify at least
| */ | ||
| const fetchTaskRunLogs = async <T>(taskRunLogRequest: TaskRunLogRequest): Promise<T> => { | ||
| const fetchTaskRunLogs = async <T>(taskRunLogRequest: TaskRunLogRequest): Promise<T | string> => { | ||
| const TEKTON_RESULTS_TASKRUN_LOGS_URL = '/api/dev-console/tekton-results/logs'; |
There was a problem hiding this comment.
reflects actual return type
| const [resolvedFactories, setResolvedFactories] = useState< | ||
| | { | ||
| properties: ResolvedExtension<TopologyDataModelFactory>['properties'] & { | ||
| properties: Omit<ResolvedExtension<TopologyDataModelFactory>['properties'], 'resources'> & { |
There was a problem hiding this comment.
this is a cleaner type definition because this removes the types of resources that we will not exist in this state
| new ReactRefreshWebpackPlugin({ | ||
| overlay: { | ||
| sockPort: WDS_PORT, | ||
| }, | ||
| }), | ||
| ] | ||
| : []), | ||
| ...(REACT_REFRESH ? [new ReactRefreshWebpackPlugin()] : []), |
There was a problem hiding this comment.
react-refresh-webpack-plugin now integrates directly with webpack-dev-server, so providing sockPort is no longer necessary.
https://github.com/pmmmwh/react-refresh-webpack-plugin/releases/tag/v0.6.0
| }, | ||
| 'vendor-plugins-shared': { | ||
| test(module: { resource?: string }) { | ||
| test(module: webpack.NormalModule) { |
There was a problem hiding this comment.
more accurate type definition for module
| "monaco-yaml": "^5.3.1", | ||
| "murmurhash-js": "1.0.x", | ||
| "node-polyfill-webpack-plugin": "^4.0.0", | ||
| "node-polyfill-webpack-plugin": "~4.0.0", |
There was a problem hiding this comment.
4.1.0 adds new polyfills we don't need
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
dev dependency only changes so we can just run this by CI /verified by CI |
openshift-ci-robot
added
the
verified
|
@logonoff: This PR has been marked as verified by DetailsIn 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. |
|
/label plugin-api-approved |
openshift-ci
bot
added
the
plugin-api-approved
|
/retest |
1 similar comment
|
/retest |
|
@logonoff: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
5 participants
This PR bumps all OpenShift dynamic plugin SDK, esbuild, and webpack-related deps. Removed
hoist-non-react-staticsdue to non usage