backstage / backstage

Backstage is an open framework for building developer portals
https://backstage.io/
Apache License 2.0
28.09k stars 5.94k forks source link

💬 RFC: Feature Reflection for the New Backend System #17538

Open sennyeya opened 1 year ago

sennyeya commented 1 year ago

🔖 Need

Primary use case: As a plugin developer for Backstage-focused plugins , I want to be able to get information about the currently installed plugins.

For the purposes of the new backend system I'm going to call this "Feature Reflection", as "Plugin Metadata" already refers to the service for getting information about the current plugin. Also, I'm expanding this to "Features" instead of just "Plugins" to support possible use cases around modules and other Feature types that may come up, it also matches the existing naming in the new backend system.

There was work to add this kind of thing to the old backend system (#10256) and I would like to propose adding a similar mechanism to the new backend system.

🎉 Proposal

There are two asks (and I can break these out into separate issues if needed),

  1. Can we add a new service that has a list of all installed features?
  2. Can we annotate features with additional metadata like owner, api spec, version, etc?

1. Getting the list of all installed plugins

Create a new service, FeatureRegistryService,

export interface FeatureRegistryService {
  getFeatures(): InternalBackendPluginRegistration | InternalBackendModuleRegistration[];
}

Add a new coreService.pluginRegistry,

export const featureRegistry = createServiceRef<
    import('./FeatureRegistryService').FeatureRegistryService
  >({ id: 'core.featureRegistry', scope: 'root' });

Support having a list of plugins on the ServiceRegistry interface

export interface EnumerableServiceHolder extends ServiceHolder {
  ...
  setFeatures(feature: InternalBackendPluginRegistration | InternalBackendModuleRegistration[]): void;
}

Populate that list of plugins during initialization of the backend system, (would need to be enforced at run time to ensure contract is consistent). For the current implementation, it would look something like this:

export class BackendInitializer {
  ...
  async #doStart(){
    this.#serviceHolder.setFeatures(
        this.#features
          .map(feature =>
            feature.getRegistrations(),
          )
          .flat(),
      );
    ...
  }
  ...
}

Users could then access this like

export const myPlugin = createBackendPlugin({
  pluginId:'my-id',
  register(env) {
    env.registerInit({
      deps: {
        pluginMetadata: coreServices.pluginMetadata,
        featureRegistry: coreServices.featureRegistry,
      },
      async init({ pluginMetadata, featureRegistry }) {
        const installedPlugins = featureRegistry
          .getFeatures()
          .filter(e=>e.type === "plugin" && e.pluginId !== pluginMetadata.getId() );
      },
    });
  },
});

2. Allowing additional metadata on the feature definition

As shown in #10256, allowing users to add additional metadata is key to information about the feature, ownership, and other feature metadata, like emails, slack channels or version. This field should be highly customizable for that reason as different adopters may have different use cases. For the purposes of this RFC, I'm ignoring the ability to add callbacks to createApp for adding additional metadata. That may be content for future RFCs.

To implement, we would update the InternalBackendPluginRegistration interface as

export interface InternalBackendPluginRegistration {
  pluginId: string;
  type: 'plugin';
  extensionPoints: Array<readonly [ExtensionPoint<unknown>, unknown]>;
  info: object;
  init: {
    deps: Record<string, ServiceRef<unknown>>;
    func(deps: Record<string, unknown>): Promise<void>;
  };
}

and then access through coreServices.featureRegistry

export const myPlugin = createBackendPlugin({
  pluginId:'my-id',
  info: {
    packageJson: {name: "test123", version: "1.0"},
    owner: "my-username"
  },
  register(env) {
    env.registerInit({
      deps: {
        pluginMetadata: coreServices.pluginMetadata,
        featureRegistry: coreServices.featureRegistry,
      },
      async init({ pluginMetadata, featureRegistry }) {
        const myInfo = featureRegistry.getFeatures().find(e=>e.pluginId === pluginMetadata.getId()).info;
        const pluginOwner = myInfo.owner;
      },
    });
  },
});

〽️ Alternatives

FeatureRegistryService

Reuse PluginMetadataService

We could add the above mentioned values and methods to PluginMetadataService. The primary con of this is it changes PluginMetadataService from a way of getting the context of the current plugin to a feature registry or library service. It would be preferable to keep these two separate.

Create a new FeatureRegistry instead of using ServiceRegistry (worth exploring)

This feels like a good direction, but adds more complexity to the existing system. It would need to be passed in to ServiceRegistry nonetheless. Features are stored on both the BackstageBackend and BackstageInitializer and this might be a use case to separate that logic into a separate class. FeatureRegistryService would still be instantiated from ServiceRegistry, but it would have a stronger separation than the proposed implementation.

Return an instance of PluginMetadataService instead of raw plugins

Basically, for this one each Plugin above would instead be the correct PluginMetadataService for a given plugin. This would ensure a consistent API for accessing the current plugin and other plugins. It also hides updates to the internal plugin instance well. The issue is that it blurs the lines between plugin sandbox and the backend system.

Metadata

Using Kubernetes format for the info field

