Determine remote endpoint for Git network operations#9139
Merged
outofambit merged 25 commits intodevelopmentfrom Feb 26, 2020
Merged
Determine remote endpoint for Git network operations#9139outofambit merged 25 commits intodevelopmentfrom
outofambit merged 25 commits intodevelopmentfrom
Conversation
Contributor
This may be premature, but I spent some time combing over the areas outlined above to search for potential inconsistencies, however nothing was found. Fetch, push, and pull, checkout, revert, clone, and the tutorial were tested, however LFS and submodules were not at this point. I will continue testing and report back if any out of the ordinary is noticed. |
This caused a "Pulling [Object object]" progress text
fred652
approved these changes
Feb 24, 2020
This comment has been minimized.
This comment has been minimized.
fred652
approved these changes
Feb 24, 2020
outofambit
approved these changes
Feb 26, 2020
Contributor
outofambit
left a comment
There was a problem hiding this comment.
two minor comments, looks good! 📳
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is another continuation of the proxy support started in #9126 and #9127.
Now that we're going to start looking up the proxy to use for Git network operations we're going to have to have an understanding of what the remote endpoint is gonna be for any given network operation.
Corporate proxies are often set up so that a script determines which proxy to use based on the url that the client wants to access (for https urls the script usually only gets the protocol and domain). See #9127 for details on that.
Unfortunately for us there's not a 1:1 correlation between git command and url. Take the simplest case of
git clone URLfor example. While the initial request will be toURLit's possible that the repository could contain submodules pointing to other hosts. It's also possible that the repository is set up to use LFS in which case there may be subsequent requests to a dedicated LFS server.While there might be multiple endpoints accessed by a single Git call we only have the ability to provide Git one proxy url per protocol (http/https) so we're gonna have do do a best-effort guess at a reasonable host. We're also gonna assume that the vast majority of repositories these days do all of their communications over https.
It's also worth noting that the model we've used to provide Git with authentication details (username/password) has been based on the premise that the same credentials will work for any submodule and/or LFS instance accessed during the operation.
I've introduced a substitute method for
envForAuthenticationcalledenvForRemoteOperation. This will now be the centralized location for determining the proxy url. The actual process of setting the required environment variables will come in a follow up pr.Notes for the reviewer
This PR touches sensitive parts of the codebase such as fetch, push, and pull. Please keep a lookout for anything that looks off or alters the behavior of either of these operations in a way not described below.
Affected operations
deleteBranch
deleteBranchis also responsible for deleting the branch on the remote which means that it's essentially apushoperation. Here I've had to introduce a new git operation to load the url of the branch remote. The risk of not being able to resolve the remote url is slim here but there's a fallback anywayExpected altered behavior: One extra Git exec to lookup the remote url of the remote tracking branch
checkout and revert
While not a network operation in Git's eyes it's possible that the repository is set up to use LFS which might have to communicate with its server on checkout or revert. While we can't tell for certain what the LFS server url is (without shelling out to git lfs) it's a pretty safe bet that the account endpoint is a decent substitute given that that's what Git LFS will use to authenticate.
Expected altered behavior: None
clone
We use whatever url is provided as the remote url
Expected altered behavior: None
fetch and fetchRefSpec
We already had access to
IRemoteobjects a few levels up in the call chain to the changes in here are just forwarding that information down rather than only using thenameproperty of theIRemoteExpected altered behavior: None
push
This one got a bit complicated because I really didn't want to risk introducing any new behavior here. See the code for a long comment explaining the tradeoff.
Expected altered behavior: None
Tutorial repository creation
Simply a refactor to pass the known remote url to
envForRemoteOperationExpected altered behavior: None