fix(ecs): iam policies are missing for firelens cloudwatch_logs log driver

[rhermes62]

We recently migrated our FireLens log driver from using cloudwatch to using the recommended cloudwatch_logs.

cloudwatch_logs is the newer and more high performance version as detailed in this guidance from AWS https://github.com/aws/amazon-cloudwatch-logs-for-fluent-bit#new-higher-performance-core-fluent-bit-plugin

However, when doing that, we lost the automatic configuration of the IAM policies to write to CloudWatch which broke our logging.

Looks like the CDK only auto adds CloudWatch IAM policies only for the cloudwatch plugin opposed to both the older cloudwatch plugin and newer and recommended cloudwatch_logs plugin.

Policies should be added to both so that customer's logging doesn't break when migrating to the newer plugin.

---

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[bot]]

[rhermes62]

Works with me! None of these plugins have integ coverage AFAIK.

I will add for the cloudwatch and cloudwatch_logs plugins since those are the primary focus of the PR.

Should I also add them for the firehose and kinesis options which are also missing integ coverage? Or is that scope creep/more suitable for a separate chore PR?

[TheRealAmazonKendra]

Works with me! None of these plugins have integ coverage AFAIK.

I will add for the cloudwatch and cloudwatch_logs plugins since those are the primary focus of the PR.

Should I also add them for the firehose and kinesis options which are also missing integ coverage? Or is that scope creep/more suitable for a separate chore PR?

I will never turn down extra test coverage! However, let's do the extra ones in a separate PR so we can get this one merged faster.

[TheRealAmazonKendra]

@rhermes62 Just to clarify, I'd like the tests for cloudwatch and cloudwatch_logs added to this PR and the others in a separate PR. After reading my own response, I wasn't sure if it sounded like I wanted them all in a separate PR or not.

[rhermes62]

Makes sense and will do! Will try and squeeze in those extra tests tonight or tomorrow

[rhermes62]

Still working on getting those cleaned up. Builds in flight likely to fail.

These first integ tests for aws-cdk written so learning how this repo does them. I had historically always just used jest's snapshotting feature + the aws-cdk-lib/assertions's Template feature to write snapshot tests like expect(Template.fromStack(stack).toJSON()).toMatchSnapshot();.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 1c85c546497fdde555047d9f2e95ddedf904855d
• Result: FAILED
• Build Logs (available for 30 days)

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

[TheRealAmazonKendra]

Still working on getting those cleaned up. Builds in flight likely to fail.

These first integ tests for aws-cdk written so learning how this repo does them. I had historically always just used jest's snapshotting feature + the aws-cdk-lib/assertions's Template feature to write snapshot tests like expect(Template.fromStack(stack).toJSON()).toMatchSnapshot();.

No worries! If you need any help troubleshooting, please let me know. I'm happy to assist.

[TheRealAmazonKendra]

@rhermes62 Actually, I just took a look and I can make myself useful here immediately. Just run yarn integ --update-on-failed integ.firelens-log-driver.js and it will update your test folder to the correct name. I ran it locally and the new test succeeds so I don't think you should have any issue on your end.

[rhermes62]

Sounds good! Thank you! We normally don't have direct access/credentials to do things like cdk deploy to AWS accounts since we have strict release processes/security so I was trying to find one to deploy to😂😅

I was about to create a temp account just to do test deploy to in order to generate the artifact files to get the integ to pass.

[TheRealAmazonKendra]

Sounds good! Thank you! We normally don't have direct access/credentials to do things like cdk deploy to AWS accounts since we have strict release processes/security so I was trying to find one to deploy to😂😅

I was about to create a temp account just to do test deploy to in order to generate the artifact files to get the integ to pass.

The test will need to deploy when you do it that way. It actually runs the test with deployment and deletion of resources. If you have issues doing that. I can upload the output for you. Just let me know if that's needed.

[aws-cdk-automation]

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.