fix(custom-resources): IAM policy incorrect for SDKv3 names

In the `AwsCustomResource` and the `Assertions` libraries, we advertise accepting all of the following service name formats: * The SDKv3 service name: `api-gateway` * The full SDKv3 package name: `@aws-sdk/client-api-gateway` * The SDKv2 constructor name: `APIGateway` * The SDKv2 constructor name in all lower case: `apigateway` And the following action name formats: * The API call name: `GetRestApi` * The API call name with a lowercase starting letter method name: `getRestApi` * The SDKv3 command class name: `GetRestApiCommand` However, the code that was taking care of mapping service names into an IAM name was not handling all cases correctly. There was also an issue with some commands that end in the word `"Command"`, like ECS's `ExecuteCommand`, which according to the rules above should work both written as `ExecuteCommand` as well as `ExecuteCommandCommand`: we did not have enough information to know if we saw the string `ExecuteCommand`, whether we should interpret it as `Execute` or `ExecuteCommand`. Also, we were recommending to use the full SDKv3 package name and class name formats: ``` { service: '@aws-sdk/client-api-gateway', action: 'GetRestApiCommand', } ``` Which looks ugly (imo) and leaks too many of the underlying implementation details. This PR changes the following: - Deprecate the `sdk-api-metadata.json` we extracted from SDKv2. - From SDKv3 models, extract a new `sdk-v3-metadata.json` which contains the following information: - IAM prefix for every service - A list of APIs that end in the word `Command`, so we can disambiguate around these. - From `aws-sdk-codemod`, extract a mapping from SDKv2 service names to SDKv3 service names (replacing the copy/pasted code we used to have with a build-time extraction). - Unfortunately, both of these mappings are duplicated: once for the construct library, and once for the handlers. I did not want to go into deduplicating between these for now. - At runtime, we now map a potential V2 service name to a V3 service name, then look up the V3 metadata to determine the IAM prefix and the normalized action name. - There was a lot of duplication between the `assertions` handler and the `AwsCustomResource` handler. Introduce a new `ApiCall` class that unifies the behavior between these two call sites. - Change the recommendation in the README from using SDKv3 names to using shorter form names (`api-gateway` and `GetRestApi`). Fixes #27255, closes #27268, closes #27270.
aws · Sep 27, 2023 · cc0a90c · cc0a90c
1 parent 96ef5ff
commit cc0a90c
Show file tree

Hide file tree

Showing 35 changed files with 4,553 additions and 3,298 deletions.
diff --git a/nx.json b/nx.json
@@ -17,7 +17,8 @@
       "outputs": [
         "{projectRoot}/**/*.integ.*.js.snapshot/*",
         "{projectRoot}/tsconfig.json",
-        "{projectRoot}/**/lib/aws-custom-resource/sdk-api-metadata.json",
+        "{projectRoot}/**/lib/aws-custom-resource/sdk-v2-to-v3.json",
+        "{projectRoot}/**/lib/aws-custom-resource/sdk-v3-metadata.json",
         "{projectRoot}/**/build-info.json",
         "{projectRoot}/**/*.js",
         "{projectRoot}/**/*.js.map",

diff --git a/package.json b/package.json
@@ -19,6 +19,7 @@
     "@types/node": "18.11.19",
     "@types/prettier": "2.6.0",
     "@yarnpkg/lockfile": "^1.1.0",
+    "aws-sdk-js-codemod": "^0.18.3",
     "cdk-generate-synthetic-examples": "^0.1.291",
     "conventional-changelog-cli": "^2.2.2",
     "fs-extra": "^9.1.0",

diff --git a/...-resource-handlers/lib/custom-resources/aws-custom-resource-handler/aws-sdk-v2-handler.ts b/...-resource-handlers/lib/custom-resources/aws-custom-resource-handler/aws-sdk-v2-handler.ts
@@ -9,7 +9,9 @@ import { join } from 'path';
 /* eslint-disable-next-line import/no-extraneous-dependencies,import/no-unresolved */
 import * as AWSLambda from 'aws-lambda';
 import { AwsSdkCall } from './construct-types';
-import { decodeCall, decodeSpecialValues, filterKeys, flatten, respond, startsWithOneOf } from './shared';
+import { decodeCall, decodeSpecialValues, filterKeys, respond, startsWithOneOf } from './shared';
+// eslint-disable-next-line import/no-extraneous-dependencies
+import { flatten } from '@aws-cdk/sdk-v2-to-v3-adapter';
 
 let latestSdkInstalled = false;
 

diff --git a/...-resource-handlers/lib/custom-resources/aws-custom-resource-handler/aws-sdk-v3-handler.ts b/...-resource-handlers/lib/custom-resources/aws-custom-resource-handler/aws-sdk-v3-handler.ts
@@ -2,15 +2,15 @@
 /* eslint-disable no-console */
 import { execSync } from 'child_process';
 // eslint-disable-next-line import/no-extraneous-dependencies
-import { coerceApiParameters, findV3ClientConstructor, getV3ClientPackageName } from '@aws-cdk/sdk-v2-to-v3-adapter';
+import { ApiCall } from '@aws-cdk/sdk-v2-to-v3-adapter';
 // import the AWSLambda package explicitly,
 // which is globally available in the Lambda runtime,
 // as otherwise linking this repository with link-all.sh
 // fails in the CDK app executed with ts-node
 /* eslint-disable-next-line import/no-extraneous-dependencies,import/no-unresolved */
 import type * as AWSLambda from 'aws-lambda';
 import type { AwsSdkCall } from './construct-types';
-import { decodeCall, decodeSpecialValues, filterKeys, flatten, respond, startsWithOneOf } from './shared';
+import { decodeCall, decodeSpecialValues, filterKeys, respond, startsWithOneOf } from './shared';
 
 let installedSdk: { [service: string]: boolean } = {};
 
@@ -89,14 +89,9 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
     }
     const call: AwsSdkCall | undefined = event.ResourceProperties[event.RequestType];
     if (call) {
-      // when provide v2 service name, transform it v3 package name.
-      const packageName = call.service.startsWith('@aws-sdk/') ? call.service : getV3ClientPackageName(call.service);
-      const serviceShortName = packageName.split('/client-')[1]; // '@aws-sdk/client-s3' -> 's3'
+      const apiCall = new ApiCall(call.service, call.action);
 
-      let awsSdk: AwsSdk | Promise<AwsSdk> = loadAwsSdk(
-        packageName,
-        event.ResourceProperties.InstallLatestAwsSdk,
-      );
+      let awsSdk: AwsSdk | Promise<AwsSdk> = loadAwsSdk(apiCall.v3PackageName, event.ResourceProperties.InstallLatestAwsSdk);
 
       console.log(JSON.stringify({ ...event, ResponseURL: '...' }));
 
@@ -109,41 +104,29 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
           RoleSessionName: `${timestamp}-${physicalResourceId}`.substring(0, 64),
         };
 
