-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Feature/gitlab deploykey updkey #1661
Feature/gitlab deploykey updkey #1661
Conversation
... the public key
This comment has been minimized.
This comment has been minimized.
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.
You need a changelog fragment.
I think it makes sense to have two sections in the fragment, one with minor_changes
(declaring this a feature) and one with breaking_changes
(warning that the behavior changes). Since the next release is 2.0.0 (next week), changing behavior is accepted, and I think this is a case where a deprecation / extra option to enable/disable this isn't necessary.
@Lunik @Shaps @dj-wasabi @marwatk @scodeman @waheedi @zanssa dear maintainers, what do you think?
@@ -145,6 +145,13 @@ def __init__(self, module, gitlab_instance): | |||
def createOrUpdateDeployKey(self, project, key_title, key_key, options): | |||
changed = False | |||
|
|||
# note: unfortunately public key cannot be updated directly by | |||
# gitlap rest api, so for that case we need to delete and |
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.
# gitlap rest api, so for that case we need to delete and | |
# GitLab REST API, so for that case we need to delete and |
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.
Ok, I added the changelog fragment and fixed the comment. Let me know if I should do anything else.
Can we review the parameters in the test integration file? I think login_token should be replaced with api_token, and server_url should be replaced with api_url! @felixfontein. will take a look during the weekend :) |
@zanssa it uses the same module options in the new integration test entry as in the other integration test entries in the same file. Well, and these are indeed wrong; I guess the integration tests have been broken and wrong for a long time now... The module and the tests didn't change (except maybe details) since they were added to this repo on the initial commit (from the ansible/ansible repo). I'd say we'd ignore the integration tests now. We have until in ~5 hours to decide whether to merge this now, or wait for community.general 3.0.0. |
@felixfontein, okay. Module looks good to me! @morco, thanks for the contribution. |
SUMMARY
This PR makes it possible to update the public key itself of a gitlab project
deploy key (identified by the key title). It can be used for regularly updating
the secret while keeping some kind of identity by reusing the same key title.
We use it together with HashiCorp Vault to short cycle secrets.
Note that this is not directly supported by the gitlab API. We solve this
internally by deleting the old key object first if one exists and than
recreate it with the new public key and the old name.
ISSUE TYPE
COMPONENT NAME
gitlab_deploy_key
ADDITIONAL INFORMATION
Be aware that technically this is a breaking change for edge cases as if beforehand
a different public key value was given as parameter to an existing key title, I'm
pretty sure it simply did nothing while with this PR it obviously actually updates the key
in the server.