Skip to content

Commit

Permalink
Merge branch 'master' into cfnspec-docs/d20220413
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Apr 13, 2022
2 parents 490a5c9 + ef2b480 commit a7c9879
Show file tree
Hide file tree
Showing 13 changed files with 197 additions and 4,662 deletions.
43 changes: 35 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,23 +172,47 @@ eval $(gp env -e)

## Pull Requests

Below is a flow chart that describes how your PR may be treated by repository maintainers:

```mermaid
graph TD
A[Incoming PR] -->B[Is an issue attached?]
B -->|Yes - labels copied from issue| C[Is it labeled P1?]
B -->|No - auto-labeled as P2| D["Is it trivial or effort/small?"]
C -->|Yes - P1| E[Is the PR build succeeding?]
C -->|No - it is P2| D
D -->|Yes| E
D -->|No| F[Unfortunately, we don't have the time to review <br/> PRs that are large effort and low priority.]
E -->|Yes| G[We will review your PR as soon as we can]
E -->|No| H[If the build is failing for more than 4 weeks <br/> without any work on it, we will close the PR.]
```

Note that, if we do not have time to review your PR, it is not the end of the road. We are asking
for more community support on the attached issue before we focus our attention there. Any `P2` issue
with 20 or more +1s will be automatically upgraded from `P2`to `P1`.

### Step 1: Find something to work on

If you want to contribute a specific feature or fix you have in mind, look at active [pull
requests](https://github.com/aws/aws-cdk/pulls) to see if someone else is already working on it. If not, you can start
contributing your changes.
If you want to contribute a specific feature or fix you have in mind, look to see if an issue
already exists in our [backlog](https://github.com/aws/aws-cdk/issues). If not, please contribute
a feature request or bug report prior to contributing the PR. We will triage this issue promptly,
and the priority of the issue (`P1` or `P2`) will give indication of how much attention your PR
may get.

It's not required to submit an issue first, but PRs that come in without attached issues will be
automatically labeled as `P2`.

On the other hand, if you are here looking for an issue to work on, explore our [backlog of
issues](https://github.com/aws/aws-cdk/issues) and find something that piques your interest. We have labeled all of our
issues for easy searching.
If you are looking for your first contribution, the ['good first issue'
label](https://github.com/aws/aws-cdk/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) will be of help.
issues](https://github.com/aws/aws-cdk/issues) and find something that piques your interest.
We have labeled all of our issues for easy searching. If you are looking for your first contribution,
the ['good first issue' label](https://github.com/aws/aws-cdk/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22)
will be of help.

It's a good idea to keep the priority of issues in mind when deciding what to
work on. If we have labelled an issue as `P2`, it means it's something we won't
get to soon, and we're waiting on more feedback from the community (in the form
of +1s and comments) to give it a higher priority. A PR for a `P2` issue may
take us some time to review, especially if it involves a complex
be closed by a maintainer, especially if it involves a complex
implementation. `P1` issues impact a significant number of customers, so we are
much more likely to give a PR for those issues prompt attention.

Expand Down Expand Up @@ -325,6 +349,9 @@ $ yarn watch & # runs in the background

* Once the pull request is submitted, a reviewer will be assigned by the maintainers.

* If the PR build is failing, update the PR with fixes until the build succeeds. You may have trouble getting attention
from maintainers if your build is failing, and after 4 weeks of staleness, your PR will be automatically closed.

* Discuss review comments and iterate until you get at least one "Approve". When iterating, push new commits to the
same branch. Usually all these are going to be squashed when you merge to master. The commit messages should be hints
for you when you finalize your merge commit message.
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ export class Group extends GroupBase {
region: '', // IAM is global in each partition
service: 'iam',
resource: 'group',
resourceName: this.physicalName,
// Removes leading slash from path
resourceName: `${props.path ? props.path.substr(props.path.charAt(0) === '/' ? 1 : 0) : ''}${this.physicalName}`,
});
}

Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ export class Role extends Resource implements IRole {
region: '', // IAM is global in each partition
service: 'iam',
resource: 'role',
resourceName: this.physicalName,
// Removes leading slash from path
resourceName: `${props.path ? props.path.substr(props.path.charAt(0) === '/' ? 1 : 0) : ''}${this.physicalName}`,
});
this.roleName = this.getResourceNameAttribute(role.ref);
this.policyFragment = new ArnPrincipal(this.roleArn).policyFragment;
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ export class User extends Resource implements IIdentity, IUser {
region: '', // IAM is global in each partition
service: 'iam',
resource: 'user',
resourceName: this.physicalName,
// Removes leading slash from path
resourceName: `${props.path ? props.path.substr(props.path.charAt(0) === '/' ? 1 : 0) : ''}${this.physicalName}`,
});

