From 4cacd1304ae7b3ec0170cfcc10e84f69b300c6d7 Mon Sep 17 00:00:00 2001 From: Drew Erny Date: Thu, 28 Mar 2019 14:06:06 -0500 Subject: [PATCH] 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 --- cli/command/service/create.go | 4 +- cli/command/service/create_test.go | 271 ++++++++++++++++++++++++ cli/command/service/parse.go | 9 +- cli/command/service/update.go | 212 +++++++++++-------- cli/command/service/update_test.go | 323 +++++++++++++++++++++++++++++ 5 files changed, 724 insertions(+), 95 deletions(-) create mode 100644 cli/command/service/create_test.go diff --git a/cli/command/service/create.go b/cli/command/service/create.go index aff8d46eb0..ae359bd687 100644 --- a/cli/command/service/create.go +++ b/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 diff --git a/cli/command/service/create_test.go b/cli/command/service/create_test.go new file mode 100644 index 0000000000..cd675c7995 --- /dev/null +++ b/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, "") +} diff --git a/cli/command/service/parse.go b/cli/command/service/parse.go index b3d35f551a..25677d3841 100644 --- a/cli/command/service/parse.go +++ b/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 } diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 7371a28bc1..c95dcf50d3 100644 --- a/cli/command/service/update.go +++ b/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) { - 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 - } + updateCredSpecConfig(flags, spec.TaskTemplate.ContainerSpec) // 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) { - newConfigs := []*swarm.ConfigReference{} +func getUpdatedConfigs(apiClient client.ConfigAPIClient, flags *pflag.FlagSet, spec *swarm.ContainerSpec) ([]*swarm.ConfigReference, error) { + 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, - // and we need to resolve its ID accordingly. - if credSpecConfig != nil { - resolveConfigs = append(resolveConfigs, credSpecConfig) + // if credSpecConfigNameis non-empty at this point, it means its a new + // config, and we need to resolve its ID accordingly. + if credSpecConfigName != "" { + 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 + } +} diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index bd35750217..6eeea55a1f 100644 --- a/cli/command/service/update_test.go +++ b/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) + }) + } +}