SPICE-0004: HTTP Proxy Support #5

bioball · 2024-05-02T05:24:01Z

This is a continuation of the design discussion started here: apple/pkl#309

translatenix · 2024-05-02T05:44:17Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+
+==== Environment Variables
+
+Proxies will _not_ be configured via environment variables.


I think this is a mistake. Env vars are the way to configure proxy settings for command-line tools. Nobody wants to configure each tool separately. I’m optimistic that the listed concerns have good enough solutions.

I'm open to the argument, but: another concern here is: how does this play with --env-var? What happens if you do:

pkl eval --env-var http_proxy=http://some.proxy foo.pkl

It feels like this shouldn't affect proxy settings, because this is ostensibly just a way to configure what read("env:") will return.

But if that doesn't affect the proxy, this is possibly a strange behavior:

# talk via the proxy export http_proxy=http://some.proxy pkl eval foo.pkl # does not talk via the proxy pkl eval --env-var http_proxy=http://some.proxy foo.pkl

There's also plenty of languages/tools whose proxies aren't configured by env vars. Just some quick skimming (as far as I can tell):

Node.js

Java

Rust, Cargo

PHP

Java supports OS proxy settings, and there are signs that env vars will be reconsidered. Honoring neither OS settings nor env vars will force users to script a solution specifically for Pkl.

Regarding --env-var, the behavior you described seems fine to me.

It would be very helpful to get input from frequent proxy users. I haven't used a proxy in over a decade.

Not sure I'm sold on configuring proxies via env vars. Nothing else in Pkl in configured using them, also, It may lead to "works on my machine" problems where you have the right env vars set.

Supporting OS proxy settings sounds more enticing than supporting envvar-based configuration, if that's possible.

translatenix · 2024-05-02T05:47:49Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+proxy: Proxy?
+----
+
+==== `PklProject`


What’s the use case for this?

I am deploying into an environment that needs proxying. Instead of setting it up on my target machine, or setting it up via a script, I can just set it in my PklProject file.

Do you have evidence that this is useful? Is it practical to only have per-project proxy settings?

I think the rationale here is that settings.pkl is a user specific configuration, which only leaves PklProject and CLI/Evaluator options left for shared configs. Configuring your proxy every time via CLI doesn't seem ideal, so adding this settings to PklProject allows for a project-wide proxy configuration.

Do you have evidence that this is useful? Is it practical to only have per-project proxy settings?

Yeah, this is something that was brought up as a desired use-case from some of our users.

translatenix · 2024-05-02T05:49:38Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+* `--proxy` -- A proxy URL to connect to
+* `--noproxy` -- A comma-separated list of hosts to not proxy.
+
+==== pkl-gradle


What’s the use case for Pkl proxy settings that differ from Gradle/JVM proxy settings?

I want to control pkl eval through Gradle, but I want it to otherwise behave exactly like my CLI does.

The correct proxy settings depend on the network you’re on. Can you think of a use case where having different proxy settings for Gradle and the Gradle Pkl plugin is useful? I can’t.

translatenix · 2024-05-02T05:50:33Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+
+To support this, `HttpClient#Builder` receives two new builder methods:
+
+* `setProxySelector(java.net.ProxySelector proxySelector)`


Is exposing ProxySelector really necessary? (For certificates, we managed to avoid dependencies on java.net classes.)

I'm open to ideas here!

What motivated you to expose it?

The goal is to allow users to configure Pkl's proxy the same way their application's proxy is configured. setProxy(proxyAddress, List<URI> noProxy) is our bespoke implementation. Having this method means that those that are already using ProxySelector can configure our client the same way.

Some prior art; both java.net.HttpClient and okhttp accept ProxySelector.

Of course java.net.HttpClient accepts ProxySelector. But with every java.net type added, it becomes more difficult to ever have another implementation of org.pkl.util.http.HttpClient (say NettyHttpClient). It’s better to wait until there is a proven need for “those that are already using ProxySelector can configure our client the same way”, then make an informed decision. That’s why I didn’t expose HttpClient.sendAsync—it’s not yet needed, and with virtual threads added in Java 21, it may never be needed.

But with every java.net type added, it becomes more difficult to ever have another implementation of org.pkl.util.http.HttpClient (say NettyHttpClient).

I'm not sure why this is desirable. Can you elaborate?

translatenix · 2024-05-02T06:49:50Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+----
+
+[[new-stdlib-module]]
+==== New stdlib module: `pkl.Proxy`


Having a stdlib module just for proxy settings feels strange.
Can’t this be part of pkl.settings, even if pkl.Project needs it too (which I’m not convinced of)?

Both pkl.settings and pkl.Project need this, and neither is a logical subset of the other, so it feels cleanest to put it in a separate module.

We can also put it in base.pkl, but that affects the global namespace, which doesn't feel great.

We also possibly should put redirect rules in there, so maybe this intermediary module should be called HttpSettings, with proxy and rewrite rules both being in there.

If a new stdlib module is needed, its scope should definitely be wider than proxy settings.

Is any of the following attractive?

pkl.Project extends pkl.settings

pkl.Project has a property of type pkl.settings

pkl.Project and pkl.settings have a common base module

Out of those three, I think the third one might be a reasonable approach; will play around with that a little bit.

spices/SPICE-0004-HTTP-Proxy-Support.adoc

translatenix · 2024-05-02T07:03:26Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+
+* HTTPS proxies
+* SOCKS proxies
+* Alternative authentication methods (e.g. setting the `Proxy-Authorization` header)


