New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 906575 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Goma client proxy handling is confusing

Project Member Reported by raphael....@intel.com, Nov 19

Issue description

This is related to  bug 905122  as well as the discussion in https://groups.google.com/d/msg/goma-early-access/h1lhhicD0mk/yVjVNs2EAwAJ

Right now, getting a Linux machine behind a corporate network proxy to connect to Goma correctly requires getting used to a few quirks. Let's assume the |http_proxy| and |https_proxy| environment variables are set to "http://myproxy.com:1234" and "http://myproxy.com:5678".

* Neither variable is used by goma_auth.py. Instead, one must set the |GOMA_PROXY_HOST| and |GOMA_PROXY_PORT| environment variables, which are not documented in https://chromium.googlesource.com/infra/goma/client/+/master/doc/early-access-guide.md

* Using the "http://" prefix in |GOMA_PROXY_HOST| causes things to fail as well. Instead, only the host part must be used.

* goma_ctl.py, on the other hand, parses the contents of |http_proxy| and |https_proxy| and use them to set |GOMA_PROXY_{HOST,PORT}| (with |https_proxy| having precedence). Contrary to goma_auth.py, it is OK have a scheme in the proxy variable.

* Having |http_proxy| set actually causes goma_ctl.py to fail, as it attempts to connect to the compiler_proxy dashboard through the proxy rather than just connecting to localhost directly. One needs to unset http_proxy and have https_proxy set for everything to work correctly.
 
I am working on making |https_proxy| is enough for using https proxy for both goma_ctl.py and goma_auth.py. i.e. you do not need to set |GOMA_PROXY_HOST| and |GOMA_PROXY_PORT|.


Project Member

Comment 2 by bugdroid1@chromium.org, Nov 20

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/goma/client/+/2532a1c59a7aab5d716d6723d0daad46b3f50fe0

commit 2532a1c59a7aab5d716d6723d0daad46b3f50fe0
Author: Yoshisato Yanagisawa <yyanagisawa@google.com>
Date: Tue Nov 20 06:17:42 2018

With https://chromium.googlesource.com/infra/goma/client/+/a81cdacab5f3977aad13daee5405399070e9f588, you only need to set https_proxy.  You do not need to use GOMA_RPOXY_HOST and GOMA_PROXY_PORT.  It is done by http_init.cc, and we stopped parsing https_proxy in goma_ctl.py.

Can I ask you what is needed to mark this issue fixed?
Owner: yyanagisawa@chromium.org
Status: Assigned (was: Untriaged)
> * Neither variable is used by goma_auth.py. Instead, one must set the |GOMA_PROXY_HOST| and |GOMA_PROXY_PORT| environment variables, which are not documented in https://chromium.googlesource.com/infra/goma/client/+/master/doc/early-access-guide.md

I can confirm setting GOMA_PROXY_{HOST,PORT} are no longer necessary. However, if they are set their values still override those in in http{s}_proxy, and those variables are still undocumented.

> * Using the "http://" prefix in |GOMA_PROXY_HOST| causes things to fail as well. Instead, only the host part must be used.

This is still true, and the behavior differs from the way http{s}_proxy are evaluated.

> * goma_ctl.py, on the other hand, parses the contents of |http_proxy| and |https_proxy| and use them to set |GOMA_PROXY_{HOST,PORT}| (with |https_proxy| having precedence). Contrary to goma_auth.py, it is OK have a scheme in the proxy variable.

As you said, goma_ctl no longer does additional proxy handling, so the behavior is now uniform between goma_auth, goma_ctl and the rest (i.e. http_proxy and https_proxy are evaluated, but GOMA_PROXY_HOST and GOMA_PROXY_PORT still have precedence as I mentioned above).

> * Having |http_proxy| set actually causes goma_ctl.py to fail, as it attempts to connect to the compiler_proxy dashboard through the proxy rather than just connecting to localhost directly. One needs to unset http_proxy and have https_proxy set for everything to work correctly.

This is still an issue. I still need to manually unset http_proxy when using goma_ctl to prevent it from trying to connect to 127.0.0.1 through a proxy.
> I can confirm setting GOMA_PROXY_{HOST,PORT} are no longer necessary. However, if they are set their values still override those in in http{s}_proxy, and those variables are still undocumented.

Since users rarely need to set them directly, I believe the tutorial document should not mention them.  If people want to know all flags, they can see the list of flags in https://chromium.googlesource.com/infra/goma/client/+/master/client/goma_flags.cc

However, we should say that goma_ctl.py and goma_auth.py understand https_proxy in the tutorial instead.

> This is still true, and the behavior differs from the way http{s}_proxy are evaluated.

This is intended behavior.  Its behavior is the same as other GOMA_*_HOST and GOMA_*_PORT behavior.
I believe it fine to give its own semantics for GOMA_* flags, and its semantics should be consistent among GOMA_* flags.

I also prefer to keep these flags' behavior as-is because they can be used for a workaround for following cases:
1. to avoid Goma client URL parser issue.  This do not parse URL, it is free from the issue.
2. to set special HTTP proxy configs only to Goma client.  GOMA_PROXY_{HOST,PORT} is used before https_proxy, this can be used to
   set yet another configs to Goma client.

> This is still an issue. I still need to manually unset http_proxy when using goma_ctl to prevent it from trying to connect to 127.0.0.1 through a proxy.

Can I ask you why you need to connect to 127.0.0.1 with goma_fetch?  I thought all connection to http://127.0.0.1:8088/* uses urllib2 but it is not?
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 26

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/goma/client/+/82280082c6100eac5f58b5ac00a2f27ac11c9da6

commit 82280082c6100eac5f58b5ac00a2f27ac11c9da6
Author: Yoshisato Yanagisawa <yyanagisawa@google.com>
Date: Mon Nov 26 04:16:04 2018

> Can I ask you why you need to connect to 127.0.0.1 with goma_fetch?  I thought all connection to http://127.0.0.1:8088/* uses urllib2 but it is not?

It's actually goma_ctl.py's GomaEnv.ControlCompilerProxy(), not goma_fetch. That method uses urllib2.urlopen to talk to compiler_proxy, and unless "127.0.0.1" is part of the |no_proxy| environment variable it will try to use |http_proxy| to talk to localhost and fail. I had "127.0.0.0" instead (along with a bunch of other entries), but urllib2 apparently needs specific host names or IPs.

While documenting |no_proxy| also works, I think in this case it makes more sense to just call urllib2.urlopen(..., proxies={}) to connect to 127.0.0.1:8088, as I can't think of many cases where a proxy would really be needed for a local connection.
Erm, it's urllib, not urllib2, that takes an optional |proxies| argument in urlopen() :/
Status: Started (was: Assigned)
Thanks for the details explanation.  I could reproduce that behavior, and wrote a cl based on your suggestion.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/goma/client/+/f32535c85dad986645e4418d0c1fc52ca82ef591

commit f32535c85dad986645e4418d0c1fc52ca82ef591
Author: Yoshisato Yanagisawa <yyanagisawa@google.com>
Date: Fri Nov 30 01:22:40 2018

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 6

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/goma/client/+/e62af20e2093a5f84152e29908f3780785290b44

commit e62af20e2093a5f84152e29908f3780785290b44
Author: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Thu Dec 06 04:50:05 2018

I guess this is now fixed since https://chromium.googlesource.com/infra/goma/client/+/f4c90513afd6e8b186b2295dfc6c1adf1efb50b0 -- there are references to https_proxy in early-access-guide.md, and goma_ctl.py uses urllib for talking to compiler_proxy.
Status: Fixed (was: Started)

Sign in to add a comment