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

Perform Image Maintenance #14

Merged
merged 12 commits into from
Jul 21, 2021
Merged

Perform Image Maintenance #14

merged 12 commits into from
Jul 21, 2021

Conversation

mcdonnnj
Copy link
Member

🗣 Description

This PR is a general maintenance update that updates dependencies and does a dash of code cleanup on the entrypoint Python script.

💭 Motivation and context

I need to implement some functionality improvements and at this point the image dependencies are out-of-date and it does not build.

🧪 Testing

I have tested this configuration locally using both docker run and docker-compose. Automated tests also pass.

✅ 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 new and existing tests pass.

mcdonnnj added 12 commits July 16, 2021 16:40
Pipenv is used to manage the requirements for the script that serves as the
entrypoint for the Docker image. Adding it to the development requirements was
an oversight as it is needed to manage the Pipfile files in src/.
Specify the minor Python version we currently build against in the Pipfile. Run
`pipenv update` to update the entrypoint script's requirements.
This updates the Python version specified from 3.9 to the more specific 3.9.6
in this project. This will help ensure repeatable builds because the underlying
Docker image should not change significantly when a new Python patch version is
released.
Update pip, pipenv, setuptools, and wheel versions to their latest in the
Dockerfile. Update the libxml2-dev package to its latest version since it
is a requirement.
The serverless-chrome approach is fraught with dependency woes. It is built for
Amazon Linux 2 usage and running it on a Debian-based image is unwieldy. As a
result we simplify things by installing Chromium with apt and using that binary
instead.
Chrome is a specific build of Chromium by Google. We are not using this so the
argument name should correctly reflect that distinction.
I bumped into this during testing. This way the run should not bomb out at any
point in the process of retrieving a hash for a domain.
The libxml2-dev and libxslt1-dev packages are dependencies of the chromium
package but are required by the Python packages we are installing. If the
chromium dependency were to change it would cause issues in building the Docker
image.
Per https://packages.debian.org/buster/chromium there is no
`90.0.4430.212-1~deb10u1` release available for the `arm64` platform. As a
result we much use the older `89.0.4389.114-1~deb10u1` version for now. I
have not been able to determine why that is the only platform out of the four
for the package that has an older build. The next time dependencies are updated
this will hopefully not be an issue.
Since we have to pin on an older version of the chromium package (from the
viewpoint of some of the platforms) we need to pin the chromium-common package
it depends on to the same version to ensure installation.
Static values like this should be stored in variables to improve visability and
maintainability.
Add information about the two command options for the script.
@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 Jul 21, 2021
@mcdonnnj mcdonnnj self-assigned this Jul 21, 2021
@mcdonnnj mcdonnnj requested review from dav3r, felddy and jsf9k as code owners July 21, 2021 17:11
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.

LGTM! Nice work here. I'm particularly enjoying the use of pipenv.

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.

Outstanding - no notes!
BTW, you did a really great job on your commit messages here - bravo sir. 🥇

@mcdonnnj mcdonnnj merged commit 05967c4 into develop Jul 21, 2021
@mcdonnnj mcdonnnj deleted the maintenance/update_image branch July 21, 2021 19:10
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