-
Notifications
You must be signed in to change notification settings - Fork 260
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
Improvements for APT keys management #698
Conversation
* By default, get keys from keys.datadoghq.com, not Ubuntu keyserver * Always add the DATADOG_APT_KEY_CURRENT.public key (contains key used to sign current repodata) * Add 'signed-by' option to all sources list lines * On Debian >= 9 and Ubuntu >= 16, only add keys to /usr/share/keyrings/datadog-archive-keyring.gpg * On older systems, also add the same keyring to /etc/apt/trusted.gpg.d
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.
Looks great to me!
Optional[Boolean] $use_apt_backup_keyserver = $datadog_agent::params::use_apt_backup_keyserver, | ||
String $apt_backup_keyserver = $datadog_agent::params::apt_backup_keyserver, | ||
String $apt_keyserver = $datadog_agent::params::apt_keyserver, |
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.
I think we should keep the options here and just ignore them (or print a warning saying they are deprecated if they are set). Otherwise, if someone was specifying them they will get an error because the argument no longer exists.
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.
Good point, I'll do that.
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.
Note you can also remove the defaults from params.pp
and make them default to undef
.
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.
Looks good to merge if tests agree ✅
What does this PR do?
Motivation
Additional Notes
Describe your test plan