No redirects in CDN Worker (#5540)

Author: kamilkisiela

docker/docker-compose.community.yml

@@ -226,7 +226,6 @@ services:
       S3_ACCESS_KEY_ID: ${MINIO_ROOT_USER}
       S3_SECRET_ACCESS_KEY: ${MINIO_ROOT_PASSWORD}
       S3_BUCKET_NAME: artifacts
-      S3_PUBLIC_URL: 'http://localhost:8083'
       CDN_AUTH_PRIVATE_KEY: ${CDN_AUTH_PRIVATE_KEY}
       CDN_API: '1'
       CDN_API_BASE_URL: 'http://localhost:8082'

integration-tests/docker-compose.integration.yaml

@@ -36,7 +36,6 @@ services:
       S3_ACCESS_KEY_ID: minioadmin
       S3_SECRET_ACCESS_KEY: minioadmin
       S3_BUCKET_NAME: artifacts
-      S3_PUBLIC_URL: 'http://localhost:8083'
 
   local_broker:
     image: node:22.6.0-alpine3.20
@@ -169,7 +168,6 @@ services:
       GITHUB_APP_PRIVATE_KEY: 5f938d51a065476c4dc1b04aeba13afb
       FEEDBACK_SLACK_TOKEN: ''
       FEEDBACK_SLACK_CHANNEL: '#hive'
-      S3_PUBLIC_URL: 'http://localhost:8083'
       USAGE_ESTIMATOR_ENDPOINT: '${USAGE_ESTIMATOR_ENDPOINT}'
       RATE_LIMIT_ENDPOINT: '${RATE_LIMIT_ENDPOINT}'
       EMAIL_PROVIDER: '${EMAIL_PROVIDER}'

integration-tests/tests/api/artifacts-cdn.spec.ts

@@ -14,7 +14,7 @@ import { createSupergraphManager } from '@graphql-hive/apollo';
 import { graphql } from '../../testkit/gql';
 import { execute } from '../../testkit/graphql';
 import { initSeed } from '../../testkit/seed';
-import { getCDNPort, getServiceHost, KnownServices } from '../../testkit/utils';
+import { getServiceHost, KnownServices } from '../../testkit/utils';
 
 const s3Client = new S3Client({
   endpoint: 'http://127.0.0.1:9000',
@@ -250,9 +250,13 @@ function runArtifactsCDNTests(
         redirect: 'manual',
       });
 
-      expect(response.status).toBe(302);
-      expect(await response.text()).toBe('Found.');
-      expect(response.headers.get('location')).toBeDefined();
+      expect(response.status).toBe(200);
+      const body = await response.text();
+      expect(body).toMatchInlineSnapshot(`
+        type Query {
+          ping: String
+        }
+      `);
 
       const artifactContents = await fetchS3ObjectArtifact(
         'artifacts',
@@ -306,21 +310,8 @@ function runArtifactsCDNTests(
         redirect: 'manual',
       });
 
-      expect(response.status).toBe(302);
-      expect(await response.text()).toBe('Found.');
-      const locationHeader = response.headers.get('location');
-      expect(locationHeader).toBeDefined();
-      const locationUrl = new URL(locationHeader!);
-      expect(locationUrl.protocol).toBe('http:');
-      expect(locationUrl.hostname).toBe('localhost');
-      expect(locationUrl.port).toBe(getCDNPort().toString());
-
-      response = await fetch(locationHeader!, {
-        method: 'GET',
-        redirect: 'manual',
-      });
-      const body = await response.text();
       expect(response.status).toBe(200);
+      const body = await response.text();
       expect(body).toMatchInlineSnapshot(
         '[{"name":"ping","sdl":"type Query { ping: String }","url":"http://ping.com"}]',
       );

packages/libraries/apollo/src/version.ts

@@ -1 +1 @@
-export const version = '0.33.4';
+export const version = '0.36.0';

packages/libraries/core/src/version.ts

@@ -1 +1 @@
-export const version = '0.7.0';
+export const version = '0.8.0';

packages/libraries/envelop/src/version.ts

@@ -1 +1 @@
-export const version = '0.33.3';
+export const version = '0.33.8';

packages/libraries/yoga/src/version.ts

@@ -1 +1 @@
-export const version = '0.33.3';
+export const version = '0.37.0';

packages/services/cdn-worker/.env.template

@@ -1,5 +1,4 @@
 S3_ENDPOINT=http://localhost:9000
 S3_ACCESS_KEY_ID="minioadmin"
 S3_SECRET_ACCESS_KEY="minioadmin"
-S3_BUCKET_NAME="artifacts"
-S3_PUBLIC_URL="http://localhost:9000"
+S3_BUCKET_NAME="artifacts"

packages/services/cdn-worker/src/artifact-handler.ts

@@ -2,7 +2,7 @@ import * as itty from 'itty-router';
 import zod from 'zod';
 import { createAnalytics, type Analytics } from './analytics';
 import { type ArtifactStorageReader, type ArtifactsType } from './artifact-storage-reader';
-import { InvalidAuthKeyResponse, MissingAuthKeyResponse, UnexpectedError } from './errors';
+import { InvalidAuthKeyResponse, MissingAuthKeyResponse } from './errors';
 import { IsAppDeploymentActive } from './is-app-deployment-active';
 import type { KeyValidator } from './key-validation';
 import { createResponse } from './tracked-response';
@@ -13,16 +13,7 @@ export type GetArtifactActionFn = (
   artifactType: ArtifactsType,
   eTag: string | null,
 ) => Promise<
-  | { type: 'notModified' }
-  | { type: 'notFound' }
-  | { type: 'body'; body: string }
-  | {
-      type: 'redirect';
-      location: {
-        public: string;
-        private: string;
-      };
-    }
+  { type: 'notModified' } | { type: 'notFound' } | { type: 'response'; response: Response }
 >;
 
 type ArtifactRequestHandler = {
@@ -161,7 +152,8 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
       return createResponse(analytics, 'Not found.', { status: 404 }, params.targetId, request);
     }
 
-    if (result.type === 'redirect') {
+    if (result.type === 'response') {
+      const etag = result.response.headers.get('etag');
       if (params.artifactType === 'metadata') {
         // To not change a lot of logic and still reuse the etag bits, we
         // fetch metadata using the redirect location.
@@ -171,19 +163,8 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
         // We're using here a private location, because the public S3 endpoint may differ from the internal S3 endpoint. E.g. within a docker network,
         // and we're fetching the artifact from within the private network.
         // If they are the same, private and public locations will be the same.
-        const metadataResponse = await fetch(result.location.private);
-
-        if (!metadataResponse.ok) {
-          console.error(
-            'Failed to fetch metadata',
-            metadataResponse.status,
-            metadataResponse.statusText,
-          );
 
-          return new UnexpectedError(analytics, request);
-        }
-
-        const body = await metadataResponse.text();
+        const body = await result.response.clone().text();
 
         // Metadata in SINGLE projects is only Mesh's Metadata, and it always defines _schema
         const isMeshArtifact = body.includes(`"#/definitions/_schema"`);
@@ -192,7 +173,6 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
         // Mesh's Metadata shared by Mesh is always an object.
         // The top-level array was caused #3291 and fixed now, but we still need to handle the old data.
         if (isMeshArtifact && hasTopLevelArray) {
-          const etag = metadataResponse.headers.get('etag');
           return createResponse(
             analytics,
             body.substring(1, body.length - 1),
@@ -211,11 +191,17 @@ export const createArtifactRequestHandler = (deps: ArtifactRequestHandler) => {
 
       return createResponse(
         analytics,
-        'Found.',
-        // We're using here a public location, because we expose the Location to the end user and
-        // the public S3 endpoint may differ from the internal S3 endpoint. E.g. within a docker network.
-        // If they are the same, private and public locations will be the same.
-        { status: 302, headers: { Location: result.location.public } },
+        await result.response.text(),
+        {
+          status: 200,
+          headers: {
+            'Content-Type':
+              params.artifactType === 'metadata' || params.artifactType === 'services'
+                ? 'application/json'
+                : 'text/plain',
+            ...(etag ? { etag } : {}),
+          },
+        },
         params.targetId,
         request,
       );

packages/services/cdn-worker/src/artifact-storage-reader.ts

@@ -2,8 +2,6 @@ import zod from 'zod';
 import type { Analytics } from './analytics';
 import { AwsClient } from './aws';
 
-const presignedUrlExpirationSeconds = 60;
-
 export function buildArtifactStorageKey(
   targetId: string,
   artifactType: string,
@@ -61,54 +59,14 @@ export function buildAppDeploymentIsEnabledKey(
  * Read an artifact/app deployment operation from S3.
  */
 export class ArtifactStorageReader {
-  private publicUrl: URL | null;
-
   constructor(
     private s3: {
       client: AwsClient;
       endpoint: string;
       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,
     private analytics: Analytics | null,
-  ) {
-    this.publicUrl = publicUrl ? new URL(publicUrl) : null;
-  }
-
-  private async generatePresignedGetUrl(key: string): Promise<{
-    public: string;
-    private: string;
-  }> {
-    const [signedUrl] = await this.s3.client.sign(
-      [this.s3.endpoint, this.s3.bucketName, key].join('/'),
-      {
-        method: 'GET',
-        aws: { signQuery: true },
-        headers: {
-          'X-Amz-Expires': String(presignedUrlExpirationSeconds),
-        },
-        timeout: READ_TIMEOUT_MS,
-      },
-    );
-
-    if (!this.publicUrl) {
-      return {
-        public: signedUrl,
-        private: signedUrl,
-      };
-    }
-
-    const publicUrl = new URL(signedUrl);
-    publicUrl.protocol = this.publicUrl.protocol;
-    publicUrl.host = this.publicUrl.host;
-    publicUrl.port = this.publicUrl.port;
-
-    return {
-      public: publicUrl.toString(),
-      private: signedUrl,
-    };
-  }
+  ) {}
 
   /** Generate a pre-signed url for reading an artifact from a bucket for a limited time period. */
   async generateArtifactReadUrl(
@@ -123,7 +81,7 @@ export class ArtifactStorageReader {
 
     const key = buildArtifactStorageKey(targetId, artifactType, contractName);
 
-    const response = await this.s3.client.fetch(
+    const headResponse = await this.s3.client.fetch(
       [this.s3.endpoint, this.s3.bucketName, key].join('/'),
       {
         method: 'HEAD',
@@ -136,27 +94,42 @@ export class ArtifactStorageReader {
     this.analytics?.track(
       {
         type: 'r2',
-        statusCode: response.status,
+        statusCode: headResponse.status,
         action: 'HEAD artifact',
       },
       targetId,
     );
 
-    if (response.status === 200) {
-      if (etagValue && response.headers.get('etag') === etagValue) {
+    if (headResponse.status === 200) {
+      if (etagValue && headResponse.headers.get('etag') === etagValue) {
         return { type: 'notModified' } as const;
       }
 
-      return {
-        type: 'redirect',
-        location: await this.generatePresignedGetUrl(key),
-      } as const;
+      const getResponse = await this.s3.client.fetch(
+        [this.s3.endpoint, this.s3.bucketName, key].join('/'),
+        {
+          method: 'GET',
+          aws: {
+            signQuery: true,
+          },
+          timeout: READ_TIMEOUT_MS,
+        },
+      );
+
+      if (getResponse.ok) {
+        return {
+          type: 'response',
+          response: getResponse,
+        } as const;
+      }
+
+      throw new Error(`GET request failed with status ${getResponse.status}`);
     }
-    if (response.status === 404) {
+    if (headResponse.status === 404) {
       return { type: 'notFound' } as const;
     }
-    const body = await response.text();
-    throw new Error(`HEAD request failed with status ${response.status}: ${body}`);
+    const body = await headResponse.text();
+    throw new Error(`HEAD request failed with status ${headResponse.status}: ${body}`);
   }
 
   async isAppDeploymentEnabled(targetId: string, appName: string, appVersion: string) {