Merge pull request moby#29802 from thaJeztah/fix-secret-rm-consistency

Update order of '--secret-rm' and '--secret-add'

command/service/parse.go

@@ -12,7 +12,7 @@ import (
 
 // parseSecrets retrieves the secrets from the requested names and converts
 // them to secret references to use with the spec
-func parseSecrets(client client.APIClient, requestedSecrets []*types.SecretRequestOption) ([]*swarmtypes.SecretReference, error) {
+func parseSecrets(client client.SecretAPIClient, requestedSecrets []*types.SecretRequestOption) ([]*swarmtypes.SecretReference, error) {
 	secretRefs := make(map[string]*swarmtypes.SecretReference)
 	ctx := context.Background()
 

command/service/update.go

@@ -6,8 +6,6 @@ import (
 	"strings"
 	"time"
 
-	"golang.org/x/net/context"
-
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
@@ -21,6 +19,7 @@ import (
 	shlex "github.com/flynn-archive/go-shlex"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"golang.org/x/net/context"
 )
 
 func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
@@ -431,22 +430,24 @@ func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
 	*field = removeItems(*field, toRemove, envKey)
 }
 
-func getUpdatedSecrets(apiClient client.APIClient, flags *pflag.FlagSet, secrets []*swarm.SecretReference) ([]*swarm.SecretReference, error) {
+func getUpdatedSecrets(apiClient client.SecretAPIClient, flags *pflag.FlagSet, secrets []*swarm.SecretReference) ([]*swarm.SecretReference, error) {
+	newSecrets := []*swarm.SecretReference{}
+
+	toRemove := buildToRemoveSet(flags, flagSecretRemove)
+	for _, secret := range secrets {
+		if _, exists := toRemove[secret.SecretName]; !exists {
+			newSecrets = append(newSecrets, secret)
+		}
+	}
+
 	if flags.Changed(flagSecretAdd) {
 		values := flags.Lookup(flagSecretAdd).Value.(*opts.SecretOpt).Value()
 
 		addSecrets, err := parseSecrets(apiClient, values)
 		if err != nil {
 			return nil, err
 		}
-		secrets = append(secrets, addSecrets...)
-	}
-	toRemove := buildToRemoveSet(flags, flagSecretRemove)
-	newSecrets := []*swarm.SecretReference{}
-	for _, secret := range secrets {
-		if _, exists := toRemove[secret.SecretName]; !exists {
-			newSecrets = append(newSecrets, secret)
-		}
+		newSecrets = append(newSecrets, addSecrets...)
 	}
 
 	return newSecrets, nil

command/service/update_test.go

@@ -6,10 +6,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/testutil/assert"
+	"golang.org/x/net/context"
 )
 
 func TestUpdateServiceArgs(t *testing.T) {
@@ -382,3 +384,58 @@ func TestValidatePort(t *testing.T) {
 		assert.Error(t, err, e)
 	}
 }
+
+type secretAPIClientMock struct {
+	listResult []swarm.Secret
+}
+
+func (s secretAPIClientMock) SecretList(ctx context.Context, options types.SecretListOptions) ([]swarm.Secret, error) {
+	return s.listResult, nil
+}
+func (s secretAPIClientMock) SecretCreate(ctx context.Context, secret swarm.SecretSpec) (types.SecretCreateResponse, error) {
+	return types.SecretCreateResponse{}, nil
+}
+func (s secretAPIClientMock) SecretRemove(ctx context.Context, id string) error {
+	return nil
+}
+func (s secretAPIClientMock) SecretInspectWithRaw(ctx context.Context, name string) (swarm.Secret, []byte, error) {
+	return swarm.Secret{}, []byte{}, nil
+}
+
+// TestUpdateSecretUpdateInPlace tests the ability to update the "target" of an secret with "docker service update"
+// by combining "--secret-rm" and "--secret-add" for the same secret.
+func TestUpdateSecretUpdateInPlace(t *testing.T) {
+	apiClient := secretAPIClientMock{
+		listResult: []swarm.Secret{
+			{
+				ID:   "tn9qiblgnuuut11eufquw5dev",
+				Spec: swarm.SecretSpec{Annotations: swarm.Annotations{Name: "foo"}},
+			},
+		},
+	}
+
+	flags := newUpdateCommand(nil).Flags()
+	flags.Set("secret-add", "source=foo,target=foo2")
+	flags.Set("secret-rm", "foo")
+
+	secrets := []*swarm.SecretReference{
+		{
+			File: &swarm.SecretReferenceFileTarget{
+				Name: "foo",
+				UID:  "0",
+				GID:  "0",
+				Mode: 292,
+			},
+			SecretID:   "tn9qiblgnuuut11eufquw5dev",
+			SecretName: "foo",
+		},
+	}
+
+	updatedSecrets, err := getUpdatedSecrets(apiClient, flags, secrets)
+
+	assert.Equal(t, err, nil)
+	assert.Equal(t, len(updatedSecrets), 1)
+	assert.Equal(t, updatedSecrets[0].SecretID, "tn9qiblgnuuut11eufquw5dev")
+	assert.Equal(t, updatedSecrets[0].SecretName, "foo")
+	assert.Equal(t, updatedSecrets[0].File.Name, "foo2")
+}