Add CredentialSpec tests

Adds tests for setting and updating swarm service CredentialSpecs,
especially when using a Config as a credential spec.

Signed-off-by: Drew Erny <drew.erny@docker.com>

cli/command/service/create.go

@@ -97,7 +97,9 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, opts *serviceOptions
 		service.TaskTemplate.ContainerSpec.Secrets = secrets
 	}
 
-	setConfigs(apiClient, &service, opts)
+	if err := setConfigs(apiClient, &service, opts); err != nil {
+		return err
+	}
 
 	if err := resolveServiceImageDigestContentTrust(dockerCli, &service); err != nil {
 		return err

cli/command/service/create_test.go

@@ -0,0 +1,271 @@
+package service
+
+import (
+	"context"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/swarm"
+	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+
+	cliopts "github.com/docker/cli/opts"
+)
+
+// fakeConfigAPIClientList is used to let us pass a closure as a
+// ConfigAPIClient, to use as ConfigList. for all the other methods in the
+// interface, it does nothing, not even return an error, so don't use them
+type fakeConfigAPIClientList func(context.Context, types.ConfigListOptions) ([]swarm.Config, error)
+
+func (f fakeConfigAPIClientList) ConfigList(ctx context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+	return f(ctx, opts)
+}
+
+func (f fakeConfigAPIClientList) ConfigCreate(_ context.Context, _ swarm.ConfigSpec) (types.ConfigCreateResponse, error) {
+	return types.ConfigCreateResponse{}, nil
+}
+
+func (f fakeConfigAPIClientList) ConfigRemove(_ context.Context, _ string) error {
+	return nil
+}
+
+func (f fakeConfigAPIClientList) ConfigInspectWithRaw(_ context.Context, _ string) (swarm.Config, []byte, error) {
+	return swarm.Config{}, nil, nil
+}
+
+func (f fakeConfigAPIClientList) ConfigUpdate(_ context.Context, _ string, _ swarm.Version, _ swarm.ConfigSpec) error {
+	return nil
+}
+
+// TestSetConfigsWithCredSpecAndConfigs tests that the setConfigs function for
+// create correctly looks up the right configs, and correctly handles the
+// credentialSpec
+func TestSetConfigsWithCredSpecAndConfigs(t *testing.T) {
+	// we can't directly access the internal fields of the ConfigOpt struct, so
+	// we need to let it do the parsing
+	configOpt := &cliopts.ConfigOpt{}
+	configOpt.Set("bar")
+	opts := &serviceOptions{
+		credentialSpec: credentialSpecOpt{
+			value: &swarm.CredentialSpec{
+				Config: "foo",
+			},
+			source: "config://foo",
+		},
+		configs: *configOpt,
+	}
+
+	// create a service spec. we need to be sure to fill in the nullable
+	// fields, like the code expects
+	service := &swarm.ServiceSpec{
+		TaskTemplate: swarm.TaskSpec{
+			ContainerSpec: &swarm.ContainerSpec{
+				Privileges: &swarm.Privileges{
+					CredentialSpec: opts.credentialSpec.value,
+				},
+			},
+		},
+	}
+
+	// set up a function to use as the list function
+	var fakeClient fakeConfigAPIClientList = func(_ context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+		f := opts.Filters
+
+		// we're expecting the filter to have names "foo" and "bar"
+		names := f.Get("name")
+		assert.Equal(t, len(names), 2)
+		assert.Assert(t, is.Contains(names, "foo"))
+		assert.Assert(t, is.Contains(names, "bar"))
+
+		return []swarm.Config{
+			{
+				ID: "fooID",
+				Spec: swarm.ConfigSpec{
+					Annotations: swarm.Annotations{
+						Name: "foo",
+					},
+				},
+			}, {
+				ID: "barID",
+				Spec: swarm.ConfigSpec{
+					Annotations: swarm.Annotations{
+						Name: "bar",
+					},
+				},
+			},
+		}, nil
+	}
+
+	// now call setConfigs
+	err := setConfigs(fakeClient, service, opts)
+	// verify no error is returned
+	assert.NilError(t, err)
+
+	credSpecConfigValue := service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config
+	assert.Equal(t, credSpecConfigValue, "fooID")
+
+	configRefs := service.TaskTemplate.ContainerSpec.Configs
+	assert.Assert(t, is.Contains(configRefs, &swarm.ConfigReference{
+		ConfigID:   "fooID",
+		ConfigName: "foo",
+		Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+	}), "expected configRefs to contain foo config")
+	assert.Assert(t, is.Contains(configRefs, &swarm.ConfigReference{
+		ConfigID:   "barID",
+		ConfigName: "bar",
+		File: &swarm.ConfigReferenceFileTarget{
+			Name: "bar",
+			// these are the default field values
+			UID:  "0",
+			GID:  "0",
+			Mode: 0444,
+		},
+	}), "expected configRefs to contain bar config")
+}
+
+// TestSetConfigsOnlyCredSpec tests that even if a CredentialSpec is the only
+// config needed, setConfigs still works
+func TestSetConfigsOnlyCredSpec(t *testing.T) {
+	opts := &serviceOptions{
+		credentialSpec: credentialSpecOpt{
+			value: &swarm.CredentialSpec{
+				Config: "foo",
+			},
+			source: "config://foo",
+		},
+	}
+
+	service := &swarm.ServiceSpec{
+		TaskTemplate: swarm.TaskSpec{
+			ContainerSpec: &swarm.ContainerSpec{
+				Privileges: &swarm.Privileges{
+					CredentialSpec: opts.credentialSpec.value,
+				},
+			},
+		},
+	}
+
+	// set up a function to use as the list function
+	var fakeClient fakeConfigAPIClientList = func(_ context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+		f := opts.Filters
+
+		names := f.Get("name")
+		assert.Equal(t, len(names), 1)
+		assert.Assert(t, is.Contains(names, "foo"))
+
+		return []swarm.Config{
+			{
+				ID: "fooID",
+				Spec: swarm.ConfigSpec{
+					Annotations: swarm.Annotations{
+						Name: "foo",
+					},
+				},
+			},
+		}, nil
+	}
+
+	// now call setConfigs
+	err := setConfigs(fakeClient, service, opts)
+	// verify no error is returned
+	assert.NilError(t, err)
+
+	credSpecConfigValue := service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config
+	assert.Equal(t, credSpecConfigValue, "fooID")
+
+	configRefs := service.TaskTemplate.ContainerSpec.Configs
+	assert.Assert(t, is.Contains(configRefs, &swarm.ConfigReference{
+		ConfigID:   "fooID",
+		ConfigName: "foo",
+		Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+	}))
+}
+
+// TestSetConfigsOnlyConfigs verifies setConfigs when only configs (and not a
+// CredentialSpec) is needed.
+func TestSetConfigsOnlyConfigs(t *testing.T) {
+	configOpt := &cliopts.ConfigOpt{}
+	configOpt.Set("bar")
+	opts := &serviceOptions{
+		configs: *configOpt,
+	}
+
+	service := &swarm.ServiceSpec{
+		TaskTemplate: swarm.TaskSpec{
+			ContainerSpec: &swarm.ContainerSpec{},
+		},
+	}
+
+	var fakeClient fakeConfigAPIClientList = func(_ context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+		f := opts.Filters
+
+		names := f.Get("name")
+		assert.Equal(t, len(names), 1)
+		assert.Assert(t, is.Contains(names, "bar"))
+
+		return []swarm.Config{
+			{
+				ID: "barID",
+				Spec: swarm.ConfigSpec{
+					Annotations: swarm.Annotations{
+						Name: "bar",
+					},
+				},
+			},
+		}, nil
+	}
+
+	// now call setConfigs
+	err := setConfigs(fakeClient, service, opts)
+	// verify no error is returned
+	assert.NilError(t, err)
+
+	configRefs := service.TaskTemplate.ContainerSpec.Configs
+	assert.Assert(t, is.Contains(configRefs, &swarm.ConfigReference{
+		ConfigID:   "barID",
+		ConfigName: "bar",
+		File: &swarm.ConfigReferenceFileTarget{
+			Name: "bar",
+			// these are the default field values
+			UID:  "0",
+			GID:  "0",
+			Mode: 0444,
+		},
+	}))
+}
+
+// TestSetConfigsNoConfigs checks that setConfigs works when there are no
+// configs of any kind needed
+func TestSetConfigsNoConfigs(t *testing.T) {
+	// add a credentialSpec that isn't a config
+	opts := &serviceOptions{
+		credentialSpec: credentialSpecOpt{
+			value: &swarm.CredentialSpec{
+				File: "foo",
+			},
+			source: "file://foo",
+		},
+	}
+	service := &swarm.ServiceSpec{
+		TaskTemplate: swarm.TaskSpec{
+			ContainerSpec: &swarm.ContainerSpec{
+				Privileges: &swarm.Privileges{
+					CredentialSpec: opts.credentialSpec.value,
+				},
+			},
+		},
+	}
+
+	var fakeClient fakeConfigAPIClientList = func(_ context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+		// assert false -- we should never call this function
+		assert.Assert(t, false, "we should not be listing configs")
+		return nil, nil
+	}
+
+	err := setConfigs(fakeClient, service, opts)
+	assert.NilError(t, err)
+
+	// ensure that the value of the credentialspec has not changed
+	assert.Equal(t, service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.File, "foo")
+	assert.Equal(t, service.TaskTemplate.ContainerSpec.Privileges.CredentialSpec.Config, "")
+}

cli/command/service/parse.go

@@ -86,19 +86,24 @@ func ParseConfigs(client client.ConfigAPIClient, requestedConfigs []*swarmtypes.
 	ctx := context.Background()
 
 	for _, config := range requestedConfigs {
+		// copy the config, so we don't mutate the args
+		configRef := new(swarmtypes.ConfigReference)
+		*configRef = *config
+
 		if config.Runtime != nil {
 			// by assigning to a map based on ConfigName, if the same Config
 			// is required as a Runtime target for multiple purposes, we only
 			// include it once in the final set of configs.
 			runtimeRefs[config.ConfigName] = config
+			// continue, so we skip the logic below for handling file-type
+			// configs
+			continue
 		}
 
 		if _, exists := configRefs[config.File.Name]; exists {
 			return nil, errors.Errorf("duplicate config target for %s not allowed", config.ConfigName)
 		}
 
-		configRef := new(swarmtypes.ConfigReference)
-		*configRef = *config
 		configRefs[config.File.Name] = configRef
 	}
 

cli/command/service/update.go

@@ -194,7 +194,7 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOpti
 
 	spec.TaskTemplate.ContainerSpec.Secrets = updatedSecrets
 
-	updatedConfigs, err := getUpdatedConfigs(apiClient, flags, spec.TaskTemplate.ContainerSpec.Configs)
+	updatedConfigs, err := getUpdatedConfigs(apiClient, flags, spec.TaskTemplate.ContainerSpec)
 	if err != nil {
 		return err
 	}
@@ -204,44 +204,7 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, options *serviceOpti
 	// set the credential spec value after get the updated configs, because we
 	// might need the updated configs to set the correct value of the
 	// CredentialSpec.
-	if flags.Changed(flagCredentialSpec) {
+	updateCredSpecConfig(flags, spec.TaskTemplate.ContainerSpec)
-		containerSpec := spec.TaskTemplate.ContainerSpec
-		credSpecOpt := flags.Lookup(flagCredentialSpec)
-		// if the flag has changed, and the value is empty string, then we
-		// should remove any credential spec that might be present
-		if credSpecOpt.Value.String() == "" {
-			if containerSpec.Privileges != nil {
-				containerSpec.Privileges.CredentialSpec = nil
-			}
-			return nil
-		}
-
-		// otherwise, set the credential spec to be the parsed value
-		credSpec := credSpecOpt.Value.(*credentialSpecOpt).Value()
-
-		// if this is a Config credential spec, we we still need to replace the
-		// value of credSpec.Config with the config ID instead of Name.
-		if credSpec.Config != "" {
-			for _, config := range updatedConfigs {
-				// if the config name matches, then set the config ID. we do
-				// not need to worry about if this is a Runtime target or not.
-				// even if it is not a Runtime target, getUpdatedConfigs
-				// ensures that a Runtime target for this config exists, and
-				// the Name is unique so the ID is correct no matter the
-				// target.
-				if config.ConfigName == credSpec.Config {
-					credSpec.Config = config.ConfigID
-					break
-				}
-			}
-		}
-
-		if containerSpec.Privileges == nil {
-			containerSpec.Privileges = &swarm.Privileges{}
-		}
-
-		containerSpec.Privileges.CredentialSpec = credSpec
-	}
 
 	// only send auth if flag was set
 	sendAuth, err := flags.GetBool(flagRegistryAuth)
@@ -773,68 +736,52 @@ func getUpdatedSecrets(apiClient client.SecretAPIClient, flags *pflag.FlagSet, s
 	return newSecrets, nil
 }
 
-func getUpdatedConfigs(apiClient client.ConfigAPIClient, flags *pflag.FlagSet, configs []*swarm.ConfigReference) ([]*swarm.ConfigReference, error) {
+func getUpdatedConfigs(apiClient client.ConfigAPIClient, flags *pflag.FlagSet, spec *swarm.ContainerSpec) ([]*swarm.ConfigReference, error) {
-	newConfigs := []*swarm.ConfigReference{}
+	var (
+		// credSpecConfigName stores the name of the config specified by the
+		// credential-spec flag. if a Runtime target Config with this name is
+		// already in the containerSpec, then this value will be set to
+		// emptystring in the removeConfigs stage. otherwise, a ConfigReference
+		// will be created to pass to ParseConfigs to get the ConfigID.
+		credSpecConfigName string
+		// credSpecConfigID stores the ID of the credential spec config if that
+		// config is being carried over from the old set of references
+		credSpecConfigID string
+	)
+
+	if flags.Changed(flagCredentialSpec) {
+		credSpec := flags.Lookup(flagCredentialSpec).Value.(*credentialSpecOpt).Value()
+		credSpecConfigName = credSpec.Config
+	} else {
+		// if the credential spec flag has not changed, then check if there
+		// already is a credentialSpec. if there is one, and it's for a Config,
+		// then it's from the old object, and its value is the config ID. we
+		// need this so we don't remove the config if the credential spec is
+		// not being updated.
+		if spec.Privileges != nil && spec.Privileges.CredentialSpec != nil {
+			if config := spec.Privileges.CredentialSpec.Config; config != "" {
+				credSpecConfigID = config
+			}
+		}
+	}
+
+	newConfigs := removeConfigs(flags, spec, credSpecConfigName, credSpecConfigID)
 
 	// resolveConfigs is a slice of any new configs that need to have the ID
 	// resolved
 	resolveConfigs := []*swarm.ConfigReference{}
 
-	// credSpecConfig is used for two things, and may be a bit confusing.
-	// First, it's used to store the temporary value of the ConfigReference for
-	// a credential spec config. This signals to the loop that removes configs
-	// that a credential spec config is needed. If a Runtime target Config with
-	// this name is found in the existing configs, then that loop will nil this
-	// variable. then, before we go looking up all the new configs, if this
-	// variable is non-nil, we'll add it to resolveConfigs and fill in its ID.
-	var credSpecConfig *swarm.ConfigReference
-
-	if flags.Changed(flagCredentialSpec) {
-		credSpec := flags.Lookup(flagCredentialSpec).Value.(*credentialSpecOpt).Value()
-		if credSpec.Config != "" {
-			credSpecConfig = &swarm.ConfigReference{
-				ConfigName: credSpec.Config,
-				Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
-			}
-		}
-	}
-
-	toRemove := buildToRemoveSet(flags, flagConfigRemove)
-	for _, config := range configs {
-		// if the config is a Runtime target, make sure it's still in use right
-		// now, the only use for Runtime target is credential specs.  if, in
-		// the future, more uses are added, then this check will need to be
-		// made more intelligent.
-		if config.Runtime != nil {
-			// for now, just check if the credential spec flag has changed, and
-			// if it has, that the credSpecConfig exists and has the same name.
-			// if the credential spec flag has changed, and it's either no
-			// longer a config or its a different config, then we do not add
-			// this config to newConfigs
-			if flags.Changed(flagCredentialSpec) && credSpecConfig != nil {
-				if credSpecConfig.ConfigName == config.ConfigName {
-					newConfigs = append(newConfigs, config)
-					credSpecConfig = nil
-				}
-			}
-			// continue the loop, to skip the part where we check if the config
-			// is in toRemove.
-			continue
-		}
-
-		if _, exists := toRemove[config.ConfigName]; !exists {
-			newConfigs = append(newConfigs, config)
-		}
-	}
-
 	if flags.Changed(flagConfigAdd) {
 		resolveConfigs = append(resolveConfigs, flags.Lookup(flagConfigAdd).Value.(*opts.ConfigOpt).Value()...)
 	}
 
-	// if credSpecConfig is non-nil at this point, it means its a new config,
+	// if credSpecConfigNameis non-empty at this point, it means its a new
-	// and we need to resolve its ID accordingly.
+	// config, and we need to resolve its ID accordingly.
-	if credSpecConfig != nil {
+	if credSpecConfigName != "" {
-		resolveConfigs = append(resolveConfigs, credSpecConfig)
+		resolveConfigs = append(resolveConfigs, &swarm.ConfigReference{
+			ConfigName: credSpecConfigName,
+			Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+		})
 	}
 
 	if len(resolveConfigs) > 0 {
@@ -848,6 +795,42 @@ func getUpdatedConfigs(apiClient client.ConfigAPIClient, flags *pflag.FlagSet, c
 	return newConfigs, nil
 }
 
+// removeConfigs figures out which configs in the existing spec should be kept
+// after the update.
+func removeConfigs(flags *pflag.FlagSet, spec *swarm.ContainerSpec, credSpecName, credSpecID string) []*swarm.ConfigReference {
+	keepConfigs := []*swarm.ConfigReference{}
+
+	toRemove := buildToRemoveSet(flags, flagConfigRemove)
+	// all configs in spec.Configs should have both a Name and ID, because
+	// they come from an already-accepted spec.
+	for _, config := range spec.Configs {
+		// if the config is a Runtime target, make sure it's still in use right
+		// now, the only use for Runtime target is credential specs.  if, in
+		// the future, more uses are added, then this check will need to be
+		// made more intelligent.
+		if config.Runtime != nil {
+			// if we're carrying over a credential spec explicitly (because the
+			// user passed --credential-spec with the same config name) then we
+			// should match on credSpecName. if we're carrying over a
+			// credential spec implicitly (because the user did not pass any
+			// --credential-spec flag) then we should match on credSpecID. in
+			// either case, we're keeping the config that already exists.
+			if config.ConfigName == credSpecName || config.ConfigID == credSpecID {
+				keepConfigs = append(keepConfigs, config)
+			}
+			// continue the loop, to skip the part where we check if the config
+			// is in toRemove.
+			continue
+		}
+
+		if _, exists := toRemove[config.ConfigName]; !exists {
+			keepConfigs = append(keepConfigs, config)
+		}
+	}
+
+	return keepConfigs
+}
+
 func envKey(value string) string {
 	kv := strings.SplitN(value, "=", 2)
 	return kv[0]
@@ -1314,3 +1297,48 @@ func updateNetworks(ctx context.Context, apiClient client.NetworkAPIClient, flag
 	spec.TaskTemplate.Networks = newNetworks
 	return nil
 }
+
+// updateCredSpecConfig updates the value of the credential spec Config field
+// to the config ID if the credential spec has changed. it mutates the passed
+// spec. it does not handle the case where the credential spec specifies a
+// config that does not exist -- that case is handled as part of
+// getUpdatedConfigs
+func updateCredSpecConfig(flags *pflag.FlagSet, containerSpec *swarm.ContainerSpec) {
+	if flags.Changed(flagCredentialSpec) {
+		credSpecOpt := flags.Lookup(flagCredentialSpec)
+		// if the flag has changed, and the value is empty string, then we
+		// should remove any credential spec that might be present
+		if credSpecOpt.Value.String() == "" {
+			if containerSpec.Privileges != nil {
+				containerSpec.Privileges.CredentialSpec = nil
+			}
+			return
+		}
+
+		// otherwise, set the credential spec to be the parsed value
+		credSpec := credSpecOpt.Value.(*credentialSpecOpt).Value()
+
+		// if this is a Config credential spec, we we still need to replace the
+		// value of credSpec.Config with the config ID instead of Name.
+		if credSpec.Config != "" {
+			for _, config := range containerSpec.Configs {
+				// if the config name matches, then set the config ID. we do
+				// not need to worry about if this is a Runtime target or not.
+				// even if it is not a Runtime target, getUpdatedConfigs
+				// ensures that a Runtime target for this config exists, and
+				// the Name is unique so the ID is correct no matter the
+				// target.
+				if config.ConfigName == credSpec.Config {
+					credSpec.Config = config.ConfigID
+					break
+				}
+			}
+		}
+
+		if containerSpec.Privileges == nil {
+			containerSpec.Privileges = &swarm.Privileges{}
+		}
+
+		containerSpec.Privileges.CredentialSpec = credSpec
+	}
+}

cli/command/service/update_test.go

@@ -925,3 +925,326 @@ func TestUpdateSysCtls(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateGetUpdatedConfigs(t *testing.T) {
+	// cannedConfigs is a set of configs that we'll use over and over in the
+	// tests. it's a map of Name to Config
+	cannedConfigs := map[string]*swarm.Config{
+		"bar": {
+			ID: "barID",
+			Spec: swarm.ConfigSpec{
+				Annotations: swarm.Annotations{
+					Name: "bar",
+				},
+			},
+		},
+		"cred": {
+			ID: "credID",
+			Spec: swarm.ConfigSpec{
+				Annotations: swarm.Annotations{
+					Name: "cred",
+				},
+			},
+		},
+		"newCred": {
+			ID: "newCredID",
+			Spec: swarm.ConfigSpec{
+				Annotations: swarm.Annotations{
+					Name: "newCred",
+				},
+			},
+		},
+	}
+	// cannedConfigRefs is the same thing, but with config references instead
+	// instead of ID, however, it just maps an arbitrary string value. this is
+	// so we could have multiple config refs using the same config
+	cannedConfigRefs := map[string]*swarm.ConfigReference{
+		"fooRef": {
+			ConfigID:   "fooID",
+			ConfigName: "foo",
+			File: &swarm.ConfigReferenceFileTarget{
+				Name: "foo",
+				UID:  "0",
+				GID:  "0",
+				Mode: 0444,
+			},
+		},
+		"barRef": {
+			ConfigID:   "barID",
+			ConfigName: "bar",
+			File: &swarm.ConfigReferenceFileTarget{
+				Name: "bar",
+				UID:  "0",
+				GID:  "0",
+				Mode: 0444,
+			},
+		},
+		"bazRef": {
+			ConfigID:   "bazID",
+			ConfigName: "baz",
+			File: &swarm.ConfigReferenceFileTarget{
+				Name: "baz",
+				UID:  "0",
+				GID:  "0",
+				Mode: 0444,
+			},
+		},
+		"credRef": {
+			ConfigID:   "credID",
+			ConfigName: "cred",
+			Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+		},
+		"newCredRef": {
+			ConfigID:   "newCredID",
+			ConfigName: "newCred",
+			Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+		},
+	}
+
+	type flagVal [2]string
+	type test struct {
+		// the name of the subtest
+		name string
+		// flags are the flags we'll be setting
+		flags []flagVal
+		// oldConfigs are the configs that would already be on the service
+		// it is a slice of strings corresponding to the the key of
+		// cannedConfigRefs
+		oldConfigs []string
+		// oldCredSpec is the credentialSpec being carried over from the old
+		// object
+		oldCredSpec *swarm.CredentialSpec
+		// lookupConfigs are the configs we're expecting to be listed. it is a
+		// slice of strings corresponding to the key of cannedConfigs
+		lookupConfigs []string
+		// expected is the configs we should get as a result. it is a slice of
+		// strings corresponding to the key in cannedConfigRefs
+		expected []string
+	}
+
+	testCases := []test{
+		{
+			name:       "no configs added or removed",
+			oldConfigs: []string{"fooRef"},
+			expected:   []string{"fooRef"},
+		}, {
+			name:          "add a config",
+			flags:         []flagVal{{"config-add", "bar"}},
+			oldConfigs:    []string{"fooRef"},
+			lookupConfigs: []string{"bar"},
+			expected:      []string{"fooRef", "barRef"},
+		}, {
+			name:       "remove a config",
+			flags:      []flagVal{{"config-rm", "bar"}},
+			oldConfigs: []string{"fooRef", "barRef"},
+			expected:   []string{"fooRef"},
+		}, {
+			name:        "include an old credential spec",
+			oldConfigs:  []string{"credRef"},
+			oldCredSpec: &swarm.CredentialSpec{Config: "credID"},
+			expected:    []string{"credRef"},
+		}, {
+			name:          "add a credential spec",
+			oldConfigs:    []string{"fooRef"},
+			flags:         []flagVal{{"credential-spec", "config://cred"}},
+			lookupConfigs: []string{"cred"},
+			expected:      []string{"fooRef", "credRef"},
+		}, {
+			name:          "change a credential spec",
+			oldConfigs:    []string{"fooRef", "credRef"},
+			oldCredSpec:   &swarm.CredentialSpec{Config: "credID"},
+			flags:         []flagVal{{"credential-spec", "config://newCred"}},
+			lookupConfigs: []string{"newCred"},
+			expected:      []string{"fooRef", "newCredRef"},
+		}, {
+			name:          "credential spec no longer config",
+			oldConfigs:    []string{"fooRef", "credRef"},
+			oldCredSpec:   &swarm.CredentialSpec{Config: "credID"},
+			flags:         []flagVal{{"credential-spec", "file://someFile"}},
+			lookupConfigs: []string{},
+			expected:      []string{"fooRef"},
+		}, {
+			name:          "credential spec becomes config",
+			oldConfigs:    []string{"fooRef"},
+			oldCredSpec:   &swarm.CredentialSpec{File: "someFile"},
+			flags:         []flagVal{{"credential-spec", "config://cred"}},
+			lookupConfigs: []string{"cred"},
+			expected:      []string{"fooRef", "credRef"},
+		}, {
+			name:          "remove credential spec",
+			oldConfigs:    []string{"fooRef", "credRef"},
+			oldCredSpec:   &swarm.CredentialSpec{Config: "credID"},
+			flags:         []flagVal{{"credential-spec", ""}},
+			lookupConfigs: []string{},
+			expected:      []string{"fooRef"},
+		}, {
+			name: "just frick my stuff up",
+			// a more complicated test. add barRef, remove bazRef, keep fooRef,
+			// change credentialSpec from credRef to newCredRef
+			oldConfigs:  []string{"fooRef", "bazRef", "credRef"},
+			oldCredSpec: &swarm.CredentialSpec{Config: "cred"},
+			flags: []flagVal{
+				{"config-add", "bar"},
+				{"config-rm", "baz"},
+				{"credential-spec", "config://newCred"},
+			},
+			lookupConfigs: []string{"bar", "newCred"},
+			expected:      []string{"fooRef", "barRef", "newCredRef"},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			flags := newUpdateCommand(nil).Flags()
+			for _, f := range tc.flags {
+				flags.Set(f[0], f[1])
+			}
+
+			// fakeConfigAPIClientList is actually defined in create_test.go,
+			// but we'll use it here as well
+			var fakeClient fakeConfigAPIClientList = func(_ context.Context, opts types.ConfigListOptions) ([]swarm.Config, error) {
+				names := opts.Filters.Get("name")
+				assert.Equal(t, len(names), len(tc.lookupConfigs))
+
+				configs := []swarm.Config{}
+				for _, lookup := range tc.lookupConfigs {
+					assert.Assert(t, is.Contains(names, lookup))
+					cfg, ok := cannedConfigs[lookup]
+					assert.Assert(t, ok)
+					configs = append(configs, *cfg)
+				}
+				return configs, nil
+			}
+
+			// build the actual set of old configs and the container spec
+			oldConfigs := []*swarm.ConfigReference{}
+			for _, config := range tc.oldConfigs {
+				cfg, ok := cannedConfigRefs[config]
+				assert.Assert(t, ok)
+				oldConfigs = append(oldConfigs, cfg)
+			}
+
+			containerSpec := &swarm.ContainerSpec{
+				Configs: oldConfigs,
+				Privileges: &swarm.Privileges{
+					CredentialSpec: tc.oldCredSpec,
+				},
+			}
+
+			finalConfigs, err := getUpdatedConfigs(fakeClient, flags, containerSpec)
+			assert.NilError(t, err)
+
+			// ensure that the finalConfigs consists of all of the expected
+			// configs
+			assert.Equal(t, len(finalConfigs), len(tc.expected),
+				"%v final configs, %v expected",
+				len(finalConfigs), len(tc.expected),
+			)
+			for _, expected := range tc.expected {
+				assert.Assert(t, is.Contains(finalConfigs, cannedConfigRefs[expected]))
+			}
+		})
+	}
+}
+
+func TestUpdateCredSpec(t *testing.T) {
+	type testCase struct {
+		// name is the name of the subtest
+		name string
+		// flagVal is the value we're setting flagCredentialSpec to
+		flagVal string
+		// spec is the existing serviceSpec with its configs
+		spec *swarm.ContainerSpec
+		// expected is the expected value of the credential spec after the
+		// function. it may be nil
+		expected *swarm.CredentialSpec
+	}
+
+	testCases := []testCase{
+		{
+			name:     "add file credential spec",
+			flagVal:  "file://somefile",
+			spec:     &swarm.ContainerSpec{},
+			expected: &swarm.CredentialSpec{File: "somefile"},
+		}, {
+			name:    "remove a file credential spec",
+			flagVal: "",
+			spec: &swarm.ContainerSpec{
+				Privileges: &swarm.Privileges{
+					CredentialSpec: &swarm.CredentialSpec{
+						File: "someFile",
+					},
+				},
+			},
+			expected: nil,
+		}, {
+			name:     "remove when no CredentialSpec exists",
+			flagVal:  "",
+			spec:     &swarm.ContainerSpec{},
+			expected: nil,
+		}, {
+			name:    "add a config credenital spec",
+			flagVal: "config://someConfigName",
+			spec: &swarm.ContainerSpec{
+				Configs: []*swarm.ConfigReference{
+					{
+						ConfigName: "someConfigName",
+						ConfigID:   "someConfigID",
+						Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+					},
+				},
+			},
+			expected: &swarm.CredentialSpec{
+				Config: "someConfigID",
+			},
+		}, {
+			name:    "remove a config credential spec",
+			flagVal: "",
+			spec: &swarm.ContainerSpec{
+				Privileges: &swarm.Privileges{
+					CredentialSpec: &swarm.CredentialSpec{
+						Config: "someConfigID",
+					},
+				},
+			},
+			expected: nil,
+		}, {
+			name:    "update a config credential spec",
+			flagVal: "config://someConfigName",
+			spec: &swarm.ContainerSpec{
+				Configs: []*swarm.ConfigReference{
+					{
+						ConfigName: "someConfigName",
+						ConfigID:   "someConfigID",
+						Runtime:    &swarm.ConfigReferenceRuntimeTarget{},
+					},
+				},
+				Privileges: &swarm.Privileges{
+					CredentialSpec: &swarm.CredentialSpec{
+						Config: "someDifferentConfigID",
+					},
+				},
+			},
+			expected: &swarm.CredentialSpec{
+				Config: "someConfigID",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			flags := newUpdateCommand(nil).Flags()
+			flags.Set(flagCredentialSpec, tc.flagVal)
+
+			updateCredSpecConfig(flags, tc.spec)
+			// handle the case where tc.spec.Privileges is nil
+			if tc.expected == nil {
+				assert.Assert(t, tc.spec.Privileges == nil || tc.spec.Privileges.CredentialSpec == nil)
+				return
+			}
+
+			assert.Assert(t, tc.spec.Privileges != nil)
+			assert.DeepEqual(t, tc.spec.Privileges.CredentialSpec, tc.expected)
+		})
+	}
+}