Merge pull request #668 from ilyasotkov/667-secret-config-names

Add "name" field to secrets and configs to allow interpolation in Compose files

cli/compose/convert/compose.go

@@ -138,9 +138,15 @@ func fileObjectConfig(namespace Namespace, name string, obj composetypes.FileObj
 		return swarmFileObject{}, err
 	}
 
+	if obj.Name != "" {
+		name = obj.Name
+	} else {
+		name = namespace.Scope(name)
+	}
+
 	return swarmFileObject{
 		Annotations: swarm.Annotations{
-			Name:   namespace.Scope(name),
+			Name:   name,
 			Labels: AddStackLabel(namespace, obj.Labels),
 		},
 		Data: data,

cli/compose/convert/service.go

@@ -258,14 +258,14 @@ func convertServiceSecrets(
 	refs := []*swarm.SecretReference{}
 
 	lookup := func(key string) (composetypes.FileObjectConfig, error) {
-		configSpec, exists := secretSpecs[key]
+		secretSpec, exists := secretSpecs[key]
 		if !exists {
 			return composetypes.FileObjectConfig{}, errors.Errorf("undefined secret %q", key)
 		}
-		return composetypes.FileObjectConfig(configSpec), nil
+		return composetypes.FileObjectConfig(secretSpec), nil
 	}
-	for _, config := range secrets {
+	for _, secret := range secrets {
-		obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(config), lookup)
+		obj, err := convertFileObject(namespace, composetypes.FileReferenceConfig(secret), lookup)
 		if err != nil {
 			return nil, err
 		}
@@ -353,8 +353,8 @@ func convertFileObject(
 	}
 
 	source := namespace.Scope(config.Source)
-	if obj.External.External {
+	if obj.Name != "" {
-		source = obj.External.Name
+		source = obj.Name
 	}
 
 	uid := config.UID

cli/compose/loader/loader.go

@@ -490,7 +490,7 @@ func LoadSecrets(source map[string]interface{}, details types.ConfigDetails) (ma
 		return secrets, err
 	}
 	for name, secret := range secrets {
-		obj, err := loadFileObjectConfig(name, types.FileObjectConfig(secret), details)
+		obj, err := loadFileObjectConfig(name, "secret", types.FileObjectConfig(secret), details)
 		if err != nil {
 			return nil, err
 		}
@@ -507,7 +507,7 @@ func LoadConfigObjs(source map[string]interface{}, details types.ConfigDetails)
 		return configs, err
 	}
 	for name, config := range configs {
-		obj, err := loadFileObjectConfig(name, types.FileObjectConfig(config), details)
+		obj, err := loadFileObjectConfig(name, "config", types.FileObjectConfig(config), details)
 		if err != nil {
 			return nil, err
 		}
@@ -516,13 +516,29 @@ func LoadConfigObjs(source map[string]interface{}, details types.ConfigDetails)
 	return configs, nil
 }
 
-func loadFileObjectConfig(name string, obj types.FileObjectConfig, details types.ConfigDetails) (types.FileObjectConfig, error) {
+func loadFileObjectConfig(name string, objType string, obj types.FileObjectConfig, details types.ConfigDetails) (types.FileObjectConfig, error) {
-	if obj.External.External && obj.External.Name == "" {
+	// if "external: true"
-		obj.External.Name = name
+	if obj.External.External {
+		// handle deprecated external.name
+		if obj.External.Name != "" {
+			if obj.Name != "" {
+				return obj, errors.Errorf("%[1]s %[2]s: %[1]s.external.name and %[1]s.name conflict; only use %[1]s.name", objType, name)
 			}
-	if obj.File != "" {
+			if versions.GreaterThanOrEqualTo(details.Version, "3.5") {
+				logrus.Warnf("%[1]s %[2]s: %[1]s.external.name is deprecated in favor of %[1]s.name", objType, name)
+			}
+			obj.Name = obj.External.Name
+			obj.External.Name = ""
+		} else {
+			if obj.Name == "" {
+				obj.Name = name
+			}
+		}
+		// if not "external: true"
+	} else {
 		obj.File = absPath(details.WorkingDir, obj.File)
 	}
+
 	return obj, nil
 }
 

cli/compose/loader/loader_test.go

@@ -626,10 +626,10 @@ networks:
 			},
 		},
 		Configs: map[string]types.ConfigObjConfig{
-			"appconfig": {External: types.External{External: true, Name: "appconfig"}},
+			"appconfig": {External: types.External{External: true}, Name: "appconfig"},
 		},
 		Secrets: map[string]types.SecretConfig{
-			"super": {External: types.External{External: true, Name: "super"}},
+			"super": {External: types.External{External: true}, Name: "super"},
 		},
 		Volumes: map[string]types.VolumeConfig{
 			"data": {External: types.External{External: true}, Name: "data"},
@@ -1464,11 +1464,24 @@ services:
     image: busybox
     isolation: process
 configs:
-  super:
+  foo:
+    name: fooqux
     external: true
+  bar:
+    name: barqux
+    file: ./example1.env
+secrets:
+  foo:
+    name: fooqux
+    external: true
+  bar:
+    name: barqux
+    file: ./full-example.yml
 `)
 	require.NoError(t, err)
-	require.Len(t, actual.Services, 1)
+	assert.Len(t, actual.Services, 1)
+	assert.Len(t, actual.Secrets, 2)
+	assert.Len(t, actual.Configs, 2)
 	assert.Equal(t, "process", actual.Services[0].Isolation)
 }
 
@@ -1488,3 +1501,44 @@ configs:
 	require.Len(t, actual.Services, 1)
 	assert.Equal(t, "invalid", actual.Services[0].Isolation)
 }
+
+func TestInvalidSecretExternalNameAndNameCombination(t *testing.T) {
+	_, err := loadYAML(`
+version: "3.5"
+secrets:
+  external_secret:
+    name: user_specified_name
+    external:
+      name:	external_name
+`)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "secret.external.name and secret.name conflict; only use secret.name")
+	assert.Contains(t, err.Error(), "external_secret")
+}
+
+func TestLoadSecretsWarnOnDeprecatedExternalNameVersion35(t *testing.T) {
+	buf, cleanup := patchLogrus()
+	defer cleanup()
+
+	source := map[string]interface{}{
+		"foo": map[string]interface{}{
+			"external": map[string]interface{}{
+				"name": "oops",
+			},
+		},
+	}
+	details := types.ConfigDetails{
+		Version: "3.5",
+	}
+	secrets, err := LoadSecrets(source, details)
+	require.NoError(t, err)
+	expected := map[string]types.SecretConfig{
+		"foo": {
+			Name:     "oops",
+			External: types.External{External: true},
+		},
+	}
+	assert.Equal(t, expected, secrets)
+	assert.Contains(t, buf.String(), "secret.external.name is deprecated")
+}

cli/compose/schema/data/config_schema_v3.5.json

@@ -470,6 +470,7 @@
       "id": "#/definitions/secret",
       "type": "object",
       "properties": {
+        "name": {"type": "string"},
         "file": {"type": "string"},
         "external": {
           "type": ["boolean", "object"],
@@ -486,6 +487,7 @@
       "id": "#/definitions/config",
       "type": "object",
       "properties": {
+        "name": {"type": "string"},
         "file": {"type": "string"},
         "external": {
           "type": ["boolean", "object"],

cli/compose/schema/schema_test.go

@@ -46,6 +46,25 @@ func TestValidateAllowsXTopLevelFields(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestValidateSecretConfigNames(t *testing.T) {
+	config := dict{
+		"version": "3.5",
+		"configs": dict{
+			"bar": dict{
+				"name": "foobar",
+			},
+		},
+		"secrets": dict{
+			"baz": dict{
+				"name": "foobaz",
+			},
+		},
+	}
+
+	err := Validate(config, "3.5")
+	assert.NoError(t, err)
+}
+
 func TestValidateInvalidVersion(t *testing.T) {
 	config := dict{
 		"version": "2.1",

cli/compose/types/types.go

@@ -348,6 +348,7 @@ type CredentialSpecConfig struct {
 
 // FileObjectConfig is a config type for a file used by a service
 type FileObjectConfig struct {
+	Name     string
 	File     string
 	External External
 	Labels   Labels