Skip to content

Commit

Permalink
fix(pipelines): pipeline-security integration test was broken
Browse files Browse the repository at this point in the history
  • Loading branch information
mrgrain committed Aug 5, 2022
1 parent 018afa0 commit 492c6c3
Show file tree
Hide file tree
Showing 12 changed files with 186 additions and 51 deletions.
1 change: 1 addition & 0 deletions packages/@aws-cdk/pipelines/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
"@aws-cdk/aws-sqs": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2",
Expand Down
14 changes: 12 additions & 2 deletions packages/@aws-cdk/pipelines/test/integ.pipeline-security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import * as sns from '@aws-cdk/aws-sns';
import * as subscriptions from '@aws-cdk/aws-sns-subscriptions';
import { App, RemovalPolicy, Stack, StackProps, Stage, StageProps } from '@aws-cdk/core';
import { App, DefaultStackSynthesizer, RemovalPolicy, Stack, StackProps, Stage, StageProps } from '@aws-cdk/core';
import * as integ from '@aws-cdk/integ-tests';
import { Construct } from 'constructs';
import * as cdkp from '../lib';

class MyStage extends Stage {
constructor(scope: Construct, id: string, props?: StageProps) {
super(scope, id, props);
const stack = new Stack(this, 'MyStack', {
synthesizer: new DefaultStackSynthesizer(),
});
const topic = new sns.Topic(stack, 'Topic');
topic.grantPublish(new iam.AccountPrincipal(stack.account));
Expand All @@ -23,6 +25,7 @@ class MySafeStage extends Stage {
constructor(scope: Construct, id: string, props?: StageProps) {
super(scope, id, props);
const stack = new Stack(this, 'MySafeStack', {
synthesizer: new DefaultStackSynthesizer(),
});
new sns.Topic(stack, 'MySafeTopic');
}
Expand Down Expand Up @@ -98,5 +101,12 @@ const app = new App({
'@aws-cdk/core:newStyleStackSynthesis': 'true',
},
});
new TestCdkStack(app, 'PipelineSecurityStack');
const stack = new TestCdkStack(app, 'PipelineSecurityStack', {
synthesizer: new DefaultStackSynthesizer(),
});

new integ.IntegTest(app, 'PipelineSecurityTest', {
testCases: [stack],
});

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@
}
}
},
"ff8909e2b3e01298b53c87d97e8e745b4f0b2e4b6d29d5680c44e5da87a207a4": {
"db81913e08aad04a7b47fcf422f74cb3e791e1d9aba3a1d6f6c6b0b8b40b8f34": {
"source": {
"path": "PipelineSecurityStack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "ff8909e2b3e01298b53c87d97e8e745b4f0b2e4b6d29d5680c44e5da87a207a4.json",
"objectKey": "db81913e08aad04a7b47fcf422f74cb3e791e1d9aba3a1d6f6c6b0b8b40b8f34.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 @@ -2623,7 +2623,7 @@
{
"Ref": "TestPipelinePipelineApplicationSecurityCheckCDKPipelinesAutoApprove1EE0AA81"
},
" --invocation-type Event --payload \\\"$payload\\\" lambda.out; export MESSAGE=\\\"No security-impacting changes detected.\\\"; else [ -z \\\"${NOTIFICATION_ARN}\\\" ] || aws sns publish --topic-arn $NOTIFICATION_ARN --subject \\\"$NOTIFICATION_SUBJECT\\\" --message \\\"An upcoming change would broaden security changes in $PIPELINE_NAME.\\nReview and approve the changes in CodePipeline to proceed with the deployment.\\n\\nReview the changes in CodeBuild:\\n\\n$LINK\\n\\nApprove the changes in CodePipeline (stage $STAGE_NAME, action $ACTION_NAME):\\n\\n$PIPELINE_LINK\\\"; export MESSAGE=\\\"Deployment would make security-impacting changes. Click the link below to inspect them, then click Approve if all changes are expected.\\\"; fi\"\n ]\n }\n },\n \"env\": {\n \"exported-variables\": [\n \"LINK\",\n \"MESSAGE\"\n ]\n }\n}"
" --invocation-type Event --cli-binary-format raw-in-base64-out --payload \\\"$payload\\\" lambda.out; export MESSAGE=\\\"No security-impacting changes detected.\\\"; else [ -z \\\"${NOTIFICATION_ARN}\\\" ] || aws sns publish --topic-arn $NOTIFICATION_ARN --subject \\\"$NOTIFICATION_SUBJECT\\\" --message \\\"An upcoming change would broaden security changes in $PIPELINE_NAME.\\nReview and approve the changes in CodePipeline to proceed with the deployment.\\n\\nReview the changes in CodeBuild:\\n\\n$LINK\\n\\nApprove the changes in CodePipeline (stage $STAGE_NAME, action $ACTION_NAME):\\n\\n$PIPELINE_LINK\\\"; export MESSAGE=\\\"Deployment would make security-impacting changes. Click the link below to inspect them, then click Approve if all changes are expected.\\\"; fi\"\n ]\n }\n },\n \"env\": {\n \"exported-variables\": [\n \"LINK\",\n \"MESSAGE\"\n ]\n }\n}"
]
]
},
Expand Down Expand Up @@ -2967,7 +2967,7 @@
{
"Ref": "UnattachedStageStageApplicationSecurityCheckCDKPipelinesAutoApprove249F82F9"
},
" --invocation-type Event --payload \\\"$payload\\\" lambda.out; export MESSAGE=\\\"No security-impacting changes detected.\\\"; else [ -z \\\"${NOTIFICATION_ARN}\\\" ] || aws sns publish --topic-arn $NOTIFICATION_ARN --subject \\\"$NOTIFICATION_SUBJECT\\\" --message \\\"An upcoming change would broaden security changes in $PIPELINE_NAME.\\nReview and approve the changes in CodePipeline to proceed with the deployment.\\n\\nReview the changes in CodeBuild:\\n\\n$LINK\\n\\nApprove the changes in CodePipeline (stage $STAGE_NAME, action $ACTION_NAME):\\n\\n$PIPELINE_LINK\\\"; export MESSAGE=\\\"Deployment would make security-impacting changes. Click the link below to inspect them, then click Approve if all changes are expected.\\\"; fi\"\n ]\n }\n },\n \"env\": {\n \"exported-variables\": [\n \"LINK\",\n \"MESSAGE\"\n ]\n }\n}"
" --invocation-type Event --cli-binary-format raw-in-base64-out --payload \\\"$payload\\\" lambda.out; export MESSAGE=\\\"No security-impacting changes detected.\\\"; else [ -z \\\"${NOTIFICATION_ARN}\\\" ] || aws sns publish --topic-arn $NOTIFICATION_ARN --subject \\\"$NOTIFICATION_SUBJECT\\\" --message \\\"An upcoming change would broaden security changes in $PIPELINE_NAME.\\nReview and approve the changes in CodePipeline to proceed with the deployment.\\n\\nReview the changes in CodeBuild:\\n\\n$LINK\\n\\nApprove the changes in CodePipeline (stage $STAGE_NAME, action $ACTION_NAME):\\n\\n$PIPELINE_LINK\\\"; export MESSAGE=\\\"Deployment would make security-impacting changes. Click the link below to inspect them, then click Approve if all changes are expected.\\\"; fi\"\n ]\n }\n },\n \"env\": {\n \"exported-variables\": [\n \"LINK\",\n \"MESSAGE\"\n ]\n }\n}"
]
]
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export declare function handler(event: AWSLambda.CloudFormationCustomResourceEvent): Promise<void>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import { S3 } from 'aws-sdk';

const AUTO_DELETE_OBJECTS_TAG = 'aws-cdk:auto-delete-objects';

const s3 = new S3();

export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
switch (event.RequestType) {
case 'Create':
return;
case 'Update':
return onUpdate(event);
case 'Delete':
return onDelete(event.ResourceProperties?.BucketName);
}
}

async function onUpdate(event: AWSLambda.CloudFormationCustomResourceEvent) {
const updateEvent = event as AWSLambda.CloudFormationCustomResourceUpdateEvent;
const oldBucketName = updateEvent.OldResourceProperties?.BucketName;
const newBucketName = updateEvent.ResourceProperties?.BucketName;
const bucketNameHasChanged = newBucketName != null && oldBucketName != null && newBucketName !== oldBucketName;

/* If the name of the bucket has changed, CloudFormation will try to delete the bucket
and create a new one with the new name. So we have to delete the contents of the
bucket so that this operation does not fail. */
if (bucketNameHasChanged) {
return onDelete(oldBucketName);
}
}

/**
* Recursively delete all items in the bucket
*
* @param bucketName the bucket name
*/
async function emptyBucket(bucketName: string) {
const listedObjects = await s3.listObjectVersions({ Bucket: bucketName }).promise();
const contents = [...listedObjects.Versions ?? [], ...listedObjects.DeleteMarkers ?? []];
if (contents.length === 0) {
return;
}

const records = contents.map((record: any) => ({ Key: record.Key, VersionId: record.VersionId }));
await s3.deleteObjects({ Bucket: bucketName, Delete: { Objects: records } }).promise();

if (listedObjects?.IsTruncated) {
await emptyBucket(bucketName);
}
}

async function onDelete(bucketName?: string) {
if (!bucketName) {
throw new Error('No BucketName was provided.');
}
if (!await isBucketTaggedForDeletion(bucketName)) {
process.stdout.write(`Bucket does not have '${AUTO_DELETE_OBJECTS_TAG}' tag, skipping cleaning.\n`);
return;
}
try {
await emptyBucket(bucketName);
} catch (e) {
if (e.code !== 'NoSuchBucket') {
throw e;
}
// Bucket doesn't exist. Ignoring
}
}

/**
* The bucket will only be tagged for deletion if it's being deleted in the same
* deployment as this Custom Resource.
*
* If the Custom Resource is every deleted before the bucket, it must be because
* `autoDeleteObjects` has been switched to false, in which case the tag would have
* been removed before we get to this Delete event.
*/
async function isBucketTaggedForDeletion(bucketName: string) {
const response = await s3.getBucketTagging({ Bucket: bucketName }).promise();
return response.TagSet.some(tag => tag.Key === AUTO_DELETE_OBJECTS_TAG && tag.Value === 'true');
}
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
{
"version": "20.0.0",
"testCases": {
"integ.pipeline-security": {
"PipelineSecurityTest/DefaultTest": {
"stacks": [
"PipelineSecurityStack"
],
"diffAssets": false,
"stackUpdateWorkflow": true
"assertionStack": "PipelineSecurityTestDefaultTestDeployAssertEE246BCA"
}
},
"synthContext": {
"@aws-cdk/core:newStyleStackSynthesis": "true"
},
"enableLookups": false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,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}/ff8909e2b3e01298b53c87d97e8e745b4f0b2e4b6d29d5680c44e5da87a207a4.json",
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/db81913e08aad04a7b47fcf422f74cb3e791e1d9aba3a1d6f6c6b0b8b40b8f34.json",
"requiresBootstrapStackVersion": 6,
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
"additionalDependencies": [
Expand Down Expand Up @@ -365,6 +365,15 @@
]
},
"displayName": "PipelineSecurityStack"
},
"PipelineSecurityTestDefaultTestDeployAssertEE246BCA": {
"type": "aws:cloudformation:stack",
"environment": "aws://unknown-account/unknown-region",
"properties": {
"templateFile": "PipelineSecurityTestDefaultTestDeployAssertEE246BCA.template.json",
"validateOnSynth": false
},
"displayName": "PipelineSecurityTest/DefaultTest/DeployAssert"
}
}
}
Loading

0 comments on commit 492c6c3

Please sign in to comment.