Alternative to what? As far as I can tell, this spice doesn’t propose any auth support. (I believe this is the right way to start given how little is offered by the JDK.)

Originally, I figured we would just use basic auth, but I'm backing off on that idea due to security concerns.

Co-authored-by: translatenix <119817707+translatenix@users.noreply.github.com>

Following design of SPICE-0004: apple/pkl-evolution#5

stackoverflow · 2024-05-06T09:44:28Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+
+==== Environment Variables
+
+Proxies will _not_ be configured via environment variables.


Not sure I'm sold on configuring proxies via env vars. Nothing else in Pkl in configured using them, also, It may lead to "works on my machine" problems where you have the right env vars set.

stackoverflow · 2024-05-06T09:51:10Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+proxy: Proxy?
+----
+
+==== `PklProject`


I think the rationale here is that settings.pkl is a user specific configuration, which only leaves PklProject and CLI/Evaluator options left for shared configs. Configuring your proxy every time via CLI doesn't seem ideal, so adding this settings to PklProject allows for a project-wide proxy configuration.

stackoverflow · 2024-05-06T09:53:09Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+[[new-stdlib-module]]
+==== New stdlib module: `pkl.Proxy`
+
+A new stdlib module is added called `pkl.Proxy`, that is used by `pkl.settings` and `pkl.Project`.


Calling it pkl.HttpSettings would future-proof if for when URL rewriting arrives, though this can be done in said SPICE.

stackoverflow · 2024-05-06T09:59:08Z

spices/SPICE-0004-HTTP-Proxy-Support.adoc

+
+== Alternatives considered
+
+TBD


URL rewriting was a considered alternative, but after some thought we found out they don't fully overlap and there may be a need for both.

bioball changed the title ~~Add SPICE for http proxy~~ SPICE-0004: HTTP Proxy Support May 2, 2024

Add SPICE for http proxy

1ebc838

bioball force-pushed the http-proxy branch from 1f72e55 to 1ebc838 Compare May 2, 2024 05:25

translatenix reviewed May 2, 2024

View reviewed changes

spices/SPICE-0004-HTTP-Proxy-Support.adoc Outdated Show resolved Hide resolved

translatenix reviewed May 2, 2024

View reviewed changes

Update spices/SPICE-0004-HTTP-Proxy-Support.adoc

819b9a3

Co-authored-by: translatenix <119817707+translatenix@users.noreply.github.com>

bioball added a commit to bioball/pkl that referenced this pull request May 3, 2024

Add support for HTTP proxying

54c23ad

Following design of SPICE-0004: apple/pkl-evolution#5

bioball added a commit to bioball/pkl that referenced this pull request May 3, 2024

Add support for HTTP proxying

abc5b44

Following design of SPICE-0004: apple/pkl-evolution#5

bioball mentioned this pull request May 3, 2024

Add support for HTTP proxying apple/pkl#472

Closed

stackoverflow reviewed May 6, 2024

View reviewed changes

holzensp mentioned this pull request May 27, 2024

Add support for HTTP proxying apple/pkl#506

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPICE-0004: HTTP Proxy Support #5

SPICE-0004: HTTP Proxy Support #5

bioball commented May 2, 2024 •

edited

translatenix May 2, 2024 •

edited

bioball May 2, 2024

translatenix May 4, 2024 •

edited

stackoverflow May 6, 2024

bioball May 19, 2024

translatenix May 2, 2024

bioball May 2, 2024

translatenix May 3, 2024

stackoverflow May 6, 2024

bioball May 19, 2024

translatenix May 2, 2024 •

edited

bioball May 2, 2024

translatenix May 3, 2024 •

edited

translatenix May 2, 2024 •

edited

bioball May 2, 2024

translatenix May 3, 2024 •

edited

bioball May 5, 2024 •

edited

translatenix May 5, 2024 •

edited

bioball May 6, 2024

translatenix May 2, 2024

bioball May 2, 2024

translatenix May 4, 2024 •

edited

bioball May 6, 2024

translatenix May 2, 2024 •

edited

bioball May 2, 2024

stackoverflow May 6, 2024

stackoverflow May 6, 2024

stackoverflow May 6, 2024

stackoverflow May 6, 2024


		==== Environment Variables

		Proxies will _not_ be configured via environment variables.


		To support this, `HttpClient#Builder` receives two new builder methods:

		* `setProxySelector(java.net.ProxySelector proxySelector)`

SPICE-0004: HTTP Proxy Support #5

Are you sure you want to change the base?

SPICE-0004: HTTP Proxy Support #5

Conversation

bioball commented May 2, 2024 • edited

translatenix May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 3, 2024 • edited

Choose a reason for hiding this comment

translatenix May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 3, 2024 • edited

Choose a reason for hiding this comment

bioball May 5, 2024 • edited

Choose a reason for hiding this comment

translatenix May 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

translatenix May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bioball commented May 2, 2024 •

edited

translatenix May 2, 2024 •

edited

translatenix May 4, 2024 •

edited

translatenix May 2, 2024 •

edited

translatenix May 3, 2024 •

edited

translatenix May 2, 2024 •

edited

translatenix May 3, 2024 •

edited

bioball May 5, 2024 •

edited

translatenix May 5, 2024 •

edited

translatenix May 4, 2024 •

edited

translatenix May 2, 2024 •

edited