CONSOLE-4840: Remove all plugin entry points + topology static extension cleanup

[logonoff]

• Align @console/topology types with SDK ones
• Remove TopologyDisplayFilters TopologyDecoratorProvider TopologyComponentFactory TopologyDisplayFilters TopologyDecoratorProvider
• Remove all plugin entry points as they export only [] and do not have imports. Thus they are a noop
• Set cycle threshold to 0

TopologyDataModelFactory will be removed in #15641. Thanks @Leo6Leo!

[openshift-ci-robot]

@logonoff: This pull request references CONSOLE-4840 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:

• Align @console/topology types with SDK ones
• Remove TopologyDisplayFilters TopologyDecoratorProvider TopologyComponentFactory TopologyDisplayFilters TopologyDecoratorProvider

TopologyDataModelFactory will be removed in #15641. Thanks @leo66leo!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@logonoff: This pull request references CONSOLE-4840 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:

• Align @console/topology types with SDK ones
• Remove TopologyDisplayFilters TopologyDecoratorProvider TopologyComponentFactory TopologyDisplayFilters TopologyDecoratorProvider

TopologyDataModelFactory will be removed in #15641. Thanks @Leo6Leo!

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]

/label px-approved
/label docs-approved

[logonoff]

Required to fix 57 type errors on knative and topology unit tests. Not a breaking change because all extra properties of K8sResourceKind are optional

[logonoff]

analysis from #15658:

Plugin API Review Report

API change: Updated TopologyResourcesObject, TopologyDataObject, and OverviewItem types to use K8sResourceKind instead of K8sResourceCommon
JIRA issue: CONSOLE-4840
PR number: #15699
Changes to changelog made? Yes

Summary

The changes consolidate topology-related types into the @openshift-console/dynamic-plugin-sdk package and update several type definitions to use K8sResourceKind instead of K8sResourceCommon. This is a backwards-compatible refactoring that moves type definitions from internal packages to the SDK for better plugin consumption.

 

Key changes:

1. Updated TopologyResourcesObject, OverviewItem, and TopologyDataObject types in topology-types.ts to use K8sResourceKind instead of K8sResourceCommon
2. Consolidated type definitions - the @console/topology package now re-exports types from the SDK instead of defining them locally
3. Removed extension type definitions from @console/topology/src/extensions/topology.ts (these remain properly defined in the SDK)

Backwards Compatibility Analysis

This change is backwards compatible because:

• K8sResourceKind is a superset of K8sResourceCommon (it extends K8sResourceCommon with optional spec?, status?, and data? properties)
• Due to TypeScript's structural typing, any K8sResourceCommon object can be assigned to a K8sResourceKind type
• The types are still exported from their original locations for internal Console packages
• External plugins import these types from @openshift-console/dynamic-plugin-sdk, which is the correct and only supported way

Findings

•  Reviewed changes to the API for backwards-compatibility and verified no breaking changes
•  Verified that the change is documented in the SDK core changelog
•  Verified that the change does not need to be documented in the "Changes to shared modules and API" section of the dynamic plugin SDK release notes (no shared module changes)
•  Verified that the change does not need to be documented in the SDK webpack changelog (no webpack-related changes)
•  Verified that the styleguide is in compliance for all changed files
•  Verified that no CSS class changes require documentation
•  Verified that JIRA issue CONSOLE-4840 is associated with the change
•  Verified that the JIRA issue is correctly referenced in the changelog ([CONSOLE-4840])
•  Verified that the PR number CONSOLE-4840: Remove all plugin entry points + topology static extension cleanup #15699 is correctly referenced in the changelog
•  Verified that the commit message follows the contribution guide format

Issues found during review

None - all requirements are satisfied.

Actions taken

No actions required. The change is properly documented and backwards compatible.

Compliance score: 10/10

Reason for score:

• All findings were satisfactorily addressed
• No breaking changes identified
• Proper documentation in CHANGELOG-core.md with correct JIRA and PR references
• Change is in the correct version section (4.21.0-prerelease.x)
• The refactoring consolidates types into the SDK, improving maintainability
• Backwards compatibility is maintained through TypeScript's structural typing
• The styleguide is followed
• Commit message follows proper format

