Support optional BOTO specification with --no_auth.

Support optional BOTO specification with --no_auth.

This prevents --no_auth from always clearing BOTO_CONFIG, since there
are times when a BOTO is needed for other things than just auth info
(e.g. proxy settings).

BUG=443523
R=hinoka@chromium.org, szager@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=293652

[Raphael Kubo da Costa (rakuco), 2015-01-12 23:23:28]

raphael.kubo.da.costa@intel.com changed reviewers:
+ raphael.kubo.da.costa@intel.com

[Raphael Kubo da Costa (rakuco), 2015-01-12 23:23:28]

If that matters, I've verified it worked here behind my corporate proxy. Perhaps
there's a better name than NO_AUTH_BOTO_CONFIG, as it sounds like "BOTO_CONFIG
without authentication".

While I have your attention: is it really necessary to unset BOTO_CONFIG or
should we give it a try and stop doing this now that we're using a newer gsutil?

[Michael Moss, 2015-01-12 23:31:22]

On 2015/01/12 23:23:28, Raphael Kubo da Costa (rakuco) wrote:
> If that matters, I've verified it worked here behind my corporate proxy.
Perhaps
> there's a better name than NO_AUTH_BOTO_CONFIG, as it sounds like "BOTO_CONFIG
> without authentication".
> 
> While I have your attention: is it really necessary to unset BOTO_CONFIG or
> should we give it a try and stop doing this now that we're using a newer
gsutil?

I'm not entirely sure of the initial rationale. The change came from
https://codereview.chromium.org/76583002, which was to fix
https://code.google.com/p/chromium/issues/detail?id=321254, but since specifying
the BOTO works for you, it clearly isn't broken in all cases. Maybe hinoka can
comment on if it's still necessary, or if there's a better fix.

We could also just go with this as a minimally intrusive/dangerous way to get
things working again, and worry about validating a full revert of --no_auth, or
some other, more surgical change, later.

[Raphael Kubo da Costa (rakuco), 2015-01-12 23:35:36]

FWIW, I don't have access to
https://code.google.com/p/chromium/issues/detail?id=321254 :(

[Michael Moss, 2015-01-12 23:40:15]

On 2015/01/12 23:35:36, Raphael Kubo da Costa (rakuco) wrote:
> FWIW, I don't have access to
> https://code.google.com/p/chromium/issues/detail?id=321254 :(

Doesn't look sensitive. Removed view restriction.

[Raphael Kubo da Costa (rakuco), 2015-01-12 23:46:43]

Ah, CrOS. vapier@ was involved in the gsutil issue
(https://github.com/GoogleCloudPlatform/gsutil/issues/241) from a CrOS
perspective and from there and the Chromium but it's not clear if they can fix
their .boto files. I guess at least for now we'll have to settle with an
intermediate solution indeed.

[Michael Moss, 2015-01-14 16:45:19]

hinoka: ping

[hinoka, 2015-01-14 18:31:06]

What about https://codereview.chromium.org/841823002/ ?  Do you prefer one
solution over the other?

[Michael Moss, 2015-01-14 18:36:12]

On 2015/01/14 18:31:06, hinoka wrote:
> What about https://codereview.chromium.org/841823002/ ?  Do you prefer one
> solution over the other?

I prefer this one since it's a both a no-op for anyone that doesn't
intentionally set the new env var, and because it doesn't involve passwords as
command-line args, which is something we should really avoid.

[hinoka, 2015-01-14 18:50:15]

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage.py
File download_from_google_storage.py (right):

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:388: os.environ.get('NO_AUTH_BOTO_CONFIG')):
Lets only trigger this if proxy settings are detected.  Otherwise this message
will show up literally everywhere.

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:391: 'it from being used.\n'
Split this into two print statements. '\n' isn't technically windows friendly.

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:394: 'BOTO_CONFIG as NO_AUTH_BOTO_CONFIG
instead.')
s/instead/also/

[Michael Moss, 2015-01-14 20:04:05]

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage.py
File download_from_google_storage.py (right):

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:388: os.environ.get('NO_AUTH_BOTO_CONFIG')):
On 2015/01/14 18:50:15, hinoka wrote:
> Lets only trigger this if proxy settings are detected.  Otherwise this message
> will show up literally everywhere.

Everyone exports BOTO_CONFIG? The case I'm concerned about it somebody exporting
BOTO_CONFIG, expecting proxying to work, and wonder why it's not. This should
make it pretty clear what's going on.

[hinoka, 2015-01-14 20:11:40]

Also this won't catch the case where boto_config isn't exported (defaulting to
~/.boto), but proxy is set.

[hinoka, 2015-01-14 20:12:22]

All the bots exports boto_config, so this would print a scary but not terribly
useful message for the bots, since none of them are behind a proxy.

[Michael Moss, 2015-01-14 21:05:11]

On 2015/01/14 20:11:40, hinoka wrote:
> Also this won't catch the case where boto_config isn't exported (defaulting to
> ~/.boto), but proxy is set.

Eh, this is not meant to be a comprehensive fix (that's the upstream gsutil
fix), so I'm not too worried about catching every case. I just want it to be a
little more helpful where it seems like the user is intending to do something
that's not going to work, and I take explicitly setting BOTO_CONFIG as a proxy
for intent (as opposed to default ~/.boto handling).

As you point out though, that doesn't work for bots, since they always set
BOTO_CONFIG. What if I skip this if CHROME_HEADLESS is set, as I think it always
is on bots?

I also didn't really want to parse the boto file looking for specific settings
that the user might intend to pass through, since I don't know what all those
settings might be (i.e. even though we're talking about proxy settings on this
bug, there are plenty of other settings in the boto file, most of which I have
no clue about).

[Michael Moss, 2015-01-14 21:05:44]

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage.py
File download_from_google_storage.py (right):

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:391: 'it from being used.\n'
On 2015/01/14 18:50:15, hinoka wrote:
> Split this into two print statements. '\n' isn't technically windows friendly.

Done.

https://codereview.chromium.org/844373002/diff/1/download_from_google_storage...
download_from_google_storage.py:394: 'BOTO_CONFIG as NO_AUTH_BOTO_CONFIG
instead.')
On 2015/01/14 18:50:15, hinoka wrote:
> s/instead/also/

Done.

[hinoka, 2015-01-15 00:59:13]

Sorry when I said proxy is set, I meant the env var HTTP_PROXY (and its
permutations).  The existence of that env var is a strong signal that the user
wants some different treatment.

[Michael Moss, 2015-01-15 04:49:24]

On 2015/01/15 00:59:13, hinoka wrote:
> Sorry when I said proxy is set, I meant the env var HTTP_PROXY (and its
> permutations).  The existence of that env var is a strong signal that the user
> wants some different treatment.

Makes sense. PTAL.

[hinoka, 2015-01-15 18:37:14]

lgtm

[szager1, 2015-01-15 18:57:40]

szager@chromium.org changed reviewers:
+ szager@chromium.org

[szager1, 2015-01-15 18:57:40]

lgtm

[Michael Moss, 2015-01-15 19:00:52]

Committed patchset #3 (id:40001) manually as 293652 (presubmit successful).