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

[O11y][Redis] Add AUTH (username) and SSL/TLS support #9777

Merged

Conversation

harnish-elastic
Copy link
Contributor

@harnish-elastic harnish-elastic commented May 3, 2024

Proposed commit message

Add AUTH (username) and SSL/TLS support for key, keyspace, and info data streams.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Related issues

@harnish-elastic harnish-elastic self-assigned this May 3, 2024
@harnish-elastic harnish-elastic marked this pull request as ready for review May 3, 2024 05:46
@harnish-elastic harnish-elastic requested a review from a team as a code owner May 3, 2024 05:46
@shmsr shmsr added the enhancement New feature or request label May 3, 2024
@elasticmachine
Copy link

elasticmachine commented May 3, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@shmsr shmsr changed the title [O11y][Redis] Support authentication with username and ssl [O11y][Redis] Add AUTH (username) and SSL/TLS support May 6, 2024
# xw23l/k8RoD1wRWaDVbgpjwSzt+kl+vJE/ip2w3h69eEZ9wbo6scRO5lCO2JM4Pr
# 7RhLQyWn2u00L7/9Omw=
# -----END CERTIFICATE-----
description: i.e. certificate_authorities, supported_protocols, verification_mode etc.
Copy link
Member

Choose a reason for hiding this comment

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

Can we improve the description?

Copy link
Member

Choose a reason for hiding this comment

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

Looks weird. How does it show up on Kibana? Can you please see?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will show up on kibana as below, Please let me know your thoughts on what should we show up to improve the description. FYI, all the packages currently having the same SSL descriptions!

image

Copy link
Member

Choose a reason for hiding this comment

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

Let's change it to something like this:

https://www.elastic.co/guide/en/beats/filebeat/current/configuration-ssl.html#:~:text=Common%20SSL%20configuration%20options%20can%20be%20used%20in%20both%20client%20and%20server%20configurations.%20You%20can%20specify%20the%20following%20options%20in%20the%20ssl%20section%20of%20each%20subsystem%20that%20supports%20SSL.

From ^^, we can probably do this?

This section allows for the configuration of SSL settings to enable secure communication between the client and server. Both common and specific SSL options can be customized to ensure a secure and reliable connection. Example: certificate_authorities, supported-protocols, verification-mode, key, certificate, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the certificate_authorities, links looks broken to me! Can you have a look and update?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this link that you are suggesting to be there for certificate_authorities?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Is this link that you are suggesting to be there for certificate_authorities?

Yep!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the screenshot of how it will show up in kibana.

image

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @harnish-elastic

Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Copy link
Contributor

@niraj-elastic niraj-elastic left a comment

Choose a reason for hiding this comment

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

LGTM

@harnish-elastic harnish-elastic merged commit 6b62e89 into elastic:main May 6, 2024
5 checks passed
@elasticmachine
Copy link

Package redis - 1.15.0 containing this change is available at https://epr.elastic.co/search?package=redis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:redis Redis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants