Test-infra: "github.com" hardcoded in git client breaks use of internal github appliance

[hampus77]

What happened:
Specified internal github endpoint for deck and hook and tried to execute a tekton pipeline

What you expected to happen:
A git resource to be created and cloned from our internal github

How to reproduce it (as minimally and precisely as possible):
Create a prowjob that has a presubmit that executes tekton

Please provide links to example occurrences, if any:

Anything else we need to know?:
The custom endpoint flags work as expected, however, "github.com" is hardcoded at https://github.com/kubernetes/test-infra/blob/master/prow/git/git.go#L87

[cjwagner]

The git client you linked to isn't used by the pipeline controller at all. I suspect you are seeing "github.com" used because you are not setting a clone_uri field: https://github.com/kubernetes/test-infra/blob/c69eb89915fca776c67b632049d9b834aaeb74f7/prow/cmd/pipeline/controller.go#L558-L567
Example here: https://github.com/kubernetes/test-infra/blob/cab76bdbd3760143a7b45dc35cf0b4d8d43b6c77/prow/pod-utilities.md#how-to-configure

Please reopen this and link to an example occurrence if you are specifying a clone_uri that is not being respected.
/close

[k8s-ci-robot]

@cjwagner: Closing this issue.

In response to this:

The git client you linked to isn't used by the pipeline controller at all. I suspect you are seeing "github.com" used because you are not setting a clone_uri field: https://github.com/kubernetes/test-infra/blob/c69eb89915fca776c67b632049d9b834aaeb74f7/prow/cmd/pipeline/controller.go#L558-L567
Example here: https://github.com/kubernetes/test-infra/blob/cab76bdbd3760143a7b45dc35cf0b4d8d43b6c77/prow/pod-utilities.md#how-to-configure

Please reopen this and link to an example occurrence if you are specifying a clone_uri that is not being respected.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[hampus77]

I´m sorry, I wasn´t really specific but this problem arise before any pipelines are run. This is in response to an pull_request event when I enter the /trigger command:
Hook logs:

{"author":"xxx","component":"hook","event-GUID":"9b415440-ca38-11e9-87c3-41615571cc0b","event-type":"pull_request","file":"prow/plugins/trigger/generic-comment.go:50","func":"k8s.io/test-infra/prow/plugins/trigger.handleGenericComment","level":"debug","msg":"Comment is made by the bot, skipping.","org":"sebshift","plugin":"trigger","pr":3,"repo":"multitenant-logger","time":"2019-08-29T08:40:08Z","url":"https:///sebshift/multitenant-logger/pull/3 "}

 {"client":"github","component":"hook","file":"prow/github/client.go:522","func":"k8s.io/test-infra/prow/github.(*client).log","level":"info","msg":"GetRef(sebshift, multitenant-logger, heads/master)","time":"2019-08-29T08:40:08Z"}

{"client":"git","component":"hook","file":"prow/git/git.go:148","func":"k8s.io/test-infra/prow/git.(*Client).Clone","level":"info","msg":"Cloning sebshift/multitenant-logger for the first time.","time":"2019-08-29T08:40:08Z"}

