zowe / zowe-cli

Zowe CLI
Eclipse Public License 2.0
108 stars 85 forks source link

Added simple proxy agent support using standard environment variables #2151

Closed phaumer closed 1 week ago

phaumer commented 1 month ago

What It Does

https://github.com/zowe/zowe-cli/issues/1305

I am proving a simple first increment for supporting a proxy http agent for Zowe CLI. It requires usage of the environment variables HTTP_PROXY, http_proxy, HTTPS_PROXY, https_proxy. If any of these env variables is set and depending if the Zowe session is configured for http or https it instantiates an appropriate http agent using the same libraries that VS Code uses: https://github.com/TooTallNate/proxy-agents

If the z/OS system uses self-signed certificates then the proxy server must be configured to accept them. If the proxy server itself is configured with self-signed certificates then the user needs to either import these certificates on their workstation or use the (not recommended) nodejs variable NODE_TLS_REJECT_UNAUTHORIZED=0.

How to Test

Review Checklist I certify that I have:

Additional Comments

For the next branch I could envision to extension Zowe profiles with new proxy properties so users can specify their proxy server in their team config files. We can also consider using another third party library to read Windows and MacOS system settings for proxies.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 94.44444% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.16%. Comparing base (4e822b2) to head (64b7f62). Report is 1 commits behind head on master.

Files Patch % Lines
packages/imperative/src/rest/src/client/Proxy.ts 95.65% 2 Missing :warning:
...perative/src/rest/src/client/AbstractRestClient.ts 87.50% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2151 +/- ## ========================================== + Coverage 91.14% 91.16% +0.01% ========================================== Files 637 638 +1 Lines 19064 19118 +54 Branches 4015 4033 +18 ========================================== + Hits 17376 17428 +52 - Misses 1687 1689 +2 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Crosswind commented 1 month ago

@phaumer Thank you for providing this. I am trying to test this but don't seem to have a lot of success. I am currently in an environment that has direct connection to the target system which makes the test a little more complicated. I am essentially trying to break the connection by sending the request through a non-existing proxy server first.

I have a proxy server running on localhost:8888 (using https://github.com/tinyproxy/tinyproxy). I can easily verify that it works by using cURL. Now, to test the Zowe CLI I am setting my environment variables to localhost:8889 (nothing is running on that port). I am expecting Zowe CLI to complain as it shouldn't be able to connect. Unfortunately I don't get any connection issues and the CLI just works.

> env | grep proxy
http_proxy=http://localhost:8889
https_proxy=http://localhost:8889
> curl -x localhost:8889 https://google.com  
curl: (7) Failed to connect to localhost port 8889 after 0 ms: Couldn't connect to server
> zowe zosmf check status --reject-unauthorized false | head
(node:16242) [DEP0051] DeprecationWarning: The `util.isNullOrUndefined` API is deprecated. Please use `arg === null || arg === undefined` instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
(node:16242) [DEP0044] DeprecationWarning: The `util.isArray` API is deprecated. Please use `Array.isArray()` instead.
(node:16242) [DEP0056] DeprecationWarning: The `util.isString` API is deprecated.  Please use `typeof arg === "string"` instead.
(node:16242) [DEP0052] DeprecationWarning: The `util.isNumber` API is deprecated. Please use `typeof arg === "number"` instead.
The user z01437 successfully connected to z/OSMF on 'S0W1.DAL-EBIS.IHOST.COM'.
zosmf_port:         10443
zosmf_saf_realm:    SAFRealm
zos_version:        04.27.00
zosmf_full_version: 27.0
api_version:        1

I haven't had the chance to dive deeper into the actual code to try and understand what may be missing. I am quite certain that my environment setup is correct. Any suggestions or things that I could test?

Edit: I am going to actually test if going through the proxy server but since it doesn't even seem to pick up the environment variables correctly testing anything further is pointless in my opinion.

phaumer commented 1 month ago

@Crosswind thanks. There should be an INFO level message in the imperative.log when the environment variable was found. I have to also test with other proxy servers. I was using Squid.

Crosswind commented 1 month ago

Apologies, that was a mess-up on my side. It's working now. I get an error message if I try to use an incorrect proxy server and I can successfully connect if I go through my tinyproxy server. I could only test the connection to a TLS-secured instance so I could only verify HTTPS_PROXY and https_proxy.

Regarding mention of self-signed certificates: I am using the cli option --reject-unauthorized false so I didn't have to specifically configure my proxy server. It should end up doing the same thing but it was much simpler and less intrusive like setting NODE_TLS_REJECT_UNAUTHORIZED=0.

For completeness: Does it make sense to also add NO_PROXY variable support? This is more or less for those people that need to use a proxy server for certain things but their zowe cli should not go through one.

phaumer commented 1 month ago

@Crosswind thanks. I quickly added reading no_proxy. Have to do more testing for it, but let me know if that matches your expectations.

sonarcloud[bot] commented 1 week ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
91.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud