-
Notifications
You must be signed in to change notification settings - Fork 231
Trust new APT and RPM keys #30
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
Conversation
Allow customizing the URL with `datadog_apt_key_url_new` (for users with local apt mirrors).
degemer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (but I'm far from being an ansible expert).
tasks/pkg-redhat.yml
Outdated
| - name: Download new RPM key | ||
| get_url: | ||
| url: "http://yum.datadoghq.com/DATADOG_RPM_KEY.public.E09422B3" | ||
| dest: /tmp/DATADOG_RPM_KEY.e09422b3.public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean this file will always be there, even when the rpm key is already imported ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. We could avoid this by first checking if the key is already imported and only download/import the new key if it's not. I didn't do it because I didn't think it was worth the added complexity, but let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's probably not worth it given the size of the key.
Download the new key through plain HTTP for best compatibility (since SSL only works when the managed nodes have a recent version of python). Ensure the integrity of the downloaded file using a sha-256 sum. We use the `sha256sum` option instead of `checksum` since `checksum` was only added in Ansible 2.0 The `rpm_key` module has a good idempotent behavior so no need to have extra steps to check whether the key is already imported.
2a29fde to
dc2874a
Compare
|
Just updated the URL of the new key to @degemer: could you do a quick sanity check of my change? 🙇 |
degemer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awful that Github is not able to show a proper diff. Still LGTM.
|
Is there a reason this wasn't merged? |
Make the pkg managers trust the new keys in addition to the current keys. Similar approach as what's described in DataDog/chef-datadog#365.
APT
apt_keymoduledatadog_apt_key_url_new(for userswith local apt mirrors)
RPM
SSL only works when the managed nodes have a recent version of python)
the
sha256sumoption instead ofchecksumsincechecksumwas onlyadded in Ansible 2.0
rpm_keymodule has a good idempotent behavior so no need to haveextra steps to check whether the key is already imported.
Testing
Tested manually w/ Ansible
1.9.2and2.1.1, against Ubuntu 14.04 and CentOS 6