-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Use configured CA certificate when downloading packages #3349
Use configured CA certificate when downloading packages #3349
Conversation
return repository | ||
|
||
def _get_links(self, package): # type: (Package) -> List[Link] | ||
repository = self._get_repository(package) | ||
|
||
links = repository.find_links_for_package(package) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the most appropriate way to pass along the repository
attribute to Executor._download_link
? Should I add a repository
attribute to the Link
class? 🤔
link.url, | ||
stream=True, | ||
io=self._sections.get(id(operation), self._io), | ||
verify=getattr(repository, 'cert'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only LegacyRepository
has the cert
attribute, should I have an isinstance()
check or is getattr()
good enough?
Ping @finswimmer @stephsamson any feedback on whether I'm on the right track with this? |
I get this same error, it would be great to get this fixed! That said, here is my work-around that is working, but it just seems unnecessary since this is the same
|
This is obsolete as of the merge of #5719. |
I don't believe this is obsolete. I'm still having the same problem in 1.2b3. If I specify a custom certificate for a private repository (or turn off certificate verification completely for that repository) everything goes fine util it tries to download the wheel. At that point I get a certificate verify failed error. It seems that the certificate.REPO.cert configuration setting applies when communicating with the api for a given repo, but not when downloading the wheels from that repo. |
This PR still conflicts/won't resolve that -- please open an issue with so we can take a look. |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Pull Request Check List
Resolves: possibly #3110 #2593 #1012 (I haven't looked into the nuances)
It seems that #1325 tried to add support to supply custom CA certificates for custom 3rd party repositories (via
poetry config certificates.$repo.cert
).But the implementation was incomplete:
poetry lock
properly uses the correct CA certs for custom repository access, butpoetry install
still fails with a cert error because the package download code path does not use the configured CA certificates.This issue surfaced for us when upgrading Poetry from 1.1.3 to 1.1.4, the minor release had change #3251 that gave our custom repository priority (it was ignored previously); the root cause was challenging to track down.
This PR is a hack to make artifact downloads use configured CA file too. I wanted to submit this first to get feedback whether I'm on the right track.
Error that occurs without this patch: