diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index a6ac503d48..f6b26ab5fd 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/cloudflare/cfssl/helpers" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/integration-cli/checker" @@ -1221,10 +1222,6 @@ func (s *DockerSwarmSuite) TestSwarmJoinPromoteLocked(c *check.C) { c.Assert(getNodeStatus(c, d), checker.Equals, swarm.LocalNodeStateActive) } - // get d3's cert - d3cert, err := ioutil.ReadFile(filepath.Join(d3.Folder, "root", "swarm", "certificates", "swarm-node.crt")) - c.Assert(err, checker.IsNil) - // demote manager back to worker - workers are not locked outs, err = d1.Cmd("node", "demote", d3.Info.NodeID) c.Assert(err, checker.IsNil) @@ -1237,12 +1234,16 @@ func (s *DockerSwarmSuite) TestSwarmJoinPromoteLocked(c *check.C) { // is set to autolock) waitAndAssert(c, defaultReconciliationTimeout, d3.CheckControlAvailable, checker.False) waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - cert, err := ioutil.ReadFile(filepath.Join(d3.Folder, "root", "swarm", "certificates", "swarm-node.crt")) + certBytes, err := ioutil.ReadFile(filepath.Join(d3.Folder, "root", "swarm", "certificates", "swarm-node.crt")) if err != nil { return "", check.Commentf("error: %v", err) } - return string(cert), check.Commentf("cert: %v", string(cert)) - }, checker.Not(checker.Equals), string(d3cert)) + certs, err := helpers.ParseCertificatesPEM(certBytes) + if err == nil && len(certs) > 0 && len(certs[0].Subject.OrganizationalUnit) > 0 { + return certs[0].Subject.OrganizationalUnit[0], nil + } + return "", check.Commentf("could not get organizational unit from certificate") + }, checker.Equals, "swarm-worker") // by now, it should *never* be locked on restart d3.Restart(c) diff --git a/vendor.conf b/vendor.conf index 75e68008fb..32b3d9fb46 100644 --- a/vendor.conf +++ b/vendor.conf @@ -106,7 +106,7 @@ github.com/docker/containerd 9048e5e50717ea4497b757314bad98ea3763c145 github.com/tonistiigi/fifo 1405643975692217d6720f8b54aeee1bf2cd5cf4 # cluster -github.com/docker/swarmkit b19d028de0a6e9ca281afeb76cea2544b9edd839 +github.com/docker/swarmkit 61a92e8ec074df5769decda985df4a3ab43c77eb github.com/gogo/protobuf 8d70fb3182befc465c4a1eac8ad4d38ff49778e2 github.com/cloudflare/cfssl 7fb22c8cba7ecaf98e4082d22d65800cf45e042a github.com/google/certificate-transparency d90e65c3a07988180c5b1ece71791c0b6506826e diff --git a/vendor/github.com/docker/swarmkit/node/node.go b/vendor/github.com/docker/swarmkit/node/node.go index efa75504e9..b6ff9b8c11 100644 --- a/vendor/github.com/docker/swarmkit/node/node.go +++ b/vendor/github.com/docker/swarmkit/node/node.go @@ -133,6 +133,30 @@ type Node struct { manager *manager.Manager notifyNodeChange chan *agent.NodeChanges // used by the agent to relay node updates from the dispatcher Session stream to (*Node).run unlockKey []byte + + // lastNodeRole is the last-seen value of Node.Role, used to make role + // changes "edge triggered" and avoid renewal loops. + lastNodeRole lastSeenRole + // lastNodeDesiredRole is the last-seen value of Node.Spec.DesiredRole, + // used to make role changes "edge triggered" and avoid renewal loops. + // This exists in addition to lastNodeRole to support older CAs that + // only fill in the DesiredRole field. + lastNodeDesiredRole lastSeenRole +} + +type lastSeenRole struct { + role *api.NodeRole +} + +// observe notes the latest value of this node role, and returns true if it +// is the first seen value, or is different from the most recently seen value. +func (l *lastSeenRole) observe(newRole api.NodeRole) bool { + changed := l.role == nil || *l.role != newRole + if l.role == nil { + l.role = new(api.NodeRole) + } + *l.role = newRole + return changed } // RemoteAPIAddr returns address on which remote manager api listens. @@ -279,17 +303,35 @@ func (n *Node) run(ctx context.Context) (err error) { return case nodeChanges := <-n.notifyNodeChange: n.Lock() - currentRole := n.role + currentRole := api.NodeRoleWorker + if n.role == ca.ManagerRole { + currentRole = api.NodeRoleManager + } n.Unlock() if nodeChanges.Node != nil { - role := ca.WorkerRole - if nodeChanges.Node.Role == api.NodeRoleManager { - role = ca.ManagerRole - } - - // If the server is sending us a ForceRenewal State, or if the new node role doesn't match our current role, renew - if currentRole != role || nodeChanges.Node.Certificate.Status.State == api.IssuanceStateRotate { + // This is a bit complex to be backward compatible with older CAs that + // don't support the Node.Role field. They only use what's presently + // called DesiredRole. + // 1) If we haven't seen the node object before, and the desired role + // is different from our current role, renew the cert. This covers + // the case of starting up after a role change. + // 2) If we have seen the node before, the desired role is + // different from our current role, and either the actual role or + // desired role has changed relative to the last values we saw in + // those fields, renew the cert. This covers the case of the role + // changing while this node is running, but prevents getting into a + // rotation loop if Node.Role isn't what we expect (because it's + // unset). We may renew the certificate an extra time (first when + // DesiredRole changes, and then again when Role changes). + // 3) If the server is sending us IssuanceStateRotate, renew the cert as + // requested by the CA. + roleChanged := n.lastNodeRole.observe(nodeChanges.Node.Role) + desiredRoleChanged := n.lastNodeDesiredRole.observe(nodeChanges.Node.Spec.DesiredRole) + if (currentRole != nodeChanges.Node.Spec.DesiredRole && + ((roleChanged && currentRole != nodeChanges.Node.Role) || + desiredRoleChanged)) || + nodeChanges.Node.Certificate.Status.State == api.IssuanceStateRotate { renewCert() } } @@ -298,7 +340,7 @@ func (n *Node) run(ctx context.Context) (err error) { // We only want to update the root CA if this is a worker node. Manager nodes directly watch the raft // store and update the root CA, with the necessary signer, from the raft store (since the managers // need the CA key as well to potentially issue new TLS certificates). - if currentRole == ca.ManagerRole || bytes.Equal(nodeChanges.RootCert, securityConfig.RootCA().Certs) { + if currentRole == api.NodeRoleManager || bytes.Equal(nodeChanges.RootCert, securityConfig.RootCA().Certs) { continue } newRootCA, err := ca.NewRootCA(nodeChanges.RootCert, nil, nil, ca.DefaultNodeCertExpiration, nil)