Skip to content

Changes made in configuration.py to accept environmental variables #2390

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 0 additions & 53 deletions .readthedocs.yaml

This file was deleted.

34 changes: 0 additions & 34 deletions devel/debug_logging.md

This file was deleted.

61 changes: 0 additions & 61 deletions examples/enable_debug_logging.py

This file was deleted.

138 changes: 118 additions & 20 deletions kubernetes/base/stream/ws_client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,70 @@
from .ws_client import get_websocket_url
from .ws_client import websocket_proxycare
from kubernetes.client.configuration import Configuration
import os
import socket
import threading
import pytest
from kubernetes import stream, client, config

try:
import urllib3
urllib3.disable_warnings()
except ImportError:
pass
@pytest.fixture(autouse=True)
def dummy_kubeconfig(tmp_path, monkeypatch):
# Creating a kubeconfig
content = """
apiVersion: v1
kind: Config
clusters:
- name: default
cluster:
server: http://127.0.0.1:8888
contexts:
- name: default
context:
cluster: default
user: default
users:
- name: default
user: {}
current-context: default
"""
cfg_file = tmp_path / "kubeconfig"
cfg_file.write_text(content)
monkeypatch.setenv("KUBECONFIG", str(cfg_file))

def dictval(dict, key, default=None):
try:
val = dict[key]
except KeyError:
val = default
return val

def dictval(dict_obj, key, default=None):

return dict_obj.get(key, default)

class DummyProxy(threading.Thread):
"""
A minimal HTTP proxy that flags any CONNECT request and returns 200 OK.
Listens on 127.0.0.1:8888 by default.
"""
def __init__(self, host='127.0.0.1', port=8888):
super().__init__(daemon=True)
self.host = host
self.port = port
self.received_connect = False
self._server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
self._server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
self._server_sock.bind((self.host, self.port))
self._server_sock.listen(1)

def run(self):
conn, _ = self._server_sock.accept()
try:
data = conn.recv(1024).decode('utf-8', errors='ignore')
if data.startswith('CONNECT '):
self.received_connect = True
conn.sendall(b"HTTP/1.1 200 Connection established\r\n\r\n")
finally:
conn.close()

class WSClientTest(unittest.TestCase):

Expand All @@ -56,21 +107,68 @@ def test_websocket_proxycare(self):
( 'http://proxy.example.com:8080/', 'user:pass', '.example.com', 'proxy.example.com', 8080, ('user','pass'), ['.example.com']),
( 'http://proxy.example.com:8080/', 'user:pass', 'localhost,.local,.example.com', 'proxy.example.com', 8080, ('user','pass'), ['localhost','.local','.example.com']),
]:
# setup input
config = Configuration()
if proxy is not None:
setattr(config, 'proxy', proxy)
if idpass is not None:
setattr(config, 'proxy_headers', urllib3.util.make_headers(proxy_basic_auth=idpass))
# input setup
cfg = Configuration()
if proxy:
cfg.proxy = proxy
if idpass:
cfg.proxy_headers = urllib3.util.make_headers(proxy_basic_auth=idpass)
if no_proxy is not None:
setattr(config, 'no_proxy', no_proxy)
# setup done
# test starts
connect_opt = websocket_proxycare( {}, config, None, None)
self.assertEqual( dictval(connect_opt,'http_proxy_host'), expect_host)
self.assertEqual( dictval(connect_opt,'http_proxy_port'), expect_port)
self.assertEqual( dictval(connect_opt,'http_proxy_auth'), expect_auth)
self.assertEqual( dictval(connect_opt,'http_no_proxy'), expect_noproxy)
cfg.no_proxy = no_proxy


connect_opts = websocket_proxycare({}, cfg, None, None)
assert dictval(connect_opts, 'http_proxy_host') == expect_host
assert dictval(connect_opts, 'http_proxy_port') == expect_port
assert dictval(connect_opts, 'http_proxy_auth') == expect_auth
assert dictval(connect_opts, 'http_no_proxy') == expect_noproxy

@pytest.fixture(scope="module")
def dummy_proxy():
#Dummy Proxy
proxy = DummyProxy(port=8888)
proxy.start()
yield proxy

@pytest.fixture(autouse=True)
def clear_proxy_env(monkeypatch):
for var in ("HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy", "NO_PROXY", "no_proxy"):
monkeypatch.delenv(var, raising=False)

def apply_proxy_to_conf():
#apply HTTPS_PROXY env var and set it as global.
cfg = client.Configuration.get_default_copy()
cfg.proxy = os.getenv("HTTPS_PROXY")
cfg.no_proxy = os.getenv("NO_PROXY", "")
client.Configuration.set_default(cfg)

