Skip to content

Moved the GPG keys to an environment variable #163

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

Closed
wants to merge 1 commit into from

Conversation

LaurentGoderre
Copy link
Member

No description provided.

@Starefossen
Copy link
Member

Is there a benefit of moving the GPG keys to an environment variable that I do not see? This will introduce another environment variable to the global scope which is passed along when extending or running the image.

Also, GPG_KEYS should be renamed to NODE_GPG_KEYS to prevent problems if you combine several images that use the same GPG keys environment variable approach.

@LaurentGoderre
Copy link
Member Author

Good call. I did this because it seems like a trend for images (especially alpine variants, to build the environment with one chained command. Moving the keys to a variables makes it easier to do this (at least, makes the code clearer)

@LaurentGoderre
Copy link
Member Author

Done!

@chorrell
Copy link
Contributor

The respective Dockerfiles should be updated with NODE_GPG_KEYS as well.

@LaurentGoderre
Copy link
Member Author

Ooops, forgot about that

@LaurentGoderre
Copy link
Member Author

Done!

@Starefossen
Copy link
Member

This comment from @yosifkit is applicable for this PR as well:

What was the reason for this change? This previously allowed multiple versions of node to share the gpg fetching layer if they had the same keys and base image, now that is not possible. 😢

@chorrell
Copy link
Contributor

Yes, good point @Starefossen. I'm 👎 on this. This change doesn't seem necessary.

@LaurentGoderre
Copy link
Member Author

Sorry, i was following example of other core docket images. Didn't see these drawback

@LaurentGoderre LaurentGoderre deleted the gpg-keys-env branch April 26, 2016 21:14
@chorrell
Copy link
Contributor

No worries!

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.

3 participants