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

refactor: clean up API for removal policy across the library #2893

Merged
merged 2 commits into from
Jun 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/@aws-cdk/app-delivery/test/integ.cicd.expected.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"ArtifactBucket7410C9EF": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"CodePipelineRoleB3A660B4": {
Expand Down Expand Up @@ -281,4 +282,4 @@
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"Bucket83908E77": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"AnAmazingWebsiteProbablyCFDistribution47E3983B": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"Bucket83908E77": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyDistributionCFDistributionDE147309": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"Bucket83908E77": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"LambdaServiceRoleA8ED4D3B": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"Bucket83908E77": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"BucketPolicyE9A3008A": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"Bucket83908E77": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyDistributionCFDistributionDE147309": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"Resources": {
"Bucket83908E77": {
"Type": "AWS::S3::Bucket"
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete"
},
"TrailS30071F172": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -124,4 +125,4 @@
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"CacheBucket41D9D0B0": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyProjectRole9BBE5233": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"MyRepoF4F48043": {
"DeletionPolicy": "Retain",
"Type": "AWS::ECR::Repository",
"Properties": {
"RepositoryPolicyText": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"MyBucketF68F3FF0": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyProjectRole9BBE5233": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"MyBucketF68F3FF0": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyProjectRole9BBE5233": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@
},
"PipelineBucketB967BD35": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
},
"PipelineBucketB967BD35": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"Resources": {
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"Resources": {
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@
},
"PipelineBucketB967BD35": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
},
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
},
"CodeDeployPipelineIntegTest9F618D61": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"Resources": {
"MyBucketF68F3FF0": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket"
},
"MyPipelineRoleC0D47CA4": {
Expand Down Expand Up @@ -226,7 +227,8 @@
}
},
"MyEcrRepo767466D0": {
"Type": "AWS::ECR::Repository"
"Type": "AWS::ECR::Repository",
"DeletionPolicy": "Retain"
eladb marked this conversation as resolved.
Show resolved Hide resolved
},
"MyEcrRepoawscdkcodepipelineecrsourceMyPipeline63CF3194SourceEventRule911FDB6D": {
"Type": "AWS::Events::Rule",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@
"Type": "AWS::ECS::Cluster"
},
"EcrRepoBB83A592": {
"Type": "AWS::ECR::Repository"
"Type": "AWS::ECR::Repository",
"DeletionPolicy": "Retain"
eladb marked this conversation as resolved.
Show resolved Hide resolved
},
"TaskDefTaskRole1EDB4A67": {
"Type": "AWS::IAM::Role",
Expand Down Expand Up @@ -310,6 +311,7 @@
},
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket",
"DeletionPolicy": "Delete",
"Properties": {
"VersioningConfiguration": {
"Status": "Enabled"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
}
},
"PipelineBucketB967BD35": {
"DeletionPolicy": "Delete",
"Type": "AWS::S3::Bucket",
"Properties": {
"VersioningConfiguration": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ export class Pipeline extends PipelineBase {
bucketName: PhysicalName.auto({ crossEnvironment: true }),
encryptionKey,
encryption: s3.BucketEncryption.Kms,
removalPolicy: RemovalPolicy.Orphan
removalPolicy: RemovalPolicy.Retain
eladb marked this conversation as resolved.
Show resolved Hide resolved
});
}
this.artifactBucket = propsBucket;
Expand Down
15 changes: 5 additions & 10 deletions packages/@aws-cdk/aws-ecr/lib/repository.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import events = require('@aws-cdk/aws-events');
import iam = require('@aws-cdk/aws-iam');
import { Construct, DeletionPolicy, IConstruct, IResource, Lazy, Resource, Stack, Token } from '@aws-cdk/cdk';
import { Construct, IConstruct, IResource, Lazy, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/cdk';
import { CfnRepository } from './ecr.generated';
import { CountType, LifecycleRule, TagStatus } from './lifecycle';

Expand Down Expand Up @@ -248,14 +248,11 @@ export interface RepositoryProps {
readonly lifecycleRegistryId?: string;

/**
* Retain the repository on stack deletion
* Determine what happens to the repository when the resource/stack is deleted.
*
* If you don't set this to true, the registry must be empty, otherwise
* your stack deletion will fail.
*
* @default false
* @default RemovalPolicy.Retain
*/
readonly retain?: boolean;
readonly removalPolicy?: RemovalPolicy;
}

export interface RepositoryAttributes {
Expand Down Expand Up @@ -347,9 +344,7 @@ export class Repository extends RepositoryBase {
lifecyclePolicy: Lazy.anyValue({ produce: () => this.renderLifecyclePolicy() }),
});

if (props.retain) {
resource.options.deletionPolicy = DeletionPolicy.Retain;
}
resource.applyRemovalPolicy(props.removalPolicy);

this.registryId = props.lifecycleRegistryId;
if (props.lifecycleRules) {
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ecr/test/integ.basic.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"Resources": {
"Repo02AC86CF": {
"Type": "AWS::ECR::Repository",
"DeletionPolicy": "Retain",
eladb marked this conversation as resolved.
Show resolved Hide resolved
"Properties": {
"LifecyclePolicy": {
"LifecyclePolicyText": "{\"rules\":[{\"rulePriority\":1,\"selection\":{\"tagStatus\":\"any\",\"countType\":\"imageCountMoreThan\",\"countNumber\":5},\"action\":{\"type\":\"expire\"}}]}"
Expand Down
26 changes: 23 additions & 3 deletions packages/@aws-cdk/aws-ecr/test/test.repository.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect, haveResource, haveResourceLike, ResourcePart } from '@aws-cdk/assert';
import iam = require('@aws-cdk/aws-iam');
import cdk = require('@aws-cdk/cdk');
import { RemovalPolicy } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import ecr = require('../lib');

Expand All @@ -18,7 +19,8 @@ export = {
expect(stack).toMatch({
Resources: {
Repo02AC86CF: {
Type: "AWS::ECR::Repository"
Type: "AWS::ECR::Repository",
DeletionPolicy: "Retain"
}
}
});
Expand Down Expand Up @@ -320,18 +322,36 @@ export = {
}
},

'"retain" can be used to retain the repo when the resource is deleted'(test: Test) {
'removal policy is "Retain" by default'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new ecr.Repository(stack, 'Repo', { retain: true });
new ecr.Repository(stack, 'Repo');

// THEN
expect(stack).to(haveResource('AWS::ECR::Repository', {
"Type": "AWS::ECR::Repository",
"DeletionPolicy": "Retain"
}, ResourcePart.CompleteDefinition));
test.done();
},

'"Delete" removal policy can be set explicitly'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

// WHEN
new ecr.Repository(stack, 'Repo', {
removalPolicy: RemovalPolicy.Destroy
});

// THEN
expect(stack).to(haveResource('AWS::ECR::Repository', {
"Type": "AWS::ECR::Repository",
"DeletionPolicy": "Delete"
}, ResourcePart.CompleteDefinition));
test.done();
}

};
10 changes: 4 additions & 6 deletions packages/@aws-cdk/aws-kms/lib/key.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import iam = require('@aws-cdk/aws-iam');
import { PolicyDocument, PolicyStatement } from '@aws-cdk/aws-iam';
import { Construct, DeletionPolicy, IResource, Resource, Stack } from '@aws-cdk/cdk';
import { Construct, IResource, RemovalPolicy, Resource, Stack } from '@aws-cdk/cdk';
import { Alias } from './alias';
import { CfnKey } from './kms.generated';

Expand Down Expand Up @@ -177,9 +177,9 @@ export interface KeyProps {
* Whether the encryption key should be retained when it is removed from the Stack. This is useful when one wants to
* retain access to data that was encrypted with a key that is being retired.
*
* @default true
* @default RemovalPolicy.Retain
*/
readonly retain?: boolean;
readonly removalPolicy?: RemovalPolicy;
}

/**
Expand Down Expand Up @@ -225,9 +225,7 @@ export class Key extends KeyBase {
});

this.keyArn = resource.attrArn;
resource.options.deletionPolicy = props.retain === false
? DeletionPolicy.Delete
: DeletionPolicy.Retain;
resource.applyRemovalPolicy(props.removalPolicy);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/// !cdk-integ *
import cdk = require('@aws-cdk/cdk');
import { RemovalPolicy } from '@aws-cdk/cdk';
import kms = require('../lib');

const app = new cdk.App();
Expand All @@ -14,7 +15,7 @@ class KeyStack extends cdk.Stack {

constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
super(scope, id, props);
this.key = new kms.Key(this, 'MyKey', { retain: false });
this.key = new kms.Key(this, 'MyKey', { removalPolicy: RemovalPolicy.Destroy });
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-kms/test/integ.key.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { PolicyStatement } from '@aws-cdk/aws-iam';
import iam = require('@aws-cdk/aws-iam');
import { App, Stack } from '@aws-cdk/cdk';
import { App, RemovalPolicy, Stack } from '@aws-cdk/cdk';
import { Key } from '../lib';

const app = new App();

const stack = new Stack(app, `aws-cdk-kms-1`);

const key = new Key(stack, 'MyKey', { retain: false });
const key = new Key(stack, 'MyKey', { removalPolicy: RemovalPolicy.Destroy });

key.addToResourcePolicy(new PolicyStatement({
resources: ['*'],
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-kms/test/test.key.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { exactlyMatchTemplate, expect, haveResource, ResourcePart } from '@aws-cdk/assert';
import { PolicyStatement, User } from '@aws-cdk/aws-iam';
import { App, Stack, Tag } from '@aws-cdk/cdk';
import { App, RemovalPolicy, Stack, Tag } from '@aws-cdk/cdk';
import { Test } from 'nodeunit';
import { Key } from '../lib';

Expand Down Expand Up @@ -68,7 +68,7 @@ export = {
const app = new App();
const stack = new Stack(app, 'TestStack');

new Key(stack, 'MyKey', { retain: false });
new Key(stack, 'MyKey', { removalPolicy: RemovalPolicy.Destroy });

expect(stack).to(haveResource('AWS::KMS::Key', { DeletionPolicy: "Delete" }, ResourcePart.CompleteDefinition));
test.done();
Expand Down
Loading