Add TLS support #153
Merged
Add TLS support #153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is based on the changes in `gomemcache` PR grobie/gomemcache#2. The original non-TLS behaviour is unchanged, however when `--tls.enable` is given, the net connection is created by the `crypto/tls` module instead of the `net` module. The PR follows a similar setup to the TLS code in `amtool` and left cert/key/ca/servername/insecure-skip-verify configurable. The `ServerName` defaults to the provided address which seems a sensible default. During testing, verification of the server certificates was expecting an IP SAN even when a hostname is provided as the connection address, hence the default. Signed-off-by: Nick Rhodes <nrhodes91@gmail.com>
Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
|
+1. If @grobie could help review that patch then it would be really appreciated. |
|
I'd forgotten about this; thanks for following up. |
|
Thanks for bringing this up to my attention @matthiasr. I'm very short on time these days, can take over the rest of the review process here? Thanks. |
|
@grobie Can you please transfer the memcached library to the Prometheus org? |
|
I think that's a good idea - at least for now. It seems the OG version has woken up recently (but TLS support is still pending, so maybe eventually we can re-merge the two. |
matthiasr
reviewed
Feb 13, 2023
Signed-off-by: Ben Ye <benye@amazon.com>
matthiasr
added a commit
that referenced
this pull request
Feb 13, 2023
Follow-up to #153. Signed-off-by: Matthias Rampke <matthias@prometheus.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR continues the work of @nrhodes91 in #125.
Addressed the review comments in the previous PR and also update the gomemcache package with the required TLS config.