Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(graphql-model-transformer): iam role name does not exceed 64 chars #8244

Merged
merged 1 commit into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/amplify-graphql-model-transformer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -186,9 +187,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',
Expand Down Expand Up @@ -1106,7 +1107,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'),
Expand Down Expand Up @@ -1179,4 +1180,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);
};
}
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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),
});
};
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions packages/amplify-graphql-transformer-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export interface TransformerResourceProvider {
generateResourceName(name: string): string;
generateIAMRoleName(name: string): string;
}
2 changes: 1 addition & 1 deletion packages/graphql-dynamodb-transformer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-transformer-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5864,7 +5864,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==
Expand Down