Skip to content

Fix: properly load and parse NO_PROXY environment variable#2523

Open
Ndugu2 wants to merge 1 commit intokubernetes-client:masterfrom
Ndugu2:fix-no-proxy-2520
Open

Fix: properly load and parse NO_PROXY environment variable#2523
Ndugu2 wants to merge 1 commit intokubernetes-client:masterfrom
Ndugu2:fix-no-proxy-2520

Conversation

@Ndugu2
Copy link
Copy Markdown

@Ndugu2 Ndugu2 commented Mar 13, 2026

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR fixes the handling of the NO_PROXY environment variable in the Configuration class. Previously:

  1. No parsing: The value was stored as a raw string instead of being parsed into a list of domains
  2. Precedence issues: When both NO_PROXY and no_proxy were set, the lowercase version could take precedence incorrectly
  3. Empty string handling: An empty NO_PROXY="" was treated the same as if the variable wasn't set
  4. Default value: self.no_proxy defaulted to None instead of an empty list

These issues caused proxy bypass settings to be ignored, breaking functionality for users relying on no_proxy configurations.

Which issue(s) this PR fixes:

Fixes #2520

Special notes for your reviewer:

The fix implements proper precedence checking using in os.environ rather than os.getenv() to correctly detect when a variable exists even if it's empty. The value is now parsed into a list of domains with whitespace stripped.

Test cases added cover:

  • Uppercase NO_PROXY
  • Lowercase no_proxy
  • Both uppercase and lowercase (precedence test)
  • Empty string values
  • No environment variable set
  • Spaces in comma-separated values

Does this PR introduce a user-facing change?

Fixed: The NO_PROXY environment variable is now properly parsed into a list of domains. Uppercase NO_PROXY correctly takes precedence over lowercase no_proxy. Empty NO_PROXY="" now results in an empty list rather than falling back to the lowercase variant.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. labels Mar 13, 2026
@k8s-ci-robot k8s-ci-robot requested a review from roycaihw March 13, 2026 08:19
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 13, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ndugu2
Once this PR has been reviewed and has the lgtm label, please assign yliaog for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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 k8s-ci-robot requested a review from yliaog March 13, 2026 08:19
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 13, 2026 •
edited
Loading

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: Ndugu2 / name: Kosea Kalema (e78a303)

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Mar 13, 2026

Welcome @Ndugu2!

It looks like this is your first PR to kubernetes-client/python 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/python has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 13, 2026
@Ndugu2 Ndugu2 mentioned this pull request Mar 13, 2026
Open
@yliaog
Copy link
Copy Markdown
Contributor

yliaog commented Mar 13, 2026

please take a look at the following fix PR:
#2459
#2513

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Apr 15, 2026

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

elementl-devtools pushed a commit to dagster-io/dagster that referenced this pull request Apr 23, 2026
@gibsondan @claude
Patch upstream bug with NO_PROXY env handling for k8s clients (#23170)
2ae80f3 
## Summary

Works around
[kubernetes-client/python#2520](kubernetes-client/python#2520):
`kubernetes.client.Configuration.__init__` reads `NO_PROXY`/`no_proxy`
from the environment and then immediately overwrites `self.no_proxy =
None`, so proxy-bypass rules from the env are silently dropped and all
traffic is forced through the configured proxy.

Adds a shared `apply_no_proxy_env_workaround()` helper in
`dagster_k8s.client` and calls it from the three user-space
Configuration-loading sites:

- `dagster_cloud` `K8sUserCodeLauncher` (the agent itself)
- `dagster_k8s` `K8sRunLauncher` (inherited by `CloudK8sRunLauncher`)
- `dagster_k8s` `_PipesK8sClient`

Not patched: `dagster-cloud-backend` metadaemons and `eks_k8s_client.py`
— these run on the control plane, not in user clusters, so the env-var
behavior doesn't apply.

## How it stays safe across versions

The helper is idempotent — on kubernetes client versions where the
upstream bug is fixed (or where `no_proxy` was never mishandled), it
just re-applies the same value the library would have set. `dagster_k8s`
is OSS so we can't pin the user's kubernetes client version; that's
handled by the canary test below rather than a version gate.

## Test plan

- [x] `pytest dagster_k8s_tests/unit_tests/test_client.py -k "no_proxy
or canary"` — 4 new tests pass
- Functional: helper applies `NO_PROXY` env var to the default
`Configuration`
- Prefers uppercase `NO_PROXY` over lowercase `no_proxy` when both are
set
  - No-op when neither env var is set
- **Canary** (`test_kubernetes_client_no_proxy_bug_canary`): directly
asserts `kubernetes.client.Configuration()` clobbers `no_proxy` to
`None` even when `NO_PROXY` is set. When a future client bump ships the
fix from [PR
#2523](kubernetes-client/python#2523), this
assertion will flip and the test will fail loudly — our cue to delete
`apply_no_proxy_env_workaround` and its call sites.
- [x] `ruff format` + `ruff check` clean on all 5 edited files

## Changelog
[dagster-k8s] Added a workaround for an upstream issue in the Kubernetes
python client kubernetes-client/python#2520)
where setting the NO_PROXY or no_proxy environment variable did not
affect the configuration of kubernetes API calls.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Internal-RevId: f1dc73b86ce59834473c3b11a256aea278a1741b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roycaihw roycaihw Awaiting requested review from roycaihw

@yliaog yliaog Awaiting requested review from yliaog

Assignees

No one assigned

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

no_proxy environment variable ignored in kubernetes-client Python library

3 participants

@Ndugu2 @k8s-ci-robot @yliaog