Also - unit tests are missing 😉.

@skinny85 it's only existing unit tests that are failing now. When I try update them there are some that are deleted snapshot files that are deleted. Is that expected? I've been running full builds and tests

@hross-frae I'm not sure what you're asking, but because you implemented my comment from here: #14381 (comment) , the generated CloudFormation template will remain exactly the same as it is now.

The build currently is failing on the @aws-cdk/aws-events-targets package:

@aws-cdk/aws-events-targets: Verifying codepipeline/integ.pipeline-event-target.js against codepipeline/integ.pipeline-event-target.expected.json ... CHANGED.
@aws-cdk/aws-events-targets: Resources
@aws-cdk/aws-events-targets: [~] AWS::KMS::Key pipelinePipeline22F2A91DArtifactsBucketEncryptionKey87C796D2 
@aws-cdk/aws-events-targets:  └─ [-] EnableKeyRotation
@aws-cdk/aws-events-targets:      └─ false

If you revert the change you made to the integ.pipeline-event-target.expected.json file, the build should pass again (same comment about the other changes to the "expected" files).

please merge, so that i can satisfy my security hub. thank you.

please merge, so that i can satisfy my security hub. thank you.

Can't really merge it if the build is failing, unfortunately.

Would be awesome to merge this PR. Unfortunately, my known workarounds for enabling KeyRotation don't work anymore with the recent v2 RCs.
Do I understand it correctly, that the only missing piece is to "revert the change you made to the integ.pipeline-event-target.expected.json file" @skinny85?
Since @hross-frae doesn't seem to respond anymore, is there any strategy on how to proceed with this PR?

Hi @ppuritscher, the main issue I can up against in the end was getting all the tests working...
As far as I can tell the code changes are all good, they might be a little out of date though. Also there might have to further changes added now due to the new CodePipeline construct

Hi @hross-frae, thanks for the update and your work there!
Are you sure, that there are any further changes required for now, concerning the new CodePipeline construct?

I assume that you already checked the suggestion of @skinny85 ("revert the change you made to the integ.pipeline-event-target.expected.json file")?

Would it be possible for you to give git rebase a try?

@ppuritscher you should be able to take this PR over if you want to (it's just a matter of getting the code from https://github.com/hross-frae/aws-cdk, the master branch, and you can then edit locally, and either push directly to @hross-frae's fork, or open a new PR).

I created a PR hross-frae#1
@hross-frae can you pleas accept the PR? I checked the tests in @aws-cdk/aws-events-targets and reverted one change there. Tests ran green afterwards. I also checked the pipeline tests, but I did not check every single test (I takes too much time to be honest :D ), so lets see if CodeBuild runs green this time.

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 8746ac0
• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Ok, the other tests fail as well.
@skinny85 its not clear to me why the key is not published in the tests? I mean I can remove the added property in the json testfiles, but shouldn't the key be propagated?

Ah ok, I think I found the "problem":

 const resource = new CfnKey(this, 'Resource', {
      description: props.description,
      enableKeyRotation: props.enableKeyRotation,
      enabled: props.enabled,
      keySpec: props.keySpec,
      keyUsage: props.keyUsage,
      keyPolicy: this.policy,
      pendingWindowInDays: pendingWindowInDays,
});

this.keyArn = resource.attrArn;

You build now a keyArn with all necessary information (as far as I understand). That means we cant search for enableKeyRotation in the resulting json.
I'm going to create a new PR and remove all the enableKeyRotation to fix the tests.

Hey @hross-frae, @renner-daniel and @skinny85!
I just created a new PR as successor. The tests run through.

Hey @hross-frae, @renner-daniel and @skinny85!
I just created a new PR as successor. The tests run through.

Thanks @ppuritscher for picking this up and following it through. I'll close this PR as it's no longer needed