[Metricbeat][Aerospike Module] Add support for TLS

[herrBez]

Proposed commit message

[Aerospike] Add TLS support

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

How to test this PR locally

Aerospike has a documentation https://aerospike.com/docs/server/guide/security/tls documentation that explains how to configure aerospike with TLS.

Nevertheless, in my tests I used NGINX with the following configuration to perform SSL termination:

user  nginx;
worker_processes  auto;

events {
}

stream {
    server {
       listen 4333 ssl;
       proxy_pass stream_aerospike;
       ssl_certificate     /etc/nginx/certs/remote_fake.crt;
       ssl_certificate_key /etc/nginx/certs/remote_fake.key;
       ssl_client_certificate /etc/nginx/certs/ca.crt;

     }

     upstream stream_aerospike {
         # hash $remote_addr consistent;
         server aerospike:3000;
     }
}

And the following docker-compose.yml:

version: '2.3'

services:
  aerospike:
    image: docker.elastic.co/integrations-ci/beats-aerospike:${AEROSPIKE_VERSION:-3.9.0}-1
    build:
      context: ./_meta
      args:
        AEROSPIKE_VERSION: ${AEROSPIKE_VERSION:-3.9.0}
    ports:
      - 3000
  reverseproxy:
    image: nginx:latest
    volumes:
      - ./nginx/certs:/etc/nginx/certs
      - ./nginx/nginx.conf:/etc/nginx/nginx.conf  
    ports:
      - 4333:4333

Related issues

• https://discuss.elastic.co/t/configuring-aerospike-metricbeat-module-with-tls/294729

Use cases

Monitor TLS and Username/Password protected Aerospike Databases

Screenshots

Logs

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @herrBez? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

[herrBez]

Not sure what does the error mean, any guidance is much appreciated

[fearful-symmetry]

The imports starting with github.com/elastic/ need to appear before other 3rd party imports, run goimports -w -local github.com/elastic metricbeat/module/aerospike

[herrBez]

Thank you. Let's see if it's fixed now :)

[elasticmachine]

💚 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

• Duration: 55 min 51 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 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]

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

[fearful-symmetry]

@herrBez so, I don't have much experience with Aerospike---is the idea there that basic auth would be used with the access controls in enterprise, or with a reverse proxy like nginx? If it's the latter (or both) it might be good to comment that and provide an example.

[herrBez]

Hi,
Yes there are two flavors of Aerospike Community Edition (CE) and Enterprise Edition (EE). The first lacks TLS and Basic Auth support, while the latter support both features. With Aerospike CE we can use a Reverse Proxy like NGINX to add these missing features. I will try to provide these examples As soon as I can in the form of Docker-Compose file.

[leehinman]

would you mind switching to Equalf and providing a description of what didn't match?

Right now if this fails in CI we will get an error like "A" does not equal "B". It is a lot easier to debug if we get something like Aerospike policy username is wrong. got: "A" expected: "B"

[herrBez]

Hi,
Thank you for the feedback. I don't mind at all ^^. So something like:

assert.Equalf(t, test.expectedClientPolicy.User, result.User, "Aerospike policy [Test '%s'] username is wrong: expected '%s' got '%s'", test.Name, test.expectedClientPolicy.User, result.User)

Or something like:

assert.Equalf(t, test.expectedClientPolicy.User, result.User, "Aerospike policy [Test '%s'] username is wrong", test.Name)

??

Should I also modify the existing tests to use Equalf?

[leehinman]

I don't think you need the test name, when it errors out go test will give us that. I prefer the ones that have expected and got (or something like that).

One thing I think that helps is to fail the test and just double check that the error message gives you enough context that you have some idea of where the problem is. For example telling me that the Username is what didn't match, the value we got, and the expected value goes a long way to helping understand where the problem might be.

I think you can just focus on the tests you are adding. That helps limit the scope of the PR.

[lalit-satapathy]

@lalit-satapathy could we have someone for a review here?

CC: @gpop63 @shmsr

[shmsr]

Are we adding the tests that @gpop63 suggested? If yes, then I'll wait for the changes and then approve. If no, then I'll approve this PR right away; the changes look good.

cc: @gpop63 @herrBez

[gpop63]

LGTM, make sure to fix the docs and x-pack/metricbeat CI. make update should probably fix them.

[herrBez]

Thank you all for the help and the patience! After the merge I noticed we did not add an entry in the release notes. Should we add one?

[shmsr]

Thank you all for the help and the patience! After the merge I noticed we did not add an entry in the release notes. Should we add one?

Yes, we should! Thanks.

[herrBez]

Hi, how should we proceed? By opening a new PR? I tried committing in my branch but this PR did not get the update.

(BTW the release notes seems to contain duplicated sections e.g line 243 and 245 . Is it expcted?)

[shmsr]

Hi, how should we proceed? By opening a new PR? I tried committing in my branch but this PR did not get the update.

(BTW the release notes seems to contain duplicated sections e.g line 243 and 245 . Is it expcted?)

No. Duplicates are not expected. Also, yes you can create a new PR to include the changelog entry. Please do it ASAP and tomorrow we have code freeze for 8.14 release.

[herrBez]

Done in #38923 .