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

Add config validation for Citadel deployment. #13383

Closed
JimmyCYJ opened this issue Apr 16, 2019 · 5 comments

Comments

5 participants
@JimmyCYJ
Copy link
Contributor

commented Apr 16, 2019

Describe the feature request
This is motivated by #13354, where there are two Citadels running in one cluster, and those two Citadels are not properly configured which causes a race condition. One Citadel keeps overwriting workload certificates generated by the other, and vise versa.

To avoid such kind of issue, we could enhance our config validation at Galley to guarantee that
(1) Only one Citadel will be deployed in a cluster; and
(2) If multiple Citadels are deployed, command flags --read-signing-cert-only and --server-only are properly set.

It is also recommended to deploy one Citadel per cluster, as Citadel is not running in a critical path. Unless there are special needs, one Citadel is sufficient to provision certificates for all workloads.

@lei-tang

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

Quote @jcetkov: "it might be appropriate to modify the helm charts not to allow setting the security replica count via variable, if such case is really not supported and results in hard to trace issues."

@lei-tang

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

@lei-tang

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2019

Regarding to "enhance our config validation at Galley to guarantee that (1) Only one Citadel will be deployed in a cluster; and (2) If multiple Citadels are deployed, command flags --read-signing-cert-only and --server-only are properly set.", currently Galley only validates CRD and doesn't do validation at the deployment level.

@geeknoid

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

Given #13519, can we close this issue or is there more work we want to do?

@lei-tang

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

Closing this issue sounds good to me.

@JimmyCYJ JimmyCYJ closed this Apr 24, 2019

@waynz0r waynz0r referenced this issue Apr 30, 2019

Merged

Allow only one replica of Citadel #184

1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.