Merge pull request #671 from dnephin/remove-secret-config-duplication

Remove secret/config duplication in cli/compose

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: obj.Annotations, Data: obj.Data})
-		result = append(result, swarm.SecretSpec{
-			Annotations: swarm.Annotations{
-				Name:   namespace.Scope(name),
-				Labels: AddStackLabel(namespace, secret.Labels),
-			},
-			Data: 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: obj.Annotations, Data: obj.Data})
-		result = append(result, swarm.ConfigSpec{
-			Annotations: swarm.Annotations{
-				Name:   namespace.Scope(name),
-				Labels: AddStackLabel(namespace, config.Labels),
-			},
-			Data: 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
+}

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)
+			return composetypes.FileObjectConfig{}, errors.Errorf("undefined secret %q", key)
-		}
+		}
-
+		return composetypes.FileObjectConfig(configSpec), nil
-		source := namespace.Scope(secret.Source)
+	}
-		if secretSpec.External.External {
+	for _, config := range secrets {
-			source = secretSpec.External.Name
+		obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(config), lookup)
-		}
+		if err != nil {
-
+			return nil, err
-		uid := secret.UID
-		gid := secret.GID
-		if uid == "" {
-			uid = "0"
-		}
-		if gid == "" {
-			gid = "0"
-		}
-		mode := secret.Mode
-		if mode == nil {
-			mode = uint32Ptr(0444)
 		}
 
+		file := swarm.SecretReferenceFileTarget(obj.File)
 		refs = append(refs, &swarm.SecretReference{
-			File: &swarm.SecretReferenceFileTarget{
+			File:       &file,
-				Name: target,
+			SecretName: obj.Name,
-				UID:  uid,
-				GID:  gid,
-				Mode: os.FileMode(*mode),
-			},
-			SecretName: source,
 		})
 	}
 
@@ -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)
+			return composetypes.FileObjectConfig{}, errors.Errorf("undefined config %q", key)
-		}
+		}
-
+		return composetypes.FileObjectConfig(configSpec), nil
-		source := namespace.Scope(config.Source)
+	}
-		if configSpec.External.External {
+	for _, config := range configs {
-			source = configSpec.External.Name
+		obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(config), lookup)
-		}
+		if err != nil {
-
+			return nil, err
-		uid := config.UID
-		gid := config.GID
-		if uid == "" {
-			uid = "0"
-		}
-		if gid == "" {
-			gid = "0"
-		}
-		mode := config.Mode
-		if mode == nil {
-			mode = uint32Ptr(0444)
 		}
 
+		file := swarm.ConfigReferenceFileTarget(obj.File)
 		refs = append(refs, &swarm.ConfigReference{
-			File: &swarm.ConfigReferenceFileTarget{
+			File:       &file,
-				Name: target,
+			ConfigName: obj.Name,
-				UID:  uid,
-				GID:  gid,
-				Mode: os.FileMode(*mode),
-			},
-			ConfigName: source,
 		})
 	}
 
@@ -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
 }

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)
+}

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 == "" {
+		obj, err := loadFileObjectConfig(name, types.FileObjectConfig(secret), details)
-			secret.External.Name = name
+		if err != nil {
-			secrets[name] = secret
+			return nil, err
-		}
-		if secret.File != "" {
-			secret.File = absPath(workingDir, secret.File)
 		}
+		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 == "" {
+		obj, err := loadFileObjectConfig(name, types.FileObjectConfig(config), details)
-			config.External.Name = name
+		if err != nil {
-			configs[name] = config
+			return nil, err
-		}
-		if config.File != "" {
-			config.File = absPath(workingDir, config.File)
 		}
+		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

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 {
@@ -344,14 +345,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