Skip to content

[ResponseOps] POC: switch to use hpagent for proxy#131971

Closed
pmuellr wants to merge 2 commits intoelastic:mainfrom
pmuellr:actions/proxy-tests-102935
Closed

[ResponseOps] POC: switch to use hpagent for proxy#131971
pmuellr wants to merge 2 commits intoelastic:mainfrom
pmuellr:actions/proxy-tests-102935

Conversation

@pmuellr
Copy link
Contributor

@pmuellr pmuellr commented May 10, 2022

resolves: #102935
resolves: #125837

Todo

  • wait for Upgrade axios dependency (0.21.10.27.2). #111655 to be merged (axios upgrade)
  • finish test cases - mark as skip those that fail that shouldn't?
  • create trouble-shooting doc on proxies, publish somewhere
  • figure out the story for dealing with proxys that require certs (eg, private-ca-https-proxy => slack) - and how would this work if the proxy and target both need certs and they're different - feels like we need a separate proxy ssl config section (verificationMode, cert data / files)
  • figure out if we're providing as much error information as we can, when errors occur

Summary

Currently just trying to build some tests for testing different proxy combinations. To start with, combinations of the following:

  • proxy protocol: http or https
  • proxy authentication: none or basic
  • target protocol: http or https
  • target authentication: none or basic

We'll probably want more, for example, testing https targets with custom certs - it's not clear if proxy's having custom certs is a thing that's even possible (due to the way the proxy CONNECT protocol works). So that makes 16 tests.

At this point, the tests I have basically hang, so hit jest timeouts. Debugging these is awful, especially in jest, so I've created a new stand-alone proxy in x-pack/plugins/actions/server/manual_tests/forward_proxy_ng.js - this one can create multiple proxies in one invocation, with different options for each - port, protocol, auth.

Here are some pre-configured webhook connectors that will make it easy to "test" the proxies from Kibana, outside of jest; the webhook-https-es is a bit of a hack, I think I'll change this to a _search instead - we want something we can POST with that we can send a simple body to for the test. More will be required, for auth and such, if we want to reuse these for other manual tests. But hoping we can automate it all.

pre-configured webhook connectors
xpack.actions.preconfigured:
  webhook-http-example:
    name: 'webhook: http://example.com'
    actionTypeId: '.webhook'
    config:
      method: "post"
      hasAuth: false
      url: "http://example.com"
  webhook-https-example:
    name: 'webhook: https://example.com'
    actionTypeId: '.webhook'
    config:
      method: "post"
      hasAuth: false
      url: "https://example.com"
  webhook-https-es:
    name: 'webhook: https://elastic:changeme@localhost:9200'
    actionTypeId: '.webhook'
    config:
      method: "post"
      hasAuth: true
      url: "https://elastic:changeme@localhost:9200/_features/_reset"
    secrets:
      user: 'elastic'
      password: 'changeme'

At this point, I'd like to wait for axios update PR to merge, and likely we will switch from using http-proxy-agent and https-proxy-agent, to using hpagent instead, which I've had better luck with than the packages I'm replacing it with.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@pmuellr
Copy link
Contributor Author

pmuellr commented May 11, 2022

Note that there appears to be a ready-to-run docker image for Squid, to use for testing a presumably http proxy:

https://hub.docker.com/r/minimum2scp/squid

Quick test seems to show it works:

$ docker run -i --rm -p 3128:3128 minimum2scp/squid
WARNING: The requested image's platform (linux/amd64) does not match the detected 
host platform (linux/arm64/v8) and no specific platform was requested

Note that it runs fine on my Mac M1 box despite the warning, though does not respond to ctrl-c to exit. So running with -i is a little pointless. Instead, find the container with docker ps and kill with docker kill <container>

