fix(graphql-model-transformer): iam role name does not exceed 64 char…

…acters (aws-amplify#8244)
marcvberg · Oct 13, 2021 · 576887d · 576887d
1 parent 80a0ed0
commit 576887d
Show file tree

Hide file tree

Showing 13 changed files with 121 additions and 26 deletions.
diff --git a/packages/amplify-graphql-model-transformer/package.json b/packages/amplify-graphql-model-transformer/package.json
@@ -59,11 +59,13 @@
         "graphql": "^14.5.8",
         "graphql-mapping-template": "4.18.3",
         "graphql-transformer-common": "4.19.9",
-        "lodash": "^4.17.21"
+        "lodash": "^4.17.21",
+        "md5": "^2.3.0"
     },
     "devDependencies": {
         "@types/fs-extra": "^8.0.1",
-        "@types/node": "^12.12.6"
+        "@types/node": "^12.12.6",
+        "@types/md5": "^2.3.1"
     },
     "jest": {
         "transform": {

diff --git a/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts b/packages/amplify-graphql-model-transformer/src/__tests__/model-transformer.test.ts
@@ -13,6 +13,7 @@ import {
   verifyInputCount,
   verifyMatchingTypes,
 } from './test-utils/helpers';
+import { expect as cdkExpect, haveResource } from '@aws-cdk/assert';
 
 const featureFlags = {
   getBoolean: jest.fn(),
@@ -982,4 +983,77 @@ describe('ModelTransformer: ', () => {
 
     validateModelSchema(parse(definition));
   });
+
+  it('should generate iam role names under 64 chars and subscriptions under 50', () => {
+    const validSchema = `
+      type ThisIsAVeryLongNameModelThatShouldNotGenerateIAMRoleNamesOver64Characters @model {
+          id: ID!
+          title: String!
+      }
+    `;
+
+    const config: SyncConfig = {
+      ConflictDetection: 'VERSION',
+      ConflictHandler: ConflictHandlerType.AUTOMERGE,
+    };
+
+    const transformer = new GraphQLTransform({
+      transformers: [new ModelTransformer()],
+      featureFlags,
+      transformConfig: {
+        ResolverConfig: {
+          project: config,
+        },
+      },
+    });
+    const out = transformer.transform(validSchema);
+    expect(out).toBeDefined();
+
+    const definition = out.schema;
+    expect(definition).toBeDefined();
+
+    const parsed = parse(definition);
+    const subscriptionType = getObjectType(parsed, 'Subscription');
+    expect(subscriptionType).toBeDefined();
+
+    subscriptionType!.fields!.forEach(it => {
+      expect(it.name.value.length <= 50).toBeTruthy();
+    });
+
+    const iamStackResource = out.stacks.ThisIsAVeryLongNameModelThatShouldNotGenerateIAMRoleNamesOver64Characters;
+    expect(iamStackResource).toBeDefined();
+    cdkExpect(iamStackResource).to(
+      haveResource('AWS::IAM::Role', {
+        AssumeRolePolicyDocument: {
+          Statement: [
+            {
+              Action: 'sts:AssumeRole',
+              Effect: 'Allow',
+              Principal: {
+                Service: 'appsync.amazonaws.com',
+              },
+            },
+          ],
+          Version: '2012-10-17',
+        },
+        RoleName: {
+          'Fn::Join': [
+            '',
+            [
+              'ThisIsAVeryLongNameM2d9fca-',
+              {
+                Ref: 'referencetotransformerrootstackGraphQLAPI20497F53ApiId',
+              },
+              '-',
+              {
+                Ref: 'referencetotransformerrootstackenv10C5A902Ref',
+              },
+            ],
+          ],
+        },
+      }),
+    );
+
+    validateModelSchema(parsed);
+  });
 });
diff --git a/packages/amplify-graphql-model-transformer/src/graphql-model-transformer.ts b/packages/amplify-graphql-model-transformer/src/graphql-model-transformer.ts
@@ -72,6 +72,7 @@ import {
   ObjectDefinitionWrapper,
 } from './wrappers/object-definition-wrapper';
 import { CfnRole } from '@aws-cdk/aws-iam';
+import md5 from 'md5';
 
 export type Nullable<T> = T | null;
 export type OptionalAndNullable<T> = Partial<T>;
@@ -188,9 +189,9 @@ export class ModelTransformer extends TransformerModelBase implements Transforme
       },
       subscriptions: {
         level: SubscriptionLevel.public,
-        onCreate: [toCamelCase(['onCreate', typeName])],
-        onDelete: [toCamelCase(['onDelete', typeName])],
-        onUpdate: [toCamelCase(['onUpdate', typeName])],
+        onCreate: [this.ensureValidSubscriptionName(toCamelCase(['onCreate', typeName]))],
+        onDelete: [this.ensureValidSubscriptionName(toCamelCase(['onDelete', typeName]))],
+        onUpdate: [this.ensureValidSubscriptionName(toCamelCase(['onUpdate', typeName]))],
       },
       timestamps: {
         createdAt: 'createdAt',
@@ -1108,7 +1109,7 @@ export class ModelTransformer extends TransformerModelBase implements Transforme
   }
 
   private createIAMRole(context: TransformerContextProvider, def: ObjectTypeDefinitionNode, stack: cdk.Stack, tableName: string) {
-    const roleName = context.resourceHelper.generateResourceName(ModelResourceIDs.ModelTableIAMRoleID(def!.name.value));
+    const roleName = context.resourceHelper.generateIAMRoleName(ModelResourceIDs.ModelTableIAMRoleID(def!.name.value));
     const role = new iam.Role(stack, ModelResourceIDs.ModelTableIAMRoleID(def!.name.value), {
       roleName: roleName,
       assumedBy: new iam.ServicePrincipal('appsync.amazonaws.com'),
@@ -1181,4 +1182,10 @@ export class ModelTransformer extends TransformerModelBase implements Transforme
       ...options,
     };
   };
+
+  private ensureValidSubscriptionName = (name: string): string => {
+    if (name.length <= 50) return name;
+
+    return name.slice(0, 45) + md5(name).slice(0, 5);
+  };
 }
diff --git a/packages/amplify-graphql-searchable-transformer/src/cdk/create-searchable-domain.ts b/packages/amplify-graphql-searchable-transformer/src/cdk/create-searchable-domain.ts
@@ -1,3 +1,4 @@
+import { TransformerContextProvider } from '@aws-amplify/graphql-transformer-interfaces';
 import { EbsDeviceVolumeType } from '@aws-cdk/aws-ec2';
 import { CfnDomain, Domain, ElasticsearchVersion } from '@aws-cdk/aws-elasticsearch';
 import { IRole, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
@@ -31,20 +32,14 @@ export const createSearchableDomain = (stack: Construct, parameterMap: Map<strin
 };
 
 export const createSearchableDomainRole = (
+  context: TransformerContextProvider,
   stack: Construct,
   parameterMap: Map<string, CfnParameter>,
-  apiId: string,
-  envParam: CfnParameter,
 ): IRole => {
   const { OpenSearchAccessIAMRoleLogicalID } = ResourceConstants.RESOURCES;
   const { OpenSearchAccessIAMRoleName } = ResourceConstants.PARAMETERS;
-  const { HasEnvironmentParameter } = ResourceConstants.CONDITIONS;
   return new Role(stack, OpenSearchAccessIAMRoleLogicalID, {
     assumedBy: new ServicePrincipal('appsync.amazonaws.com'),
-    roleName: Fn.conditionIf(
-      HasEnvironmentParameter,
-      Fn.join('-', [parameterMap.get(OpenSearchAccessIAMRoleName)!.valueAsString, apiId, envParam.valueAsString]),
-      Fn.join('-', [parameterMap.get(OpenSearchAccessIAMRoleName)!.valueAsString, apiId, envParam.valueAsString]),
-    ).toString(),
+    roleName: context.resourceHelper.generateIAMRoleName(parameterMap.get(OpenSearchAccessIAMRoleName)!.valueAsString),
   });
 };
diff --git a/packages/amplify-graphql-searchable-transformer/src/cdk/create-streaming-lambda.ts b/packages/amplify-graphql-searchable-transformer/src/cdk/create-streaming-lambda.ts
@@ -1,4 +1,4 @@
-import { GraphQLAPIProvider } from '@aws-amplify/graphql-transformer-interfaces';
+import { GraphQLAPIProvider, TransformerContextProvider } from '@aws-amplify/graphql-transformer-interfaces';
 import { EventSourceMapping, IFunction, LayerVersion, Runtime, StartingPosition } from '@aws-cdk/aws-lambda';
 import { CfnParameter, Construct, Fn, Stack, Duration } from '@aws-cdk/core';
 import { Effect, IRole, Policy, PolicyStatement, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
@@ -45,12 +45,12 @@ export const createLambda = (
   );
 };
 
-export const createLambdaRole = (stack: Construct, parameterMap: Map<string, CfnParameter>): IRole => {
+export const createLambdaRole = (context: TransformerContextProvider, stack: Construct, parameterMap: Map<string, CfnParameter>): IRole => {
   const { OpenSearchStreamingLambdaIAMRoleLogicalID } = ResourceConstants.RESOURCES;
   const { OpenSearchStreamingIAMRoleName } = ResourceConstants.PARAMETERS;
   const role = new Role(stack, OpenSearchStreamingLambdaIAMRoleLogicalID, {
     assumedBy: new ServicePrincipal('lambda.amazonaws.com'),
-    roleName: parameterMap.get(OpenSearchStreamingIAMRoleName)?.valueAsString,
+    roleName: context.resourceHelper.generateIAMRoleName(parameterMap.get(OpenSearchStreamingIAMRoleName)?.valueAsString ?? ''),
   });
   role.attachInlinePolicy(
     new Policy(stack, 'CloudwatchLogsAccess', {

diff --git a/packages/amplify-graphql-searchable-transformer/src/graphql-searchable-transformer.ts b/packages/amplify-graphql-searchable-transformer/src/graphql-searchable-transformer.ts
@@ -86,7 +86,7 @@ export class SearchableModelTransformer extends TransformerPluginBase {
 
     const domain = createSearchableDomain(stack, parameterMap, context.api.apiId);
 
-    const openSearchRole = createSearchableDomainRole(stack, parameterMap, context.api.apiId, envParam);
+    const openSearchRole = createSearchableDomainRole(context, stack, parameterMap);
 
     domain.grantReadWrite(openSearchRole);
 
@@ -99,7 +99,7 @@ export class SearchableModelTransformer extends TransformerPluginBase {
     );
 
     // streaming lambda role
-    const lambdaRole = createLambdaRole(stack, parameterMap);
+    const lambdaRole = createLambdaRole(context, stack, parameterMap);
     domain.grantWrite(lambdaRole);
 
     // creates streaming lambda

diff --git a/packages/amplify-graphql-transformer-core/package.json b/packages/amplify-graphql-transformer-core/package.json
@@ -62,11 +62,13 @@
         "graphql": "^14.5.8",
         "graphql-transformer-common": "4.19.9",
         "lodash": "^4.17.21",
-        "ts-dedent": "^2.0.0"
+        "ts-dedent": "^2.0.0",
+        "md5": "^2.3.0"
     },
     "devDependencies": {
         "@types/fs-extra": "^8.0.1",
-        "@types/node": "^12.12.6"
+        "@types/node": "^12.12.6",
+        "@types/md5": "^2.3.1"
     },
     "jest": {
         "transform": {

diff --git a/packages/amplify-graphql-transformer-core/src/transformation/sync-utils.ts b/packages/amplify-graphql-transformer-core/src/transformation/sync-utils.ts
@@ -41,7 +41,7 @@ export function createSyncTable(context: TransformerContext) {
 
 function createSyncIAMRole(context: TransformerContext, stack: cdk.Stack, tableName: string) {
   const role = new iam.Role(stack, SyncResourceIDs.syncIAMRoleName, {
-    roleName: context.resourceHelper.generateResourceName(SyncResourceIDs.syncIAMRoleName),
+    roleName: context.resourceHelper.generateIAMRoleName(SyncResourceIDs.syncIAMRoleName),
     assumedBy: new iam.ServicePrincipal('appsync.amazonaws.com'),
   });
 

diff --git a/packages/amplify-graphql-transformer-core/src/transformer-context/resource-helper.ts b/packages/amplify-graphql-transformer-core/src/transformer-context/resource-helper.ts
@@ -1,21 +1,35 @@
 import { GraphQLAPIProvider, TransformerResourceHelperProvider } from '@aws-amplify/graphql-transformer-interfaces';
 import { CfnParameter } from '@aws-cdk/core';
 import { StackManager } from './stack-manager';
+import md5 from 'md5';
 
 export class TransformerResourceHelper implements TransformerResourceHelperProvider {
   private api?: GraphQLAPIProvider;
   // eslint-disable-next-line no-useless-constructor
   constructor(private stackManager: StackManager) {}
   public generateResourceName = (name: string): string => {
     if (!this.api) {
-      throw new Error('API not initalized');
+      throw new Error('API not initialized');
     }
     this.ensureEnv();
     const env = (this.stackManager.getParameter('env') as CfnParameter).valueAsString;
     const apiId = this.api!.apiId;
     return `${name}-${apiId}-${env}`;
   };
 
+  public generateIAMRoleName = (name: string): string => {
+    if (!this.api) {
+      throw new Error('API not initialized');
+    }
+    this.ensureEnv();
+    const env = (this.stackManager.getParameter('env') as CfnParameter).valueAsString;
+    const apiId = this.api!.apiId;
+
+    // 38 = 26(apiId) + 10(env) + 2(-)
+    const shortName = name.slice(0, 64 - 38 - 6) + md5(name).slice(0, 6);
+    return `${shortName}-${apiId}-${env}`; // max of 64.
+  };
+
   bind(api: GraphQLAPIProvider) {
     this.api = api;
   }

diff --git a/...lify-graphql-transformer-interfaces/src/transformer-context/resource-resource-provider.ts b/...lify-graphql-transformer-interfaces/src/transformer-context/resource-resource-provider.ts
@@ -1,3 +1,4 @@
 export interface TransformerResourceProvider {
   generateResourceName(name: string): string;
+  generateIAMRoleName(name: string): string;
 }
diff --git a/packages/graphql-dynamodb-transformer/package.json b/packages/graphql-dynamodb-transformer/package.json
@@ -33,7 +33,7 @@
     "pluralize": "^8.0.0"
   },
   "devDependencies": {
-    "@types/md5": "^2.1.33",
+    "@types/md5": "^2.3.1",
     "@types/node": "^12.12.6"
   },
   "jest": {

diff --git a/packages/graphql-transformer-common/package.json b/packages/graphql-transformer-common/package.json
@@ -29,7 +29,7 @@
     "pluralize": "8.0.0"
   },
   "devDependencies": {
-    "@types/md5": "^2.1.33",
+    "@types/md5": "^2.3.1",
     "@types/node": "^12.12.6"
   },
   "jest": {

diff --git a/yarn.lock b/yarn.lock
@@ -5874,7 +5874,7 @@
   resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.172.tgz#aad774c28e7bfd7a67de25408e03ee5a8c3d028a"
   integrity sha512-/BHF5HAx3em7/KkzVKm3LrsD6HZAXuXO1AJZQ3cRRBZj4oHZDviWPYu0aEplAqDFNHZPW6d3G7KN+ONcCCC7pw==
 
-"@types/md5@^2.1.33":
+"@types/md5@^2.3.1":
   version "2.3.1"
   resolved "https://registry.yarnpkg.com/@types/md5/-/md5-2.3.1.tgz#010bcf3bb50a2cff3a574cb1c0b4051a9c67d6bc"
   integrity sha512-OK3oe+ALIoPSo262lnhAYwpqFNXbiwH2a+0+Z5YBnkQEwWD8fk5+PIeRhYA48PzvX9I4SGNpWy+9bLj8qz92RQ==