(core): addUnacknowledgeableWarning() method for Annotations
- Dominant language
- TypeScript
- Stars
- 12.9k
- Forks
- 4.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 83
Description
### Describe the feature
Exactly the [addWarning](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.Annotations.html#addwbrwarningmessagespan-classapi-icon-api-icon-deprecated-titlethis-api-element-is-deprecated-its-use-is-not-recommended%EF%B8%8Fspan) method, but not deprecated
### Use Case
[cdk-nag](https://github.com/cdklabs/cdk-nag) uses the cdk Annotation system to emit `Warnings` and `Errors`. Currently it uses the [addWarning](https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.Annotations.html#addwbrwarningmessagespan-classapi-icon-api-icon-deprecated-titlethis-api-element-is-deprecated-its-use-is-not-recommended%EF%B8%8Fspan) method for `Warnings`. `addWarning` was deprecated with the [introduction of `addWarningV2`](https://github.com/aws/aws-cdk/pull/26144) in the latest cdk release. All `cdk-nag` Warnings now emit deprecation messages along with the warning, which adds a lot of clutter the the CLI output. While this could be fixed by switching to `addWarningV2`, I don't think that `addWarningV2` is the correct choice in this case, primarily due to how the `cdk-nag` warning/error suppression system is intended to function.
#### Primary Concern `acknowledgeWarning` vs `NagSuppression`
[`NagSuppression`s](https://github.com/cdklabs/cdk-nag#suppressing-a-rule) are `cdk-nag`'s way of acknowledging warnings and errors. The key difference between `acknowledgeWarning` and `NagSuppression`s are that `NagSuppresion`s write the suppression to the CloudFormation Metadata in a `cdk-nag` [specific format](https://github.com/cdklabs/cdk-nag#using-on-cloudformation-templates). That Cfn metadata is important when using SAST tooling to look over the generated CloudFormation templates (Anecdotally, I know of several use cases where the `cdk-nag` Metadata is reviewed in CI/CD pipelines). If `cdk-nag` were to switch from `addWarning` to `addWarningV2` users would be able to use both `acknowledgeWarning` and `NagSuppresion`s to silence warnings. While technically not a breaking change giving users the ability to use `acknowledgeWarning` for `cdk-nag` isn't a good choice.
#### Smaller Concerns/Annoyances with `addWarningV2` and `cdk-nag`
if `cdk-nag` were to switch to `addWarningV2` and we were to ignore the primary concern there would be a few other minor annoyances.
1. The CLI output of `warnings` and `errors` would be slightly different from one another. `Warnings` would have an additional `addWarningV2` identifier appended to the end of the message, while `errors` would not. While that would be the existing `RuleId` already used for `cdk-nag`, `errors` don't have a corresponding `addErrorV2` (and in my opinion it doesn't make sense to have this) so this adds a bit of clutter and confusion
2. Since there is no `addErrorV2` method there would a delta between the options to acknowledge/suppress `Warnings` and `Errors`
### Proposed Solution
I don't think that "undeprecating" `addWarning` is the correct solution, because in cdk internal uses cases`addWarningV2` is the correct choice and users should use `addWarningV2`. Having a clone of `addWarning` with a better name would add some "guardrails"(?) on usage
1. Add an `addUnacknowledgeableWarning()` method to `Annotations` which is a clone of of the current `addWarning` method
2. Put a disclaimer message message that users should likely be using `addWarningV2` unless they have a specific use case where the warnings should not be acknowledgeable and that unhandled warnings will prohibit users from deploying cdk applications with `strict` mode.
### Other Information
Related to https://github.com/cdklabs/cdk-nag/issues/1418
### Acknowledgements
- [X] I may be able to implement this feature request
- [ ] This feature might incur a breaking change
### CDK version used
2.93.0
### Environment details (OS name and version, etc.)
Not relevant, but macOs 13.4 (Ventura) 😄
Contributor guide
Research direction
Start at the Annotations entry point and compare the existing addWarning and addWarningV2 APIs. Done means a non-deprecated addUnacknowledgeableWarning method mirrors addWarning, includes the proposed disclaimer, and preserves the stated acknowledgment behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cloud, infrastructure
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100