Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gateway-js: adjust for Apollo Server de-'engine' renames #148

Merged
merged 1 commit into from
Sep 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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": "0.0.4",
"@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