def test_rest_call_ignores_env(dummy_proxy, monkeypatch):
# HTTPS_PROXY to dummy proxy
monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:8888")
# Avoid real HTTP request
monkeypatch.setattr(client.CoreV1Api, "list_namespace", lambda self, *_args, **_kwargs: None)
# Load config using kubeconfig
config.load_kube_config(config_file=os.environ["KUBECONFIG"])
apply_proxy_to_conf()
# HTTPS_PROXY to dummy proxy
monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:8888")
config.load_kube_config(config_file=os.environ["KUBECONFIG"])
apply_proxy_to_conf()
v1 = client.CoreV1Api()
v1.list_namespace(_preload_content=False)
assert not dummy_proxy.received_connect, "REST path should ignore HTTPS_PROXY"

def test_websocket_call_honors_env(dummy_proxy, monkeypatch):
# set HTTPS_PROXY again
monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:8888")
# Load kubeconfig
config.load_kube_config(config_file=os.environ["KUBECONFIG"])
apply_proxy_to_conf()
opts = websocket_proxycare({}, client.Configuration.get_default_copy(), None, None)
assert opts.get('http_proxy_host') == '127.0.0.1'
assert opts.get('http_proxy_port') == 8888
# Optionally verify no_proxy parsing
assert opts.get('http_no_proxy') is None

if __name__ == '__main__':
unittest.main()
7 changes: 7 additions & 0 deletions kubernetes/client/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import multiprocessing
import sys
import urllib3
import os

import six
from six.moves import http_client as httplib
Expand Down Expand Up @@ -158,9 +159,15 @@ def __init__(self, host="http://localhost",
"""

self.proxy = None
if(os.getenv("HTTPS_PROXY")):self.proxy=os.getenv("HTTPS_PROXY")
if(os.getenv("https_proxy")):self.proxy=os.getenv("https_proxy")
if(os.getenv("HTTP_PROXY")):self.proxy=os.getenv("HTTP_PROXY")
if(os.getenv("http_proxy")):self.proxy=os.getenv("http_proxy")
Copy link
Member

Choose a reason for hiding this comment

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

This file is generated by the upstream openapi generator. Could you make the change upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have checked https://github.com/OpenAPITools/openapi-generator. We need to change in https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache (as it is python code change.). But while I am at it. I observed that there is no no_proxy variable there.
image
I am not sure whether I can add no_proxy variable to it or not. It was removed for some reasons according to this issue OpenAPITools/openapi-generator#20226.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I observed that there is no no_proxy variable there.

This client is based on an old version of the upstream generator. The upstream generator probably had some change to the no_proxy variable since then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of upstreaming the changes can we downstream the changes to files that using configuration.py variables. i.e. python_kubernetes\kubernetes\base\stream\ws_client.py and python_kubernetes\kubernetes\client\rest.py. We can add this change in above files. But I have a doubt will it impact this changes if we use Open-api generator...
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm not sure. From the title "Merge branch 'kubernetes-client:master' into master" it sounds like it was trying to merge the upstream master into your master branch.

An alternative is to open a new PR with a fresh branch, if you cannot delete this commit

Even I cann't delete it. It is not my commit.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roycaihw Can you check and approve my PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roycaihw Can you check and approve my PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@roycaihw Can you check and approve my PR

Copy link
Member

Choose a reason for hiding this comment

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

Even though this client is using a old version of the generator, can we first get this change accepted by the upstream generator, and then we can patch this client?

Copy link
Member

Choose a reason for hiding this comment

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

If both HTTPS_PROXY and HTTP_PROXY are set, does this mean HTTP_PROXY will override HTTPS_PROXY? Is this behavior matching the websocket client behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If both HTTPS_PROXY and HTTP_PROXY are set, HTTP_PROXY will override HTTPS_PROXY. But Web client has only one self.proxy variable. So, I think it will work fine unless user provide both HTTPS_PROXY and HTTP_PROXY. If user needed both variables we need to check that condition. Even then we can just give HTTPS_PROXY priority...

"""Proxy URL
"""
self.no_proxy = None
if(os.getenv("NO_PROXY")):self.no_proxy=os.getenv("NO_PROXY")
if(os.getenv("no_proxy")):self.no_proxy=os.getenv("no_proxy")
Copy link
Member

Choose a reason for hiding this comment

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

Similar question here. Does no_proxy override NO_PROXY if both of them are set?

Copy link
Contributor Author

@p172913 p172913 May 28, 2025

Choose a reason for hiding this comment

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

Yes no_proxy override NO_PROXY. But this is for case sensitive environments like macOS . Only if user declared both no_proxy and NO_PROXY. In windows it will treat both as same. So, I added both no_proxy and NO_PROXY. But they are same...

"""bypass proxy for host in the no_proxy list.
"""
self.proxy_headers = None
Expand Down
Loading