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

Updates admin Dockerfile Debian Buster as a base image, fixes requirements build #5437

Merged
merged 2 commits into from
Aug 24, 2020

Conversation

zenmonkeykstop
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop commented Aug 5, 2020

Status

Ready for review

Description of Changes

Fixes #5436

  • Updates admin Docker image from Debian Stretch to Buster
  • Pins pip version to 19.1 to work around pip-tools issue
  • Updates admin/bin/dev-shell wrapper for admin Docker container to mount a shared volume with the Docker host, allowing requirements files to be updated on the host.
  • pins six module to 1.15.0 to work around multiple declarations in requirements files

Testing

  • The digest value for the base image in admin/Dockerfile matches the digest found for the latest Buster version via the command: docker pull debian:buster
  • The command make update-admin-pip-requirements completes without error
  • Update a dependency in admin/requirements.in and run make update-pip-requirements:
    • The file admin/requirements.txt includes the updated dependency.
  • hash for six verson 1.15.0 matches the hash in https://github.com/freedomofpress/securedrop-debian-packaging/wiki/six-1.11.0-to-1.15.0

Checklist

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

If you added or updated a code dependency:

Choose one of the following:

@zenmonkeykstop zenmonkeykstop force-pushed the 5436-fix-admin-docker branch from 35c513f to ff4d2f9 Compare August 5, 2020 21:50
@zenmonkeykstop zenmonkeykstop force-pushed the 5436-fix-admin-docker branch 2 times, most recently from 8d4b61e to 6a1db0c Compare August 19, 2020 02:09
@zenmonkeykstop zenmonkeykstop changed the title Updated admin requirements build to write output files to working directory. Updated admin requirements Docker build to use Debian Buster as a base image and output files to working directory. Aug 19, 2020
@zenmonkeykstop zenmonkeykstop changed the title Updated admin requirements Docker build to use Debian Buster as a base image and output files to working directory. Updates admin Dockerfile Debian Buster as a base image, fixes requirements build Aug 19, 2020
Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

  • The digest value for the base image in admin/Dockerfile matches the digest found for the latest Buster version via the command: docker pull debian:buster
  • The command make update-admin-pip-requirements completes without error
  • Update a dependency in admin/requirements.in and run make update-pip-requirements:
    • The file admin/requirements.txt includes the updated dependency.
  • hash for six verson 1.15.0 matches the hash in https://github.com/freedomofpress/securedrop-debian-packaging/wiki/six-1.11.0-to-1.15.0

@@ -23,7 +26,18 @@ function docker_image() {
}

function docker_run() {
docker run --rm -ti ${DOCKER_RUN_ARGUMENTS:-} securedrop-admin "$@"
if [ -z "${CIRCLECI-}" ]
Copy link
Contributor

@rmol rmol Aug 24, 2020

Choose a reason for hiding this comment

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

Had never seen that form of expansion! "When not performing substring expansion, using the forms documented below (e.g., :-), bash tests for a parameter that is unset or null. Omitting the colon results in a test only for a parameter that is unset." Nice.

@rmol rmol merged commit db603ba into develop Aug 24, 2020
@rmol rmol deleted the 5436-fix-admin-docker branch August 24, 2020 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make update-admin-pip-requirements does not output files to the working directory
2 participants