-        const { fromTemporaryCredentials } = await import('@aws-sdk/credential-providers' as string);
+        const { fromTemporaryCredentials } = await import('@aws-sdk/credential-providers');
         credentials = fromTemporaryCredentials({
           params,
           clientConfig: call.region !== undefined ? { region: call.region } : undefined,
         });
       }
 
       awsSdk = await awsSdk;
-      const ServiceClient = findV3ClientConstructor(awsSdk);
-
-      const client = new ServiceClient({
-        apiVersion: call.apiVersion,
-        credentials: credentials,
-        region: call.region,
-      });
-      const commandName = call.action.endsWith('Command') ? call.action : `${call.action}Command`;
-      const shortCommandName = commandName.replace(/Command$/, ''); // 'PutObjectCommand' -> 'PutObject'
-      const Command = Object.entries(awsSdk).find(
-        ([name]) => name.toLowerCase() === commandName.toLowerCase(),
-      )?.[1] as { new (input: any): any };
-
-      let flatData: { [key: string]: string } = {};
+
+      const flatData: { [key: string]: string } = {};
       try {
-        // Command must pass input value https://github.com/aws/aws-sdk-js-v3/issues/424
-        const response = await client.send(
-          new Command(call.parameters
-            ? coerceApiParameters(serviceShortName, shortCommandName, decodeSpecialValues(call.parameters, physicalResourceId))
-            : {},
-          ),
-        );
-        flatData = {
-          apiVersion: client.config.apiVersion, // For test purposes: check if apiVersion was correctly passed.
-          region: await client.config.region().catch(() => undefined), // For test purposes: check if region was correctly passed.
-          ...flatten(response),
-        };
+        const response = await await apiCall.invoke({
+          sdkPackage: awsSdk,
+          apiVersion: call.apiVersion,
+          credentials: credentials,
+          region: call.region,
+          parameters: decodeSpecialValues(call.parameters, physicalResourceId),
+          flattenResponse: true,
+        });
+
+        flatData.apiVersion = apiCall.client.config.apiVersion; // For test purposes: check if apiVersion was correctly passed.
+        flatData.region = await apiCall.client.config.region().catch(() => undefined); // For test purposes: check if region was correctly passed.
+        Object.assign(flatData, response);
 
         let outputPaths: string[] | undefined;
         if (call.outputPath) {

diff --git a/...s-cdk/custom-resource-handlers/lib/custom-resources/aws-custom-resource-handler/shared.ts b/...s-cdk/custom-resource-handlers/lib/custom-resources/aws-custom-resource-handler/shared.ts
@@ -5,45 +5,6 @@ import * as AWSLambda from 'aws-lambda';
  */
 export const PHYSICAL_RESOURCE_ID_REFERENCE = 'PHYSICAL:RESOURCEID:';
 
-/**
- * Text decoder used for Uint8Array response parsing
- */
-const decoder = new TextDecoder();
-
-/**
- * Parse both buffers and ArrayBuffers which can be returned by sdkv3
- */
-function parseField(value: any): any {
-  if (Buffer.isBuffer(value)) {
-    return value.toString('utf8');
-  } else if (ArrayBuffer.isView(value)) {
-    return decoder.decode(value.buffer);
-  }
-
-  return value;
-}
-/**
- * Flattens a nested object
- *
- * @param object the object to be flattened
- * @returns a flat object with path as keys
- */
-export function flatten(object: object): { [key: string]: any } {
-  function _flatten(child: any, path: string[] = []): any {
-    return [].concat(...Object.keys(child)
-      .map(key => {
-        const childKey = parseField(child[key]);
-        return typeof childKey === 'object' && childKey !== null
-          ? _flatten(childKey, path.concat([key]))
-          : ({ [path.concat([key]).join('.')]: childKey });
-      }));
-  }
-  return Object.assign(
-    {},
-    ..._flatten(object),
-  );
-}
-
 /**
  * Decodes encoded special values (physicalResourceId)
  */

diff --git a/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/lambda-handler/sdk.ts b/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/lambda-handler/sdk.ts
@@ -1,98 +1,21 @@
 /* eslint-disable no-console */
 import { CustomResourceHandler } from './base';
 import { AwsApiCallRequest, AwsApiCallResult } from './types';
-import {
-  getV3ClientPackageName,
-  findV3ClientConstructor,
-  coerceApiParameters,
-} from '@aws-cdk/sdk-v2-to-v3-adapter';
-import { decodeParameters, coerceResponse } from './utils';
-
-/**
- * Flattens a nested object
- *
- * @param object the object to be flattened
- * @returns a flat object with path as keys
- */
-export function flatten(object: object): { [key: string]: any } {
-  return Object.assign(
-    {},
-    ...function _flatten(child: any, path: string[] = []): any {
-      return [].concat(...Object.keys(child)
-        .map(key => {
-          let childKey = Buffer.isBuffer(child[key]) ? child[key].toString('utf8') : child[key];
-          // if the value is a json string then treat it as an object
-          // and keep recursing. This allows for easier assertions against complex json strings
-          if (typeof childKey === 'string') {
-            childKey = isJsonString(childKey);
-          }
-          return typeof childKey === 'object' && childKey !== null
-            ? _flatten(childKey, path.concat([key]))
-            : ({ [path.concat([key]).join('.')]: childKey });
-        }));
-    }(object),
-  );
-}
-
-interface V3SdkPkg {
-  service: string;
-  packageName: string;
-  pkg: object;
-}
-
-function getServicePackage(service: string): V3SdkPkg {
-  const packageName = getV3ClientPackageName(service);
-  try {
-    /* eslint-disable-next-line @typescript-eslint/no-require-imports */
-    const pkg = require(packageName);
-
-    return {
-      service,
-      pkg,
-      packageName,
-    };
-  } catch (e) {
-    throw Error(`Service ${service} client package with name '${packageName}' does not exist.`);
-  }
-}
-
-function getServiceClient(sdkPkg: V3SdkPkg): any {
-  try {
-    const ServiceClient = findV3ClientConstructor(sdkPkg.pkg);
-    return new ServiceClient({});
-  } catch (e) {
-    console.error(e);
-    throw Error(`No client constructor found within package: ${sdkPkg.packageName}`);
-  }
-}
-
-function getSdkCommand(sdkPkg: V3SdkPkg, api: string): any {
-  const commandName = api.endsWith('Command') ? api : `${api}Command`;
-  const command = Object.entries(sdkPkg.pkg).find(
-    ([name]) => name.toLowerCase() === commandName.toLowerCase(),
-  )?.[1] as { new (input: any): any };
-
-  if (!command) {
-    throw new Error(`Unable to find command named: ${commandName} for api: ${api} in service package`);
-  }
-  return command;
-}
+import { ApiCall, flatten } from '@aws-cdk/sdk-v2-to-v3-adapter';
+import { decodeParameters, deepParseJson } from './utils';
 
 export class AwsApiCallHandler extends CustomResourceHandler<AwsApiCallRequest, AwsApiCallResult | { [key: string]: string }> {
   protected async processEvent(request: AwsApiCallRequest): Promise<AwsApiCallResult | { [key: string]: string } | undefined> {
-    const sdkPkg = getServicePackage(request.service);
-    const client = getServiceClient(sdkPkg);
+    const apiCall = new ApiCall(request.service, request.api);
 
-    const Command = getSdkCommand(sdkPkg, request.api);
-    const parameters = (request.parameters && decodeParameters(request.parameters)) ?? {};
-    const commandInput = coerceApiParameters(request.service, request.api, parameters);
+    const parameters = request.parameters ? decodeParameters(request.parameters) : {};
+    console.log(`SDK request to ${apiCall.service}.${apiCall.action} with parameters ${JSON.stringify(parameters)}`);
+    const response = await apiCall.invoke({ parameters }) as Record<string, unknown>;
+    const parsedResponse = deepParseJson(response);
 
-    console.log(`SDK request to ${sdkPkg.service}.${request.api} with parameters ${JSON.stringify(commandInput)}`);
-    const response = await client.send(new Command(commandInput));
-    await coerceResponse(response);
+    console.log(`SDK response received ${JSON.stringify(parsedResponse)}`);
+    delete parsedResponse.$metadata;
 
-    console.log(`SDK response received ${JSON.stringify(response)}`);
-    delete response.$metadata;
     const respond = {
       apiCallResponse: response,
     };
@@ -120,12 +43,4 @@ function filterKeys(object: object, searchStrings: string[]): { [key: string]: s
     }
     return filteredObject;
   }, {});
-}
-
-function isJsonString(value: string): any {
-  try {
-    return JSON.parse(value);
-  } catch {
-    return value;
-  }
-}
+}
diff --git a/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/lambda-handler/utils.ts b/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/lambda-handler/utils.ts
@@ -1,43 +1,38 @@
-async function coerceValue(v: any) {
-
-  if (v && typeof(v) === 'object' && typeof((v as any).transformToString) === 'function') {
-    // in sdk v3 some return types are now adapters that we need to explicitly
-    // convert to strings. see example: https://github.com/aws/aws-sdk-js-v3/blob/main/UPGRADING.md?plain=1#L573-L576
-    // note we don't use 'instanceof Unit8Array' because observations show this won't always return true, even though
-    // the `transformToString` function will be available. (for example S3::GetObject)
-    const text = await (v as any).transformToString();
-    return tryJsonParse(text);
+/**
+ * Recurse into the given object, trying to parse any string as JSON
+ */
+export function deepParseJson<A extends string>(x: A): unknown;
+export function deepParseJson<A extends object>(x: A): A;
+export function deepParseJson(x: unknown): unknown {
+  if (typeof x === 'string') {
+    return tryJsonParse(x);
+  }
+  if (Array.isArray(x)) {
+    return x.map(deepParseJson);
+  }
+  if (x && typeof x === 'object') {
+    for (const [key, value] of Object.entries(x)) {
+      (x as any)[key] = deepParseJson(value);
+    }
+
+    return x;
   }
-  return tryJsonParse(v);
 
+  return x;
 }
 
-function tryJsonParse(v: any) {
+function tryJsonParse(v: string): unknown {
   if (typeof(v) !== 'string') {
     return v;
   }
+
   try {
     return JSON.parse(v);
   } catch {
     return v;
   }
 }
 
-export async function coerceResponse(response: any) {
-
-  if (response == null) {
-    return;
-  }
-
-  for (const key of Object.keys(response)) {
-    response[key] = await coerceValue(response[key]);
-    if (typeof response[key] === 'object') {
-      await coerceResponse(response[key]);
-    }
-  }
-
-}
-
 export function decodeParameters(obj: Record<string, any>): any {
   return Object.fromEntries(Object.entries(obj).map(([key, value]) => {
     try {

diff --git a/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/provider.ts b/packages/@aws-cdk/integ-tests-alpha/lib/assertions/providers/provider.ts
@@ -1,8 +1,7 @@
 import * as path from 'path';
 import { Duration, CfnResource, AssetStaging, Stack, FileAssetPackaging, Token, Lazy, Reference } from 'aws-cdk-lib/core';
 import { Construct } from 'constructs';
-
-let SDK_METADATA: any = undefined;
+import { awsSdkToIamAction } from 'aws-cdk-lib/custom-resources/lib/helpers-internal';
 
 /**
  * Properties for a lambda function provider
@@ -157,15 +156,8 @@ class SingletonFunction extends Construct {
    * Create a policy statement from a specific api call
    */
   public addPolicyStatementFromSdkCall(service: string, api: string, resources?: string[]): void {
-    if (SDK_METADATA === undefined) {
-      // eslint-disable-next-line
-      SDK_METADATA = require('./sdk-api-metadata.json');
-    }
-    const srv = service.toLowerCase();
-    const iamService = (SDK_METADATA[srv] && SDK_METADATA[srv].prefix) || srv;
-    const iamAction = api.charAt(0).toUpperCase() + api.slice(1);
     this.lambdaFunction.addPolicies([{
-      Action: [`${iamService}:${iamAction}`],
+      Action: [awsSdkToIamAction(service, api)],
       Effect: 'Allow',
       Resource: resources || ['*'],
     }]);