Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic plugin #64

Merged
merged 11 commits into from
Jun 6, 2024

Conversation

aliok
Copy link
Member

@aliok aliok commented May 31, 2024

Changes

  • 🎁 Made the plugin dynamic

  • Backstage has a WIP proposal to allow plugins to be loaded dynamically. While this is not done in upstream Backstage yet, Janus IDP has implemented this feature.

  • With this PR, we make the plugin dynamic so that it can be used by Janus IDP without changing the source code of Backstage.

For testing, you can clone my fork of Janus Showcase, using the feature-dynamic-plugin-works-now branch: https://github.com/aliok/backstage-showcase/tree/feature-dynamic-plugin-works-now

I've overridden the Git ignore rules to check in the generated files, so you should be able to simply do yarn install and yarn start to start the showcase and see the plugin working. However, please note that it requires a couple of things:

  1. Knative Eventing and event mesh backend installed on your Kubernetes cluster
  2. SA and token setup
  3. Event mesh accessible from Backstage backend (you may use port forwarding)
  4. Some resources (broker, trigger, eventType) available on the cluster

Essentially the same steps from https://github.com/aliok/knative-backstage-demo?tab=readme-ov-file#starting-up until the "Start Backstage" step.

Screenshot:

Screenshot 2024-05-31 at 14 43 51

Next steps:

  • GitHub action to release the dynamic plugin on upstream knative-extensions org

Signed-off-by: Ali Ok <aliok@redhat.com>
Copy link

knative-prow bot commented May 31, 2024

@aliok: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

In response to this:

Changes

/kind

Fixes #

Release Note


Docs


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.

@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 31, 2024
Copy link

knative-prow bot commented May 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aliok

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

@knative-prow knative-prow bot requested review from Cali0707 and pierDipi May 31, 2024 07:08
@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 31, 2024
aliok added 8 commits May 31, 2024 10:46
Signed-off-by: Ali Ok <aliok@redhat.com>
Signed-off-by: Ali Ok <aliok@redhat.com>
Signed-off-by: Ali Ok <aliok@redhat.com>
…xport-dynamic` command overrides it

Signed-off-by: Ali Ok <aliok@redhat.com>
Signed-off-by: Ali Ok <aliok@redhat.com>
Signed-off-by: Ali Ok <aliok@redhat.com>
Signed-off-by: Ali Ok <aliok@redhat.com>
@aliok aliok changed the title [WIP] Dynamic plugin preliminary work Dynamic plugin preliminary work Jun 5, 2024
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 5, 2024
@aliok aliok changed the title Dynamic plugin preliminary work Dynamic plugin Jun 5, 2024
@aliok
Copy link
Member Author

aliok commented Jun 5, 2024

Going to do the release scripts in another problem, because I see there are some issues.

Signed-off-by: Ali Ok <aliok@redhat.com>
@Cali0707
Copy link
Member

Cali0707 commented Jun 5, 2024

@Zaperex could you take a look at this PR?

@pierDipi
Copy link
Member

pierDipi commented Jun 6, 2024

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jun 6, 2024
@knative-prow knative-prow bot merged commit 5273de6 into knative-extensions:main Jun 6, 2024
16 checks passed
Copy link
Contributor

@Zaperex Zaperex left a comment

Choose a reason for hiding this comment

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

Hi @aliok, sorry I didn't submit this review in time. The PR looks good overall, just some nits about the way the new module is setup, and the lack of documentation for the new backend installation.

I made a separate PR to address those issues here: #67

register(env) {
env.registerInit({
deps: {
catalog: catalogProcessingExtensionPoint,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
catalog: catalogProcessingExtensionPoint,
catalog: catalogProcessingExtensionPoint,
catalogApi: catalogServiceRef,

Comment on lines +30 to +34
const catalogApi = new CatalogClient({
discoveryApi: discovery,
});

const knativeEventMeshProcessor = new KnativeEventMeshProcessor(catalogApi, loggerToWinstonLogger(logger));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const catalogApi = new CatalogClient({
discoveryApi: discovery,
});
const knativeEventMeshProcessor = new KnativeEventMeshProcessor(catalogApi, loggerToWinstonLogger(logger));
const knativeEventMeshProcessor = new KnativeEventMeshProcessor(catalogApi, loggerToWinstonLogger(logger));

Copy link
Contributor

@Zaperex Zaperex Jun 6, 2024

Choose a reason for hiding this comment

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

Also heads up, but for your processor, you should change the CatalogClient to CatalogApi since CatalogClient is a class not a type/interface.

Comment on lines +6 to +7
import {CatalogClient} from "@backstage/catalog-client";
import {catalogProcessingExtensionPoint} from '@backstage/plugin-catalog-node/alpha';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import {CatalogClient} from "@backstage/catalog-client";
import {catalogProcessingExtensionPoint} from '@backstage/plugin-catalog-node/alpha';
import { catalogServiceRef, catalogProcessingExtensionPoint } from '@backstage/plugin-catalog-node/alpha';

scheduler: coreServices.scheduler,
discovery: coreServices.discovery,
},
async init({catalog, config, logger, scheduler, discovery}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
async init({catalog, config, logger, scheduler, discovery}) {
async init({ catalogApi, catalog, config, logger, scheduler }) {

@aliok aliok deleted the 2024-05-31-dynamic-plugin branch June 6, 2024 14:49
@aliok aliok mentioned this pull request Jun 14, 2024
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. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants