From a6a0880a22e2b135d8a20a46b9ba34c7a9cf2f10 Mon Sep 17 00:00:00 2001
From: Ying Li <ying.li@docker.com>
Date: Thu, 15 Dec 2016 18:36:37 -0800
Subject: [PATCH] Before asking a user for the unlock key when they run `docker
 swarm unlock`, actually check to see if the node is part of a swarm, and if
 so, if it is unlocked first. If neither of these are true, abort the command.

Signed-off-by: Ying Li <ying.li@docker.com>
---
 cli/command/swarm/unlock.go              | 17 +++++++++++++++++
 integration-cli/daemon/daemon_swarm.go   | 13 +++++++++++++
 integration-cli/docker_api_swarm_test.go |  8 ++++++++
 integration-cli/docker_cli_swarm_test.go | 12 ++++++------
 4 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/cli/command/swarm/unlock.go b/cli/command/swarm/unlock.go
index 048fb56e3d..abb9e89fe7 100644
--- a/cli/command/swarm/unlock.go
+++ b/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
diff --git a/integration-cli/daemon/daemon_swarm.go b/integration-cli/daemon/daemon_swarm.go
index 2b7f71908b..4fc2298f15 100644
--- a/integration-cli/daemon/daemon_swarm.go
+++ b/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)
 
diff --git a/integration-cli/docker_api_swarm_test.go b/integration-cli/docker_api_swarm_test.go
index 497da43415..5680d19005 100644
--- a/integration-cli/docker_api_swarm_test.go
+++ b/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")
+}
diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go
index b5cd0b99e8..a67af79998 100644
--- a/integration-cli/docker_cli_swarm_test.go
+++ b/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) {