x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to share ports #33377

efd6 · 2022-10-18T03:22:25Z

What does this PR do?

This change adds a pool of http servers that share listening addresses and ports but respond to different mux endpoints.

Why is it important?

This reduces configuration complexity and port exposure in filebeat agents.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

I won't address the linter complaints.

How to test this PR locally

The tests can be run using the go test command in x-pack/filebeat/input/http_endpoint.

Related issues

Closes [Filebeat] http_endpoint input instances should be able to "share" port #32578

Use cases

Screenshots

Logs

efd6 · 2022-10-18T04:00:38Z

x-pack/filebeat/input/http_endpoint/input_test.go

+					_ = servers.serve(ctx, cfg, &pub) // Always returns a non-nil error.
+				}()
+			}
+			time.Sleep(time.Second)


I'm not particularly proud of this, but I don't see another way to ensure that we have the servers up without doing nasty things inside pool.serve.

elasticmachine · 2022-10-18T04:36:55Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-10-27T00:54:19.477+0000
Duration: 72 min 58 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2299
Skipped	167
Total	2466

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2022-10-18T05:23:26Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

andrewkroh

I had some questions relating to handling of conflicting listener config between input instances.

Once we are settled on the behavior, the documentation for http_endpoint should be updated to describe this feature.

x-pack/filebeat/input/http_endpoint/input.go

andrewkroh · 2022-10-24T21:39:43Z

x-pack/filebeat/input/http_endpoint/input.go

+	p.servers[e.addr] = s
+	p.mu.Unlock()
+
+	if e.tlsConfig != nil {


For the case where one input instance is running with TLS and another is not, I think we should detect and prevent this.

On a similar note, should we prevent two inputs that are configured with different/inconsistent TLS options too?

I will add this check, and yes, I think inconsistent configs should be disallowed.

Hang on. This may not be the right thing to do; we already prevent two bindings of a pattern to an address (IP:port) so it is not possible to get into the situation where we have a conflicting TLS config. The next level of binding, where we are looking at an address as IP-only doesn't really make sense to check this for since there may be cases where one port expects TLS and another doesn't — I would imagine that this is rare, but the case where they have different TLS configurations would be less so.

Here's an example config that demonstrates the problem I am thinking of.

--- filebeat.inputs: - type: http_endpoint id: http_endpoint-no-ssl listen_address: 127.0.0.1 listen_port: 9001 url: '/webhook-alpha/' preserve_original_event: true # This one will start listening without TLS, but I would expect an error # since we cannot have both an HTTP and HTTPS listener on 127.0.0.1:9001. - type: http_endpoint id: http_endpoint-with-ssl listen_address: 127.0.0.1 listen_port: 9001 url: '/webhook-beta/' preserve_original_event: true ssl: verification_mode: none certificate: | -----BEGIN CERTIFICATE----- MIIBzjCCATGgAwIBAgIBATAKBggqhkjOPQQDBDASMRAwDgYDVQQKEwdBY21lIENv MB4XDTIyMTAyNTEyNDQ0OFoXDTIzMDQyMzEyNDQ0OFowEjEQMA4GA1UEChMHQWNt ZSBDbzCBmzAQBgcqhkjOPQIBBgUrgQQAIwOBhgAEANdwExxtIgYhw8kN485JxdAX Y/XzbXA5w1G1ubO3sOpXIcQnuOZC/ea0xnbq9JLOxs9u1glr/K9awsXGlg5JQ3K6 Aa6dBg5IQIGc6UBbetCaOx3QQkrrKJFrlF+6oiwn3Cs+V/qZMwsNgDdEeq2gFINj 3ede5TUE+jmsqy+qcNTZ9ik6ozUwMzAOBgNVHQ8BAf8EBAMCBaAwEwYDVR0lBAww CgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADAKBggqhkjOPQQDBAOBigAwgYYCQQH2 bwUzZWpwyvXoBGwWfGeKcmNSRFK+5Ur9k2C+PW+C+tVw0oqJvuEcJys78cB9TVwM OLBBY4Lip6CDGZ9Zu8MvAkETMcxFC2g3YIiHMFztfkMmU5XnVjjwjelmxNxREKUB pyD8tx7ptfI6hI8EUrqD1mQwnLLFU7Rq7IAbNuFXksYeWw== -----END CERTIFICATE----- # Not a secret. key: | -----BEGIN EC PRIVATE KEY----- MIHcAgEBBEIBtHeKAVWSypQo+PUDhUSMmh41Es6uCd+0OQhr3K9S3ntq7OjuMces CN2dG3WJBDo3TEfVJOhrNKE31zoM7+PE+kygBwYFK4EEACOhgYkDgYYABADXcBMc bSIGIcPJDePOScXQF2P1821wOcNRtbmzt7DqVyHEJ7jmQv3mtMZ26vSSzsbPbtYJ a/yvWsLFxpYOSUNyugGunQYOSECBnOlAW3rQmjsd0EJK6yiRa5RfuqIsJ9wrPlf6 mTMLDYA3RHqtoBSDY93nXuU1BPo5rKsvqnDU2fYpOg== -----END EC PRIVATE KEY----- output.console.enable: true

mergify · 2022-10-25T00:26:15Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 32578-http_endpoint upstream/32578-http_endpoint
git merge upstream/main
git push upstream 32578-http_endpoint

… share ports

andrewkroh

Can you please add a description of the new feature to the http_endpoint asciidocs.

andrewkroh

LGTM

… share ports (#33377) This makes it possible for a single address/port to be configured with multiple endpoints. Prior to this change different endpoint would require exposing unique ports for each, increasing configuration complexity and attack surface.

efd6 added enhancement Filebeat Filebeat Team:Security-External Integrations backport-skip Skip notification from the automated backport with mergify 8.6 candidate labels Oct 18, 2022

efd6 self-assigned this Oct 18, 2022

efd6 force-pushed the 32578-http_endpoint branch from 7d0ebe5 to 595baa8 Compare October 18, 2022 03:23

efd6 commented Oct 18, 2022

View reviewed changes

efd6 marked this pull request as ready for review October 18, 2022 05:23

efd6 requested a review from a team as a code owner October 18, 2022 05:23

efd6 requested a review from andrewkroh October 24, 2022 21:42

andrewkroh reviewed Oct 24, 2022

View reviewed changes

efd6 force-pushed the 32578-http_endpoint branch from 9f8c5a5 to 4b0c496 Compare October 25, 2022 00:01

efd6 added 2 commits October 25, 2022 11:11

x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to…

a3deeb3

… share ports

address pr comment: improve logging in failure case

3b86c3e

efd6 force-pushed the 32578-http_endpoint branch from 4b0c496 to 3b86c3e Compare October 25, 2022 00:42

address pr comment: detect inconsistent TLS configurations

3508085

andrewkroh approved these changes Oct 27, 2022

View reviewed changes

add doc for multiple endpoints

996e9db

efd6 requested a review from andrewkroh October 27, 2022 00:57

andrewkroh approved these changes Oct 27, 2022

View reviewed changes

efd6 merged commit d45c349 into elastic:main Oct 27, 2022

efd6 mentioned this pull request Nov 7, 2022

filebeat HTTP Endpoint does not handle trailing-slash redirect #33425

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to share ports #33377

x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to share ports #33377

efd6 commented Oct 18, 2022 •

edited

Loading

efd6 Oct 18, 2022

elasticmachine commented Oct 18, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

elasticmachine commented Oct 18, 2022

andrewkroh left a comment

andrewkroh Oct 24, 2022

efd6 Oct 24, 2022

efd6 Oct 24, 2022

andrewkroh Oct 25, 2022 •

edited

Loading

efd6 Oct 25, 2022

mergify bot commented Oct 25, 2022

andrewkroh left a comment

andrewkroh left a comment

x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to share ports #33377

x-pack/filebeat/input/http_endpoint: allow http_endpoint instances to share ports #33377

Conversation

efd6 commented Oct 18, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

efd6 Oct 18, 2022

Choose a reason for hiding this comment

elasticmachine commented Oct 18, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Oct 18, 2022

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Oct 24, 2022

Choose a reason for hiding this comment

efd6 Oct 24, 2022

Choose a reason for hiding this comment

efd6 Oct 24, 2022

Choose a reason for hiding this comment

andrewkroh Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

efd6 Oct 25, 2022

Choose a reason for hiding this comment

mergify bot commented Oct 25, 2022

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

efd6 commented Oct 18, 2022 •

edited

Loading

elasticmachine commented Oct 18, 2022 •

edited by jenkins-beats-ci bot

Loading

andrewkroh Oct 25, 2022 •

edited

Loading