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.

Sign up for GitHub

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

Adds SSL proxy variables to default passenv #1439

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Adds SSL proxy variables to default passenv #1439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Pierre-Luc Tessier Gagné
Ronald Evers
Ronny Pfannschmidt
Selim Belhaouane
Sorin Sbarnea
Sridhar Ratnakumar
Stephen Finucane
Sviatoslav Sydorenko
Expand Down
1 change: 1 addition & 0 deletions docs/changelog/1437.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Adds ``CURL_CA_BUNDLE``, ``REQUESTS_CA_BUNDLE``, ``SSL_CERT_FILE`` to the default passenv values. - by :user:`ssbarnea`
6 changes: 4 additions & 2 deletions docs/config.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,10 @@ Complete list of settings that you can put into ``testenv*`` sections:
Some variables are always passed through to ensure the basic functionality
of standard library functions or tooling like pip:

* passed through on all platforms: ``PATH``, ``LANG``, ``LANGUAGE``,
``LD_LIBRARY_PATH``, ``PIP_INDEX_URL``
* passed through on all platforms: ``CURL_CA_BUNDLE`, ``PATH``,
``LANG``, ``LANGUAGE``,
``LD_LIBRARY_PATH``, ``PIP_INDEX_URL``,
``REQUESTS_CA_BUNDLE``, ``SSL_CERT_FILE``
* Windows: ``SYSTEMDRIVE``, ``SYSTEMROOT``, ``PATHEXT``, ``TEMP``, ``TMP``
``NUMBER_OF_PROCESSORS``, ``USERPROFILE``, ``MSYSTEM``
* Others (e.g. UNIX, macOS): ``TMPDIR``
Expand Down
7 changes: 5 additions & 2 deletions src/tox/config/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,11 +663,14 @@ def passenv(testenv_config, value):
value = list(itertools.chain.from_iterable([x.split(" ") for x in value]))

passenv = {
"PATH",
"PIP_INDEX_URL",
"CURL_CA_BUNDLE",
"LANG",
"LANGUAGE",
"LD_LIBRARY_PATH",
"PATH",
"PIP_INDEX_URL",
"REQUESTS_CA_BUNDLE",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think tox should know things about requests

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asottile If I remember well, even pip is using requests more recently. If we do not include it, it will likely fail to run even pre-commit -- in fact that was why drove me to add them, I was not able to tox -e lint without them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip vendors a ca-cert bundle, which should be active without needing anything right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well not really behind a corporate proxy; as the ca-cert bundle obviously doesn't know about corporate proxy certificates. These usually are set as env-vars though.

"SSL_CERT_FILE",
"TOX_WORK_DIR",
str(REPORTER_TIMESTAMP_ON_ENV),
str(PARALLEL_ENV_VAR_KEY),
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/config/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,8 +1068,11 @@ def test_passenv_as_multiline_list(self, newconfig, monkeypatch, plat):
assert "MSYSTEM" in envconfig.passenv
else:
assert "TMPDIR" in envconfig.passenv
assert "CURL_CA_BUNDLE" in envconfig.passenv
assert "PATH" in envconfig.passenv
assert "PIP_INDEX_URL" in envconfig.passenv
assert "REQUESTS_CA_BUNDLE" in envconfig.passenv
assert "SSL_CERT_FILE" in envconfig.passenv
assert "LANG" in envconfig.passenv
assert "LANGUAGE" in envconfig.passenv
assert "LD_LIBRARY_PATH" in envconfig.passenv
Expand Down
9 changes: 8 additions & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ description = run the tests with pytest under {basepython}
setenv = PIP_DISABLE_VERSION_CHECK = 1
COVERAGE_FILE = {env:COVERAGE_FILE:{toxworkdir}/.coverage.{envname}}
VIRTUALENV_NO_DOWNLOAD = 1
passenv = http_proxy https_proxy no_proxy SSL_CERT_FILE PYTEST_*
passenv =
CURL_CA_BUNDLE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would you need it to set it there if it's done internally 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the tox version used to test tox itself is an older version, one that is not able to
pass these vars. I had to do this locally in order to be able to run tox-on-tox. If I would
install from source before it could have worked.

http_proxy
https_proxy
no_proxy
REQUESTS_CA_BUNDLE
SSL_CERT_FILE
PYTEST_*
deps = pip == 19.1.1
extras = testing
commands = pytest \
Expand Down