feat(cloud9-alpha): add support for federated-user and assumed-role for Cloud9 environment ownership

[markusz]

Currently, the cloud9-alpha module only supports two IAM entities as the owners of a Cloud9 environment

• Account Root
• User

However, in many environments, access to an AWS account is gained via Federation. To use Cloud9 via the CDK in such environments, workarounds like the following one where required:

const cloud9 = new Ec2Environment(this, 'Cloud9', {..});

const cfnC9 = cloud9.node.findChild('Resource') as CfnEnvironmentEC2;
cfnC9.ownerArn = 'arn:sts:..;

This merge request adds support for assumed roles and federated users to be owners of C9 environments directly in the CDK construct.

---

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

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[vinayak-kukreja]

Hey, this is great. Thank you for opening a PR with us.

I have some queries. Please let me know if any of it does not make sense.

[vinayak-kukreja]

Why do we need this?

[markusz]

I assume you refer to { force: true }. This is a left-over from debugging the tests, we don't need it. Removed in 170f23f

If you refer to the entire LoC: It's the only way I know how to programmatically synthesize CDK templates

[markusz]

@vinayak-kukreja Thank you for the review.

I incorporated some changes and argued to keep my implementation for others.

Happy to hear your thoughts

[vinayak-kukreja]

Hey, just to confirm, this is not your AWS account id or a valid one?

[mikewrighton]

Seems to be somewhat of a convention to use 123456789012 for tests?

[markusz]

It is an actual accountId and an actual user in that account. If I understood integration tests in the CDK correctly, they synth, deploy and destroy an actual stack in an actual environment to verify correctness.

In my case, I need to refer to existing IAM entities in the account that I run the integration tests in and I don't really see another way to do this here.

I could avoid having to specify the account id by using Stack.of(this).account, but that would require a breaking change to environment.ts:Owner as the functions are static and not members of the Ec2Environment construct. This would also make assumed-roles that are defined in other accounts impossible (although I don't think that IAM allows such entities exist in the first place)

Please advise if you see another way to test this

[vinayak-kukreja]

But these integ tests would fail if the resource is deleted from the account or the permissions no longer allow operations on the account :)
Also, it is not recommended usually to hardcode account ids in code base.

---

What you can do here is,

1. Create a role in the integ test and that should be a dependency for provisioning your C9 Environment. For instance,

constructA.node.addDependency(constructB);

2. Use the environment from the CLI which has the account and the region. For instance,

new MyDevStack(app, 'dev', { 
  env: { 
    account: process.env.CDK_DEFAULT_ACCOUNT, 
    region: process.env.CDK_DEFAULT_REGION 
}});

Let me know if this makes sense.

[vinayak-kukreja]

Also, would accountId ever be different from the user's account id? If not, then that value would not be needed to be passed in the function params

[vinayak-kukreja]

@vinayak-kukreja Thank you for the review.

I incorporated some changes and argued to keep my implementation for others.

Happy to hear your thoughts

Hey, @markusz thank you for addressing feedback. Just want to know about the account id mentioned in the test.

[markusz]

@vinayak-kukreja I read up again on how integration tests work and I think I found a good solution that makes the test self-contained and requires no hardcoded values.

Can you have a look at the changes in c4589cc ?

Thanks

[vinayak-kukreja]

Hey, thank you for making the changes. We will take a look at the PR soon.

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[vinayak-kukreja]

Hey, thank you for your contribution. Apologies for the delay. 🙏

[aws-cdk-automation]

AWS CodeBuild CI Report

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