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

Document defining ciphers in example etcd config file #15576

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

jmhbnz
Copy link
Member

@jmhbnz jmhbnz commented Mar 27, 2023

This pull request proposes to update our etcd.conf.yml with an example for setting cipher-suites.

This topic has come up in discussions and issues and it appears there has been some confusion about how it should be done, so let's provide a concrete example.

Relates to:

@serathius
Copy link
Member

This is great suggestion for users of config files. Have we seen similar problem for command line flags? or the help message is enough for users.

@jmhbnz
Copy link
Member Author

jmhbnz commented Mar 28, 2023

This is great suggestion for users of config files. Have we seen similar problem for command line flags? or the help message is enough for users.

There has been a couple references in discussions and issues for cli config but I think for command line we are better off as we have nice examples in the docs to show how it needs to be done https://etcd.io/docs/v3.6/op-guide/security/#example-2-client-to-server-authentication-with-https-client-certificates

@ahrtr
Copy link
Member

ahrtr commented Mar 28, 2023

Thanks @jmhbnz for the PR, which looks good to me.

The workflow failures are not caused by this PR. Recently the workflow are more flaky. Can we focus on fixing the flaky test, at least can we spend more time on them? thx cc @ptabor @serathius @chaochn47 @fuweid @jmhbnz

FYI. I am still focusing on the bboltDB critical issue etcd-io/bbolt#402. I may have got some clues

@chaochn47
Copy link
Member

chaochn47 commented Mar 29, 2023

The PR itself looks good to me. I suggest the failed tests can be retried. For example, an empty line in the end of etcd.conf.yml.sample

The failed test is tracked in #15581 and proposed fix PR will be sent.

Signed-off-by: James Blair <mail@jamesblair.net>
@ahrtr ahrtr merged commit b4c2f9b into etcd-io:main Mar 29, 2023
@fuweid
Copy link
Member

fuweid commented Mar 29, 2023

can we spend more time on them?

Sure. I will focus on deflaking cases recently.

@chaochn47
Copy link
Member

chaochn47 commented Apr 5, 2023

The workflow failures are not caused by this PR.

@ahrtr

No, I think the workflow failures help identify breaking changes. Please see the explanation in #15581 (comment).

And thanks to it, the PR was modified to remove the breaking change then all the tests passed eventually.

@jmhbnz jmhbnz deleted the document-cipher-config branch July 16, 2023 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants