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: add support for SCRAM authentication for kafka metricbeat module, fixes #19648 #24810

Merged
merged 13 commits into from
Apr 1, 2021

Conversation

fholzer
Copy link
Contributor

@fholzer fholzer commented Mar 29, 2021

What does this PR do?

Adds SCRAM authentication for kafka to metricbeat's kafka module. Moves shared code from kafka output plugin to libbeat/common/kafka/

Why is it important?

Currently metricbeat can't pull metrics from Kafka clusters that require authentication, which is a problem in enterprise environments where authentication is a requirement.

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.

I think most of this isn't applicable, as this PR virtually just moves files around and makes couple things public in the shared code. This virtually no new code introduced.

Reg. "I have made corresponding changes to the documentation", it seems that none of the kafka module's fields in metricbeat are separately documented. Instead the config.yml is pulled into the asciidoc file, which I updated.

How to test this PR locally

As this PR mostly just moves existing files around and introduces only minimal code changes, the test procedures remain the same as those for #12867 which initially introduced SCRAM code. Though it seems those were not documented at the time. Generally, though, the procedure documented in beats/blob/master/metricbeat/module/kafka/README.md remains the same, with the exception that the test kafka cluster needs to use one of the SCAM mechanisms, eg. in server.properties have listener.name.external.sasl.enabled.mechanisms=SCRAM-SHA-256 configured. (if you external listener is called EXTERNAL)

Related issues

Use cases

This PR introduces sasl.mechanism in metricbeat's kafka input module. It works the same way as the similarly named configuration directive in libbeat's kafka output plugin.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 29, 2021
@cla-checker-service
Copy link

cla-checker-service bot commented Mar 29, 2021

💚 CLA has been signed

@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 29, 2021

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #24810 updated

  • Start Time: 2021-03-31T18:20:55.173+0000

  • Duration: 53 min 55 sec

  • Commit: bc819df

Test stats 🧪

Test Results
Failed 1
Passed 45948
Skipped 5012
Total 50961

Trends 🧪

Image of Build Times

Image of Tests

Test errors 1

Expand to view the tests failures

Build&Test / filebeat-windows-windows-2019 / test_restart_recursive_glob – filebeat.tests.system.test_input.Test
    Expand to view the error details

     beat.beat.TimeoutError: Timeout waiting for 'output contains 'entry2'' to be true. Waited 10 seconds. 
    

    Expand to view the stacktrace

     self = <test_input.Test testMethod=test_restart_recursive_glob>
    
        def test_restart_recursive_glob(self):
            """
            Check that file reading via recursive glob patterns continues after restart
            """
            self.render_config_template(
                path=os.path.abspath(self.working_dir) + "/log/**",
                scan_frequency="1s"
            )
        
            testfile_dir = os.path.join(self.working_dir, "log", "some", "other", "subdir")
            os.makedirs(testfile_dir)
            testfile_path = os.path.join(testfile_dir, "input")
        
            filebeat = self.start_beat()
        
            with open(testfile_path, 'w') as testfile:
                testfile.write("entry1\n")
        
            self.wait_until(
                lambda: self.output_has_message("entry1"),
                max_timeout=10,
                name="output contains 'entry1'")
        
            filebeat.check_kill_and_wait()
        
            # Append to file
            with open(testfile_path, 'a') as testfile:
                testfile.write("entry2\n")
        
            filebeat = self.start_beat(output="filebeat2.log")
        
    >       self.wait_until(
                lambda: self.output_has_message("entry2"),
                max_timeout=10,
                name="output contains 'entry2'")
    
    tests\system\test_input.py:641: 
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
    
    self = <test_input.Test testMethod=test_restart_recursive_glob>
    cond = <function Test.test_restart_recursive_glob.<locals>.<lambda> at 0x000001A35E344280>
    max_timeout = 10, poll_interval = 0.1, name = "output contains 'entry2'"
    
        def wait_until(self, cond, max_timeout=10, poll_interval=0.1, name="cond"):
            """
            Waits until the cond function returns true,
            or until the max_timeout is reached. Calls the cond
            function every poll_interval seconds.
        
            If the max_timeout is reached before cond() returns
            true, an exception is raised.
            """
            start = datetime.now()
            while not cond():
                if datetime.now() - start > timedelta(seconds=max_timeout):
    >               raise TimeoutError("Timeout waiting for '{}' to be true. ".format(name) +
                                       "Waited {} seconds.".format(max_timeout))
    E               beat.beat.TimeoutError: Timeout waiting for 'output contains 'entry2'' to be true. Waited 10 seconds.
    
    ..\libbeat\tests\system\beat\beat.py:362: TimeoutError 
    

Steps errors 2

Expand to view the steps failures

filebeat-windows-windows-2019 - mage build unitTest
  • Took 6 min 7 sec . View more details on here
  • Description: mage build unitTest
Error signal
  • Took 0 min 0 sec . View more details on here
  • Description: Error 'hudson.AbortException: script returned exit code 1'

Log output

Expand to view the last 100 lines of log output

[2021-03-31T19:12:46.150Z]   Version:          1.4.4
[2021-03-31T19:12:46.150Z]   GitCommit:        05f951a3781f4f2c1911b05e61c160e9c30eaa8e
[2021-03-31T19:12:46.150Z]  runc:
[2021-03-31T19:12:46.150Z]   Version:          1.0.0-rc93
[2021-03-31T19:12:46.150Z]   GitCommit:        12644e614e25b05da6fd08a38ffa0cfe1903fdec
[2021-03-31T19:12:46.150Z]  docker-init:
[2021-03-31T19:12:46.150Z]   Version:          0.19.0
[2021-03-31T19:12:46.150Z]   GitCommit:        de40ad0
[2021-03-31T19:12:46.150Z] Change ownership of all files inside the specific folder from root/root to current user/group
[2021-03-31T19:12:49.182Z] Cleaning up /var/lib/jenkins/workspace/PR-24810-10-877106c2-446f-4650-80f1-cd13092a7838
[2021-03-31T19:12:49.182Z] Client: Docker Engine - Community
[2021-03-31T19:12:49.182Z]  Version:           20.10.3
[2021-03-31T19:12:49.182Z]  API version:       1.41
[2021-03-31T19:12:49.182Z]  Go version:        go1.13.15
[2021-03-31T19:12:49.182Z]  Git commit:        48d30b5
[2021-03-31T19:12:49.182Z]  Built:             Fri Jan 29 14:33:13 2021
[2021-03-31T19:12:49.182Z]  OS/Arch:           linux/amd64
[2021-03-31T19:12:49.182Z]  Context:           default
[2021-03-31T19:12:49.182Z]  Experimental:      true
[2021-03-31T19:12:49.182Z] 
[2021-03-31T19:12:49.182Z] Server: Docker Engine - Community
[2021-03-31T19:12:49.182Z]  Engine:
[2021-03-31T19:12:49.182Z]   Version:          20.10.3
[2021-03-31T19:12:49.182Z]   API version:      1.41 (minimum version 1.12)
[2021-03-31T19:12:49.182Z]   Go version:       go1.13.15
[2021-03-31T19:12:49.182Z]   Git commit:       46229ca
[2021-03-31T19:12:49.182Z]   Built:            Fri Jan 29 14:31:25 2021
[2021-03-31T19:12:49.182Z]   OS/Arch:          linux/amd64
[2021-03-31T19:12:49.182Z]   Experimental:     false
[2021-03-31T19:12:49.182Z]  containerd:
[2021-03-31T19:12:49.182Z]   Version:          1.4.4
[2021-03-31T19:12:49.182Z]   GitCommit:        05f951a3781f4f2c1911b05e61c160e9c30eaa8e
[2021-03-31T19:12:49.182Z]  runc:
[2021-03-31T19:12:49.182Z]   Version:          1.0.0-rc93
[2021-03-31T19:12:49.182Z]   GitCommit:        12644e614e25b05da6fd08a38ffa0cfe1903fdec
[2021-03-31T19:12:49.182Z]  docker-init:
[2021-03-31T19:12:49.182Z]   Version:          0.19.0
[2021-03-31T19:12:49.182Z]   GitCommit:        de40ad0
[2021-03-31T19:12:49.182Z] Change ownership of all files inside the specific folder from root/root to current user/group
[2021-03-31T19:12:49.182Z] Unable to find image 'alpine:3.4' locally
[2021-03-31T19:12:49.753Z] 3.4: Pulling from library/alpine
[2021-03-31T19:12:50.014Z] c1e54eec4b57: Pulling fs layer
[2021-03-31T19:12:50.282Z] c1e54eec4b57: Download complete
[2021-03-31T19:12:50.545Z] c1e54eec4b57: Pull complete
[2021-03-31T19:12:50.545Z] Digest: sha256:b733d4a32c4da6a00a84df2ca32791bb03df95400243648d8c539e7b4cce329c
[2021-03-31T19:12:50.545Z] Status: Downloaded newer image for alpine:3.4
[2021-03-31T19:12:52.459Z] Change permissions with write access of all files inside the specific folder
[2021-03-31T19:12:53.866Z] Running in /var/lib/jenkins/workspace/PR-24810-10-877106c2-446f-4650-80f1-cd13092a7838/src/github.com/elastic/beats/build
[2021-03-31T19:12:54.164Z] + rm -rf ve
[2021-03-31T19:12:54.165Z] + find . -type d -name vendor -exec rm -r {} ;
[2021-03-31T19:12:54.484Z] + python .ci/scripts/pre_archive_test.py
[2021-03-31T19:12:56.401Z] Copy ./x-pack/filebeat/build into build/x-pack/filebeat/build
[2021-03-31T19:12:56.419Z] Running in /var/lib/jenkins/workspace/PR-24810-10-877106c2-446f-4650-80f1-cd13092a7838/src/github.com/elastic/beats/build
[2021-03-31T19:12:56.481Z] Recording test results
[2021-03-31T19:12:58.225Z] [Checks API] No suitable checks publisher found.
[2021-03-31T19:12:58.596Z] + go clean -modcache
[2021-03-31T19:13:02.216Z] Cleaning up /var/lib/jenkins/workspace/PR-24810-10-877106c2-446f-4650-80f1-cd13092a7838
[2021-03-31T19:13:02.216Z] Client: Docker Engine - Community
[2021-03-31T19:13:02.216Z]  Version:           20.10.3
[2021-03-31T19:13:02.216Z]  API version:       1.41
[2021-03-31T19:13:02.216Z]  Go version:        go1.13.15
[2021-03-31T19:13:02.216Z]  Git commit:        48d30b5
[2021-03-31T19:13:02.216Z]  Built:             Fri Jan 29 14:33:13 2021
[2021-03-31T19:13:02.216Z]  OS/Arch:           linux/amd64
[2021-03-31T19:13:02.216Z]  Context:           default
[2021-03-31T19:13:02.216Z]  Experimental:      true
[2021-03-31T19:13:02.216Z] 
[2021-03-31T19:13:02.216Z] Server: Docker Engine - Community
[2021-03-31T19:13:02.216Z]  Engine:
[2021-03-31T19:13:02.216Z]   Version:          20.10.3
[2021-03-31T19:13:02.216Z]   API version:      1.41 (minimum version 1.12)
[2021-03-31T19:13:02.216Z]   Go version:       go1.13.15
[2021-03-31T19:13:02.216Z]   Git commit:       46229ca
[2021-03-31T19:13:02.216Z]   Built:            Fri Jan 29 14:31:25 2021
[2021-03-31T19:13:02.216Z]   OS/Arch:          linux/amd64
[2021-03-31T19:13:02.216Z]   Experimental:     false
[2021-03-31T19:13:02.216Z]  containerd:
[2021-03-31T19:13:02.216Z]   Version:          1.4.4
[2021-03-31T19:13:02.216Z]   GitCommit:        05f951a3781f4f2c1911b05e61c160e9c30eaa8e
[2021-03-31T19:13:02.216Z]  runc:
[2021-03-31T19:13:02.216Z]   Version:          1.0.0-rc93
[2021-03-31T19:13:02.216Z]   GitCommit:        12644e614e25b05da6fd08a38ffa0cfe1903fdec
[2021-03-31T19:13:02.216Z]  docker-init:
[2021-03-31T19:13:02.216Z]   Version:          0.19.0
[2021-03-31T19:13:02.216Z]   GitCommit:        de40ad0
[2021-03-31T19:13:02.216Z] Change ownership of all files inside the specific folder from root/root to current user/group
[2021-03-31T19:13:08.817Z] Change permissions with write access of all files inside the specific folder
[2021-03-31T19:13:09.113Z] Running in /var/lib/jenkins/workspace/PR-24810-10-877106c2-446f-4650-80f1-cd13092a7838
[2021-03-31T19:13:42.440Z] Change permissions with write access of all files inside the specific folder
[2021-03-31T19:13:42.470Z] Running in /var/lib/jenkins/workspace/PR-24810-10-23176139-8373-43bf-a90c-e514248f148d
[2021-03-31T19:13:48.571Z] Stage "Packaging" skipped due to earlier failure(s)
[2021-03-31T19:13:48.647Z] Running in /var/lib/jenkins/workspace/Beats_beats_PR-24810/src/github.com/elastic/beats
[2021-03-31T19:13:48.971Z] Running on worker-395930 in /var/lib/jenkins/workspace/Beats_beats_PR-24810
[2021-03-31T19:13:49.027Z] [INFO] getVaultSecret: Getting secrets
[2021-03-31T19:13:49.124Z] Masking supported pattern matches of $VAULT_ADDR or $VAULT_ROLE_ID or $VAULT_SECRET_ID
[2021-03-31T19:13:51.218Z] + chmod 755 generate-build-data.sh
[2021-03-31T19:13:51.218Z] + ./generate-build-data.sh https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-24810/ https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-24810/runs/10 FAILURE 3174650
[2021-03-31T19:13:51.218Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-24810/runs/10/steps/?limit=10000 -o steps-info.json
[2021-03-31T19:14:00.805Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-24810/runs/10/tests/?status=FAILED -o tests-errors.json
[2021-03-31T19:14:02.255Z] INFO: curl https://beats-ci.elastic.co/blue/rest/organizations/jenkins/pipelines/Beats/beats/PR-24810/runs/10/log/ -o pipeline-log.txt

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Test stats 🧪

Test Results
Failed 1
Passed 45948
Skipped 5012
Total 50961

Genuine test errors 1

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

  • Name: Build&Test / filebeat-windows-windows-2019 / test_restart_recursive_glob – filebeat.tests.system.test_input.Test

@jsoriano jsoriano added the Team:Integrations Label for the Integrations team label Mar 29, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 29, 2021
@jsoriano jsoriano self-assigned this Mar 29, 2021
@jsoriano jsoriano self-requested a review March 29, 2021 19:19
@jsoriano
Copy link
Member

/test

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

Could you please add a changelog entry in CHANGELOG.next.asciidoc mentioning that SCRAM is supported in the Metricbeat module?

You will also need to sign the CLA so we can accept the change.

metricbeat/module/kafka/broker.go Outdated Show resolved Hide resolved
metricbeat/module/kafka/broker.go Outdated Show resolved Hide resolved
libbeat/common/kafka/sasl.go Outdated Show resolved Hide resolved
libbeat/common/kafka/sasl.go Show resolved Hide resolved
libbeat/common/kafka/sasl.go Outdated Show resolved Hide resolved
metricbeat/module/kafka/broker.go Outdated Show resolved Hide resolved
fholzer and others added 3 commits March 30, 2021 18:34
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
At this point input validation should have been done already.
@fholzer
Copy link
Contributor Author

fholzer commented Mar 30, 2021

CLA is signed

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for signing the CLA and applying the suggested changes! Added a couple of small comments.

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
libbeat/common/kafka/sasl.go Show resolved Hide resolved
@jsoriano
Copy link
Member

/test

fholzer and others added 2 commits March 31, 2021 10:39
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jsoriano
Copy link
Member

/test

@jsoriano jsoriano added the needs_backport PR is waiting to be backported to other branches. label Mar 31, 2021
@jsoriano
Copy link
Member

@fholzer linting failures in metricbeat are related to this change, you will need to run mage update in metricbeat and x-pack/metricbeat directories and commit the changes. Failures in filebeat are not related.

@jsoriano
Copy link
Member

jsoriano commented Apr 1, 2021

Failing test is not related, merging.

@jsoriano jsoriano added the test-plan Add this PR to be manual test plan label Apr 1, 2021
@jsoriano jsoriano merged commit 66829ff into elastic:master Apr 1, 2021
jsoriano pushed a commit to jsoriano/beats that referenced this pull request Apr 1, 2021
@jsoriano jsoriano added v7.13.0 and removed needs_backport PR is waiting to be backported to other branches. labels Apr 1, 2021
jsoriano added a commit that referenced this pull request Apr 1, 2021
…) (#24889)

(cherry picked from commit 66829ff)

Co-authored-by: Ferdinand Holzer <fholzer@users.noreply.github.com>
@fholzer fholzer deleted the metricbeat-kafka-scram2 branch April 6, 2021 06:46
v1v added a commit to v1v/beats that referenced this pull request Apr 7, 2021
* upstream/master: (91 commits)
  [Filebeat] Change okta.target to nested field (elastic#24636)
  Add RFC5424 format support for syslog input  (elastic#23954)
  Fix links to Beats product pages (elastic#24821)
  [DOCS] Fix 'make setup' instructions for a new beat (elastic#24944)
  Remove duplicate decode_xml entry (elastic#24941)
  [libbeat] Add wineventlog schema to decode_xml processor (elastic#24726)
  [Elastic Agent] Add check for URL set when cert and cert key. (elastic#24904)
  feat: stage execution cache (elastic#24780)
  Fix error in Journalbeat commands (elastic#24880)
  Add baseline ECS 1.9.0 upgrade (elastic#24909)
  [Elastic Agent] Cloud container legacy apm files. (elastic#24896)
  [Elastic Agent]: Reduce allowed socket path length (elastic#24914)
  Add ability to destroy indices with wildcards in testing (elastic#24915)
  Add status subcommand to report status of running daemon. (elastic#24856)
  Fix types of fields GetHits and Ops in Metricbeat module for Couchbase (elastic#23287)
  Add support for Filestream input in elastic agent. (elastic#24820)
  Implement k8s secrets provider for Agent (elastic#24789)
  Sort processor list in docs (elastic#24874)
  Add support for SCRAM authentication in kafka metricbeat module (elastic#24810)
  Properly update offset in case of unparasable line (elastic#22685)
  ...
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SCRAM authentication to kafka metrics source
4 participants