-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: support specifying cipher suites in tls connection #3019 #3026
Conversation
ae6cbd1
to
986a4ce
Compare
@yurishkuro Let me know what do you think? |
pkg/config/tlscfg/options.go
Outdated
@@ -18,6 +18,7 @@ import ( | |||
"crypto/tls" | |||
"crypto/x509" | |||
"fmt" | |||
"github.com/coreos/etcd/pkg/tlsutil" |
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 we want an external dependency for such minor functionality. Also, I am not seeing this package in https://github.com/etcd-io/etcd/tree/main/pkg
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.
@yurishkuro removed this external dependency and made it a sample. Hope this makes sense.
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.
as it was mentioned in this document https://docs.fluentd.org/configuration/transport-section#tls-setting do you want to provide some default cipher in the repo itself?
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.
and one more doubt why naming ciphers if want to pass just string ? are we expecting multiple ciphers ?
986a4ce
to
e47eeab
Compare
…#3019 Signed-off-by: Rajdeep Kaur <[email protected]>
e47eeab
to
4a7310c
Compare
I have questions on the original ticket. |
What's the status of this PR? |
@jpkrohling there were questions on the original tickets have to look but haven’t got the chance can take a look in the evening though |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You may re-open it if you need more time. |
Already addressed by #3564 |
Which problem is this PR solving?
Short description of the changes