Skip to content
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

Don't respect HTTP_PROXY env in k8 forwarder #11257

Merged
merged 5 commits into from
Mar 25, 2022

Conversation

smallinsky
Copy link
Contributor

What

Kube proxy service should not respect HTTP_PROXY env variable and dial kube agent directly.

How

Replace apimachinery transport with standard x/net/http2 package where HTTP_PROXY envs are not respected.

Copy link
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@smallinsky This definitely needs test coverage, I can imagine this can easily regress again.

@smallinsky smallinsky requested review from r0mant and removed request for timothyb89 and greedy52 March 18, 2022 17:52
@smallinsky
Copy link
Contributor Author

Added UT regression.
@r0mant, @NajiObeid Could you take a second look ?

@smallinsky smallinsky force-pushed the smallinsky/kube-fwd-switch-to-x-net-http2 branch from 8da79e9 to 5f6f8b6 Compare March 22, 2022 13:43
Copy link
Contributor

@NajiObeid NajiObeid left a comment

Choose a reason for hiding this comment

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

All good on my side. But I personally would have preferred to see this part of the kube h2 integration tests. You know to be less reliant on mocks or testing edge cases, but more along the lines of testing different setups clients might have as weird as they can get.

@smallinsky smallinsky merged commit 335adf1 into master Mar 25, 2022
@smallinsky smallinsky deleted the smallinsky/kube-fwd-switch-to-x-net-http2 branch March 25, 2022 12:50
@webvictim webvictim mentioned this pull request Apr 19, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
tigrato added a commit that referenced this pull request Aug 16, 2023
PR #11257 disabled support of `HTTP_PROXY`, `HTTPS_PROXY` and
`NO_PROXY` environement flags for Kubernetes Access. The desired
behavior was expected to be respected only by the Kubernetes Proxy and
Kubernetes Legacy Proxy when dialing over reverse tunnel but ended up
applied to all outbound connections from Kube Access flow.

This PR enables support for proxy env's when dialing directly to the
Kubernetes Cluster - `kubernetes_service` and `legacy_proxy` when the
cluster is local.

Fixes #30550

Signed-off-by: Tiago Silva <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Aug 17, 2023
* Respect `[HTTP(S)|NO]_PROXY` envs when dialing directly to Kube

PR #11257 disabled support of `HTTP_PROXY`, `HTTPS_PROXY` and
`NO_PROXY` environement flags for Kubernetes Access. The desired
behavior was expected to be respected only by the Kubernetes Proxy and
Kubernetes Legacy Proxy when dialing over reverse tunnel but ended up
applied to all outbound connections from Kube Access flow.

This PR enables support for proxy env's when dialing directly to the
Kubernetes Cluster - `kubernetes_service` and `legacy_proxy` when the
cluster is local.

Fixes #30550

Signed-off-by: Tiago Silva <[email protected]>

* fix func name

* fix comment

---------

Signed-off-by: Tiago Silva <[email protected]>
tigrato added a commit that referenced this pull request Aug 17, 2023
* Respect `[HTTP(S)|NO]_PROXY` envs when dialing directly to Kube

PR #11257 disabled support of `HTTP_PROXY`, `HTTPS_PROXY` and
`NO_PROXY` environement flags for Kubernetes Access. The desired
behavior was expected to be respected only by the Kubernetes Proxy and
Kubernetes Legacy Proxy when dialing over reverse tunnel but ended up
applied to all outbound connections from Kube Access flow.

This PR enables support for proxy env's when dialing directly to the
Kubernetes Cluster - `kubernetes_service` and `legacy_proxy` when the
cluster is local.

Fixes #30550

Signed-off-by: Tiago Silva <[email protected]>

* fix func name

* fix comment

---------

Signed-off-by: Tiago Silva <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2023
…) (#30615)

* Respect `[HTTP(S)|NO]_PROXY` envs when dialing directly to Kube

PR #11257 disabled support of `HTTP_PROXY`, `HTTPS_PROXY` and
`NO_PROXY` environement flags for Kubernetes Access. The desired
behavior was expected to be respected only by the Kubernetes Proxy and
Kubernetes Legacy Proxy when dialing over reverse tunnel but ended up
applied to all outbound connections from Kube Access flow.

This PR enables support for proxy env's when dialing directly to the
Kubernetes Cluster - `kubernetes_service` and `legacy_proxy` when the
cluster is local.

Fixes #30550



* fix func name

* fix comment

---------

Signed-off-by: Tiago Silva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants