Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Specify Multiple Masking Strategies #1428

Merged
merged 6 commits into from
Oct 19, 2022

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Oct 14, 2022

Purpose

Allow multiple masking strategies to be specified when using fides as a masking engine.

Changes

  • We could already specify a single masking strategy. Add in a backwards compatible way so you can specify a single strategy or a list of strategies to the same field. If multiple strategies are supplied, they are executed in order.
  • Allow specifying the config for the masking strategy to be much more flexible to accommodate the needs of new masking strategies

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes https://github.com/ethyca/fidesops-plus/issues/74

…s-compatible way.

Pass in one or more strategies under "masking_strategy", either as a single strategy or a list of strategies. If a list is supplied, the strategies will be applied in the same order as the list.
@pattisdr
Copy link
Contributor Author

@ethyca/docs-authors Adding this PR in fidesops instead of fides after talking to Sean - sounds like there may be another release -

Small docs changes around using fides as a masking engine

…le - it still needs to be a Dict with string keys, but the values are much more open.

This supports other masking strategies with more complex configurations.
@pattisdr pattisdr marked this pull request as ready for review October 14, 2022 20:42
)
masked_values = strategy.mask(values, None)
values = request.values or []
masked_values: Optional[List[Any]] = request.values.copy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be optional? It looks like it might always be an empty list if nothing else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes I agree with you, I was getting "Incompatible types in assignment" mypy errors because masking.mask is allowed to take in None. I think we should instead ignore the type hint error then when we call masking.mask

@conceptualshark
Copy link
Contributor

👍 Looks good on the docs front, thank you!

@pattisdr
Copy link
Contributor Author

thanks for reviewing @conceptualshark 😄

@pattisdr
Copy link
Contributor Author

Made some type adjustments @seanpreston back to you!

Copy link
Contributor

@seanpreston seanpreston left a comment

Choose a reason for hiding this comment

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

Thanks @pattisdr

@seanpreston seanpreston merged commit 395c8d8 into main Oct 19, 2022
@seanpreston seanpreston deleted the fides_74_add_multiple_masking_strategies branch October 19, 2022 19:51
pattisdr added a commit to ethyca/fides that referenced this pull request Nov 1, 2022
…l/1428 to allow multiple masking strategies to be specified when using fides as a masking engine.
NevilleS pushed a commit to ethyca/fides that referenced this pull request Nov 1, 2022
* add redis settings fix

* Copy and paste over the work inhttps://github.com/ethyca/fidesops/pull/1428 to allow multiple masking strategies to be specified when using fides as a masking engine.

* Consolidate multiple Fixed headings, and update Changelog with the redis db index and the multiple masking strategies changes.

Co-authored-by: Dawn Pattison <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants