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

feature request: system proxy support #407

Closed
himself65 opened this issue Apr 27, 2020 · 7 comments · Fixed by #408
Closed

feature request: system proxy support #407

himself65 opened this issue Apr 27, 2020 · 7 comments · Fixed by #408
Labels
feature request New features for node-core-utils

Comments

@himself65
Copy link
Member

My PC have to use proxy to access GitHub (you know why)

git node --proxy $URL land $PR
# or
git node set proxy="$URL" # set proxy to config
# or
git node --proxy land $PR # use proxy in config

and I will take some time to handle this

@targos
Copy link
Member

targos commented Apr 27, 2020

I think if we support git node set proxy, the configured proxy should then be used automatically, not with an explicit --proxy flag in the commands.

@himself65
Copy link
Member Author

sure, and use proxy config in global .gitconfig is also ok. Just need a way to use proxy whatever it is :|

# in .gitignore
[https]
	proxy = http://127.0.0.1:7890
[http]
	proxy = http://127.0.0.1:7890

@targos
Copy link
Member

targos commented Apr 27, 2020

another way to provide proxy that I have seen is to set the http_proxy and https_proxy environment variables.

@himself65
Copy link
Member Author

I don't think it has any effect... :(
image

@targos
Copy link
Member

targos commented Apr 27, 2020

No, but I mean that we could support that or use a library that does

@joyeecheung
Copy link
Member

joyeecheung commented Apr 27, 2020

We use node-fetch for git-node-land (not sure about other commands) so at least it should be fairly doable by using one of the proxy agent implementations on npm with https://github.com/node-fetch/node-fetch#custom-agent in this abstraction that most of our HTTP requests go through But be aware that users need to configure the proxy setting for the git repository so that git picks it up and that may be tricky, especially for people who use SSH instead of HTTPS (then the setting needs to go to the SSH config, which is global), to make git-node support all of them automatically, we may have to change the setting of the entire system.

Anyway I'd say PRs to make the option effective for our own HTTP requests are welcomed. We could just prompt the users that they need to do something about their own git/ssh configs for everything to go through the proxy and leave it at that at least.

@joyeecheung joyeecheung added the feature request New features for node-core-utils label Apr 27, 2020
@himself65
Copy link
Member Author

I code a simple implementation for self-use, later I will finish the PR

diff --git a/lib/request.js b/lib/request.js
index 3aff3e9..d5cccb5 100644
--- a/lib/request.js
+++ b/lib/request.js
@@ -1,13 +1,15 @@
 'use strict';

 const fetch = require('node-fetch');
+const HttpsProxyAgent = require('https-proxy-agent');
 const fs = require('fs');
 const path = require('path');
 const { CI_DOMAIN } = require('./ci/ci_type_parser');

 class Request {
-  constructor(credentials) {
+  constructor(credentials, proxyUrl) {
     this.credentials = credentials;
+    this.proxyAgent = new HttpsProxyAgent('http://127.0.0.1:7890');
   }

   loadQuery(file) {
@@ -16,6 +18,7 @@ class Request {
   }

   async text(url, options = {}) {
+    options.agent = this.proxyAgent;
     if (url.startsWith(`https://${CI_DOMAIN}`)) {
       options.headers = options.headers || {};
       Object.assign(options.headers, this.getJenkinsHeaders());
@@ -24,6 +27,7 @@ class Request {
   }

   async json(url, options = {}) {
+    options.agent = this.proxyAgent;
     options.headers = options.headers || {};
     options.headers.Accept = 'application/json';
     if (url.startsWith(`https://${CI_DOMAIN}`)) {
@@ -65,6 +69,7 @@ class Request {
     }
     const url = 'https://api.github.com/graphql';
     const options = {
+      agent: this.proxyAgent,
       method: 'POST',
       headers: {
         Authorization: `Basic ${githubCredentials}`,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New features for node-core-utils
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants