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(redshift-alpha): use same role for database-query singleton function #32363

Merged
merged 13 commits into from
Jan 3, 2025
16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/lib/private/database-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrant

const provider = new customresources.Provider(this, 'Provider', {
onEventHandler: handler,
role: this.getOrCreateInvokerRole(handler),
});

const queryHandlerProps: DatabaseQueryHandlerProps & HandlerProps = {
Expand Down Expand Up @@ -116,4 +117,19 @@ export class DatabaseQuery<HandlerProps> extends Construct implements iam.IGrant
}
return adminUser;
}

/**
* Get or create the IAM role for the singleton lambda function.
* We only need one function since it's just acting as an invoker.
* */
private getOrCreateInvokerRole(handler: lambda.SingletonFunction): iam.IRole {
const id = handler.constructName + 'InvokerRole';
const existing = cdk.Stack.of(this).node.tryFindChild(id);
return existing != null
? existing as iam.Role
: new iam.Role(cdk.Stack.of(this), id, {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
managedPolicies: [iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole')],
});
}
}
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/lib/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ abstract class UserBase extends Construct implements IUser {
...this.databaseProps,
user: this,
});

// The privilege should be granted or revoked when the table exists.
this.privileges.node.addDependency(table);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for my understanding, why do we need this to be add as dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've encountered errors during stack deletion when the table is removed before its associated privileges are deleted.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding the reason as a code comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will add a comment to it.

}

this.privileges.addPrivileges(table, ...actions);
Expand Down
21 changes: 21 additions & 0 deletions packages/@aws-cdk/aws-redshift-alpha/test/database-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,4 +229,25 @@ describe('database query', () => {
expect(stack.resolve(query.getAtt('attribute'))).toStrictEqual({ 'Fn::GetAtt': ['Query435140A1', 'attribute'] });
expect(stack.resolve(query.getAttString('attribute'))).toStrictEqual({ 'Fn::GetAtt': ['Query435140A1', 'attribute'] });
});

it('creates at most one IAM invoker role for handler', () => {
new DatabaseQuery(stack, 'Query0', {
...minimalProps,
});

new DatabaseQuery(stack, 'Query1', {
...minimalProps,
});

new DatabaseQuery(stack, 'Query2', {
...minimalProps,
});

const template = Template.fromStack(stack).toJSON();
const iamRoles = Object.entries(template.Resources)
.map(([k, v]) => [k, Object.getOwnPropertyDescriptor(v, 'Type')?.value])
.filter(([k, v]) => v === 'AWS::IAM::Role' && k.toString().includes('InvokerRole'));

expect(iamRoles.length === 1);
});
});
Binary file not shown.

Large diffs are not rendered by default.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading