fix(pipelines): 'ConfirmPermissionsBroadening' incorrectly invokes lambda for AWS CLI v2

[trevorharwell]

Whenever our pipelines use the construct we get the following error:

An error occurred (InvalidRequestContentException) when calling the Invoke operation: Could not parse request body into json: Could not parse payload into json: Unexpected character ('>' (code 62)): expected a valid value (JSON String, Number, Array, Object or token 'null', 'true' or 'false')

Turns out for AWS CLI v2 you need to specify a flag to send in raw text input.

Documentation here
https://docs.aws.amazon.com/lambda/latest/dg/invocation-async.html

Explained here
https://medium.com/cloud-recipes/use-cli-binary-format-flag-with-aws-cli-version-2-34d590479280

All Submissions:

• Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• This PR adds new unconventional dependencies following the process described here

New Features

• Have you added the new feature to an integration test?
  • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[mrgrain]

Hi @trevorharwell Could you elaborate on the issue and your motivation to fix? Specifically I'm wondering about the difference between AWS CLI v1 vs v2. Is v2 the default on these pipeline or something you added in yourself?

[trevorharwell]

@mrgrain My primary motivation is that this construct has been broken for my organization since May 2022. Every build gets the error I describe above. To my knowledge, I am not (nor am I able to without some seriously messy escape hatches) making any changes to the codebuild machine. I believe aws/codebuild/standard:5.0 comes standard with AWS CLI v2. As such, the invoke lambda command is not correct per the AWS CLI v2 documentation.

Let me know if I am missing something.

Also, struggling to run the integration tests. Keep getting an empty zip file error which prevents deployment.

[trevorharwell]

@mrgrain Confirmed that aws/codebuild/standard:5.0 uses AWS CLI v2

https://github.com/aws/aws-codebuild-docker-images/blob/master/ubuntu/standard/5.0/Dockerfile#L126

[mdownhower]

@mrgrain, I am one of Trevor's co-workers, this error only occurs when there are no changes to the permissions. When there are changes to permissions we do not get this error and the output does display the differences. Unfortunately this has created a manual approval step regardless of whether permissions have broadened or not.

[mrgrain]

Thanks @trevorharwell That's already helpful. Do you know what might have changed in May 2022 for it to suddenly break?

[mdownhower]

[> Thanks @trevorharwell That's already helpful. Do you know what might have changed in May 2022 for it to suddenly break?

If I remember correctly we began noticing this error after #20739 ](#20739)

[trevorharwell]

@mdownhower Correct. It was then subsequently fixed by #20861

Which is what I think might have caused the codebuild box to start using AWS CLI v2.

[mrgrain]

I'll need some time to investigate this a bit further. My concern is that ApplicationSecurityCheck is agnostic to the build image, i.e. it can be run with any build image and while we are now defaulting to an image with AWS CLI v2, not everyone will.

I definitely would like to see integration tests for this, so we don't run into the same issue again.

[TheRealAmazonKendra]

+1 on everything @mrgrain said. We definitely need thorough testing for this. It seems that this change breaks the build so I'm concerned that this wasn't actually tested or built at all before this PR was submitted. It couldn't have passed locally.

[mrgrain]

Confirmed. This change updated the image. The modern image includes AWS CLI v2.

Pull request has been modified.

[trevorharwell]

@mrgrain I'm happy to work on integration tests. As I stated above, I could never actually run the integration tests locally because I get the following error on deployment:

Failed resources:
PipelineSecurityStack | 10:43:59 AM | CREATE_FAILED        | AWS::Lambda::Function       | Custom::S3AutoDeleteObjectsCustomResourceProvider/Handler (CustomS3AutoDeleteObjectsCustomResourceProviderHandler9D90184F) Resource handler returned message: "Uploaded file must be a non-empty zip (Service: Lambda, Status Code: 400, Request ID: 327c83c1-c094-416c-8497-889ae48b3f22)" (RequestToken: 21970a97-cb76-2c13-dce3-cd3245c16f84, HandlerErrorCode: InvalidRequest)

This is the command I ran: yarn integ test/integ.pipeline-security.js --profiles sandbox --update-on-failed --parallel-regions us-east-1

[mrgrain]

@trevorharwell Just letting you know it fails for me as well. I'm looking into it.

[mrgrain]

@trevorharwell This commit fixes the broken test and updates the snapshot for your change: mrgrain@492c6c3
Feel free to rebase/cherry-pick/whatever or let me know if you'd like me to push this to your branch.

Furhtermore @TheRealAmazonKendra and I have discussed this and unfortunately we don't really have the means at the moment to test this in an automated way. So with my change this is as good as it gets.

Thanks again for this contribution. I'm sorry this has been broken for you for such a long time. 😬

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: b0465e3
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).