This is something that has come up for similar ways of adding information to the catalog, see #2292. It's a great solution for entities, items that are already in Kubernetes format and are expected to be shown in the UI. In this case, we are just supplying a formless set of data for internal backend use, similar to the metadata or spec fields of the Kubernetes format, so the extra rigidity of the format is excessive.

Add getInfo method to PluginMetadataService

This isn't a required use case on our side, a plugin should already know its own info, the primary use case would be consumption outside of a given plugin. If we use PluginMetadataService, we should revisit this.

❌ Risks

  1. Exposing information about features from the existing system breaks the separation of concerns between the BackstageBackend / BackstageInitializer and ServiceRegistry.
  2. Plugins that use this will need to not rely on the list of features as having been already initialized. If passed featureRegistry, you cannot immediately do
    await fetch(`${await discovery.getBaseUrl(pluginMetadata.getId()}/my-endpoint`))

    and expect it to work. Everything must happen at runtime on the router or in the task system.

  3. Info may quickly require type enforcement or recommendations as users will look to the project for examples of proper usage, similar to annotations on the main Entity object.

👀 Have you spent some time to check if this RFC has been raised before?

🏢 Have you read the Code of Conduct?

freben commented 1 year ago

Thinking some more about this one - one of the end goals here is to allow you to expose a collected view over the OpenAPI interfaces that plugins have, right?

For that use case, doesn't it make more sense for plugins to instead intentionally, wilfully, register their apis into an apiRegistryService or whatnot instead? Or maybe even better - if we make an OpenapiAggregatorPlugin that's dedicated for this purpose, then that one could have a specific Extension Point instead that other plugins might register stuff into. That ensures that this registration happens before the aggregator starts up, and needs no core framework for it.

This is just meant as an open question, not hard direction :) It would allow the plugins to be much more deliberate and caring in what gets exposed anyway; they might add additional parameters such as saying that this is an admin-only interface while this is the pubic surface, or adding aggregators for multiple protocols such as openapi and graphql, or only exposing something based on whether a feature flag was set or not, etc etc

sennyeya commented 1 year ago

@freben I like that suggestion for the OpenAPI aggregator service. Might even work as a module on the overall apiRegistrationService service. That might even allow run time api definition updates, which would be cool. Still, I think the OpenAPI work is in a special place as a Backstage-provided plugin.

What if I'm a developer in my organization and I want to add RAML or Smithy to the existing plugins? If I have to create a service and have plugins register with it, I would either have to contribute the upstream repository and make my whole plugin open source or fork the given plugin definitions and maintain that fork. That may be an acceptable solution, but I don't love it. Edit: The plugin could just allow plugins that aren't the main plugin to override or through a module/extension point add the required values. There should be enough flexibility in the existing system to solve this.

I think allowing tools like @ahhhndre's DevTools or the one referenced in #10256 to reference installed plugins, features and at some point services is important too. Yes, they can be extrapolated from the installed package in package.json, but getting the run time list of registered plugins/features/services seems really helpful for admin work.There's also extra information that we may want to share like service dependencies that may not make sense to store as a separate service, especially since it already exists on the plugin definition.

In that same vein, I don't think it makes sense to create a separate service for ownerRegistrationService and versionRegistrationService or integrating them to PluginMetadataService as a setOwner and setVersion call. Each plugin would have the same 2-3 lines of code and no guarantee that it's set up correctly. It would be better to have those listed on the Plugin definition in createBackendPlugin since they're unlikely to change. We can also enforce properties like owner and version to ensure hard ownership and versioning at compile time, that gets murkier at run time.

awanlin commented 1 year ago

Yeah, was just coming here to mention that this would be great to visualize in the DevTools plugin eventually. Also, that I really appreciate the detail in this RFC.

aaronnickovich commented 1 year ago

Yeah, was just coming here to mention that this would be great to visualize in the DevTools plugin eventually. Also, that I really appreciate the detail in this RFC.

This sounds like the quarkus dev ui: https://quarkus.io/guides/dev-ui The dev ui is only visible in dev mode by default.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

davidfestal commented 9 months ago

In the context of the permission framework, there is also the need to know the list of available plugins, as well as their pluginIds: see this discussion : https://discord.com/channels/687207715902193673/935836080563966042/1197230529595064422

This is one additional use-case for the task 1 of this RFC.

davidfestal commented 9 months ago

@Rugvip I started looking into implementing task 1. Should I open a dedicated issue for this ?

sennyeya commented 9 months ago

@davidfestal If possible, I'd like to be keyed in for any discussion that happen for #1. It's been in the back of my mind since I opened this issue and as mentioned above the big problem is distributed backends.

davidfestal commented 9 months ago

@sennyeya Sure. I'll keep you informed. I have a WIP PR (currently testing it) which afaict would allow provide an overridden featureRegistryService implementation that would support distributed backends.

davidfestal commented 9 months ago

which afaict would allow provide an overridden featureRegistryService implementation that would support distributed backends.

unless I'm missing a completely obvious thing :-)

davidfestal commented 9 months ago

@sennyeya PR is there: https://github.com/backstage/backstage/pull/22332.

Feel free to comment.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

davidfestal commented 1 month ago

@Rugvip Do you think we could we please reopen this ? It seems to me there's a real need there.