Set the fides dev environment to the `prod` security environment #2588

ThomasLaPiana · 2023-02-14T07:18:11Z

Closes #2461

Code Changes

update the fides.toml
update docker compose run -> docker exec where possible. This not only saves resources (one less container running!) but allows multiple commands to run in the same environment (i.e. when something gets run via docker compose run, it is ephemeral and can't be used by following commands
Add a "login" step that runs before any of the ctl tests get run. This only happens as part of nox, so when running locally a manual login will be needed for the ctl tests to pass
add the auth_header to the fides db CLI command requests

Steps to Confirm

all tests pass

Pre-Merge Checklist

All CI Pipelines Succeeded
Documentation:
- documentation complete, PR opened in fidesdocs
- documentation issue created in fidesdocs
Issue Requirements are Met
Relevant Follow-Up Issues Created
Update CHANGELOG.md

Description Of Changes

With the change in this PR, a CLI login will be required during development, as the new default is prod for the security env.

cypress · 2023-02-14T07:32:18Z

Passing run #194 ↗︎

0	3	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

Merge `b01515a` into `ea5cf1d`...
Project: fides	Commit: `eafcb73362 ℹ️`
Status: Passed	Duration: 00:51 💡
Started: Feb 17, 2023 2:39 PM	Ended: Feb 17, 2023 2:40 PM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

…osed to only when the CLI is invoked

ThomasLaPiana · 2023-02-15T14:40:16Z

I got all of the tests passing locally but then it looks like they're not passing in CI or when building the test image locally and running it that way...will trouble shoot more locally

ThomasLaPiana · 2023-02-15T15:05:41Z

if I log into a container locally and run fides user login and then run the tests everything works just fine...but the login command within the fixture doesn't seem to be doing the trick despite the fact that it definitely writes a file

ThomasLaPiana · 2023-02-15T15:22:41Z

Found the problem in the logs....

2023-02-15 15:19:26.362 [DEBUG] (oauth_util:verify_oauth_client:174): Unable to parse auth token.

So it looks like the order of execution for the fixtures might be wrong....the token gets created at the beginning, but then gets reset because its used to reset the db, so then becomes invalid. So this fixture needs to get called twice

sanders41 · 2023-02-15T15:49:16Z

Is it autouse fixtures you are having issues with or fixtures you are adding to the test yourself? If the latter try playing with order, fixtures run bottom to top so you may be able to change the order up and get a different result.

ThomasLaPiana · 2023-02-15T15:50:28Z

Is it autouse fixtures you are having issues with or fixtures you are adding to the test yourself? If the latter try playing with order, fixtures run bottom to top so you may be able to change the order up and get a different result.

I'm trying to wrap all of the resets as needed....getting closer

Figured out was actually a cache issue 🤦 it was grabbing the old auth header 🤔

ThomasLaPiana · 2023-02-15T16:31:39Z

I've once again confirmed that it works perfectly if you have a credentials file before running the tests, but if it tries to create one during the test it doesn't work. I'm not sure why this happens. Going to try using pytest tricks to generate one as a non-fixture?

Despite all of the logs showing that one gets created just fine during the tests 🤔

…sible, add a login command before running pytest for ctl

codecov · 2023-02-15T17:53:14Z

Codecov Report

Base: 85.74% // Head: 86.23% // Increases project coverage by +0.49% 🎉

Coverage data is based on head (b01515a) compared to base (d3700cb).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2588      +/-   ##
==========================================
+ Coverage   85.74%   86.23%   +0.49%     
==========================================
  Files         285      289       +4     
  Lines       15477    15784     +307     
  Branches     1945     1985      +40     
==========================================
+ Hits        13270    13611     +341     
+ Misses       1832     1785      -47     
- Partials      375      388      +13

Impacted Files	Coverage Δ
src/fides/cli/__init__.py	`92.00% <ø> (-0.73%)`	⬇️
src/fides/cli/commands/db.py	`81.48% <ø> (ø)`
src/fides/core/api.py	`82.85% <100.00%> (ø)`
src/fides/core/config/user_settings.py	`100.00% <100.00%> (ø)`
src/fides/core/user.py	`100.00% <100.00%> (ø)`
src/fides/core/utils.py	`86.15% <100.00%> (+5.30%)`	⬆️
src/fides/api/ops/schemas/messaging/messaging.py	`98.69% <0.00%> (-0.57%)`	⬇️
src/fides/api/ops/models/privacy_request.py	`95.87% <0.00%> (-0.19%)`	⬇️
src/fides/api/ops/tasks/__init__.py	`79.41% <0.00%> (ø)`
src/fides/api/ops/api/v1/urn_registry.py	`100.00% <0.00%> (ø)`
... and 19 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

…e the vanilla "RUN" command

SteveDMurphy

Thanks for explaining the difference between docker run and docker exec and the value of the change 👍🏽 👍🏽 when spinning up the environment in Docker I am not able to run any commands for some reason, including the fides user login ... - installing the branch locally in a virtual env doesn't seem to have the same issue from what I can see 🤔

sanders41 · 2023-02-16T02:36:41Z

I accidently posted my comment in the issue instead of here. I'm seeing things similar to Steve. #2461 (comment)

ThomasLaPiana · 2023-02-16T08:46:23Z

ah I'm a total rookie! Sorry for wasting y'alls time, the exec for the interactive shell needs to have the -it flag

Added and confirmed working, please try again 🙏

SteveDMurphy

Tried out a few of these successfully! Thanks for the guidance in testing as well 🙌🏽

The only other thing that might be helpful is if we wanted to add something around running pytest locally and needing to log in to the readme or similar? Can imagine people hitting some snags

TheAndrewJackson

This worked great for me locally 👍 I just left a few optional nits

noxfiles/ci_nox.py

noxfiles/constants_nox.py

tests/ctl/cli/test_cli.py

sanders41

Worked for me now also. I agree with @SteveDMurphy that we may want to add something to the readme or somewhere like it about logging in. The ctl tests worked fine for me when I logged in, but I only knew to do that from the PR instructions.

Co-authored-by: Andrew Jackson <andrew.c.j1995@gmail.com>

ThomasLaPiana · 2023-02-17T06:57:32Z

@sanders41 seeing codecov failures here

sanders41 · 2023-02-17T10:33:36Z

Looks like they passed. Did you have to rerun for them to pass?

sanders41 · 2023-02-17T10:58:06Z

I see it now. This is the flaky issue I mentioned. It has been terrible this week. Looks like adding a token is helping, but not completely preventing the issue. I restarted the failed tests, they usually pass after a rerun.

ThomasLaPiana · 2023-02-17T14:48:51Z

it failed the codecov upload, merging

Set the dev environment to default to the prod security environment

70428ed

ThomasLaPiana self-assigned this Feb 14, 2023

ThomasLaPiana added 9 commits February 15, 2023 14:33

fix: update the fides_db_scan nox session to login first

5712a74

fix: add an autouse fixture that logs into the CLI

bc2de33

Merge branch 'main' into ThomasLaPiana-prod-env-default

f0551db

feat: generate the auth headers whenever the config is loaded, as opp…

9c1ce4d

…osed to only when the CLI is invoked

fix: bad exception catch that prevented the app from starting

abe6416

fix: added auth_header to test fixture that reset the db

8c66610

fix: add more auth headers to various tests

21b84e6

fix: errant deletion causing a test regression

070be40

docs: changelog update

9159ac1

feat: proper error message for permissions check failure

554f1ea

ThomasLaPiana added 4 commits February 16, 2023 00:56

refactor: use 'docker exec' instead of 'docker compose run' where pos…

232363e

…sible, add a login command before running pytest for ctl

fix: static_checks

dfb37fe

fix: TTY output for the exec commands

380e6df

fix: more CI_ARGS replacement

23940a3

ThomasLaPiana added 3 commits February 16, 2023 02:55

fix: add a test for coverage

decd535

refactor: docker compose run -> exec in dev_nox

a9b7139

refactor: add analytics opt out env var passthrough to exec and remov…

abec978

…e the vanilla "RUN" command

ThomasLaPiana marked this pull request as ready for review February 15, 2023 20:26

ThomasLaPiana requested review from sanders41 and pattisdr February 15, 2023 20:27

ThomasLaPiana requested review from TheAndrewJackson and SteveDMurphy February 15, 2023 20:27

SteveDMurphy reviewed Feb 16, 2023

View reviewed changes

ThomasLaPiana mentioned this pull request Feb 16, 2023

Convert all config values to Fields and add descriptions/exclusions #2613

Merged

18 tasks

fix: update the dev shell to use an interactive exec session

661cb71

ThomasLaPiana requested a review from SteveDMurphy February 16, 2023 09:26

SteveDMurphy approved these changes Feb 16, 2023

View reviewed changes

TheAndrewJackson approved these changes Feb 16, 2023

View reviewed changes

noxfiles/ci_nox.py Outdated Show resolved Hide resolved

noxfiles/constants_nox.py Show resolved Hide resolved

tests/ctl/cli/test_cli.py Show resolved Hide resolved

sanders41 approved these changes Feb 16, 2023

View reviewed changes

ThomasLaPiana and others added 2 commits February 16, 2023 22:08

Update noxfiles/ci_nox.py

8a5616c

Co-authored-by: Andrew Jackson <andrew.c.j1995@gmail.com>

fix: a small bug, add a bit of logging around login requirement

aa8d050

ThomasLaPiana added 2 commits February 17, 2023 15:07

fix: test env regression

09c3063

fix: test_env keep alive

7a0dd29

docs: add a note about CLI login to the contrib docs

b01515a

ThomasLaPiana merged commit f13801a into main Feb 17, 2023

ThomasLaPiana deleted the ThomasLaPiana-prod-env-default branch February 17, 2023 14:49

allisonking mentioned this pull request Feb 17, 2023

Add auth to all slices #2645

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set the fides dev environment to the `prod` security environment #2588

Set the fides dev environment to the `prod` security environment #2588

ThomasLaPiana commented Feb 14, 2023 •

edited

Loading

cypress bot commented Feb 14, 2023 •

edited

Loading

ThomasLaPiana commented Feb 15, 2023

ThomasLaPiana commented Feb 15, 2023

ThomasLaPiana commented Feb 15, 2023

sanders41 commented Feb 15, 2023 •

edited

Loading

ThomasLaPiana commented Feb 15, 2023 •

edited

Loading

ThomasLaPiana commented Feb 15, 2023

codecov bot commented Feb 15, 2023 •

edited

Loading

SteveDMurphy left a comment

sanders41 commented Feb 16, 2023

ThomasLaPiana commented Feb 16, 2023

SteveDMurphy left a comment

TheAndrewJackson left a comment

sanders41 left a comment

ThomasLaPiana commented Feb 17, 2023

sanders41 commented Feb 17, 2023

sanders41 commented Feb 17, 2023

ThomasLaPiana commented Feb 17, 2023

Set the fides dev environment to the prod security environment #2588

Set the fides dev environment to the prod security environment #2588

Conversation

ThomasLaPiana commented Feb 14, 2023 • edited Loading

Code Changes

Steps to Confirm

Pre-Merge Checklist

Description Of Changes

cypress bot commented Feb 14, 2023 • edited Loading

Passing run #194 ↗︎

ThomasLaPiana commented Feb 15, 2023

ThomasLaPiana commented Feb 15, 2023

ThomasLaPiana commented Feb 15, 2023

sanders41 commented Feb 15, 2023 • edited Loading

ThomasLaPiana commented Feb 15, 2023 • edited Loading

ThomasLaPiana commented Feb 15, 2023

codecov bot commented Feb 15, 2023 • edited Loading

Codecov Report

SteveDMurphy left a comment

Choose a reason for hiding this comment

sanders41 commented Feb 16, 2023

ThomasLaPiana commented Feb 16, 2023

SteveDMurphy left a comment

Choose a reason for hiding this comment

TheAndrewJackson left a comment

Choose a reason for hiding this comment

sanders41 left a comment

Choose a reason for hiding this comment

ThomasLaPiana commented Feb 17, 2023

sanders41 commented Feb 17, 2023

sanders41 commented Feb 17, 2023

ThomasLaPiana commented Feb 17, 2023

Set the fides dev environment to the `prod` security environment #2588

Set the fides dev environment to the `prod` security environment #2588

ThomasLaPiana commented Feb 14, 2023 •

edited

Loading

cypress bot commented Feb 14, 2023 •

edited

Loading

sanders41 commented Feb 15, 2023 •

edited

Loading

ThomasLaPiana commented Feb 15, 2023 •

edited

Loading

codecov bot commented Feb 15, 2023 •

edited

Loading