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

p172913 · 2025-04-29T06:03:02Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
This PRs will read environment variables assigned for proxy and no_proxy.

Which issue(s) this PR fixes:
Fixes #2321

Special notes for your reviewer:
Unit test is added kubernetes\base\stream\ws_client_test.py

Does this PR introduce a user-facing change?

Allow configuration.py to read proxy and no_proxy environmental variables

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
None

p172913 · 2025-04-29T18:01:06Z

@roycaihw , @fabianvf Please approve my PR

p172913 · 2025-05-01T19:14:29Z

@roycaihw , @fabianvf Please review my PR...

p172913 · 2025-05-05T15:50:20Z

@roycaihw , @fabianvf Please review my PR...

roycaihw · 2025-05-07T20:04:00Z

/assign

roycaihw · 2025-05-07T22:44:26Z

kubernetes/client/configuration.py

+        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")


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

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.

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.

If it is only for self.proxy=None. I can add it to https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/python/configuration.mustache.

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.

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...

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.

@roycaihw Can you check and approve my PR

@roycaihw Can you check and approve my PR

@roycaihw Can you check and approve my PR

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?

roycaihw

Please remove the "Merge branch 'kubernetes-client:master' into master" commit to make the commits clear. You may have to start from a new branch to do so.

roycaihw · 2025-05-27T20:02:42Z

kubernetes/client/configuration.py

+        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")


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?

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...

If user need to declare both HTTPS_PROXY and HTTP_PROXY. He need to mention which proxy he is using.

If user declared both HTTPS_PROXY and HTTP_PROXY and he didn't mention then it will choose latest declared.

If we need to give HTTPS_PROXY priority over HTTP_PROXY. Then I need to change the code accordingly....

@roycaihw What is the exact requirement here...?

I'm okay with having override behavior, just want to make sure we are aligned with what websocket client is doing

According to the original issue, websocket client makes use of these env variables, so I wanted to understand if the same override behavior exists there.

