-
Notifications
You must be signed in to change notification settings - Fork 812
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
Add tls config to consul #6374
Add tls config to consul #6374
Conversation
260cefd
to
3f6d656
Compare
thanks |
3f6d656
to
33c19b7
Compare
|
||
# Override the expected name on the server certificate. | ||
# CLI flag: -<prefix>.consul.tls-server-name | ||
[tls_server_name: <string> | default = ""] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this flag is used. Could we update the implementation so that it is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch :D
}) | ||
} | ||
|
||
if cfg.EnableTLS { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the pr. Thanks!
33c19b7
to
36f54f3
Compare
Signed-off-by: SungJin1212 <[email protected]>
36f54f3
to
9db957d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This PR adds a tls config to consul like other kv stores (etcd, memberlist).
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]