Skip to content

Conversation

@Leo6Leo
Copy link
Contributor

@Leo6Leo Leo6Leo commented Oct 24, 2025

Description

As a part of the alignment of OpenShift console's dynamic plugin sdk and the OpenShift dynamic plugin sdk, we want to get rid of the long-standing legacy "static plugin" API which is still used in console.

To do this, we can gradually migrate each "static plugin extension" to the equivalent "dynamic extension". This ensures better alignment of web console code with its own SDK.

AC: All static extension in the plugin mentioned in the ticket name is removed and replaced with the dynamic counterpart

…ate console-extensions.json to include new topology data model factories for serving, eventing, and kamelets, and remove deprecated topology-plugin.ts file
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 24, 2025
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Oct 24, 2025

@Leo6Leo: This pull request references CONSOLE-4806 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:

Description

As a part of the alignment of OpenShift console's dynamic plugin sdk and the OpenShift dynamic plugin sdk, we want to get rid of the long-standing legacy "static plugin" API which is still used in console.

To do this, we can gradually migrate each "static plugin extension" to the equivalent "dynamic extension". This ensures better alignment of web console code with its own SDK.

AC: All static extension in the plugin mentioned in the ticket name is removed and replaced with the dynamic counterpart

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

@openshift-ci openshift-ci bot requested review from TheRealJon and spadgett October 24, 2025 14:25
@openshift-ci openshift-ci bot added the component/knative Related to knative-plugin label Oct 24, 2025
Comment on lines 2017 to 2019
"resources": {
"$codeRef": "getKnativeResources.getKnativeServingResources"
},
Copy link
Member

Choose a reason for hiding this comment

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

resources expects a WatchK8sResourcesGeneric and not a codeRef

Copy link
Member

@logonoff logonoff Oct 30, 2025

Choose a reason for hiding this comment

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

also since getKnativeServingResources always returns the same object we can embed the contents of that directly into the json file--no codeRef needed

Comment on lines +2043 to +2045
"resources": {
"$codeRef": "getKnativeResources.getKnativeEventingResources"
},
Copy link
Member

@logonoff logonoff Oct 24, 2025

Choose a reason for hiding this comment

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

resources expects a WatchK8sResourcesGeneric and not a codeRef.

The challenge of this ticket is that getKnativeEventingResources is dynamic, so we need to modify console.topology/data/factory to also allow codeRefs.

We discussed in the dynamic plugin sync that this coderef should be typed with

resources: WatchK8sResourcesGeneric | CodeRef<() => Promise<WatchK8sResourcesGeneric>>.

The promise is needed as plugins may want to do some fetching in order to determine which k8s resources should be wtached.

This will require changes to any code which consumes the TopologyDataModelFactory extension. I suggest possibly writing a useTopologyDataModelFactory hook in the topology package to allow resolve the codeRefs

This API change is already approved by @spadgett so you will get plugin-api-approved with this change

Comment on lines 2063 to 2065
"resources": {
"$codeRef": "getKnativeResources.getKnativeEventingKameletsResources"
},
Copy link
Member

Choose a reason for hiding this comment

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

resources expects a WatchK8sResourcesGeneric and not a codeRef.

Copy link
Member

Choose a reason for hiding this comment

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

also since getKnativeEventingKameletsResources always returns the same object we can embed the contents of that directly into the json file--no codeRef needed

Copy link
Member

@jhadvig jhadvig left a comment

Choose a reason for hiding this comment

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

Nice job @Leo6Leo 👍

/lgtm
/approve

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 24, 2025
@Leo6Leo
Copy link
Contributor Author

Leo6Leo commented Oct 24, 2025

/hold for addressing the PR review from @logonoff

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 24, 2025
Leo6Leo and others added 2 commits October 28, 2025 11:38
Enable dynamic plugin extensions to use CodeRef for the resources
property in topology data model factories. This allows plugins to
fetch resources dynamically at runtime (e.g., discovering CRDs).

Changes:
- Update TopologyDataModelFactory type to accept CodeRef<() => Promise<WatchK8sResourcesGeneric>>
- Add Promise resolution in DataModelExtension for CodeRef resources
- Update get-knative-resources functions to return Promises with resource definitions
- Maintain backward compatibility with static resource definitions

The implementation follows the same pattern as other CodeRef properties
(getDataModel, isResourceDepicted) by resolving the Promise eagerly
during extension initialization and caching the result.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@openshift-ci openshift-ci bot added component/sdk Related to console-plugin-sdk component/topology Related to topology plugin-api-changed Categorizes a PR as containing plugin API changes and removed lgtm Indicates that a PR is ready to be merged. labels Oct 28, 2025
Copy link
Member

@logonoff logonoff left a comment

Choose a reason for hiding this comment

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

nice!

