feat(core): acknowledge warnings (#26144)

This PR adds the ability to acknowledge annotation warning messages. 

The main motivation behind this is to allow people to set the `--strict` mode to fail synthesis on warnings. Currently it is all or nothing, you have to get rid of _all_ warnings to use `--strict`. With this feature users will be able to `acknowledge` warnings saying that they are aware, but it does not apply to them.

Since we want all warnings to now have an id this will deprecate the `addWarning` method and adds a new `addWarningV2` method.

Since the acknowledgements and warnings are written as metadata, it is possible to enhance this in the future to report on warnings and acknowledgements.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: corymhall

packages/@aws-cdk/aws-gamelift-alpha/lib/fleet-base.ts

@@ -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'));
   }
-}
+}

packages/@aws-cdk/aws-servicecatalogappregistry-alpha/lib/aspects/stack-associator.ts

@@ -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.');
         }
       }
@@ -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);
   }
 
   /**
@@ -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);
     }
   }
@@ -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;
     }
 
@@ -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;
       }
     }

packages/@aws-cdk/aws-servicecatalogappregistry-alpha/test/application-associator.test.ts

@@ -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', () => {
@@ -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', () => {
@@ -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', () => {
@@ -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', () => {

packages/@aws-cdk/aws-servicecatalogappregistry-alpha/test/application.test.ts

@@ -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]');
   });
 });
 

packages/aws-cdk-lib/assertions/test/annotations.test.ts

@@ -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', () => {
@@ -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:/);
     });
   });
@@ -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 {
@@ -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 {
@@ -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);
   }
-}
+}

packages/aws-cdk-lib/aws-apigateway/lib/method.ts

@@ -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);
   }
@@ -512,4 +512,4 @@ export enum AuthorizationType {
 
 function pathForArn(path: string): string {
   return path.replace(/\{[^\}]*\}/g, '*'); // replace path parameters (like '{bookId}') with asterisk
-}
+}

packages/aws-cdk-lib/aws-applicationautoscaling/lib/schedule.ts

@@ -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);
       }

packages/aws-cdk-lib/aws-autoscaling/lib/aspects/require-imdsv2-aspect.ts

@@ -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}`);
   }
 }

packages/aws-cdk-lib/aws-autoscaling/lib/auto-scaling-group.ts

@@ -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;
@@ -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');
       }
     }
 

packages/aws-cdk-lib/aws-autoscaling/lib/schedule.ts

@@ -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);
       }

packages/aws-cdk-lib/aws-autoscaling/test/auto-scaling-group.test.ts

@@ -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', () => {
@@ -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', () => {

packages/aws-cdk-lib/aws-autoscaling/test/scheduled-action.test.ts

@@ -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', () => {

packages/aws-cdk-lib/aws-cloudwatch/lib/alarm.ts

@@ -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);
     }
   }
 

packages/aws-cdk-lib/aws-cloudwatch/lib/dashboard.ts

@@ -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];

packages/aws-cdk-lib/aws-cloudwatch/lib/metric-types.ts

@@ -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
    */