Skip to content

Commit 28b0080

Browse files
fix: cdk diff prints upgrade bootstrap warning even when current version exceeds the recommended version (#29938)
Closes #28888 ### Reason for this change Currently, we just check for if a minimum bootstrap version is needed for a functionality and give a warning. But, we do not check the bootstrap version of the stack itself. This gives a confusing warning for upgrading to a bootstrap version (example: version 8) when the stack bootstrap version is already higher than the recommended version. ### Description of changes We cannot know successfully what the bootstrap version is in the AWS account without the lookup role. We are getting this warning while trying to assume the lookup role and failing to assume it. I am removing upgrade related warnings since we are emitting them without any confirmation of the user's account bootstrap version. This will lead to confusion. Instead, I am trying to make some of the existing error messages more clear. ### Description of how you validated changes Updated unit tests. Will run this through test pipeline. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent f00a79e commit 28b0080

File tree

2 files changed

+18
-15
lines changed

2 files changed

+18
-15
lines changed

Diff for: packages/aws-cdk/lib/api/deployments.ts

+16-11
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { StackActivityProgress } from './util/cloudformation/stack-activity-moni
1313
import { replaceEnvPlaceholders } from './util/placeholders';
1414
import { makeBodyParameterAndUpload } from './util/template-body-parameter';
1515
import { Tag } from '../cdk-toolkit';
16-
import { debug, warning } from '../logging';
16+
import { debug, warning, error } from '../logging';
1717
import { buildAssets, publishAssets, BuildAssetsOptions, PublishAssetsOptions, PublishingAws, EVENT_TO_LOGGER } from '../util/asset-publishing';
1818

1919
/**
@@ -532,8 +532,9 @@ export class Deployments {
532532

533533
// try to assume the lookup role
534534
const warningMessage = `Could not assume ${arns.lookupRoleArn}, proceeding anyway.`;
535-
const upgradeMessage = `(To get rid of this warning, please upgrade to bootstrap version >= ${stack.lookupRole?.requiresBootstrapStackVersion})`;
535+
536536
try {
537+
// Trying to assume lookup role and cache the sdk for the environment
537538
const stackSdk = await this.cachedSdkForEnvironment(resolvedEnvironment, Mode.ForReading, {
538539
assumeRoleArn: arns.lookupRoleArn,
539540
assumeRoleExternalId: stack.lookupRole?.assumeRoleExternalId,
@@ -545,22 +546,26 @@ export class Deployments {
545546
if (stackSdk.didAssumeRole && stack.lookupRole?.bootstrapStackVersionSsmParameter && stack.lookupRole.requiresBootstrapStackVersion) {
546547
const version = await envResources.versionFromSsmParameter(stack.lookupRole.bootstrapStackVersionSsmParameter);
547548
if (version < stack.lookupRole.requiresBootstrapStackVersion) {
548-
throw new Error(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'.`);
549+
throw new Error(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'. To get rid of this error, please upgrade to bootstrap version >= ${stack.lookupRole.requiresBootstrapStackVersion}`);
549550
}
550-
// we may not have assumed the lookup role because one was not provided
551-
// if that is the case then don't print the upgrade warning
552-
} else if (!stackSdk.didAssumeRole && stack.lookupRole?.requiresBootstrapStackVersion) {
553-
warning(upgradeMessage);
551+
} else if (!stackSdk.didAssumeRole) {
552+
const lookUpRoleExists = stack.lookupRole ? true : false;
553+
warning(`Lookup role ${ lookUpRoleExists ? 'exists but' : 'does not exist, hence'} was not assumed. Proceeding with default credentials.`);
554554
}
555555
return { ...stackSdk, resolvedEnvironment, envResources };
556556
} catch (e: any) {
557557
debug(e);
558-
// only print out the warnings if the lookupRole exists AND there is a required
559-
// bootstrap version, otherwise the warnings will print `undefined`
560-
if (stack.lookupRole && stack.lookupRole.requiresBootstrapStackVersion) {
558+
559+
// only print out the warnings if the lookupRole exists
560+
if (stack.lookupRole) {
561561
warning(warningMessage);
562-
warning(upgradeMessage);
563562
}
563+
564+
// This error should be shown even if debug mode is off
565+
if (e instanceof Error && e.message.includes('Bootstrap stack version')) {
566+
error(e.message);
567+
}
568+
564569
throw (e);
565570
}
566571
}

Diff for: packages/aws-cdk/test/cdk-toolkit.test.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ describe('readCurrentTemplate', () => {
241241
// THEN
242242
expect(flatten(stderrMock.mock.calls)).toEqual(expect.arrayContaining([
243243
expect.stringMatching(/Could not assume bloop-lookup:here:123456789012/),
244-
expect.stringMatching(/please upgrade to bootstrap version >= 5/),
244+
expect.stringContaining("Bootstrap stack version '5' is required, found version '1'. To get rid of this error, please upgrade to bootstrap version >= 5"),
245245
]));
246246
expect(requestedParameterName!).toEqual('/bootstrap/parameter');
247247
expect(mockForEnvironment.mock.calls.length).toEqual(3);
@@ -276,7 +276,6 @@ describe('readCurrentTemplate', () => {
276276
// THEN
277277
expect(flatten(stderrMock.mock.calls)).toEqual(expect.arrayContaining([
278278
expect.stringMatching(/Could not assume bloop-lookup:here:123456789012/),
279-
expect.stringMatching(/please upgrade to bootstrap version >= 5/),
280279
]));
281280
expect(mockForEnvironment.mock.calls.length).toEqual(3);
282281
expect(mockForEnvironment.mock.calls[0][2]).toEqual({
@@ -315,7 +314,6 @@ describe('readCurrentTemplate', () => {
315314
expect(mockCloudExecutable.sdkProvider.sdk.ssm).not.toHaveBeenCalled();
316315
expect(flatten(stderrMock.mock.calls)).toEqual(expect.arrayContaining([
317316
expect.stringMatching(/Could not assume bloop-lookup:here:123456789012/),
318-
expect.stringMatching(/please upgrade to bootstrap version >= 5/),
319317
]));
320318
expect(mockForEnvironment.mock.calls.length).toEqual(3);
321319
expect(mockForEnvironment.mock.calls[0][2]).toEqual({
@@ -350,7 +348,7 @@ describe('readCurrentTemplate', () => {
350348

351349
// THEN
352350
expect(flatten(stderrMock.mock.calls)).toEqual(expect.arrayContaining([
353-
expect.stringMatching(/please upgrade to bootstrap version >= 5/),
351+
expect.stringMatching(/Lookup role exists but was not assumed. Proceeding with default credentials./),
354352
]));
355353
expect(mockCloudExecutable.sdkProvider.sdk.ssm).not.toHaveBeenCalled();
356354
expect(mockForEnvironment.mock.calls.length).toEqual(3);

0 commit comments

Comments
 (0)