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.

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

Export data map from server resources instead of manifest dir #662

Merged
merged 7 commits into from
May 23, 2022

Conversation

SteveDMurphy
Copy link
Contributor

@SteveDMurphy SteveDMurphy commented May 17, 2022

Closes #659

Code Changes

  • Include an organization_fides_key when exporting a data map
  • Filter resources by organization_fides_key
  • Remove requirement to provide a manifest directory (currently used as output location still)
  • Pull all resources from the server instead of just what is listed in the manifest directory
  • Updated guide to a data map with option to use demo_resources/

Steps to Confirm

  • Ensure exporting a data map contains all resources from both .fides/ and demo_resources/ after applying them to the server
  • Ensure items from a separate organization are filtered off of an exported data map

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • 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)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

Previously only fides keys found in a specified manifest directory were included in a data map. This not only led to a bug when an organization is not defined in a manifest directory but also is impractical when working inside a large organization.

This change will have a data map exported for a specific organization, which is more aligned with our documentation around organizations and expectations of users.

Still looking at the arguments used here and seeing if an option is a valid replacement, either for output file or organization_fides_key

Updated steps for validation:

  1. nox -s cli
  2. Apply both sets of resources to the server: fidesctl apply && fidesctl apply demo_resources/
  3. Export the combined data map: fidesctl export datamap
  4. Validate the exported data map in the .fides/ directory contains three systems
  5. Change the output directory: fidesctl export datamap demo_resources/

Filtering test (slightly more complicated)
6. Create a basic organization in the .fides/ directory with a different name (i.e. new_organization)
7. Apply the new organization_fides_key to the other resources in the .fides/ directory
8. fidesctl export datamap demo_resources/
9. Validate the resources from the .fides/ directory are no longer in the exported datamap

Some open questions:

  • Currently, the other resources to export still point to a manifest directory - should we change this as well?
  • The command itself feels a bit chunky, organization_fides_key is now an option versus an argument but opinions welcome

Previously only fides keys found in a specified manifest directory were included in a data map. This not only led to a bug when an organization is not defined in a manifest directory but also is impractical when working inside a large organization.

This change will have a data map exported for a specific organization, which is more aligned with our documentation around organizations and expactations of users.
@SteveDMurphy SteveDMurphy marked this pull request as ready for review May 20, 2022 01:17
@SteveDMurphy SteveDMurphy requested a review from a team May 20, 2022 01:17
The positional argument felt a bit out of place. As of today, we can expect most users to have a default_organization and to use the .fides/ directory. These options will allow for deviations from that plan without requiring any specific positional arguments to take place.
@SteveDMurphy SteveDMurphy requested review from a team and ThomasLaPiana May 21, 2022 02:34
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! A few small nits, but a nice self-contained change

@SteveDMurphy
Copy link
Contributor Author

@conceptualshark there were some very minor changes made to the data map user guide for this change, would you be able to check them out and make sure you are ok with the updates? Essentially now we are pulling resources from the server but I felt like it made sense to use the output-dir option to keep the user working in the demo_resources/ directory. Thanks!!

@conceptualshark
Copy link
Contributor

👍 this works for me - if everything else in the guide works the same, it makes sense to keep them contained to that directory.

@SteveDMurphy SteveDMurphy merged commit 9fa0ce5 into main May 23, 2022
@SteveDMurphy SteveDMurphy deleted the SteveDMurphy-659-datamap-from-server branch May 23, 2022 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data Map Limited to Fides Keys found in manifest directory
3 participants