Skip to content
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 #351

Merged
merged 12 commits into from
May 6, 2021

Conversation

bkabrda
Copy link
Contributor

@bkabrda bkabrda commented Apr 22, 2021

  • 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

* 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
@bkabrda bkabrda requested review from a team as code owners April 22, 2021 10:05
Copy link

@apigirl apigirl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one small content suggestion

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Contributor

@KSerrania KSerrania left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main thing that's worrying me is that the role cannot be idempotent (since we always perform at least one action, importing the current key, each run). While that does reflect the truth, it may not sit well with customers (we've already had issues open because our role would show as doing a change every run).

However, making the role idempotent seems to be a non-trivial effort: one way to do it could be to import the key, and check if something was really imported or changed in the GPG command output (and if not, mark the task as not changed), but I don't know how feasible / easy that is in practice.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Slavek Kabrda and others added 2 commits April 29, 2021 13:41
Co-authored-by: Kylian Serrania <[email protected]>
@bkabrda
Copy link
Contributor Author

bkabrda commented Apr 29, 2021

@KSerrania that's a good point about idempotency. I think perhaps this might be doable, I'll try to take a look.

@bkabrda
Copy link
Contributor Author

bkabrda commented Apr 29, 2021

@KSerrania I think I managed to implement a reasonable solution. Here's the comment that I included in the code:

# NOTE: in order to display Ansible's `changed: [hostname]` properly throughout
# tasks in this file, we added `changed_when: false` to a lot of them, even if
# they actually run every time (e.g. importing the CURRENT key). The reason is
# that they operate inside a temporary directory and they don't have a
# permanent effect on the host (nothing will actually change on the host
# whether these tasks run or not) except the last one - the actual import of
# the key to `datadog_apt_usr_share_keyring`.

@bkabrda bkabrda merged commit 90f2d41 into master May 6, 2021
@bkabrda bkabrda deleted the slavek.kabrda/apt-keys-improvements branch May 6, 2021 07:44
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.

4 participants