This is a well-executed refactoring that improves the plugin API organization without introducing any breaking changes.

[logonoff]

/assign @yapei

[yapei]

@XiyunZhao Please help verify, thanks!

[XiyunZhao]

/verified by @XiyunZhao
no issue was found after verify that the function on topology related page (eg: graphs render, hover, click, dropdown, and etc.), also no issue was found on knative-specific topology features

[openshift-ci-robot]

@logonoff: This PR has been marked as verified by active-plugins.spec.ts,XiyunZhao.

In response to this:

grammar fix

/verified by active-plugins.spec.ts,XiyunZhao

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]

rebase

/verified by active-plugins.spec.ts,XiyunZhao

[openshift-ci-robot]

@logonoff: This PR has been marked as verified by active-plugins.spec.ts,XiyunZhao.

In response to this:

rebase

/verified by active-plugins.spec.ts,XiyunZhao

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts (1)

15-23: Add test coverage for the positive case.

The isPluginPackage test suite only covers the negative case (when consolePlugin is missing). Consider adding a test to explicitly verify that isPluginPackage returns true when consolePlugin: {} is present, ensuring the new plugin package shape is correctly recognized.

+  it('returns true if package.consolePlugin is present', () => {
+    expect(
+      isPluginPackage({
+        ...getTemplatePackage(),
+        consolePlugin: {},
+      }),
+    ).toBe(true);
+  });

frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (1)

50-50: Topology type alignment to K8sResourceKind is coherent and non-invasive

Switching TopologyResourcesObject, OverviewItem (default T and hpas), TopologyDataObject.pods/resource, and the various helper signatures (GetTopologyResourceObject, GetResource, edge/group/node helpers, GetWorkloadResources) over to K8sResourceKind tightens typings in a way that matches how these APIs are actually used, without introducing new runtime behavior.

Retaining K8sResourceCommon in a few ancillary types (like TopologyDataModelDepicted and BuildConfigOverviewItem) is acceptable and can be unified to K8sResourceKind later if you decide consistency there is worth the churn.

Also applies to: 104-107, 131-140, 264-267, 268-283, 305-309

frontend/packages/topology/src/topology-types.ts (1)

1-31: Re-exporting topology types from SDK simplifies the type surface

Consolidating all topology-related types and constants into re-exports from the SDK module keeps frontend/packages/topology/src/topology-types.ts as a stable import point while ensuring there’s a single source of truth for the actual definitions, which improves maintainability and reduces duplication.

frontend/packages/topology/src/components/page/TopologyView.tsx (1)

231-273: Consider decoupling filtersLoaded from “has any displayFilterExtensions” and reducing redundant updates

Two subtle behaviors in this effect are worth tightening up:

1. filtersLoaded never flips to true if there are zero display filter extensions

   • filtersLoaded is only set inside the displayFilterExtensions.forEach body.
   • If displayFilterExtensionsResolved is true but displayFilterExtensions is empty, the if block runs but the forEach body never executes, so filtersLoaded remains false.
   • Because the downstream effect that calls updateModelFromFilters is guarded by if (filtersLoaded), filteredModel would never be set in a “no display filters” configuration, and the component would keep returning null.

   Today there is at least one TopologyDisplayFilters provider, so this likely doesn’t surface, but the current implementation relies on that assumption.

2. onFiltersChange and setFiltersLoaded(true) are invoked once per extension

   • Both are called inside the displayFilterExtensions.forEach loop. With multiple extensions, this triggers multiple identical or incremental updates and re-renders.
   • Functionally it works, but it’s noisier than necessary and makes the control flow harder to follow.

