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

Update the image configuration #39

Merged

Conversation

mcdonnnj
Copy link
Member

🗣 Description

This pull request updates the image configuration to both modernize it and improve platform support.

💭 Motivation and context

Modernizing the Docker image configuration will help with any necessary maintenance and development in the future. It also aligns the image with Docker best practices such as strict version pinning and multi-stage builds. Using a multi-stage build also allows us to restore support for non-amd64/arm64 platforms.

🧪 Testing

Automated tests pass. I am able to use the latest pre-release to generate an appropriate code.json (barring some issues with llnl-scraper that will be resolved separately).

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

Switch from using the `3.7.11-slim-stretch` tag to using the
`3.10.5-alpine3.16` tag. This requires changes for switching the
underlying OS from Debian Stretch to Alpine 3.16.
Call pip as a Python module, use the `--no-cache-dir` option to
reduce the created image size, use long arguments, and install the
wheel package so images can readily be built if an appropriate version
is not available on PyPI.
These dependencies are necessary when there is no pre-built wheel for
the cryptogrpahy package available on PyPI. These dependencies are
listed here: https://cryptography.io/en/latest/installation/#alpine
Use the CISA_USER variable in the definition for the CISA_HOME variable.
Consistently use braces when referencing environment variables.
Switch to chowning files as part of the COPY instruction instead of
manually running the chown command after they are copied. Set the image
to use the CISA_USER user.
Switch to using a Python venv with dependencies managed by pipenv. This
will provide better dependency control because of its lockfile
mechanism.
Switching to a multi-stage Docker build ensures that any build
dependencies are not included in the final Docker image which results
in an image size ~20% of the single-stage build.
This is the last step to give more repeatable builds. This ensures that
the same versions of all dependencies are installed during successive
builds. The environment variables with package information are removed
since we no longer combine them and to pass Hadolint linting (this is
manual and not included in our automatic linting).
The way the `--system-site-packages` option works causes problems when
pip tries to build a wheel from source. It tries to use the system
Python environment during the build process which can result in trying
to use outdated or missing packages.
Update the list of supported platforms based on the new base image and
platforms that were able to build successfully.
Switch to modifying the system Python as little as possible by only
installing pipenv into system Python and then everything else is
installed into the venv that was created. We also bump the versions
of the core Python packages we use that are defined in the Dockerfile.
Update the lock file by running `pipenv lock`.
Now that Alpine 3.16's python3-dev package has been updated to 3.10.8
we can update the base image versions we use. We also upgrade any other
packages that have seen updates so we can successfully build the image.
Install the latest versions of the pipenv, pip, setuptools, and wheel
packages in the Docker configuration.
This aligns us with changes made in cisagov/cyhy_amis to set up the
`cyhy` user/group with a uid of 2048. This should allow secrets that
are set to read-only for the `cyhy` user to be usable by the Docker
image's unprivileged user.
Update the lock file by running `pipenv lock` in the src/ directory.
This updates the Docker image from Alpine 3.16 to 3.17 to keep us on
the latest supported release of Alpine Linux. All of the Alpine
packages that are installed are updated to the versions available for
the 3.17 branch.
Switch to using the ansible:3.17 Docker image for the compile-stage
since the python3-dev package will install the python3 package and
this results in two different Python installs in the compile-stage. We
instead keep the python Docker image used in the build-stage on the
same version of Python as is available in Alpine Linux 3.17 to ensure
compatibility with the created venv.
Add the Alpine packages corresponding to the Python setuptools and
wheel packages. This fully aligns that stage with our typical base
Python packages (pip, setuptools, and wheel).
Update these packages to their latest versions. The `python` Docker
image tag is also bumped to reflect the change in the `python3` package
version being installed.
Store the versions of Python packages installed with `pip` in
environment variables in the Dockerfile. This allows easy visibility
of the versions and ensures that if they are used in multiple places
that the versions used are consistent.
Update the Python version in the Pipfile and update the dependency
lockfile using `pipenv lock` in the `src/` directory.
Only create the cargo configuration file with the setting to use the
git CLI if the target architecture is not amd64 or arm64. This is
especially relevant Since these two platforms are natively supported by
the cryptography package so they should not need to build wheels in the
first place.
Now that the latest Cargo release is available for all potential
platforms (based on the base Docker images) we should support them as
long as they can successfully build. This change resolves
#31.
Instead of creating a Cargo configuration file in the Dockerfile we
instead store an appropriate Cargo configuration file and COPY it into
the image. This allows the file to be checked by pre-commit and
appropriately version controlled. It also makes it easier to make
additional configuration changes from default if needed.
Add another `pip` package ecosystem for the `src/` directory so the
`pipenv` configuration is seen and maintained by dependabot.
Perform the most static instructions first and perform instructions
that can change based on build time arguments or involve copied files
as late as possible. This will better leverage Docker's build cache if
actively developing the project.
This ensures expected behavior by running the file as a Python script
instead of executing the file and allowing bash to resolve how it runs.
Makes sure that the pip, pipenv, and setuptools packages installed are
the latest version.
Update the dependencies installed in the Python virtual environment by
running `pipenv lock` in the `src/` directory.
@mcdonnnj mcdonnnj added improvement This issue or pull request will add or improve functionality, maintainability, or ease of use upstream update This issue or pull request pulls in upstream updates labels Mar 14, 2023
@mcdonnnj mcdonnnj requested a review from dav3r as a code owner March 14, 2023 15:56
@mcdonnnj mcdonnnj self-assigned this Mar 14, 2023
@mcdonnnj mcdonnnj requested a review from a team March 14, 2023 15:56
Copy link
Member

@jsf9k jsf9k left a comment

Choose a reason for hiding this comment

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

Approved, but I would like to see these improvements (including the Pipfile foo) make their way into the skeleton before we apply them manually to too many more repos.

Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

👍 👍

Dockerfile Outdated Show resolved Hide resolved
src/config.toml Outdated Show resolved Hide resolved
Remove unnecessary capitalization and fix a typo in a package name.

Co-authored-by: dav3r <[email protected]>
@mcdonnnj
Copy link
Member Author

Disabled the snyk requirement because we've hit a test cap.

@mcdonnnj mcdonnnj merged commit 6f992cb into improvement/modernize_project Mar 14, 2023
@mcdonnnj mcdonnnj deleted the improvement/update_image_configuration branch March 14, 2023 22:22
@mcdonnnj mcdonnnj mentioned this pull request Mar 17, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add or improve functionality, maintainability, or ease of use upstream update This issue or pull request pulls in upstream updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants