From 29df38d9cb61913c2ae9f11d17eec28d3795d668 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 29 Nov 2021 11:15:02 +0000 Subject: [PATCH 01/13] RFC 308: CLI advisories --- text/0308-cli-advisories.md | 182 ++++++++++++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 text/0308-cli-advisories.md diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md new file mode 100644 index 000000000..699ecf762 --- /dev/null +++ b/text/0308-cli-advisories.md @@ -0,0 +1,182 @@ +# CLI advisories RFC (draft) + +A new CLI feature to notify customers about urgent and important issues that require their attention. + +## Working backwards + +Starting on version x.y.z of the CDK CLI, customers will be notified, on every command, about security vulnerabilities, regressions and usage of unsupported versions: + +``` +$ cdk deploy + +... # Normal output of the command + +ADVISORIES + +16603 Toggling off auto_delete_objects for Bucket empties the bucket + + Summary: If a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. + + Affected versions: <1.126.0. + + More information at: https://github.com/aws/aws-cdk/issues/16603 + + +17061 Error when building EKS cluster with monocdk import + + Summary: When using monocdk/aws-eks to build a stack containing + an EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. + + Affected versions: >=1.126.0 <=1.130.0. + + More information at: https://github.com/aws/aws-cdk/issues/17061 + +If you don’t want to see an advisory anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". +``` + +By acknowledging a particular advisory, it won’t show anymore in subsequent calls: + +``` +$ cdk acknowledge 16603 + +ADVISORIES + +17061 Error when building EKS cluster with monocdk import + + Summary: When using monocdk/aws-eks to build a stack containing + an EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. + + Affected versions: >=1.126.0 <=1.130.0. + + More information at: https://github.com/aws/aws-cdk/issues/17061 + +If you don’t want to see an advisory anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 17061". +``` + +You can suppress all warnings per individual execution: + +``` +$ cdk deploy --no-advisories +``` + +And you can disable all advisories indefinitely by adding this entry to `~/.cdk.json`: + +``` +"supressAllAdvisories": true +``` + +Regardless of the state of this flag and the advisories you have acknowledged, you can always show the currently active advisories: + +``` +$ cdk advisories +``` + +This command returns zero if there is no advisory and non-zero otherwise. Users can then plug this into a pipeline approval workflow and expect manual review if there are any advisories. + +## Public FAQ + +### What are we launching today? + +A new communication channel between AWS and users of the CDK. Starting on version x.y.z of the CDK CLI, customers will be notified, on every command, about security vulnerabilities, regressions and usage of unsupported versions. + +### Why should I use this feature? + +These advisories shown by the CLI contain very important and actionable information about problems that directly affect you. They give you an opportunity to upgrade the CLI or the construct library when necessary or to work around high-impacting issues. + +## Internal FAQ + +### Why are we doing this? + +In case of emergency announcements, such as security vulnerabilities or regressions, the only mechanisms we have right now are email campaigns and pinned GitHub issues. These are not always the best means to reach out to customers. Many email addresses are not monitored by anyone and customers don’t necessarily check the GitHub repository for updates. The output of the CLI, on the other hand, is seen by many customers every day. + +### Why should we *not* do this? + +This is a powerful feature to convey urgent and important messages to customers. However, to keep its relevance, it should be used sparingly and the messages must meet a high bar. Otherwise it will just become a tool for spamming customers. We will introduce filters to reduce this risk, by only showing content that applies to each particular environment. But ultimately, it hinges on responsible use. In particular, this feature should not be used for things like marketing campaigns, blog post announcements or upcoming events. If we don’t have a good mechanism for defining and enforcing this constraint, we should not implement this feature. + +### What is the technical solution (design) of this feature? + +To publish a new advisory, all that the CDK team will have to do is create a GitHub issue that fulfils some requirements. The CLI will consume these issues using the GitHub API and apply a set of filters to narrow down the list of advisories to the context in which it is being run. To improve the publishing experience, we will implement a GitHub action that will validate whether the issues that are candidates for advisories fulfil the requirements. For a more detailed explanation, see the Appendix. + +### Is this a breaking change? + +No. + +### What alternative solutions did you consider? + +All the solutions listed below were considered and discarded for adding unnecessary complexity to the overall architecture. + +On the publishing side: + +* Implementing a new internal REST service to manage the advisories. +* Storing the advisories as structured data (e.g., JSON files) in some GitHub repository. + +On the distribution side: + +* Assuming the advisories were stored in an S3 file (using one of the publishing solutions considered above), we would use CloudFront to distribute them. +* Use GitHub pages as our distribution mechanism, directly from the repository were the data were stored. + +### What is the high-level project plan? + +1. Implement the issue validation logic as an NPM package. +2. Implement the GitHub action, using the validation package. +3. Implement the CLI changes. +4. Add the GitHub action to the aws-cdk repository. + +### What is the expected lifetime of advisories? + +We should expect advisories to be available for months. This is usually the case when we want users to upgrade the construct library version. + +### Why do we need to give users the option to suppress advisories? + +This is useful when the CDK is running as part of larger script and users want to hide the output. + +### Why do we need to give users the option to acknowledge advisories? + +Given the expected lifetime of advisories, they will eventually become just noise and users will try to silence it. If the only option we give users is to suppress them (per execution or indefinitely), they will use that and miss new advisories. + +### What future improvements can we make? + +**Resource filter**: Ideally, advisories should be as targeted as possible. The version filter addresses part of that requirement, but we can make it better by also adding a resource filter: if an issue only affects `FargateTaskDefinition` resources, for example, but this resource is not present in any of the stacks, the customer doesn’t need to see that particular warning. + +**Pipeline awareness**: Initially, we won’t treat the pipeline as a special case. By default, the advisories will be fetched and displayed and we expect users to never look at them. This behavior can always be overridden by suppressing the advisories. In future releases, the CLI will use the information about whether it’s running on a pipeline and block the deployment in case there is any advisory. + +## Appendix + +### Detailed design + +There are three parts to this solution: the data format, the GitHub workflow and the CLI logic. + +#### Data format + +We will use GitHub issues to publish and manage advisories. To be considered a valid advisory, the body of a GitHub issue must have the following fields: + +| Field | Description | Format | Mandatory? | +| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ---------- | +| Summary | A paragraph with more information | Free-form text | Yes | +| Affected component | The CLI or the construct library (or both) | A non-empty subset of {CLI, Framework} | Yes | +| Affected version range | Version range using the semver format | semver | No | +| Suggested fix | An action the user can take to mitigate the problem. If it requires a longer explanation, this field should be omitted in favor of the resource linked to. | Free-form text | No | + +In order to keep the issue human readable as well as easily parseable, each of these fields should be indicated by a Markdown header (`### Summary`, `### Affected component` etc). The piece of text following the header will be interpreted as the content of the field. + +#### GitHub workflow + +To be considered an advisory, a GitHub issue must be pinned and have the following tags: `p0` and a new tag, `advisory`. The `advisory` tag will work as an acknowledgement that author of the issue is making a conscious decision to publish an advisory that will be seen by potentially hundreds of thousands of users. Adding this tag will trigger a new workflow, which will inspect the issue and check whether its body contains the right data format (see **Data format** section). If the issue doesn’t comply with the specification, the workflow will post a message (similar to a CI report, for example). + +#### CLI logic + +The CLI will make unauthenticated requests to the GitHub Issues API and query for issues that match the tag system described in the **GitHub workflow** section. Then it will apply additional filters on the result: + +* Format filter: whether the body of the issue complies with the data format specification. +* Version filter: whether the version range contained in the issue matches the CLI version or the framework version (depending on the affected component, also present in the issue). To check which version of the framework is being used, the CLI will read the cloud assembly (`tree.json` file, specifically). + +Issues that pass these filters will be displayed on the standard output. + +The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated requests. To avoid hitting that limit, the CLI will cache the results for a set period of time (provisionally defined as 1 hour). The filtered advisories and the expiration time will be saved to a file in the `$HOME/.cdk` folder. When the expiration time is reached, the cache is considered invalid and, at the next command execution, the CLI will make a new request to get fresh results. + +The CLI will also store the IDs of the acknowledged issues in a file in the `$HOME/.cdk` folder. From 2d996012ad5c72d01b858f739a26427004de991f Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 29 Nov 2021 11:24:14 +0000 Subject: [PATCH 02/13] Added metadata --- text/0308-cli-advisories.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 699ecf762..8fe35dc03 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -1,4 +1,8 @@ -# CLI advisories RFC (draft) +# CLI advisories + +* **Original Author(s)**: [@otaviomacedo](https://github.com/otaviomacedo) +* **Tracking Issue**: [#308](https://github.com/aws/aws-cdk-rfcs/issues/308) +* **API Bar Raiser**: @{BAR_RAISER_USER} A new CLI feature to notify customers about urgent and important issues that require their attention. @@ -78,6 +82,10 @@ $ cdk advisories This command returns zero if there is no advisory and non-zero otherwise. Users can then plug this into a pipeline approval workflow and expect manual review if there are any advisories. +``` +[ ] Signed-off by API Bar Raiser @xxxxx +``` + ## Public FAQ ### What are we launching today? @@ -177,6 +185,6 @@ The CLI will make unauthenticated requests to the GitHub Issues API and query fo Issues that pass these filters will be displayed on the standard output. -The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated requests. To avoid hitting that limit, the CLI will cache the results for a set period of time (provisionally defined as 1 hour). The filtered advisories and the expiration time will be saved to a file in the `$HOME/.cdk` folder. When the expiration time is reached, the cache is considered invalid and, at the next command execution, the CLI will make a new request to get fresh results. +The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated requests. To avoid hitting that limit, the CLI will cache the results for a set period of time (provisionally defined as 1 hour). The filtered advisories and the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. When the expiration time is reached, the cache is considered invalid and, at the next command execution, the CLI will make a new request to get fresh results. The CLI will also store the IDs of the acknowledged issues in a file in the `$HOME/.cdk` folder. From a2cbe860d4b661a33161bac2f3cf06a6aba732e5 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 29 Nov 2021 11:33:42 +0000 Subject: [PATCH 03/13] Lines wrapped --- text/0308-cli-advisories.md | 129 ++++++++++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 28 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 8fe35dc03..273bf31cc 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -4,11 +4,14 @@ * **Tracking Issue**: [#308](https://github.com/aws/aws-cdk-rfcs/issues/308) * **API Bar Raiser**: @{BAR_RAISER_USER} -A new CLI feature to notify customers about urgent and important issues that require their attention. +A new CLI feature to notify customers about urgent and important issues that +require their attention. ## Working backwards -Starting on version x.y.z of the CDK CLI, customers will be notified, on every command, about security vulnerabilities, regressions and usage of unsupported versions: +Starting on version x.y.z of the CDK CLI, customers will be notified, on every +command, about security vulnerabilities, regressions and usage of unsupported +versions: ``` $ cdk deploy @@ -68,19 +71,23 @@ You can suppress all warnings per individual execution: $ cdk deploy --no-advisories ``` -And you can disable all advisories indefinitely by adding this entry to `~/.cdk.json`: +And you can disable all advisories indefinitely by adding this entry to +`~/.cdk.json`: ``` "supressAllAdvisories": true ``` -Regardless of the state of this flag and the advisories you have acknowledged, you can always show the currently active advisories: +Regardless of the state of this flag and the advisories you have acknowledged, +you can always show the currently active advisories: ``` $ cdk advisories ``` -This command returns zero if there is no advisory and non-zero otherwise. Users can then plug this into a pipeline approval workflow and expect manual review if there are any advisories. +This command returns zero if there is no advisory and non-zero otherwise. Users +can then plug this into a pipeline approval workflow and expect manual review if +there are any advisories. ``` [ ] Signed-off by API Bar Raiser @xxxxx @@ -90,25 +97,49 @@ This command returns zero if there is no advisory and non-zero otherwise. Users ### What are we launching today? -A new communication channel between AWS and users of the CDK. Starting on version x.y.z of the CDK CLI, customers will be notified, on every command, about security vulnerabilities, regressions and usage of unsupported versions. +A new communication channel between AWS and users of the CDK. Starting on +version x.y.z of the CDK CLI, customers will be notified, on every command, +about security vulnerabilities, regressions and usage of unsupported versions. ### Why should I use this feature? -These advisories shown by the CLI contain very important and actionable information about problems that directly affect you. They give you an opportunity to upgrade the CLI or the construct library when necessary or to work around high-impacting issues. +These advisories shown by the CLI contain very important and actionable +information about problems that directly affect you. They give you an +opportunity to upgrade the CLI or the construct library when necessary or to +work around high-impacting issues. ## Internal FAQ ### Why are we doing this? -In case of emergency announcements, such as security vulnerabilities or regressions, the only mechanisms we have right now are email campaigns and pinned GitHub issues. These are not always the best means to reach out to customers. Many email addresses are not monitored by anyone and customers don’t necessarily check the GitHub repository for updates. The output of the CLI, on the other hand, is seen by many customers every day. +In case of emergency announcements, such as security vulnerabilities or +regressions, the only mechanisms we have right now are email campaigns and +pinned GitHub issues. These are not always the best means to reach out to +customers. Many email addresses are not monitored by anyone and customers don’t +necessarily check the GitHub repository for updates. The output of the CLI, on +the other hand, is seen by many customers every day. ### Why should we *not* do this? -This is a powerful feature to convey urgent and important messages to customers. However, to keep its relevance, it should be used sparingly and the messages must meet a high bar. Otherwise it will just become a tool for spamming customers. We will introduce filters to reduce this risk, by only showing content that applies to each particular environment. But ultimately, it hinges on responsible use. In particular, this feature should not be used for things like marketing campaigns, blog post announcements or upcoming events. If we don’t have a good mechanism for defining and enforcing this constraint, we should not implement this feature. +This is a powerful feature to convey urgent and important messages to customers. +However, to keep its relevance, it should be used sparingly and the messages +must meet a high bar. Otherwise it will just become a tool for spamming +customers. We will introduce filters to reduce this risk, by only showing +content that applies to each particular environment. But ultimately, it hinges +on responsible use. In particular, this feature should not be used for things +like marketing campaigns, blog post announcements or upcoming events. If we +don’t have a good mechanism for defining and enforcing this constraint, we +should not implement this feature. ### What is the technical solution (design) of this feature? -To publish a new advisory, all that the CDK team will have to do is create a GitHub issue that fulfils some requirements. The CLI will consume these issues using the GitHub API and apply a set of filters to narrow down the list of advisories to the context in which it is being run. To improve the publishing experience, we will implement a GitHub action that will validate whether the issues that are candidates for advisories fulfil the requirements. For a more detailed explanation, see the Appendix. +To publish a new advisory, all that the CDK team will have to do is create a +GitHub issue that fulfils some requirements. The CLI will consume these issues +using the GitHub API and apply a set of filters to narrow down the list of +advisories to the context in which it is being run. To improve the publishing +experience, we will implement a GitHub action that will validate whether the +issues that are candidates for advisories fulfil the requirements. For a more +detailed explanation, see the Appendix. ### Is this a breaking change? @@ -116,17 +147,21 @@ No. ### What alternative solutions did you consider? -All the solutions listed below were considered and discarded for adding unnecessary complexity to the overall architecture. +All the solutions listed below were considered and discarded for adding +unnecessary complexity to the overall architecture. On the publishing side: * Implementing a new internal REST service to manage the advisories. -* Storing the advisories as structured data (e.g., JSON files) in some GitHub repository. +* Storing the advisories as structured data (e.g., JSON files) in some GitHub + repository. On the distribution side: -* Assuming the advisories were stored in an S3 file (using one of the publishing solutions considered above), we would use CloudFront to distribute them. -* Use GitHub pages as our distribution mechanism, directly from the repository were the data were stored. +* Assuming the advisories were stored in an S3 file (using one of the publishing + solutions considered above), we would use CloudFront to distribute them. +* Use GitHub pages as our distribution mechanism, directly from the repository + were the data were stored. ### What is the high-level project plan? @@ -137,31 +172,47 @@ On the distribution side: ### What is the expected lifetime of advisories? -We should expect advisories to be available for months. This is usually the case when we want users to upgrade the construct library version. +We should expect advisories to be available for months. This is usually the case +when we want users to upgrade the construct library version. ### Why do we need to give users the option to suppress advisories? -This is useful when the CDK is running as part of larger script and users want to hide the output. +This is useful when the CDK is running as part of larger script and users want +to hide the output. ### Why do we need to give users the option to acknowledge advisories? -Given the expected lifetime of advisories, they will eventually become just noise and users will try to silence it. If the only option we give users is to suppress them (per execution or indefinitely), they will use that and miss new advisories. +Given the expected lifetime of advisories, they will eventually become just +noise and users will try to silence it. If the only option we give users is to +suppress them (per execution or indefinitely), they will use that and miss new +advisories. ### What future improvements can we make? -**Resource filter**: Ideally, advisories should be as targeted as possible. The version filter addresses part of that requirement, but we can make it better by also adding a resource filter: if an issue only affects `FargateTaskDefinition` resources, for example, but this resource is not present in any of the stacks, the customer doesn’t need to see that particular warning. +**Resource filter**: Ideally, advisories should be as targeted as possible. The +version filter addresses part of that requirement, but we can make it better by +also adding a resource filter: if an issue only affects `FargateTaskDefinition` +resources, for example, but this resource is not present in any of the stacks, +the customer doesn’t need to see that particular warning. -**Pipeline awareness**: Initially, we won’t treat the pipeline as a special case. By default, the advisories will be fetched and displayed and we expect users to never look at them. This behavior can always be overridden by suppressing the advisories. In future releases, the CLI will use the information about whether it’s running on a pipeline and block the deployment in case there is any advisory. +**Pipeline awareness**: Initially, we won’t treat the pipeline as a special +case. By default, the advisories will be fetched and displayed and we expect +users to never look at them. This behavior can always be overridden by +suppressing the advisories. In future releases, the CLI will use the information +about whether it’s running on a pipeline and block the deployment in case there +is any advisory. ## Appendix ### Detailed design -There are three parts to this solution: the data format, the GitHub workflow and the CLI logic. +There are three parts to this solution: the data format, the GitHub workflow and +the CLI logic. #### Data format -We will use GitHub issues to publish and manage advisories. To be considered a valid advisory, the body of a GitHub issue must have the following fields: +We will use GitHub issues to publish and manage advisories. To be considered a +valid advisory, the body of a GitHub issue must have the following fields: | Field | Description | Format | Mandatory? | | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ---------- | @@ -170,21 +221,43 @@ We will use GitHub issues to publish and manage advisories. To be considered a v | Affected version range | Version range using the semver format | semver | No | | Suggested fix | An action the user can take to mitigate the problem. If it requires a longer explanation, this field should be omitted in favor of the resource linked to. | Free-form text | No | -In order to keep the issue human readable as well as easily parseable, each of these fields should be indicated by a Markdown header (`### Summary`, `### Affected component` etc). The piece of text following the header will be interpreted as the content of the field. +In order to keep the issue human readable as well as easily parseable, each of +these fields should be indicated by a Markdown header (`### Summary`, `### +Affected component` etc). The piece of text following the header will be +interpreted as the content of the field. #### GitHub workflow -To be considered an advisory, a GitHub issue must be pinned and have the following tags: `p0` and a new tag, `advisory`. The `advisory` tag will work as an acknowledgement that author of the issue is making a conscious decision to publish an advisory that will be seen by potentially hundreds of thousands of users. Adding this tag will trigger a new workflow, which will inspect the issue and check whether its body contains the right data format (see **Data format** section). If the issue doesn’t comply with the specification, the workflow will post a message (similar to a CI report, for example). +To be considered an advisory, a GitHub issue must be pinned and have the +following tags: `p0` and a new tag, `advisory`. The `advisory` tag will work as +an acknowledgement that author of the issue is making a conscious decision to +publish an advisory that will be seen by potentially hundreds of thousands of +users. Adding this tag will trigger a new workflow, which will inspect the issue +and check whether its body contains the right data format (see **Data format** +section). If the issue doesn’t comply with the specification, the workflow will +post a message (similar to a CI report, for example). #### CLI logic -The CLI will make unauthenticated requests to the GitHub Issues API and query for issues that match the tag system described in the **GitHub workflow** section. Then it will apply additional filters on the result: +The CLI will make unauthenticated requests to the GitHub Issues API and query +for issues that match the tag system described in the **GitHub workflow** +section. Then it will apply additional filters on the result: -* Format filter: whether the body of the issue complies with the data format specification. -* Version filter: whether the version range contained in the issue matches the CLI version or the framework version (depending on the affected component, also present in the issue). To check which version of the framework is being used, the CLI will read the cloud assembly (`tree.json` file, specifically). +* Format filter: whether the body of the issue complies with the data format + specification. +* Version filter: whether the version range contained in the issue matches the + CLI version or the framework version (depending on the affected component, + also present in the issue). To check which version of the framework is being + used, the CLI will read the cloud assembly (`tree.json` file, specifically). Issues that pass these filters will be displayed on the standard output. -The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated requests. To avoid hitting that limit, the CLI will cache the results for a set period of time (provisionally defined as 1 hour). The filtered advisories and the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. When the expiration time is reached, the cache is considered invalid and, at the next command execution, the CLI will make a new request to get fresh results. +The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated +requests. To avoid hitting that limit, the CLI will cache the results for a set +period of time (provisionally defined as 1 hour). The filtered advisories and +the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. +When the expiration time is reached, the cache is considered invalid and, at the +next command execution, the CLI will make a new request to get fresh results. -The CLI will also store the IDs of the acknowledged issues in a file in the `$HOME/.cdk` folder. +The CLI will also store the IDs of the acknowledged issues in a file in the +`$HOME/.cdk` folder. From 6731d72840c2bd74f2360ec9c18a0234beabb59a Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 29 Nov 2021 14:04:54 +0000 Subject: [PATCH 04/13] Addressed comments --- text/0308-cli-advisories.md | 83 +++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 273bf31cc..aff6d75ca 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -9,6 +9,8 @@ require their attention. ## Working backwards +### README + Starting on version x.y.z of the CDK CLI, customers will be notified, on every command, about security vulnerabilities, regressions and usage of unsupported versions: @@ -75,7 +77,7 @@ And you can disable all advisories indefinitely by adding this entry to `~/.cdk.json`: ``` -"supressAllAdvisories": true +"advisories": false ``` Regardless of the state of this flag and the advisories you have acknowledged, @@ -89,6 +91,72 @@ This command returns zero if there is no advisory and non-zero otherwise. Users can then plug this into a pipeline approval workflow and expect manual review if there are any advisories. +> Please note that the acknowledgements are made project by project. If you +acknowledge an advisory in one CDK project, it will still appear on other +projects when you run any CDK commands, unless you have suppressed or disabled +advisories. + +### Runbook section (internal to the CDK team) + +In case of a high-impact issue, follow these steps: +1. Create or update an issue for this incident on the aws-cdk GitHub repository. +2. If there is an error message related to issue, include it in the title so +that it appears in web searches. +3. In the body of the issue, use the following template: + +```markdown +Please add your +1 👍 to let us know you have encountered this +--- + +### Status: ____ + +### Overview + +#### Complete Error Message + +### Workaround + + +### Solution + + +### Related Issues + + +### Affected component + + + +### Affected version range + + +``` +4. Remove the `needs-triage` label. +5. Add the labels `p0`, `management/tracking` and all relevant + subject/package labels. +6. Pin the issue. +7. Add the `advisory` label. If you followed the previous steps correctly, CLI + users will start seeing this information as part of the output of every + command. If you missed anything, a GitHub action will post a warning on the + issue. + ``` [ ] Signed-off by API Bar Raiser @xxxxx ``` @@ -216,15 +284,16 @@ valid advisory, the body of a GitHub issue must have the following fields: | Field | Description | Format | Mandatory? | | ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ---------- | -| Summary | A paragraph with more information | Free-form text | Yes | +| Overview | A paragraph with more information | Free-form text | Yes | | Affected component | The CLI or the construct library (or both) | A non-empty subset of {CLI, Framework} | Yes | | Affected version range | Version range using the semver format | semver | No | -| Suggested fix | An action the user can take to mitigate the problem. If it requires a longer explanation, this field should be omitted in favor of the resource linked to. | Free-form text | No | +| Workaround | Actions the user can take to mitigate the problem. | Free-form text | No | In order to keep the issue human readable as well as easily parseable, each of -these fields should be indicated by a Markdown header (`### Summary`, `### +these fields should be indicated by a Markdown header (`### Overview`, `### Affected component` etc). The piece of text following the header will be -interpreted as the content of the field. +interpreted as the content of the field. See the template in the **Working +backwards** section. #### GitHub workflow @@ -259,5 +328,5 @@ the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. When the expiration time is reached, the cache is considered invalid and, at the next command execution, the CLI will make a new request to get fresh results. -The CLI will also store the IDs of the acknowledged issues in a file in the -`$HOME/.cdk` folder. +The CLI will store the IDs of the acknowledged issues in the project specific +`./cdk.json` file. From eebd9df508ef42a5b0b8ff5c751febf6f8abe8f3 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 7 Dec 2021 14:22:53 +0000 Subject: [PATCH 05/13] Description of the behavior in case of errors --- text/0308-cli-advisories.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index aff6d75ca..90fc555e7 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -326,7 +326,10 @@ requests. To avoid hitting that limit, the CLI will cache the results for a set period of time (provisionally defined as 1 hour). The filtered advisories and the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. When the expiration time is reached, the cache is considered invalid and, at the -next command execution, the CLI will make a new request to get fresh results. +next command execution, the CLI will make a new request to get fresh results. + +If an error or timeout occurs when retrieving the issues, the CLI will simply +skip the display of advisories and try again at the next command execution. The CLI will store the IDs of the acknowledged issues in the project specific `./cdk.json` file. From ee97872079acacc77b60cdf5fcd9314021cd0037 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Thu, 9 Dec 2021 16:02:19 +0000 Subject: [PATCH 06/13] Design change: storing the advisories in JSON files --- text/0308-cli-advisories.md | 220 +++++++++++++----------------------- 1 file changed, 81 insertions(+), 139 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 90fc555e7..dcad74d25 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -24,10 +24,10 @@ ADVISORIES 16603 Toggling off auto_delete_objects for Bucket empties the bucket - Summary: If a stack is deployed with an S3 bucket with - auto_delete_objects=True, and then re-deployed with - auto_delete_objects=False, all the objects in the bucket - will be deleted. + Overview: If a stack is deployed with an S3 bucket with + auto_delete_objects=True, and then re-deployed with + auto_delete_objects=False, all the objects in the bucket + will be deleted. Affected versions: <1.126.0. @@ -36,9 +36,9 @@ ADVISORIES 17061 Error when building EKS cluster with monocdk import - Summary: When using monocdk/aws-eks to build a stack containing - an EKS cluster, error is thrown about missing - lambda-layer-node-proxy-agent/layer/package.json. + Overview: When using monocdk/aws-eks to build a stack containing + an EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. Affected versions: >=1.126.0 <=1.130.0. @@ -56,9 +56,9 @@ ADVISORIES 17061 Error when building EKS cluster with monocdk import - Summary: When using monocdk/aws-eks to build a stack containing - an EKS cluster, error is thrown about missing - lambda-layer-node-proxy-agent/layer/package.json. + Overview: When using monocdk/aws-eks to build a stack containing + an EKS cluster, error is thrown about missing + lambda-layer-node-proxy-agent/layer/package.json. Affected versions: >=1.126.0 <=1.130.0. @@ -99,67 +99,29 @@ advisories. ### Runbook section (internal to the CDK team) In case of a high-impact issue, follow these steps: + 1. Create or update an issue for this incident on the aws-cdk GitHub repository. -2. If there is an error message related to issue, include it in the title so -that it appears in web searches. -3. In the body of the issue, use the following template: - -```markdown -Please add your +1 👍 to let us know you have encountered this ---- - -### Status: ____ - -### Overview - -#### Complete Error Message - -### Workaround - - -### Solution - - -### Related Issues - - -### Affected component - - - -### Affected version range - - +2. Update the file `advisories.json` on repository _TBD_, adding an entry for + the incident. Example: +```json + { + "title": "Toggling off auto_delete_objects for Bucket empties the bucket", + "issueUrl": "https://github.com/aws/aws-cdk/issues/16603", + "overview": "If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted.", + "components": ["framework"], + "version": "<1.126.0" + } ``` -4. Remove the `needs-triage` label. -5. Add the labels `p0`, `management/tracking` and all relevant - subject/package labels. -6. Pin the issue. -7. Add the `advisory` label. If you followed the previous steps correctly, CLI - users will start seeing this information as part of the output of every - command. If you missed anything, a GitHub action will post a warning on the - issue. -``` +3. Create a PR with this change and wait for an approval. When the PR gets + merged, the advisory will be visible to all CLI installations. The GitHub + issue will also be automatically updated with the information contained in + the file. All the necessary tags will also be added automatically. +4. You can keep updating the issue normally, as new information comes in, but + you're not allowed to touch the sections auto-generated from the advisories + file. + [ ] Signed-off by API Bar Raiser @xxxxx -``` ## Public FAQ @@ -193,21 +155,20 @@ This is a powerful feature to convey urgent and important messages to customers. However, to keep its relevance, it should be used sparingly and the messages must meet a high bar. Otherwise it will just become a tool for spamming customers. We will introduce filters to reduce this risk, by only showing -content that applies to each particular environment. But ultimately, it hinges -on responsible use. In particular, this feature should not be used for things -like marketing campaigns, blog post announcements or upcoming events. If we -don’t have a good mechanism for defining and enforcing this constraint, we -should not implement this feature. +content that applies to each particular environment and also require PR approval +for any changes in advisories. But ultimately, it hinges on responsible use. In +particular, this feature should not be used for things like marketing campaigns, +blog post announcements and things like that. If the mechanisms proposed in this +RFC are not considered strong enough by the CDK team, we should not implement +this feature. ### What is the technical solution (design) of this feature? -To publish a new advisory, all that the CDK team will have to do is create a -GitHub issue that fulfils some requirements. The CLI will consume these issues -using the GitHub API and apply a set of filters to narrow down the list of -advisories to the context in which it is being run. To improve the publishing -experience, we will implement a GitHub action that will validate whether the -issues that are candidates for advisories fulfil the requirements. For a more -detailed explanation, see the Appendix. +Advisory information will be available as a static file, served from GitHub. +The CLI will consume this file from and apply a set of filters to narrow down +the list of advisories to the context in which it is being run. We will also +implement some GitHub actions to validate and copy the contents of the file over +to the issue. For a more detailed explanation, see the Appendix. ### Is this a breaking change? @@ -215,28 +176,26 @@ No. ### What alternative solutions did you consider? -All the solutions listed below were considered and discarded for adding -unnecessary complexity to the overall architecture. - On the publishing side: -* Implementing a new internal REST service to manage the advisories. -* Storing the advisories as structured data (e.g., JSON files) in some GitHub - repository. +* Implementing a new internal REST service to manage the advisories. Overly + complex for this use case. +* Authoring the content directly on the GitHub issue. There is good way to + implement an approval workflow that includes a human verification step. On the distribution side: -* Assuming the advisories were stored in an S3 file (using one of the publishing - solutions considered above), we would use CloudFront to distribute them. -* Use GitHub pages as our distribution mechanism, directly from the repository - were the data were stored. +* Assuming the advisories were stored in an S3 file (in case of the REST + service), we would use CloudFront to distribute them. +* Using the GitHub API to query for special issues that are considered + advisories (in case of using GitHub issues as the source of truth). ### What is the high-level project plan? -1. Implement the issue validation logic as an NPM package. -2. Implement the GitHub action, using the validation package. -3. Implement the CLI changes. -4. Add the GitHub action to the aws-cdk repository. +1. Implement the GitHub actions of validation and issue sync-up and issue + protection. +2. Add the construct library version to the cloud assembly metadata. +2. Implement and release the CLI changes. ### What is the expected lifetime of advisories? @@ -274,62 +233,45 @@ is any advisory. ### Detailed design -There are three parts to this solution: the data format, the GitHub workflow and -the CLI logic. - -#### Data format +#### Publishing advisories -We will use GitHub issues to publish and manage advisories. To be considered a -valid advisory, the body of a GitHub issue must have the following fields: +We will create a new repository, dedicated to host the advisories file. As +usual, any change to this file will have to be published as a PR and approved to +be merged. The file will contain a list of advisories, each having the following +fields: -| Field | Description | Format | Mandatory? | -| ---------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | ---------- | -| Overview | A paragraph with more information | Free-form text | Yes | -| Affected component | The CLI or the construct library (or both) | A non-empty subset of {CLI, Framework} | Yes | -| Affected version range | Version range using the semver format | semver | No | -| Workaround | Actions the user can take to mitigate the problem. | Free-form text | No | +| Field | Description | Format | Mandatory? | +|:------------:|:--------------------------------------------------------------:|---------------------------------|:----------:| +| `title` | The title of the incident | Free form text | Yes | +| `issueUrl` | A link to the GitHub issue where the incident is being tracked | URL | Yes | +| `overview` | A paragraph with more information about the incident | Free form text | Yes | +| `components` | The CLI or the Framework | Either `"cli"` or `"framework"` | Yes | +| `version` | Version range using the semver format | Semantic Versioning | No | -In order to keep the issue human readable as well as easily parseable, each of -these fields should be indicated by a Markdown header (`### Overview`, `### -Affected component` etc). The piece of text following the header will be -interpreted as the content of the field. See the template in the **Working -backwards** section. -#### GitHub workflow +We will also need to implement three GitHub actions on this repository: -To be considered an advisory, a GitHub issue must be pinned and have the -following tags: `p0` and a new tag, `advisory`. The `advisory` tag will work as -an acknowledgement that author of the issue is making a conscious decision to -publish an advisory that will be seen by potentially hundreds of thousands of -users. Adding this tag will trigger a new workflow, which will inspect the issue -and check whether its body contains the right data format (see **Data format** -section). If the issue doesn’t comply with the specification, the workflow will -post a message (similar to a CI report, for example). +1. File validation on PR. It will block merging if the structure of the file is + not compliant with the rules. +2. Issue sync-up. When the PR is merged, this action will copy the content of + the file over to the GitHub issue it's linked to. +3. Issue protection. Every change to issues that are linked to advisories will + be checked by this action, to avoid corruption. #### CLI logic -The CLI will make unauthenticated requests to the GitHub Issues API and query -for issues that match the tag system described in the **GitHub workflow** -section. Then it will apply additional filters on the result: - -* Format filter: whether the body of the issue complies with the data format - specification. -* Version filter: whether the version range contained in the issue matches the - CLI version or the framework version (depending on the affected component, - also present in the issue). To check which version of the framework is being - used, the CLI will read the cloud assembly (`tree.json` file, specifically). - -Issues that pass these filters will be displayed on the standard output. +The CLI will fetch the file from GitHub, parse the content and check whether the +version range contained in the advisory matches the CLI version or the framework +version (depending on the affected component, also present in the advisory). -The GitHub API imposes a rate limit of 60 requests per hour for unauthenticated -requests. To avoid hitting that limit, the CLI will cache the results for a set -period of time (provisionally defined as 1 hour). The filtered advisories and -the expiration time will be saved to a file in the `$HOME/.cdk/cache` folder. -When the expiration time is reached, the cache is considered invalid and, at the -next command execution, the CLI will make a new request to get fresh results. +Since the CLI knows its own version, checking against the version range of the +advisory is trivial. The version of the framework, however, is not readily +available anywhere. To address this, we will start writing the framework version +to the Cloud Assembly, in a place where the CLI can read it. -If an error or timeout occurs when retrieving the issues, the CLI will simply -skip the display of advisories and try again at the next command execution. +Issues that pass this filter will be displayed on the standard output. If an +error or timeout occurs when retrieving the issues, the CLI will simply skip +the display of advisories and try again at the next command execution. The CLI will store the IDs of the acknowledged issues in the project specific `./cdk.json` file. From dc78763d2340cbbd5d3d74dcc82b30d6eb65e851 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 13 Dec 2021 09:57:28 +0000 Subject: [PATCH 07/13] Update text/0308-cli-advisories.md Co-authored-by: Elad Ben-Israel --- text/0308-cli-advisories.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index dcad74d25..2b2dd06b7 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -101,7 +101,7 @@ advisories. In case of a high-impact issue, follow these steps: 1. Create or update an issue for this incident on the aws-cdk GitHub repository. -2. Update the file `advisories.json` on repository _TBD_, adding an entry for +2. Update the file `advisories.json` on repository `cdklabs/aws-cdk-advisories`, adding an entry for the incident. Example: ```json { From c69faed32c2018c80ca67e17fb99841957cd225e Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 13 Dec 2021 10:05:18 +0000 Subject: [PATCH 08/13] Added CODEOWNERS idea --- text/0308-cli-advisories.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 2b2dd06b7..deadb9560 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -113,11 +113,13 @@ In case of a high-impact issue, follow these steps: } ``` -3. Create a PR with this change and wait for an approval. When the PR gets - merged, the advisory will be visible to all CLI installations. The GitHub - issue will also be automatically updated with the information contained in - the file. All the necessary tags will also be added automatically. -4. You can keep updating the issue normally, as new information comes in, but +3. Create a PR with this change and wait for an approval. Only SDMs have + permission to approve PRs in this repository. +4. When the PR gets merged, the advisory will be visible to all CLI + installations. The GitHub issue will also be automatically updated with the + information contained in the file. All the necessary tags will also be added + automatically. +5. You can keep updating the issue normally, as new information comes in, but you're not allowed to touch the sections auto-generated from the advisories file. @@ -235,7 +237,8 @@ is any advisory. #### Publishing advisories -We will create a new repository, dedicated to host the advisories file. As +We will create a new repository, dedicated to host the advisories file. Using a +CODEOWNERS file, we will restrict the permission to approve PRs only to SDMs. As usual, any change to this file will have to be published as a PR and approved to be merged. The file will contain a list of advisories, each having the following fields: From 1ae9cf89badadbb432e6c95e1aed1f8c9024fe09 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Mon, 13 Dec 2021 10:26:08 +0000 Subject: [PATCH 09/13] components -> component --- text/0308-cli-advisories.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index deadb9560..484f66083 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -108,7 +108,7 @@ In case of a high-impact issue, follow these steps: "title": "Toggling off auto_delete_objects for Bucket empties the bucket", "issueUrl": "https://github.com/aws/aws-cdk/issues/16603", "overview": "If a stack is deployed with an S3 bucket with auto_delete_objects=True, and then re-deployed with auto_delete_objects=False, all the objects in the bucket will be deleted.", - "components": ["framework"], + "component": ["framework"], "version": "<1.126.0" } ``` @@ -248,7 +248,7 @@ fields: | `title` | The title of the incident | Free form text | Yes | | `issueUrl` | A link to the GitHub issue where the incident is being tracked | URL | Yes | | `overview` | A paragraph with more information about the incident | Free form text | Yes | -| `components` | The CLI or the Framework | Either `"cli"` or `"framework"` | Yes | +| `component` | The CLI or the Framework | Either `"cli"` or `"framework"` | Yes | | `version` | Version range using the semver format | Semantic Versioning | No | From b82ebfd99dcf80f553256d42b5d993062d093fa2 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 14 Dec 2021 15:33:28 +0000 Subject: [PATCH 10/13] Minor clarifications --- text/0308-cli-advisories.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 484f66083..7ff03a050 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -237,11 +237,11 @@ is any advisory. #### Publishing advisories -We will create a new repository, dedicated to host the advisories file. Using a -CODEOWNERS file, we will restrict the permission to approve PRs only to SDMs. As -usual, any change to this file will have to be published as a PR and approved to -be merged. The file will contain a list of advisories, each having the following -fields: +We will create a new repository, `cdklabs/aws-cdk-advisories`, dedicated to host +the advisories file. As usual, in order to be merged, any change to this file +will have to be published as a PR and approved. Using a CODEOWNERS file, we will +restrict the permission to approve PRs only to SDMs. The file will contain a +list of advisories, each having the following fields: | Field | Description | Format | Mandatory? | |:------------:|:--------------------------------------------------------------:|---------------------------------|:----------:| @@ -252,20 +252,20 @@ fields: | `version` | Version range using the semver format | Semantic Versioning | No | -We will also need to implement three GitHub actions on this repository: +We will also implement three GitHub actions on this repository: 1. File validation on PR. It will block merging if the structure of the file is - not compliant with the rules. + not compliant with the specification above. 2. Issue sync-up. When the PR is merged, this action will copy the content of the file over to the GitHub issue it's linked to. -3. Issue protection. Every change to issues that are linked to advisories will - be checked by this action, to avoid corruption. +3. Issue protection. Every change to issues that are linked to some advisory + will be checked by this action, to avoid corruption. #### CLI logic The CLI will fetch the file from GitHub, parse the content and check whether the version range contained in the advisory matches the CLI version or the framework -version (depending on the affected component, also present in the advisory). +version (depending on the affected component). Since the CLI knows its own version, checking against the version range of the advisory is trivial. The version of the framework, however, is not readily From 4dc7fccfc5129cb0421b6ee7e8ed70ec16f3589d Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 14 Dec 2021 16:28:19 +0000 Subject: [PATCH 11/13] Update text/0308-cli-advisories.md Co-authored-by: Elad Ben-Israel --- text/0308-cli-advisories.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 7ff03a050..60c8ae6b2 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -182,7 +182,7 @@ On the publishing side: * Implementing a new internal REST service to manage the advisories. Overly complex for this use case. -* Authoring the content directly on the GitHub issue. There is good way to +* Authoring the content directly on the GitHub issue. There is no good way to implement an approval workflow that includes a human verification step. On the distribution side: From eb295f78c17065283e8d898371bd25b5d363b4c2 Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 11 Jan 2022 14:53:57 +0000 Subject: [PATCH 12/13] Addressed comments --- text/0308-cli-advisories.md | 48 ++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 60c8ae6b2..786dc078f 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -2,7 +2,7 @@ * **Original Author(s)**: [@otaviomacedo](https://github.com/otaviomacedo) * **Tracking Issue**: [#308](https://github.com/aws/aws-cdk-rfcs/issues/308) -* **API Bar Raiser**: @{BAR_RAISER_USER} +* **API Bar Raiser**: @eladb A new CLI feature to notify customers about urgent and important issues that require their attention. @@ -115,11 +115,18 @@ In case of a high-impact issue, follow these steps: 3. Create a PR with this change and wait for an approval. Only SDMs have permission to approve PRs in this repository. -4. When the PR gets merged, the advisory will be visible to all CLI +4. A PR check will validate the `advisories.json` file and verify that the + following conditions hold: + * The file is compliant with the schema. + * The issue exists. + * Title and overview are within the length constraints. + * The semantic version is valid. + * The component is valid. +5. When the PR gets merged, the advisory will be visible to all CLI installations. The GitHub issue will also be automatically updated with the information contained in the file. All the necessary tags will also be added automatically. -5. You can keep updating the issue normally, as new information comes in, but +6. You can keep updating the issue normally, as new information comes in, but you're not allowed to touch the sections auto-generated from the advisories file. @@ -166,11 +173,12 @@ this feature. ### What is the technical solution (design) of this feature? -Advisory information will be available as a static file, served from GitHub. -The CLI will consume this file from and apply a set of filters to narrow down -the list of advisories to the context in which it is being run. We will also -implement some GitHub actions to validate and copy the contents of the file over -to the issue. For a more detailed explanation, see the Appendix. +Advisory information will be available as a static file on a GitHub repository. +This file will be automatically synced to an S3 bucket and distributed via +CloudFront. The CLI will consume this file from and apply a set of filters to +narrow down the list of advisories to the context in which it is being run. We +will also implement some GitHub actions to validate and copy the contents of the +file over to the issue. For a more detailed explanation, see the Appendix. ### Is this a breaking change? @@ -182,18 +190,19 @@ On the publishing side: * Implementing a new internal REST service to manage the advisories. Overly complex for this use case. -* Authoring the content directly on the GitHub issue. There is no good way to - implement an approval workflow that includes a human verification step. +* Authoring the content directly on the GitHub issue. Hard to enfore + constraints on the content. On the distribution side: -* Assuming the advisories were stored in an S3 file (in case of the REST - service), we would use CloudFront to distribute them. +* Serving the file directly form GitHub. * Using the GitHub API to query for special issues that are considered advisories (in case of using GitHub issues as the source of truth). ### What is the high-level project plan? +1. Implement and deploy the infastructure necessary for distribution (S3 + + CloudFront). 1. Implement the GitHub actions of validation and issue sync-up and issue protection. 2. Add the construct library version to the cloud assembly metadata. @@ -245,7 +254,7 @@ list of advisories, each having the following fields: | Field | Description | Format | Mandatory? | |:------------:|:--------------------------------------------------------------:|---------------------------------|:----------:| -| `title` | The title of the incident | Free form text | Yes | +| `title` | The title of the incident (max length: 100) | Free form text | Yes | | `issueUrl` | A link to the GitHub issue where the incident is being tracked | URL | Yes | | `overview` | A paragraph with more information about the incident | Free form text | Yes | | `component` | The CLI or the Framework | Either `"cli"` or `"framework"` | Yes | @@ -261,11 +270,15 @@ We will also implement three GitHub actions on this repository: 3. Issue protection. Every change to issues that are linked to some advisory will be checked by this action, to avoid corruption. +For the distribution, we will implement a mechanism to sync the contents of the +GitHub repository with the S3 bucket (e.g., a Event Bridge event). The S3 +content will be served by a CloudFront distribution. + #### CLI logic -The CLI will fetch the file from GitHub, parse the content and check whether the -version range contained in the advisory matches the CLI version or the framework -version (depending on the affected component). +On every command, the CLI will fetch the file from the backend, parse the +content and check whether the version range contained in the advisory matches +the CLI version or the framework version (depending on the affected component). Since the CLI knows its own version, checking against the version range of the advisory is trivial. The version of the framework, however, is not readily @@ -274,7 +287,8 @@ to the Cloud Assembly, in a place where the CLI can read it. Issues that pass this filter will be displayed on the standard output. If an error or timeout occurs when retrieving the issues, the CLI will simply skip -the display of advisories and try again at the next command execution. +the display of advisories and try again at the next command execution. Results +will be cached for a period of one hour. The CLI will store the IDs of the acknowledged issues in the project specific `./cdk.json` file. From c3076e12bfd6f13807e4f2ebcba249a03c948f7a Mon Sep 17 00:00:00 2001 From: Otavio Macedo Date: Tue, 11 Jan 2022 15:39:22 +0000 Subject: [PATCH 13/13] Renamed advisory -> notice --- text/0308-cli-advisories.md | 88 ++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/text/0308-cli-advisories.md b/text/0308-cli-advisories.md index 786dc078f..1b4ea2e9f 100644 --- a/text/0308-cli-advisories.md +++ b/text/0308-cli-advisories.md @@ -1,4 +1,4 @@ -# CLI advisories +# CLI notices * **Original Author(s)**: [@otaviomacedo](https://github.com/otaviomacedo) * **Tracking Issue**: [#308](https://github.com/aws/aws-cdk-rfcs/issues/308) @@ -20,7 +20,7 @@ $ cdk deploy ... # Normal output of the command -ADVISORIES +NOTICES 16603 Toggling off auto_delete_objects for Bucket empties the bucket @@ -44,15 +44,15 @@ ADVISORIES More information at: https://github.com/aws/aws-cdk/issues/17061 -If you don’t want to see an advisory anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". +If you don’t want to see an notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 16603". ``` -By acknowledging a particular advisory, it won’t show anymore in subsequent calls: +By acknowledging a particular notice, it won’t show anymore in subsequent calls: ``` $ cdk acknowledge 16603 -ADVISORIES +NOTICES 17061 Error when building EKS cluster with monocdk import @@ -64,44 +64,44 @@ ADVISORIES More information at: https://github.com/aws/aws-cdk/issues/17061 -If you don’t want to see an advisory anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 17061". +If you don’t want to see an notice anymore, use "cdk acknowledge ID". For example, "cdk acknowledge 17061". ``` You can suppress all warnings per individual execution: ``` -$ cdk deploy --no-advisories +$ cdk deploy --no-notices ``` -And you can disable all advisories indefinitely by adding this entry to +And you can disable all notices indefinitely by adding this entry to `~/.cdk.json`: ``` -"advisories": false +"notices": false ``` -Regardless of the state of this flag and the advisories you have acknowledged, -you can always show the currently active advisories: +Regardless of the state of this flag and the notices you have acknowledged, +you can always show the currently active notices: ``` -$ cdk advisories +$ cdk notices ``` -This command returns zero if there is no advisory and non-zero otherwise. Users +This command returns zero if there is no notice and non-zero otherwise. Users can then plug this into a pipeline approval workflow and expect manual review if -there are any advisories. +there are any notices. > Please note that the acknowledgements are made project by project. If you -acknowledge an advisory in one CDK project, it will still appear on other +acknowledge an notice in one CDK project, it will still appear on other projects when you run any CDK commands, unless you have suppressed or disabled -advisories. +notices. ### Runbook section (internal to the CDK team) In case of a high-impact issue, follow these steps: 1. Create or update an issue for this incident on the aws-cdk GitHub repository. -2. Update the file `advisories.json` on repository `cdklabs/aws-cdk-advisories`, adding an entry for +2. Update the file `notices.json` on repository `cdklabs/aws-cdk-notices`, adding an entry for the incident. Example: ```json { @@ -115,19 +115,19 @@ In case of a high-impact issue, follow these steps: 3. Create a PR with this change and wait for an approval. Only SDMs have permission to approve PRs in this repository. -4. A PR check will validate the `advisories.json` file and verify that the +4. A PR check will validate the `notices.json` file and verify that the following conditions hold: * The file is compliant with the schema. * The issue exists. * Title and overview are within the length constraints. * The semantic version is valid. * The component is valid. -5. When the PR gets merged, the advisory will be visible to all CLI +5. When the PR gets merged, the notice will be visible to all CLI installations. The GitHub issue will also be automatically updated with the information contained in the file. All the necessary tags will also be added automatically. 6. You can keep updating the issue normally, as new information comes in, but - you're not allowed to touch the sections auto-generated from the advisories + you're not allowed to touch the sections auto-generated from the notices file. [ ] Signed-off by API Bar Raiser @xxxxx @@ -142,7 +142,7 @@ about security vulnerabilities, regressions and usage of unsupported versions. ### Why should I use this feature? -These advisories shown by the CLI contain very important and actionable +These notices shown by the CLI contain very important and actionable information about problems that directly affect you. They give you an opportunity to upgrade the CLI or the construct library when necessary or to work around high-impacting issues. @@ -165,7 +165,7 @@ However, to keep its relevance, it should be used sparingly and the messages must meet a high bar. Otherwise it will just become a tool for spamming customers. We will introduce filters to reduce this risk, by only showing content that applies to each particular environment and also require PR approval -for any changes in advisories. But ultimately, it hinges on responsible use. In +for any changes in notices. But ultimately, it hinges on responsible use. In particular, this feature should not be used for things like marketing campaigns, blog post announcements and things like that. If the mechanisms proposed in this RFC are not considered strong enough by the CDK team, we should not implement @@ -173,10 +173,10 @@ this feature. ### What is the technical solution (design) of this feature? -Advisory information will be available as a static file on a GitHub repository. +Notice information will be available as a static file on a GitHub repository. This file will be automatically synced to an S3 bucket and distributed via CloudFront. The CLI will consume this file from and apply a set of filters to -narrow down the list of advisories to the context in which it is being run. We +narrow down the list of notices to the context in which it is being run. We will also implement some GitHub actions to validate and copy the contents of the file over to the issue. For a more detailed explanation, see the Appendix. @@ -188,7 +188,7 @@ No. On the publishing side: -* Implementing a new internal REST service to manage the advisories. Overly +* Implementing a new internal REST service to manage the notices. Overly complex for this use case. * Authoring the content directly on the GitHub issue. Hard to enfore constraints on the content. @@ -197,7 +197,7 @@ On the distribution side: * Serving the file directly form GitHub. * Using the GitHub API to query for special issues that are considered - advisories (in case of using GitHub issues as the source of truth). + notices (in case of using GitHub issues as the source of truth). ### What is the high-level project plan? @@ -208,49 +208,49 @@ On the distribution side: 2. Add the construct library version to the cloud assembly metadata. 2. Implement and release the CLI changes. -### What is the expected lifetime of advisories? +### What is the expected lifetime of notices? -We should expect advisories to be available for months. This is usually the case +We should expect notices to be available for months. This is usually the case when we want users to upgrade the construct library version. -### Why do we need to give users the option to suppress advisories? +### Why do we need to give users the option to suppress notices? This is useful when the CDK is running as part of larger script and users want to hide the output. -### Why do we need to give users the option to acknowledge advisories? +### Why do we need to give users the option to acknowledge notices? -Given the expected lifetime of advisories, they will eventually become just +Given the expected lifetime of notices, they will eventually become just noise and users will try to silence it. If the only option we give users is to suppress them (per execution or indefinitely), they will use that and miss new -advisories. +notices. ### What future improvements can we make? -**Resource filter**: Ideally, advisories should be as targeted as possible. The +**Resource filter**: Ideally, notices should be as targeted as possible. The version filter addresses part of that requirement, but we can make it better by also adding a resource filter: if an issue only affects `FargateTaskDefinition` resources, for example, but this resource is not present in any of the stacks, the customer doesn’t need to see that particular warning. **Pipeline awareness**: Initially, we won’t treat the pipeline as a special -case. By default, the advisories will be fetched and displayed and we expect +case. By default, the notices will be fetched and displayed and we expect users to never look at them. This behavior can always be overridden by -suppressing the advisories. In future releases, the CLI will use the information +suppressing the notices. In future releases, the CLI will use the information about whether it’s running on a pipeline and block the deployment in case there -is any advisory. +is any notice. ## Appendix ### Detailed design -#### Publishing advisories +#### Publishing notices -We will create a new repository, `cdklabs/aws-cdk-advisories`, dedicated to host -the advisories file. As usual, in order to be merged, any change to this file +We will create a new repository, `cdklabs/aws-cdk-notices`, dedicated to host +the notices file. As usual, in order to be merged, any change to this file will have to be published as a PR and approved. Using a CODEOWNERS file, we will restrict the permission to approve PRs only to SDMs. The file will contain a -list of advisories, each having the following fields: +list of notices, each having the following fields: | Field | Description | Format | Mandatory? | |:------------:|:--------------------------------------------------------------:|---------------------------------|:----------:| @@ -267,7 +267,7 @@ We will also implement three GitHub actions on this repository: not compliant with the specification above. 2. Issue sync-up. When the PR is merged, this action will copy the content of the file over to the GitHub issue it's linked to. -3. Issue protection. Every change to issues that are linked to some advisory +3. Issue protection. Every change to issues that are linked to some notice will be checked by this action, to avoid corruption. For the distribution, we will implement a mechanism to sync the contents of the @@ -277,17 +277,17 @@ content will be served by a CloudFront distribution. #### CLI logic On every command, the CLI will fetch the file from the backend, parse the -content and check whether the version range contained in the advisory matches +content and check whether the version range contained in the notice matches the CLI version or the framework version (depending on the affected component). Since the CLI knows its own version, checking against the version range of the -advisory is trivial. The version of the framework, however, is not readily +notice is trivial. The version of the framework, however, is not readily available anywhere. To address this, we will start writing the framework version to the Cloud Assembly, in a place where the CLI can read it. Issues that pass this filter will be displayed on the standard output. If an error or timeout occurs when retrieving the issues, the CLI will simply skip -the display of advisories and try again at the next command execution. Results +the display of notices and try again at the next command execution. Results will be cached for a period of one hour. The CLI will store the IDs of the acknowledged issues in the project specific