From 7bd26ed69092f90861eca7b0c602308c61c5d570 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 6 Nov 2017 17:30:10 -0500 Subject: [PATCH 1/3] Remove duplication in loader Signed-off-by: Daniel Nephin --- cli/compose/loader/loader.go | 38 +++++++++++++++++++++--------------- cli/compose/types/types.go | 7 ++++--- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index a1a6ed0a..378e8d0e 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -112,14 +112,14 @@ func loadSections(config map[string]interface{}, configDetails types.ConfigDetai { key: "secrets", fnc: func(config map[string]interface{}) error { - cfg.Secrets, err = LoadSecrets(config, configDetails.WorkingDir) + cfg.Secrets, err = LoadSecrets(config, configDetails) return err }, }, { key: "configs", fnc: func(config map[string]interface{}) error { - cfg.Configs, err = LoadConfigObjs(config, configDetails.WorkingDir) + cfg.Configs, err = LoadConfigObjs(config, configDetails) return err }, }, @@ -484,42 +484,48 @@ func LoadVolumes(source map[string]interface{}, version string) (map[string]type // LoadSecrets produces a SecretConfig map from a compose file Dict // the source Dict is not validated if directly used. Use Load() to enable validation -func LoadSecrets(source map[string]interface{}, workingDir string) (map[string]types.SecretConfig, error) { +func LoadSecrets(source map[string]interface{}, details types.ConfigDetails) (map[string]types.SecretConfig, error) { secrets := make(map[string]types.SecretConfig) if err := transform(source, &secrets); err != nil { return secrets, err } for name, secret := range secrets { - if secret.External.External && secret.External.Name == "" { - secret.External.Name = name - secrets[name] = secret - } - if secret.File != "" { - secret.File = absPath(workingDir, secret.File) + obj, err := loadFileObjectConfig(name, types.FileObjectConfig(secret), details) + if err != nil { + return nil, err } + secrets[name] = types.SecretConfig(obj) } return secrets, nil } // LoadConfigObjs produces a ConfigObjConfig map from a compose file Dict // the source Dict is not validated if directly used. Use Load() to enable validation -func LoadConfigObjs(source map[string]interface{}, workingDir string) (map[string]types.ConfigObjConfig, error) { +func LoadConfigObjs(source map[string]interface{}, details types.ConfigDetails) (map[string]types.ConfigObjConfig, error) { configs := make(map[string]types.ConfigObjConfig) if err := transform(source, &configs); err != nil { return configs, err } for name, config := range configs { - if config.External.External && config.External.Name == "" { - config.External.Name = name - configs[name] = config - } - if config.File != "" { - config.File = absPath(workingDir, config.File) + obj, err := loadFileObjectConfig(name, types.FileObjectConfig(config), details) + if err != nil { + return nil, err } + configs[name] = types.ConfigObjConfig(obj) } return configs, nil } +func loadFileObjectConfig(name string, obj types.FileObjectConfig, details types.ConfigDetails) (types.FileObjectConfig, error) { + if obj.External.External && obj.External.Name == "" { + obj.External.Name = name + } + if obj.File != "" { + obj.File = absPath(details.WorkingDir, obj.File) + } + return obj, nil +} + func absPath(workingDir string, filePath string) string { if filepath.IsAbs(filePath) { return filePath diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go index b5afc9b2..4e68da08 100644 --- a/cli/compose/types/types.go +++ b/cli/compose/types/types.go @@ -344,14 +344,15 @@ type CredentialSpecConfig struct { Registry string } -type fileObjectConfig struct { +// FileObjectConfig is a config type for a file used by a service +type FileObjectConfig struct { File string External External Labels Labels } // SecretConfig for a secret -type SecretConfig fileObjectConfig +type SecretConfig FileObjectConfig // ConfigObjConfig is the config for the swarm "Config" object -type ConfigObjConfig fileObjectConfig +type ConfigObjConfig FileObjectConfig From a68c940f1a8688e7451c946caeeb122e749e6d61 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 6 Nov 2017 17:56:36 -0500 Subject: [PATCH 2/3] Remove duplication in compose/convert Signed-off-by: Daniel Nephin --- cli/compose/convert/compose.go | 42 ++++++---- cli/compose/convert/service.go | 147 +++++++++++++++++++-------------- cli/compose/types/types.go | 7 +- 3 files changed, 111 insertions(+), 85 deletions(-) diff --git a/cli/compose/convert/compose.go b/cli/compose/convert/compose.go index 02b1dccb..699c7b85 100644 --- a/cli/compose/convert/compose.go +++ b/cli/compose/convert/compose.go @@ -101,18 +101,11 @@ func Secrets(namespace Namespace, secrets map[string]composetypes.SecretConfig) continue } - data, err := ioutil.ReadFile(secret.File) + obj, err := fileObjectConfig(namespace, name, composetypes.FileObjectConfig(secret)) if err != nil { return nil, err } - - result = append(result, swarm.SecretSpec{ - Annotations: swarm.Annotations{ - Name: namespace.Scope(name), - Labels: AddStackLabel(namespace, secret.Labels), - }, - Data: data, - }) + result = append(result, swarm.SecretSpec{Annotations: obj.Annotations, Data: obj.Data}) } return result, nil } @@ -125,18 +118,31 @@ func Configs(namespace Namespace, configs map[string]composetypes.ConfigObjConfi continue } - data, err := ioutil.ReadFile(config.File) + obj, err := fileObjectConfig(namespace, name, composetypes.FileObjectConfig(config)) if err != nil { return nil, err } - - result = append(result, swarm.ConfigSpec{ - Annotations: swarm.Annotations{ - Name: namespace.Scope(name), - Labels: AddStackLabel(namespace, config.Labels), - }, - Data: data, - }) + result = append(result, swarm.ConfigSpec{Annotations: obj.Annotations, Data: obj.Data}) } return result, nil } + +type swarmFileObject struct { + Annotations swarm.Annotations + Data []byte +} + +func fileObjectConfig(namespace Namespace, name string, obj composetypes.FileObjectConfig) (swarmFileObject, error) { + data, err := ioutil.ReadFile(obj.File) + if err != nil { + return swarmFileObject{}, err + } + + return swarmFileObject{ + Annotations: swarm.Annotations{ + Name: namespace.Scope(name), + Labels: AddStackLabel(namespace, obj.Labels), + }, + Data: data, + }, nil +} diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go index 75c6db8f..d6924489 100644 --- a/cli/compose/convert/service.go +++ b/cli/compose/convert/service.go @@ -255,43 +255,24 @@ func convertServiceSecrets( secretSpecs map[string]composetypes.SecretConfig, ) ([]*swarm.SecretReference, error) { refs := []*swarm.SecretReference{} - for _, secret := range secrets { - target := secret.Target - if target == "" { - target = secret.Source - } - secretSpec, exists := secretSpecs[secret.Source] + lookup := func(key string) (composetypes.FileObjectConfig, error) { + configSpec, exists := secretSpecs[key] if !exists { - return nil, errors.Errorf("undefined secret %q", secret.Source) - } - - source := namespace.Scope(secret.Source) - if secretSpec.External.External { - source = secretSpec.External.Name - } - - uid := secret.UID - gid := secret.GID - if uid == "" { - uid = "0" - } - if gid == "" { - gid = "0" - } - mode := secret.Mode - if mode == nil { - mode = uint32Ptr(0444) + return composetypes.FileObjectConfig{}, errors.Errorf("undefined secret %q", key) + } + return composetypes.FileObjectConfig(configSpec), nil + } + for _, config := range secrets { + obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(config), lookup) + if err != nil { + return nil, err } + file := swarm.SecretReferenceFileTarget(obj.File) refs = append(refs, &swarm.SecretReference{ - File: &swarm.SecretReferenceFileTarget{ - Name: target, - UID: uid, - GID: gid, - Mode: os.FileMode(*mode), - }, - SecretName: source, + File: &file, + SecretName: obj.Name, }) } @@ -312,43 +293,24 @@ func convertServiceConfigObjs( configSpecs map[string]composetypes.ConfigObjConfig, ) ([]*swarm.ConfigReference, error) { refs := []*swarm.ConfigReference{} - for _, config := range configs { - target := config.Target - if target == "" { - target = config.Source - } - configSpec, exists := configSpecs[config.Source] + lookup := func(key string) (composetypes.FileObjectConfig, error) { + configSpec, exists := configSpecs[key] if !exists { - return nil, errors.Errorf("undefined config %q", config.Source) - } - - source := namespace.Scope(config.Source) - if configSpec.External.External { - source = configSpec.External.Name - } - - uid := config.UID - gid := config.GID - if uid == "" { - uid = "0" - } - if gid == "" { - gid = "0" - } - mode := config.Mode - if mode == nil { - mode = uint32Ptr(0444) + return composetypes.FileObjectConfig{}, errors.Errorf("undefined config %q", key) + } + return composetypes.FileObjectConfig(configSpec), nil + } + for _, config := range configs { + obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(config), lookup) + if err != nil { + return nil, err } + file := swarm.ConfigReferenceFileTarget(obj.File) refs = append(refs, &swarm.ConfigReference{ - File: &swarm.ConfigReferenceFileTarget{ - Name: target, - UID: uid, - GID: gid, - Mode: os.FileMode(*mode), - }, - ConfigName: source, + File: &file, + ConfigName: obj.Name, }) } @@ -361,6 +323,63 @@ func convertServiceConfigObjs( return confs, err } +type swarmReferenceTarget struct { + Name string + UID string + GID string + Mode os.FileMode +} + +type swarmReferenceObject struct { + File swarmReferenceTarget + ID string + Name string +} + +func convertFileObject( + namespace Namespace, + config composetypes.FileReferenceConfig, + lookup func(key string) (composetypes.FileObjectConfig, error), +) (swarmReferenceObject, error) { + target := config.Target + if target == "" { + target = config.Source + } + + obj, err := lookup(config.Source) + if err != nil { + return swarmReferenceObject{}, err + } + + source := namespace.Scope(config.Source) + if obj.External.External { + source = obj.External.Name + } + + uid := config.UID + gid := config.GID + if uid == "" { + uid = "0" + } + if gid == "" { + gid = "0" + } + mode := config.Mode + if mode == nil { + mode = uint32Ptr(0444) + } + + return swarmReferenceObject{ + File: swarmReferenceTarget{ + Name: target, + UID: uid, + GID: gid, + Mode: os.FileMode(*mode), + }, + Name: source, + }, nil +} + func uint32Ptr(value uint32) *uint32 { return &value } diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go index 4e68da08..99c53688 100644 --- a/cli/compose/types/types.go +++ b/cli/compose/types/types.go @@ -278,7 +278,8 @@ type ServiceVolumeVolume struct { NoCopy bool `mapstructure:"nocopy"` } -type fileReferenceConfig struct { +// FileReferenceConfig for a reference to a swarm file object +type FileReferenceConfig struct { Source string Target string UID string @@ -287,10 +288,10 @@ type fileReferenceConfig struct { } // ServiceConfigObjConfig is the config obj configuration for a service -type ServiceConfigObjConfig fileReferenceConfig +type ServiceConfigObjConfig FileReferenceConfig // ServiceSecretConfig is the secret configuration for a service -type ServiceSecretConfig fileReferenceConfig +type ServiceSecretConfig FileReferenceConfig // UlimitsConfig the ulimit configuration type UlimitsConfig struct { From f1cc679618fda89a87efc908c2229b4f5b8d52b0 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 10 Nov 2017 16:24:32 -0500 Subject: [PATCH 3/3] Add unit tests for some convert/service Signed-off-by: Daniel Nephin --- cli/compose/convert/service_test.go | 52 +++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/cli/compose/convert/service_test.go b/cli/compose/convert/service_test.go index 42e0a29d..c9534bca 100644 --- a/cli/compose/convert/service_test.go +++ b/cli/compose/convert/service_test.go @@ -1,6 +1,7 @@ package convert import ( + "os" "sort" "strings" "testing" @@ -9,7 +10,9 @@ import ( composetypes "github.com/docker/cli/cli/compose/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestConvertRestartPolicyFromNone(t *testing.T) { @@ -372,3 +375,52 @@ func TestConvertUpdateConfigOrder(t *testing.T) { }) assert.Equal(t, updateConfig.Order, "stop-first") } + +func TestConvertFileObject(t *testing.T) { + namespace := NewNamespace("testing") + config := composetypes.FileReferenceConfig{ + Source: "source", + Target: "target", + UID: "user", + GID: "group", + Mode: uint32Ptr(0644), + } + swarmRef, err := convertFileObject(namespace, config, lookupConfig) + require.NoError(t, err) + + expected := swarmReferenceObject{ + Name: "testing_source", + File: swarmReferenceTarget{ + Name: config.Target, + UID: config.UID, + GID: config.GID, + Mode: os.FileMode(0644), + }, + } + assert.Equal(t, expected, swarmRef) +} + +func lookupConfig(key string) (composetypes.FileObjectConfig, error) { + if key != "source" { + return composetypes.FileObjectConfig{}, errors.New("bad key") + } + return composetypes.FileObjectConfig{}, nil +} + +func TestConvertFileObjectDefaults(t *testing.T) { + namespace := NewNamespace("testing") + config := composetypes.FileReferenceConfig{Source: "source"} + swarmRef, err := convertFileObject(namespace, config, lookupConfig) + require.NoError(t, err) + + expected := swarmReferenceObject{ + Name: "testing_source", + File: swarmReferenceTarget{ + Name: config.Source, + UID: "0", + GID: "0", + Mode: os.FileMode(0444), + }, + } + assert.Equal(t, expected, swarmRef) +}