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 all 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
36 changes: 36 additions & 0 deletions packages/@aws-cdk/aws-redshift/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -454,3 +454,39 @@ const redshiftCluster = new Cluster(stack, 'Redshift', {

redshiftCluster.addDefaultIamRole(defaultRole);
```

## IAM roles

Attaching IAM roles to a Redshift Cluster grants permissions to the Redshift service to perform actions on your behalf.

```ts
declare const vpc: ec2.Vpc

const role = new iam.Role(this, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
});
const cluster = new Cluster(this, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
roles: [role],
});
```

Additional IAM roles can be attached to a cluster using the `addIamRole` method.

```ts
declare const vpc: ec2.Vpc

const role = new iam.Role(this, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
});
const cluster = new Cluster(this, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
});
cluster.addIamRole(role);
```
36 changes: 30 additions & 6 deletions packages/@aws-cdk/aws-redshift/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ 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 secretsmanager from '@aws-cdk/aws-secretsmanager';
import { Duration, IResource, RemovalPolicy, Resource, SecretValue, Token } from '@aws-cdk/core';
import { AwsCustomResource, PhysicalResourceId, AwsCustomResourcePolicy } from '@aws-cdk/custom-resources';
import { Duration, IResource, Lazy, 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 @@ -299,7 +299,7 @@ export interface ClusterProps {

/**
* A list of AWS Identity and Access Management (IAM) role that can be used by the cluster to access other AWS services.
* Specify a maximum of 10 roles.
* The maximum number of roles to attach to a cluster is subject to a quota.
*
* @default - No role is attached to the cluster.
*/
Expand Down Expand Up @@ -470,6 +470,13 @@ export class Cluster extends ClusterBase {
*/
protected parameterGroup?: IClusterParameterGroup;

/**
* The ARNs of the roles that will be attached to the cluster.
*
* **NOTE** Please do not access this directly, use the `addIamRole` method instead.
*/
private readonly roles: iam.IRole[];

constructor(scope: Construct, id: string, props: ClusterProps) {
super(scope, id);

Expand All @@ -478,6 +485,7 @@ export class Cluster extends ClusterBase {
subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS,
};
this.parameterGroup = props.parameterGroup;
this.roles = props?.roles ? [...props.roles] : [];

const removalPolicy = props.removalPolicy ?? RemovalPolicy.RETAIN;

Expand Down Expand Up @@ -557,7 +565,7 @@ export class Cluster extends ClusterBase {
nodeType: props.nodeType || NodeType.DC2_LARGE,
numberOfNodes: nodeCount,
loggingProperties,
iamRoles: props?.roles?.map(role => role.roleArn),
iamRoles: Lazy.list({ produce: () => this.roles.map(role => role.roleArn) }, { omitEmpty: true }),
dbName: props.defaultDatabaseName || 'default_db',
publiclyAccessible: props.publiclyAccessible || false,
// Encryption
Expand Down Expand Up @@ -688,12 +696,12 @@ export class Cluster extends ClusterBase {
*/
public addDefaultIamRole(defaultIamRole: iam.IRole): void {
// Get list of IAM roles attached to cluster
const clusterRoleList = this.cluster.iamRoles ?? [];
const clusterRoleList = this.roles ?? [];

// Check to see if default role is included in list of cluster IAM roles
var roleAlreadyOnCluster = false;
for (var i = 0; i < clusterRoleList.length; i++) {
if (clusterRoleList[i] == defaultIamRole.roleArn) {
if (clusterRoleList[i] === defaultIamRole) {
roleAlreadyOnCluster = true;
break;
}
Expand Down Expand Up @@ -729,8 +737,24 @@ export class Cluster extends ClusterBase {
policy: AwsCustomResourcePolicy.fromSdkCalls({
resources: AwsCustomResourcePolicy.ANY_RESOURCE,
}),
installLatestAwsSdk: false,
Rizxcviii marked this conversation as resolved.
Show resolved Hide resolved
});

defaultIamRole.grantPassRole(defaultRoleCustomResource.grantPrincipal);
}

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

if (clusterRoleList.includes(role)) {
throw new Error(`Role '${role.roleArn}' is already attached to the cluster`);
}

clusterRoleList.push(role);
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable-next-line import/no-unresolved */
import * as AWSLambda from 'aws-lambda';
import { TablePrivilege, UserTablePrivilegesHandlerProps } from '../handler-props';
import { executeStatement } from './redshift-data';
import { ClusterProps } from './types';
import { makePhysicalId } from './util';
import { TablePrivilege, UserTablePrivilegesHandlerProps } from '../handler-props';

export async function handler(props: UserTablePrivilegesHandlerProps & ClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) {
const username = props.username;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable-next-line import/no-unresolved */
import * as AWSLambda from 'aws-lambda';
import { Column } from '../../table';
import { executeStatement } from './redshift-data';
import { ClusterProps, TableAndClusterProps, TableSortStyle } from './types';
import { areColumnsEqual, getDistKeyColumn, getSortKeyColumns } from './util';
import { Column } from '../../table';

export async function handler(props: TableAndClusterProps, event: AWSLambda.CloudFormationCustomResourceEvent) {
const tableNamePrefix = props.tableName.prefix;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
import * as AWSLambda from 'aws-lambda';
/* eslint-disable-next-line import/no-extraneous-dependencies */
import * as SecretsManager from 'aws-sdk/clients/secretsmanager';
import { UserHandlerProps } from '../handler-props';
import { executeStatement } from './redshift-data';
import { ClusterProps } from './types';
import { makePhysicalId } from './util';
import { UserHandlerProps } from '../handler-props';

const secretsManager = new SecretsManager();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Column } from '../../table';
import { ClusterProps } from './types';
import { Column } from '../../table';

export function makePhysicalId(resourceName: string, clusterProps: ClusterProps, requestId: string): string {
return `${clusterProps.clusterName}:${clusterProps.databaseName}:${resourceName}:${requestId}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as cdk from '@aws-cdk/core';
import * as customresources from '@aws-cdk/custom-resources';
import { Construct } from 'constructs';
import { DatabaseQueryHandlerProps } from './handler-props';
import { Cluster } from '../cluster';
import { DatabaseOptions } from '../database-options';
import { DatabaseQueryHandlerProps } from './handler-props';

export interface DatabaseQueryProps<HandlerProps> extends DatabaseOptions {
readonly handler: string;
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-redshift/lib/private/privileges.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { DatabaseOptions } from '../database-options';
import { ITable, TableAction } from '../table';
import { IUser } from '../user';
import { DatabaseQuery } from './database-query';
import { HandlerName } from './database-query-provider/handler-name';
import { TablePrivilege as SerializedTablePrivilege, UserTablePrivilegesHandlerProps } from './handler-props';
import { DatabaseOptions } from '../database-options';
import { ITable, TableAction } from '../table';
import { IUser } from '../user';

/**
* The Redshift table and action that make up a privilege that can be granted to a Redshift user.
Expand Down
97 changes: 97 additions & 0 deletions packages/@aws-cdk/aws-redshift/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,103 @@ describe('default IAM role', () => {
});
});

describe('IAM role', () => {
test('roles can be directly attached to cluster during declaration', () => {
// GIVEN
const role = new iam.Role(stack, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
});
new Cluster(stack, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
roles: [role],
});

// THEN
Template.fromStack(stack).hasResource('AWS::Redshift::Cluster', {
Properties: {
IamRoles: Match.arrayEquals([
{ 'Fn::GetAtt': [Match.stringLikeRegexp('Role*'), 'Arn'] },
]),
},
});
});

test('roles can be attached to cluster after declaration', () => {
// 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,
});

// WHEN
cluster.addIamRole(role);

// THEN
Template.fromStack(stack).hasResource('AWS::Redshift::Cluster', {
Properties: {
IamRoles: Match.arrayEquals([
{ 'Fn::GetAtt': [Match.stringLikeRegexp('Role*'), 'Arn'] },
]),
},
});
});

test('roles can be attached to cluster in another stack', () => {
// GIVEN
const cluster = new Cluster(stack, 'Redshift', {
masterUser: {
masterUsername: 'admin',
},
vpc,
});

const newTestStack = new cdk.Stack(stack, 'NewTestStack', { env: { account: stack.account, region: stack.region } });
const role = new iam.Role(newTestStack, 'Role', {
assumedBy: new iam.ServicePrincipal('redshift.amazonaws.com'),
});

// WHEN
cluster.addIamRole(role);

// THEN
Template.fromStack(stack).hasResource('AWS::Redshift::Cluster', {
Properties: {
IamRoles: Match.arrayEquals([
{ 'Fn::ImportValue': Match.stringLikeRegexp('NewTestStack:ExportsOutputFnGetAttRole*') },
]),
},
});
});

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 '${role.roleArn}' 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
Expand Up @@ -17,7 +17,7 @@
"validateOnSynth": false,
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/fa991089a30fe26fab8225c5ca4145a65cd9c6f1c7ebebae477f130972053ca5.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/2a0f381c4c3f8c34bfd6f8afdccd71f2d437e9a354e3383b625429e833e7a53f.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -207,12 +207,6 @@
"data": "Cluster192CD0375"
}
],
"/redshift-defaultiamrole-integ/Cluster1/default-role": [
{
"type": "aws:cdk:warning",
"data": "installLatestAwsSdk was not specified, and defaults to true. You probably do not want this. Set the global context flag '@aws-cdk/customresources:installLatestAwsSdkDefault' to false to switch this behavior off project-wide, or set the property explicitly to true if you know you need to call APIs that are not in Lambda's built-in SDK version."
}
],
"/redshift-defaultiamrole-integ/Cluster1/default-role/Resource/Default": [
{
"type": "aws:cdk:logicalId",
Expand Down Expand Up @@ -273,12 +267,6 @@
"data": "Cluster2720FF351"
}
],
"/redshift-defaultiamrole-integ/Cluster2/default-role": [
{
"type": "aws:cdk:warning",
"data": "installLatestAwsSdk was not specified, and defaults to true. You probably do not want this. Set the global context flag '@aws-cdk/customresources:installLatestAwsSdkDefault' to false to switch this behavior off project-wide, or set the property explicitly to true if you know you need to call APIs that are not in Lambda's built-in SDK version."
}
],
"/redshift-defaultiamrole-integ/Cluster2/default-role/Resource/Default": [
{
"type": "aws:cdk:logicalId",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
}
}
},
"fa991089a30fe26fab8225c5ca4145a65cd9c6f1c7ebebae477f130972053ca5": {
"2a0f381c4c3f8c34bfd6f8afdccd71f2d437e9a354e3383b625429e833e7a53f": {
"source": {
"path": "redshift-defaultiamrole-integ.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "fa991089a30fe26fab8225c5ca4145a65cd9c6f1c7ebebae477f130972053ca5.json",
"objectKey": "2a0f381c4c3f8c34bfd6f8afdccd71f2d437e9a354e3383b625429e833e7a53f.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@
]
]
},
"InstallLatestAwsSdk": "false"
"InstallLatestAwsSdk": false
},
"DependsOn": [
"Cluster1defaultroleCustomResourcePolicy6ECBAB35"
Expand Down Expand Up @@ -932,7 +932,7 @@
]
]
},
"InstallLatestAwsSdk": "false"
"InstallLatestAwsSdk": false
},
"DependsOn": [
"Cluster2defaultroleCustomResourcePolicy042F9AF5"
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": {}
}