Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs(data-masking): add docs for data masking utility #3186

Merged
merged 178 commits into from
Feb 1, 2024
Merged

docs(data-masking): add docs for data masking utility #3186

merged 178 commits into from
Feb 1, 2024

Conversation

seshubaws
Copy link
Contributor

@seshubaws seshubaws commented Oct 10, 2023 •
edited by leandrodamascena
Loading

Issue number: #3488

Summary

Add docs for data masking utility

Changes

Please provide a summary of what's being changed

Adding documentation for the sensitive data masking utility.

User experience

Please share what the user experience looks like before and after this change

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number: #1858

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Oct 10, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 10, 2023
@leandrodamascena leandrodamascena changed the title Add docs for data masking utility docs(data-masking): add docs for data masking utility Oct 10, 2023
@boring-cyborg boring-cyborg bot added the tests label Oct 11, 2023
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 11, 2023
@seshubaws seshubaws marked this pull request as ready for review October 12, 2023 20:45
@seshubaws seshubaws requested a review from a team as a code owner October 12, 2023 20:45
@leandrodamascena
Copy link
Contributor

Hi @seshubaws! Thanks for sending this PR. I will begin this review tomorrow.

leandrodamascena
leandrodamascena reviewed Nov 6, 2023
View reviewed changes
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @seshubaws! I left some comments and am making some changes to add new content to the documentation.

examples/data_masking/sam/template.yaml Outdated Show resolved Hide resolved
examples/data_masking/src/getting_started_encrypt_data.py Outdated Show resolved Hide resolved
examples/data_masking/tests/src/single_mock.py Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Show resolved Hide resolved
leandrodamascena
leandrodamascena requested changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @seshubaws! I left some comments and am making some changes to add new content to the documentation.

@heitorlessa heitorlessa added this to the Sensitive Data Masking milestone Nov 13, 2023
heitorlessa
heitorlessa reviewed Feb 1, 2024
View reviewed changes
README.md Show resolved Hide resolved
heitorlessa
heitorlessa suggested changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last changes ;) look awesome now!

The only piece I wish we had but we can merge without is a sequence diagram to demonstrate what happens when you have multiple KMS keys.

docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
docs/utilities/data_masking.md Outdated Show resolved Hide resolved
@leandrodamascena
Copy link
Contributor

last changes ;) look awesome now!

The only piece I wish we had but we can merge without is a sequence diagram to demonstrate what happens when you have multiple KMS keys.

Hello @heitorlessa! I created the diagram but I don't know if it's enough. Can you review, please?

leandrodamascena
leandrodamascena previously approved these changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@leandrodamascena leandrodamascena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVED!!!!!!!!

@heitorlessa
Copy link
Contributor

last changes ;) look awesome now!
The only piece I wish we had but we can merge without is a sequence diagram to demonstrate what happens when you have multiple KMS keys.

Hello @heitorlessa! I created the diagram but I don't know if it's enough. Can you review, please?

done ;) Changes we made

  • We've changed the diagram from Alt to Loop so it's easier to know this operation will repeat for N keys specified
  • We've used KEY_1 and KEY_2 so it's easier to follow which operation happens under which key
  • We've updated the SAM template to include kms:Encrypt least privilege permission along with a link on why
  • We've updated a quick note explaining that ALL Encrypted Data keys will be included in the encrypted message for portability, allowing higher availability.

heitorlessa
heitorlessa previously approved these changes Feb 1, 2024
View reviewed changes
@heitorlessa
Copy link
Contributor

we're good to go after ensuring deps are included, since Layers will include (and optimize size) Encryption SDK and JSONPath-Ng too.

@leandrodamascena leandrodamascena dismissed stale reviews from heitorlessa and themself via cfeb833 February 1, 2024 11:43
@boring-cyborg boring-cyborg bot added the internal Maintenance changes label Feb 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 1, 2024

⚠️Large PR detected⚠️

Please consider breaking into smaller PRs to avoid significant review delays. Ignore if this PR has naturally grown to this size after reviews.

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 1, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@heitorlessa heitorlessa merged commit ced0a3d into aws-powertools:develop Feb 1, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sthulb sthulb sthulb left review comments

@heitorlessa heitorlessa heitorlessa approved these changes

@leandrodamascena leandrodamascena Awaiting requested review from leandrodamascena

Assignees

@seshubaws seshubaws

Labels
commons dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation internal Maintenance changes size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tests typing Static typing definition related issues (mypy, pyright, etc.)
Projects
None yet
Milestone
Sensitive Data Masking
5 participants
@seshubaws @leandrodamascena @codecov-commenter @heitorlessa @sthulb