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.

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

Set validate_certs to API call to avoid effects from environment vars #1184

Merged
merged 1 commit into from
Apr 17, 2021

Conversation

saito-hideki
Copy link
Contributor

Summary

If REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE has been set as an environment variables, requests will use those variables with API call even if validate_certs set False.

This PR addresses the above issue.

@evgeni
Copy link
Member

evgeni commented Apr 16, 2021

I would have expected

self.session.verify = self.get_option('validate_certs')
to be sufficient?

What would I need to reproduce this? Just point REQUESTS_CA_BUNDLE env var at some CA that hasn't singed my Foreman?

@evgeni
Copy link
Member

evgeni commented Apr 16, 2021

indeed, the following script allows to repro:

import requests

s = requests.Session()
s.verify = False

s.get('https://somehost.example.com/')

@evgeni
Copy link
Member

evgeni commented Apr 16, 2021

psf/requests#5209 / psf/requests#3829 is a known issue

@saito-hideki would you mind adding a reference to the requests issue as a comment to the code? it's good to be merged then!

* Addresses an issue where "requests" uses REQUESTS_CA_BUNDLE or
   CURL_CA_BUNDLE values for API call even if validate_certs set False

Signed-off-by: Hideki Saito <saito@fgrep.org>
@saito-hideki
Copy link
Contributor Author

@evgeni thank you for your review. I have added references about Requests issues. please let me know if I have missed something.

@evgeni evgeni merged commit ab4faa1 into theforeman:develop Apr 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants