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.

Sign up for GitHub

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

Jump to bottom

fix: kind installer/Resource not present #3994

Merged
merged 1 commit into from Sep 28, 2023
Merged

fix: kind installer/Resource not present #3994

merged 1 commit into from Sep 28, 2023

Conversation

jeffmaury
Copy link
Contributor

@jeffmaury jeffmaury commented Sep 21, 2023 •
edited

Fixes #2794

What does this PR do?

Ensure libraries using NodeJS fetch implementation are proxy compatible. That would include the kind and compose extensions.

WARNING This part makes the assumption that the proxy configuration is either httpProxy if the proxy is using HTTP with the browser/Podman Desktop or httpsProxy if using HTTPS (ie configuring both makes no sense). Also this PR does not implement support for NO_PROXY, should be a follow up PR

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

Fixes #2794

How to test this PR?

  1. Configure your workstation with proxy (no network access except for the proxy)
  2. Remove kind or compose extensions
  3. Launch Podman Desktop
  4. Install kind or compose

@jeffmaury jeffmaury force-pushed the GH-2794-1 branch 3 times, most recently from 67e96a8 to fddd98c Compare September 21, 2023 09:40
@jeffmaury
fix: kind installer/Resource not present
2da9d72 
Fixes containers#2794

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
@jeffmaury jeffmaury marked this pull request as ready for review September 21, 2023 11:57
@jeffmaury jeffmaury requested review from benoitf and a team as code owners September 21, 2023 11:57
@jeffmaury jeffmaury requested review from dgolovin and lstocchi and removed request for a team September 21, 2023 11:57
@benoitf
Copy link
Collaborator

benoitf commented Sep 21, 2023

cc @dgolovin you invested transparent proxy , there was a missing fetch case ?

@lstocchi
Copy link
Contributor

Just want to say that i'm setting my env to test this PR. i didn't forget about it and sorry for the super late response.

lstocchi
lstocchi reviewed Sep 26, 2023
View reviewed changes
Copy link
Contributor

@lstocchi lstocchi left a comment •
edited

Choose a reason for hiding this comment

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

I'll keep trying but it looks to me that something does not work as expected.

What i've done:
I set up fiddler so i could use it as a proxy.
I set the proxy to 127.0.0.1:8888 in podman desktop but i receive a http error fetch failed when it tries to fetch kind releases on https://api.github.com/repos/kubernetes-sigs/kind/releases.

If i try the same on postman or on a browser i get the response fine.

@jeffmaury
Copy link
Contributor Author

I'll keep trying but it looks to me that something does not work as expected.

What i've done: I set up fiddler so i could use it as a proxy. I set the proxy to 127.0.0.1:8888 in podman desktop but i receive a http error fetch failed when it tries to fetch kind releases on https://api.github.com/repos/kubernetes-sigs/kind/releases.

If i try the same on postman or on a browser i get the response fine.

Can you see the request going through Fiddler ?

@lstocchi
Copy link
Contributor

I'll keep trying but it looks to me that something does not work as expected.
What i've done: I set up fiddler so i could use it as a proxy. I set the proxy to 127.0.0.1:8888 in podman desktop but i receive a http error fetch failed when it tries to fetch kind releases on https://api.github.com/repos/kubernetes-sigs/kind/releases.
If i try the same on postman or on a browser i get the response fine.

Can you see the request going through Fiddler ?

Yes
image

@jeffmaury
Copy link
Contributor Author

I'll keep trying but it looks to me that something does not work as expected.
What i've done: I set up fiddler so i could use it as a proxy. I set the proxy to 127.0.0.1:8888 in podman desktop but i receive a http error fetch failed when it tries to fetch kind releases on https://api.github.com/repos/kubernetes-sigs/kind/releases.
If i try the same on postman or on a browser i get the response fine.

Can you see the request going through Fiddler ?

Yes image

Any idea why it's being rejected ?

@lstocchi
Copy link
Contributor

Any idea why it's being rejected ?

it seems it's a problem with the certificate.
By setting process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; it works fine

lstocchi
lstocchi approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

So i guess it's not an actual problem. Just me that i used an internal signed certificate.
LGTM, GJ!! 🚀

@lstocchi lstocchi mentioned this pull request Sep 27, 2023
Closed
@jeffmaury jeffmaury merged commit a93e08f into containers:main Sep 28, 2023
10 checks passed
@jeffmaury jeffmaury deleted the GH-2794-1 branch September 28, 2023 06:58
@podman-desktop-bot podman-desktop-bot added this to the 1.5.0 milestone Sep 28, 2023
benoitf
benoitf reviewed Sep 28, 2023
View reviewed changes
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

it looks I never sent my review (I have a pending review item), so clicking on submit review even if it's too late

packages/main/src/plugin/proxy.ts
Comment on lines +137 to +151

private overrideFetch() {
const original = globalThis.fetch;
// eslint-disable-next-line @typescript-eslint/no-this-alias
const _me = this;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
globalThis.fetch = function (url: RequestInfo | URL, opts?: any) {
const proxyurl = getProxyUrl(_me);
if (proxyurl) {
opts = Object.assign({}, opts, { dispatcher: new ProxyAgent(proxyurl) });
}

return original(url, opts);
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if it should not go the https://github.com/containers/podman-desktop/blob/main/packages/main/src/plugin/extension-loader.ts#L217-L218

Also I'm wondering about the need of undici library

it looks like that for createHttpPatchedModules https://www.npmjs.com/package/hpagent is used so it looks like to be a duplicated dependency

Probably some code could be shared with createHttpPatchedModules from proxy-resolver.ts class

@benoitf benoitf mentioned this pull request Sep 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benoitf benoitf benoitf left review comments

@dgolovin dgolovin dgolovin approved these changes

@lstocchi lstocchi lstocchi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.

KinD installer/Resource not present
5 participants
@jeffmaury @benoitf @lstocchi @dgolovin @podman-desktop-bot