Skip to content

Conversation

SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Nov 26, 2024

This PR adds a tls config to consul like other kv stores (etcd, memberlist).

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 26, 2024
@alanprot
Copy link
Member

thanks

# Override the expected name on the server certificate.
# CLI flag: -<prefix>.consul.tls-server-name
[tls_server_name: <string> | default = ""]
Copy link
Member

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?

Copy link
Member Author

@SungJin1212 SungJin1212 Nov 28, 2024

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 {
Copy link
Member

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?

Copy link
Member Author

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!

Signed-off-by: SungJin1212 <[email protected]>
Copy link
Member

@CharlieTLe CharlieTLe left a comment

Choose a reason for hiding this comment

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

Thanks!

@yeya24 yeya24 merged commit 784578a into cortexproject:master Nov 30, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend/consul lgtm This PR has been approved by a maintainer size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants