fix: move cors to open api (#338)

sam-goodwin · Apr 13, 2023 · c840fce · c840fce
1 parent f70f852
commit c840fce
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 53 deletions.
diff --git a/apps/tests/aws-runtime-cdk/src/app.mts b/apps/tests/aws-runtime-cdk/src/app.mts
@@ -59,6 +59,9 @@ const testService = new eventual.Service<typeof testServiceRuntime>(
         logLevel: LogLevel.DEBUG,
       },
     },
+    cors: {
+      allowOrigins: ["http://some-url.com"],
+    },
   }
 );
 

diff --git a/packages/@eventual/aws-cdk/src/command-service.ts b/packages/@eventual/aws-cdk/src/command-service.ts
@@ -4,8 +4,8 @@ import {
   IHttpApi,
 } from "@aws-cdk/aws-apigatewayv2-alpha";
 import {
-  ENV_NAMES,
   commandServiceFunctionSuffix,
+  ENV_NAMES,
   sanitizeFunctionName,
 } from "@eventual/aws-runtime";
 import { isDefaultNamespaceCommand } from "@eventual/core";
@@ -15,7 +15,7 @@ import {
   EVENTUAL_SYSTEM_COMMAND_NAMESPACE,
   generateOpenAPISpec,
 } from "@eventual/core/internal";
-import { Arn, Duration, Lazy, Stack, aws_iam } from "aws-cdk-lib";
+import { Arn, aws_iam, Duration, Lazy, Stack } from "aws-cdk-lib";
 import {
   Effect,
   IGrantable,
@@ -200,19 +200,7 @@ export class CommandService<Service = any> {
 
     // Service => Gateway
     this.gateway = new SpecHttpApi(props.serviceScope, "Gateway", {
-      // apiName: `eventual-api-${props.serviceName}`,
       apiDefinition: ApiDefinition.fromInline(this.specification),
-      corsPreflight: props.cors
-        ? {
-            ...props.cors,
-            allowMethods: Array.from(
-              new Set([
-                ...(props.cors.allowMethods ?? []),
-                CorsHttpMethod.OPTIONS,
-              ])
-            ),
-          }
-        : undefined,
     });
 
     this.gateway.node.addDependency(this.integrationRole);
@@ -346,6 +334,18 @@ export class CommandService<Service = any> {
           },
           ...spec.paths,
         },
+        ...(props.cors
+          ? {
+              [XAmazonApigatewayCors]: {
+                allowCredentials: props.cors.allowCredentials,
+                allowHeaders: props.cors.allowHeaders,
+                allowMethods: props.cors.allowMethods,
+                allowOrigins: props.cors.allowOrigins,
+                exposeHeaders: props.cors.exposeHeaders,
+                maxAge: props.cors.maxAge?.toSeconds(),
+              } satisfies XAmazonApigatewayCors,
+            }
+          : {}),
       };
     }
   }
@@ -487,6 +487,17 @@ interface XAmazonApigatewayAnyMethod {
   [XAmazonApiGatewayIntegration]: XAmazonApiGatewayIntegration;
 }
 
+const XAmazonApigatewayCors = "x-amazon-apigateway-cors";
+
+interface XAmazonApigatewayCors {
+  allowOrigins?: string[];
+  allowCredentials?: boolean;
+  exposeHeaders?: string[];
+  maxAge?: number;
+  allowMethods?: CorsHttpMethod[];
+  allowHeaders?: string[];
+}
+
 function commandNamespaceName(command: CommandSpec<any, any, any, any>) {
   let sanitizedName = sanitizeFunctionName(command.name);
   if (sanitizedName !== command.name) {

diff --git a/packages/@eventual/aws-cdk/src/constructs/spec-http-api.ts b/packages/@eventual/aws-cdk/src/constructs/spec-http-api.ts
@@ -95,44 +95,12 @@ export class SpecHttpApi extends HttpApiBase {
 
     props.apiDefinition.bindAfterCreate(this, this);
 
-    let corsConfiguration: CfnApi.CorsProperty | undefined;
-    if (props?.corsPreflight) {
-      const cors = props.corsPreflight;
-      if (
-        cors.allowOrigins &&
-        cors.allowOrigins.includes("*") &&
-        cors.allowCredentials
-      ) {
-        throw new Error(
-          "CORS preflight - allowCredentials is not supported when allowOrigin is '*'"
-        );
-      }
-      const {
-        allowCredentials,
-        allowHeaders,
-        allowMethods,
-        allowOrigins,
-        exposeHeaders,
-        maxAge,
-      } = props.corsPreflight;
-      corsConfiguration = {
-        allowCredentials,
-        allowHeaders,
-        allowMethods,
-        allowOrigins,
-        exposeHeaders,
-        maxAge: maxAge?.toSeconds(),
-      };
-    }
-
     const resource = new CfnApi(this, "Resource", {
       name: props.apiName,
       body: apiDefConfig.inlineDefinition ?? undefined,
       bodyS3Location: apiDefConfig.inlineDefinition
         ? undefined
         : apiDefConfig.s3Location,
-      corsConfiguration,
-      description: props?.description,
     });
 
     this.apiId = resource.ref;
@@ -165,10 +133,5 @@ export class SpecHttpApi extends HttpApiBase {
   }
 }
 
-export interface SpecHttpApiProps extends HttpApiProps {
-  /**
-   * Name for the HTTP API resource
-   * @default - id of the HttpApi construct.
-   */
-  readonly apiName?: string;
-}
+export interface SpecHttpApiProps
+  extends Omit<HttpApiProps, "corsPreflight" | "description"> {}
-Original file line number
+Diff line change
@@ Expand Up @@
             logLevel: LogLevel.DEBUG,
           },
         },
+        cors: {
+          allowOrigins: ["http://some-url.com"],
+        },
       }
     );
@@ Expand Down @@