Skip to content

Commit

Permalink
chore(bootstrap): improve permissions boundary switch message (#24354)
Browse files Browse the repository at this point in the history
If you were not applying a permissions boundary at all (previously or currently) then you would get a message like `Switching from  to undefined...`. This PR fixes that and updates the wording a little.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
corymhall committed Feb 27, 2023
1 parent bb4311b commit 5aadf2a
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 3 deletions.
16 changes: 13 additions & 3 deletions packages/aws-cdk/lib/api/bootstrap/bootstrap-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,26 @@ export class Bootstrapper {
* - the name indicating the custom permissions boundary to be used
* Re-bootstrapping will NOT be blocked by either tightening or relaxing the permissions' boundary.
*/
const currentPermissionsBoundary = current.parameters.InputPermissionsBoundary;

// InputPermissionsBoundary is an `any` type and if it is not defined it
// appears as an empty string ''. We need to force it to evaluate an empty string
// as undefined
const currentPermissionsBoundary: string | undefined = current.parameters.InputPermissionsBoundary || undefined;
const inputPolicyName = params.examplePermissionsBoundary ? CDK_BOOTSTRAP_PERMISSIONS_BOUNDARY : params.customPermissionsBoundary;
let policyName;
let policyName: string | undefined;
if (inputPolicyName) {
// If the example policy is not already in place, it must be created.
const sdk = (await sdkProvider.forEnvironment(environment, Mode.ForWriting)).sdk;
policyName = await this.getPolicyName(environment, sdk, inputPolicyName, partition, params);
}
if (currentPermissionsBoundary !== policyName) {
warning(`Switching from ${currentPermissionsBoundary} to ${policyName} as permissions boundary`);
if (!currentPermissionsBoundary) {
warning(`Adding new permissions boundary ${policyName}`);
} else if (!policyName) {
warning(`Removing existing permissions boundary ${currentPermissionsBoundary}`);
} else {
warning(`Changing permissions boundary from ${currentPermissionsBoundary} to ${policyName}`);
}
}

return current.update(
Expand Down
79 changes: 79 additions & 0 deletions packages/aws-cdk/test/api/bootstrap2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ import { mockBootstrapStack, MockSdk, MockSdkProvider } from '../util/mock-sdk';
let bootstrapper: Bootstrapper;
let mockGetPolicyIamCode: (params: IAM.Types.GetPolicyRequest) => IAM.Types.GetPolicyResponse;
let mockCreatePolicyIamCode: (params: IAM.Types.CreatePolicyRequest) => IAM.Types.CreatePolicyResponse;
let stderrMock: jest.SpyInstance;

beforeEach(() => {
bootstrapper = new Bootstrapper({ source: 'default' });
stderrMock = jest.spyOn(process.stderr, 'write').mockImplementation(() => { return true; });
});

afterEach(() => {
stderrMock.mockRestore();
});

function mockTheToolkitInfo(stackProps: Partial<AWS.CloudFormation.Stack>) {
Expand Down Expand Up @@ -114,6 +120,14 @@ describe('Bootstrapping v2', () => {
});

test('passes value to PermissionsBoundary', async () => {
mockTheToolkitInfo({
Parameters: [
{
ParameterKey: 'InputPermissionsBoundary',
ParameterValue: 'existing-pb',
},
],
});
await bootstrapper.bootstrapEnvironment(env, sdk, {
parameters: {
customPermissionsBoundary: 'permissions-boundary-name',
Expand All @@ -125,6 +139,71 @@ describe('Bootstrapping v2', () => {
InputPermissionsBoundary: 'permissions-boundary-name',
}),
}));
expect(stderrMock.mock.calls).toEqual(expect.arrayContaining([
expect.arrayContaining([
expect.stringMatching(/Changing permissions boundary from existing-pb to permissions-boundary-name/),
]),
]));
});

test('permission boundary switch message does not appear', async () => {
mockTheToolkitInfo({
Parameters: [
{
ParameterKey: 'InputPermissionsBoundary',
ParameterValue: '',
},
],
});
await bootstrapper.bootstrapEnvironment(env, sdk);

expect(stderrMock.mock.calls).toEqual(expect.arrayContaining([
expect.not.arrayContaining([
expect.stringMatching(/Changing permissions boundary/),
]),
]));
});

test('adding new permissions boundary', async () => {
mockTheToolkitInfo({
Parameters: [
{
ParameterKey: 'InputPermissionsBoundary',
ParameterValue: '',
},
],
});
await bootstrapper.bootstrapEnvironment(env, sdk, {
parameters: {
customPermissionsBoundary: 'permissions-boundary-name',
},
});

expect(stderrMock.mock.calls).toEqual(expect.arrayContaining([
expect.arrayContaining([
expect.stringMatching(/Adding new permissions boundary permissions-boundary-name/),
]),
]));
});

test('removing existing permissions boundary', async () => {
mockTheToolkitInfo({
Parameters: [
{
ParameterKey: 'InputPermissionsBoundary',
ParameterValue: 'permissions-boundary-name',
},
],
});
await bootstrapper.bootstrapEnvironment(env, sdk, {
parameters: {},
});

expect(stderrMock.mock.calls).toEqual(expect.arrayContaining([
expect.arrayContaining([
expect.stringMatching(/Removing existing permissions boundary permissions-boundary-name/),
]),
]));
});

test('passing trusted accounts without CFN managed policies results in an error', async () => {
Expand Down

0 comments on commit 5aadf2a

Please sign in to comment.