Skip to content

Commit b5c17ff

Browse files
benjamnbenjamn
authored
Mark connect/v0.4 as preview to prevent supergraph @link leak (#3413)
1 parent ba3175e commit b5c17ff

5 files changed

Lines changed: 115 additions & 8 deletions

File tree

‎.changeset/fix-connect-v04-preview.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@apollo/composition": patch
3+
"@apollo/federation-internals": patch
4+
---
5+
6+
Mark connect/v0.4 as a preview version so composition does not inject it into the supergraph `@link` unless a subgraph explicitly uses it. Previously, upgrading to Federation v2.13 would unconditionally stamp `connect/v0.4` into the supergraph, causing Router to require the `connectors.preview_connect_v0_4` flag even when no subgraph used v0.4 features. (RH-1321)

‎composition-js/src/__tests__/connectors.test.ts‎

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe("connect spec and join__directive", () => {
2929
const expectedSupergraphSdl = `schema
3030
@link(url: "https://specs.apollo.dev/link/v1.0")
3131
@link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION)
32-
@link(url: "https://specs.apollo.dev/connect/v0.4", for: EXECUTION)
32+
@link(url: "https://specs.apollo.dev/connect/v0.3", for: EXECUTION)
3333
@join__directive(graphs: [WITH_CONNECTORS], name: "link", args: {url: "https://specs.apollo.dev/connect/v0.1", import: ["@connect", "@source"]})
3434
@join__directive(graphs: [WITH_CONNECTORS], name: "source", args: {name: "v1", http: {baseURL: "http://v1"}})
3535
{
@@ -240,7 +240,7 @@ type Resource
240240
"schema
241241
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
242242
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
243-
@link(url: \\"https://specs.apollo.dev/connect/v0.4\\", for: EXECUTION)
243+
@link(url: \\"https://specs.apollo.dev/connect/v0.3\\", for: EXECUTION)
244244
@join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@source\\"]})
245245
@join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
246246
{
@@ -459,7 +459,7 @@ type Resource
459459
"schema
460460
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
461461
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
462-
@link(url: \\"https://specs.apollo.dev/connect/v0.4\\", for: EXECUTION)
462+
@link(url: \\"https://specs.apollo.dev/connect/v0.3\\", for: EXECUTION)
463463
@join__directive(graphs: [WITH_CONNECTORS_V0_1_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
464464
@join__directive(graphs: [WITH_CONNECTORS_V0_2_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.2\\", import: [\\"@connect\\", \\"@source\\"]})
465465
@join__directive(graphs: [WITH_CONNECTORS_V0_1_], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
@@ -643,7 +643,7 @@ type Resource
643643
"schema
644644
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
645645
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
646-
@link(url: \\"https://specs.apollo.dev/connect/v0.4\\", for: EXECUTION)
646+
@link(url: \\"https://specs.apollo.dev/connect/v0.3\\", for: EXECUTION)
647647
@join__directive(graphs: [WITH_CONNECTORS_V0_1_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
648648
@join__directive(graphs: [WITH_CONNECTORS_V0_3_], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.3\\", import: [\\"@connect\\", \\"@source\\"]})
649649
@join__directive(graphs: [WITH_CONNECTORS_V0_1_], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
@@ -744,6 +744,81 @@ type Resource
744744
}
745745
});
746746

747+
it("uses v0.4 in supergraph @link when a subgraph explicitly links to v0.4", () => {
748+
const subgraphs = [
749+
{
750+
name: "with-connectors-v0_4",
751+
typeDefs: parse(`
752+
extend schema
753+
@link(
754+
url: "https://specs.apollo.dev/federation/v2.13"
755+
import: ["@key"]
756+
)
757+
@link(
758+
url: "https://specs.apollo.dev/connect/v0.4"
759+
import: ["@connect", "@source"]
760+
)
761+
@source(name: "v1", http: { baseURL: "http://v1" })
762+
763+
type Query {
764+
resources: [Resource!]!
765+
@connect(source: "v1", http: { GET: "/resources" }, selection: "")
766+
}
767+
768+
type Resource @key(fields: "id") {
769+
id: ID!
770+
name: String!
771+
}
772+
`),
773+
},
774+
];
775+
776+
const result = composeServices(subgraphs);
777+
expect(result.errors ?? []).toEqual([]);
778+
const printed = printSchema(result.schema!);
779+
expect(printed).toContain(
780+
'@link(url: "https://specs.apollo.dev/connect/v0.4", for: EXECUTION)'
781+
);
782+
});
783+
784+
it("uses v0.3 (not v0.4) in supergraph @link when subgraphs only use v0.3 and earlier", () => {
785+
const subgraphs = [
786+
{
787+
name: "with-connectors-v0_3",
788+
typeDefs: parse(`
789+
extend schema
790+
@link(
791+
url: "https://specs.apollo.dev/federation/v2.12"
792+
import: ["@key"]
793+
)
794+
@link(
795+
url: "https://specs.apollo.dev/connect/v0.3"
796+
import: ["@connect", "@source"]
797+
)
798+
@source(name: "v1", http: { baseURL: "http://v1" })
799+
800+
type Query {
801+
resources: [Resource!]!
802+
@connect(source: "v1", http: { GET: "/resources" }, selection: "")
803+
}
804+
805+
type Resource @key(fields: "id") {
806+
id: ID!
807+
name: String!
808+
}
809+
`),
810+
},
811+
];
812+
813+
const result = composeServices(subgraphs);
814+
expect(result.errors ?? []).toEqual([]);
815+
const printed = printSchema(result.schema!);
816+
expect(printed).toContain(
817+
'@link(url: "https://specs.apollo.dev/connect/v0.3", for: EXECUTION)'
818+
);
819+
expect(printed).not.toContain('connect/v0.4');
820+
});
821+
747822
it("composes with renames", () => {
748823
const subgraphs = [
749824
{
@@ -784,7 +859,7 @@ type Resource
784859
"schema
785860
@link(url: \\"https://specs.apollo.dev/link/v1.0\\")
786861
@link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
787-
@link(url: \\"https://specs.apollo.dev/connect/v0.4\\", for: EXECUTION)
862+
@link(url: \\"https://specs.apollo.dev/connect/v0.3\\", for: EXECUTION)
788863
@join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", as: \\"http\\", import: [{name: \\"@connect\\", as: \\"@http\\"}, {name: \\"@source\\", as: \\"@api\\"}]})
789864
@join__directive(graphs: [WITH_CONNECTORS], name: \\"api\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
790865
{

‎composition-js/src/merging/merge.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3522,7 +3522,7 @@ class Merger {
35223522
// (The original feature version is still recorded in a @join__directive
35233523
// so we're not losing any information.)
35243524
const latestOrHighestLinkByIdentity = [...linksToPersist].reduce((map, link) => {
3525-
let latest = this.joinDirectiveFeatureDefinitionsByIdentity.get(link.identity)?.latest();
3525+
let latest = this.joinDirectiveFeatureDefinitionsByIdentity.get(link.identity)?.latestNonPreview();
35263526

35273527
const existing = map.get(link.identity) ?? link;
35283528
if (!latest || existing?.version.gt(latest.version)) {

‎internals-js/src/specs/connectSpec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ export const CONNECT_VERSIONS = new FeatureDefinitions<ConnectSpecDefinition>(
389389
new FeatureVersion(0, 4),
390390
new FeatureVersion(2, 13),
391391
),
392+
{ preview: true },
392393
);
393394

394395
registerKnownFeature(CONNECT_VERSIONS);

‎internals-js/src/specs/coreSpec.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,24 +578,37 @@ export class CoreSpecDefinition extends FeatureDefinition {
578578
}
579579
}
580580

581+
/**
582+
* Options for {@link FeatureDefinitions.add}. Currently only used for the
583+
* connect spec, where v0.4 is a preview version that should not be
584+
* automatically selected by {@link FeatureDefinitions.latestNonPreview}.
585+
*/
586+
interface FeatureDefinitionAddOptions {
587+
preview?: boolean;
588+
}
589+
581590
export class FeatureDefinitions<T extends FeatureDefinition = FeatureDefinition> {
582591
// The list of definition corresponding to the known version of the particular feature this object handles,
583592
// sorted by _decreased_ versions.
584593
private readonly _definitions: T[] = [];
594+
private readonly _previewVersions = new Set<string>();
585595

586596
constructor(readonly identity: string) {
587597
}
588598

589-
add(definition: T): FeatureDefinitions<T> {
599+
add(definition: T, options?: FeatureDefinitionAddOptions): FeatureDefinitions<T> {
590600
if (definition.identity !== this.identity) {
591601
throw buildError(`Cannot add definition for ${definition} to the versions of definitions for ${this.identity}`);
592602
}
593603
if (this._definitions.find(def => definition.version.equals(def.version))) {
594604
return this;
595605
}
596606
this._definitions.push(definition);
597-
// We sort by decreased versions sa it feels somewhat natural anyway to have more recent versions first.
607+
// We sort by decreased versions as it feels somewhat natural anyway to have more recent versions first.
598608
this._definitions.sort((def1, def2) => -def1.version.compareTo(def2.version));
609+
if (options?.preview) {
610+
this._previewVersions.add(definition.version.toString());
611+
}
599612
return this;
600613
}
601614

@@ -615,6 +628,18 @@ export class FeatureDefinitions<T extends FeatureDefinition = FeatureDefinition>
615628
return this._definitions[0];
616629
}
617630

631+
/**
632+
* Like {@link latest}, but skips versions marked as preview. Used by
633+
* `addJoinDirectiveDirectives` in the composition merger to avoid
634+
* stamping a preview spec version (e.g. connect/v0.4) into the
635+
* supergraph `@link` when no subgraph explicitly uses it. Falls back
636+
* to {@link latest} if all versions are preview.
637+
*/
638+
latestNonPreview(): T {
639+
assert(this._definitions.length > 0, 'Trying to get latest when no definitions exist');
640+
return this._definitions.find(def => !this._previewVersions.has(def.version.toString())) ?? this._definitions[0];
641+
}
642+
618643
getMinimumRequiredVersion(fedVersion: FeatureVersion): T {
619644
// this._definitions is already sorted with the most recent first
620645
// get the first definition that is compatible with the federation version

0 commit comments

Comments
 (0)