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

`--trusted-host` not passed to `pip` when installing from a lockfile #2979

Closed
orf opened this Issue Oct 10, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@orf

orf commented Oct 10, 2018

Issue description

On the latest pipenv release, running pipenv install does not pass --trusted-host to pip.

Running pipenv install xyz does.

Expected result

--trusted-host is passed to pip.

Actual result

> pipenv install --verbose                                
Installing dependencies from Pipfile…
Installing 'aiohttp'▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉▉ 0/7 — 00:00:00
$ ['/Users/tom/.local/share/virtualenvs/tmp.YkTM0kln-68P7VPaZ/bin/pip', 'install', '--verbose', '--upgrade', '--no-deps', '-r', '/var/folders/9w/twrv54nd0v19gsbjhjs3mlhc0000gn/T/pipenv-wgs4narf-requirements/pipenv-h6s3thx1-requirement.txt', '-i', 'http://x.x.co.uk:8080', '--require-hashes']

And:

> pipenv install --verbose pytest
⠋Installing 'pytest'
$ ['/Users/tom/.local/share/virtualenvs/tmp.YkTM0kln-68P7VPaZ/bin/pip', 'install', '--verbose', '--upgrade', 'pytest', '-i', 'http://x.x.co.uk:8080', '--trusted-host', 'x.x.co.uk']

Steps to replicate

Have a Pipfile like this:

[[source]]
url = "http://INTERNAL_PYPI:8080"
verify_ssl = false
name = "xyz"

[packages]
aiohttp = "*"

Notice how the argument is not passed to pip when calling pipenv install.

@orf

This comment has been minimized.

orf commented Oct 10, 2018

I believe db5a862 by @techalchemy (or some change around that, 7429881 / 683df2f) introduced this. This section:

pipenv/pipenv/core.py

Lines 749 to 761 in ddb40ec

# Use a specific index, if specified.
indexes, trusted_hosts, dep = parse_indexes(dep)
index = None
extra_indexes = []
if indexes:
index = indexes[0]
if len(indexes) > 0:
extra_indexes = indexes[1:]
dep = Requirement.from_line(" ".join(dep))
if index:
_index = None
try:
_index = project.find_source(index).get("name")

Ends up calling pip_install with index="some index url", whereas pipenv install xyz calls it with index=None. pip_install will call prepare_pip_source_args with just [{'url': 'some index url'}] if index=string, whereas if index=none the correct [{url: ..., verify_ssl: ..., }] object is passed.

It also seems to correctly parse the trusted_hosts in parse_indexes, but does nothing with it.

Currently pipenv is broken for anyone using verify_ssl: false!

@techalchemy

This comment has been minimized.

Member

techalchemy commented Oct 10, 2018

Thanks for catching this. Is this before the bug fix release goes out! Note that there was another issue on the topic already so it’s helpful to find the actual cause. I think you’re spot on, we just need to actually do something with the correctly parsed trusted host. There is some redundant code there as well, but I’m not confident enough to remove it just now

We have had an endless stream of issues around sources so I’m anxious to have this sorted.

@orf

This comment has been minimized.

orf commented Oct 10, 2018

We can sort this in a slightly hacky way by just passing trusted_hosts to pip_install, and then from pip_install to prepare_pip_source_args where it does the right thing ™️

I don't think this is the best long term solution, but if it sounds OK I could try and prepare a PR?

techalchemy added a commit that referenced this issue Oct 10, 2018

Fix trusted-host passthru
- Fix marker cleaning
- Fixes #2979

Signed-off-by: Dan Ryan <dan@danryan.co>

techalchemy added a commit that referenced this issue Oct 10, 2018

Fix trusted-host passthru
- Fix marker cleaning
- Fixes #2979

Signed-off-by: Dan Ryan <dan@danryan.co>
@royrusso

This comment has been minimized.

royrusso commented Oct 10, 2018

Is there a way to brew install a previous version. Seems like the current version break on trusted hosts, so until this PR is merged... ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment