Skip to content

Commit db950b3

Browse files
authored
fix(redshift-alpha): use same role for database-query singleton function (#32363)
### Issue # (if applicable) Closes #32089. ### Reason for this change The Redshift tables use a singleton function as the invoker for various custom resource onEvent Lambda functions. Currently, each custom resource lambda function has a dedicated IAM role to assume. However, since it’s the same singleton function, a shared role could achieve the same outcome. ### Description of changes Use the same IAM role for the singleton invoker function to assume. ### Description of how you validated changes deployed to my local stack ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent de04742 commit db950b3

File tree

121 files changed

+25019
-54088
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

121 files changed

+25019
-54088
lines changed

packages/@aws-cdk/aws-redshift-alpha/lib/private/database-query.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrant
6464

6565
const provider = new customresources.Provider(this, 'Provider', {
6666
onEventHandler: handler,
67+
role: this.getOrCreateInvokerRole(handler),
6768
});
6869

6970
const queryHandlerProps: DatabaseQueryHandlerProps & HandlerProps = {
@@ -116,4 +117,19 @@ export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrant
116117
}
117118
return adminUser;
118119
}
120+
121+
/**
122+
* Get or create the IAM role for the singleton lambda function.
123+
* We only need one function since it's just acting as an invoker.
124+
* */
125+
private getOrCreateInvokerRole(handler: lambda.SingletonFunction): iam.IRole {
126+
const id = handler.constructName + 'InvokerRole';
127+
const existing = cdk.Stack.of(this).node.tryFindChild(id);
128+
return existing != null
129+
? existing as iam.Role
130+
: new iam.Role(cdk.Stack.of(this), id, {
131+
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
132+
managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole')],
133+
});
134+
}
119135
}

packages/@aws-cdk/aws-redshift-alpha/lib/user.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,9 @@ abstract class UserBase extends Construct implements IUser {
112112
...this.databaseProps,
113113
user: this,
114114
});
115+
116+
// The privilege should be granted or revoked when the table exists.
117+
this.privileges.node.addDependency(table);
115118
}
116119

117120
this.privileges.addPrivileges(table, ...actions);

packages/@aws-cdk/aws-redshift-alpha/test/database-query.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,4 +229,25 @@ describe('database query', () => {
229229
expect(stack.resolve(query.getAtt('attribute'))).toStrictEqual({ 'Fn::GetAtt': ['Query435140A1', 'attribute'] });
230230
expect(stack.resolve(query.getAttString('attribute'))).toStrictEqual({ 'Fn::GetAtt': ['Query435140A1', 'attribute'] });
231231
});
232+
233+
it('creates at most one IAM invoker role for handler', () => {
234+
new DatabaseQuery(stack, 'Query0', {
235+
...minimalProps,
236+
});
237+
238+
new DatabaseQuery(stack, 'Query1', {
239+
...minimalProps,
240+
});
241+
242+
new DatabaseQuery(stack, 'Query2', {
243+
...minimalProps,
244+
});
245+
246+
const template = Template.fromStack(stack).toJSON();
247+
const iamRoles = Object.entries(template.Resources)
248+
.map(([k, v]) => [k, Object.getOwnPropertyDescriptor(v, 'Type')?.value])
249+
.filter(([k, v]) => v === 'AWS::IAM::Role' && k.toString().includes('InvokerRole'));
250+
251+
expect(iamRoles.length === 1);
252+
});
232253
});
Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/aws-redshift-alpha/test/integ.cluster-distkey.js.snapshot/aws-cdk-redshift-distkey-create.assets.json

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)