Skip to content

[CatalogClient] extra headers in Catalog request#21324

Closed
alde wants to merge 3 commits intomasterfrom
catalog-client/allow-custom-headers-in-request
Closed

[CatalogClient] extra headers in Catalog request#21324
alde wants to merge 3 commits intomasterfrom
catalog-client/allow-custom-headers-in-request

Conversation

@alde
Copy link
Copy Markdown
Collaborator

@alde alde commented Nov 15, 2023

Allow the setting of custom extra headers in CatalogRequestOptions.

Use-case: Passing in information from the front-end or another backend service for example for logging request information (like identity, correlation-id, etc.).

Hey, I just made a Pull Request!

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

Allow the setting of custom extra headers in CatalogRequestOptions.

Use-case: Passing in information from the front-end or another backend
service for example for logging request information (like identity,
correlation-id, etc.).

Signed-off-by: Rickard Dybeck <dybeck@spotify.com>
@alde alde requested review from a team as code owners November 15, 2023 14:43
@alde alde requested review from Rugvip and camilaibs November 15, 2023 14:43
@github-actions github-actions bot added the area:catalog Related to the Catalog Project Area label Nov 15, 2023
@backstage-goalie
Copy link
Copy Markdown
Contributor

backstage-goalie bot commented Nov 15, 2023

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/catalog-client packages/catalog-client minor v1.4.6

Copy link
Copy Markdown
Contributor

@camilaibs camilaibs left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Rickard Dybeck <dybeck@spotify.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 15, 2023

Uffizzi Ephemeral Environment - Virtual Cluster

Your cluster pr-21324 was successfully created. Learn more about Uffizzi virtual clusters
To connect to this cluster, follow these steps:

  1. Download and install the Uffizzi CLI from https://docs.uffizzi.com/install
  2. Login to Uffizzi, then select the backstage account and project:
uffizzi login
Select an account: 
  ‣ backstage
    jdoe

Select a project or create a new project: 
  ‣ backstage-6783521
  1. Update your kubeconfig: uffizzi cluster update-kubeconfig pr-21324 --kubeconfig=[PATH_TO_KUBECONFIG]
    After updating your kubeconfig, you can manage your cluster with kubectl, kustomize, helm, and other tools that use kubeconfig files: kubectl get namespace --kubeconfig [PATH_TO_KUBECONFIG]

Access the backstage endpoint at https://backstage-default-pr-21324-c1959.uclusters.app.uffizzi.com

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 15, 2023

Uffizzi Cluster pr-21324 was deleted.

Signed-off-by: Rickard Dybeck <dybeck@spotify.com>
Copy link
Copy Markdown
Member

@Rugvip Rugvip left a comment

Choose a reason for hiding this comment

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

I believe you can already do this by decorating the fetch API before passing it to the catalog client. You should be able to use createFetchApi to help out with the wrapping of the existing fetch as well:

@alde
Copy link
Copy Markdown
Collaborator Author

alde commented Nov 15, 2023

@Rugvip I spent some time to look into that, and I'm not at all sure how to do it modifying the fetchApi with a middleware without making the code really complicated instead of just passing it on the call that already has support for (very limited) options

@freben
Copy link
Copy Markdown
Member

freben commented Nov 17, 2023

Hm the examples are identity and correlation-ID - at least the second of those is a per-request thing right?

Identity in the form of specifically the Backstage token should be injected automatically by the fetchApi as long as it's been passed to the client in the first place - which it is by default.

@Rugvip
Copy link
Copy Markdown
Member

Rugvip commented Nov 21, 2023

Realized there's no need for using createFetchApi, it's possible, but just adds a bit more code. Anyway, here's an example of how to add a totally random correlation ID to all requests:

createApiFactory({
  api: catalogApiRef,
  deps: {
    discoveryApi: discoveryApiRef,
    fetchApi: fetchApiRef,
  },
  factory: ({ discoveryApi, fetchApi: { fetch: innerFetch } }) =>
    new CatalogClient({
      discoveryApi,
      fetchApi: {
        fetch: async (input, init) => {
          const request = new Request(input as any, init);
          request.headers.set('correlation-id', '4');
          return innerFetch(request);
        },
      },
    }),
}),

@alde
Copy link
Copy Markdown
Collaborator Author

alde commented Nov 21, 2023

@Rugvip The problem that I'm trying to solve is setting it to a different thing in each call to forward headers from an incoming call for example

const headerFromUpstream = request.headers['x-tracing-id'];
catalogApi.getLocationById(locationId, { headers: { 'x-tracing-id': headerFromUpstream } });

@Rugvip Rugvip mentioned this pull request Nov 21, 2023
19 tasks
@Rugvip
Copy link
Copy Markdown
Member

Rugvip commented Nov 21, 2023

I noted this down in #15999 so that we can at very least try to tackle this as part of broader service auth/protected by default work. Also noted that the particular problem of forwarding information from an incoming request to outgoing ones is something that could potentially be solved by context/thread local storage, which is something we might explore again as part of that work.

@alde alde closed this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:catalog Related to the Catalog Project Area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants