Skip to content

Commit

Permalink
gateway-js: adjust for Apollo Server de-'engine' renames (#148)
Browse files Browse the repository at this point in the history
apollographql/apollo-server#4453 removes the old product
name "Engine" from the Apollo Server API. Most relevantly for the JS Gateway:

- The package `apollo-engine-reporting-protobuf` (used in the gateway to build
  federated traces) is renamed to `apollo-reporting-protobuf`.

- The `GraphQLService` interface implemented by `ApolloGateway` now passes
  configuration related to Apollo's hosted services in the `apollo` key instead of
  the `engine` key, and the values in that object change slightly
  too (`apiKeyHash` changes to `keyHash`, and `graphVariant` is always
  provided). Apollo Server continues to pass the old `engine` value as well for
  compatibility purposes. After this change, `ApolloGateway` can read its
  configuration from either key too.

There is no constraint on the order in which you upgrade Apollo Server and
Apollo Gateway, though it is still a good idea to upgrade them at the same
time (eg, so you only are using one protobuf package).

Also update a comment in federation-js, and upgrade the top-level
`@types/node-fetch` dependency so that (after running `npm dedup`) there's no
need for a `gateway-js/node_modules` directory.
  • Loading branch information
glasser authored Sep 24, 2020
1 parent da906d9 commit 749801a
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 250 deletions.
2 changes: 1 addition & 1 deletion federation-js/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export const entitiesField: GraphQLFieldConfig<any, any> = {
return reference;
};

// FIXME somehow get this to show up special in Engine traces?
// FIXME somehow get this to show up special in Studio traces?
const result = resolveReference(reference, context, info);

if (isPromise(result)) {
Expand Down
2 changes: 2 additions & 0 deletions gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
- __FIX__: Directives which are located on inline fragments should not be skipped and should be sent to the service [PR #178](https://github.com/apollographql/federation/pull/178)

- Read managed federation configuration from the `apollo` option to `ApolloGateway.load` rather than the deprecated `engine` option, when available (ie, when running Apollo Server v2.18+), and update error messages referring to the old Engine and Graph Manager product names. [PR #148](https://github.com/apollographql/federation/pull/148)

## v0.20.2

- __FIX__: Minifying a String argument should escape quotes and slashes [PR #174](https://github.com/apollographql/federation/pull/174)
Expand Down
6 changes: 3 additions & 3 deletions gateway-js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@
"@apollo/federation": "file:../federation-js",
"@apollo/query-planner-wasm": "file:../query-planner-wasm",
"@types/node-fetch": "2.5.4",
"apollo-engine-reporting-protobuf": "^0.5.2",
"apollo-reporting-protobuf": "^0.6.0",
"apollo-graphql": "^0.6.0",
"apollo-server-caching": "^0.5.2",
"apollo-server-core": "^2.17.0",
"apollo-server-core": "^2.18.0",
"apollo-server-env": "^2.4.5",
"apollo-server-errors": "^2.4.2",
"apollo-server-types": "^0.5.1",
"apollo-server-types": "^0.6.0",
"loglevel": "^1.6.1",
"make-fetch-happen": "^8.0.0",
"pretty-format": "^26.0.0"
Expand Down
14 changes: 9 additions & 5 deletions gateway-js/src/__tests__/gateway/reporting.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { GraphQLSchemaModule } from 'apollo-graphql';
import gql from 'graphql-tag';
import { buildFederatedSchema } from '@apollo/federation';
import { ApolloServer } from 'apollo-server';
import { ApolloServerPluginUsageReporting } from 'apollo-server-core';
import { execute, toPromise } from 'apollo-link';
import { createHttpLink } from 'apollo-link-http';
import fetch from 'node-fetch';
import { ApolloGateway } from '../..';
import { Plugin, Config, Refs } from 'pretty-format';
import { Report } from 'apollo-engine-reporting-protobuf';
import { Report } from 'apollo-reporting-protobuf';
import { fixtures } from 'apollo-federation-integration-testsuite';

// Normalize specific fields that change often (eg timestamps) to static values,
Expand Down Expand Up @@ -96,7 +97,7 @@ describe('reporting', () => {
reportResolver = resolve;
});

nockScope = nock('https://engine-report.apollodata.com')
nockScope = nock('https://usage-reporting.api.apollographql.com')
.post('/api/ingress/traces')
.reply(200, (_: any, requestBody: string) => {
reportResolver(requestBody);
Expand All @@ -116,10 +117,13 @@ describe('reporting', () => {
gatewayServer = new ApolloServer({
schema,
executor,
engine: {
apiKey: 'service:foo:bar',
sendReportsImmediately: true,
apollo: {
key: 'service:foo:bar',
graphVariant: 'current',
},
plugins: [ApolloServerPluginUsageReporting({
sendReportsImmediately: true,
})],
});
({ url: gatewayUrl } = await gatewayServer.listen({ port: 0 }));
});
Expand Down
26 changes: 13 additions & 13 deletions gateway-js/src/__tests__/integration/networkRequests.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ it('Extracts service definitions from remote storage', async () => {

const gateway = new ApolloGateway({ logger });

await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });
expect(gateway.schema!.getType('User')!.description).toBe('This is my User');
});

Expand Down Expand Up @@ -167,14 +167,14 @@ it.each([
logger
});

await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });
await blocker; // Wait for the definitions to be "fetched".

(isConflict
? expect(logger.warn)
: expect(logger.warn).not
).toHaveBeenCalledWith(expect.stringMatching(
/A local gateway service list is overriding an Apollo Graph Manager managed configuration/));
/A local gateway service list is overriding a managed federation configuration/));
spyGetServiceDefinitionsFromStorage.mockRestore();
});

