Event Rule for CodePipeline S3 Source Action Missing Event Names

[njlaw]

The current CloudWatch Event created for CodePipeline S3SourceAction only matches on the event name PutObject to trigger the event. However, there are other actions that can update the object in the S3 bucket that should trigger this event. E.g., in the CloudPipeline user guide, the example event rule matches on PutObject, CopyObject, and CompleteMultipartUpload .

As a specific example, I have a CodeBuild project (not in a pipeline) that outputs its artifact to S3. It does this using a multipart upload, so the pipeline that uses this as the source never triggers.

Reproduction Steps

1. Create a pipeline with an S3 source action that uses events as a trigger.
2. Upload the source file using a means other than PutObject.
3. Pipeline will not trigger.

class MyTestStack extends cdk.Stack {
    constructor(parent: cdk.App, name: string) {
        super(parent, name);

        const bucket = new s3.Bucket(this, 'Bucket');
        const pipeline = new codepipeline.Pipeline(this, 'Pipeline');
        const sourceArtifact = new codepipeline.Artifact();
        const s3SourceAction = new codepipelineActions.S3SourceAction({
            actionName: 'SourceFromS3',
            bucket: bucket,
            bucketKey: `source-file.zip`,
            output: sourceArtifact,
            trigger: codepipelineActions.S3Trigger.EVENTS,
        });
        pipeline.addStage({
            stageName: 'Source',
            actions: [ s3SourceAction ]
        });

        const dummyAction = new codepipelineActions.S3DeployAction({
            actionName: 'Dummy',
            bucket: bucket,
            input: sourceArtifact,
            objectKey: 'dummy.zip'
        });
        pipeline.addStage({
            stageName: 'Dummy',
            actions: [ dummyAction ]
        });
    }
}

Error Log

There is no error message. In the CloudWatch CloudTrail logs, you see the CompleteMultipartUpload event, but obviously this does not trigger the Pipeline since the event rule that is generated does not match on it.

{
    "eventVersion": "1.05",
    "userIdentity": {
        ...
    },
    "eventTime": "2019-10-21T18:24:59Z",
    "eventSource": "s3.amazonaws.com",
    "eventName": "CompleteMultipartUpload",
    "awsRegion": "us-east-1",
    "sourceIPAddress": "10.0.38.0",
    "userAgent": "[aws-sdk-go/1.25.8 (go1.12.10; linux; amd64) exec-env/AWS_ECS_EC2 S3Manager]",
    "requestParameters": {
        "bucketName": "XXX",
        "Host": "myteststack-bucketa1a1a1a1-a1a1a1a1a1a1.s3.amazonaws.com",
        "uploadId": "XXX",
        "key": "source-file.zip",
        ...
    },
    ...
    "eventID": "XXX",
    "readOnly": false,
    "resources": [
        {
            "type": "AWS::S3::Object",
            "ARN": "arn:aws:s3:::myteststack-bucketa1a1a1a1-a1a1a1a1a1a1/source-file.zip"
        },
        {
            "accountId": "XXX",
            "type": "AWS::S3::Bucket",
            "ARN": "arn:aws:s3:::myteststack-bucketa1a1a1a1-a1a1a1a1a1a1"
        }
    ],
    "eventType": "AwsApiCall",
    "recipientAccountId": "XXX",
    "vpcEndpointId": "vpce-XXX"
}

Environment

• CLI Version : v1.14.0
• Framework Version: v1.14.0
• OS : Windows 10
• Language : English

Other

I'll submit a pull request for this.

This is 🐛 Bug Report

[skinny85]

Thanks for reporting @njlaw . I think the issue is actually in the S3 library: the onCloudTrailPutObject method should encapsulate all events that mean "put" for a given S3 object.

While we're in the area, we should fix the docs on this method -- they're pretty clearly copy-pasted from ECR's Repository.

Re-assigning to S3 and @eladb .

[njlaw]

Thanks for the follow up @skinny85 . One hiccup came up when I was testing solutions for this. It looks like it won't be as simple as changing the Event Rule to

rule.addEventPattern({
  detail: {
    eventName: [
      'CopyObject',
      'PutObject',
      'CompleteMultipartUpload',
    ],
  },
});

since this ends up matching on copies from the S3 bucket to another bucket as well. At least under some conditions CodePipeline itself uses CopyObject to copy the source from the source bucket to the artifact bucket, resulting in an infinite loop triggering the CodePipeline. I'm playing with rule combinations that may work and will post an update here when I figure something out.

[skinny85]

But 'CopyObject' is given in the official CodePipeline guide. Does that mean following that guide results in an infinite event loop...? Because if it does, we should report that as well.

[njlaw]

I'm wondering if maybe there are only certain conditions under which it results in an infinite loop, but it definitely did when I tested it. It should be easy for me to reproduce it if necessary. I've reported the documentation issue.

[njlaw]

I've confirmed that the following Event Rule works correctly for CopyObject. It triggers only when the object is copied to the bucket/key and not from it.

{
  "source": [
    "aws.s3"
  ],
  "detail-type": [
    "AWS API Call via CloudTrail"
  ],
  "detail": {
    "eventSource": [
      "s3.amazonaws.com"
    ],
    "eventName": [
      "CopyObject"
    ],
    "requestParameters": {
      "bucketName": [
        "<bucketName>"
      ],
      "key": [
        "<key>"
      ]
    }
  }
}

What do you think about changing onCloudTrailPutObject() to onCloudTrailPutObjectOrCompleteMultipartUpload() and including CompleteMultipartUpload then adding a method to Bucket, onCloudTrailCopyObjectTo()? Or if that's too verbose, the actual CloudTrail event names could be replaced: onCloudTrailPutOrUpload() and onCloudTrailCopyObjectTo().

If you think it would be helpful, I'm happy to try putting together a pull request for this; otherwise, I can leave it in yours and @eladb 's hands.

[skinny85]

Thanks for the research @njlaw . But I don't think we need a new method here. In my opinion, onCloudTrailPutObject should encapsulate all details of "I want to be notified when an object is put into a bucket (possibly under a given path)", and it should work for every way that objects can be put into buckets in S3. That's the entire value proposition of the CDK - the fact that we can encapsulate complicated constructs, like the event pattern you've discovered, and put it in a nicely-named method, so any client of that method doesn't need to know all of those details anymore. More pragmatically, I don't want to fix it only for the CodePipeline S3 source action, as everyone using onCloudTrailPutObject will very likely want it to work for all upload methods as well.

@eladb do you agree?

[eladb]

Absolutely!

[eladb]

@skinny85 why did you assign this to me?

[skinny85]

@skinny85 why did you assign this to me?

Because this needs to be changed in the S3 library, like (I thought) we agreed to in this comment?

[joekiller]

@eladb the (cc @skinny85) the s3 bucket lib needs to be updated to accommodate the desire of

"onCloudTrailPutObject should encapsulate all details of "I want to be notified when an object is put into a bucket (possibly under a given path)"

There are two changes that need to happen to make this happen.

1. First the
   s3.onCloudTrailPutObject event needs to be updated to allow all "putting scenarios" to be captured.

2. Second the s3.onCloudTrailEvent needs to listen for the detail.requestParameters instead of detail.resources because resources will trigger events where the key is the source of an action while requestParameters will ensure that the key is always the target of actions so CopyEvents are picked up correctly.

1 Updated onCloudTrailPutObject EventPattern

    rule.addEventPattern({
      detail: {
        eventName: ['PutObject'],
      },

to

    rule.addEventPattern({
      detail: {
        eventName: ['PutObject', 'CopyObject', 'CompleteMultiPartUpload'],
      },

2 Updated onCloudTrailEvent EventPattern

    rule.addEventPattern({
      source: ['aws.s3'],
      detailType: ['AWS API Call via CloudTrail'],
      detail: {
        resources: {
          ARN: options.paths ? options.paths.map(p => this.arnForObjects(p)) : [this.bucketArn],
        },
      }
    });
    return rule;

to... something like

    rule.addEventPattern({
      source: ['aws.s3'],
      detailType: ['AWS API Call via CloudTrail'],
      detail: {
        requestParameters: {
          bucketName: [this.bucketArn],
          key: options.paths
        },
      }
    });
    return rule;

This issue blocked me from using S3Trigger.EVENTS again.

[njlaw]

@joekiller 's post brought something to mind and made me revisit the rules. I had assumed that we would need two separate rules:

1. One for PutObject and CompleteMultipartUpload, specifying only eventName and resources, and
2. One for CopyObject that also specifying requestParameters

However, it appears that the requestParameters should be valid for all three event types, which means that a single rule should work. I'll update my fork and test it to verify it works. Any suggestions for a name for the Bucket.onCloudTrailPutObject method? 'PutObject' implies that it is specific to that event name, but if that's the convention for cdk, I'll leave it and just note in the documentation that it matches on PutObject, CopyObject, and CompleteMultipartUpload.

[skinny85]

Any suggestions for a name for the Bucket.onCloudTrailPutObject method? 'PutObject' implies that it is specific to that event name, but if that's the convention for cdk, I'll leave it and just note in the documentation that it matches on PutObject, CopyObject, and CompleteMultipartUpload.

I think onCloudTrailPutObject is a fine name - at least, I have trouble coming up with a better one (onCloudTrailObjectAdded? Not sure if that's better...). I think making sure to specify in the documentation that it works for all means of placing objects in buckets is the way to go.

[njlaw]

I've gone ahead and created a pull request for this, keeping the method name as onCloudTrailPutObject and adding details in the documentation. There wasn't an existing unit test for this function; and based on what I saw in other packages, an integration test seemed more appropriate since the output affected is really an AWS::Event::Rule and not an S3 resource. If I should create a unit test for onCloudTrailPutObject as well, please let me know.

PS. I've also learned that I should not use the fixes #NNNN syntax in intermediate commits; otherwise, GitHub tags everything! Sorry about the spam.

[Edited to change 'should' to 'should not']

[njlaw]

Sorry, I'm new to this... I missed the unit tests for onCloudTrailPutObject in aws-s3-notifications, so I'm adding units tests for the changes there and had to update the existing tests to not expect just PutObject as well. This also makes the integration test I put in aws-s3 pointless, so I'm removing that.