feat: Allow custom headers when introspecting in supergraph compose

[dbanty]

Supports the same syntax as Apollo Router for variable expansion (e.g., ${env.MY_SECRET}) when defining introspection_headers.

Closes #615

[EverlastingBugstopper]

this is looking great, thanks for the work on this - i just left one note

[EverlastingBugstopper]

i really like that this is backwards compatible with older versions of federation - when folks update their version of rover they don't also need to update their composition binary, because rover is what is handling the schema resolution. by the time the config file makes its way to the plugin, the sdl is resolved inline and introspection queries don't need to be performed/different config doesn't need to be serialized/deserialized 👍🏼

[dbanty]

i really like that this is backwards compatible with older versions of federation

That was part of the plan 😅. I'm pretty sure serialization won't get messed up in older versions because by default serde ignores extra keys... right? So if someone tried to use new config with old fed/rover it wouldn't error, it just wouldn't have the new feature?

[EverlastingBugstopper]

The cool thing about this is that it doesn't matter.

Input to rover can be like this:

subgraphs:
  products:
    routing_url: "https://products.example.com"
    schema:
      subgraph_url: "http://localhost:4003"
      introspection_headers:
        "new": "thing"

but the actual YAML that gets fed to the plugin looks like this:

subgraphs:
  products:
    routing_url: "https://products.example.com"
    schema:
      sdl:
        extend schema
          @link(url: "https://specs.apollo.dev/federation/v2.0",
             import: ["@key", "@shareable", "@tag", "@inaccessible"])

      type Query {
        allProducts: [ProductItf]
        product(id: ID!): ProductItf
      }
      ...... truncated

so adding this feature (and potentially other ways of resolving schemas in the future) doesn't matter to pre-compiled pre-distributed composition binaries, because all they care about is name, routing_url, and sdl for each subgraph. how rover gets that is completely decoupled.

[dbanty]

If we're not just matching on either env var or static value, do we handle resolving multiple variables in a single string? What about nesting env vars like Weird: ${env.${env.I_WANT_INDIRECTION}}. Matching combinations and resolving as expected might get a little messy.

My idea for an algorithm right now is:

1. Look for the last instance of ${env., then find the first }. If one is missing, break.
2. Interpret the content between the markers from 1 as the name of an env var
3. Update the whole string
4. Loop back to 1

[EverlastingBugstopper]

I think we should explicitly disallow nested configuration like that because it's a pain to handle, prone to bugs, and probably just a bad idea for someone to do.

I tried doing this for the router by doing the following from ./examples/supergraph-demo:

first, create ./router.yaml with these contents:

supergraph:
  path: "/"

telemetry:
  experimental_logging:
    when_header:
      - name: apollo-router-${env.${env.MY_ENV_VAR}}
        value: my_client
        headers: true
        body: true

get a composed schema

$ rover supergraph compose --config ./supergraph.yaml --output ./supergraph.graphql

download the router

$ curl -sSL https://router.apollo.dev/nix/latest | sh

run the router with our config file and supergraph file

$ MY_ENV_VAR="CLIENT_NAME" CLIENT_NAME="client-name" ./router -s ./supergraph.graphql -c ./router.yaml --dev
... truncated logs ...
2023-04-17T20:45:44.407486Z  ERROR could not expand variable: ${env.MY_ENV_VAR, environment variable not found
2023-04-17T20:45:44.408319Z  INFO stopped
2023-04-17T20:45:44.410397Z  ERROR no valid configuration was supplied
no valid configuration was supplied

I think if we detect nesting like this we just error and say it's invalid config.

[dbanty]

@EverlastingBugstopper I pushed a change that implements the algorithm I mentioned above—I think detecting nesting would be as hard as resolving nesting (if we're still allowing multiple values). Maybe I am wrong though, if you can think of a simple want to handle it.

[dbanty]

@EverlastingBugstopper I switched this to use the same variable expansion rules as router for more consistency. Now the docs about it can just point to those 😁 . I also opened #1578 and #1579 as potential follow-ups.

Left to do:

• Release apollo-federation-types and update here
• Add tests for file-based expansion

[EverlastingBugstopper]

few small notes and then i think we're off to the races!

[EverlastingBugstopper]

oh and would you also mind updating the top level PR comment to reflect the current state of the PR? we use "squash and merge, use the PR body for the commit body" settings on this repo

[dbanty]

@EverlastingBugstopper I think this is good to go now, but let me know if I missed something!

[EverlastingBugstopper]

LGTM! just pushed one commit that moves the shellexpand dep to be a workspace dep. i think we're good here 👍🏼