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

Correct security configuration for peers in sample config file. #5913

Merged
merged 1 commit into from
Jul 12, 2016
Merged

Correct security configuration for peers in sample config file. #5913

merged 1 commit into from
Jul 12, 2016

Conversation

rboyer
Copy link
Contributor

@rboyer rboyer commented Jul 11, 2016

Using a leading "peer-" prefix on the config file entries does not work. Both security configuration yaml blocks take the same options (which differs from the cmdline args).

@heyitsanthony
Copy link
Contributor

@rboyer I don't think this is the right fix; peer-* is for TLS among peers, no prefix is for TLS with clients. What error are you seeing?

@rboyer
Copy link
Contributor Author

rboyer commented Jul 11, 2016

The config file splits out the tls configuration into two separate sections
(one for peer, one for client).

Within each yaml hash the fields are parsed into the same go struct.

The command line argument and environment variable code paths aren't
structured, so the names are namespaces as you describe, but the config
file parsing is different.
On Jul 11, 2016 11:35 AM, "Anthony Romano" notifications@github.com wrote:

@rboyer https://github.com/rboyer I don't think this is the right fix;
peer-* is for TLS among peers, no prefix is for TLS with clients. What
error are you seeing?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5913 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAATJ1xh1n6c_riHb_SILs7bwwdcs2-8ks5qUnDegaJpZM4JJcdm
.

@heyitsanthony
Copy link
Contributor

@rboyer ok, I see, you're right. Could you git commit --amend this patch so it conforms to the title style so this passes CI and can be merged? (maybe *: correct security configuration for peers in sample config file). Thanks!

@xiang90
Copy link
Contributor

xiang90 commented Jul 12, 2016

lgtm. @gyuho probably we need test to start etcd from configuration file.

@gyuho
Copy link
Contributor

gyuho commented Jul 12, 2016

Ok will add more tests.

@xiang90 xiang90 merged commit cc26f2c into etcd-io:master Jul 12, 2016
@rboyer rboyer deleted the correct-sample-peer-config-file branch July 12, 2016 02:52
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.

4 participants