-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
gitlab modules: deprecate basic auth method #8383
gitlab modules: deprecate basic auth method #8383
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
22ebefc
to
1aff691
Compare
This comment was marked as outdated.
This comment was marked as outdated.
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. I seriously doubt there'd still be any GitLab instances out in the wild still supporting basic auth.
Personally I'm also not sure if job token auth makes sense, because none of the current GitLab API resources in the modules here support job token auth. Just for your consideration if you think it would simplify things more. See https://docs.gitlab.com/ee/ci/jobs/ci_job_token.html.
gitlab_job_token = module.params['api_job_token'] |
(not sure if this also warrants a changelog entry)
1aff691
to
e30c53e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Thanks for your review @nejch !
I hope no one still run such old versions. 🤞
You're right, but I don't know if it makes sense to remove a feature which would be re-added later for another module (like a gitlab release creation for example).
EDIT : I added a changelog fragment, since I'm also not sure about how to handle sanity issues on A collection maintainer may help us of the last 3 points. |
e30c53e
to
79b25d7
Compare
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.
Thanks for your contribution!
79b25d7
to
bb27b90
Compare
bb27b90
to
51ab10e
Compare
@felixfontein I just applied your remarks 😉 Should I open another PR removing the deprecated code now, or should I do it once 9.0.0 will be released ?
What do you think about that (see details in the previous comments) ? |
Once this is merged ;) It won't get merged until we have a stable-9 branch, but since that will happen tomorrow, feel free to already create it ;-) |
That's a good question. It would probably be a good idea to move it to a separate docs fragment, and modify the argspec creation in module_utils to add this only when requested. Then we can deprecate it from the current set of modules, but still use it later for specific modules where this is actually needed. What do you think? |
@lgatellier thanks for your contribution! |
Thats a nice solution, thanks for your advice ! |
SUMMARY
This PR deprecates basic auth for GitLab modules, since this auth method has been removed from GitLab a long time ago (in GitLab 9.5, released in August 2017, if I'm right).
It will allow to remove some specific code from
module_utils/gitlab.py
and enhance maintainability.ISSUE TYPE
COMPONENT NAME
gitlab_deploy_key
gitlab_project_badge
gitlab_hook
gitlab_project_variable
gitlab_label
gitlab_instance_variable
gitlab_project_members
gitlab_project_access_token
gitlab_group_access_token
gitlab_merge_request
gitlab_branch
gitlab_project
gitlab_milestone
gitlab_runner
gitlab_protected_branch
gitlab_group_variable
gitlab_user
gitlab_group_members
gitlab_group
gitlab_issue
ADDITIONAL INFORMATION
Another PR will follow to remove this code in
community.general
v9.0.0