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

Add support for rotating docker secrets #293

Merged
merged 18 commits into from
Feb 12, 2022
Merged

Add support for rotating docker secrets #293

merged 18 commits into from
Feb 12, 2022

Conversation

andrasmaroy
Copy link
Contributor

@andrasmaroy andrasmaroy commented Feb 5, 2022

SUMMARY

Add support for rolling updates for docker secrets. This change does roughly what was described in #21.

Adding two additional parameters for the docker_secret module:

  • rolling_versions (default: false): whether to use versioning secrets for rolling updates.
  • versions_to_keep (default: 5): when using rolling updates, how many of the old versions to keep.

When rolling_versions is set to true secrets are created with a _vX postfix in their names where X is replaced with an incremental counter. That counter is also added to the secrets as a label with the name of ansible_version. This way when a secret is changed a new version is created with an incremented version number instead of deleting it first then creating again. This aims to solve the problem where the secret is attached to a service, thus deleting it fails.

This is the recommended approach of updating secrets based on the official Docker documentation.

The change is not breaking, users of this module shouldn't expect any changes.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

docker_secret

ADDITIONAL INFORMATION

The problem this PR aims to solve is the following:
When a secret is attached to a service docker won't let it get deleted, thus the current version of the module fails execution if one tries to update a secret that is attached to a service already. The recommendation from docker for this is to create a new secret with the updated data then update the service definition adding the new secret in place of the old one. Afterwards the old secret can be safely deleted. Having different names for the secrets is a non-issue since one can define the mountpoint for the secret in the container regardless of its name.

If this change gets accepted I'd implement the same functionality for docker_config as well, as the same issue stands there to fix issue #109.

The PR addresses #21, #246

Fixes #21.

With this change `docker_secret` now supports the case where we store
multiple versions of a secret with the `_v123` postfix.

`absent` state implicitly handles removing these this way.
To make rolling updates actually work instead of failing on trying to
remove a secret that is attached to a service, use the
`versions_to_keep` parameter to remove old versions of the secret after
creating the new one. This way the secret with the new data is created
with a different name and can be attached to the service by its ID
without having to delete the previous one first which would fail if it
is already attached to a service.
Attach the incremental version number to the secret name as a `_v123`
postfix where `123` is replaced with an incremental counter starting
from 1.
A label with the numeric version is also attached to the secret to ease
calculating the new version number upon change with the name
`ansible_version`.
@github-actions
Copy link

github-actions bot commented Feb 5, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@softwarefactory-project-zuul
Copy link

Build succeeded (third-party-check pipeline).

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Please also add a changelog fragment.

plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Show resolved Hide resolved
plugins/modules/docker_secret.py Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link

Build succeeded (third-party-check pipeline).

plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Outdated Show resolved Hide resolved
plugins/modules/docker_secret.py Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link

Build succeeded (third-party-check pipeline).

@softwarefactory-project-zuul
Copy link

Build succeeded (third-party-check pipeline).

@felixfontein
Copy link
Collaborator

I think it looks good now. The only part missing is a changelog fragment.

@softwarefactory-project-zuul
Copy link

Build succeeded (third-party-check pipeline).

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

I think it looks good. I will merge if nobody complains until, say, next Saturday.

@WojciechowskiPiotr
Copy link
Collaborator

shipit :) I'm not merging it to let others review it as well :)

@felixfontein felixfontein merged commit b481fa4 into ansible-collections:main Feb 12, 2022
@felixfontein
Copy link
Collaborator

@andrasmaroy thanks for implementing this!
@WojciechowskiPiotr thanks for reviewing!

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.

docker_secret.py: use secret versioning to avoid secret deletion on update
3 participants