this.policyFragment = new ArnPrincipal(this.userArn).policyFragment;
Expand Down
31 changes: 30 additions & 1 deletion packages/@aws-cdk/aws-iam/test/group.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Template } from '@aws-cdk/assertions';
import { App, Stack } from '@aws-cdk/core';
import { App, CfnResource, Stack } from '@aws-cdk/core';
import { Group, ManagedPolicy, User } from '../lib';

describe('IAM groups', () => {
Expand Down Expand Up @@ -74,3 +74,32 @@ describe('IAM groups', () => {
});
});
});

test('cross-env group ARNs include path', () => {
const app = new App();
const groupStack = new Stack(app, 'group-stack', { env: { account: '123456789012', region: 'us-east-1' } });
const referencerStack = new Stack(app, 'referencer-stack', { env: { region: 'us-east-2' } });
const group = new Group(groupStack, 'Group', {
path: '/sample/path/',
groupName: 'sample-name',
});
new CfnResource(referencerStack, 'Referencer', {
type: 'Custom::GroupReferencer',
properties: { GroupArn: group.groupArn },
});

Template.fromStack(referencerStack).hasResourceProperties('Custom::GroupReferencer', {
GroupArn: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::123456789012:group/sample/path/sample-name',
],
],
},
});
});
34 changes: 32 additions & 2 deletions packages/@aws-cdk/aws-iam/test/role.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Template } from '@aws-cdk/assertions';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
import { Duration, Stack, App } from '@aws-cdk/core';
import { Duration, Stack, App, CfnResource } from '@aws-cdk/core';
import { AnyPrincipal, ArnPrincipal, CompositePrincipal, FederatedPrincipal, ManagedPolicy, PolicyStatement, Role, ServicePrincipal, User, Policy, PolicyDocument } from '../lib';

describe('IAM role', () => {
Expand Down Expand Up @@ -569,4 +569,34 @@ test('managed policy ARNs are deduplicated', () => {
},
],
});
});
});

test('cross-env role ARNs include path', () => {
const app = new App();
const roleStack = new Stack(app, 'role-stack', { env: { account: '123456789012', region: 'us-east-1' } });
const referencerStack = new Stack(app, 'referencer-stack', { env: { region: 'us-east-2' } });
const role = new Role(roleStack, 'Role', {
assumedBy: new ServicePrincipal('sns.amazonaws.com'),
path: '/sample/path/',
roleName: 'sample-name',
});
new CfnResource(referencerStack, 'Referencer', {
type: 'Custom::RoleReferencer',
properties: { RoleArn: role.roleArn },
});

Template.fromStack(referencerStack).hasResourceProperties('Custom::RoleReferencer', {
RoleArn: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::123456789012:role/sample/path/sample-name',
],
],
},
});
});
31 changes: 30 additions & 1 deletion packages/@aws-cdk/aws-iam/test/user.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Template } from '@aws-cdk/assertions';
import { App, SecretValue, Stack, Token } from '@aws-cdk/core';
import { App, CfnResource, SecretValue, Stack, Token } from '@aws-cdk/core';
import { Group, ManagedPolicy, Policy, PolicyStatement, User } from '../lib';

describe('IAM user', () => {
Expand Down Expand Up @@ -289,3 +289,32 @@ describe('IAM user', () => {
});
});
});

test('cross-env user ARNs include path', () => {
const app = new App();
const userStack = new Stack(app, 'user-stack', { env: { account: '123456789012', region: 'us-east-1' } });
const referencerStack = new Stack(app, 'referencer-stack', { env: { region: 'us-east-2' } });
const user = new User(userStack, 'User', {
path: '/sample/path/',
userName: 'sample-name',
});
new CfnResource(referencerStack, 'Referencer', {
type: 'Custom::UserReferencer',
properties: { UserArn: user.userArn },
});

Template.fromStack(referencerStack).hasResourceProperties('Custom::UserReferencer', {
UserArn: {
'Fn::Join': [
'',
[
'arn:',
{
Ref: 'AWS::Partition',
},
':iam::123456789012:user/sample/path/sample-name',
],
],
},
});
});
Loading

0 comments on commit a7c9879

Please sign in to comment.