github-pr-resource fix: ensure insteadOf gets picked up for submodules

Hi @itsdalmo. I know you just changed this for the e2e test, but I'm opening it back up for feedback.

We have some private repos that must use git instead of https. Unless the git config is applied globally, I have found that I am unable to get the insteadOf directive to work on the submodules. I've tried a number of variations on it, including setting GIT_CONFIG and supplying a file identical to the global config. Unfortunately, that doesn't supplement the .git/config, but rather simply overrides it.

I'm wondering if you or anyone else may have suggestions on how to fix? Do the e2e tests break, or does it just leave your local test environment in a bad state?

Apr 29 '20 00:04 jhosteny

Hi! I'm a bit confused by this @jhosteny:

We have some private repos that must use git instead of https.

I thought the current configuration for insteadOf would replace git:// and [email protected] with https://, and the entire point was to use https:// to clone submodules since we only pass a token to the resource (no deploy key)?

On that subject, shouldn't g.command("git", "config", "url.https://.insteadOf", "git://") actually be g.command("git", "config", "url.https://[email protected]/.insteadOf", "git://")? I.e. it needs to include x-oauth-basic? 🤔

Have you tried doing something similar to the comment below this Stackoverflow response? i.e. git -c [email protected]:.insteadOf=https://github.com/ submodule update --init --remote --recursive?

May 27 '20 20:05 itsdalmo

I'm sorry, that wasn't very clear of me. I mean that our default configuration of the subrepo should use [email protected], not https, since developers want to use their SSH keys to perform operations. In CI, however, we use insteadOf to replace that with access via https and access tokens. Your explanation is exactly what we want to do.

Hi! I'm a bit confused by this @jhosteny:

We have some private repos that must use git instead of https.

I thought the current configuration for insteadOf would replace git:// and [email protected] with https://, and the entire point was to use https:// to clone submodules since we only pass a token to the resource (no deploy key)?

Yes, you are correct - we use [email protected], not git, so I forgot this case. I will fix (assuming the below does not work).

On that subject, shouldn't g.command("git", "config", "url.https://.insteadOf", "git://") actually be g.command("git", "config", "url.https://[email protected]/.insteadOf", "git://")? I.e. it needs to include x-oauth-basic? thinking

Hmm - I thought I pretty exhaustively tried all other options, but I don't recall now if I tried this. I will check, and amend the PR if this works.

Have you tried doing something similar to the comment below this Stackoverflow response? i.e. git -c [email protected]:.insteadOf=https://github.com/ submodule update --init --remote --recursive?

May 27 '20 20:05 jhosteny

@itsdalmo actually, I recall now why the git: protocol did not have the x-oauth-basic token specified - you can only clone a public repo with the git: protocol at GH, since git: offers no security. Thus, it could probably just be removed entirely from this PR, as it does not need to be transformed to https.

May 27 '20 21:05 jhosteny

@itsdalmo the method you referenced appears to be working properly (though it is a bit more complicated). I am going to soak this in our environment for awhile with a local version of the resource. Is it possible that the subrepo in your e2e test can be made private to ensure this works correctly?

May 28 '20 00:05 jhosteny

Great to hear @jhosteny!

Is it possible that the subrepo in your e2e test can be made private to ensure this works correctly?

Would that not make it hard for others to run the E2E tests? 🤔

I am going to soak this in our environment for awhile with a local version of the resource.

Roger that, let me know when you feel ready for a review!

May 29 '20 09:05 itsdalmo

Would that not make it hard for others to run the E2E tests? thinking

Yeah - I'm not sure how to handle this case. Is it worth having an extra tagged case that is not run normally, but can run against a private repo? Or just skip it? I suppose if the git config were wrong, git operations would fail on submodules, so there is value as-is.

May 29 '20 12:05 jhosteny

@itsdalmo this has been running successfully for us over the last month or so, if you would like to review. I am not sure how to add tests unless there is tagged set that only get run manually against private sub repos.

Jun 24 '20 13:06 jhosteny

@itsdalmo is there anything I can do on this one to help get it merged? We are running a fork that has this, along with #186, #189 and #224. We're running a number of buildroot builds with numerous nested private submodules, and it has been working well now.

Sep 26 '20 17:09 jhosteny

@jhosteny I'll have look this week, thank you for contribution, I'll investigate making selective test-case for E2E targeting a private repo, which can be optionally overridden locally.