Limit version of auto-upgraded subgraphs (#2933)

Do not auto-upgrade federation subgraphs past fed 2.4. This is so that
we don't inadvertently require a router that supports the latest join
spec when it's not needed.

Fixes #2932

Author: clenfest

.changeset/lemon-yaks-think.md

@@ -0,0 +1,6 @@
+---
+"@apollo/query-planner": patch
+"@apollo/federation-internals": patch
+---
+
+When auto-upgrading a subgraph (i.e. one that does not explicitly @link the federation spec) do not go past v2.4. This is so that subgraphs will not inadvertently require the latest join spec (which cause the router or gateway not to start if running an older version).

internals-js/src/__tests__/schemaUpgrader.test.ts

@@ -1,4 +1,4 @@
-import { FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS, printSchema } from '..';
+import { FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS_UPGRADED, printSchema } from '..';
 import { ObjectType } from '../definitions';
 import { buildSubgraph, Subgraphs } from '../federation';
 import { UpgradeChangeID, UpgradeResult, upgradeSubgraphsIfNecessary } from '../schemaUpgrader';
@@ -92,7 +92,7 @@ test('upgrade complex schema', () => {
 
   expect(res.subgraphs?.get('s1')?.toString()).toMatchString(`
     schema
-      ${FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS}
+      ${FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS_UPGRADED}
     {
       query: Query
     }
@@ -148,7 +148,7 @@ test('update federation directive non-string arguments', () => {
 
   expect(res.subgraphs?.get('s')?.toString()).toMatchString(`
     schema
-      ${FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS}
+      ${FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS_UPGRADED}
     {
       query: Query
     }
@@ -320,7 +320,7 @@ test("fully upgrades a schema with no @link directive", () => {
   expect(printSchema(result.subgraphs!.get("subgraph")!.schema!)).toContain(
 `schema
   @link(url: "https://specs.apollo.dev/link/v1.0")
-  @link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])
+  @link(url: "https://specs.apollo.dev/federation/v2.4", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])
 {
   query: Query
 }`

internals-js/src/federation.ts

@@ -79,6 +79,7 @@ import {
   FederationTypeName,
   FEDERATION1_TYPES,
   FEDERATION1_DIRECTIVES,
+  FederationSpecDefinition,
 } from "./specs/federationSpec";
 import { defaultPrintOptions, PrintOptions as PrintOptions, printSchema } from "./print";
 import { createObjectTypeSpecification, createScalarTypeSpecification, createUnionTypeSpecification } from "./directiveAndTypeSpecification";
@@ -93,10 +94,18 @@ import {
 
 const linkSpec = LINK_VERSIONS.latest();
 const tagSpec = TAG_VERSIONS.latest();
-const federationSpec = FEDERATION_VERSIONS.latest();
+const federationSpec = (version?: FeatureVersion): FederationSpecDefinition => {
+  if (!version) return FEDERATION_VERSIONS.latest();
+  const spec = FEDERATION_VERSIONS.find(version);
+  assert(spec, `Federation spec version ${version} is not known`);
+  return spec;
+};
+
 // Some users rely on auto-expanding fed v1 graphs with fed v2 directives. While technically we should only expand @tag
 // directive from v2 definitions, we will continue expanding other directives (up to v2.4) to ensure backwards compatibility.
-const autoExpandedFederationSpec = FEDERATION_VERSIONS.find(new FeatureVersion(2, 4))!;
+const autoExpandedFederationSpec = federationSpec(new FeatureVersion(2, 4));
+
+const latestFederationSpec = federationSpec();
 
 // We don't let user use this as a subgraph name. That allows us to use it in `query graphs` to name the source of roots
 // in the "federated query graph" without worrying about conflict (see `FEDERATED_GRAPH_ROOT_SOURCE` in `querygraph.ts`).
@@ -601,7 +610,7 @@ export class FederationMetadata {
   }
 
   federationFeature(): CoreFeature | undefined {
-    return this.schema.coreFeatures?.getByIdentity(federationSpec.identity);
+    return this.schema.coreFeatures?.getByIdentity(latestFederationSpec.identity);
   }
 
   private externalTester(): ExternalTester {
@@ -663,7 +672,7 @@ export class FederationMetadata {
     if (this.isFed2Schema()) {
       const coreFeatures = this.schema.coreFeatures;
       assert(coreFeatures, 'Schema should be a core schema');
-      const federationFeature = coreFeatures.getByIdentity(federationSpec.identity);
+      const federationFeature = coreFeatures.getByIdentity(latestFederationSpec.identity);
       assert(federationFeature, 'Schema should have the federation feature');
       return federationFeature.directiveNameInSchema(name);
     } else {
@@ -685,7 +694,7 @@ export class FederationMetadata {
     if (this.isFed2Schema()) {
       const coreFeatures = this.schema.coreFeatures;
       assert(coreFeatures, 'Schema should be a core schema');
-      const federationFeature = coreFeatures.getByIdentity(federationSpec.identity);
+      const federationFeature = coreFeatures.getByIdentity(latestFederationSpec.identity);
       assert(federationFeature, 'Schema should have the federation feature');
       return federationFeature.typeNameInSchema(name);
     } else {
@@ -1190,7 +1199,7 @@ function findUnusedNamedForLinkDirective(schema: Schema): string | undefined {
   }
 }
 
-export function setSchemaAsFed2Subgraph(schema: Schema) {
+export function setSchemaAsFed2Subgraph(schema: Schema, useLatest: boolean = false) {
   let core = schema.coreFeatures;
   let spec: CoreSpecDefinition;
   if (core) {
@@ -1209,11 +1218,16 @@ export function setSchemaAsFed2Subgraph(schema: Schema) {
     assert(core, 'Schema should now be a core schema');
   }
 
-  assert(!core.getByIdentity(federationSpec.identity), 'Schema already set as a federation subgraph');
+  const fedSpec = useLatest ? latestFederationSpec : autoExpandedFederationSpec;
+
+  assert(!core.getByIdentity(fedSpec.identity), 'Schema already set as a federation subgraph');
   schema.schemaDefinition.applyDirective(
     core.coreItself.nameInSchema,
     {
-      url: federationSpec.url.toString(),
+      // note that there is a mismatch between url and directives that are imported. This is because
+      // we want to maintain backward compatibility for those who have already upgraded and we had been upgrading the url to
+      // latest, but we never automatically import directives that exist past 2.4
+      url: fedSpec.url.toString(),
       import: autoExpandedFederationSpec.directiveSpecs().map((spec) => `@${spec.name}`),
     }
   );
@@ -1226,29 +1240,33 @@ export function setSchemaAsFed2Subgraph(schema: Schema) {
 // This is the full @link declaration as added by `asFed2SubgraphDocument`. It's here primarily for uses by tests that print and match
 // subgraph schema to avoid having to update 20+ tests every time we use a new directive or the order of import changes ...
 export const FEDERATION2_LINK_WITH_FULL_IMPORTS = '@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject", "@authenticated", "@requiresScopes", "@policy", "@sourceAPI", "@sourceType", "@sourceField"])';
-// This is the full @link declaration that is added when upgrading fed v1 subgraphs to v2 version. It should only be used by tests.
+
+// This is the federation @link for tests that go through the asFed2SubgraphDocument function.
 export const FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS = '@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])';
 
+// This is the federation @link for tests that go through the SchemaUpgrader.
+export const FEDERATION2_LINK_WITH_AUTO_EXPANDED_IMPORTS_UPGRADED = '@link(url: "https://specs.apollo.dev/federation/v2.4", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])';
+
 /**
  * Given a document that is assumed to _not_ be a fed2 schema (it does not have a `@link` to the federation spec),
  * returns an equivalent document that `@link` to the last known federation spec.
  *
  * @param document - the document to "augment".
- * @param options.addAsSchemaExtension - defines whethere the added `@link` is added as a schema extension (`extend schema`) or
+ * @param options.addAsSchemaExtension - defines whether the added `@link` is added as a schema extension (`extend schema`) or
  *   added to the schema definition. Defaults to `true` (added as an extension), as this mimics what we tends to write manually.
  * @param options.includeAllImports - defines whether we should auto import ALL latest federation v2 directive definitions or include
  *   only limited set of directives (i.e. federation v2.4 definitions)
  */
 export function asFed2SubgraphDocument(document: DocumentNode, options?: { addAsSchemaExtension?: boolean, includeAllImports?: boolean }): DocumentNode {
-  const importedDirectives = options?.includeAllImports ? federationSpec.directiveSpecs() : autoExpandedFederationSpec.directiveSpecs();
+  const importedDirectives = options?.includeAllImports ? latestFederationSpec.directiveSpecs() : autoExpandedFederationSpec.directiveSpecs();
   const directiveToAdd: ConstDirectiveNode = ({
     kind: Kind.DIRECTIVE,
     name: { kind: Kind.NAME, value: linkDirectiveDefaultName },
     arguments: [
       {
         kind: Kind.ARGUMENT,
         name: { kind: Kind.NAME, value: 'url' },
-        value: { kind: Kind.STRING, value: federationSpec.url.toString() }
+        value: { kind: Kind.STRING, value: latestFederationSpec.url.toString() }
       },
       {
         kind: Kind.ARGUMENT,
@@ -1374,7 +1392,7 @@ export function buildSubgraph(
 
 export function newEmptyFederation2Schema(config?: SchemaConfig): Schema {
   const schema = new Schema(new FederationBlueprint(true), config);
-  setSchemaAsFed2Subgraph(schema);
+  setSchemaAsFed2Subgraph(schema, true);
   return schema;
 }
 

query-planner-js/src/__tests__/testHelper.ts

@@ -6,13 +6,17 @@ expect.addSnapshotSerializer(astSerializer);
 expect.addSnapshotSerializer(queryPlanSerializer);
 
 export function composeAndCreatePlanner(...services: ServiceDefinition[]): [Schema, QueryPlanner] {
-  return composeAndCreatePlannerWithOptions(services, {});
+  return composeAndCreatePlannerWithOptions(services, {}, false);
 }
 
-export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[], config: QueryPlannerConfig): [Schema, QueryPlanner] {
-  const compositionResults = composeServices(
-    services.map((s) => ({ ...s, typeDefs: asFed2SubgraphDocument(s.typeDefs) }))
-  );
+export function composeFed2SubgraphsAndCreatePlanner(...services: ServiceDefinition[]): [Schema, QueryPlanner] {
+  return composeAndCreatePlannerWithOptions(services, {}, true);
+}
+
+export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[], config: QueryPlannerConfig, isFed2Subgraph: boolean = false): [Schema, QueryPlanner] {
+  const updatedServices = isFed2Subgraph ? services : services.map((s) => ({ ...s, typeDefs: asFed2SubgraphDocument(s.typeDefs) }));
+
+  const compositionResults = composeServices(updatedServices);
   expect(compositionResults.errors).toBeUndefined();
   return [
     compositionResults.schema!.toAPISchema(),