Share the compose loading code between swarm and k8s stack deploy

To ensure we are loading the composefile the same wether we are pointing
to swarm or kubernetes, we need to share the loading code between both.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>

cli/command/stack/kubernetes/deploy.go

@@ -5,8 +5,9 @@ import (
 	"io/ioutil"
 	"path"
 
+	"github.com/docker/cli/cli/command/stack/loader"
 	"github.com/docker/cli/cli/command/stack/options"
-	composeTypes "github.com/docker/cli/cli/compose/types"
+	composetypes "github.com/docker/cli/cli/compose/types"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -19,6 +20,17 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error {
 	if len(opts.Composefiles) == 0 {
 		return errors.Errorf("Please specify only one compose file (with --compose-file).")
 	}
+
+	// Parse the compose file
+	cfg, version, err := loader.LoadComposefile(dockerCli, opts)
+	if err != nil {
+		return err
+	}
+	stack, err := LoadStack(opts.Namespace, version, cfg)
+	if err != nil {
+		return err
+	}
+
 	// Initialize clients
 	stacks, err := dockerCli.stacks()
 	if err != nil {
@@ -36,12 +48,6 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error {
 		Pods: pods,
 	}
 
-	// Parse the compose file
-	stack, cfg, err := LoadStack(opts.Namespace, opts.Composefiles)
-	if err != nil {
-		return err
-	}
-
 	// FIXME(vdemeester) handle warnings server-side
 	if err = IsColliding(services, stack, cfg); err != nil {
 		return err
@@ -82,7 +88,7 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error {
 }
 
 // createFileBasedConfigMaps creates a Kubernetes ConfigMap for each Compose global file-based config.
-func createFileBasedConfigMaps(stackName string, globalConfigs map[string]composeTypes.ConfigObjConfig, configMaps corev1.ConfigMapInterface) error {
+func createFileBasedConfigMaps(stackName string, globalConfigs map[string]composetypes.ConfigObjConfig, configMaps corev1.ConfigMapInterface) error {
 	for name, config := range globalConfigs {
 		if config.File == "" {
 			continue
@@ -102,7 +108,7 @@ func createFileBasedConfigMaps(stackName string, globalConfigs map[string]compos
 	return nil
 }
 
-func serviceNames(cfg *composeTypes.Config) []string {
+func serviceNames(cfg *composetypes.Config) []string {
 	names := []string{}
 
 	for _, service := range cfg.Services {
@@ -113,7 +119,7 @@ func serviceNames(cfg *composeTypes.Config) []string {
 }
 
 // createFileBasedSecrets creates a Kubernetes Secret for each Compose global file-based secret.
-func createFileBasedSecrets(stackName string, globalSecrets map[string]composeTypes.SecretConfig, secrets corev1.SecretInterface) error {
+func createFileBasedSecrets(stackName string, globalSecrets map[string]composetypes.SecretConfig, secrets corev1.SecretInterface) error {
 	for name, secret := range globalSecrets {
 		if secret.File == "" {
 			continue

cli/command/stack/kubernetes/loader.go

@@ -1,170 +1,32 @@
 package kubernetes
 
 import (
-	"bufio"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/docker/cli/cli/compose/loader"
-	"github.com/docker/cli/cli/compose/template"
 	composetypes "github.com/docker/cli/cli/compose/types"
 	apiv1beta1 "github.com/docker/cli/kubernetes/compose/v1beta1"
-	"github.com/pkg/errors"
 	yaml "gopkg.in/yaml.v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// LoadStack loads a stack from a Compose file, with a given name.
-// FIXME(vdemeester) remove this and use cli/compose/loader for both swarm and kubernetes
-func LoadStack(name string, composeFiles []string) (*apiv1beta1.Stack, *composetypes.Config, error) {
-	if len(composeFiles) != 1 {
-		return nil, nil, errors.New("compose-file must be set (and only one)")
-	}
-	composeFile := composeFiles[0]
-
-	workingDir, err := os.Getwd()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	composePath := composeFile
-	if !strings.HasPrefix(composePath, "/") {
-		composePath = filepath.Join(workingDir, composeFile)
-	}
-
-	if _, err := os.Stat(composePath); os.IsNotExist(err) {
-		return nil, nil, errors.Errorf("no compose file found in %s", filepath.Dir(composePath))
-	}
-
-	binary, err := ioutil.ReadFile(composePath)
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "cannot read compose file")
-	}
-
-	env := env(workingDir)
-	return load(name, binary, workingDir, env)
+type versionedConfig struct {
+	composetypes.Config
+	Version string
 }
 
-func load(name string, binary []byte, workingDir string, env map[string]string) (*apiv1beta1.Stack, *composetypes.Config, error) {
-	processed, err := template.Substitute(string(binary), func(key string) (string, bool) { return env[key], true })
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "cannot load compose file")
-	}
-
-	parsed, err := loader.ParseYAML([]byte(processed))
-	if err != nil {
-		return nil, nil, errors.Wrapf(err, "cannot load compose file")
-	}
-
-	cfg, err := loader.Load(composetypes.ConfigDetails{
-		WorkingDir: workingDir,
-		ConfigFiles: []composetypes.ConfigFile{
-			{
-				Config: parsed,
-			},
-		},
+// LoadStack loads a stack from a Compose config, with a given name.
+func LoadStack(name, version string, cfg *composetypes.Config) (*apiv1beta1.Stack, error) {
+	res, err := yaml.Marshal(versionedConfig{
+		Version: version,
+		Config:  *cfg,
 	})
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "cannot load compose file")
+		return nil, err
 	}
-
-	result, err := processEnvFiles(processed, parsed, cfg)
-	if err != nil {
-		return nil, nil, errors.Wrapf(err, "cannot load compose file")
-	}
-
 	return &apiv1beta1.Stack{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: name,
 		},
 		Spec: apiv1beta1.StackSpec{
-			ComposeFile: result,
+			ComposeFile: string(res),
 		},
-	}, cfg, nil
-}
-
-type iMap = map[string]interface{}
-
-func processEnvFiles(input string, parsed map[string]interface{}, config *composetypes.Config) (string, error) {
-	changed := false
-
-	for _, svc := range config.Services {
-		if len(svc.EnvFile) == 0 {
-			continue
-		}
-		// Load() processed the env_file for us, we just need to inject back into
-		// the intermediate representation
-		env := iMap{}
-		for k, v := range svc.Environment {
-			env[k] = v
-		}
-		parsed["services"].(iMap)[svc.Name].(iMap)["environment"] = env
-		delete(parsed["services"].(iMap)[svc.Name].(iMap), "env_file")
-		changed = true
-	}
-	if !changed {
-		return input, nil
-	}
-	res, err := yaml.Marshal(parsed)
-	if err != nil {
-		return "", err
-	}
-	return string(res), nil
-}
-
-func env(workingDir string) map[string]string {
-	// Apply .env file first
-	config := readEnvFile(filepath.Join(workingDir, ".env"))
-
-	// Apply env variables
-	for k, v := range envToMap(os.Environ()) {
-		config[k] = v
-	}
-
-	return config
-}
-
-func readEnvFile(path string) map[string]string {
-	config := map[string]string{}
-
-	file, err := os.Open(path)
-	if err != nil {
-		return config // Ignore
-	}
-	defer file.Close()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		line := scanner.Text()
-		if strings.HasPrefix(strings.TrimSpace(line), "#") {
-			continue
-		}
-
-		parts := strings.SplitN(line, "=", 2)
-		if len(parts) == 2 {
-			key := parts[0]
-			value := parts[1]
-
-			config[key] = value
-		}
-	}
-
-	return config
-}
-
-func envToMap(env []string) map[string]string {
-	config := map[string]string{}
-
-	for _, value := range env {
-		parts := strings.SplitN(value, "=", 2)
-
-		key := parts[0]
-		value := parts[1]
-
-		config[key] = value
-	}
-
-	return config
+	}, nil
 }

cli/command/stack/kubernetes/loader_test.go

@@ -1,34 +0,0 @@
-package kubernetes
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestPlaceholders(t *testing.T) {
-	env := map[string]string{
-		"TAG": "_latest_",
-		"K1":  "V1",
-		"K2":  "V2",
-	}
-
-	prefix := "version: '3'\nvolumes:\n  data:\n    external:\n      name: "
-	var tests = []struct {
-		input          string
-		expectedOutput string
-	}{
-		{prefix + "BEFORE${TAG}AFTER", prefix + "BEFORE_latest_AFTER"},
-		{prefix + "BEFORE${K1}${K2}AFTER", prefix + "BEFOREV1V2AFTER"},
-		{prefix + "BEFORE$TAG AFTER", prefix + "BEFORE_latest_ AFTER"},
-		{prefix + "BEFORE$$TAG AFTER", prefix + "BEFORE$TAG AFTER"},
-		{prefix + "BEFORE $UNKNOWN AFTER", prefix + "BEFORE  AFTER"},
-	}
-
-	for _, test := range tests {
-		output, _, err := load("stack", []byte(test.input), ".", env)
-		require.NoError(t, err)
-		assert.Equal(t, test.expectedOutput, output.Spec.ComposeFile)
-	}
-}

cli/command/stack/loader/loader.go

@@ -0,0 +1,152 @@
+package loader
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+
+	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/command/stack/options"
+	"github.com/docker/cli/cli/compose/loader"
+	"github.com/docker/cli/cli/compose/schema"
+	composetypes "github.com/docker/cli/cli/compose/types"
+	"github.com/pkg/errors"
+)
+
+// LoadComposefile parse the composefile specified in the cli and returns its Config and version.
+func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, string, error) {
+	configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In())
+	if err != nil {
+		return nil, "", err
+	}
+
+	dicts := getDictsFrom(configDetails.ConfigFiles)
+	config, err := loader.Load(configDetails)
+	if err != nil {
+		if fpe, ok := err.(*loader.ForbiddenPropertiesError); ok {
+			return nil, "", errors.Errorf("Compose file contains unsupported options:\n\n%s\n",
+				propertyWarnings(fpe.Properties))
+		}
+
+		return nil, "", err
+	}
+
+	unsupportedProperties := loader.GetUnsupportedProperties(dicts...)
+	if len(unsupportedProperties) > 0 {
+		fmt.Fprintf(dockerCli.Err(), "Ignoring unsupported options: %s\n\n",
+			strings.Join(unsupportedProperties, ", "))
+	}
+
+	deprecatedProperties := loader.GetDeprecatedProperties(dicts...)
+	if len(deprecatedProperties) > 0 {
+		fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n",
+			propertyWarnings(deprecatedProperties))
+	}
+	return config, configDetails.Version, nil
+}
+
+func getDictsFrom(configFiles []composetypes.ConfigFile) []map[string]interface{} {
+	dicts := []map[string]interface{}{}
+
+	for _, configFile := range configFiles {
+		dicts = append(dicts, configFile.Config)
+	}
+
+	return dicts
+}
+
+func propertyWarnings(properties map[string]string) string {
+	var msgs []string
+	for name, description := range properties {
+		msgs = append(msgs, fmt.Sprintf("%s: %s", name, description))
+	}
+	sort.Strings(msgs)
+	return strings.Join(msgs, "\n\n")
+}
+
+func getConfigDetails(composefiles []string, stdin io.Reader) (composetypes.ConfigDetails, error) {
+	var details composetypes.ConfigDetails
+
+	if len(composefiles) == 0 {
+		return details, errors.New("no composefile(s)")
+	}
+
+	if composefiles[0] == "-" && len(composefiles) == 1 {
+		workingDir, err := os.Getwd()
+		if err != nil {
+			return details, err
+		}
+		details.WorkingDir = workingDir
+	} else {
+		absPath, err := filepath.Abs(composefiles[0])
+		if err != nil {
+			return details, err
+		}
+		details.WorkingDir = filepath.Dir(absPath)
+	}
+
+	var err error
+	details.ConfigFiles, err = loadConfigFiles(composefiles, stdin)
+	if err != nil {
+		return details, err
+	}
+	// Take the first file version (2 files can't have different version)
+	details.Version = schema.Version(details.ConfigFiles[0].Config)
+	details.Environment, err = buildEnvironment(os.Environ())
+	return details, err
+}
+
+func buildEnvironment(env []string) (map[string]string, error) {
+	result := make(map[string]string, len(env))
+	for _, s := range env {
+		// if value is empty, s is like "K=", not "K".
+		if !strings.Contains(s, "=") {
+			return result, errors.Errorf("unexpected environment %q", s)
+		}
+		kv := strings.SplitN(s, "=", 2)
+		result[kv[0]] = kv[1]
+	}
+	return result, nil
+}
+
+func loadConfigFiles(filenames []string, stdin io.Reader) ([]composetypes.ConfigFile, error) {
+	var configFiles []composetypes.ConfigFile
+
+	for _, filename := range filenames {
+		configFile, err := loadConfigFile(filename, stdin)
+		if err != nil {
+			return configFiles, err
+		}
+		configFiles = append(configFiles, *configFile)
+	}
+
+	return configFiles, nil
+}
+
+func loadConfigFile(filename string, stdin io.Reader) (*composetypes.ConfigFile, error) {
+	var bytes []byte
+	var err error
+
+	if filename == "-" {
+		bytes, err = ioutil.ReadAll(stdin)
+	} else {
+		bytes, err = ioutil.ReadFile(filename)
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	config, err := loader.ParseYAML(bytes)
+	if err != nil {
+		return nil, err
+	}
+
+	return &composetypes.ConfigFile{
+		Filename: filename,
+		Config:   config,
+	}, nil
+}

cli/command/stack/loader/loader_test.go

@@ -0,0 +1,47 @@
+package loader
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/gotestyourself/gotestyourself/fs"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetConfigDetails(t *testing.T) {
+	content := `
+version: "3.0"
+services:
+  foo:
+    image: alpine:3.5
+`
+	file := fs.NewFile(t, "test-get-config-details", fs.WithContent(content))
+	defer file.Remove()
+
+	details, err := getConfigDetails([]string{file.Path()}, nil)
+	require.NoError(t, err)
+	assert.Equal(t, filepath.Dir(file.Path()), details.WorkingDir)
+	require.Len(t, details.ConfigFiles, 1)
+	assert.Equal(t, "3.0", details.ConfigFiles[0].Config["version"])
+	assert.Len(t, details.Environment, len(os.Environ()))
+}
+
+func TestGetConfigDetailsStdin(t *testing.T) {
+	content := `
+version: "3.0"
+services:
+  foo:
+    image: alpine:3.5
+`
+	details, err := getConfigDetails([]string{"-"}, strings.NewReader(content))
+	require.NoError(t, err)
+	cwd, err := os.Getwd()
+	require.NoError(t, err)
+	assert.Equal(t, cwd, details.WorkingDir)
+	require.Len(t, details.ConfigFiles, 1)
+	assert.Equal(t, "3.0", details.ConfigFiles[0].Config["version"])
+	assert.Len(t, details.Environment, len(os.Environ()))
+}

cli/command/stack/swarm/deploy_composefile.go

@@ -2,17 +2,11 @@ package swarm
 
 import (
 	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"sort"
-	"strings"
 
 	"github.com/docker/cli/cli/command"
+	"github.com/docker/cli/cli/command/stack/loader"
 	"github.com/docker/cli/cli/command/stack/options"
 	"github.com/docker/cli/cli/compose/convert"
-	"github.com/docker/cli/cli/compose/loader"
 	composetypes "github.com/docker/cli/cli/compose/types"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
@@ -24,34 +18,11 @@ import (
 )
 
 func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error {
-	configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In())
+	config, _, err := loader.LoadComposefile(dockerCli, opts)
 	if err != nil {
 		return err
 	}
 
-	config, err := loader.Load(configDetails)
-	if err != nil {
-		if fpe, ok := err.(*loader.ForbiddenPropertiesError); ok {
-			return errors.Errorf("Compose file contains unsupported options:\n\n%s\n",
-				propertyWarnings(fpe.Properties))
-		}
-
-		return err
-	}
-
-	dicts := getDictsFrom(configDetails.ConfigFiles)
-	unsupportedProperties := loader.GetUnsupportedProperties(dicts...)
-	if len(unsupportedProperties) > 0 {
-		fmt.Fprintf(dockerCli.Err(), "Ignoring unsupported options: %s\n\n",
-			strings.Join(unsupportedProperties, ", "))
-	}
-
-	deprecatedProperties := loader.GetDeprecatedProperties(dicts...)
-	if len(deprecatedProperties) > 0 {
-		fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n",
-			propertyWarnings(deprecatedProperties))
-	}
-
 	if err := checkDaemonIsSwarmManager(ctx, dockerCli); err != nil {
 		return err
 	}
@@ -98,16 +69,6 @@ func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Depl
 	return deployServices(ctx, dockerCli, services, namespace, opts.SendRegistryAuth, opts.ResolveImage)
 }
 
-func getDictsFrom(configFiles []composetypes.ConfigFile) []map[string]interface{} {
-	dicts := []map[string]interface{}{}
-
-	for _, configFile := range configFiles {
-		dicts = append(dicts, configFile.Config)
-	}
-
-	return dicts
-}
-
 func getServicesDeclaredNetworks(serviceConfigs []composetypes.ServiceConfig) map[string]struct{} {
 	serviceNetworks := map[string]struct{}{}
 	for _, serviceConfig := range serviceConfigs {
@@ -122,96 +83,6 @@ func getServicesDeclaredNetworks(serviceConfigs []composetypes.ServiceConfig) ma
 	return serviceNetworks
 }
 
-func propertyWarnings(properties map[string]string) string {
-	var msgs []string
-	for name, description := range properties {
-		msgs = append(msgs, fmt.Sprintf("%s: %s", name, description))
-	}
-	sort.Strings(msgs)
-	return strings.Join(msgs, "\n\n")
-}
-
-func getConfigDetails(composefiles []string, stdin io.Reader) (composetypes.ConfigDetails, error) {
-	var details composetypes.ConfigDetails
-
-	if len(composefiles) == 0 {
-		return details, errors.New("no composefile(s)")
-	}
-
-	if composefiles[0] == "-" && len(composefiles) == 1 {
-		workingDir, err := os.Getwd()
-		if err != nil {
-			return details, err
-		}
-		details.WorkingDir = workingDir
-	} else {
-		absPath, err := filepath.Abs(composefiles[0])
-		if err != nil {
-			return details, err
-		}
-		details.WorkingDir = filepath.Dir(absPath)
-	}
-
-	var err error
-	details.ConfigFiles, err = loadConfigFiles(composefiles, stdin)
-	if err != nil {
-		return details, err
-	}
-	details.Environment, err = buildEnvironment(os.Environ())
-	return details, err
-}
-
-func buildEnvironment(env []string) (map[string]string, error) {
-	result := make(map[string]string, len(env))
-	for _, s := range env {
-		// if value is empty, s is like "K=", not "K".
-		if !strings.Contains(s, "=") {
-			return result, errors.Errorf("unexpected environment %q", s)
-		}
-		kv := strings.SplitN(s, "=", 2)
-		result[kv[0]] = kv[1]
-	}
-	return result, nil
-}
-
-func loadConfigFiles(filenames []string, stdin io.Reader) ([]composetypes.ConfigFile, error) {
-	var configFiles []composetypes.ConfigFile
-
-	for _, filename := range filenames {
-		configFile, err := loadConfigFile(filename, stdin)
-		if err != nil {
-			return configFiles, err
-		}
-		configFiles = append(configFiles, *configFile)
-	}
-
-	return configFiles, nil
-}
-
-func loadConfigFile(filename string, stdin io.Reader) (*composetypes.ConfigFile, error) {
-	var bytes []byte
-	var err error
-
-	if filename == "-" {
-		bytes, err = ioutil.ReadAll(stdin)
-	} else {
-		bytes, err = ioutil.ReadFile(filename)
-	}
-	if err != nil {
-		return nil, err
-	}
-
-	config, err := loader.ParseYAML(bytes)
-	if err != nil {
-		return nil, err
-	}
-
-	return &composetypes.ConfigFile{
-		Filename: filename,
-		Config:   config,
-	}, nil
-}
-
 func validateExternalNetworks(
 	ctx context.Context,
 	client dockerclient.NetworkAPIClient,

cli/command/stack/swarm/deploy_composefile_test.go

@@ -1,56 +1,16 @@
 package swarm
 
 import (
-	"os"
-	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/docker/cli/internal/test/network"
 	"github.com/docker/cli/internal/test/testutil"
 	"github.com/docker/docker/api/types"
-	"github.com/gotestyourself/gotestyourself/fs"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
 )
 
-func TestGetConfigDetails(t *testing.T) {
-	content := `
-version: "3.0"
-services:
-  foo:
-    image: alpine:3.5
-`
-	file := fs.NewFile(t, "test-get-config-details", fs.WithContent(content))
-	defer file.Remove()
-
-	details, err := getConfigDetails([]string{file.Path()}, nil)
-	require.NoError(t, err)
-	assert.Equal(t, filepath.Dir(file.Path()), details.WorkingDir)
-	require.Len(t, details.ConfigFiles, 1)
-	assert.Equal(t, "3.0", details.ConfigFiles[0].Config["version"])
-	assert.Len(t, details.Environment, len(os.Environ()))
-}
-
-func TestGetConfigDetailsStdin(t *testing.T) {
-	content := `
-version: "3.0"
-services:
-  foo:
-    image: alpine:3.5
-`
-	details, err := getConfigDetails([]string{"-"}, strings.NewReader(content))
-	require.NoError(t, err)
-	cwd, err := os.Getwd()
-	require.NoError(t, err)
-	assert.Equal(t, cwd, details.WorkingDir)
-	require.Len(t, details.ConfigFiles, 1)
-	assert.Equal(t, "3.0", details.ConfigFiles[0].Config["version"])
-	assert.Len(t, details.Environment, len(os.Environ()))
-}
-
 type notFound struct {
 	error
 }