* Resources type that can be:
* 1. Static WatchK8sResourcesGeneric (from dynamic plugin SDK)
* 2. CodeRef<() => Promise<WatchK8sResourcesGeneric>> (from dynamic plugin SDK, for dynamic resources)
* 3. Function (namespace: string) => WatchK8sResources (from old internal plugin system or converted dynamic plugins)
Copy link
Member

Choose a reason for hiding this comment

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

we should only be supporting 1 and 2, this really should be TopologyDataModelFactory.properties.resources.

you can build on my PR, #15617, which removes the final usage of TopologyDataModelFacotry. then we no longer care about its support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once #15617 merges, I will clean up the DataModelProvider to remove the old modelFactories usage.

Copy link
Member

Choose a reason for hiding this comment

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

FYI this PR has merged

Comment on lines 65 to 72
return {
...extension,
properties: {
...extension.properties,
// Keep the original CodeRef - it will be resolved by useResolvedResources hook
resources,
},
};
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this just return extension?

Leo6Leo and others added 4 commits October 28, 2025 15:01
…ic definitions

Replaces dynamic CodeRef-based resource definitions with inline static definitions for knative-serving and kamelets topology model factories, removing the need for runtime resolution. Adds compatibility layer for converting dynamic model factories to internal plugin format.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@Leo6Leo
Copy link
Contributor Author

Leo6Leo commented Nov 3, 2025

/retest

Copy link
Member

@logonoff logonoff left a comment

Choose a reason for hiding this comment

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

/lgtm

I guess our prettier/eslint setup is too old for satisfies... 🤷

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, Leo6Leo, logonoff

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 3, 2025

New changes are detected. LGTM label has been removed.

Refactor topology package to remove static extension support and rely
exclusively on dynamic plugin SDK for topology data model factories.

Key changes:
- Remove TopologyDataModelFactory extension interface and related types
- Simplify ApplicationDropdown to use only base watched resources
- Remove legacy function format support in useTopologyDataModelFactory
- Eliminate conversion layer between dynamic and static factories
- Update DataModelProvider to handle only dynamic extensions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@Leo6Leo Leo6Leo requested a review from logonoff November 3, 2025 20:41
Comment on lines 38 to 42
if (typeof resources === 'function') {
// CodeRef format: CodeRef<() => Promise<WatchK8sResourcesGeneric>>
// which is a function that returns Promise<() => Promise<WatchK8sResourcesGeneric>>
setLoading(true);
const fetchResources = async () => {
Copy link
Member

@logonoff logonoff Nov 3, 2025

Choose a reason for hiding this comment

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

if the initial code that fetches TopologyResourcesType uses instead useResolvedExtensions, the resolution of the CodeRef is done automatically for us.

We can then also type the param resources as resources: ResolvedExtension<TopologyDataModelFactory>['resources']

Comment on lines 17 to 39
/**
* Converts a single resource from WatchK8sResourcesGeneric format to WatchK8sResource format.
* This handles the namespace injection and model reference resolution.
*/
const convertGenericResource = (
namespace: string,
model?: { group?: string; version?: string; kind: string },
opts?: Partial<WatchK8sResource>,
): WatchK8sResource | null => {
if (!model) {
return { namespace, ...opts };
}

// Try to find the internal model
const internalModel = modelForGroupKind(model.group, model.kind);
if (!internalModel) {
// CRD not found - log warning and skip
// eslint-disable-next-line no-console
console.warn(
`Could not find model (CRD) for group "${model.group}" and kind "${model.kind}". Resource will be skipped.`,
);
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

is this needed to work? my understanding was that WatchK8sResourcesGeneric allows you to provide all the options that WatchK8sResource does

@logonoff
Copy link
Member

logonoff commented Nov 3, 2025

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 3, 2025
@jhadvig jhadvig added the plugin-api-approved Indicates a PR with plugin API changes has been approved by an API reviewer label Nov 4, 2025
Leo6Leo and others added 5 commits November 4, 2025 16:19
Replace custom useResolvedResources hook with built-in useResolvedExtensions
to simplify the topology data model extension handling. This removes the need
for manual CodeRef resolution and improves error messaging with plugin context.

Changes:
- Use useResolvedExtensions instead of useExtensions in DataModelProvider
- Remove useTopologyDataModelFactory hook (no longer needed)
- Simplify DataModelExtension to work with pre-resolved extensions
- Add plugin ID to error messages for better debugging
- Use referenceForExtensionModel for better CRD reference resolution

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 6, 2025

@Leo6Leo: 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 6ec040b 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/knative Related to knative-plugin component/sdk Related to console-plugin-sdk component/topology Related to topology do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. plugin-api-approved Indicates a PR with plugin API changes has been approved by an API reviewer plugin-api-changed Categorizes a PR as containing plugin API changes tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants