backstage / backstage

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

Use fetch middlewares in event-source-polifyll (re: #23015) #24820

Closed alexef closed 1 week ago

alexef commented 2 weeks ago

PoC to validate this would be the way forward in #23015 (scaffolder, techdocs use event-source-polyfill library that only allows headers to be passed but not the entire fetch implementation)

:heavy_check_mark: Checklist

backstage-goalie[bot] commented 2 weeks ago

Missing Changesets

The following package(s) are changed by this PR but do not have a changeset:

See CONTRIBUTING.md for more information about how to add changesets.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/core-app-api packages/core-app-api none v1.12.5
@backstage/core-plugin-api packages/core-plugin-api none v1.9.2
@backstage/test-utils packages/test-utils none v1.5.5
@backstage/plugin-scaffolder plugins/scaffolder none v1.20.0
@backstage/plugin-techdocs plugins/techdocs none v1.10.5
alexef commented 2 weeks ago

@aramissennyeydd / @freben pulling you from the original ticket, can you have a look and see if this would be an acceptable approach?

I can implement it as an eventSourceApi as you suggested, but I wouldn't want to duplicate the header.name configuration in FetchMiddlewares.injectIdentityAuth.

Not yet sure, but I assume if the new eventSourceApi depended on the fetchApi, then it could call the new fetchApi.headers? method so the configuration is done only once.

My current code looks like this:

createApiFactory({
    api: fetchApiRef,
    deps: {
      configApi: configApiRef,
      identityApi: identityApiRef,
      discoveryApi: discoveryApiRef,
    },
    factory: ({ configApi, identityApi, discoveryApi }) => {
      return createFetchApi({
        middleware: [
          FetchMiddlewares.resolvePluginProtocol({
            discoveryApi,
          }),
          FetchMiddlewares.injectIdentityAuth({
            identityApi,
            config: configApi,
            header: {
              name: 'Backstage-Authorization',
              value: (backstageToken) => `Bearer ${backstageToken}`,
            }
          }),
        ],
      });
    },
  }),
Rugvip commented 2 weeks ago

Thank you! 👍

Wanna note here as well that I think we should explore https://github.com/backstage/backstage/issues/23015#issuecomment-2117398605 before making additions to the core APIs

alexef commented 1 week ago

@Rugvip please check https://github.com/backstage/backstage/pull/24861 which should replace this PR if viable.

alexef commented 1 week ago

closing in favor of: https://github.com/backstage/backstage/pull/24861

github-actions[bot] commented 1 week ago

Uffizzi Cluster pr-24820 was deleted.