Skip to content

Commit

Permalink
Merge branch 'main' into mergify/bp/main/pr-20016
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Jun 30, 2022
2 parents 6689e68 + e0d375b commit 7a7ca65
Show file tree
Hide file tree
Showing 32 changed files with 320 additions and 646 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export interface EcrSourceVariables {
export interface EcrSourceActionProps extends codepipeline.CommonAwsActionProps {
/**
* The image tag that will be checked for changes.
* Provide an empty string to trigger on changes to any tag.
*
* It is not possible to trigger on changes to more than one tag.
*
* @default 'latest'
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,9 @@
},
"excludeCharacters": "\"@/"
}
}
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
}
},
"Mappings": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"17.0.0"}
{"version":"20.0.0"}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"version": "18.0.0",
"version": "20.0.0",
"testCases": {
"aws-docdb/test/integ.cluster-rotation.lit": {
"integ.cluster-rotation.lit": {
"stacks": [
"aws-cdk-docdb-cluster-rotation"
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "17.0.0",
"version": "20.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"id": "Tree",
"path": "Tree",
"constructInfo": {
"fqn": "@aws-cdk/core.Construct",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.1.33"
}
},
"aws-cdk-docdb-cluster-rotation": {
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/policy-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export class PolicyDocument implements cdk.IResolvable {
}

public resolve(context: cdk.IResolveContext): any {
this.freezeStatements();
this._maybeMergeStatements(context.scope);

// In the previous implementation of 'merge', sorting of actions/resources on
Expand Down Expand Up @@ -212,6 +213,7 @@ export class PolicyDocument implements cdk.IResolvable {
const newDocs: PolicyDocument[] = [];

// Maps final statements to original statements
this.freezeStatements();
let statementsToOriginals = new Map(this.statements.map(s => [s, [s]]));

// We always run 'mergeStatements' to minimize the policy before splitting.
Expand Down Expand Up @@ -298,4 +300,13 @@ export class PolicyDocument implements cdk.IResolvable {
private shouldMerge(scope: IConstruct) {
return this.minimize ?? cdk.FeatureFlags.of(scope).isEnabled(cxapi.IAM_MINIMIZE_POLICIES) ?? false;
}

/**
* Freeze all statements
*/
private freezeStatements() {
for (const statement of this.statements) {
statement.freeze();
}
}
}
86 changes: 74 additions & 12 deletions packages/@aws-cdk/aws-iam/lib/policy-statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,28 +69,21 @@ export class PolicyStatement {
return ret;
}

/**
* Statement ID for this statement
*/
public sid?: string;

/**
* Whether to allow or deny the actions in this statement
*/
public effect: Effect;

private readonly _action = new Array<string>();
private readonly _notAction = new Array<string>();
private readonly _principal: { [key: string]: any[] } = {};
private readonly _notPrincipal: { [key: string]: any[] } = {};
private readonly _resource = new Array<string>();
private readonly _notResource = new Array<string>();
private readonly _condition: { [key: string]: any } = { };
private _sid?: string;
private _effect: Effect;
private principalConditionsJson?: string;

// Hold on to those principals
private readonly _principals = new Array<IPrincipal>();
private readonly _notPrincipals = new Array<IPrincipal>();
private _frozen = false;

constructor(props: PolicyStatementProps = {}) {
// Validate actions
Expand All @@ -101,8 +94,8 @@ export class PolicyStatement {
}
}

this.sid = props.sid;
this.effect = props.effect || Effect.ALLOW;
this._sid = props.sid;
this._effect = props.effect || Effect.ALLOW;

this.addActions(...props.actions || []);
this.addNotActions(...props.notActions || []);
Expand All @@ -115,6 +108,36 @@ export class PolicyStatement {
}
}

/**
* Statement ID for this statement
*/
public get sid(): string | undefined {
return this._sid;
}

/**
* Set Statement ID for this statement
*/
public set sid(sid: string | undefined) {
this.assertNotFrozen('sid');
this._sid = sid;
}

/**
* Whether to allow or deny the actions in this statement
*/
public get effect(): Effect {
return this._effect;
}

/**
* Set effect for this statement
*/
public set effect(effect: Effect) {
this.assertNotFrozen('effect');
this._effect = effect;
}

//
// Actions
//
Expand All @@ -127,6 +150,7 @@ export class PolicyStatement {
* @param actions actions that will be allowed.
*/
public addActions(...actions: string[]) {
this.assertNotFrozen('addActions');
if (actions.length > 0 && this._notAction.length > 0) {
throw new Error('Cannot add \'Actions\' to policy statement if \'NotActions\' have been added');
}
Expand All @@ -142,6 +166,7 @@ export class PolicyStatement {
* @param notActions actions that will be denied. All other actions will be permitted.
*/
public addNotActions(...notActions: string[]) {
this.assertNotFrozen('addNotActions');
if (notActions.length > 0 && this._action.length > 0) {
throw new Error('Cannot add \'NotActions\' to policy statement if \'Actions\' have been added');
}
Expand All @@ -167,6 +192,7 @@ export class PolicyStatement {
* @param principals IAM principals that will be added
*/
public addPrincipals(...principals: IPrincipal[]) {
this.assertNotFrozen('addPrincipals');
this._principals.push(...principals);
if (Object.keys(principals).length > 0 && Object.keys(this._notPrincipal).length > 0) {
throw new Error('Cannot add \'Principals\' to policy statement if \'NotPrincipals\' have been added');
Expand All @@ -188,6 +214,7 @@ export class PolicyStatement {
* @param notPrincipals IAM principals that will be denied access
*/
public addNotPrincipals(...notPrincipals: IPrincipal[]) {
this.assertNotFrozen('addNotPrincipals');
this._notPrincipals.push(...notPrincipals);
if (Object.keys(notPrincipals).length > 0 && Object.keys(this._principal).length > 0) {
throw new Error('Cannot add \'NotPrincipals\' to policy statement if \'Principals\' have been added');
Expand Down Expand Up @@ -280,6 +307,7 @@ export class PolicyStatement {
* @param arns Amazon Resource Names (ARNs) of the resources that this policy statement applies to
*/
public addResources(...arns: string[]) {
this.assertNotFrozen('addResources');
if (arns.length > 0 && this._notResource.length > 0) {
throw new Error('Cannot add \'Resources\' to policy statement if \'NotResources\' have been added');
}
Expand All @@ -295,6 +323,7 @@ export class PolicyStatement {
* @param arns Amazon Resource Names (ARNs) of the resources that this policy statement does not apply to
*/
public addNotResources(...arns: string[]) {
this.assertNotFrozen('addNotResources');
if (arns.length > 0 && this._resource.length > 0) {
throw new Error('Cannot add \'NotResources\' to policy statement if \'Resources\' have been added');
}
Expand Down Expand Up @@ -344,6 +373,7 @@ export class PolicyStatement {
* ```
*/
public addCondition(key: string, value: Condition) {
this.assertNotFrozen('addCondition');
const existingValue = this._condition[key];
this._condition[key] = existingValue ? { ...existingValue, ...value } : value;
}
Expand Down Expand Up @@ -544,6 +574,29 @@ export class PolicyStatement {
return { ...this._condition };
}

/**
* Make the PolicyStatement immutable
*
* After calling this, any of the `addXxx()` methods will throw an exception.
*
* Libraries that lazily generate statement bodies can override this method to
* fill the actual PolicyStatement fields. Be aware that this method may be called
* multiple times.
*/
public freeze(): PolicyStatement {
this._frozen = true;
return this;
}

/**
* Whether the PolicyStatement has been frozen
*
* The statement object is frozen when `freeze()` is called.
*/
public get frozen(): boolean {
return this._frozen;
}

/**
* Estimate the size of this policy statement
*
Expand Down Expand Up @@ -577,6 +630,15 @@ export class PolicyStatement {
}
}
}

/**
* Throw an exception when the object is frozen
*/
private assertNotFrozen(method: string) {
if (this._frozen) {
throw new Error(`${method}: freeze() has been called on this PolicyStatement previously, so it can no longer be modified`);
}
}
}

/**
Expand Down
33 changes: 33 additions & 0 deletions packages/@aws-cdk/aws-iam/test/merge-statements.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,25 @@ test('keep merging even if it requires multiple passes', () => {
]);
});

test('lazily generated statements are merged correctly', () => {
assertMerged([
new LazyStatement((s) => {
s.addActions('service:A');
s.addResources('R1');
}),
new LazyStatement((s) => {
s.addActions('service:B');
s.addResources('R1');
}),
], [
{
Effect: 'Allow',
Action: ['service:A', 'service:B'],
Resource: 'R1',
},
]);
});

function assertNoMerge(statements: iam.PolicyStatement[]) {
const app = new App();
const stack = new Stack(app, 'Stack');
Expand Down Expand Up @@ -499,3 +518,17 @@ function assertMerged(statements: iam.PolicyStatement[], expected: any[]) {
function assertMergedC(doMerge: boolean, statements: iam.PolicyStatement[], expected: any[]) {
return doMerge ? assertMerged(statements, expected) : assertNoMerge(statements);
}

/**
* A statement that fills itself only when freeze() is called.
*/
class LazyStatement extends iam.PolicyStatement {
constructor(private readonly modifyMe: (x: iam.PolicyStatement) => void) {
super();
}

public freeze() {
this.modifyMe(this);
return super.freeze();
}
}
29 changes: 28 additions & 1 deletion packages/@aws-cdk/aws-iam/test/policy-statement.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Stack } from '@aws-cdk/core';
import { AnyPrincipal, Group, PolicyDocument, PolicyStatement } from '../lib';
import { AnyPrincipal, Group, PolicyDocument, PolicyStatement, Effect } from '../lib';

describe('IAM policy statement', () => {

Expand Down Expand Up @@ -214,4 +214,31 @@ describe('IAM policy statement', () => {
expect(() => policyStatement.addNotPrincipals(group))
.toThrow(/Cannot use an IAM Group as the 'Principal' or 'NotPrincipal' in an IAM Policy/);
});


test('a frozen policy statement cannot be modified any more', () => {
// GIVEN
const statement = new PolicyStatement({
actions: ['action:a'],
resources: ['*'],
});
statement.freeze();

// WHEN
const modifications = [
() => statement.sid = 'asdf',
() => statement.effect = Effect.DENY,
() => statement.addActions('abc:def'),
() => statement.addNotActions('abc:def'),
() => statement.addResources('*'),
() => statement.addNotResources('*'),
() => statement.addPrincipals(new AnyPrincipal()),
() => statement.addNotPrincipals(new AnyPrincipal()),
() => statement.addCondition('key', 'value'),
];

for (const mod of modifications) {
expect(mod).toThrow(/can no longer be modified/);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,9 @@
},
"excludeCharacters": " %+~`#$&*()|[]{}:;<>?!'/@\"\\"
}
}
},
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
}
},
"Mappings": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"version":"17.0.0"}
{"version":"20.0.0"}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"version": "18.0.0",
"version": "20.0.0",
"testCases": {
"aws-rds/test/integ.cluster-rotation.lit": {
"integ.cluster-rotation.lit": {
"stacks": [
"aws-cdk-rds-cluster-rotation"
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"version": "17.0.0",
"version": "20.0.0",
"artifacts": {
"Tree": {
"type": "cdk:tree",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
"id": "Tree",
"path": "Tree",
"constructInfo": {
"fqn": "@aws-cdk/core.Construct",
"version": "0.0.0"
"fqn": "constructs.Construct",
"version": "10.1.33"
}
},
"aws-cdk-rds-cluster-rotation": {
Expand Down

This file was deleted.

Loading

0 comments on commit 7a7ca65

Please sign in to comment.