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

Use environment variables to configure HTTP proxies #2958

Open
1 task
kgregory-chariot opened this issue Jan 10, 2022 · 11 comments · May be fixed by #4165
Open
1 task

Use environment variables to configure HTTP proxies #2958

kgregory-chariot opened this issue Jan 10, 2022 · 11 comments · May be fixed by #4165
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue proxy This issue is related to a proxy configuration

Comments

@kgregory-chariot
Copy link

Describe the feature

The V1 Java SDK and others use the environment variables PROXY_HTTP and PROXY_HTTPs to configure a proxy. The v2 SDK only allows system properties.

There was already request to add this, but it was incorrectly closed by the PR that supported system properties (and I see that there's an outstanding issue that would have been solved by that PR, so perhaps the wrong issue was closed?)

Is your Feature Request related to a problem?

This means that you can't use a consistent deployment approach for deploying AWS applications in a polyglot environment. Or you have to write explicit code in your Java v2 SDK projects to retrieve the relevant environment variables and set system properties, before anything else happens in the program.

Proposed Solution

Update ProxyConfiguration.java for all three HTTP client implementations to look for environment variables as well as system properties.

Describe alternatives you've considered

Not using the v2 SDK is the simplest.

Otherwise, as mentioned above, writing explicit code to set the system properties from the value of the environment variables (because Lambda doesn't let you set system properties when the JVM is first run).

Acknowledge

  • I may be able to implement this feature request

AWS Java SDK version used

2.17.101

JDK version used

openjdk version "11.0.13" 2021-10-19

Operating System and version

Ubuntu 20.04

@kgregory-chariot kgregory-chariot added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 10, 2022
@debora-ito
Copy link
Member

@kgregory-chariot sorry for the confusion 😓 yes, it seems that environment variables are not supported yet.

#1334 was not addressed by the PR #2771 either, the ask is to support HTTPS system properties.

Let's use this issue to track the support for PROXY_HTTP and PROXY_HTTPS environment variables.

@debora-ito debora-ito removed the needs-triage This issue or PR still needs to be triaged. label Jan 27, 2022
@kgregory-chariot
Copy link
Author

As a comment on #2771: the approach that it took was to use the same set of system properties for all proxies. Which is perhaps reasonable, as I believe that the v2 SDK doesn't make non-TLS connections? However, the naming of those properties is inconsistent with the convention used in v1.

@haslam22
Copy link

haslam22 commented May 27, 2022

+1 would be really nice to see this implemented. It's a real pain to have to look up https_proxy and http_proxy environment variables, parse the username/password/host/port out of them, and then provide those to SDK v2 manually when creating a client. This was implemented nicely in SDK v1 in this pull request: aws/aws-sdk-java#1967 and almost makes me want to downgrade to use that functionality.

@patrickgombert
Copy link

Any updates here? It would be great to have this feature added.

@haslam22
Copy link

@debora-ito Would it be reasonable for me to start working on this and open up a pull request so we can get this v1 functionality into v2? Just checking because this was raised a while ago and I want to double check it's still valid and appropriate to be worked on. Thanks

@unoexperto
Copy link

I don't know if it's bug or feature but environmentVariable() of software.amazon.awssdk.utils.ProxySystemSetting returns null thus fallback mechanism in SystemSettingUtils.resolveSetting() is not working.

For now I just added this on start of my app but it's kinda a hack:

// Copy proxy settings from system env to VM env.
for (key in software.amazon.awssdk.utils.ProxySystemSetting.values()) {
    val value = System.getenv(key.property())
    if (value != null)
        System.setProperty(key.property(), value)
}

@yasminetalby yasminetalby added the p2 This is a standard priority issue label Nov 28, 2022
@maslakov
Copy link

Hi! Is there any progress on this? @haslam22 why have you closed your PR?
This is a real pain now! All software and frameworks understand http[s]_proxy env variables but AWS SDK v2.

@Mythra
Copy link

Mythra commented Jul 6, 2023

I've attempted to update every single client in the source tree HERE to support these variables -- but as far as I can tell it requires a breaking change to small parts of the public api (which I see no way to do right now 😅). Opening it up early to hopefully kick off discussions about how we can change it without breaking, or how we could approach this breaking change.

aws-sdk-java-automation added a commit that referenced this issue Apr 9, 2024
…68fa6d016

Pull request: release <- staging/6908b666-368a-466a-b97b-28a68fa6d016
@sethnelson-tecton
Copy link

I am interested in this change. As @unoexperto called out above - if this already works via system properties through software.amazon.awssdk.utils.ProxySystemSetting, shouldn't implementing environmentVariable() be sufficient to address this?

@Mythra
Copy link

Mythra commented May 6, 2024

@sethnelson-tecton , you are correct that we could just start checking the environment variables. Unfortunately, while doing that and adding in test cases I noticed a bug that (as far as I can tell requires a breaking change) which i'm not sure how to navigate, and no one has responded to my PR yet of if we should keep the bug, or try to fix it and maybe do it in a non-breaking way.

The rough synopsis of my PR mentions the bug:

While making this change, I also noticed a bug in how we were selecting the http.proxyHost & https.proxyHost System Properties. I've gotten them to follow: https://docs.oracle.com/javase/8/docs/technotes/guides/net/proxies.html correctly now. Before we weren't passing the scheme in and only selecting based on explicit configuration (which you can still do, and doesn't break)!

Effectively java says you should look at the scheme of the host you're requesting, and then use the appropriate environment variable/property/etc. If you're connecting to an HTTPS site, use https_proxy, If connecting to an HTTP site use http_proxy; this also makes sense when you realize in order to do HTTPS Proxying you may need to do TLS MITM to effectively serve those requests, some proxies may support either TLS/Non-TLS totally fine & just passthrough, and some proxies may outright not support TLS at all. Unfortunately, the current SDKs do not consider scheme at all when taking in a request. Which is not what they're documented to do, not what other SDKs do, and is probably not ideal for using a proxy either.

There's a couple options I see here:

  • We could just add in the environment calls, keep the bug, and document it as something that goes against other SDKs/Normal Java Behavior/Docs/etc.
  • We could add in new APIs, deprecate the old ones that don't take into account scheme (and just "guess" what the best is in those cases, maybe preserving the bug).
  • We could add in the scheme parameter to the APIs/change how the properties are handled to be properly set (breaking -- i don't see a way to do that, but this is the most ideal probably.)

Considering I don't work for AWS, I don't wanna speak for them, and I'm happy to do any of the three. I just am not sure which one of them to do (I stopped when I realized I couldn't do the third option because of some automation saying no breaking changes).

@sethnelson-tecton
Copy link

Gotcha, thank you for breaking them down so succinctly! For what it's worth - that first option sounds like the right path forward IMO (or rather -- addressing the "existing behavior should support both system props and env vars" and "the existing behavior has a bug and it ought to be fixed" are separable problems+solutions).

@debora-ito debora-ito added the proxy This issue is related to a proxy configuration label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue proxy This issue is related to a proxy configuration
Projects
None yet
9 participants