[ca] Separate server signing root CA from security config root CA#2336
Conversation
…ityConfig's. Signed-off-by: Ying Li <ying.li@docker.com>
external force to call `UpdateRootCA`. Signed-off-by: Ying Li <ying.li@docker.com>
cyli
force-pushed
the
ca-signer-not-security-config
branch
from
cfee09a to
9147d5f
Compare
Codecov Report
@@ Coverage Diff @@
## master #2336 +/- ##
==========================================
- Coverage 60.59% 60.55% -0.05%
==========================================
Files 128 128
Lines 26048 26056 +8
==========================================
- Hits 15783 15777 -6
- Misses 8878 8885 +7
- Partials 1387 1394 +7 |
|
ping @nishanttotla for review, if you have time. :) |
| for _, secConfig := range secConfigs { | ||
| s, err := secConfig.RootCA().Signer() | ||
| for i, server := range append(otherServers) { | ||
| s, err := server.RootCA().Signer() |
There was a problem hiding this comment.
Should this be range otherServers?
There was a problem hiding this comment.
Good point, thanks! Also, 👋 hello!
| if !bytes.Equal(s.Key, rotationKey) { | ||
| return errors.New("all the sec configs haven't been updated yet") | ||
| return errors.Errorf("all the servers' root CAs haven't been updated yet: server %d", i) | ||
| } |
There was a problem hiding this comment.
Maybe "server %d's root CA hasn't been updated yet"? Or even better, could this give the server name instead of the index?
There was a problem hiding this comment.
Oh, just realized this is a test, so it doesn't really matter.
There was a problem hiding this comment.
Have updated the message, although still just with the index, since the name might not be that useful in debugging which one failed.
… for both manager and worker nodes. Signed-off-by: Ying Li <ying.li@docker.com>
cyli
force-pushed
the
ca-signer-not-security-config
branch
from
9147d5f to
82c36fe
Compare
|
Thanks @aaronlehmann and @diogomonica! |
3 participants
This finishes separating signing functionality from the security config that was started with #2262. This way, the mechanism for updating the root certs in the security config is the same for both managers and workers (via the dispatcher), which makes the root update logic a little less complex.
This also means that the CA server is more standalone, again - it performs its own watch on the store for cluster update changes.