-
Notifications
You must be signed in to change notification settings - Fork 7
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
Update Dockerfile configuration #195
Merged
mcdonnnj
merged 38 commits into
develop
from
improvement/update_Dockerfile_configuration
Dec 6, 2024
Merged
Update Dockerfile configuration #195
mcdonnnj
merged 38 commits into
develop
from
improvement/update_Dockerfile_configuration
Dec 6, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcdonnnj
added
breaking change
This issue or pull request involves changes to existing functionality
improvement
This issue or pull request will add or improve functionality, maintainability, or ease of use
version bump
This issue or pull request increments the version number
labels
Mar 6, 2024
mcdonnnj
requested review from
dav3r,
felddy,
jasonodoom and
jsf9k
as code owners
March 6, 2024 20:30
jsf9k
approved these changes
Mar 6, 2024
dav3r
reviewed
Mar 7, 2024
dav3r
reviewed
Mar 7, 2024
dav3r
approved these changes
Mar 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 after symlinkgate has been resolved.
jsf9k
approved these changes
Mar 14, 2024
felddy
approved these changes
May 28, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fully charged! 🔋
You deserve an award for this modernization! 🏆
I had a few comments.
cisagovbot
pushed a commit
that referenced
this pull request
Nov 1, 2024
Add missing dependabot ignore directives
mcdonnnj
force-pushed
the
improvement/update_Dockerfile_configuration
branch
from
December 6, 2024 09:04
479537b
to
9e0fe4e
Compare
jsf9k
approved these changes
Dec 6, 2024
jsf9k
approved these changes
Dec 6, 2024
dav3r
approved these changes
Dec 6, 2024
This helps ensure that when a Docker image is built the expacted source image is used regardless of what repository is configured as the default on the host system. It also makes our Dockerfiles more seamlessly convertible to using the GitHub Container Registry or any other Open Container Initiative (OCI) compatible registry.
Instead of downloading the source archive, extracting it, and then installing it with pip we instead just let pip directly install the package.
Use the full tag that includes the Alpine Linux version to ensure the pulled image is always the same.
Since we are now installing cisagov/skeleton-python-library directly with pip we no longer need these OS packages.
Use the full path for source container images
We should not blindly upgrade all pre-installed packages. This can create inconsistent build results due to changes in installed versions.
Now that we are not overwriting the internal Python package file the text we look for must match what is output by default. The Docker Compose secret configuration is left in place to continue to serve as an example and to be leveraged for a future update to cisagov/skeleton-python-library that can provide similar functionality to what was removed in this project.
Pin the versions of the pip, setuptools, and wheel packages that are installed.
…hon-library_directly Install cisagov/skeleton-python-library directly with `pip`
This configuration includes a Pipfile configuration file and the generated Pipfile.lock file that pins to specific versions for the Python dependencies for this project. This will help us ensure repeatable builds. The pipenv package is added as a developmental requirement to support these files.
Since we cannot use long options on Alpine Linux we should explain what the short options we are using do. I also changed the order of options so that they are in alphabetical order. Co-authored-by: Shane Frasier <jeremy.frasier@gwe.cisa.dhs.gov>
Now that we have a pipenv configuration we will use it to install the Python dependencies for the image. The `build` workflow is updated to no longer pass the VERSION build argument in line with this change.
Use a Python virtual environment
Switch to using a multi-stage build in the Dockerfile. This reduces image size since pipenv and its dependencices are not needed in the final image. It also ensures that the system Python environment is unmodified.
Install the core Python packages (pip, setuptools, and wheel) into the system Python environment before installing pipenv. This keeps things consistent with our usual approach to Python environments.
The comment references a command that is no longer being run. Co-authored-by: Shane Frasier <jeremy.frasier@gwe.cisa.dhs.gov>
Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
Change the tags used in the table to match the version of the project. Previously "1.2.3" was used as an example version but there is no reason not to use the real version of the image.
…tion Install Python dependencies with `pipenv`
Update the README
Update the Dockerfile and testing to accommodate changes in the new version.
Update dependencies
Co-authored-by: dav3r <david.redmin@trio.dhs.gov>
The version of Python listed in the Pipfile is updated to match the new Docker image tag.
- pip from 24.0 to 24.3.1 - pipenv from 2023.12.1 to 2024.4.0 - setuptools from 69.1.1 to 75.6.0 - wheel from 0.42.0 to 0.45.1
Update the dependencies installed in the Python virtual environment by running `pipenv lock` in the `src/` directory.
Update image dependencies
This resolves the following warning from Docker when building the image: FromAsCasing: 'as' and 'FROM' keywords' casing do not match Co-authored-by: Shane Frasier <jeremy.frasier@gwe.cisa.dhs.gov>
mcdonnnj
force-pushed
the
improvement/update_Dockerfile_configuration
branch
from
December 6, 2024 20:33
daeb027
to
fd69f45
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
breaking change
This issue or pull request involves changes to existing functionality
improvement
This issue or pull request will add or improve functionality, maintainability, or ease of use
version bump
This issue or pull request increments the version number
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗣 Description
This pull request represents a rework of this project's Docker configuration. It includes the following pull requests:
pip
#188pipenv
#191💭 Motivation and context
We have been striving to use configurations that are better at producing repeatable builds in downstream projects. This represents a backporting of a lot of that work to this skeleton so that all of our Docker projects are similarly configured.
🧪 Testing
Automated tests pass.
✅ Pre-approval checklist
to reflect the changes in this PR.
✅ Post-merge checklist