{"client":"git","component":"hook","file":"prow/git/git.go:336","func":"k8s.io/test-infra/prow/git.retryCmd","level":"warning","msg":"Running /usr/bin/git [clone --mirror https://xxx:[e-mail]/sebshift/multitenant-logger /tmp/git909188829/sebshift/multitenant-logger.git] returned error exit status 128 with output

Cloning into bare repository '/tmp/git909188829/sebshift/multitenant-logger.git'...nfatal: unable to access 'https://xxx:[e-mail]/sebshift/multitenant-logger/': Could not resolve host: github.comn.","time":"2019-08-29T08:40:09Z"}

[hampus77]

/reopen

[k8s-ci-robot]

@hampus77: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[cjwagner]

@alvaroaleman Looks like introducing a dependency on the Git client into the trigger plugin with this PR is causing some problems because the client assumes github.com.

We might need to make the client use the value from here (or add more values to this struct):
https://github.com/kubernetes/test-infra/blob/a5b618700f2c7275260b948f736a14578e0457ce/prow/config/config.go#L559-L569

See also: https://github.com/kubernetes/test-infra/issues/10146 Prow doesn't fully support GitHub enterprise yet. We'd certainly like to though so feel free to submit a PR to help make this happen!

[alvaroaleman]

@cjwagner TILd about config.GitHubOptions. How exactly do we define its priority vs flagutil.GitHubOptions.endpoints?

I am inclined to say flagutil.GitHubOptions.endpoints makes more sense here, because config.GitHubOptions could be a SSH url which is guaranteed to not work in any of the plugins. It opens up the question which of the endpoints we should use if there is more than one, thought.

[cjwagner]

I am inclined to say flagutil.GitHubOptions.endpoints makes more sense here, because config.GitHubOptions could be a SSH url which is guaranteed to not work in any of the plugins.

There are multiple distinct types of GitHub URLs that I think we're conflating :upside_down_face:

• If I understand the comments correctly, config.GitHubOptions.LinkURL should never be an SSH URL. The comment suggests that it is for linking to GitHub like when generating a link to an OWNERS file. e.g. https://github.com
• flagutil.GitHubOptions.endpoints serves a different purpose. It identifies the API endpoints that the github client should use. e.g. https://api.github.com
• v1.Refs.CloneURI serves a third purpose. It identifies the URI that refs should be cloned from. Only this field could make sense as an SSH URL. e.g. [email protected]:kubernetes/test-infra.git

I think GetPresubmits (and maybe the git client) just need to respect the Refs.CloneURI field.

[alvaroaleman]

I think GetPresubmits (and maybe the git client) just need to respect the Refs.CloneURI field.

How is this supposed to work in case the CloneURI is a SSH uri? AFAIK we do not have SSH keys in any of the plugins and no way to set that up or did I miss that?

Also I think this problem is not really specific to GetPresubmits but is a general problem, e.G. this will keep Tide from working with GH enterprise.

If I understand the comments correctly, config.GitHubOptions.LinkURL should never be an SSH URL. The comment suggests that it is for linking to GitHub like when generating a link to an OWNERS file. e.g. https://github.com

That _sounds to me_ like this would be the ideal candidate then.

[stevekuznetsov]

AFAIK we do not have SSH keys in any of the plugins and no way to set that up or did I miss that?

Only jobs were cloning in the past, I guess. Yikes.

[alvaroaleman]

Only jobs were cloning in the past, I guess. Yikes.

No, that is not true. This is part of at least Tide since we started using it (~1 year ago), it tries to merge PRs prior to forming batches. This implies that it needs to clone the whole thing.

This "just works" thought because it sets the GitHub token as basic auth when performing the requests.

[cjwagner]

If I understand the comments correctly, config.GitHubOptions.LinkURL should never be an SSH URL. The comment suggests that it is for linking to GitHub like when generating a link to an OWNERS file. e.g. https://github.com

That _sounds to me_ like this would be the ideal candidate then.

Why would this be an ideal candidate for the clone URL? My point is that the URL used for linking is distinct from the clone URL. They are not interchangeable.

[alvaroaleman]

Why would this be an ideal candidate for the clone URL

Because:

• We can not use --github-endpoint as that is supposed to be an api endpoint
• We can not use CloneURI because that will break everyone that uses Prow with private repos, as that forces you to put a SSH link into it which won't work for plugins
• config.GitHubOptions will just work if the same base url is used for the GitHub UI and clone uris.

There is a fourth case where different base uris for the Github UI and clone uris are used, but my presumption would be that using config.GitHubOptions.LinkURLFromConfig fixes this for the majority of users. @hampus77 WDYT?

[cjwagner]

I haven't used GitHub enterprise myself, but it seems like it would be coincidental for the link url to match the clone URI? Also wouldn't using config.GitHubOptions fail for private repos in the same way CloneURI would (or do we use basic auth with the token in this case)?

[hampus77]

Isn´t there a way of reusing the urls provided by the payload of the event? Perhaps that involves to big a change?

[alvaroaleman]

Also wouldn't using config.GitHubOptions fail for private repos in the same way CloneURI would (or do we use basic auth with the token in this case)?

We do use it for basic auth, thats why it works for private repos:

• https://github.com/kubernetes/test-infra/blob/fe9f237a8d30b76255dea35f9ca501c9afe0b921/prow/flagutil/github.go#L140
• https://github.com/kubernetes/test-infra/blob/fe9f237a8d30b76255dea35f9ca501c9afe0b921/prow/git/git.go#L141

Isn´t there a way of reusing the urls provided by the payload of the event? Perhaps that involves to big a change?

I guess this is not impossible but it would be a pretty big change, as we would have to basically change everything that uses the gitClient to use the clone URL from the event. Also, I am not sure if we have an event everywhere we use that git client.

@hampus77 Does (your) GitHub enterprise use the same url for the UI links as for the clone links? Do you know if its reasonable to assume thats the case or is it common to have it set up differently?

[hampus77]

I think it works pretty much like github.com, not sure really if there are any options in setting it up differently

[stevekuznetsov]

/assign

[jonico]

Does (your) GitHub enterprise use the same url for the UI links as for the clone links? Do you know if its reasonable to assume thats the case or is it common to have it set up differently?

@alvaroaleman: As a GitHub staff member I can confirm: A GitHub UI repository URL on GitHub Enterprise Server is always the same URL as the http(s) clone URL, no matter how GitHub Enterprise Server is configured.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.