Merge pull request #895 from vdemeester/simplify-yaml-marshal

thaJeztah · web-flow · commit 88ee1a67f4e5 · 2018-02-21T19:02:04.000+01:00
Simplify the marshaling of compose types.Config
diff --git a/cli/command/stack/kubernetes/deploy.go b/cli/command/stack/kubernetes/deploy.go
@@ -22,11 +22,11 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error {
 	}
 
 	// Parse the compose file
-	cfg, version, err := loader.LoadComposefile(dockerCli, opts)
+	cfg, err := loader.LoadComposefile(dockerCli, opts)
 	if err != nil {
 		return err
 	}
-	stack, err := LoadStack(opts.Namespace, version, *cfg)
+	stack, err := LoadStack(opts.Namespace, *cfg)
 	if err != nil {
 		return err
 	}
diff --git a/cli/command/stack/kubernetes/loader.go b/cli/command/stack/kubernetes/loader.go
@@ -7,32 +7,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-type versionedConfig struct {
-	composetypes.Config
-	version string
-}
-
-func (c versionedConfig) MarshalYAML() (interface{}, error) {
-	services := map[string]composetypes.ServiceConfig{}
-	for _, service := range c.Services {
-		services[service.Name] = service
-	}
-	return map[string]interface{}{
-		"services": services,
-		"networks": c.Networks,
-		"volumes":  c.Volumes,
-		"secrets":  c.Secrets,
-		"configs":  c.Configs,
-		"version":  c.version,
-	}, nil
-}
-
 // 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,
-	})
+func LoadStack(name string, cfg composetypes.Config) (*apiv1beta1.Stack, error) {
+	res, err := yaml.Marshal(cfg)
 	if err != nil {
 		return nil, err
 	}
diff --git a/cli/command/stack/kubernetes/loader_test.go b/cli/command/stack/kubernetes/loader_test.go
@@ -10,7 +10,8 @@ import (
 )
 
 func TestLoadStack(t *testing.T) {
-	s, err := LoadStack("foo", "3.1", composetypes.Config{
+	s, err := LoadStack("foo", composetypes.Config{
+		Version:  "3.1",
 		Filename: "banana",
 		Services: []composetypes.ServiceConfig{
 			{
@@ -29,16 +30,16 @@ func TestLoadStack(t *testing.T) {
 			Name: "foo",
 		},
 		Spec: apiv1beta1.StackSpec{
-			ComposeFile: string(`configs: {}
-networks: {}
-secrets: {}
+			ComposeFile: string(`version: "3.1"
 services:
   bar:
     image: bar
   foo:
     image: foo
-version: "3.1"
+networks: {}
 volumes: {}
+secrets: {}
+configs: {}
 `),
 		},
 	}, s)
diff --git a/cli/command/stack/loader/loader.go b/cli/command/stack/loader/loader.go
@@ -18,21 +18,21 @@ import (
 )
 
 // 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) {
+func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, error) {
 	configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In())
 	if err != nil {
-		return nil, "", err
+		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",
+			return nil, errors.Errorf("Compose file contains unsupported options:\n\n%s\n",
 				propertyWarnings(fpe.Properties))
 		}
 
-		return nil, "", err
+		return nil, err
 	}
 
 	unsupportedProperties := loader.GetUnsupportedProperties(dicts...)
@@ -46,7 +46,7 @@ func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.
 		fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n",
 			propertyWarnings(deprecatedProperties))
 	}
-	return config, configDetails.Version, nil
+	return config, nil
 }
 
 func getDictsFrom(configFiles []composetypes.ConfigFile) []map[string]interface{} {
diff --git a/cli/command/stack/swarm/deploy_composefile.go b/cli/command/stack/swarm/deploy_composefile.go
@@ -18,7 +18,7 @@ import (
 )
 
 func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error {
-	config, _, err := loader.LoadComposefile(dockerCli, opts)
+	config, err := loader.LoadComposefile(dockerCli, opts)
 	if err != nil {
 		return err
 	}
diff --git a/cli/compose/loader/full-struct_test.go b/cli/compose/loader/full-struct_test.go
@@ -8,6 +8,7 @@ import (
 
 func fullExampleConfig(workingDir, homeDir string) *types.Config {
 	return &types.Config{
+		Version:  "3.6",
 		Services: services(workingDir, homeDir),
 		Networks: networks(),
 		Volumes:  volumes(),
diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go
@@ -98,7 +98,9 @@ func validateForbidden(configDict map[string]interface{}) error {
 
 func loadSections(config map[string]interface{}, configDetails types.ConfigDetails) (*types.Config, error) {
 	var err error
-	cfg := types.Config{}
+	cfg := types.Config{
+		Version: schema.Version(config),
+	}
 
 	var loaders = []struct {
 		key string
diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go
@@ -119,6 +119,7 @@ func strPtr(val string) *string {
 }
 
 var sampleConfig = types.Config{
+	Version: "3.0",
 	Services: []types.ServiceConfig{
 		{
 			Name:        "foo",
@@ -174,6 +175,7 @@ func TestParseYAML(t *testing.T) {
 func TestLoad(t *testing.T) {
 	actual, err := Load(buildConfigDetails(sampleDict, nil))
 	require.NoError(t, err)
+	assert.Equal(t, sampleConfig.Version, actual.Version)
 	assert.Equal(t, serviceSort(sampleConfig.Services), serviceSort(actual.Services))
 	assert.Equal(t, sampleConfig.Networks, actual.Networks)
 	assert.Equal(t, sampleConfig.Volumes, actual.Volumes)
@@ -573,6 +575,7 @@ networks:
 	require.NoError(t, err)
 	expected := &types.Config{
 		Filename: "filename.yml",
+		Version:  "3.4",
 		Services: []types.ServiceConfig{
 			{
 				Name: "web",
diff --git a/cli/compose/loader/merge_test.go b/cli/compose/loader/merge_test.go
@@ -207,6 +207,7 @@ func TestLoadLogging(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -325,6 +326,7 @@ func TestLoadMultipleServicePorts(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -450,6 +452,7 @@ func TestLoadMultipleSecretsConfig(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -575,6 +578,7 @@ func TestLoadMultipleConfigobjsConfig(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -690,6 +694,7 @@ func TestLoadMultipleUlimits(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -808,6 +813,7 @@ func TestLoadMultipleNetworks(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, &types.Config{
 				Filename: "base.yml",
+				Version:  "3.4",
 				Services: []types.ServiceConfig{
 					{
 						Name:        "foo",
@@ -895,6 +901,7 @@ func TestLoadMultipleConfigs(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, &types.Config{
 		Filename: "base.yml",
+		Version:  "3.4",
 		Services: []types.ServiceConfig{
 			{
 				Name:        "bar",
diff --git a/cli/compose/loader/types_test.go b/cli/compose/loader/types_test.go
@@ -9,26 +9,7 @@ import (
 
 func TestMarshallConfig(t *testing.T) {
 	cfg := fullExampleConfig("/foo", "/bar")
-	expected := `configs: {}
-networks:
-  external-network:
-    name: external-network
-    external: true
-  other-external-network:
-    name: my-cool-network
-    external: true
-  other-network:
-    driver: overlay
-    driver_opts:
-      baz: "1"
-      foo: bar
-    ipam:
-      driver: overlay
-      config:
-      - subnet: 172.16.238.0/24
-      - subnet: 2001:3984:3989::/64
-  some-network: {}
-secrets: {}
+	expected := `version: "3.6"
 services:
   foo:
     build:
@@ -292,6 +273,24 @@ services:
       tmpfs:
         size: 10000
     working_dir: /code
+networks:
+  external-network:
+    name: external-network
+    external: true
+  other-external-network:
+    name: my-cool-network
+    external: true
+  other-network:
+    driver: overlay
+    driver_opts:
+      baz: "1"
+      foo: bar
+    ipam:
+      driver: overlay
+      config:
+      - subnet: 172.16.238.0/24
+      - subnet: 2001:3984:3989::/64
+  some-network: {}
 volumes:
   another-volume:
     name: user_specified_name
@@ -314,6 +313,8 @@ volumes:
       baz: "1"
       foo: bar
   some-volume: {}
+secrets: {}
+configs: {}
 `
 
 	actual, err := yaml.Marshal(cfg)
diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go
@@ -71,26 +71,24 @@ func (cd ConfigDetails) LookupEnv(key string) (string, bool) {
 // Config is a full compose file configuration
 type Config struct {
 	Filename string `yaml:"-"`
-	Services []ServiceConfig
+	Version  string
+	Services Services
 	Networks map[string]NetworkConfig
 	Volumes  map[string]VolumeConfig
 	Secrets  map[string]SecretConfig
 	Configs  map[string]ConfigObjConfig
 }
 
-// MarshalYAML makes Config implement yaml.Marshaller
-func (c *Config) MarshalYAML() (interface{}, error) {
+// Services is a list of ServiceConfig
+type Services []ServiceConfig
+
+// MarshalYAML makes Services implement yaml.Marshaller
+func (s Services) MarshalYAML() (interface{}, error) {
 	services := map[string]ServiceConfig{}
-	for _, service := range c.Services {
+	for _, service := range s {
 		services[service.Name] = service
 	}
-	return map[string]interface{}{
-		"services": services,
-		"networks": c.Networks,
-		"volumes":  c.Volumes,
-		"secrets":  c.Secrets,
-		"configs":  c.Configs,
-	}, nil
+	return services, nil
 }
 
 // ServiceConfig is the configuration of one service

Original file line number	Diff line number	Diff line change
`@@ -22,11 +22,11 @@ func RunDeploy(dockerCli *KubeCli, opts options.Deploy) error {`
`22`	`22`	`}`
`23`	`23`
`24`	`24`	`// Parse the compose file`
`25`		`- cfg, version, err := loader.LoadComposefile(dockerCli, opts)`
	`25`	`+ cfg, err := loader.LoadComposefile(dockerCli, opts)`
`26`	`26`	`if err != nil {`
`27`	`27`	`return err`
`28`	`28`	`}`
`29`		`- stack, err := LoadStack(opts.Namespace, version, *cfg)`
	`29`	`+ stack, err := LoadStack(opts.Namespace, *cfg)`
`30`	`30`	`if err != nil {`
`31`	`31`	`return err`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,21 +18,21 @@ import (`
`18`	`18`	`)`
`19`	`19`
`20`	`20`	`// LoadComposefile parse the composefile specified in the cli and returns its Config and version.`
`21`		`-func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, string, error) {`
	`21`	`+func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.Config, error) {`
`22`	`22`	`configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In())`
`23`	`23`	`if err != nil {`
`24`		`- return nil, "", err`
	`24`	`+ return nil, err`
`25`	`25`	`}`
`26`	`26`
`27`	`27`	`dicts := getDictsFrom(configDetails.ConfigFiles)`
`28`	`28`	`config, err := loader.Load(configDetails)`
`29`	`29`	`if err != nil {`
`30`	`30`	`if fpe, ok := err.(*loader.ForbiddenPropertiesError); ok {`
`31`		`- return nil, "", errors.Errorf("Compose file contains unsupported options:\n\n%s\n",`
	`31`	`+ return nil, errors.Errorf("Compose file contains unsupported options:\n\n%s\n",`
`32`	`32`	`propertyWarnings(fpe.Properties))`
`33`	`33`	`}`
`34`	`34`
`35`		`- return nil, "", err`
	`35`	`+ return nil, err`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`unsupportedProperties := loader.GetUnsupportedProperties(dicts...)`
`@@ -46,7 +46,7 @@ func LoadComposefile(dockerCli command.Cli, opts options.Deploy) (*composetypes.`
`46`	`46`	`fmt.Fprintf(dockerCli.Err(), "Ignoring deprecated options:\n\n%s\n\n",`
`47`	`47`	`propertyWarnings(deprecatedProperties))`
`48`	`48`	`}`
`49`		`- return config, configDetails.Version, nil`
	`49`	`+ return config, nil`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`func getDictsFrom(configFiles []composetypes.ConfigFile) []map[string]interface{} {`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ import (`
`18`	`18`	`)`
`19`	`19`
`20`	`20`	`func deployCompose(ctx context.Context, dockerCli command.Cli, opts options.Deploy) error {`
`21`		`- config, _, err := loader.LoadComposefile(dockerCli, opts)`
	`21`	`+ config, err := loader.LoadComposefile(dockerCli, opts)`
`22`	`22`	`if err != nil {`
`23`	`23`	`return err`
`24`	`24`	`}`