-
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
Git steps: implement auth by user/password #7543
Conversation
50caf48
to
f2ae9eb
Compare
FYI |
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, except single nit.
f2ae9eb
to
d64fad5
Compare
Since calling an async function from a deferred one (with yield) does not await the call, I felt it was less error prone to use the decorator on every async function, it could then be removed in the future when all code is async. Other option would be to either propagate async conversion to caller (recursively) or use (might be helpful to set guidelines for async in #7371?). |
d64fad5
to
9cccd58
Compare
This is surprising. Supporting async functions from inlineCallbacks would be great migration strategy. I will check what's missing from it on twisted side, but this PR does not need to wait. |
I was surprised as well. The only issue was when calling an async function from an inlineCallback with yield. |
9cccd58
to
3321755
Compare
3321755
to
f7982af
Compare
This implement user/password authentication for the Git and GitPush steps (first part of #7384).
Authentication is handled with the builtin git credential flow using the git-credential-store backend.
Credentials are stored using
git credential approve
.An alternative implementation could directly write the credential file that git-credential-store would read, but it's not supposed to be user facing (see doc: "Do not view or edit the file with editors") so might be subject to change in the future.
Whereas the git credential IOFormat is defined and should be stable.
Also, it allow to implement auth through another git-credential backend in the future if needed.
Credentials can be provided to the steps as a simple user/password tuple with
auth_credentials
, in which case, it will complete the auth form with therepourl
the step is supposed to clone.It's also possible to directly provide the
git credential
form throughgit_credentials
(GitCredentialOptions
). This might be needed for repositories that have submodule which are private and require a different set of credentials.GitCredentialOptions
also has ause_http_path
member to override the config value of the same name. This is needed in case multiple credentials are needed for the same host (eg. GIthub repositories owner by different org/user). See doc.I'll implement the same mechanism in
GitPoller
in another PR once this one is accepted as it require some more code change to provide a way forGitServiceAuth
to run commands.PS: Thanks for the
async_to_deferred
helper!Contributor Checklist:
newsfragments
directory (and read theREADME.txt
in that directory)