Respect no_proxy settings next to the proxy settings.

[rajbos]

With #21 there is now support for proxy settings, which do not adhere to the no_proxy settings, breaking in our setup where we set both http_proxy and no_proxy.

[rajbos]

I've been checking to see if I can contribute, and wanted to discuss first about the direction😁.

From what I can see, https-proxy-agent does not support the no_proxy settings 😞. That means we have two options:

1. Add a setting to ignore the https_proxy environment variable: ignore_proxy: false, which requires user input.
2. Write a check to see if the target url is in the no_proxy list, and if so, do not inject the https-proxy-agent here

Tagging @peter-murray for visibility.

[peter-murray]

Option 1 is the quickest to deliver in this case, but seeing that we are reading the settings from the environment ti does present Option 2 as being a more consistent approach.

If I was to do this, if you explicitly inject the proxy as an input to the action, there would be no ignoring the proxy URLs, otherwise why did you inject it?
Then if the proxy is coming from the environment, and no_proxy was set, then that should be respected, which means that the test for that needs to go elsewhere...

[rajbos]

Good point. Then it's Option 3?

3. Make the injection of the proxy as input required and do not load the environment settings at all.

Or would you want do do both 3 and 2 then (sorry if this is getting confusing)?

[peter-murray]

I originally liked the idea of loading it from the runner environment if it was set, as this keeps the workflows DRY, especially if you set it inside the workflow as a default instead of inside the runner itself.

I think now that this has shipped we need support for the following:

1. If the user has provided a proxy as an input to the action, use it. This also overrides any environment settings (most specific)
2. If the action is falling back to environment variables, then if no_proxy is also set in the environment variables, respect that is the server is covered by that.
   • This does create a bit of an annoyance here; https://github.com/peter-murray/workflow-application-token-action/blob/main/lib/github-application.js#L2 as this has environment defaults for Actions in reading the API URL that I need to duplicate in the fallbacks, as the user could optionally specify the base URL, or rely on the actions library defaults which is to read it from the GITHUB_API_URL

This is achievable, just not as clean as I would like.

[peter-murray]

After going around the bush a few times with this, I landed on the assumption that the no_proxy would be host names, so in the case of GitHub.com would be api.github.com in the string and it works in my limited testing.

@rajbos would you mind checking that the branch no-proxy-support does what you need it to do before I publish this to main and release a new version?

[rajbos]

Works like a charm @peter-murray!

I've tested:

• With our environment settings (HTTPS_PROXY and NO_PROXY), with a github_api_base_url set
• With our environment settings (HTTPS_PROXY and NO_PROXY), without a github_api_base_url set
• With our environment settings (HTTPS_PROXY and NO_PROXY), with the https_proxy variable set

Things work as expected with these combinations! Thanks for turning this around so quickly 🤗.