Skip to content

Commit

Permalink
Merge branch 'main' into awsgh-26827
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Aug 23, 2023
2 parents 5dca98c + dd912da commit afdac07
Show file tree
Hide file tree
Showing 89 changed files with 734 additions and 208 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-gamelift-alpha/lib/fleet-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,10 +639,10 @@ export abstract class FleetBase extends cdk.Resource implements IFleet {
}

protected warnVpcPeeringAuthorizations(scope: Construct): void {
cdk.Annotations.of(scope).addWarning([
cdk.Annotations.of(scope).addWarningV2('@aws-cdk/aws-gamelift:fleetAutorizeVpcPeering', [
'To authorize the VPC peering, call the GameLift service API CreateVpcPeeringAuthorization() or use the AWS CLI command create-vpc-peering-authorization.',
'Make this call using the account that manages your non-GameLift resources.',
'See: https://docs.aws.amazon.com/gamelift/latest/developerguide/vpc-peering.html',
].join('\n'));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ abstract class StackAssociatorBase implements IAspect {
if (Stage.isStage(childNode)) {
var stageAssociated = this.applicationAssociator?.isStageAssociated(childNode);
if (stageAssociated === false) {
this.warning(childNode, 'Associate Stage: ' + childNode.stageName + ' to ensure all stacks in your cdk app are associated with AppRegistry. '
this.warning('StackNotAssociated', childNode, 'Associate Stage: ' + childNode.stageName + ' to ensure all stacks in your cdk app are associated with AppRegistry. '
+ 'You can use ApplicationAssociator.associateStage to associate any stage.');
}
}
Expand Down Expand Up @@ -73,8 +73,8 @@ abstract class StackAssociatorBase implements IAspect {
* @param node The scope to add the warning to.
* @param message The error message.
*/
private warning(node: IConstruct, message: string): void {
Annotations.of(node).addWarning(message);
private warning(id: string, node: IConstruct, message: string): void {
Annotations.of(node).addWarningV2(`@aws-cdk/servicecatalogappregistry:${id}`, message);
}

/**
Expand All @@ -87,12 +87,12 @@ abstract class StackAssociatorBase implements IAspect {
*/
private handleCrossRegionStack(node: Stack): void {
if (isRegionUnresolved(this.application.env.region, node.region)) {
this.warning(node, 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack.');
this.warning('EnvironmentAgnosticStack', node, 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack.');
return;
}

if (node.region != this.application.env.region) {
this.warning(node, 'AppRegistry does not support cross region associations, deployment might fail if there is cross region stacks in the app. Application region '
this.warning('CrossRegionAssociation', node, 'AppRegistry does not support cross region associations, deployment might fail if there is cross region stacks in the app. Application region '
+ this.application.env.region + ', stack region ' + node.region);
}
}
Expand All @@ -106,7 +106,7 @@ abstract class StackAssociatorBase implements IAspect {
*/
private handleCrossAccountStack(node: Stack): void {
if (isAccountUnresolved(this.application.env.account!, node.account)) {
this.warning(node, 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack.');
this.warning('EnvironmentAgnosticStack', node, 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack.');
return;
}

Expand All @@ -121,7 +121,7 @@ abstract class StackAssociatorBase implements IAspect {

this.sharedAccounts.add(node.account);
} else {
this.warning(node, 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
this.warning('AssociationSkipped', node, 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
const crossAccountStack = new cdk.Stack(app, 'crossRegionStack', {
env: { account: 'account', region: 'region' },
});
Annotations.fromStack(crossAccountStack).hasWarning('*', 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled.');
Annotations.fromStack(crossAccountStack).hasWarning('*', 'Cross-account stack detected but application sharing and association will be skipped because cross-account option is not enabled. [ack: @aws-cdk/servicecatalogappregistry:AssociationSkipped]');
});

test('ApplicationAssociator with cross account stacks inside cdkApp does not give warning if associateCrossAccountStacks is set to true', () => {
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
env: { account: 'account', region: 'region' },
});
Annotations.fromStack(crossRegionStack).hasWarning('*', 'AppRegistry does not support cross region associations, deployment might fail if there is cross region stacks in the app.'
+ ' Application region region2, stack region region');
+ ' Application region region2, stack region region [ack: @aws-cdk/servicecatalogappregistry:CrossRegionAssociation]');
});

test('Environment Agnostic ApplicationAssociator with cross region stacks inside cdkApp gives warning', () => {
Expand All @@ -237,7 +237,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
const crossRegionStack = new cdk.Stack(app, 'crossRegionStack', {
env: { account: 'account', region: 'region' },
});
Annotations.fromStack(crossRegionStack).hasWarning('*', 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack.');
Annotations.fromStack(crossRegionStack).hasWarning('*', 'Environment agnostic stack determined, AppRegistry association might not work as expected in case you deploy cross-region or cross-account stack. [ack: @aws-cdk/servicecatalogappregistry:EnvironmentAgnosticStack]');
});

test('Cdk App Containing Pipeline with stage but stage not associated throws error', () => {
Expand All @@ -253,7 +253,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
});
app.synth();
Annotations.fromStack(pipelineStack).hasWarning('*',
'Associate Stage: SampleStage to ensure all stacks in your cdk app are associated with AppRegistry. You can use ApplicationAssociator.associateStage to associate any stage.');
'Associate Stage: SampleStage to ensure all stacks in your cdk app are associated with AppRegistry. You can use ApplicationAssociator.associateStage to associate any stage. [ack: @aws-cdk/servicecatalogappregistry:StackNotAssociated]');
});

test('Cdk App Containing Pipeline with stage and stage associated successfully gets synthesized', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ describe('Scope based Associations with Application with Cross Region/Account',
application.associateAllStacksInScope(stage);
Annotations.fromStack(stageStack).hasWarning('*',
'AppRegistry does not support cross region associations, deployment might fail if there is cross region stacks in the app.'
+ ' Application region region, stack region region1');
+ ' Application region region, stack region region1 [ack: @aws-cdk/servicecatalogappregistry:CrossRegionAssociation]');
});
});

Expand Down
17 changes: 9 additions & 8 deletions packages/aws-cdk-lib/assertions/test/annotations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ describe('Messages', () => {

describe('hasWarning', () => {
test('match', () => {
annotations.hasWarning('/Default/Fred', 'this is a warning');
annotations.hasWarning('/Default/Fred', 'this is a warning [ack: Fred]');
});

test('no match', () => {
Expand All @@ -89,7 +89,7 @@ describe('Messages', () => {
});

test('no match', () => {
expect(() => annotations.hasNoWarning('/Default/Fred', 'this is a warning'))
expect(() => annotations.hasNoWarning('/Default/Fred', 'this is a warning [ack: Fred]'))
.toThrowError(/Expected no matches, but stack has 1 messages as follows:/);
});
});
Expand Down Expand Up @@ -183,16 +183,16 @@ describe('Multiple Messages on the Resource', () => {
test('succeeds on hasXxx APIs', () => {
annotations.hasError('/Default/Foo', 'error: this is an error');
annotations.hasError('/Default/Foo', 'error: unsupported type Foo::Bar');
annotations.hasWarning('/Default/Foo', 'warning: Foo::Bar is deprecated');
annotations.hasWarning('/Default/Foo', 'warning: Foo::Bar is deprecated [ack: Foo]');
});

test('succeeds on findXxx APIs', () => {
const result1 = annotations.findError('*', Match.stringLikeRegexp('error:.*'));
expect(result1.length).toEqual(4);
const result2 = annotations.findError('/Default/Bar', Match.stringLikeRegexp('error:.*'));
expect(result2.length).toEqual(2);
const result3 = annotations.findWarning('/Default/Bar', 'warning: Foo::Bar is deprecated');
expect(result3[0].entry.data).toEqual('warning: Foo::Bar is deprecated');
const result3 = annotations.findWarning('/Default/Bar', 'warning: Foo::Bar is deprecated [ack: Bar]');
expect(result3[0].entry.data).toEqual('warning: Foo::Bar is deprecated [ack: Bar]');
});
});
class MyAspect implements IAspect {
Expand All @@ -209,7 +209,8 @@ class MyAspect implements IAspect {
};

protected warn(node: IConstruct, message: string): void {
Annotations.of(node).addWarning(message);
// Use construct ID as suppression string, just to make it unique easily
Annotations.of(node).addWarningV2(node.node.id, message);
}

protected error(node: IConstruct, message: string): void {
Expand All @@ -231,10 +232,10 @@ class MultipleAspectsPerNode implements IAspect {
}

protected warn(node: IConstruct, message: string): void {
Annotations.of(node).addWarning(message);
Annotations.of(node).addWarningV2(node.node.id, message);
}

protected error(node: IConstruct, message: string): void {
Annotations.of(node).addError(message);
}
}
}
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-apigateway/lib/method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ export class Method extends Resource {
public addMethodResponse(methodResponse: MethodResponse): void {
const mr = this.methodResponses.find((x) => x.statusCode === methodResponse.statusCode);
if (mr) {
Annotations.of(this).addWarning(`addMethodResponse called multiple times with statusCode=${methodResponse.statusCode}, deployment will be nondeterministic. Use a single addMethodResponse call to configure the entire response.`);
Annotations.of(this).addWarningV2('@aws-cdk/aws-apigateway:duplicateStatusCodes', `addMethodResponse called multiple times with statusCode=${methodResponse.statusCode}, deployment will be nondeterministic. Use a single addMethodResponse call to configure the entire response.`);
}
this.methodResponses.push(methodResponse);
}
Expand Down Expand Up @@ -512,4 +512,4 @@ export enum AuthorizationType {

function pathForArn(path: string): string {
return path.replace(/\{[^\}]*\}/g, '*'); // replace path parameters (like '{bookId}') with asterisk
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export abstract class Schedule {
public readonly expressionString: string = `cron(${minute} ${hour} ${day} ${month} ${weekDay} ${year})`;
public _bind(scope: Construct) {
if (!options.minute) {
Annotations.of(scope).addWarning('cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.');
Annotations.of(scope).addWarningV2('@aws-cdk/aws-applicationautoscaling:defaultRunEveryMinute', 'cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.');
}
return new LiteralSchedule(this.expressionString);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ export class AutoScalingGroupRequireImdsv2Aspect implements cdk.IAspect {
* @param message The warning message.
*/
protected warn(node: IConstruct, message: string) {
cdk.Annotations.of(node).addWarning(`${AutoScalingGroupRequireImdsv2Aspect.name} failed on node ${node.node.id}: ${message}`);
cdk.Annotations.of(node).addWarningV2(`@aws-cdk/aws-autoscaling:imdsv2${AutoScalingGroupRequireImdsv2Aspect.name}`, `${AutoScalingGroupRequireImdsv2Aspect.name} failed on node ${node.node.id}: ${message}`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
});

if (desiredCapacity !== undefined) {
Annotations.of(this).addWarning('desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment. See https://github.com/aws/aws-cdk/issues/5215');
Annotations.of(this).addWarningV2('@aws-cdk/aws-autoscaling:desiredCapacitySet', 'desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment. See https://github.com/aws/aws-cdk/issues/5215');
}

this.maxInstanceLifetime = props.maxInstanceLifetime;
Expand Down Expand Up @@ -2296,7 +2296,7 @@ function synthesizeBlockDeviceMappings(construct: Construct, blockDevices: Block
throw new Error('iops property is required with volumeType: EbsDeviceVolumeType.IO1');
}
} else if (volumeType !== EbsDeviceVolumeType.IO1) {
Annotations.of(construct).addWarning('iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
Annotations.of(construct).addWarningV2('@aws-cdk/aws-autoscaling:iopsIgnored', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-autoscaling/lib/schedule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export abstract class Schedule {
public readonly expressionString: string = `${minute} ${hour} ${day} ${month} ${weekDay}`;
public _bind(scope: Construct) {
if (!options.minute) {
Annotations.of(scope).addWarning('cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.');
Annotations.of(scope).addWarningV2('@aws-cdk/aws-autoscaling:scheduleDefaultRunsEveryMinute', 'cron: If you don\'t pass \'minute\', by default the event runs every minute. Pass \'minute: \'*\'\' if that\'s what you intend, or \'minute: 0\' to run once per hour instead.');
}
return new LiteralSchedule(this.expressionString);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ describe('auto scaling group', () => {
});

// THEN
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1 [ack: @aws-cdk/aws-autoscaling:iopsIgnored]');
});

test('warning if iops and volumeType !== IO1', () => {
Expand All @@ -1259,7 +1259,7 @@ describe('auto scaling group', () => {
});

// THEN
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1');
Annotations.fromStack(stack).hasWarning('/Default/MyStack', 'iops will be ignored without volumeType: EbsDeviceVolumeType.IO1 [ack: @aws-cdk/aws-autoscaling:iopsIgnored]');
});

test('step scaling on metric', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ describeDeprecated('scheduled action', () => {
});

// THEN
Annotations.fromStack(stack).hasWarning('/Default/ASG/ScheduledActionScaleOutInTheMorning', "cron: If you don't pass 'minute', by default the event runs every minute. Pass 'minute: '*'' if that's what you intend, or 'minute: 0' to run once per hour instead.");
Annotations.fromStack(stack).hasWarning('/Default/ASG/ScheduledActionScaleOutInTheMorning', "cron: If you don't pass 'minute', by default the event runs every minute. Pass 'minute: '*'' if that's what you intend, or 'minute: 0' to run once per hour instead. [ack: @aws-cdk/aws-autoscaling:scheduleDefaultRunsEveryMinute]");
});

test('scheduled scaling shows no warning when minute is * in cron', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ export class Alarm extends AlarmBase {
value: props.threshold,
};

for (const w of this.metric.warnings ?? []) {
Annotations.of(this).addWarning(w);
for (const [i, message] of Object.entries(this.metric.warningsV2 ?? {})) {
Annotations.of(this).addWarningV2(i, message);
}
}

Expand Down
11 changes: 8 additions & 3 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,14 @@ export class Dashboard extends Resource {
return;
}

const warnings = allWidgetsDeep(widgets).flatMap(w => w.warnings ?? []);
for (const w of warnings) {
Annotations.of(this).addWarning(w);
const warnings = allWidgetsDeep(widgets).reduce((prev, curr) => {
return {
...prev,
...curr.warningsV2,
};
}, {} as { [id: string]: string });
for (const [id, message] of Object.entries(warnings ?? {})) {
Annotations.of(this).addWarningV2(id, message);
}

const w = widgets.length > 1 ? new Row(...widgets) : widgets[0];
Expand Down
10 changes: 10 additions & 0 deletions packages/aws-cdk-lib/aws-cloudwatch/lib/metric-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@ export interface IMetric {
* Should be attached to the consuming construct.
*
* @default - None
* @deprecated - use warningsV2
*/
readonly warnings?: string[];

/**
* Any warnings related to this metric
*
* Should be attached to the consuming construct.
*
* @default - None
*/
readonly warningsV2?: { [id: string]: string };

/**
* Inspect the details of the metric object
*/
Expand Down
Loading

0 comments on commit afdac07

Please sign in to comment.