chore(doc): update INTEGRATION_TESTS.md #30880
Conversation
callout the env.account should not be defined
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
pahud
changed the title
INTEGRATION_TESTS.md
Outdated
| @@ -119,6 +119,9 @@ This will: | |||
| Now when you run `npm test` it will synth the integ app and compare the result with the snapshot. | |||
| If the snapshot has changed the same process must be followed to update the snapshot. | |||
|
|
|||
| *Note - When running integration tests, refrain from defining the `env.account` property for stacks. This ensures that the synthesized | |||
There was a problem hiding this comment.
Note should be bold 😊
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
INTEGRATION_TESTS.md
Outdated
| @@ -119,6 +119,9 @@ This will: | |||
| Now when you run `npm test` it will synth the integ app and compare the result with the snapshot. | |||
| If the snapshot has changed the same process must be followed to update the snapshot. | |||
|
|
|||
| *Note - When running integration tests, refrain from defining the `env.account` property for stacks. This ensures that the synthesized | |||
There was a problem hiding this comment.
I don't think this is the answer to this issue. I don't think account numbers are inherently considered sensitive information, but even if we decide to them as such, we should write a linter rule that explicitly prevents users from including them in snapshots.
This might not apply to all tests, some of them need real account numbers (like cross account tests)
There was a problem hiding this comment.
@comcalvi Agree. I have rephrased it this way
Note - When running integration tests, env.account is generally not required unless some special cases like cross-account tests.
Feel free to suggest changes. I think we still have to remind users that env.account is generally not required and not recommended. Thoughts?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Have discussed with the team. Closing in favor of a linter rule. |
callout the
env.accountshould not be defined in most cases, which might have a security concern discussed in #30831Issue # (if applicable)
Closes #.
Reason for this change
Description of changes
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license