fix: ensure insteadOf gets picked up for submodules

[jhosteny]

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?

[itsdalmo]

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 git@github.com 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://x-oauth-basic@github.com/.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 url.git@github.com:.insteadOf=https://github.com/ submodule update --init --remote --recursive?

[jhosteny]

I'm sorry, that wasn't very clear of me. I mean that our default configuration of the subrepo should use git@github.com, 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 git@github.com 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 git@github.com, 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://x-oauth-basic@github.com/.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 url.git@github.com:.insteadOf=https://github.com/ submodule update --init --remote --recursive?

[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.

[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?

[itsdalmo]

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!

[jhosteny]

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.

[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.

[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.

[rickardl]

@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.

[jhosteny]

Note that the latest change also fixes the issue observed in #234

[mrthehud]

This certainly looks like it would fix the issue I had, but I've not had a chance to test it yet I'm afraid.

Update: I've tested this, and it does fix the issue I had.

[schmurfy]

I got the same issue being unable to fetch a repository with submodules (I got an authentication error), I tried this pr and it fixes my issue too.

[lsjostro]

yes, please! 😄

[mrthehud]

Is there any chance we can get this merged?

[chrs-myrs]

Is this still going in at some point?

[mmwtsn]

We are also running into issues cloning a repository with submodules.

If this is approved, can we merge and release it @itsdalmo @rickardl?

[ezequielgarcia]

@itsdalmo gentle ping on this!