$ http_proxy=http://127.0.0.1:3128 curl -v http://example.com/
* Uses proxy env variable http_proxy == 'http://127.0.0.1:3128'
*   Trying 127.0.0.1:3128...
* Connected to 127.0.0.1 (127.0.0.1) port 3128 (#0)
> GET http://example.com/ HTTP/1.1
> Host: example.com
> User-Agent: curl/7.79.1
> Accept: */*
> Proxy-Connection: Keep-Alive
> ...

@pmuellr pmuellr force-pushed the actions/proxy-tests-102935 branch from 780aae7 to 5e4cfd2 Compare June 8, 2022 19:36
@kibana-ci
Copy link

kibana-ci commented Jun 8, 2022

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #4 / execute() calls the mock executor with success proxy
  • [job] [logs] Jest Tests #4 / execute() ensure proxy bypass will not bypass when expected
  • [job] [logs] Jest Tests #4 / execute() ensure proxy only will proxy when expected
  • [job] [logs] Jest Tests #4 / getCustomAgents get agents for valid proxy URL
  • [job] [logs] Jest Tests #4 / getCustomAgents handles custom host settings with proxy
  • [job] [logs] Jest Tests #4 / getCustomAgents handles overriding global verificationMode "full" with a proxy
  • [job] [logs] Jest Tests #4 / getCustomAgents handles overriding global verificationMode "none" with a proxy
  • [job] [logs] Jest Tests #4 / getCustomAgents returns proxy agents for matching proxyOnlyHosts
  • [job] [logs] Jest Tests #4 / getCustomAgents returns proxy agents for non-matching proxyBypassHosts
  • [job] [logs] Jest Tests #4 / request it does not bypass with proxyBypassHosts when expected
  • [job] [logs] Jest Tests #4 / request it have been called with proper proxy agent for a valid url
  • [job] [logs] Jest Tests #4 / request it proxies with proxyOnlyHosts when expected
  • [job] [logs] Jest Tests #4 / SubActionConnector Proxy does not bypass with proxyBypassHosts when expected
  • [job] [logs] Jest Tests #4 / SubActionConnector Proxy have been called with proper proxy agent for a valid url
  • [job] [logs] Jest Tests #4 / SubActionConnector Proxy proxies with proxyOnlyHosts when expected

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
apm 14 13 -1
securitySolution 71 70 -1
synthetics 8 7 -1
total -3

ESLint disabled line counts

id before after diff
actions 22 23 +1
apm 84 81 -3
enterpriseSearch 9 7 -2
synthetics 61 55 -6
ux 13 12 -1
total -11

Total ESLint disabled count

id before after diff
actions 28 29 +1
apm 98 94 -4
enterpriseSearch 9 7 -2
securitySolution 514 513 -1
synthetics 69 62 -7
ux 16 15 -1
total -14

History

  • 💔 Build #43845 failed 780aae78d4d85961735ec60637a5d6c9c60a6adb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@pmuellr
Copy link
Contributor Author

pmuellr commented Jun 8, 2022

As of commit 5e4cfd2, there are a number of new proxy tests added; testing http vs https for the target and proxy, and using auth or not. The proxy code this is using does not really support https proxy's, unless the CA is somehow automatically trusted by node, which is unlikely to be the case. See delvedor/hpagent#69 for more details. I think we'd want a way to use rejectUnauthorized: false, or equivalent, in the agent, before we adopt this.

The tests are kind of fudged as well. It appears we may not be setting all the options correctly for private certs, so some of the tests are setting options to do that, that are known to work. Hoping this can be resolved with some additional tests in the get_custom_agents code.

Now, the telling moment. I took this new test, and ran it with the old agents.

Results, with all of the newly added tests in a new section proxy:

test results
result test target protocol target auth proxy protocol proxy auth
basic; http http
basic; http http y
basic; http y http
basic; http y http y
basic; https http
rejectUnauthorized target; https http
custom CA target; https http
verModeNone target; https http
basic; https http y
rejectUnauthorized target; https http y
custom CA target; https http y
verModeNone target; https http y
basic; https y http
rejectUnauthorized target; https y http
custom CA target; https y http
verModeNone target; https y http
basic; https y http y
rejectUnauthorized target; https y http y
custom CA target; https y http y
verModeNone target; https y http y

most of them failed! :-(

Some of the failures could be because of the proxy implementation added in these tests. The tests did all pass with hpagent, but perhaps there was something slightly off. Some known failure scenarios in the field (https target with http proxy) failed here also, so there's a least a little correlation.

The table removed tests that were expected to be errors, since that's non-interesting, and it turns out the two agents handle errors differently. hpagent throws errors, where the old agent code returns http status responses when possible. It doesn't matter too much, an error will end up getting thrown by the connector run, regardless. And those are just kinda of noisy with all the other combinations.

Tests using an https proxy were not run, as they are known to be unsupported at the moment in hpagent (for what we need - support for self-signed certs), but they also all failed with the older agents.

raw test results using existing agent
  axios connections
    http
      ✓ it works (17 ms)
    https
      ✓ it fails with self-signed cert and no overrides (11 ms)
      ✓ it works with verificationMode "none" config (5 ms)
      ✓ it works with verificationMode "none" for custom host config (4 ms)
      ✓ it works with ca in custom host config (4 ms)
      ✓ it fails with incorrect ca in custom host config (3 ms)
      ✓ it works with incorrect ca in custom host config but verificationMode "none" (4 ms)
      ✓ it works with incorrect ca in custom host config but verificationMode config "full" (7 ms)
      ✓ it fails with no matching custom host settings (4 ms)
      ✓ it fails cleanly with a garbage CA 1 (3 ms)
      ✓ it fails cleanly with a garbage CA 2 (4 ms)
    proxy
      ✓ basic; target https x auth x; proxy https x auth x (5 ms)
      ✓ basic; target https x auth x; proxy https x auth ✓ (4 ms)
      ✕ wrong proxy password; target https x auth x; proxy https x auth ✓ (4 ms)
      ✕ missing proxy password; target https x auth x; proxy https x auth ✓ (4 ms)
      ✕ basic; target https x auth x; proxy https ✓ auth x (5 ms)
      ✕ basic; target https x auth x; proxy https ✓ auth ✓ (4 ms)
      ✕ wrong proxy password; target https x auth x; proxy https ✓ auth ✓ (5 ms)
      ✕ missing proxy password; target https x auth x; proxy https ✓ auth ✓ (10 ms)
      ✓ basic; target https x auth ✓; proxy https x auth x (13 ms)
      ✓ wrong target password; target https x auth ✓; proxy https x auth x (3 ms)
      ✓ missing target password; target https x auth ✓; proxy https x auth x (3 ms)
      ✓ basic; target https x auth ✓; proxy https x auth ✓ (2 ms)
      ✓ wrong target password; target https x auth ✓; proxy https x auth ✓ (2 ms)
      ✓ missing target password; target https x auth ✓; proxy https x auth ✓ (3 ms)
      ✕ wrong proxy password; target https x auth ✓; proxy https x auth ✓ (3 ms)
      ✕ missing proxy password; target https x auth ✓; proxy https x auth ✓ (4 ms)
      ✕ basic; target https x auth ✓; proxy https ✓ auth x (3 ms)
      ✕ wrong target password; target https x auth ✓; proxy https ✓ auth x (4 ms)
      ✕ missing target password; target https x auth ✓; proxy https ✓ auth x (4 ms)
      ✕ basic; target https x auth ✓; proxy https ✓ auth ✓ (4 ms)
      ✕ wrong target password; target https x auth ✓; proxy https ✓ auth ✓ (3 ms)
      ✕ missing target password; target https x auth ✓; proxy https ✓ auth ✓ (4 ms)
      ✕ wrong proxy password; target https x auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✕ missing proxy password; target https x auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✕ basic; target https ✓ auth x; proxy https x auth x (12 ms)
      ✓ missing CA; target https ✓ auth x; proxy https x auth x (5 ms)
      ✕ rejectUnauthorized target; target https ✓ auth x; proxy https x auth x (7 ms)
      ✕ custom CA target; target https ✓ auth x; proxy https x auth x (5 ms)
      ✕ verModeNone target; target https ✓ auth x; proxy https x auth x (6 ms)
      ✕ basic; target https ✓ auth x; proxy https x auth ✓ (4 ms)
      ✕ wrong proxy password; target https ✓ auth x; proxy https x auth ✓ (5 ms)
      ✕ missing proxy password; target https ✓ auth x; proxy https x auth ✓ (4 ms)
      ✕ missing CA; target https ✓ auth x; proxy https x auth ✓ (5 ms)
      ✕ rejectUnauthorized target; target https ✓ auth x; proxy https x auth ✓ (4 ms)
      ✕ custom CA target; target https ✓ auth x; proxy https x auth ✓ (4 ms)
      ✕ verModeNone target; target https ✓ auth x; proxy https x auth ✓ (4 ms)
      ✕ basic; target https ✓ auth x; proxy https ✓ auth x (6 ms)
      ✓ missing CA; target https ✓ auth x; proxy https ✓ auth x (4 ms)
      ✕ rejectUnauthorized target; target https ✓ auth x; proxy https ✓ auth x (4 ms)
      ✕ custom CA target; target https ✓ auth x; proxy https ✓ auth x (5 ms)
      ✕ verModeNone target; target https ✓ auth x; proxy https ✓ auth x (6 ms)
      ✕ basic; target https ✓ auth x; proxy https ✓ auth ✓ (7 ms)
      ✕ wrong proxy password; target https ✓ auth x; proxy https ✓ auth ✓ (5 ms)
      ✕ missing proxy password; target https ✓ auth x; proxy https ✓ auth ✓ (10 ms)
      ✓ missing CA; target https ✓ auth x; proxy https ✓ auth ✓ (5 ms)
      ✕ rejectUnauthorized target; target https ✓ auth x; proxy https ✓ auth ✓ (5 ms)
      ✕ custom CA target; target https ✓ auth x; proxy https ✓ auth ✓ (4 ms)
      ✕ verModeNone target; target https ✓ auth x; proxy https ✓ auth ✓ (5 ms)
      ✕ basic; target https ✓ auth ✓; proxy https x auth x (8 ms)
      ✕ wrong target password; target https ✓ auth ✓; proxy https x auth x (6 ms)
      ✕ missing target password; target https ✓ auth ✓; proxy https x auth x (6 ms)
      ✓ missing CA; target https ✓ auth ✓; proxy https x auth x (6 ms)
      ✕ rejectUnauthorized target; target https ✓ auth ✓; proxy https x auth x (6 ms)
      ✕ custom CA target; target https ✓ auth ✓; proxy https x auth x (5 ms)
      ✕ verModeNone target; target https ✓ auth ✓; proxy https x auth x (7 ms)
      ✕ basic; target https ✓ auth ✓; proxy https x auth ✓ (3 ms)
      ✕ wrong target password; target https ✓ auth ✓; proxy https x auth ✓ (5 ms)
      ✕ missing target password; target https ✓ auth ✓; proxy https x auth ✓ (4 ms)
      ✕ wrong proxy password; target https ✓ auth ✓; proxy https x auth ✓ (4 ms)
      ✕ missing proxy password; target https ✓ auth ✓; proxy https x auth ✓ (2 ms)
      ✕ missing CA; target https ✓ auth ✓; proxy https x auth ✓ (4 ms)
      ✕ rejectUnauthorized target; target https ✓ auth ✓; proxy https x auth ✓ (4 ms)
      ✕ custom CA target; target https ✓ auth ✓; proxy https x auth ✓ (3 ms)
      ✕ verModeNone target; target https ✓ auth ✓; proxy https x auth ✓ (4 ms)
      ✕ basic; target https ✓ auth ✓; proxy https ✓ auth x (5 ms)
      ✕ wrong target password; target https ✓ auth ✓; proxy https ✓ auth x (4 ms)
      ✕ missing target password; target https ✓ auth ✓; proxy https ✓ auth x (4 ms)
      ✓ missing CA; target https ✓ auth ✓; proxy https ✓ auth x (4 ms)
      ✕ rejectUnauthorized target; target https ✓ auth ✓; proxy https ✓ auth x (4 ms)
      ✕ custom CA target; target https ✓ auth ✓; proxy https ✓ auth x (5 ms)
      ✕ verModeNone target; target https ✓ auth ✓; proxy https ✓ auth x (4 ms)
      ✕ basic; target https ✓ auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✕ wrong target password; target https ✓ auth ✓; proxy https ✓ auth ✓ (8 ms)
      ✕ missing target password; target https ✓ auth ✓; proxy https ✓ auth ✓ (4 ms)
      ✕ wrong proxy password; target https ✓ auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✕ missing proxy password; target https ✓ auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✓ missing CA; target https ✓ auth ✓; proxy https ✓ auth ✓ (5 ms)
      ✕ rejectUnauthorized target; target https ✓ auth ✓; proxy https ✓ auth ✓ (6 ms)
      ✕ custom CA target; target https ✓ auth ✓; proxy https ✓ auth ✓ (4 ms)
      ✕ verModeNone target; target https ✓ auth ✓; proxy https ✓ auth ✓ (4 ms)

@pmuellr
Copy link
Contributor Author

pmuellr commented Jun 28, 2022

Brandon suggested moving the tests to a jest integration folder, since they are more integrate-y than unit-y.

It appears what we'll need to do is add a new x-pack/plugins/actions/jest.integration.config.js file (copy pasta from x-pack/plugins/task_manager/jest.integration.config.js), and then move the test to x-pack/plugins/actions/server/integration_tests

@pmuellr
Copy link
Contributor Author

pmuellr commented Jul 12, 2022

I re-found this SO post regarding the "old" http-proxy package we're currently using for integration tests. It doesn't support HTTP CONNECT, just simple http forwarding, and we likely should replace it with the proxy package, https usage, etc.

https://stackoverflow.com/questions/8165570/https-proxy-server-in-node-js

@pmuellr pmuellr changed the title WIP: adds additional actions proxy tests [ResponseOps] POC: switch to use hpagent for proxy Aug 8, 2022
@legrego legrego closed this Jul 17, 2024
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.

[actions] add extensive proxy + email jest tests [ResponseOps][connectors] investigate using http proxy service forwarding to https services

3 participants