Skip to content

Commit

Permalink
fix(dynamodb): old global table replicas cannot be deleted (#8224)
Browse files Browse the repository at this point in the history 
The permissions required to clean up old DynamoDB Global Tables replicas
were set up in such a way that removing a replication region, or
dropping replication entirely (or when causing a table replacement),
they were removed before CloudFormation gets to the `CLEAN_UP` phase,
causing a clean up failure (and old tables would remain there).

This changes the way permissions are granted to the replication handler
resource so that they are added using a separate `iam.Policy` resource,
so that deleted permissions are also removed during the `CLEAN_UP` phase
after the resources depending on them have been deleted.

The tradeoff is that two additional resources are added to the stack
that defines the DynamoDB Global Tables, where previously those
permissions were mastered in the nested stack that holds the replication
handler. Unofrtunately, the nested stack gets it's `CLEAN_UP` phase
executed as part of the nested stack resource update, not during it's
parent stack's `CLEAN_UP` phase.

Fixes #7189


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@RomainMuller
RomainMuller authored Jun 8, 2020
1 parent 1fabd98 commit 00884c7
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 24 deletions.
65 changes: 60 additions & 5 deletions packages/@aws-cdk/aws-dynamodb/lib/table.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import * as appscaling from '@aws-cdk/aws-applicationautoscaling';
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as iam from '@aws-cdk/aws-iam';
import * as kms from '@aws-cdk/aws-kms';
import { Aws, CfnCondition, CfnCustomResource, Construct, CustomResource, Fn, IResource, Lazy, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core';
import {
Aws, CfnCondition, CfnCustomResource, Construct, CustomResource, Fn,
IResource, Lazy, RemovalPolicy, Resource, Stack, Token,
} from '@aws-cdk/core';
import { CfnTable, CfnTableProps } from './dynamodb.generated';
import * as perms from './perms';
import { ReplicaProvider } from './replica-provider';
Expand Down Expand Up @@ -931,7 +934,7 @@ export class Table extends TableBase {
this.tableSortKey = props.sortKey;
}

if (props.replicationRegions) {
if (props.replicationRegions && props.replicationRegions.length > 0) {
this.createReplicaTables(props.replicationRegions);
}
}
Expand Down Expand Up @@ -1245,9 +1248,12 @@ export class Table extends TableBase {
// Documentation at https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/V2gt_IAM.html
// is currently incorrect. AWS Support recommends `dynamodb:*` in both source and destination regions

const onEventHandlerPolicy = new SourceTableAttachedPolicy(this, provider.onEventHandler.role!);
const isCompleteHandlerPolicy = new SourceTableAttachedPolicy(this, provider.isCompleteHandler.role!);

// Permissions in the source region
this.grant(provider.onEventHandler, 'dynamodb:*');
this.grant(provider.isCompleteHandler, 'dynamodb:DescribeTable');
this.grant(onEventHandlerPolicy, 'dynamodb:*');
this.grant(isCompleteHandlerPolicy, 'dynamodb:DescribeTable');

let previousRegion;
for (const region of new Set(regions)) { // Remove duplicates
Expand All @@ -1261,6 +1267,10 @@ export class Table extends TableBase {
Region: region,
},
});
currentRegion.node.addDependency(
onEventHandlerPolicy.policy,
isCompleteHandlerPolicy.policy,
);

// Deploy time check to prevent from creating a replica in the region
// where this stack is deployed. Only needed for environment agnostic
Expand Down Expand Up @@ -1292,7 +1302,7 @@ export class Table extends TableBase {

// Permissions in the destination regions (outside of the loop to
// minimize statements in the policy)
provider.onEventHandler.addToRolePolicy(new iam.PolicyStatement({
onEventHandlerPolicy.grantPrincipal.addToPolicy(new iam.PolicyStatement({
actions: ['dynamodb:*'],
resources: this.regionalArns,
}));
Expand Down Expand Up @@ -1428,3 +1438,48 @@ interface ScalableAttributePair {
scalableReadAttribute?: ScalableTableAttribute;
scalableWriteAttribute?: ScalableTableAttribute;
}

/**
* An inline policy that is logically bound to the source table of a DynamoDB Global Tables
* "cluster". This is here to ensure permissions are removed as part of (and not before) the
* CleanUp phase of a stack update, when a replica is removed (or the entire "cluster" gets
* replaced).
*
* If statements are added directly to the handler roles (as opposed to in a separate inline
* policy resource), new permissions are in effect before clean up happens, and so replicas that
* need to be dropped can no longer be due to lack of permissions.
*/
class SourceTableAttachedPolicy extends Construct implements iam.IGrantable {
public readonly grantPrincipal: iam.IPrincipal;
public readonly policy: iam.IPolicy;

public constructor(sourceTable: Table, role: iam.IRole) {
super(sourceTable, `SourceTableAttachedPolicy-${role.node.uniqueId}`);

const policy = new iam.Policy(this, 'Resource', { roles: [role] });
this.policy = policy;
this.grantPrincipal = new SourceTableAttachedPrincipal(role, policy);
}
}

/**
* An `IPrincipal` entity that can be used as the target of `grant` calls, used by the
* `SourceTableAttachedPolicy` class so it can act as an `IGrantable`.
*/
class SourceTableAttachedPrincipal extends iam.PrincipalBase {
public constructor(private readonly role: iam.IRole, private readonly policy: iam.Policy) {
super();
}

public get policyFragment(): iam.PrincipalPolicyFragment {
return this.role.policyFragment;
}

public addToPrincipalPolicy(statement: iam.PolicyStatement): iam.AddToPrincipalPolicyResult {
this.policy.addStatements(statement);
return {
policyDependable: this.policy,
statementAdded: true,
};
}
}
169 changes: 150 additions & 19 deletions packages/@aws-cdk/aws-dynamodb/test/integ.global.expected.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,140 @@
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderOnEventHandlerServiceRole6F43DF4AA4E210EA": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "dynamodb:*",
"Effect": "Allow",
"Resource": [
{
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
{
"Fn::Join": [
"",
[
{
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
"/index/*"
]
]
}
]
},
{
"Action": "dynamodb:*",
"Effect": "Allow",
"Resource": [
{
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":dynamodb:eu-west-2:",
{
"Ref": "AWS::AccountId"
},
":table/",
{
"Ref": "TableCD117FA1"
}
]
]
},
{
"Fn::Join": [
"",
[
"arn:",
{
"Ref": "AWS::Partition"
},
":dynamodb:eu-central-1:",
{
"Ref": "AWS::AccountId"
},
":table/",
{
"Ref": "TableCD117FA1"
}
]
]
}
]
}
],
"Version": "2012-10-17"
},
"PolicyName": "TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderOnEventHandlerServiceRole6F43DF4AA4E210EA",
"Roles": [
{
"Fn::GetAtt": [
"awscdkawsdynamodbReplicaProviderNestedStackawscdkawsdynamodbReplicaProviderNestedStackResource18E3F12D",
"Outputs.cdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderOnEventHandlerServiceRole3E8625F3Ref"
]
}
]
}
},
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole397161288F61AAFA": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "dynamodb:DescribeTable",
"Effect": "Allow",
"Resource": [
{
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
{
"Fn::Join": [
"",
[
{
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
"/index/*"
]
]
}
]
}
],
"Version": "2012-10-17"
},
"PolicyName": "leSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole397161288F61AAFA",
"Roles": [
{
"Fn::GetAtt": [
"awscdkawsdynamodbReplicaProviderNestedStackawscdkawsdynamodbReplicaProviderNestedStackResource18E3F12D",
"Outputs.cdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole2F936EC4Ref"
]
}
]
}
},
"TableReplicaeuwest290D3CD3A": {
"Type": "Custom::DynamoDBReplica",
"Properties": {
Expand All @@ -55,6 +189,10 @@
},
"Region": "eu-west-2"
},
"DependsOn": [
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole397161288F61AAFA",
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderOnEventHandlerServiceRole6F43DF4AA4E210EA"
],
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
Expand All @@ -73,7 +211,9 @@
"Region": "eu-central-1"
},
"DependsOn": [
"TableReplicaeuwest290D3CD3A"
"TableReplicaeuwest290D3CD3A",
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole397161288F61AAFA",
"TableSourceTableAttachedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderOnEventHandlerServiceRole6F43DF4AA4E210EA"
],
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
Expand All @@ -91,7 +231,7 @@
},
"/",
{
"Ref": "AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510S3BucketCE06C497"
"Ref": "AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947S3Bucket5148F39F"
},
"/",
{
Expand All @@ -101,7 +241,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510S3VersionKey6B6B0A66"
"Ref": "AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947S3VersionKey0618C4C3"
}
]
}
Expand All @@ -114,7 +254,7 @@
"Fn::Split": [
"||",
{
"Ref": "AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510S3VersionKey6B6B0A66"
"Ref": "AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947S3VersionKey0618C4C3"
}
]
}
Expand All @@ -124,15 +264,6 @@
]
},
"Parameters": {
"referencetocdkdynamodbglobal20191121TableB640876BArn": {
"Fn::GetAtt": [
"TableCD117FA1",
"Arn"
]
},
"referencetocdkdynamodbglobal20191121TableB640876BRef": {
"Ref": "TableCD117FA1"
},
"referencetocdkdynamodbglobal20191121AssetParameters012c6b101abc4ea1f510921af61a3e08e05f30f84d7b35c40ca4adb1ace60746S3BucketE0999323Ref": {
"Ref": "AssetParameters012c6b101abc4ea1f510921af61a3e08e05f30f84d7b35c40ca4adb1ace60746S3BucketBDDEC9DD"
},
Expand Down Expand Up @@ -174,17 +305,17 @@
"Type": "String",
"Description": "Artifact hash for asset \"5e49cf64d8027f48872790f80cdb76c5b836ecf9a70b71be1eb937a5c25a47c1\""
},
"AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510S3BucketCE06C497": {
"AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947S3Bucket5148F39F": {
"Type": "String",
"Description": "S3 bucket for asset \"1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510\""
"Description": "S3 bucket for asset \"ffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947\""
},
"AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510S3VersionKey6B6B0A66": {
"AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947S3VersionKey0618C4C3": {
"Type": "String",
"Description": "S3 key for asset version \"1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510\""
"Description": "S3 key for asset version \"ffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947\""
},
"AssetParameters1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510ArtifactHashAB28BC52": {
"AssetParametersffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947ArtifactHashBF6B619B": {
"Type": "String",
"Description": "Artifact hash for asset \"1e7110d85a2e13b58c2a0fb09f018c144489abfafc62bf10f8ab3561a9cb8510\""
"Description": "Artifact hash for asset \"ffa367e57788c5b58cfac966968712006cbe11cfd301e6c94eb067350f8de947\""
}
}
}

0 comments on commit 00884c7

Please sign in to comment.