Skip to content
This repository has been archived by the owner on Jan 22, 2024. It is now read-only.

Changing dockerfiles to use gpg key setup #3

Closed
wants to merge 4 commits into from

Conversation

ruffsl
Copy link
Contributor

@ruffsl ruffsl commented Nov 12, 2015

Just some suggested improvements for security,
See official image docs for motivation

Changes include:

  • Using apt-key with key server and key fingerprint, see security
  • Cleaning up apt-get after install and only update only once, again see security
  • Pinning package version, see repeatability
  • Adding comments, see clarity

Great job guys, really appreciate the work here!
Looking forward to see what this will become.

@flx42
Copy link
Member

flx42 commented Nov 12, 2015

Thank you for contribution!

A few comments:

  • You need to sign the CLA before we can accept your code, please look at this section:
    https://github.com/NVIDIA/nvidia-docker#issues-and-contributing
  • As you said yourself, you have 3 changes, so it would be nice to have 3 different commits, one for each fix/feature.
  • You are absolutely right about the security improvement. But we might end up with a different solution like downloading the GPG key through a https endpoint. We are currently looking into it!
  • I don't think the comments are bringing value, they are merely describing the commands in plain English.
  • I agree with grouping apt-get update and apt-get install and the cleanup in the end.

@3XX0 do you see something else?

So, while we are evaluating a better solution for adding the key, could you create a new commit with the apt-get change and also send us the CLA?

@ruffsl
Copy link
Contributor Author

ruffsl commented Nov 12, 2015

@3XX0
If you do update the public key, wouldn't you want this to break? Just like if you bumped up the version the cuda toolkit. This makes sure the maintainer would explicitly specify update the Dockerfile to reflect these changes in the source of downloaded packages, leaving a committed "paper trail" to notify users and more importantly breaking docker's internal catch found in user engines as well as in Docker Hub. What alternative would you have in mind?

Also, I think the docker version of ubuntu has some modifications to its apt-get that might clean implicitly:
https://github.com/tianon/docker-brew-ubuntu-core/blob/6cf0ef2af6762b751cc95ad6576a9f4eb16a7cbd/trusty/Dockerfile
But we'd have to ask tianon.

@3XX0
Copy link
Member

3XX0 commented Nov 13, 2015

Merging 1712056 and 02ad0f3.
We took the GPG issue into consideration and will address it shortly in a separate commit.

@3XX0 3XX0 closed this in 34b4d80 Nov 13, 2015
3XX0 pushed a commit that referenced this pull request Nov 13, 2015
3XX0 added a commit that referenced this pull request Nov 13, 2015
@tfboyd tfboyd mentioned this pull request Feb 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants