Skip to content

Commit

Permalink
fix(cli): warning to upgrade to bootstrap version >= undefined (#18489)
Browse files Browse the repository at this point in the history
When we try to assume the lookup role and fail, we print out a warning
telling the user to upgrade to the required bootstrap stack version.

In cases where the user is not using the default stack synthesizer
(legacy, custom, etc.) the cli might not have information on the lookup
role. In those cases where we don't have the necessary information on
the ARN of the lookup role or the required bootstrap version, we will
not print any warnings.


----

*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 authored Jan 18, 2022
1 parent 2415a80 commit da5a305
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 3 deletions.
12 changes: 9 additions & 3 deletions packages/aws-cdk/lib/api/cloudformation-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,20 @@ export async function prepareSdkWithLookupRoleFor(
if (version < stack.lookupRole.requiresBootstrapStackVersion) {
throw new Error(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'.`);
}
} else if (!stackSdk.didAssumeRole) {
// we may not have assumed the lookup role because one was not provided
// if that is the case then don't print the upgrade warning
} else if (!stackSdk.didAssumeRole && stack.lookupRole?.requiresBootstrapStackVersion) {
warning(upgradeMessage);
}
return { ...stackSdk, resolvedEnvironment };
} catch (e) {
debug(e);
warning(warningMessage);
warning(upgradeMessage);
// only print out the warnings if the lookupRole exists AND there is a required
// bootstrap version, otherwise the warnings will print `undefined`
if (stack.lookupRole && stack.lookupRole.requiresBootstrapStackVersion) {
warning(warningMessage);
warning(upgradeMessage);
}
throw (e);
}
}
Expand Down
44 changes: 44 additions & 0 deletions packages/aws-cdk/test/cdk-toolkit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ describe('readCurrentTemplate', () => {
},
},
},
{
stackName: 'Test-Stack-A',
template,
properties: {
assumeRoleArn: 'bloop:${AWS::Region}:${AWS::AccountId}',
},
},
],
});
mockForEnvironment = jest.fn().mockImplementation(() => { return { sdk: mockCloudExecutable.sdkProvider.sdk, didAssumeRole: true }; });
Expand All @@ -145,6 +152,11 @@ describe('readCurrentTemplate', () => {
StackStatus: 'CREATE_COMPLETE',
CreationTime: new Date(),
},
{
StackName: 'Test-Stack-A',
StackStatus: 'CREATE_COMPLETE',
CreationTime: new Date(),
},
],
};
},
Expand Down Expand Up @@ -329,6 +341,38 @@ describe('readCurrentTemplate', () => {
assumeRoleArn: 'bloop:here:123456789012',
});
});

test('do not print warnings if lookup role not provided in stack artifact', async () => {
// GIVEN
mockCloudExecutable.sdkProvider.stubSSM({
getParameter() {
return {};
},
});
const cdkToolkit = new CdkToolkit({
cloudExecutable: mockCloudExecutable,
configuration: mockCloudExecutable.configuration,
sdkProvider: mockCloudExecutable.sdkProvider,
cloudFormation: new CloudFormationDeployments({ sdkProvider: mockCloudExecutable.sdkProvider }),
});

// WHEN
await cdkToolkit.deploy({
selector: { patterns: ['Test-Stack-A'] },
});

// THEN
expect(flatten(stderrMock.mock.calls)).not.toEqual(expect.arrayContaining([
expect.stringMatching(/Could not assume/),
expect.stringMatching(/please upgrade to bootstrap version/),
]));
expect(mockCloudExecutable.sdkProvider.sdk.ssm).not.toHaveBeenCalled();
expect(mockForEnvironment.mock.calls.length).toEqual(2);
expect(mockForEnvironment.mock.calls[0][2]).toEqual({
assumeRoleArn: undefined,
assumeRoleExternalId: undefined,
});
});
});

describe('deploy', () => {
Expand Down

0 comments on commit da5a305

Please sign in to comment.