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

Clean up docker build #1252

Merged
merged 9 commits into from
Sep 6, 2022
Merged

Clean up docker build #1252

merged 9 commits into from
Sep 6, 2022

Conversation

sanders41
Copy link
Contributor

@sanders41 sanders41 commented Sep 2, 2022

Purpose

Clean up the docker file to make a smaller production image.

Changes

  • Add files to .dockerignore

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 #1239

@sanders41
Copy link
Contributor Author

If you try to run the production image without a fidesops.toml file it will try to load everything from environment variables. Currently you get the message fidesops.toml or fidesctl.toml or fides.toml could not be loaded so it is not obvious that is what is happening. I plan to update the message in fideslib to fidesops.toml or fidesctl.toml or fides.toml could not be loaded. Trying to load from environment variables. to make it more clear.

@sanders41
Copy link
Contributor Author

CI tests are using a production build so it doesn't include a fidesops.toml file and fails. I'll have to find a work around for that.

@sanders41 sanders41 marked this pull request as ready for review September 6, 2022 15:05
@sanders41 sanders41 added the run unsafe ci checks Triggers running of unsafe CI checks label Sep 6, 2022
@seanpreston seanpreston self-assigned this Sep 6, 2022
@sanders41 sanders41 added run unsafe ci checks Triggers running of unsafe CI checks and removed run unsafe ci checks Triggers running of unsafe CI checks labels Sep 6, 2022
@NevilleS
Copy link
Contributor

NevilleS commented Sep 6, 2022

Good- I'm glad the CI tests are now failing @sanders41 as that means we can truly configure it from CI (with GitHub secrets) 👍

docker-compose.yml Outdated Show resolved Hide resolved
@seanpreston seanpreston merged commit 9c5b887 into main Sep 6, 2022
@seanpreston seanpreston deleted the ps-docker-image branch September 6, 2022 23:50
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
* Add files to .dockerignore

* Add config path to docker-compose

* Mount volume for build check

* Fix pylint error

* Update CHANGELOG

* Fix failing external test

* Remove worker env var from docker-compose

Co-authored-by: Paul Sanders <pau@ethyca.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Published Docker image includes test data
3 participants