Skip to content
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

Respect https_proxy environment variables by default. #2098

Closed
anzboi opened this issue May 20, 2021 · 8 comments
Closed

Respect https_proxy environment variables by default. #2098

anzboi opened this issue May 20, 2021 · 8 comments
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs
Projects

Comments

@anzboi
Copy link

anzboi commented May 20, 2021

What’s missing?

Currently, octokit requires code in order to respect https_proxy environment variables, which looks something like this.

let agent = {};
const proxy = process.env.https_proxy || process.env.HTTPS_PROXY;
if (proxy) {
    agent = new HttpsProxyAgent(proxy)
}

export const MyOctokit = Octokit.defaults({
    request: { agent: agent }
})

It would be nice if octokit can automatically read https_proxy (and the others) and respect those environment variables.

Why?

Not having this feature creates a problem with the proliferation of third party tools that don't specifically add something like the above code to enable running behind a proxy.

My specific use case involves github actions. We have requirements to run actions on self-hosted runners that sit behind a proxy. We are able to make requests from our runners to the github API through the proxy, but any action that uses octokit and does NOT include the code above fails.

Alternatives you tried

We are currently forced to fork and modify every javascript action that uses octokit just to get the action to work on our self-hosted runners that sit behind a proxy. This process is somewhat time-consuming to do, and VERY time consuming keep forks up to date with the base repo. There are instances of other, non-github action tools where we have had to do the same as well.

@anzboi anzboi added the Type: Feature New feature or request label May 20, 2021
@ghost ghost added this to Features in JS May 20, 2021
@gr2m gr2m added Type: Support Any questions, information, or general needs around the SDK or GitHub APIs and removed Type: Feature New feature or request labels May 20, 2021
@ghost ghost moved this from Features to Support in JS May 20, 2021
@gr2m
Copy link
Contributor

gr2m commented May 20, 2021

A https_proxy environment variable is only relevant to node and maybe Deno, but not browsers, so it's not something we'd add to Octokit core.

But you can quite easily create your own Octokit constructor which respects the environment variable. Try this

// octokit plugin
function autoProxyAgent(octokit, options) {
  const proxy = process.env.https_proxy || process.env.HTTPS_PROXY;
  if (!proxy) return

  const agent = new HttpsProxyAgent(proxy)
  octokit.hook.before("request", (options) => {
    options.request.agent = agent
  });
}

const MyOctokit = Octokit.plugin(autoProxyAgent);
const octokit = new MyOctokit();

You could publish the autoProxyAgent as npm package, it might be useful for others who have a similar use case to yours? You can run npm init octokit-project to bootstrap an @octokit plugin package & repository

@anzboi
Copy link
Author

anzboi commented May 20, 2021

Unfortunately those fixes aren't going to work for my use cases. The issue is not with my code, rather with tools already written by third parties. Your suggested fix is essentially what I have been doing, fork the third party code and modify to respect proxy, I was just hoping for a better solution.

Thanks for the explanation as to why this wouldn't be added though. I suppose in this case the responsibility is upon authors to make sure their tools are widely useable for their intended purpose.

@anzboi anzboi closed this as completed May 20, 2021
JS automation moved this from Support to Done May 20, 2021
@peter-evans
Copy link

@gr2m Thank you for the snippet to support https_proxy with Octokit. Very useful.

At the moment, most GitHub actions are broken for those that run self-hosted runners behind a corporate proxy. It might be worth having an official Octokit plugin for "autoProxyAgent" because I think ideally it should be included in the Actions Tookit github package so that many GitHub actions will get this functionality for free.

Could be included here:
https://github.com/actions/toolkit/blob/main/packages/github/src/utils.ts#L20-L23

@gr2m
Copy link
Contributor

gr2m commented May 23, 2021

Good point! I’ll incorporate it into @octokit/action, I want to rework this one anyway

@gr2m
Copy link
Contributor

gr2m commented May 23, 2021

you can follow octokit/action.js#300 for updates

@peter-evans
Copy link

@gr2m Just to clarify, can we expect a standalone autoProxyAgent plugin, similar to retry, paginateRest, etc.? It sounds like you intend to just bake it into @octokit/action, rather than make it available to any project constructing Octokit from @octokit/core.

@gr2m
Copy link
Contributor

gr2m commented May 24, 2021

Not sure yet. But you can use the above code as plug-in already

@peter-evans
Copy link

Yep, that's what I'm intending to do here. I just thought it would be easier to support this feature in the wider community if it was standalone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Support Any questions, information, or general needs around the SDK or GitHub APIs
Projects
No open projects
JS
  
Done
Development

No branches or pull requests

3 participants