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

feat(redshift): IAM roles can be attached to a cluster, post creation #23791

Merged
merged 27 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a7f5330
Adding custom resource, tests
Rizxcviii Jan 18, 2023
39a43bd
modification: using interface instead
Rizxcviii Jan 23, 2023
adbfa48
removing console.log
Rizxcviii Jan 23, 2023
88c5bb2
modification: changing custom resource type
Rizxcviii Jan 23, 2023
fab283d
addition: generating integ test snapshot
Rizxcviii Jan 23, 2023
d6d377e
addition: updating README.md
Rizxcviii Jan 23, 2023
400fd22
modification: MD047/single-trailing-newline
Rizxcviii Jan 24, 2023
63cfe78
another test
Rizxcviii Jan 24, 2023
5fcd84f
diffAssets
Rizxcviii Jan 26, 2023
4dd83dc
removing test
Rizxcviii Jan 30, 2023
ff9900c
testing again
Rizxcviii Jan 30, 2023
18d2da9
line break EOF
Rizxcviii Jan 30, 2023
e48cf82
modification: preventing install of latest sdk, uses 2 or 3 version a…
Rizxcviii Jan 30, 2023
7abd3f5
Merge branch 'main' into feature/add-iam-role-after-declaration
Rizxcviii Feb 1, 2023
b647db6
modification: specifying role arn that is already attached
Rizxcviii Feb 1, 2023
9de8013
removal: test for max roles. Will let cloudformation take over as rol…
Rizxcviii Feb 1, 2023
dd9a42b
modification: lazy evaluation using private readonly constant
Rizxcviii Feb 2, 2023
ac08152
modification: using roleArns variable
Rizxcviii Feb 2, 2023
13a38ab
modification: removed custom resource
Rizxcviii Feb 2, 2023
69bd204
addition: importing Lazy module
Rizxcviii Feb 2, 2023
3b18cb4
addition, removal
Rizxcviii Feb 2, 2023
fa1b4fa
change in integ test
Rizxcviii Feb 2, 2023
83052d3
modification: reverting change in installLatestAwsSdk
Rizxcviii Feb 2, 2023
4967340
modification: cleanup, max roles are nott 10, but subject to a quota
Rizxcviii Feb 9, 2023
ddc5ff8
modification: eslint import order
Rizxcviii Feb 9, 2023
1b8d104
modification: changing to use private iam.IRole list instead of strin…
Rizxcviii Feb 9, 2023
8ec3caf
Merge branch 'main' into feature/add-iam-role-after-declaration
mergify[bot] Feb 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions packages/@aws-cdk/aws-redshift/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import { Duration, IResource, RemovalPolicy, Resource, SecretValue, Token } from '@aws-cdk/core';
import { AwsCustomResource, AwsCustomResourcePolicy, PhysicalResourceId } from '@aws-cdk/custom-resources';
import { Construct } from 'constructs';
import { DatabaseSecret } from './database-secret';
import { Endpoint } from './endpoint';
Expand Down Expand Up @@ -662,4 +663,52 @@ export class Cluster extends ClusterBase {
throw new Error('Cannot add a parameter to an imported parameter group.');
}
}

/**
* Adds a role to the cluster
*
* @param role the role to add
*/
public addIamRole(role: iam.IRole): void {
const clusterRoleList = this.cluster.iamRoles ?? [];

if (clusterRoleList.includes(role.roleArn)) {
throw new Error('Role is already attached to the cluster');
Rizxcviii marked this conversation as resolved.
Show resolved Hide resolved
}
if (clusterRoleList.length >= 10) {
throw new Error('Maximum number of IAM roles for a cluster is 10');
}

// On UPDATE or CREATE define the new list of roles. On DELETE, detech the role from the cluster
const roleCustomResource = new AwsCustomResource(this, `add-role-${role.node.id}`, {
Rizxcviii marked this conversation as resolved.
Show resolved Hide resolved
onUpdate: {
service: 'Redshift',
action: 'modifyClusterIamRoles',
parameters: {
ClusterIdentifier: this.cluster.ref,
AddIamRoles: [role.roleArn],
},
physicalResourceId: PhysicalResourceId.of(
`${role.roleArn}-${this.cluster.ref}`,
),
},
onDelete: {
service: 'Redshift',
action: 'modifyClusterIamRoles',
parameters: {
ClusterIdentifier: this.cluster.ref,
RemoveIamRoles: [role.roleArn],
},
physicalResourceId: PhysicalResourceId.of(
`${role.roleArn}-${this.cluster.ref}`,
),
},
policy: AwsCustomResourcePolicy.fromSdkCalls({
resources: AwsCustomResourcePolicy.ANY_RESOURCE,
}),
resourceType: 'Custom::ModifyClusterIamRoles',
});

role.grantPassRole(roleCustomResource.grantPrincipal);
}
}
62 changes: 62 additions & 0 deletions packages/@aws-cdk/aws-redshift/test/cluster.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Match, Template } from '@aws-cdk/assertions';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import * as cdk from '@aws-cdk/core';
Expand Down Expand Up @@ -613,6 +614,67 @@ test('elastic ip address', () => {
});
});

describe('IAM role', () => {
test('adding a role after cluster declaration creates a custom resource', () => {
// GIVEN
const cluster = new Cluster(stack, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
});

// WHEN
cluster.addIamRole(new iam.Role(stack, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
}));

// THEN
Template.fromStack(stack).hasResource('Custom::ModifyClusterIamRoles', {});
});

test('throws when adding more than 10 roles to cluster', () => {
// GIVEN
const cluster = new Cluster(stack, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
roles: new Array(10).fill(0).map((_, i) => new iam.Role(stack, `Role${i+1}`, {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
})),
});

expect(() =>
// WHEN
cluster.addIamRole(new iam.Role(stack, 'Role11', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
})),
// THEN
).toThrow(/Maximum number of IAM roles for a cluster is 10/);
});

test('throws when adding role that is already in cluster', () => {
// GIVEN
const role = new iam.Role(stack, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
});
const cluster = new Cluster(stack, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
roles: [role],
});

expect(() =>
// WHEN
cluster.addIamRole(role),
// THEN
).toThrow(/Role is already attached to the cluster/);
});
});

function testStack() {
const newTestStack = new cdk.Stack(undefined, undefined, { env: { account: '12345', region: 'us-test-1' } });
newTestStack.node.setContext('availability-zones:12345:us-test-1', ['us-test-1a', 'us-test-1b']);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"version": "29.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
"path": "IamRoleIntegDefaultTestDeployAssertBEF20992.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}