You can address both points by aggregating filters across all extensions first, then calling onFiltersChange / setFiltersLoaded(true) once, even when there are no extensions:

   React.useEffect(() => {
-    if (displayFilterExtensionsResolved) {
-      const updateFilters = [...filters];
-      displayFilterExtensions.forEach((extension) => {
-        const extFilters = extension.properties.getTopologyFilters();
-        extFilters?.forEach((filter) => {
-          if (!updateFilters.find((f) => f.id === filter.id)) {
-            if (appliedFilters[filter.id] !== undefined) {
-              filter.value = appliedFilters[filter.id];
-            }
-            updateFilters.push(filters.find((f) => f.id === filter.id) || filter);
-          }
-        });
-        onFiltersChange(updateFilters);
-        setFiltersLoaded(true);
-      });
-    }
+    if (displayFilterExtensionsResolved) {
+      const updateFilters = [...filters];
+
+      displayFilterExtensions.forEach((extension) => {
+        const extFilters = extension.properties.getTopologyFilters();
+        extFilters?.forEach((filter) => {
+          if (!updateFilters.find((f) => f.id === filter.id)) {
+            if (appliedFilters[filter.id] !== undefined) {
+              filter.value = appliedFilters[filter.id];
+            }
+            updateFilters.push(filters.find((f) => f.id === filter.id) || filter);
+          }
+        });
+      });
+
+      onFiltersChange(updateFilters);
+      setFiltersLoaded(true);
+    }
     // Only update on extension changes
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [displayFilterExtensionsResolved, displayFilterExtensions]);
+  }, [displayFilterExtensionsResolved, displayFilterExtensions]);

This keeps the existing behavior when extensions are present, avoids redundant updates, and makes the filtersLoaded flag robust even if the system ever runs without display-filter providers.

📜 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7def0 and 15d5482.

📒 Files selected for processing (48)

• frontend/packages/console-app/package.json (0 hunks)
• frontend/packages/console-app/src/plugin.ts (0 hunks)
• frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md (3 hunks)
• frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (3 hunks)
• frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.ts (9 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts (5 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/active-plugins.ts (2 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/plugin-resolver.ts (1 hunks)
• frontend/packages/console-plugin-sdk/src/typings/base.ts (0 hunks)
• frontend/packages/console-telemetry-plugin/package.json (0 hunks)
• frontend/packages/console-telemetry-plugin/src/plugin.ts (0 hunks)
• frontend/packages/container-security/package.json (0 hunks)
• frontend/packages/container-security/src/plugin.ts (0 hunks)
• frontend/packages/dev-console/package.json (0 hunks)
• frontend/packages/dev-console/src/plugin.ts (0 hunks)
• frontend/packages/helm-plugin/package.json (0 hunks)
• frontend/packages/helm-plugin/src/plugin.ts (0 hunks)
• frontend/packages/insights-plugin/package.json (0 hunks)
• frontend/packages/insights-plugin/src/plugin.ts (0 hunks)
• frontend/packages/knative-plugin/package.json (0 hunks)
• frontend/packages/knative-plugin/src/plugin.ts (0 hunks)
• frontend/packages/knative-plugin/src/topology/knative-topology-utils.ts (2 hunks)
• frontend/packages/knative-plugin/src/topology/topology-types.ts (1 hunks)
• frontend/packages/metal3-plugin/package.json (0 hunks)
• frontend/packages/metal3-plugin/src/plugin.ts (0 hunks)
• frontend/packages/operator-lifecycle-manager-v1/package.json (0 hunks)
• frontend/packages/operator-lifecycle-manager-v1/src/plugin.ts (0 hunks)
• frontend/packages/operator-lifecycle-manager/package.json (0 hunks)
• frontend/packages/operator-lifecycle-manager/src/plugin.ts (0 hunks)
• frontend/packages/pipelines-plugin/package.json (0 hunks)
• frontend/packages/pipelines-plugin/src/plugin.ts (0 hunks)
• frontend/packages/shipwright-plugin/package.json (0 hunks)
• frontend/packages/shipwright-plugin/src/plugin.ts (0 hunks)
• frontend/packages/topology/package.json (0 hunks)
• frontend/packages/topology/src/components/graph-view/Topology.tsx (4 hunks)
• frontend/packages/topology/src/components/page/TopologyView.tsx (5 hunks)
• frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (3 hunks)
• frontend/packages/topology/src/data-transforms/transform-utils.ts (2 hunks)
• frontend/packages/topology/src/extensions/index.ts (0 hunks)
• frontend/packages/topology/src/extensions/topology.ts (0 hunks)
• frontend/packages/topology/src/plugin.ts (0 hunks)
• frontend/packages/topology/src/topology-types.ts (1 hunks)
• frontend/packages/vsphere-plugin/package.json (0 hunks)
• frontend/packages/vsphere-plugin/src/plugin.ts (0 hunks)
• frontend/packages/webterminal-plugin/package.json (0 hunks)
• frontend/packages/webterminal-plugin/src/plugin.ts (0 hunks)
• frontend/webpack.config.ts (0 hunks)

💤 Files with no reviewable changes (34)

• frontend/packages/knative-plugin/package.json
• frontend/packages/operator-lifecycle-manager/package.json
• frontend/packages/vsphere-plugin/src/plugin.ts
• frontend/packages/helm-plugin/package.json
• frontend/packages/console-app/package.json
• frontend/packages/console-plugin-sdk/src/typings/base.ts
• frontend/packages/operator-lifecycle-manager/src/plugin.ts
• frontend/packages/container-security/src/plugin.ts
• frontend/webpack.config.ts
• frontend/packages/helm-plugin/src/plugin.ts
• frontend/packages/topology/package.json
• frontend/packages/shipwright-plugin/src/plugin.ts
• frontend/packages/insights-plugin/package.json
• frontend/packages/metal3-plugin/src/plugin.ts
• frontend/packages/console-telemetry-plugin/src/plugin.ts
• frontend/packages/operator-lifecycle-manager-v1/src/plugin.ts
• frontend/packages/knative-plugin/src/plugin.ts
• frontend/packages/dev-console/src/plugin.ts
• frontend/packages/console-telemetry-plugin/package.json
• frontend/packages/pipelines-plugin/src/plugin.ts
• frontend/packages/metal3-plugin/package.json
• frontend/packages/webterminal-plugin/src/plugin.ts
• frontend/packages/shipwright-plugin/package.json
• frontend/packages/topology/src/extensions/topology.ts
• frontend/packages/insights-plugin/src/plugin.ts
• frontend/packages/console-app/src/plugin.ts
• frontend/packages/webterminal-plugin/package.json
• frontend/packages/vsphere-plugin/package.json
• frontend/packages/operator-lifecycle-manager-v1/package.json
• frontend/packages/dev-console/package.json
• frontend/packages/pipelines-plugin/package.json
• frontend/packages/topology/src/extensions/index.ts
• frontend/packages/container-security/package.json
• frontend/packages/topology/src/plugin.ts

🚧 Files skipped from review as they are similar to previous changes (6)

• frontend/packages/console-dynamic-plugin-sdk/src/types.ts
• frontend/packages/console-plugin-sdk/src/codegen/active-plugins.ts
• frontend/packages/topology/src/components/graph-view/Topology.tsx
• frontend/packages/knative-plugin/src/topology/knative-topology-utils.ts
• frontend/packages/knative-plugin/src/topology/topology-types.ts
• frontend/packages/console-plugin-sdk/src/codegen/plugin-resolver.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/codegen/__tests__/plugin-resolver.spec.ts
• frontend/packages/topology/src/data-transforms/DataModelProvider.tsx
• frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.ts
• frontend/packages/topology/src/topology-types.ts
• frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts
• frontend/packages/topology/src/data-transforms/transform-utils.ts
• frontend/packages/topology/src/components/page/TopologyView.tsx

🔇 Additional comments (16)

frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts (2)

36-87: LGTM!

The readPackages test correctly validates that plugin packages with consolePlugin: {} are identified and returned, aligning with the removal of the entry field requirement.

---

90-169: LGTM!

The filterActivePluginPackages tests correctly verify plugin filtering logic with the updated consolePlugin: {} shape across multiple scenarios (app dependencies, app as plugin, ordering).

frontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.ts (4)

32-103: LGTM!

The test correctly verifies that getActivePluginsModule generates the expected module code with dynamic-only extensions for plugins with consolePlugin: {}, consistent with the removal of static entry-based plugin loading.

---

105-167: LGTM!

The test correctly verifies that loadActivePluginsForTestPurposes loads and returns plugins with dynamic-only extensions, aligning with the refactor to remove static plugin entry points.

---

169-239: LGTM!

The getExecutableCodeRefSource tests correctly validate code reference transformation. The helper function properly uses consolePlugin: { exposedModules } to provide module mapping when needed for code reference resolution.

---

241-475: LGTM!

The getDynamicExtensions tests comprehensively verify dynamic extension loading from console-extensions.json files across multiple scenarios (valid extensions, missing file, schema errors, transformation errors), correctly using consolePlugin: {} throughout.

frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (5)

1-8: LGTM: Import updates align with SDK consolidation.

The switch to named imports and updated SDK types (isTopologyDataModelFactory, TopologyDataModelFactory, ResolvedExtension, WatchK8sResourcesGeneric) correctly implements the PR objective to consolidate topology types with the dynamic-plugin-sdk.

---

18-23: LGTM: Component signature and state initialization.

The switch from React.FC to FC and named hook imports is consistent with modern React patterns and the updated imports.

---

26-28: LGTM: Updated extension resolution.

The variable renaming (modelFactories, factoriesResolved) and type updates (TopologyDataModelFactory, isTopologyDataModelFactory) improve clarity and align with the SDK consolidation.

---

31-39: LGTM: Enhanced type safety for resolved factories.

The explicit typing using ResolvedExtension<TopologyDataModelFactory>['properties'] with the optional resources field improves type safety and clearly documents the shape of resolved factories.

---

42-83: LGTM: Enhanced resource resolution with error handling.

The refactored effect correctly updates variable references and adds valuable error handling for resource resolution failures. The fallback to undefined resources allows graceful degradation while logging helps with debugging.

frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md (1)

21-22: Changelog entries accurately describe topology type updates

The new bullets for allowing K8sResourceKind in topology types and async resources for console.topology/data/factory, plus the added [CONSOLE-4806] reference, are consistent with the PR scope and keep the changelog aligned with the implemented API changes.

Also applies to: 140-140

frontend/packages/topology/src/data-transforms/transform-utils.ts (1)

10-16: Import realignment with SDK topology types looks correct

Pulling OverviewItem from the SDK topology-types module and importing isKnativeServing from pod-utils aligns this file with the new type/utility layout without changing runtime behavior; the createTopologyNodeData signature and usage remain consistent.

Also applies to: 32-32, 104-112

frontend/packages/topology/src/components/page/TopologyView.tsx (3)

19-29: Static topology imports and useResolvedExtensions wiring look consistent

Switching to useResolvedExtensions and importing topology extension types / type guards from @console/dynamic-plugin-sdk/src/extensions/topology aligns this component with the static SDK topology extension surface and keeps a single source of truth for these contracts. No issues spotted here.

---

133-145: Extension resolution for filters/connectors/decorators is wired correctly

The useResolvedExtensions hooks for TopologyDisplayFilters, TopologyCreateConnector, TopologyDecoratorProvider, and TopologyRelationshipProvider are set up consistently: you gate filter/connector/decorator usage on the corresponding ...Resolved flag where needed and pass the resolved extension arrays through graphData and subsequent effects. This matches the intended SDK usage and keeps the topology view driven entirely by the resolved extension set.

---

206-229: Decorator aggregation from extension providers is sound

The decorator effect reduces extensionDecorators into quadrant buckets, initializes all quadrants up front, and sorts by priority before storing in state. This ensures deterministic ordering and avoids quadrant key checks downstream. The dependency list (extensionDecorators, extensionDecoratorsResolved) is correct for only recomputing when the decorator extension set changes.

[logonoff]

readme fix

/verified by active-plugins.spec.ts,XiyunZhao

[openshift-ci-robot]

@logonoff: This PR has been marked as verified by active-plugins.spec.ts,XiyunZhao.

In response to this:

readme fix

/verified by active-plugins.spec.ts,XiyunZhao

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

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

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

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.

[logonoff]

rebase

/verified by active-plugins.spec.ts,XiyunZhao

[openshift-ci-robot]

@logonoff: This PR has been marked as verified by active-plugins.spec.ts,XiyunZhao.

In response to this:

rebase

/verified by active-plugins.spec.ts,XiyunZhao

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)

frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (1)

131-140: Fix type compatibility between OdcNodeModel and TopologyDataObject.

The PR introduces a type mismatch: OdcNodeModel.resource is K8sResourceCommon, but TopologyDataObject.resource is now K8sResourceKind | null. These types are incompatible—K8sResourceKind is a superset of K8sResourceCommon with additional optional fields (spec, status, data), so assigning a K8sResourceCommon to a K8sResourceKind field is not type-safe.

The conversion function dataObjectFromModel (line 58 in frontend/packages/topology/src/data-transforms/transform-utils.ts) directly assigns node.resource without type conversion, which will cause TypeScript compilation errors.

Update OdcNodeModel.resource to use K8sResourceKind consistently with TopologyDataObject.resource, or adjust the conversion logic to handle the type difference appropriately.

🧹 Nitpick comments (1)

frontend/packages/topology/src/components/graph-view/Topology.tsx (1)

294-301: Consider separating static vs dynamic component factory registration

The effect correctly waits for extensionsResolved before registering plugin-driven factories, and the dependency list should limit it to one meaningful run in normal usage. However, it currently re-registers the static componentFactory each time the effect runs:

visualization.registerComponentFactory(componentFactory);
componentFactoryExtensions.forEach((factory) => {
  visualization.registerComponentFactory(factory.properties.getFactory);
});

To avoid any chance of duplicate registrations (if extension resolution ever changes at runtime) and to make intent clearer, you could:

• Register the static componentFactory once when creating the Visualization (inside createVisualization), and
• Keep this effect focused solely on registering componentFactoryExtensions once extensionsResolved is true.

This is a maintainability/performance polish rather than a correctness issue.

Also applies to: 311-311

📜 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

📥 Commits

Reviewing files that changed from the base of the PR and between fc1e954 and 303a839.

📒 Files selected for processing (48)

• frontend/packages/console-app/package.json (0 hunks)
• frontend/packages/console-app/src/plugin.ts (0 hunks)
• frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md (3 hunks)
• frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (3 hunks)
• frontend/packages/console-dynamic-plugin-sdk/src/types.ts (1 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/active-plugins.spec.ts (9 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts (5 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/active-plugins.ts (2 hunks)
• frontend/packages/console-plugin-sdk/src/codegen/plugin-resolver.ts (1 hunks)
• frontend/packages/console-plugin-sdk/src/typings/base.ts (0 hunks)
• frontend/packages/console-telemetry-plugin/package.json (0 hunks)
• frontend/packages/console-telemetry-plugin/src/plugin.ts (0 hunks)
• frontend/packages/container-security/package.json (0 hunks)
• frontend/packages/container-security/src/plugin.ts (0 hunks)
• frontend/packages/dev-console/package.json (0 hunks)
• frontend/packages/dev-console/src/plugin.ts (0 hunks)
• frontend/packages/helm-plugin/package.json (0 hunks)
• frontend/packages/helm-plugin/src/plugin.ts (0 hunks)
• frontend/packages/insights-plugin/package.json (0 hunks)
• frontend/packages/insights-plugin/src/plugin.ts (0 hunks)
• frontend/packages/knative-plugin/package.json (0 hunks)
• frontend/packages/knative-plugin/src/plugin.ts (0 hunks)
• frontend/packages/knative-plugin/src/topology/knative-topology-utils.ts (2 hunks)
• frontend/packages/knative-plugin/src/topology/topology-types.ts (1 hunks)
• frontend/packages/metal3-plugin/package.json (0 hunks)
• frontend/packages/metal3-plugin/src/plugin.ts (0 hunks)
• frontend/packages/operator-lifecycle-manager-v1/package.json (0 hunks)
• frontend/packages/operator-lifecycle-manager-v1/src/plugin.ts (0 hunks)
• frontend/packages/operator-lifecycle-manager/package.json (0 hunks)
• frontend/packages/operator-lifecycle-manager/src/plugin.ts (0 hunks)
• frontend/packages/pipelines-plugin/package.json (0 hunks)
• frontend/packages/pipelines-plugin/src/plugin.ts (0 hunks)
• frontend/packages/shipwright-plugin/package.json (0 hunks)
• frontend/packages/shipwright-plugin/src/plugin.ts (0 hunks)
• frontend/packages/topology/package.json (0 hunks)
• frontend/packages/topology/src/components/graph-view/Topology.tsx (4 hunks)
• frontend/packages/topology/src/components/page/TopologyView.tsx (5 hunks)
• frontend/packages/topology/src/data-transforms/DataModelProvider.tsx (3 hunks)
• frontend/packages/topology/src/data-transforms/transform-utils.ts (2 hunks)
• frontend/packages/topology/src/extensions/index.ts (0 hunks)
• frontend/packages/topology/src/extensions/topology.ts (0 hunks)
• frontend/packages/topology/src/plugin.ts (0 hunks)
• frontend/packages/topology/src/topology-types.ts (1 hunks)
• frontend/packages/vsphere-plugin/package.json (0 hunks)
• frontend/packages/vsphere-plugin/src/plugin.ts (0 hunks)
• frontend/packages/webterminal-plugin/package.json (0 hunks)
• frontend/packages/webterminal-plugin/src/plugin.ts (0 hunks)
• frontend/webpack.config.ts (0 hunks)

💤 Files with no reviewable changes (34)

• frontend/packages/console-app/src/plugin.ts
• frontend/packages/webterminal-plugin/src/plugin.ts
• frontend/packages/console-telemetry-plugin/package.json
• frontend/packages/operator-lifecycle-manager/package.json
• frontend/packages/console-app/package.json
• frontend/packages/dev-console/src/plugin.ts
• frontend/packages/metal3-plugin/package.json
• frontend/packages/console-telemetry-plugin/src/plugin.ts
• frontend/packages/container-security/src/plugin.ts
• frontend/packages/topology/package.json
• frontend/packages/pipelines-plugin/src/plugin.ts
• frontend/packages/dev-console/package.json
• frontend/packages/shipwright-plugin/src/plugin.ts
• frontend/packages/vsphere-plugin/src/plugin.ts
• frontend/packages/operator-lifecycle-manager-v1/package.json
• frontend/packages/shipwright-plugin/package.json
• frontend/packages/insights-plugin/package.json
• frontend/packages/helm-plugin/src/plugin.ts
• frontend/packages/vsphere-plugin/package.json
• frontend/packages/topology/src/extensions/index.ts
• frontend/packages/operator-lifecycle-manager/src/plugin.ts
• frontend/packages/insights-plugin/src/plugin.ts
• frontend/packages/helm-plugin/package.json
• frontend/packages/metal3-plugin/src/plugin.ts
• frontend/packages/pipelines-plugin/package.json
• frontend/packages/console-plugin-sdk/src/typings/base.ts
• frontend/webpack.config.ts
• frontend/packages/topology/src/extensions/topology.ts
• frontend/packages/topology/src/plugin.ts
• frontend/packages/knative-plugin/src/plugin.ts
• frontend/packages/container-security/package.json
• frontend/packages/operator-lifecycle-manager-v1/src/plugin.ts
• frontend/packages/knative-plugin/package.json
• frontend/packages/webterminal-plugin/package.json

🚧 Files skipped from review as they are similar to previous changes (8)

• frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md
• frontend/packages/console-plugin-sdk/src/codegen/tests/active-plugins.spec.ts
• frontend/packages/topology/src/data-transforms/DataModelProvider.tsx
• frontend/packages/topology/src/data-transforms/transform-utils.ts
• frontend/packages/topology/src/topology-types.ts
• frontend/packages/knative-plugin/src/topology/knative-topology-utils.ts
• frontend/packages/console-dynamic-plugin-sdk/src/types.ts
• frontend/packages/console-plugin-sdk/src/codegen/active-plugins.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/topology/src/components/graph-view/Topology.tsx
• frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts
• frontend/packages/console-plugin-sdk/src/codegen/plugin-resolver.ts
• frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts
• frontend/packages/topology/src/components/page/TopologyView.tsx
• frontend/packages/knative-plugin/src/topology/topology-types.ts

🔇 Additional comments (11)

frontend/packages/topology/src/components/graph-view/Topology.tsx (1)

26-30: Topology component factory resolution wiring looks consistent

Importing useResolvedExtensions, isTopologyComponentFactory, and TopologyComponentFactory and then wiring them via:

const [componentFactoryExtensions, extensionsResolved] =
  useResolvedExtensions<TopologyComponentFactory>(isTopologyComponentFactory);

is type-safe, aligns with the SDK topology extension pattern, and keeps the dynamic plugin dependency surface clear in this component.

Also applies to: 150-152

frontend/packages/topology/src/components/page/TopologyView.tsx (5)

19-29: Import consolidation looks good.

The imports have been cleanly refactored to use the dynamic-plugin-SDK types and APIs directly, aligning with the PR objectives to consolidate topology extension definitions.

---

133-141: Extension resolution properly unified.

All extension types now use the consistent useResolvedExtensions pattern with appropriate type guards and resolved flags.

---

207-229: Decorator aggregation correctly refactored.

The effect properly uses the static extension decorators with correct dependency tracking and maintains the existing quadrant-based grouping logic.

---

232-250: Display filter resolution correctly updated.

The effect properly uses static display filter extensions with correct dependency tracking, maintaining the intentional design to update only when extensions change.

---

252-273: Model filtering correctly uses static extensions.

The filtered model update properly maps over the static display filter extensions and includes them in the dependency array.

frontend/packages/console-plugin-sdk/src/codegen/plugin-resolver.ts (1)

22-25: isPluginPackage guard matches new plugin shape and looks correct

The new guard (!_.isNil(consolePlugin) && _.isObject(consolePlugin)) correctly treats any non-null object consolePlugin as a plugin marker, aligning with the updated PluginPackage.consolePlugin type and the entry-less configuration model. No functional or maintainability issues here.

frontend/packages/console-plugin-sdk/src/codegen/__tests__/plugin-resolver.spec.ts (1)

45-53: Tests correctly exercise the new “presence-only” consolePlugin contract

Updating fixtures to use consolePlugin: {} ensures the tests now validate the intended behavior: any package with a consolePlugin object is treated as a plugin, including the app package when applicable. The coverage around dependencies and ordering still holds and now aligns with the updated type/guard semantics.

Also applies to: 102-118, 122-129, 134-161

frontend/packages/knative-plugin/src/topology/topology-types.ts (1)

1-2: Import consolidation is correct and functional.

Verification confirms:

• Both imported types exist and are properly exported from the SDK
• OverviewItem (with K8sResourceKind default) and PodControllerOverviewItem are available at the specified paths
• KnativeServiceOverviewItem properly extends OverviewItem and includes PodControllerOverviewItem for the current property
• All usages in knative-topology-utils.ts and test files are compatible with the type definitions

No issues detected.

frontend/packages/console-dynamic-plugin-sdk/src/extensions/topology-types.ts (2)

50-50: Type change from K8sResourceCommon to K8sResourceKind is valid and type-safe.

The change narrows TopologyResourcesObject to use K8sResourceKind[] instead of K8sResourceCommon[]. Since K8sResourceKind extends K8sResourceCommon, this is a valid type narrowing. All identified consumers in the codebase (data-transforms, TopologyDataRetriever) are compatible with the more specific type. No type mismatches detected.

---

104-106: No issues found. The generic default change is backward compatible.

K8sResourceKind is defined as K8sResourceCommon & { spec?, status?, data? }, making it a structural supertype. All code using OverviewItem without explicit generic parameters will receive K8sResourceKind instead of the previous K8sResourceCommon default. Since K8sResourceKind extends K8sResourceCommon, this change is safe and backward compatible. No usages of OverviewItem<K8sResourceCommon> were found in the codebase, confirming that this modification poses no breaking changes.

Likely an incorrect or invalid review comment.

[vojtechszocs]

/lgtm

[openshift-ci]

[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

Needs approval from an approver in each of these files:

• frontend/OWNERS [logonoff,vojtechszocs]

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