From eb83c02ac26e318ce668979795034726906e19fc Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Mon, 9 Jan 2023 13:02:15 +0100 Subject: [PATCH 1/6] feat: use aws4fetch fork --- .../providers/artifact-storage-reader.ts | 64 +-- packages/services/api/src/shared/aws.ts | 463 ++++++++++++++++++ packages/services/cdn-worker/package.json | 1 - packages/services/cdn-worker/src/dev.ts | 15 +- packages/services/cdn-worker/src/index.ts | 21 +- packages/services/server/src/index.ts | 6 +- pnpm-lock.yaml | 2 - 7 files changed, 515 insertions(+), 57 deletions(-) create mode 100644 packages/services/api/src/shared/aws.ts diff --git a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts index a18efa09de..abcf810fa0 100644 --- a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts +++ b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts @@ -1,9 +1,8 @@ /** * IMPORTANT NOTE: This file needs to be kept platform-agnostic, don't use any Node.js specific APIs. */ -import { type S3Client, GetObjectCommand, HeadObjectCommand } from '@aws-sdk/client-s3'; -import { getSignedUrl } from '@aws-sdk/s3-request-presigner'; -import { fetch } from '@whatwg-node/fetch'; +import { Request } from '@whatwg-node/fetch'; +import { AwsClient } from '../../../shared/aws'; const presignedUrlExpirationSeconds = 60; @@ -19,31 +18,45 @@ export type ArtifactsType = SDLArtifactTypes | 'metadata' | 'services' | 'superg */ export class ArtifactStorageReader { private publicUrl: URL | null; + private awsClient: AwsClient; + private s3Endpoint: string; constructor( - private s3Client: S3Client, + s3Config: { + accessKeyId: string; + secretAccessKey: string; + endpoint: string; + }, private bucketName: string, /** The public URL in case the public S3 endpoint differs from the internal S3 endpoint. E.g. within a docker network. */ publicUrl: string | null, ) { this.publicUrl = publicUrl ? new URL(publicUrl) : null; + this.awsClient = new AwsClient({ + accessKeyId: s3Config.accessKeyId, + secretAccessKey: s3Config.secretAccessKey, + region: 'auto', + service: 's3', + }); + this.s3Endpoint = s3Config.endpoint; } private async generatePresignedGetUrl(key: string): Promise { - const command = new GetObjectCommand({ - Bucket: this.bucketName, - Key: key, - }); - - const presignedUrl = await getSignedUrl(this.s3Client, command, { - expiresIn: presignedUrlExpirationSeconds, - }); + const signedUrl = await this.awsClient.sign( + new Request(this.s3Endpoint + `/` + this.bucketName + '/' + key, { method: 'GET' }), + { + aws: { signQuery: true }, + headers: { + 'x-amz-expires': String(presignedUrlExpirationSeconds), + }, + }, + ); if (!this.publicUrl) { - return presignedUrl; + return signedUrl.url; } - const publicUrl = new URL(presignedUrl); + const publicUrl = new URL(signedUrl.url); publicUrl.protocol = this.publicUrl.protocol; publicUrl.host = this.publicUrl.host; publicUrl.port = this.publicUrl.port; @@ -63,26 +76,13 @@ export class ArtifactStorageReader { const key = buildArtifactStorageKey(targetId, artifactType); - // In case you are wondering why we generate a pre-signed URL for doing the HEAD - // request instead of just run the command with the AWS SDK: - // The S3 client is not platform agnostic and will fail when - // executed from within a Cloudflare Worker. - // Signing, on the other hand, is platform agnostic. - // AWS does not seem to fix this any time soon. - // See https://github.com/aws/aws-sdk-js-v3/issues/3104 - - const headCommand = await getSignedUrl( - this.s3Client, - new HeadObjectCommand({ - Bucket: this.bucketName, - Key: key, - }), + const response = await this.awsClient.fetch( + this.s3Endpoint + '/' + this.bucketName + '/' + key, + { + method: 'HEAD', + }, ); - const response = await fetch(headCommand, { - method: 'HEAD', - }); - if (response.status === 200) { if (etagValue && response.headers.get('etag') === etagValue) { return { type: 'notModified' } as const; diff --git a/packages/services/api/src/shared/aws.ts b/packages/services/api/src/shared/aws.ts new file mode 100644 index 0000000000..7b4260b443 --- /dev/null +++ b/packages/services/api/src/shared/aws.ts @@ -0,0 +1,463 @@ +/** + * This is a copy of https://github.com/mhart/aws4fetch which is licensed MIT + * See https://github.com/mhart/aws4fetch/issues/22 + */ +import { crypto, fetch, Headers, Request, TextEncoder } from '@whatwg-node/fetch'; + +const encoder = new TextEncoder(); + +const HOST_SERVICES: Record = { + appstream2: 'appstream', + cloudhsmv2: 'cloudhsm', + email: 'ses', + marketplace: 'aws-marketplace', + mobile: 'AWSMobileHubService', + pinpoint: 'mobiletargeting', + queue: 'sqs', + 'git-codecommit': 'codecommit', + 'mturk-requester-sandbox': 'mturk-requester', + 'personalize-runtime': 'personalize', +}; + +// https://github.com/aws/aws-sdk-js/blob/cc29728c1c4178969ebabe3bbe6b6f3159436394/lib/signers/v4.js#L190-L198 +const UNSIGNABLE_HEADERS = new Set([ + 'authorization', + 'content-type', + 'content-length', + 'user-agent', + 'presigned-expires', + 'expect', + 'x-amzn-trace-id', + 'range', + 'connection', +]); + +type AwsRequestInit = RequestInit & { + aws?: { + accessKeyId?: string; + secretAccessKey?: string; + sessionToken?: string; + service?: string; + region?: string; + cache?: Map; + datetime?: string; + signQuery?: boolean; + appendSessionToken?: boolean; + allHeaders?: boolean; + singleEncode?: boolean; + }; +}; + +export class AwsClient { + private secretAccessKey: string; + private accessKeyId: string; + private sessionToken?: string; + private service?: string; + private region?: string; + private cache: Map; + private retries: number; + private initRetryMs: number; + + constructor({ + accessKeyId, + secretAccessKey, + sessionToken, + service, + region, + cache, + retries, + initRetryMs, + }: { + accessKeyId: string; + secretAccessKey: string; + sessionToken?: string; + service?: string; + region?: string; + cache?: Map; + retries?: number; + initRetryMs?: number; + }) { + if (accessKeyId == null) throw new TypeError('accessKeyId is a required option'); + if (secretAccessKey == null) throw new TypeError('secretAccessKey is a required option'); + this.accessKeyId = accessKeyId; + this.secretAccessKey = secretAccessKey; + this.sessionToken = sessionToken; + this.service = service; + this.region = region; + this.cache = cache || new Map(); + this.retries = retries != null ? retries : 10; // Up to 25.6 secs + this.initRetryMs = initRetryMs || 50; + } + + async sign(input: RequestInfo, init?: AwsRequestInit) { + if (input instanceof Request) { + const { method, url, headers, body } = input; + init = Object.assign({ method, url, headers }, init); + if (init.body == null && headers.has('Content-Type')) { + init.body = + body != null && headers.has('X-Amz-Content-Sha256') + ? body + : await input.clone().arrayBuffer(); + } + input = url; + } + const signer = new AwsV4Signer(Object.assign({ url: input }, init, this, init && init.aws)); + const signed = Object.assign({}, init, await signer.sign()); + delete signed.aws; + try { + return new Request(signed.url.toString(), signed); + } catch (e) { + if (e instanceof TypeError) { + // https://bugs.chromium.org/p/chromium/issues/detail?id=1360943 + return new Request(signed.url.toString(), Object.assign({ duplex: 'half' }, signed)); + } + throw e; + } + } + + async fetch(input: RequestInfo, init: AwsRequestInit): Promise { + for (let i = 0; i <= this.retries; i++) { + const fetched = fetch(await this.sign(input, init)); + if (i === this.retries) { + return fetched; // No need to await if we're returning anyway + } + const res = await fetched; + if (res.status < 500 && res.status !== 429) { + return res; + } + await new Promise(resolve => + setTimeout(resolve, Math.random() * this.initRetryMs * Math.pow(2, i)), + ); + } + throw new Error('An unknown error occurred, ensure retries is not negative'); + } +} + +export class AwsV4Signer { + private method: string; + private url: URL; + private headers: Headers; + private body?: BodyInit | null; + private accessKeyId: string; + private secretAccessKey: string; + private sessionToken?: string; + private service: string; + private region: string; + private cache: Map; + private datetime: string; + private signQuery?: boolean; + private appendSessionToken?: boolean; + private signableHeaders: Array; + private signedHeaders: string; + private canonicalHeaders: string; + private credentialString: string; + private encodedPath: string; + private encodedSearch: string; + + constructor({ + method, + url, + headers, + body, + accessKeyId, + secretAccessKey, + sessionToken, + service, + region, + cache, + datetime, + signQuery, + appendSessionToken, + allHeaders, + singleEncode, + }: { + method?: string; + url: string; + headers?: HeadersInit; + body?: BodyInit | null; + accessKeyId: string; + secretAccessKey: string; + sessionToken?: string; + service?: string; + region?: string; + cache?: Map; + datetime?: string; + signQuery?: boolean; + appendSessionToken?: boolean; + allHeaders?: boolean; + singleEncode?: boolean; + }) { + if (url == null) throw new TypeError('url is a required option'); + if (accessKeyId == null) throw new TypeError('accessKeyId is a required option'); + if (secretAccessKey == null) throw new TypeError('secretAccessKey is a required option'); + + this.method = method || (body ? 'POST' : 'GET'); + this.url = new URL(url); + this.headers = new Headers(headers || {}); + this.body = body; + + this.accessKeyId = accessKeyId; + this.secretAccessKey = secretAccessKey; + this.sessionToken = sessionToken; + + let guessedService, guessedRegion; + if (!service || !region) { + [guessedService, guessedRegion] = guessServiceRegion(this.url, this.headers); + } + /** @type {string} */ + this.service = service || guessedService || ''; + this.region = region || guessedRegion || 'us-east-1'; + + this.cache = cache || new Map(); + this.datetime = datetime || new Date().toISOString().replace(/[:-]|\.\d{3}/g, ''); + this.signQuery = signQuery; + this.appendSessionToken = appendSessionToken || this.service === 'iotdevicegateway'; + + this.headers.delete('Host'); // Can't be set in insecure env anyway + + if (this.service === 's3' && !this.signQuery && !this.headers.has('X-Amz-Content-Sha256')) { + this.headers.set('X-Amz-Content-Sha256', 'UNSIGNED-PAYLOAD'); + } + + const params = this.signQuery ? this.url.searchParams : this.headers; + + params.set('X-Amz-Date', this.datetime); + if (this.sessionToken && !this.appendSessionToken) { + params.set('X-Amz-Security-Token', this.sessionToken); + } + + const theHeaders: Array = ['host']; + this.headers.forEach((_, key) => theHeaders.push(key)); + + // headers are always lowercase in keys() + this.signableHeaders = theHeaders + .filter(header => allHeaders || !UNSIGNABLE_HEADERS.has(header)) + .sort(); + + this.signedHeaders = this.signableHeaders.join(';'); + + // headers are always trimmed: + // https://fetch.spec.whatwg.org/#concept-header-value-normalize + this.canonicalHeaders = this.signableHeaders + .map( + header => + header + + ':' + + (header === 'host' + ? this.url.host + : (this.headers.get(header) || '').replace(/\s+/g, ' ')), + ) + .join('\n'); + + this.credentialString = [ + this.datetime.slice(0, 8), + this.region, + this.service, + 'aws4_request', + ].join('/'); + + if (this.signQuery) { + if (this.service === 's3' && !params.has('X-Amz-Expires')) { + params.set('X-Amz-Expires', '86400'); // 24 hours + } + params.set('X-Amz-Algorithm', 'AWS4-HMAC-SHA256'); + params.set('X-Amz-Credential', this.accessKeyId + '/' + this.credentialString); + params.set('X-Amz-SignedHeaders', this.signedHeaders); + } + + if (this.service === 's3') { + try { + /** @type {string} */ + this.encodedPath = decodeURIComponent(this.url.pathname.replace(/\+/g, ' ')); + } catch (e) { + this.encodedPath = this.url.pathname; + } + } else { + this.encodedPath = this.url.pathname.replace(/\/+/g, '/'); + } + if (!singleEncode) { + this.encodedPath = encodeURIComponent(this.encodedPath).replace(/%2F/g, '/'); + } + this.encodedPath = encodeRfc3986(this.encodedPath); + + const searchParams: Array<[string, string]> = []; + + this.url.searchParams.forEach((value, key) => searchParams.push([key, value])); + + const seenKeys = new Set(); + this.encodedSearch = searchParams + .filter(([k]) => { + if (!k) return false; // no empty keys + if (this.service === 's3') { + if (seenKeys.has(k)) return false; // first val only for S3 + seenKeys.add(k); + } + return true; + }) + .map(pair => pair.map(p => encodeRfc3986(encodeURIComponent(p)))) + .sort(([k1, v1], [k2, v2]) => (k1 < k2 ? -1 : k1 > k2 ? 1 : v1 < v2 ? -1 : v1 > v2 ? 1 : 0)) + .map(pair => pair.join('=')) + .join('&'); + } + + /** + * @returns {Promise<{ + * method: string + * url: URL + * headers: Headers + * body?: BodyInit | null + * }>} + */ + async sign() { + if (this.signQuery) { + this.url.searchParams.set('X-Amz-Signature', await this.signature()); + if (this.sessionToken && this.appendSessionToken) { + this.url.searchParams.set('X-Amz-Security-Token', this.sessionToken); + } + } else { + this.headers.set('Authorization', await this.authHeader()); + } + + return { + method: this.method, + url: this.url, + headers: this.headers, + body: this.body, + }; + } + + async authHeader(): Promise { + return [ + 'AWS4-HMAC-SHA256 Credential=' + this.accessKeyId + '/' + this.credentialString, + 'SignedHeaders=' + this.signedHeaders, + 'Signature=' + (await this.signature()), + ].join(', '); + } + + async signature(): Promise { + const date = this.datetime.slice(0, 8); + const cacheKey = [this.secretAccessKey, date, this.region, this.service].join(); + let kCredentials = this.cache.get(cacheKey); + if (!kCredentials) { + const kDate = await hmac('AWS4' + this.secretAccessKey, date); + const kRegion = await hmac(kDate, this.region); + const kService = await hmac(kRegion, this.service); + kCredentials = await hmac(kService, 'aws4_request'); + this.cache.set(cacheKey, kCredentials); + } + return buf2hex(await hmac(kCredentials, await this.stringToSign())); + } + + async stringToSign(): Promise { + return [ + 'AWS4-HMAC-SHA256', + this.datetime, + this.credentialString, + buf2hex(await hash(await this.canonicalString())), + ].join('\n'); + } + + async canonicalString(): Promise { + return [ + this.method.toUpperCase(), + this.encodedPath, + this.encodedSearch, + this.canonicalHeaders + '\n', + this.signedHeaders, + await this.hexBodyHash(), + ].join('\n'); + } + + async hexBodyHash(): Promise { + let hashHeader = + this.headers.get('X-Amz-Content-Sha256') || + (this.service === 's3' && this.signQuery ? 'UNSIGNED-PAYLOAD' : null); + if (hashHeader == null) { + if (this.body && typeof this.body !== 'string' && !('byteLength' in this.body)) { + throw new Error( + 'body must be a string, ArrayBuffer or ArrayBufferView, unless you include the X-Amz-Content-Sha256 header', + ); + } + hashHeader = buf2hex(await hash(this.body || '')); + } + return hashHeader; + } +} + +async function hmac( + key: string | ArrayBufferView | ArrayBuffer, + string: string, +): Promise { + const cryptoKey = await crypto.subtle.importKey( + 'raw', + typeof key === 'string' ? encoder.encode(key) : key, + { name: 'HMAC', hash: { name: 'SHA-256' } }, + false, + ['sign'], + ); + return crypto.subtle.sign('HMAC', cryptoKey, encoder.encode(string)); +} + +async function hash(content: string | ArrayBufferView | ArrayBuffer): Promise { + return crypto.subtle.digest( + 'SHA-256', + typeof content === 'string' ? encoder.encode(content) : content, + ); +} + +function buf2hex(buffer: ArrayBuffer) { + return Array.prototype.map + .call(new Uint8Array(buffer), x => ('0' + x.toString(16)).slice(-2)) + .join(''); +} + +function encodeRfc3986(urlEncodedStr: string): string { + return urlEncodedStr.replace(/[!'()*]/g, c => '%' + c.charCodeAt(0).toString(16).toUpperCase()); +} + +function guessServiceRegion(url: URL, headers: Headers) { + const { hostname, pathname } = url; + + if (hostname.endsWith('.r2.cloudflarestorage.com')) { + return ['s3', 'auto']; + } + if (hostname.endsWith('.backblazeb2.com')) { + const match = hostname.match(/^(?:[^.]+\.)?s3\.([^.]+)\.backblazeb2\.com$/); + return match != null ? ['s3', match[1]] : ['', '']; + } + const match = hostname + .replace('dualstack.', '') + .match(/([^.]+)\.(?:([^.]*)\.)?amazonaws\.com(?:\.cn)?$/); + let [service, region] = (match || ['', '']).slice(1, 3); + + if (region === 'us-gov') { + region = 'us-gov-west-1'; + } else if (region === 's3' || region === 's3-accelerate') { + region = 'us-east-1'; + service = 's3'; + } else if (service === 'iot') { + if (hostname.startsWith('iot.')) { + service = 'execute-api'; + } else if (hostname.startsWith('data.jobs.iot.')) { + service = 'iot-jobs-data'; + } else { + service = pathname === '/mqtt' ? 'iotdevicegateway' : 'iotdata'; + } + } else if (service === 'autoscaling') { + const targetPrefix = (headers.get('X-Amz-Target') || '').split('.')[0]; + if (targetPrefix === 'AnyScaleFrontendService') { + service = 'application-autoscaling'; + } else if (targetPrefix === 'AnyScaleScalingPlannerFrontendService') { + service = 'autoscaling-plans'; + } + } else if (region == null && service.startsWith('s3-')) { + region = service.slice(3).replace(/^fips-|^external-1/, ''); + service = 's3'; + } else if (service.endsWith('-fips')) { + service = service.slice(0, -5); + } else if (region && /-\d$/.test(service) && !/-\d$/.test(region)) { + [service, region] = [region, service]; + } + + return [HOST_SERVICES[service] || service, region]; +} diff --git a/packages/services/cdn-worker/package.json b/packages/services/cdn-worker/package.json index 82a5ef4f73..fea07e36aa 100644 --- a/packages/services/cdn-worker/package.json +++ b/packages/services/cdn-worker/package.json @@ -13,7 +13,6 @@ "graphql": "^16.0.0" }, "dependencies": { - "@aws-sdk/client-s3": "3.241.0", "graphql": "16.6.0", "toucan-js": "2.7.0", "zod": "3.20.2" diff --git a/packages/services/cdn-worker/src/dev.ts b/packages/services/cdn-worker/src/dev.ts index 884bfe1246..e744a0ec6c 100644 --- a/packages/services/cdn-worker/src/dev.ts +++ b/packages/services/cdn-worker/src/dev.ts @@ -1,6 +1,5 @@ import './dev-polyfill'; import { createServer } from 'http'; -import { S3Client } from '@aws-sdk/client-s3'; import { ArtifactStorageReader } from '@hive/api/src/modules/schema/providers/artifact-storage-reader'; import { createServerAdapter } from '@whatwg-node/server'; import itty from 'itty-router'; @@ -36,17 +35,15 @@ declare let S3_SECRET_ACCESS_KEY: string; declare let S3_BUCKET_NAME: string; declare let S3_PUBLIC_URL: string; -const s3Client = new S3Client({ - endpoint: S3_ENDPOINT, - credentials: { +const artifactStorageReader = new ArtifactStorageReader( + { accessKeyId: S3_ACCESS_KEY_ID, secretAccessKey: S3_SECRET_ACCESS_KEY, + endpoint: S3_ENDPOINT, }, - forcePathStyle: true, - region: 'auto', -}); - -const artifactStorageReader = new ArtifactStorageReader(s3Client, S3_BUCKET_NAME, S3_PUBLIC_URL); + S3_BUCKET_NAME, + S3_PUBLIC_URL, +); const handleArtifactRequest = createArtifactRequestHandler({ isKeyValid: createIsKeyValid({ keyData: KEY_DATA }), diff --git a/packages/services/cdn-worker/src/index.ts b/packages/services/cdn-worker/src/index.ts index 37cce03975..91a5934252 100644 --- a/packages/services/cdn-worker/src/index.ts +++ b/packages/services/cdn-worker/src/index.ts @@ -1,4 +1,3 @@ -import { S3Client } from '@aws-sdk/client-s3'; import { ArtifactStorageReader } from '@hive/api/src/modules/schema/providers/artifact-storage-reader'; import itty from 'itty-router'; import Toucan from 'toucan-js'; @@ -38,16 +37,6 @@ declare let S3_ACCESS_KEY_ID: string; declare let S3_SECRET_ACCESS_KEY: string; declare let S3_BUCKET_NAME: string; -const s3Client = new S3Client({ - endpoint: S3_ENDPOINT, - credentials: { - accessKeyId: S3_ACCESS_KEY_ID, - secretAccessKey: S3_SECRET_ACCESS_KEY, - }, - forcePathStyle: true, - region: 'auto', -}); - const analytics = createAnalytics({ usage: USAGE_ANALYTICS, error: ERROR_ANALYTICS, @@ -59,7 +48,15 @@ const handleRequest = createRequestHandler({ analytics, }); -const artifactStorageReader = new ArtifactStorageReader(s3Client, S3_BUCKET_NAME, null); +const artifactStorageReader = new ArtifactStorageReader( + { + accessKeyId: S3_ACCESS_KEY_ID, + secretAccessKey: S3_SECRET_ACCESS_KEY, + endpoint: S3_ENDPOINT, + }, + S3_BUCKET_NAME, + null, +); const handleArtifactRequest = createArtifactRequestHandler({ isKeyValid, diff --git a/packages/services/server/src/index.ts b/packages/services/server/src/index.ts index 11cd7ab14e..28f9c5f5b6 100644 --- a/packages/services/server/src/index.ts +++ b/packages/services/server/src/index.ts @@ -325,7 +325,11 @@ export async function main() { if (env.cdn.providers.api !== null) { const artifactStorageReader = new ArtifactStorageReader( - s3Client, + { + endpoint: env.s3.endpoint, + accessKeyId: env.s3.credentials.accessKeyId, + secretAccessKey: env.s3.credentials.secretAccessKey, + }, env.s3.bucketName, env.s3.publicUrl, ); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 23bc18bc3b..9fdfbe55b3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -423,7 +423,6 @@ importers: packages/services/cdn-worker: specifiers: - '@aws-sdk/client-s3': 3.241.0 '@cloudflare/workers-types': 4.20221111.1 '@hive/api': workspace:* '@types/service-worker-mock': 2.0.1 @@ -437,7 +436,6 @@ importers: toucan-js: 2.7.0 zod: 3.20.2 dependencies: - '@aws-sdk/client-s3': 3.241.0 graphql: 16.6.0 toucan-js: 2.7.0 zod: 3.20.2 From 476cb8e856593c298abca4c69e89d3715af0b5f1 Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Mon, 9 Jan 2023 15:47:59 +0100 Subject: [PATCH 2/6] weird code --- .../providers/artifact-storage-reader.ts | 3 ++- packages/services/api/src/shared/aws.ts | 19 +++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts index abcf810fa0..de8d7d3c89 100644 --- a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts +++ b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts @@ -45,9 +45,10 @@ export class ArtifactStorageReader { const signedUrl = await this.awsClient.sign( new Request(this.s3Endpoint + `/` + this.bucketName + '/' + key, { method: 'GET' }), { + method: 'GET', aws: { signQuery: true }, headers: { - 'x-amz-expires': String(presignedUrlExpirationSeconds), + 'X-Amz-Expires': String(presignedUrlExpirationSeconds), }, }, ); diff --git a/packages/services/api/src/shared/aws.ts b/packages/services/api/src/shared/aws.ts index 7b4260b443..c40d10bcf8 100644 --- a/packages/services/api/src/shared/aws.ts +++ b/packages/services/api/src/shared/aws.ts @@ -190,7 +190,6 @@ export class AwsV4Signer { if (url == null) throw new TypeError('url is a required option'); if (accessKeyId == null) throw new TypeError('accessKeyId is a required option'); if (secretAccessKey == null) throw new TypeError('secretAccessKey is a required option'); - this.method = method || (body ? 'POST' : 'GET'); this.url = new URL(url); this.headers = new Headers(headers || {}); @@ -227,7 +226,6 @@ export class AwsV4Signer { } const theHeaders: Array = ['host']; - this.headers.forEach((_, key) => theHeaders.push(key)); // headers are always lowercase in keys() this.signableHeaders = theHeaders @@ -258,7 +256,7 @@ export class AwsV4Signer { if (this.signQuery) { if (this.service === 's3' && !params.has('X-Amz-Expires')) { - params.set('X-Amz-Expires', '86400'); // 24 hours + params.set('X-Amz-Expires', this.headers.get('X-Amz-Expires') ?? '86400'); // 24 hours } params.set('X-Amz-Algorithm', 'AWS4-HMAC-SHA256'); params.set('X-Amz-Credential', this.accessKeyId + '/' + this.credentialString); @@ -300,15 +298,12 @@ export class AwsV4Signer { .join('&'); } - /** - * @returns {Promise<{ - * method: string - * url: URL - * headers: Headers - * body?: BodyInit | null - * }>} - */ - async sign() { + async sign(): Promise<{ + method: string; + url: URL; + headers: Headers; + body?: BodyInit | null; + }> { if (this.signQuery) { this.url.searchParams.set('X-Amz-Signature', await this.signature()); if (this.sessionToken && this.appendSessionToken) { From 32c72961c1a60939a2051ba1b0d31fad6f8333e2 Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 11 Jan 2023 11:19:01 +0100 Subject: [PATCH 3/6] refactor: no need to pass a request --- .../src/modules/schema/providers/artifact-storage-reader.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts index de8d7d3c89..ff6edf5b4a 100644 --- a/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts +++ b/packages/services/api/src/modules/schema/providers/artifact-storage-reader.ts @@ -1,7 +1,6 @@ /** * IMPORTANT NOTE: This file needs to be kept platform-agnostic, don't use any Node.js specific APIs. */ -import { Request } from '@whatwg-node/fetch'; import { AwsClient } from '../../../shared/aws'; const presignedUrlExpirationSeconds = 60; @@ -43,7 +42,7 @@ export class ArtifactStorageReader { private async generatePresignedGetUrl(key: string): Promise { const signedUrl = await this.awsClient.sign( - new Request(this.s3Endpoint + `/` + this.bucketName + '/' + key, { method: 'GET' }), + this.s3Endpoint + `/` + this.bucketName + '/' + key, { method: 'GET', aws: { signQuery: true }, From 149a7480f7aef798689d023bf5cd1f48ada4185f Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 11 Jan 2023 12:31:31 +0100 Subject: [PATCH 4/6] feat: add retry logic for client SDKs --- .changeset/nine-moose-itch.md | 5 + packages/libraries/client/src/apollo.ts | 64 +++++++----- packages/libraries/client/src/gateways.ts | 122 ++++++++++++++-------- 3 files changed, 120 insertions(+), 71 deletions(-) create mode 100644 .changeset/nine-moose-itch.md diff --git a/.changeset/nine-moose-itch.md b/.changeset/nine-moose-itch.md new file mode 100644 index 0000000000..584f556dda --- /dev/null +++ b/.changeset/nine-moose-itch.md @@ -0,0 +1,5 @@ +--- +'@graphql-hive/client': minor +--- + +Retry failed requests upon CDN issues. diff --git a/packages/libraries/client/src/apollo.ts b/packages/libraries/client/src/apollo.ts index aa683a520b..b679140d94 100644 --- a/packages/libraries/client/src/apollo.ts +++ b/packages/libraries/client/src/apollo.ts @@ -30,36 +30,48 @@ export function createSupergraphSDLFetcher({ endpoint, key }: SupergraphSDLFetch headers['If-None-Match'] = cacheETag; } - return axios - .get(endpoint + '/supergraph', { - headers, - }) - .then(response => { - if (response.status >= 200 && response.status < 300) { - const supergraphSdl = response.data; - const result = { - id: createHash('sha256').update(supergraphSdl).digest('base64'), - supergraphSdl, - }; - - const etag = response.headers['etag']; - if (etag) { - cached = result; - cacheETag = etag; + let retryCount = 0; + + const fetchWithRetry = (): Promise<{ id: string; supergraphSdl: string }> => { + return axios + .get(endpoint + '/supergraph', { + headers, + }) + .then(response => { + if (response.status >= 200 && response.status < 300) { + const supergraphSdl = response.data; + const result = { + id: createHash('sha256').update(supergraphSdl).digest('base64'), + supergraphSdl, + }; + + const etag = response.headers['etag']; + if (etag) { + cached = result; + cacheETag = etag; + } + + return result; } - return result; - } + if (retryCount > 10 || response.status < 500) { + return Promise.reject(new Error(`Failed to fetch supergraph [${response.status}]`)); + } - return Promise.reject(new Error(`Failed to fetch supergraph [${response.status}]`)); - }) - .catch(async error => { - if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { - return cached; - } + retryCount = retryCount + 1; - throw error; - }); + return fetchWithRetry(); + }) + .catch(async error => { + if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { + return cached; + } + + throw error; + }); + }; + + return fetchWithRetry(); }; } diff --git a/packages/libraries/client/src/gateways.ts b/packages/libraries/client/src/gateways.ts index 343a6f35b7..6eba3ae397 100644 --- a/packages/libraries/client/src/gateways.ts +++ b/packages/libraries/client/src/gateways.ts @@ -9,14 +9,14 @@ interface Schema { name: string; } -function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetcherOptions) { +function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetcherOptions) { let cacheETag: string | null = null; let cached: { id: string; supergraphSdl: string; } | null = null; - return function fetcher(): Promise { + return function fetcher(): Promise { const headers: { [key: string]: string; } = { @@ -29,64 +29,96 @@ function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetc headers['If-None-Match'] = cacheETag; } - return axios - .get(endpoint + '/services', { - headers, - responseType: 'json', - }) - .then(response => { - if (response.status >= 200 && response.status < 300) { - const result = response.data; - - const etag = response.headers['etag']; - if (etag) { - cached = result; - cacheETag = etag; + let retryCount = 0; + + const fetchWithRetry = (): Promise => { + return axios + .get(endpoint + '/services', { + headers, + responseType: 'json', + }) + .then(response => { + if (response.status >= 200 && response.status < 300) { + const result = response.data; + + const etag = response.headers['etag']; + if (etag) { + cached = result; + cacheETag = etag; + } + + return result; } - return result; - } + if (retryCount > 10 || response.status < 500) { + return Promise.reject(new Error(`Failed to fetch [${response.status}]`)); + } - return Promise.reject(new Error(`Failed to fetch [${response.status}]`)); - }) - .catch(async error => { - if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { - return cached; - } + retryCount = retryCount + 1; + + return fetchWithRetry(); + }) + .catch(async error => { + if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { + return cached; + } - throw error; - }); + throw error; + }); + }; + + return fetchWithRetry(); }; } export function createSchemaFetcher({ endpoint, key }: SchemaFetcherOptions) { - const fetcher = createFetcher({ endpoint, key }); + const fetcher = createFetcher({ endpoint, key }); return function schemaFetcher() { - return fetcher().then(schema => ({ - id: createHash('sha256') - .update(schema.sdl) - .update(schema.url || '') - .update(schema.name) - .digest('base64'), - ...schema, - })); + return fetcher().then(schema => { + let service: Schema; + // Before the new artifacts endpoint the body returned an array or a single object depending on the project type. + // This handles both in a backwards-compatible way. + if (schema instanceof Array) { + if (schema.length !== 1) { + throw new Error( + 'Encountered multiple services instead of a single service. Please use createServicesFetcher instead.', + ); + } + service = schema[0]; + } else { + service = schema; + } + + return { + id: createSchemaId(service), + ...service, + }; + }); }; } export function createServicesFetcher({ endpoint, key }: ServicesFetcherOptions) { - const fetcher = createFetcher({ endpoint, key }); + const fetcher = createFetcher({ endpoint, key }); return function schemaFetcher() { - return fetcher().then(services => - services.map(service => ({ - id: createHash('sha256') - .update(service.sdl) - .update(service.url || '') - .update(service.name) - .digest('base64'), - ...service, - })), - ); + return fetcher().then(services => { + if (services instanceof Array) { + return services.map(service => ({ + id: createSchemaId(service), + ...service, + })); + } + throw new Error( + 'Encountered a single service instead of a multiple services. Please use createSchemaFetcher instead.', + ); + }); }; } + +const createSchemaId = (service: Schema) => + createHash('sha256') + .update(service.sdl) + .update(service.url || '') + .update(service.name) + .digest('base64'); From a3ec6e5f4731d1233e0afa87f7c0d0df78e3f99d Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 11 Jan 2023 14:01:34 +0100 Subject: [PATCH 5/6] fix: refetch implementation + tests --- packages/libraries/client/src/apollo.ts | 28 +++++--- packages/libraries/client/src/gateways.ts | 28 +++++--- .../libraries/client/tests/apollo.spec.ts | 52 ++++++++++++++ .../libraries/client/tests/gateways.spec.ts | 71 +++++++++++++++++++ 4 files changed, 161 insertions(+), 18 deletions(-) diff --git a/packages/libraries/client/src/apollo.ts b/packages/libraries/client/src/apollo.ts index b679140d94..7b374e053c 100644 --- a/packages/libraries/client/src/apollo.ts +++ b/packages/libraries/client/src/apollo.ts @@ -32,6 +32,16 @@ export function createSupergraphSDLFetcher({ endpoint, key }: SupergraphSDLFetch let retryCount = 0; + const retry = (status: number) => { + if (retryCount >= 10 || status <= 499) { + return Promise.reject(new Error(`Failed to fetch [${status}]`)); + } + + retryCount = retryCount + 1; + + return fetchWithRetry(); + }; + const fetchWithRetry = (): Promise<{ id: string; supergraphSdl: string }> => { return axios .get(endpoint + '/supergraph', { @@ -54,17 +64,17 @@ export function createSupergraphSDLFetcher({ endpoint, key }: SupergraphSDLFetch return result; } - if (retryCount > 10 || response.status < 500) { - return Promise.reject(new Error(`Failed to fetch supergraph [${response.status}]`)); - } - - retryCount = retryCount + 1; - - return fetchWithRetry(); + return retry(response.status); }) .catch(async error => { - if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { - return cached; + if (axios.isAxiosError(error)) { + if (error.response?.status === 304 && cached !== null) { + return cached; + } + + if (error.response?.status) { + return retry(error.response.status); + } } throw error; diff --git a/packages/libraries/client/src/gateways.ts b/packages/libraries/client/src/gateways.ts index 6eba3ae397..6534fb8724 100644 --- a/packages/libraries/client/src/gateways.ts +++ b/packages/libraries/client/src/gateways.ts @@ -31,6 +31,16 @@ function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetcher let retryCount = 0; + const retry = (status: number) => { + if (retryCount >= 10 || status <= 499) { + return Promise.reject(new Error(`Failed to fetch [${status}]`)); + } + + retryCount = retryCount + 1; + + return fetchWithRetry(); + }; + const fetchWithRetry = (): Promise => { return axios .get(endpoint + '/services', { @@ -50,17 +60,17 @@ function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetcher return result; } - if (retryCount > 10 || response.status < 500) { - return Promise.reject(new Error(`Failed to fetch [${response.status}]`)); - } - - retryCount = retryCount + 1; - - return fetchWithRetry(); + return retry(response.status); }) .catch(async error => { - if (axios.isAxiosError(error) && error.response?.status === 304 && cached !== null) { - return cached; + if (axios.isAxiosError(error)) { + if (error.response?.status === 304 && cached !== null) { + return cached; + } + + if (error.response?.status) { + return retry(error.response.status); + } } throw error; diff --git a/packages/libraries/client/tests/apollo.spec.ts b/packages/libraries/client/tests/apollo.spec.ts index 6c2f914c7a..f1bc9fd725 100644 --- a/packages/libraries/client/tests/apollo.spec.ts +++ b/packages/libraries/client/tests/apollo.spec.ts @@ -82,3 +82,55 @@ test('createSupergraphSDLFetcher', async () => { expect(staleResult.id).toBeDefined(); expect(staleResult.supergraphSdl).toEqual(newSupergraphSdl); }); + +test('createSupergraphSDLFetcher retry with unexpected status code (nRetryCount=10)', async () => { + const supergraphSdl = 'type SuperQuery { sdl: String }'; + const key = 'secret-key'; + nock('http://localhost') + .get('/supergraph') + .times(10) + .reply(500) + .get('/supergraph') + .once() + .matchHeader('X-Hive-CDN-Key', key) + .reply(200, supergraphSdl, { + ETag: 'first', + }); + + const fetcher = createSupergraphSDLFetcher({ + endpoint: 'http://localhost', + key, + }); + + const result = await fetcher(); + + expect(result.id).toBeDefined(); + expect(result.supergraphSdl).toEqual(supergraphSdl); +}); + +test('createSupergraphSDLFetcher retry with unexpected status code (nRetryCount=11)', async () => { + expect.assertions(1); + const supergraphSdl = 'type SuperQuery { sdl: String }'; + const key = 'secret-key'; + nock('http://localhost') + .get('/supergraph') + .times(11) + .reply(500) + .get('/supergraph') + .once() + .matchHeader('X-Hive-CDN-Key', key) + .reply(200, supergraphSdl, { + ETag: 'first', + }); + + const fetcher = createSupergraphSDLFetcher({ + endpoint: 'http://localhost', + key, + }); + + try { + await fetcher(); + } catch (err) { + expect(err).toMatchInlineSnapshot(`[Error: Failed to fetch [500]]`); + } +}); diff --git a/packages/libraries/client/tests/gateways.spec.ts b/packages/libraries/client/tests/gateways.spec.ts index 4559792084..b9a3a7c671 100644 --- a/packages/libraries/client/tests/gateways.spec.ts +++ b/packages/libraries/client/tests/gateways.spec.ts @@ -222,3 +222,74 @@ test('createSchemaFetcher with ETag', async () => { expect(staleResult.sdl).toEqual(newSchema.sdl); expect(staleResult.url).toEqual(newSchema.url); }); + +test('retry in case of unexpected CDN status code (nRetryCount=10)', async () => { + const schema = { + sdl: 'type Query { noop: String }', + url: 'service-url', + name: 'service-name', + }; + + const key = 'secret-key'; + + nock('http://localhost') + .get('/services') + .times(10) + .matchHeader('X-Hive-CDN-Key', key) + .matchHeader('accept', 'application/json') + .reply(500) + .get('/services') + .once() + .matchHeader('X-Hive-CDN-Key', key) + .matchHeader('accept', 'application/json') + .reply(200, schema, { + ETag: 'first', + }); + + const fetcher = createSchemaFetcher({ + endpoint: 'http://localhost', + key, + }); + + const result = await fetcher(); + expect(result.id).toBeDefined(); + expect(result.name).toEqual(result.name); + expect(result.sdl).toEqual(result.sdl); + expect(result.url).toEqual(result.url); +}); + +test('fail in case of unexpected CDN status code (nRetryCount=11)', async () => { + expect.assertions(1); + const schema = { + sdl: 'type Query { noop: String }', + url: 'service-url', + name: 'service-name', + }; + + const key = 'secret-key'; + + nock('http://localhost') + .get('/services') + .times(11) + .matchHeader('X-Hive-CDN-Key', key) + .matchHeader('accept', 'application/json') + .reply(500) + .get('/services') + .once() + .matchHeader('X-Hive-CDN-Key', key) + .matchHeader('accept', 'application/json') + .reply(200, schema, { + ETag: 'first', + }); + + const fetcher = createSchemaFetcher({ + endpoint: 'http://localhost', + key, + }); + + try { + await fetcher(); + } catch (e) { + expect(e).toMatchInlineSnapshot(`[Error: Failed to fetch [500]]`); + } +}); From fe1d4ab0ea2467c5402d85a3b7eaebf71956ef1c Mon Sep 17 00:00:00 2001 From: Laurin Quast Date: Wed, 11 Jan 2023 14:22:21 +0100 Subject: [PATCH 6/6] fix: thank you cloudflare for throwing 499 --- packages/libraries/client/src/apollo.ts | 2 +- packages/libraries/client/src/gateways.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/libraries/client/src/apollo.ts b/packages/libraries/client/src/apollo.ts index 7b374e053c..73ac88613c 100644 --- a/packages/libraries/client/src/apollo.ts +++ b/packages/libraries/client/src/apollo.ts @@ -33,7 +33,7 @@ export function createSupergraphSDLFetcher({ endpoint, key }: SupergraphSDLFetch let retryCount = 0; const retry = (status: number) => { - if (retryCount >= 10 || status <= 499) { + if (retryCount >= 10 || status < 499) { return Promise.reject(new Error(`Failed to fetch [${status}]`)); } diff --git a/packages/libraries/client/src/gateways.ts b/packages/libraries/client/src/gateways.ts index 6534fb8724..7fdc177b64 100644 --- a/packages/libraries/client/src/gateways.ts +++ b/packages/libraries/client/src/gateways.ts @@ -32,7 +32,7 @@ function createFetcher({ endpoint, key }: SchemaFetcherOptions & ServicesFetcher let retryCount = 0; const retry = (status: number) => { - if (retryCount >= 10 || status <= 499) { + if (retryCount >= 10 || status < 499) { return Promise.reject(new Error(`Failed to fetch [${status}]`)); }