Expand Down Expand Up @@ -222,7 +222,7 @@ it.skip('Rollsback to a previous schema when triggered', async () => {
gateway.experimental_pollInterval = 100;

gateway.onSchemaChange(onChange);
await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });

await firstSchemaChangeBlocker;
expect(onChange).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -258,7 +258,7 @@ it(`Retries GCS (up to ${GCS_RETRY_COUNT} times) on failure for each request and

const gateway = new ApolloGateway({ fetcher, logger });

await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });
expect(gateway.schema!.getType('User')!.description).toBe('This is my User');
});

Expand All @@ -271,9 +271,9 @@ it.skip(`Fails after the ${GCS_RETRY_COUNT + 1}th attempt to reach GCS`, async (

const gateway = new ApolloGateway({ fetcher, logger });
await expect(
gateway.load({ engine: { apiKeyHash, graphId } }),
gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } }),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Could not communicate with Apollo Graph Manager storage: "`,
`"Could not communicate with Apollo storage: "`,
);
});

Expand All @@ -287,9 +287,9 @@ it(`Errors when the secret isn't hosted on GCS`, async () => {

const gateway = new ApolloGateway({ fetcher, logger });
await expect(
gateway.load({ engine: { apiKeyHash, graphId } }),
gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } }),
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Unable to authenticate with Apollo Graph Manager storage while fetching https://storage-secrets.api.apollographql.com/federated-service/storage-secret/dd55a79d467976346d229a7b12b673ce.json. Ensure that the API key is configured properly and that a federated service has been pushed. For details, see https://go.apollo.dev/g/resolve-access-denied."`,
`"Unable to authenticate with Apollo storage while fetching https://storage-secrets.api.apollographql.com/federated-service/storage-secret/dd55a79d467976346d229a7b12b673ce.json. Ensure that the API key is configured properly and that a federated service has been pushed. For details, see https://go.apollo.dev/g/resolve-access-denied."`,
);
});

Expand Down Expand Up @@ -337,7 +337,7 @@ describe('Downstream service health checks', () => {

const gateway = new ApolloGateway({ serviceHealthCheck: true, logger });

await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });
expect(gateway.schema!.getType('User')!.description).toBe('This is my User');
});

Expand All @@ -353,7 +353,7 @@ describe('Downstream service health checks', () => {
const gateway = new ApolloGateway({ serviceHealthCheck: true, logger });

await expect(
gateway.load({ engine: { apiKeyHash, graphId } }),
gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } }),
).rejects.toThrowErrorMatchingInlineSnapshot(`"500: Internal Server Error"`);
});

Expand Down Expand Up @@ -394,7 +394,7 @@ describe('Downstream service health checks', () => {
gateway.experimental_pollInterval = 100;

gateway.onSchemaChange(onChange);
await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });

await schemaChangeBlocker1;
expect(gateway.schema!.getType('User')!.description).toBe('This is my User');
Expand Down Expand Up @@ -457,7 +457,7 @@ describe('Downstream service health checks', () => {
gateway.updateComposition = mockUpdateComposition;

// load the gateway as usual
await gateway.load({ engine: { apiKeyHash, graphId } });
await gateway.load({ apollo: { keyHash: apiKeyHash, graphId, graphVariant: 'current' } });

expect(gateway.schema!.getType('User')!.description).toBe('This is my User');

Expand Down
8 changes: 4 additions & 4 deletions gateway-js/src/executeQueryPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
TypeNameMetaFieldDef,
GraphQLFieldResolver,
} from 'graphql';
import { Trace, google } from 'apollo-engine-reporting-protobuf';
import { Trace, google } from 'apollo-reporting-protobuf';
import { defaultRootOperationNameLookup } from '@apollo/federation';
import { GraphQLDataSource } from './datasources/types';
import {
Expand Down Expand Up @@ -103,7 +103,7 @@ export async function executeQueryPlan<TContext>(
// Note: this function always returns a protobuf QueryPlanNode tree, even if
// we're going to ignore it, because it makes the code much simpler and more
// typesafe. However, it doesn't actually ask for traces from the backend
// service unless we are capturing traces for Engine.
// service unless we are capturing traces for Studio.
async function executeNode<TContext>(
context: ExecutionContext<TContext>,
node: PlanNode,
Expand Down Expand Up @@ -286,7 +286,7 @@ async function executeFetch<TContext>(
// GraphQLRequest.http is supposed to have if it exists.
let http: any;

// If we're capturing a trace for Engine, then save the operation text to
// If we're capturing a trace for Studio, then save the operation text to
// the node we're building and tell the federated service to include a trace
// in its response.
if (traceNode) {
Expand Down Expand Up @@ -327,7 +327,7 @@ async function executeFetch<TContext>(
context.errors.push(...errors);
}

// If we're capturing a trace for Engine, save the received trace into the
// If we're capturing a trace for Studio, save the received trace into the
// query plan.
if (traceNode) {
traceNode.receivedTime = dateToProtoTimestamp(new Date());
Expand Down
54 changes: 31 additions & 23 deletions gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
GraphQLExecutionResult,
Logger,
GraphQLRequestContextExecutionDidStart,
ApolloConfig,
} from 'apollo-server-types';
import { InMemoryLRUCache } from 'apollo-server-caching';
import {
Expand Down Expand Up @@ -193,7 +194,7 @@ export class ApolloGateway implements GraphQLService {
protected config: GatewayConfig;
private logger: Logger;
protected queryPlanStore?: InMemoryLRUCache<QueryPlan>;
private engineConfig: GraphQLServiceEngineConfig | undefined;
private apolloConfig?: ApolloConfig;
private pollingTimer?: NodeJS.Timer;
private onSchemaChangeListeners = new Set<SchemaChangeCallback>();
private serviceDefinitions: ServiceDefinition[] = [];
Expand Down Expand Up @@ -308,11 +309,16 @@ export class ApolloGateway implements GraphQLService {
}
}

public async load(options?: { engine?: GraphQLServiceEngineConfig }) {
if (options && options.engine) {
if (!options.engine.graphVariant)
this.logger.warn('No graph variant provided. Defaulting to `current`.');
this.engineConfig = options.engine;
public async load(options?: { apollo?: ApolloConfig; engine?: GraphQLServiceEngineConfig }) {
if (options?.apollo) {
this.apolloConfig = options.apollo;
} else if (options?.engine) {
// Older version of apollo-server-core that isn't passing 'apollo' yet.
this.apolloConfig = {
keyHash: options.engine.apiKeyHash,
graphId: options.engine.graphId,
graphVariant: options.engine.graphVariant || 'current',
}
}

await this.updateComposition();
Expand All @@ -323,12 +329,13 @@ export class ApolloGateway implements GraphQLService {
this.pollServices();
}

const { graphId, graphVariant } = (options && options.engine) || {};
const mode = isManagedConfig(this.config) ? 'managed' : 'unmanaged';

this.logger.info(
`Gateway successfully loaded schema.\n\t* Mode: ${mode}${
graphId ? `\n\t* Service: ${graphId}@${graphVariant || 'current'}` : ''
(this.apolloConfig && this.apolloConfig.graphId)
? `\n\t* Service: ${this.apolloConfig.graphId}@${this.apolloConfig.graphVariant}`
: ''
}`,
);

Expand Down Expand Up @@ -578,32 +585,33 @@ export class ApolloGateway implements GraphQLService {
protected async loadServiceDefinitions(
config: GatewayConfig,
): ReturnType<Experimental_UpdateServiceDefinitions> {
const canUseManagedConfig =
this.apolloConfig?.graphId && this.apolloConfig?.keyHash;
// This helper avoids the repetition of options in the two cases this method
// is invoked below. It is a helper, rather than an options object, since it
// depends on the presence of `this.engineConfig`, which is guarded against
// further down in this method in two separate places.
const getManagedConfig = (engineConfig: GraphQLServiceEngineConfig) => {
// is invoked below. Only call it if canUseManagedConfig is true
// (which makes its uses of ! safe)
const getManagedConfig = () => {
return getServiceDefinitionsFromStorage({
graphId: engineConfig.graphId,
apiKeyHash: engineConfig.apiKeyHash,
graphVariant: engineConfig.graphVariant,
graphId: this.apolloConfig!.graphId!,
apiKeyHash: this.apolloConfig!.keyHash!,
graphVariant: this.apolloConfig!.graphVariant,
federationVersion:
(config as ManagedGatewayConfig).federationVersion || 1,
fetcher: this.fetcher,
});
};

if (isLocalConfig(config) || isRemoteConfig(config)) {
if (this.engineConfig && !this.warnedStates.remoteWithLocalConfig) {
if (canUseManagedConfig && !this.warnedStates.remoteWithLocalConfig) {
// Only display this warning once per start-up.
this.warnedStates.remoteWithLocalConfig = true;
// This error helps avoid common misconfiguration.
// We don't await this because a local configuration should assume
// remote is unavailable for one reason or another.
getManagedConfig(this.engineConfig).then(() => {
getManagedConfig().then(() => {
this.logger.warn(
"A local gateway service list is overriding an Apollo Graph " +
"Manager managed configuration. To use the managed " +
"A local gateway service list is overriding a managed federation " +
"configuration. To use the managed " +
"configuration, do not specify a service list locally.",
);
}).catch(() => {}); // Don't mind errors if managed config is missing.
Expand All @@ -629,18 +637,18 @@ export class ApolloGateway implements GraphQLService {
});
}

if (!this.engineConfig) {
if (!canUseManagedConfig) {
throw new Error(
'When `serviceList` is not set, an Apollo Engine configuration must be provided. See https://www.apollographql.com/docs/apollo-server/federation/managed-federation/ for more information.',
'When `serviceList` is not set, an Apollo configuration must be provided. See https://www.apollographql.com/docs/apollo-server/federation/managed-federation/ for more information.',
);
}

return getManagedConfig(this.engineConfig);
return getManagedConfig();
}

// XXX Nothing guarantees that the only errors thrown or returned in
// result.errors are GraphQLErrors, even though other code (eg
// apollo-engine-reporting) assumes that. In fact, errors talking to backends
// ApolloServerPluginUsageReporting) assumes that. In fact, errors talking to backends
// are unlikely to show up as GraphQLErrors. Do we need to use
// formatApolloErrors or something?
public executor = async <TContext>(
Expand Down
12 changes: 4 additions & 8 deletions gateway-js/src/loadServicesFromStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function fetchApolloGcs(
return fetcher(input, init)
.catch(fetchError => {
throw new Error(
"Cannot access Apollo Graph Manager storage: " + fetchError)
"Cannot access Apollo storage: " + fetchError)
})
.then(async (response) => {
// If the fetcher has a cache and has implemented ETag validation, then
Expand All @@ -87,7 +87,7 @@ function fetchApolloGcs(
body.includes("Anonymous caller does not have storage.objects.get")
) {
throw new Error(
"Unable to authenticate with Apollo Graph Manager storage " +
"Unable to authenticate with Apollo storage " +
"while fetching " + url + ". Ensure that the API key is " +
"configured properly and that a federated service has been " +
"pushed. For details, see " +
Expand All @@ -97,7 +97,7 @@ function fetchApolloGcs(
// Normally, we'll try to keep the logs clean with errors we expect.
// If it's not a known error, reveal the full body for debugging.
throw new Error(
"Could not communicate with Apollo Graph Manager storage: " + body);
"Could not communicate with Apollo storage: " + body);
});
};

Expand All @@ -110,7 +110,7 @@ export async function getServiceDefinitionsFromStorage({
}: {
graphId: string;
apiKeyHash: string;
graphVariant?: string;
graphVariant: string;
federationVersion: number;
fetcher: typeof fetch;
}): ReturnType<Experimental_UpdateServiceDefinitions> {
Expand All @@ -121,10 +121,6 @@ export async function getServiceDefinitionsFromStorage({
const secret: string =
await fetchApolloGcs(fetcher, storageSecretUrl).then(res => res.json());

if (!graphVariant) {
graphVariant = 'current';
}

const baseUrl = `${urlPartialSchemaBase}/${secret}/${graphVariant}/v${federationVersion}`;

const compositionConfigResponse =
Expand Down
Loading

0 comments on commit 749801a

Please sign in to comment.