Extend ProxyHandler to support CIDR ranges for no_proxy #83085
Conversation
ansibot
added
feature
ansibot
removed
the
needs_revision
bcoca
removed
the
needs_triage
| return False | ||
| return host in bypass_network | ||
|
|
||
| def proxy_open(self, req, *args): |
There was a problem hiding this comment.
The biggest concern I have about this implementation, is in overriding this method. proxy_open is not a documented method at https://docs.python.org/3/library/urllib.request.html
At closest it the <protocol>_open methods are described as part of https://docs.python.org/3/library/urllib.request.html#protocol-open but proxy isn't really a protocol like http, https, file, etc are.
And further more, it's highly dependent on urllib.request.ProxyHandler.__init__: https://github.com/python/cpython/blob/8f25cc992021d6ffc62bb110545b97a92f7cb295/Lib/urllib/request.py#L770-L774
However, the use of proxy_open has been around, unchanged for 25+ years, so it seems relatively safe and stable: python/cpython@6d7e47b
I guess at a minimum, this is why we have tests, which this PR includes.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -0,0 +1,35 @@ | |||
| - name: install proxy.py | |||
| pip: | |||
| name: proxy.py | |||
There was a problem hiding this comment.
It might be a good idea to pin it to the most recent release candidate (otherwise it may end up being pretty old). FYI.
| @@ -0,0 +1,2 @@ | |||
| - name: stop proxy.py | |||
| command: kill {{ proxy_py_pid.content|b64decode }} | |||
There was a problem hiding this comment.
Could you explain why is re-encoding of a.. number is needed? I'm a bit confused here. Oh, is it because it was produced by a slurp? Can we decode and put into a var right after reading the file? This piece of logic seems disconnected...
There was a problem hiding this comment.
It is because of slurp. Technically we could do a set_fact afterwards. Once we get the proejctions feature in, maybe for 2.18 we can do it as part of the register itself.
ansibot
added
the
needs_revision
|
The test |
sivel
force-pushed
the
urls-no-proxy-cidr
branch
from
8b4c2a9
to
9a3803d
Compare
sivel
force-pushed
the
urls-no-proxy-cidr
branch
from
9a3803d
to
2447f66
Compare
ansibot
removed
the
needs_revision
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
ansibot
added
needs_revision
SUMMARY
Extend ProxyHandler to support CIDR ranges for no_proxy
ISSUE TYPE
ADDITIONAL INFORMATION