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

feat: Improved proxy #122

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

MatsAnd
Copy link

@MatsAnd MatsAnd commented Dec 22, 2022

Hi, I ran into some problems when using this package in Kubernetes using a HTTP proxy that requires authentication, so I made some improvements to the HTTP proxy implementation.

Improvements:

  1. Added optional username and password proxy options. When specified this will set the Proxy-Authorization header with basic authentication.

    var options = {
      token: {
        key: "path/to/APNsAuthKey_XXXXXXXXXX.p8",
        keyId: "key-id",
        teamId: "developer-team-id"
      },
      proxy: {
        host: "192.168.10.92",
        port: 8080,
        username: "secretUsername",
        password: "secretPassword"
      },
      production: false
    };
    
    var apnProvider = new apn.Provider(options);
  2. Added support for retrieving HTTP proxy config from environment variables, instead of needing to specify it in options.
    This is useful in environments where HTTP proxy is configured automatically through Kubernetes for example. This also follows a normal convention across tools and other request libraries, and is normally the expected behaviour (reference).
    When one of the following environment variables is set we're using this value to connect (in this order):

    1. apn_proxy
    2. npm_config_[http/https]_proxy (npm config variable for http or https proxy)
    3. [http/https]_proxy
    4. all_proxy
    5. npm_config_proxy (npm config variable for general proxy)
    6. proxy

    To completely disable proxies and ignore environment variables set { proxy: false }. This is similar to how axios is handling proxy config.

  3. Added support for the no_proxy environment variable that makes sure urls that shouldn't be proxied isn't. This is also a commonly supported variable across tools, and is become a convention. (Reference).

These improvements are tested locally here, but help with some more testing would be appreciated!
(The same pr was created in the original module/node-apn#717, but ain't holding my breath for that to be merged..)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Dec 22, 2022

Thanks for opening this pull request!

  • ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant