Merge pull request moby#29470 from cyli/ask-for-unlock-key-only-if-lo…

…cked

Check if a swarm is locked before asking a user to enter their unlock key

cli/command/swarm/unlock.go

@@ -2,6 +2,7 @@ package swarm
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -24,6 +25,22 @@ func newUnlockCommand(dockerCli *command.DockerCli) *cobra.Command {
 			client := dockerCli.Client()
 			ctx := context.Background()
 
+			// First see if the node is actually part of a swarm, and if it's is actually locked first.
+			// If it's in any other state than locked, don't ask for the key.
+			info, err := client.Info(ctx)
+			if err != nil {
+				return err
+			}
+
+			switch info.Swarm.LocalNodeState {
+			case swarm.LocalNodeStateInactive:
+				return errors.New("Error: This node is not part of a swarm")
+			case swarm.LocalNodeStateLocked:
+				break
+			default:
+				return errors.New("Error: swarm is not locked")
+			}
+
 			key, err := readKey(dockerCli.In(), "Please enter unlock key: ")
 			if err != nil {
 				return err

integration-cli/daemon/daemon_swarm.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/go-check/check"
+	"github.com/pkg/errors"
 )
 
 // Swarm is a test daemon with helpers for participating in a swarm.
@@ -96,6 +97,18 @@ func (d *Swarm) SwarmInfo() (swarm.Info, error) {
 	return info.Swarm, nil
 }
 
+// Unlock tries to unlock a locked swarm
+func (d *Swarm) Unlock(req swarm.UnlockRequest) error {
+	status, out, err := d.SockRequest("POST", "/swarm/unlock", req)
+	if status != http.StatusOK {
+		return fmt.Errorf("unlocking swarm: invalid statuscode %v, %q", status, out)
+	}
+	if err != nil {
+		err = errors.Wrap(err, "unlocking swarm")
+	}
+	return err
+}
+
 // ServiceConstructor defines a swarm service constructor function
 type ServiceConstructor func(*swarm.Service)
 

integration-cli/docker_api_swarm_test.go

@@ -1309,3 +1309,11 @@ func (s *DockerSwarmSuite) TestAPISwarmSecretsDelete(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(status, checker.Equals, http.StatusNotFound, check.Commentf("secret delete: %s", string(out)))
 }
+
+// Unlocking an unlocked swarm results in an error
+func (s *DockerSwarmSuite) TestAPISwarmUnlockNotLocked(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+	err := d.Unlock(swarm.UnlockRequest{UnlockKey: "wrong-key"})
+	c.Assert(err, checker.NotNil)
+	c.Assert(err.Error(), checker.Contains, "swarm is not locked")
+}

integration-cli/docker_cli_swarm_test.go

@@ -866,24 +866,24 @@ func checkSwarmUnlockedToLocked(c *check.C, d *daemon.Swarm) {
 func (s *DockerSwarmSuite) TestUnlockEngineAndUnlockedSwarm(c *check.C) {
 	d := s.AddDaemon(c, false, false)
 
-	// unlocking a normal engine should return an error
+	// unlocking a normal engine should return an error - it does not even ask for the key
 	cmd := d.Command("swarm", "unlock")
-	cmd.Stdin = bytes.NewBufferString("wrong-secret-key")
 	outs, err := cmd.CombinedOutput()
 
 	c.Assert(err, checker.NotNil, check.Commentf("out: %v", string(outs)))
-	c.Assert(string(outs), checker.Contains, "This node is not a swarm manager.")
+	c.Assert(string(outs), checker.Contains, "Error: This node is not part of a swarm")
+	c.Assert(string(outs), checker.Not(checker.Contains), "Please enter unlock key")
 
 	_, err = d.Cmd("swarm", "init")
 	c.Assert(err, checker.IsNil)
 
-	// unlocking an unlocked swarm should return an error
+	// unlocking an unlocked swarm should return an error - it does not even ask for the key
 	cmd = d.Command("swarm", "unlock")
-	cmd.Stdin = bytes.NewBufferString("wrong-secret-key")
 	outs, err = cmd.CombinedOutput()
 
 	c.Assert(err, checker.NotNil, check.Commentf("out: %v", string(outs)))
-	c.Assert(string(outs), checker.Contains, "swarm is not locked")
+	c.Assert(string(outs), checker.Contains, "Error: swarm is not locked")
+	c.Assert(string(outs), checker.Not(checker.Contains), "Please enter unlock key")
 }
 
 func (s *DockerSwarmSuite) TestSwarmInitLocked(c *check.C) {