Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@aws-cdk/aws-cloud9-alpha: Missing owner- property #22474

Labels
@aws-cdk/aws-cloud9 Related to AWS Cloud9 bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@jolo-dev
Copy link
Contributor

Describe the bug

The @aws-cdk/aws-cloud9-alpha is missing the owner- property but the Cloudformation has it.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cloud9-environmentec2.html#cfn-cloud9-environmentec2-ownerarn

Expected Behavior

Can add owner- property.

Current Behavior

owner is missing

Reproduction Steps

import * as cloud9 from '@aws-cdk/aws-cloud9-alpha';

const c9env = new cloud9.Ec2Environment(this, 'Cloud9Env', {
      vpc,
      imageId: cloud9.ImageId.AMAZON_LINUX_2,
      instanceType: new ec2.InstanceType('t3.large'),
      clonedRepositories: [
        cloud9.CloneRepository.fromCodeCommit(repo, '/src'),
      ],
     owner: 'arn::*' // error
 });

Possible Solution

Add all the Props from CfnEnvironmentEce the cloud9.Ec2Environment.

https:/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-cloud9/lib/environment.ts#L182

Additional Information/Context

No response

CDK CLI Version

2.45.0

Framework Version

No response

Node.js Version

16.13.1

OS

MacOS

Language

Typescript

Language Version

4.8.4

Other information

No response

@jolo-dev jolo-dev added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 12, 2022
@github-actions github-actions bot added the @aws-cdk/aws-cloud9 Related to AWS Cloud9 label Oct 12, 2022
@corymhall
Copy link
Contributor

Yes, we should support that property.

This issue has been classified as p2. That means a workaround is available or it is deemed a nice-to-have feature. Given the amount of work there is to do and the relative priority of this issue, the CDK team is unlikely to address it. That does not mean the issue will never be fixed! If someone from the community submits a PR to fix this issue, and the PR is small and straightforward enough, and meets the quality bars to be reviewed and merged with little effort we will accept that PR. PRs that do not build or need complex or multiple rounds of reviews are unlikely to be merged and will be closed to keep our backlog manageable.

In the mean time, remember that you can always use the escape hatch (https://docs.aws.amazon.com/cdk/v2/guide/cfn_layer.html) mechanism to have fine control over the CloudFormation output you want. We will keep the issue open for discoverability, to collect upvotes, and to facilitate discussion around this topic.

We use +1s on this issue to help prioritize our work, and are happy to re-evaluate the prioritization of this issue based on community feedback. You can reach out to the cdk.dev community on Slack (https://cdk.dev/) to solicit support for reprioritization.

@corymhall corymhall added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 13, 2022
@corymhall corymhall removed their assignment Oct 13, 2022
@jolo-dev
Copy link
Contributor Author

Hey @corymhall,
Thanks for your message.
I am happy to pick up this issue and will contribute to it.

@mergify mergify bot closed this as completed in #23878 Feb 8, 2023
mergify bot pushed a commit that referenced this issue Feb 8, 2023
Closes #22474

----


### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [x] This PR adds new construct runtime dependencies following the process described [here](https:/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features
1. Setting environment owner .
2. The 'owner' now could be an IAMuser or Account root user(It allows AWS to determine who has permissions to manage the environment, either an IAM user or the account root user)

* [ ] Have you added the new feature to an [integration test](https:/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?
	* [x] Unit test for ownerarn

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Feb 8, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment