feat(batch): ComputeEnvironment implements IConnectable

[tcutts]

closes #20983

ComputeEnvironments have embedded security groups in them. These are currently difficult to reach and modify in CDK stacks, which forces users into undesirable practices when integrating batch queues with other services such as as EFS filesystems or RDS instances. Ideally, it should be possible to use compute environments as a target:

something.connections.allowDefaultPortFrom(computeEnvironment);

but this isn't currently possible, so the user may try to work around it by allowing from any IPv4, or by having to use an escape hatch, which is not simple.

This pull request adds ec2.IConnectable to batch.ComputeEnvironment. It still seems to pass all the existing tests and integration tests, so I don't think it's a breaking change, but I suspect it could be done in a better way than I've done it, to make things even simpler for the user.

---

All Submissions:

• [yes] Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

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

New Features

• [yes] Have you added the new feature to an integration test?
  • [yes] 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 @tcutts and thanks for the contribution! This looks exciting and I think you've found the right approach to implement this. Before I go ahead with a more thorough review, can you please make sure the build passes and with this kind of logic change, we will definitely need unit tests in addition to the integration test you've already added.

[tcutts]

There you go @mrgrain - builds properly now. My example in the README needed fixing... Thanks for the encouraging words.

[TheRealAmazonKendra]

Looks like we still need unit tests on this change.

[mrgrain]

A few minor changes and since we are changing the logical, I'd like to see a unit test.

Pull request has been modified.

[tcutts]

I've added a couple of unit tests, but it's not a particular area of expertise for me (not being an SDE!); I've borrowed some of the connections test logic from aws-ec2, and re-purposed it here to check that the right ingress rules are created, for both fargate and EC2 managed compute environments. Is that sufficient?

[mrgrain]

Yes, these look great! Thank you for your work here.

[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).

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 375d4b7
• 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).