-
Notifications
You must be signed in to change notification settings - Fork 222
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
Trust new APT and RPM keys #30
Conversation
Allow customizing the URL with `datadog_apt_key_url_new` (for users with local apt mirrors).
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).
- 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? 🙇 |
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_key
moduledatadog_apt_key_url_new
(for userswith local apt mirrors)
RPM
SSL only works when the managed nodes have a recent version of python)
the
sha256sum
option instead ofchecksum
sincechecksum
was onlyadded in Ansible 2.0
rpm_key
module 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.2
and2.1.1
, against Ubuntu 14.04 and CentOS 6