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

feat(cdk): introduce the new cfn-changeset-review-role for the diff command #30568

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,9 @@ export async function installNpmPackages(fixture: TestFixture, packages: Record<
}, undefined, 2), { encoding: 'utf-8' });

// Now install that `package.json` using NPM7
await fixture.shell(['node', require.resolve('npm'), 'install']);
const isRepoMode = !!process.env.REPO_ROOT;
const npmPath = isRepoMode ? path.join(__dirname, 'package-sources/repo-tools/npm') : require.resolve('npm');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The require.resolve function searches under the node_modules, so we were not able to use fake npm to use local packages.

await fixture.shell(['node', npmPath, 'install']);
}

const ALREADY_BOOTSTRAPPED_IN_THIS_RUN = new Set();
14 changes: 14 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,18 @@ class BuiltinLambdaStack extends cdk.Stack {
}
}

class IamRolesStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);

for(let i = 1; i <= Number(process.env.NUMBER_OF_ROLES) ; i++) {
new iam.Role(this, `Role${i}`, {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
});
}
}
}

class NotificationArnPropStack extends cdk.Stack {
constructor(parent, id, props) {
super(parent, id, props);
Expand Down Expand Up @@ -807,6 +819,8 @@ switch (stackSet) {
new DockerStack(app, `${stackPrefix}-docker`);
new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);

new IamRolesStack(app, `${stackPrefix}-iam-roles`);

new NotificationArnPropStack(app, `${stackPrefix}-notification-arn-prop`, {
notificationArns: [`arn:aws:sns:${defaultEnv.region}:${defaultEnv.account}:${stackPrefix}-test-topic-prop`],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,26 @@ integTest(
}),
);

integTest('cdk diff with a large template', withDefaultFixture(async (fixture) => {
// In this test, we confirm that the changeset role can upload assets to the bucket and create a changeset
await fixture.cdkDeploy('iam-roles', {
modEnv: {
NUMBER_OF_ROLES: '1',
},
});
const diff = await fixture.cdk(['diff', fixture.fullStackName('iam-roles')], {
modEnv: {
NUMBER_OF_ROLES: '200',
},
});
expect(diff).not.toContain('changeset role does not exist, hence was not assumed');
expect(diff).not.toContain('Could not create a change set');
expect(diff).not.toContain('deploy-role');
expect(diff).toContain('cfn-changeset-review-role');
expect(diff).toContain('success: Published');
expect(diff).toContain('Number of stacks with differences');
}));

integTest(
'enableDiffNoFail',
withDefaultFixture(async (fixture) => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions packages/@aws-cdk/app-staging-synthesizer-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ const app = new App({
cloudFormationExecutionRole: BootstrapRole.fromRoleArn('arn:aws:iam::123456789012:role/Execute'),
deploymentRole: BootstrapRole.fromRoleArn('arn:aws:iam::123456789012:role/Deploy'),
lookupRole: BootstrapRole.fromRoleArn('arn:aws:iam::123456789012:role/Lookup'),
changesetRole: BootstrapRole.fromRoleArn('arn:aws:iam::123456789012:role/Changeset'),
}),
}),
});
Expand All @@ -167,8 +168,8 @@ const app = new App({
Or, you can ask to use the CLI credentials that exist at deploy-time.
These credentials must have the ability to perform CloudFormation calls,
lookup resources in your account, and perform CloudFormation deployment.
For a full list of what is necessary, see `LookupRole`, `DeploymentActionRole`,
and `CloudFormationExecutionRole` in the
For a full list of what is necessary, see `LookupRole`, `DeploymentActionRole`, `CloudFormationExecutionRole`,
and `CloudFormationChangeSetReviewRole` in the
[bootstrap template](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk/lib/api/bootstrap/bootstrap-template.yaml).

```ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ export class AppStagingSynthesizer extends StackSynthesizer implements IReusable
*/
public static readonly DEFAULT_LOOKUP_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-lookup-role-${AWS::AccountId}-${AWS::Region}';

/**
* Default changeset role ARN for missing values.
*/
public static readonly DEFAULT_CHANGESET_ROLE_ARN = 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-${Qualifier}-cfn-changeset-review-role-${AWS::AccountId}-${AWS::Region}';

/**
* Use the Default Staging Resources, creating a single stack per environment this app is deployed in
*/
Expand Down Expand Up @@ -177,7 +182,8 @@ export class AppStagingSynthesizer extends StackSynthesizer implements IReusable
this.roles = {
deploymentRole: identities.deploymentRole ?? defaultIdentities.deploymentRole!,
cloudFormationExecutionRole: identities.cloudFormationExecutionRole ?? defaultIdentities.cloudFormationExecutionRole!,
lookupRole: identities.lookupRole ?? identities.lookupRole!,
lookupRole: identities.lookupRole ?? defaultIdentities.lookupRole!,
changesetRole: identities.changesetRole ?? defaultIdentities.changesetRole!,
};
}

Expand Down Expand Up @@ -207,6 +213,7 @@ export class AppStagingSynthesizer extends StackSynthesizer implements IReusable
deployRole,
cloudFormationExecutionRole: this.roles.cloudFormationExecutionRole._specialize(spec),
lookupRole: this.roles.lookupRole._specialize(spec),
changesetRole: this.roles.changesetRole._specialize(spec),
qualifier,
});
}
Expand Down Expand Up @@ -292,6 +299,11 @@ interface BoundAppStagingSynthesizerProps {
* Lookup Role
*/
readonly lookupRole: BootstrapRole;

/**
* Changeset Role
*/
readonly changesetRole: BootstrapRole;
}

class BoundAppStagingSynthesizer extends StackSynthesizer implements IBoundAppStagingSynthesizer {
Expand Down Expand Up @@ -323,12 +335,14 @@ class BoundAppStagingSynthesizer extends StackSynthesizer implements IBoundAppSt
const assetManifestId = this.assetManifest.emitManifest(this.boundStack, session, {}, dependencies);

const lookupRoleArn = this.props.lookupRole?._arnForCloudAssembly();
const changesetRoleArn = this.props.changesetRole?._arnForCloudAssembly();

this.emitArtifact(session, {
assumeRoleArn: this.props.deployRole?._arnForCloudAssembly(),
additionalDependencies: [assetManifestId],
stackTemplateAssetObjectUrl: templateAsset.s3ObjectUrlWithPlaceholders,
cloudFormationExecutionRoleArn: this.props.cloudFormationExecutionRole?._arnForCloudAssembly(),
changesetRole: changesetRoleArn ? { arn: changesetRoleArn } : undefined,
lookupRole: lookupRoleArn ? { arn: lookupRoleArn } : undefined,
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export class DeploymentIdentities {
cloudFormationExecutionRole: BootstrapRole.cliCredentials(),
deploymentRole: BootstrapRole.cliCredentials(),
lookupRole: BootstrapRole.cliCredentials(),
changesetRole: BootstrapRole.cliCredentials(),
});
}

Expand All @@ -93,6 +94,7 @@ export class DeploymentIdentities {
deploymentRole: BootstrapRole.fromRoleArn(replacePlaceholders(AppStagingSynthesizer.DEFAULT_DEPLOY_ROLE_ARN)),
cloudFormationExecutionRole: BootstrapRole.fromRoleArn(replacePlaceholders(AppStagingSynthesizer.DEFAULT_CLOUDFORMATION_ROLE_ARN)),
lookupRole: BootstrapRole.fromRoleArn(replacePlaceholders(AppStagingSynthesizer.DEFAULT_LOOKUP_ROLE_ARN)),
changesetRole: BootstrapRole.fromRoleArn(replacePlaceholders(AppStagingSynthesizer.DEFAULT_CHANGESET_ROLE_ARN)),
});
}

Expand All @@ -112,13 +114,20 @@ export class DeploymentIdentities {
*/
public readonly lookupRole?: BootstrapRole;

/**
* Changeset Role
@default - use bootstrapped role
*/
public readonly changesetRole?: BootstrapRole;

private constructor(
/** roles that are bootstrapped to your account. */
roles: BootstrapRoles,
) {
this.cloudFormationExecutionRole = roles.cloudFormationExecutionRole;
this.deploymentRole = roles.deploymentRole;
this.lookupRole = roles.lookupRole;
this.changesetRole = roles.changesetRole;
}
}

Expand Down Expand Up @@ -160,6 +169,13 @@ export interface BootstrapRoles {
* @default - use bootstrapped role
*/
readonly lookupRole?: BootstrapRole;

/**
* Changeset Role
*
* @default - use bootstrapped role
*/
readonly changesetRole?: BootstrapRole;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { AppStagingSynthesizer, BootstrapRole, DeploymentIdentities } from '../l
const CLOUDFORMATION_EXECUTION_ROLE = 'cloudformation-execution-role';
const DEPLOY_ACTION_ROLE = 'deploy-action-role';
const LOOKUP_ROLE = 'lookup-role';
const CHANG_SET_ROLE = 'changeset-role';

describe('Boostrap Roles', () => {
test('default bootstrap role name is always under 64 characters', () => {
Expand Down Expand Up @@ -48,6 +49,7 @@ describe('Boostrap Roles', () => {
cloudFormationExecutionRole: BootstrapRole.fromRoleArn(CLOUDFORMATION_EXECUTION_ROLE),
lookupRole: BootstrapRole.fromRoleArn(LOOKUP_ROLE),
deploymentRole: BootstrapRole.fromRoleArn(DEPLOY_ACTION_ROLE),
changesetRole: BootstrapRole.fromRoleArn(CHANG_SET_ROLE),
}),
stagingBucketEncryption: BucketEncryption.S3_MANAGED,
}),
Expand All @@ -72,6 +74,7 @@ describe('Boostrap Roles', () => {
expect(stackArtifact.cloudFormationExecutionRoleArn).toEqual(CLOUDFORMATION_EXECUTION_ROLE);
expect(stackArtifact.lookupRole).toEqual({ arn: LOOKUP_ROLE });
expect(stackArtifact.assumeRoleArn).toEqual(DEPLOY_ACTION_ROLE);
expect(stackArtifact.changesetRole).toEqual({ arn: CHANG_SET_ROLE });
});

test('can request other bootstrap region', () => {
Expand All @@ -93,6 +96,7 @@ describe('Boostrap Roles', () => {
expect(stackArtifact.cloudFormationExecutionRoleArn).toEqual('arn:${AWS::Partition}:iam::000000000000:role/cdk-hnb659fds-cfn-exec-role-000000000000-us-west-2');
expect(stackArtifact.lookupRole).toEqual({ arn: 'arn:${AWS::Partition}:iam::000000000000:role/cdk-hnb659fds-lookup-role-000000000000-us-west-2' });
expect(stackArtifact.assumeRoleArn).toEqual('arn:${AWS::Partition}:iam::000000000000:role/cdk-hnb659fds-deploy-role-000000000000-us-west-2');
expect(stackArtifact.changesetRole).toEqual({ arn: 'arn:${AWS::Partition}:iam::000000000000:role/cdk-hnb659fds-cfn-changeset-review-role-000000000000-us-west-2' });
});

test('can request other qualifier', () => {
Expand All @@ -115,6 +119,7 @@ describe('Boostrap Roles', () => {
expect(stackArtifact.cloudFormationExecutionRoleArn).toEqual('arn:${AWS::Partition}:iam::000000000000:role/cdk-Q-cfn-exec-role-000000000000-us-west-2');
expect(stackArtifact.lookupRole).toEqual({ arn: 'arn:${AWS::Partition}:iam::000000000000:role/cdk-Q-lookup-role-000000000000-us-west-2' });
expect(stackArtifact.assumeRoleArn).toEqual('arn:${AWS::Partition}:iam::000000000000:role/cdk-Q-deploy-role-000000000000-us-west-2');
expect(stackArtifact.changesetRole).toEqual({ arn: 'arn:${AWS::Partition}:iam::000000000000:role/cdk-Q-cfn-changeset-review-role-000000000000-us-west-2' });
});

test('can supply existing arn for bucket staging role', () => {
Expand Down Expand Up @@ -210,6 +215,7 @@ describe('Boostrap Roles', () => {
expect(stackArtifact.cloudFormationExecutionRoleArn).toBeUndefined();
expect(stackArtifact.lookupRole).toBeUndefined();
expect(stackArtifact.assumeRoleArn).toBeUndefined();
expect(stackArtifact.changesetRole).toBeUndefined();
});

test('qualifier is resolved in the synthesizer', () => {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
Loading
Loading