diff --git a/README.md b/README.md index f77d95cc6f..0e927c6482 100644 --- a/README.md +++ b/README.md @@ -96,19 +96,16 @@ export OPENSHIFT_PYTHON_WRAPPER_LOG_LEVEL= # can be: "DEBUG", "INFO", ## Proxy Enablement This configuration allows the client to route traffic through a specified proxy server. -It can be enabled via the environment variable `OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY`. To enable proxy configuration for the client: -1. Set the environment variable `OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY=` +1. Define either `HTTPS_PROXY` or `HTTP_PROXY` environment variable with your proxy URL: -2. Define either `HTTPS_PROXY` or `HTTP_PROXY` environment variable with your proxy URL: ```bash export HTTPS_PROXY="http://proxy.example.com:8080" # or export HTTP_PROXY="http://proxy.example.com:8080" ``` -If neither variable is set when proxy is enabled, a `ValueError` will be raised. ## Code check diff --git a/ocp_resources/resource.py b/ocp_resources/resource.py index 9fe83241c8..fe87aef0e8 100644 --- a/ocp_resources/resource.py +++ b/ocp_resources/resource.py @@ -77,10 +77,13 @@ def _get_api_version(dyn_client: DynamicClient, api_group: str, kind: str) -> st def get_client( - config_file: str = "", + config_file: str | None = None, config_dict: dict[str, Any] | None = None, - context: str = "", - **kwargs: Any, + context: str | None = None, + client_configuration: kubernetes.client.Configuration | None = None, + persist_config: bool = True, + temp_file_path: str | None = None, + try_refresh_token: bool = True, ) -> DynamicClient: """ Get a kubernetes client. @@ -97,19 +100,31 @@ def get_client( config_file (str): path to a kubeconfig file. config_dict (dict): dict with kubeconfig configuration. context (str): name of the context to use. + persist_config (bool): whether to persist config file. + temp_file_path (str): path to a temporary kubeconfig file. + try_refresh_token (bool): try to refresh token Returns: DynamicClient: a kubernetes client. """ + proxy = os.environ.get("HTTPS_PROXY") or os.environ.get("HTTP_PROXY") + + client_configuration = client_configuration or kubernetes.client.Configuration() + + if not client_configuration.proxy and proxy: + LOGGER.info(f"Setting proxy from environment variable: {proxy}") + client_configuration.proxy = proxy + # Ref: https://github.com/kubernetes-client/python/blob/v26.1.0/kubernetes/base/config/kube_config.py if config_dict: - return kubernetes.dynamic.DynamicClient( - client=kubernetes.config.new_client_from_config_dict( - config_dict=config_dict, context=context or None, **kwargs - ) + _client = kubernetes.config.new_client_from_config_dict( + config_dict=config_dict, + context=context, + client_configuration=client_configuration, + persist_config=persist_config, + temp_file_path=temp_file_path, ) - client_configuration = kwargs.get("client_configuration", kubernetes.client.Configuration()) - try: + else: # Ref: https://github.com/kubernetes-client/python/blob/v26.1.0/kubernetes/base/config/__init__.py LOGGER.info("Trying to get client via new_client_from_config") @@ -118,36 +133,22 @@ def get_client( # is populated during import which comes before setting the variable in code. config_file = config_file or os.environ.get("KUBECONFIG", "~/.kube/config") - if os.environ.get("OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY"): - proxy = os.environ.get("HTTPS_PROXY") or os.environ.get("HTTP_PROXY") - if not proxy: - raise ValueError( - "Proxy configuration is enabled but neither HTTPS_PROXY nor HTTP_PROXY environment variables are set." - ) - if client_configuration.proxy and client_configuration.proxy != proxy: - raise ValueError( - f"Conflicting proxy settings: client_configuration.proxy={client_configuration.proxy}, " - f"but the environment variable 'HTTPS_PROXY/HTTP_PROXY' defines proxy as {proxy}." - ) - client_configuration.proxy = proxy - - kwargs["client_configuration"] = client_configuration - - return kubernetes.dynamic.DynamicClient( - client=kubernetes.config.new_client_from_config( - config_file=config_file, - context=context or None, - **kwargs, - ) + _client = kubernetes.config.new_client_from_config( + config_file=config_file, + context=context, + client_configuration=client_configuration, + persist_config=persist_config, ) + + try: + return kubernetes.dynamic.DynamicClient(client=_client) except MaxRetryError: # Ref: https://github.com/kubernetes-client/python/blob/v26.1.0/kubernetes/base/config/incluster_config.py LOGGER.info("Trying to get client via incluster_config") return kubernetes.dynamic.DynamicClient( client=kubernetes.config.incluster_config.load_incluster_config( - client_configuration=client_configuration, - try_refresh_token=kwargs.get("try_refresh_token", True), - ) + client_configuration=client_configuration, try_refresh_token=try_refresh_token + ), ) @@ -429,9 +430,9 @@ def __init__( dry_run: bool = False, node_selector: dict[str, Any] | None = None, node_selector_labels: dict[str, str] | None = None, - config_file: str = "", + config_file: str | None = None, config_dict: dict[str, Any] | None = None, - context: str = "", + context: str | None = None, label: dict[str, str] | None = None, annotations: dict[str, str] | None = None, api_group: str = "", @@ -486,11 +487,6 @@ def __init__( self.node_selector = node_selector self.node_selector_labels = node_selector_labels self.config_file = config_file - if not isinstance(self.config_file, str): - # If we pass config_file which isn't a string, get_client will fail and it will be very hard to know why. - # Better fail here and let the user know. - raise ValueError("config_file must be a string") - self.config_dict = config_dict or {} self.context = context self.label = label @@ -967,10 +963,10 @@ def retry_cluster_exceptions( def get( cls, config_file: str = "", - context: str = "", singular_name: str = "", exceptions_dict: dict[type[Exception], list[str]] = DEFAULT_CLUSTER_RETRY_EXCEPTIONS, raw: bool = False, + context: str | None = None, dyn_client: DynamicClient | None = None, *args: Any, **kwargs: Any, @@ -1196,7 +1192,7 @@ def events( def get_all_cluster_resources( client: DynamicClient | None = None, config_file: str = "", - context: str = "", + context: str | None = None, config_dict: dict[str, Any] | None = None, *args: Any, **kwargs: Any, @@ -1333,10 +1329,10 @@ def __init__( def get( cls, config_file: str = "", - context: str = "", singular_name: str = "", exceptions_dict: dict[type[Exception], list[str]] = DEFAULT_CLUSTER_RETRY_EXCEPTIONS, raw: bool = False, + context: str | None = None, dyn_client: DynamicClient | None = None, *args: Any, **kwargs: Any, diff --git a/tests/test_resources.py b/tests/test_resources.py index 110cfeb874..4f9bd34f69 100644 --- a/tests/test_resources.py +++ b/tests/test_resources.py @@ -1,7 +1,8 @@ +from __future__ import annotations + import pytest import yaml from docker.errors import DockerException -import kubernetes from testcontainers.k3s import K3SContainer from ocp_resources.exceptions import ResourceTeardownError @@ -19,13 +20,18 @@ def clean_up(self, wait: bool = True, timeout: int | None = None) -> bool: return False -@pytest.fixture(scope="session") -def client(): +@pytest.fixture(scope="class") +def k3scontainer_config(): try: with K3SContainer() as k3s: - yield get_client(config_dict=yaml.safe_load(k3s.config_yaml())) - except DockerException: - pytest.skip("K3S container not available") + yield yaml.safe_load(k3s.config_yaml()) + except DockerException as ex: + pytest.skip(f"K3S container not available. {ex}") + + +@pytest.fixture(scope="class") +def client(k3scontainer_config): + yield get_client(config_dict=k3scontainer_config) @pytest.fixture(scope="class") @@ -109,25 +115,21 @@ def test_resource_context_manager_exit(self, client): with SecretTestExit(name="test-context-manager-exit", namespace="default", client=client): pass - def test_proxy_enabled_but_no_proxy_set(self, monkeypatch): - monkeypatch.setenv(name="OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY", value="1") - with pytest.raises( - ValueError, - match="Proxy configuration is enabled but neither HTTPS_PROXY nor HTTP_PROXY environment variables are set.", - ): - get_client() +class TestClient: + def test_client_with_proxy(self, monkeypatch, k3scontainer_config): + http_proxy = "http://env-http-proxy.com" + monkeypatch.setenv(name="HTTPS_PROXY", value=http_proxy) - def test_proxy_conflict_raises_value_error(self, monkeypatch): - monkeypatch.setenv(name="OPENSHIFT_PYTHON_WRAPPER_CLIENT_USE_PROXY", value="1") - monkeypatch.setenv(name="HTTPS_PROXY", value="http://env-proxy.com") + client = get_client(config_dict=k3scontainer_config) + assert client.configuration.proxy == http_proxy - client_configuration = kubernetes.client.Configuration() - client_configuration.proxy = "http://not-env-proxy.com" + def test_proxy_precedence(self, monkeypatch, k3scontainer_config): + https_proxy = "https://env-https-proxy.com" + http_proxy = "http://env-http-proxy.com" + monkeypatch.setenv(name="HTTPS_PROXY", value=https_proxy) + monkeypatch.setenv(name="HTTP_PROXY", value=http_proxy) - with pytest.raises( - ValueError, - match="Conflicting proxy settings: client_configuration.proxy=http://not-env-proxy.com, " - "but the environment variable 'HTTPS_PROXY/HTTP_PROXY' defines proxy as http://env-proxy.com.", - ): - get_client(client_configuration=client_configuration) + client = get_client(config_dict=k3scontainer_config) + # Verify HTTPS_PROXY takes precedence over HTTP_PROXY + assert client.configuration.proxy == https_proxy