In the original issue (#2321), he gave only one variable HTTPS_PROXY. So, the changes will not override the self.proxy variable.

roycaihw · 2025-05-27T20:03:13Z

kubernetes/client/configuration.py

        """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")


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

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...

roycaihw · 2025-05-27T20:04:11Z

kubernetes/client/configuration.py

+        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")


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?

p172913 · 2025-05-28T10:39:07Z

Please remove the "Merge branch 'kubernetes-client:master' into master" commit to make the commits clear. You may have to start from a new branch to do so.

This commit is reverted back.

p172913 · 2025-05-28T10:56:02Z

I added a bash script file. This will edit configuration.py file if we create it using new opneapi generator.

roycaihw

Please squash the commits.

roycaihw · 2025-06-06T00:02:16Z

kubernetes/client/configuration.py

+        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")


I'm okay with having override behavior, just want to make sure we are aligned with what websocket client is doing

According to the original issue, websocket client makes use of these env variables, so I wanted to understand if the same override behavior exists there.

roycaihw · 2025-06-06T00:05:56Z

.readthedocs.yaml

@@ -1,53 +0,0 @@
-# Read the Docs configuration file for Sphinx projects


I think the file deletion is unexpected. Please fix.

bachorp · 2025-06-08T07:31:25Z

@roycaihw I have been working get this upstream: OpenAPITools/openapi-generator#21394

Edit: I ceased working on it for the time being.

p172913 · 2025-06-08T10:54:52Z

@roycaihw I have been working get this upstream: OpenAPITools/openapi-generator#21394

Oh ok. Then @roycaihw I will remove statement to execute insert_proxy_config.sh from scripts/release.sh. So, that we can execute insert_proxy_config.sh only if needed instead of running always....

p172913 · 2025-06-09T16:07:03Z

@roycaihw I have been working get this upstream: OpenAPITools/openapi-generator#21394

Oh ok. Then @roycaihw I will remove statement to execute insert_proxy_config.sh from scripts/release.sh. So, that we can execute insert_proxy_config.sh only if needed instead of running always....

@roycaihw What do you think...

roycaihw · 2025-06-09T23:41:05Z

Could you squash the commits? Otherwise LGTM. https://www.kubernetes.dev/docs/guide/github-workflow/#squash-commits

What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy. Update ws_client_test.py Update configuration.py What type of PR is this? /kind bug What this PR does / why we need it: This PRs will read environment variables assigned for proxy and no_proxy. Update configuration.py Add debug logging doc and example add .readthedocs.yaml config file Added Added insert_proxy_config.sh to edit configuration.py in client Revert "Added insert_proxy_config.sh to edit configuration.py in client" This reverts commit b295c2d. To avoid condition self.no_proxy is already present

p172913 · 2025-06-10T11:10:41Z

Could you squash the commits? Otherwise LGTM. https://www.kubernetes.dev/docs/guide/github-workflow/#squash-commits

@roycaihw squashed commits as per instructions.

p172913 · 2025-06-12T16:24:36Z

Could you squash the commits? Otherwise LGTM. https://www.kubernetes.dev/docs/guide/github-workflow/#squash-commits

@roycaihw squashed commits as per instructions.

@roycaihw please approve my commit.

p172913 · 2025-06-16T16:10:00Z

Could you squash the commits? Otherwise LGTM. https://www.kubernetes.dev/docs/guide/github-workflow/#squash-commits

@roycaihw squashed commits as per instructions.

@roycaihw please approve my commit.

@roycaihw Do i need to add anything to this...?

p172913 · 2025-06-23T15:19:24Z

@roycaihw Can you please look into my PR. I squashed my commits as you mentioned. Please approve my Pull request....

roycaihw · 2025-06-24T19:59:11Z

/lgtm
/approve

Thank you!

k8s-ci-robot · 2025-06-24T19:59:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: p172913, roycaihw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [roycaihw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from fabianvf and roycaihw April 29, 2025 06:03

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 29, 2025

p172913 force-pushed the master branch from a14e698 to 8f356ef Compare April 30, 2025 11:03

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Apr 30, 2025

k8s-ci-robot assigned roycaihw May 7, 2025

roycaihw reviewed May 7, 2025

View reviewed changes

p172913 force-pushed the master branch from 638abe2 to afd1d44 Compare May 20, 2025 14:05

p172913 requested a review from roycaihw May 27, 2025 07:00

roycaihw reviewed May 27, 2025

View reviewed changes

roycaihw reviewed Jun 6, 2025

View reviewed changes

p172913 mentioned this pull request Jun 8, 2025

I'm okay with having override behavior, just want to make sure we are aligned with what websocket client is doing #2411

Closed

p172913 force-pushed the master branch from f258b31 to e3b373f Compare June 10, 2025 11:01

Merge branch 'kubernetes-client:master' into master

945de4e

p172913 requested a review from roycaihw June 19, 2025 20:12

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 24, 2025

roycaihw added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/bug Categorizes issue or PR as related to a bug. labels Jun 24, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2025

k8s-ci-robot merged commit 51f4db5 into kubernetes-client:master Jun 24, 2025
2 checks passed

		@@ -1,53 +0,0 @@
		# Read the Docs configuration file for Sphinx projects

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

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

Conversation

p172913 commented Apr 29, 2025 • edited by roycaihw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p172913 commented Apr 29, 2025

Uh oh!

p172913 commented May 1, 2025

Uh oh!

p172913 commented May 5, 2025

Uh oh!

roycaihw commented May 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roycaihw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

p172913 commented May 28, 2025

Uh oh!

p172913 commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roycaihw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bachorp commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p172913 commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p172913 commented Jun 9, 2025

p172913 commented Apr 29, 2025 •

edited by roycaihw

Loading

p172913 May 30, 2025 •

edited

Loading

p172913 May 28, 2025 •

edited

Loading

p172913 commented May 28, 2025 •

edited

Loading

bachorp commented Jun 8, 2025 •

edited

Loading

p172913 commented Jun 8, 2025 •

edited

Loading

p172913 commented Jun 10, 2025 •

edited

Loading