Skip to content
1 change: 1 addition & 0 deletions cli/command/stack/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func NewStackCommand(dockerCli command.Cli) *cobra.Command {
newPsCommand(dockerCli, &opts),
newRemoveCommand(dockerCli, &opts),
newServicesCommand(dockerCli, &opts),
newConfigCommand(dockerCli),
)
flags := cmd.PersistentFlags()
flags.String("kubeconfig", "", "Kubernetes config file")
Expand Down
61 changes: 61 additions & 0 deletions cli/command/stack/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package stack

import (
"fmt"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/stack/loader"
"github.com/docker/cli/cli/command/stack/options"
composeLoader "github.com/docker/cli/cli/compose/loader"
composetypes "github.com/docker/cli/cli/compose/types"
"github.com/spf13/cobra"
yaml "gopkg.in/yaml.v2"
)

func newConfigCommand(dockerCli command.Cli) *cobra.Command {
var opts options.Config

cmd := &cobra.Command{
Use: "config [OPTIONS]",
Short: "Outputs the final config file, after doing merges and interpolations",
Args: cli.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
configDetails, err := loader.GetConfigDetails(opts.Composefiles, dockerCli.In())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: @SMFloris why not using LoadComposefile function instead, which actually calls getConfigDetails ?

Copy link
Contributor Author

@SMFloris SMFloris Oct 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question!

If I recall correctly, I didn't use it because I wanted to have control on how the compose files are actually getting loaded.
The --skip-interpolation flag is being sent down to the composeLoader.load() call, like so:

optsFunc := func(options *composeLoader.Options) {
	options.SkipInterpolation = skipInterpolation
}
config, err := composeLoader.Load(configFiles, optsFunc)

This is kinda important to the use-case of this new command, seeing the output with and without interpolation to figure out where things went wrong and if you're deploying the right thing.

Another reason is that LoadComposefile also does some other things besides loading compose files, it also does some validation (forbidden, deprecated and unsupported properties). I thought those validations were just for deploy since the validation of the composeLoader.load is also used. 🤔 Also, since in my mind I'm piping the output back to docker stack deploy -c - I saw no reason in doing deploy validations when not doing a deploy.

The final reason, since I wasn't that familiar with the code-base and because I added a new command I didn't want to mess too much with the existing signatures of the methods 😅

Now that I'm more familiar with the codebase. I can change the LoadComposefile function to pass down some options to composeLoader.load() call if you want me too (perhaps even adding an option to skip those other validations).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah any thoughts on that? I'm a bit on the fence, as if the output is pipped to the docker stack deploy command, then the config should print the warnings I guess... but it can also be pipped to the docker-compose command...
@SMFloris propose to call LoadComposeFile and add the --skip-validation option so one can disable the validation... WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah , could we get your input on the above, please? 😄 We are doing some ugly hacks with downgrading docker-compose in order to get valid docker stack deploy files

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also does some validation (forbidden, deprecated and unsupported properties). I thought those validations were just for deploy since the validation of the composeLoader.load is also used

So if we're trying to replicate docker-compose config, that command is described as "Validate and view the Compose file"; from that perspective, I think it would make sense to have it validate the compose-file, so that only a valid file would be generated.

but it can also be pipped to the docker-compose command...

Not sure if we should consider if for that (as docker-compose has a config command itself 🤔

propose to call LoadComposeFile and add the --skip-validation option so one can disable the validation... WDYT?

Do we know why the --skip-validation option was added in compose? Wondering what the exact use-case is if that would allow it to produce invalid output (or is it skipping specific validation steps?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah , I managed to track the inclusion of these options down to #1128

Maybe reading through that a bit can help jog some memories 😄

if err != nil {
return err
}

cfg, err := outputConfig(configDetails, opts.SkipInterpolation)
if err != nil {
return err
}

_, err = fmt.Fprintf(dockerCli.Out(), "%s", cfg)
return err
},
Annotations: map[string]string{"experimental": ""},
}

flags := cmd.Flags()
flags.StringSliceVarP(&opts.Composefiles, "compose-file", "c", []string{}, `Path to a Compose file, or "-" to read from stdin`)
flags.BoolVar(&opts.SkipInterpolation, "skip-interpolation", false, "Skip interpolation and output only merged config")
return cmd
}

// outputConfig returns the merged and interpolated config file
func outputConfig(configFiles composetypes.ConfigDetails, skipInterpolation bool) (string, error) {
optsFunc := func(options *composeLoader.Options) {
options.SkipInterpolation = skipInterpolation
}
config, err := composeLoader.Load(configFiles, optsFunc)
if err != nil {
return "", err
}

d, err := yaml.Marshal(&config)
if err != nil {
return "", err
}
return string(d), nil
}
106 changes: 106 additions & 0 deletions cli/command/stack/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package stack

import (
"io/ioutil"
"testing"

"github.com/docker/cli/cli/compose/loader"
composetypes "github.com/docker/cli/cli/compose/types"
"github.com/docker/cli/internal/test"
"gotest.tools/v3/assert"
)

func TestConfigWithEmptyComposeFile(t *testing.T) {
cmd := newConfigCommand(test.NewFakeCli(&fakeClient{}))
cmd.SetOut(ioutil.Discard)

assert.ErrorContains(t, cmd.Execute(), `Please specify a Compose file`)
}

var configMergeTests = []struct {
name string
skipInterpolation bool
first string
second string
merged string
}{
{
name: "With Interpolation",
skipInterpolation: false,
first: `version: "3.7"
services:
foo:
image: busybox:latest
command: cat file1.txt
`,
second: `version: "3.7"
services:
foo:
image: busybox:${VERSION}
command: cat file2.txt
`,
merged: `version: "3.7"
services:
foo:
command:
- cat
- file2.txt
image: busybox:1.0
`,
},
{
name: "Without Interpolation",
skipInterpolation: true,
first: `version: "3.7"
services:
foo:
image: busybox:latest
command: cat file1.txt
`,
second: `version: "3.7"
services:
foo:
image: busybox:${VERSION}
command: cat file2.txt
`,
merged: `version: "3.7"
services:
foo:
command:
- cat
- file2.txt
image: busybox:${VERSION}
`,
},
}

func TestConfigMergeInterpolation(t *testing.T) {

for _, tt := range configMergeTests {
t.Run(tt.name, func(t *testing.T) {
firstConfig := []byte(tt.first)
secondConfig := []byte(tt.second)

firstConfigData, err := loader.ParseYAML(firstConfig)
assert.NilError(t, err)
secondConfigData, err := loader.ParseYAML(secondConfig)
assert.NilError(t, err)

env := map[string]string{
"VERSION": "1.0",
}

cfg, err := outputConfig(composetypes.ConfigDetails{
ConfigFiles: []composetypes.ConfigFile{
{Config: firstConfigData, Filename: "firstConfig"},
{Config: secondConfigData, Filename: "secondConfig"},
},
Environment: env,
}, tt.skipInterpolation)
assert.NilError(t, err)

assert.Equal(t, cfg, tt.merged)
})
}

}
5 changes: 3 additions & 2 deletions cli/command/stack/loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ 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, error) {
configDetails, err := getConfigDetails(opts.Composefiles, dockerCli.In())
configDetails, err := GetConfigDetails(opts.Composefiles, dockerCli.In())
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -68,7 +68,8 @@ func propertyWarnings(properties map[string]string) string {
return strings.Join(msgs, "\n\n")
}

func getConfigDetails(composefiles []string, stdin io.Reader) (composetypes.ConfigDetails, error) {
// GetConfigDetails parse the composefiles specified in the cli and returns their ConfigDetails
func GetConfigDetails(composefiles []string, stdin io.Reader) (composetypes.ConfigDetails, error) {
var details composetypes.ConfigDetails

if len(composefiles) == 0 {
Expand Down
4 changes: 2 additions & 2 deletions cli/command/stack/loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ services:
file := fs.NewFile(t, "test-get-config-details", fs.WithContent(content))
defer file.Remove()

details, err := getConfigDetails([]string{file.Path()}, nil)
details, err := GetConfigDetails([]string{file.Path()}, nil)
assert.NilError(t, err)
assert.Check(t, is.Equal(filepath.Dir(file.Path()), details.WorkingDir))
assert.Assert(t, is.Len(details.ConfigFiles, 1))
Expand All @@ -36,7 +36,7 @@ services:
foo:
image: alpine:3.5
`
details, err := getConfigDetails([]string{"-"}, strings.NewReader(content))
details, err := GetConfigDetails([]string{"-"}, strings.NewReader(content))
assert.NilError(t, err)
cwd, err := os.Getwd()
assert.NilError(t, err)
Expand Down
6 changes: 6 additions & 0 deletions cli/command/stack/options/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ type Deploy struct {
Prune bool
}

// Config holds docker stack config options
type Config struct {
Composefiles []string
SkipInterpolation bool
}

// List holds docker stack ls options
type List struct {
Format string
Expand Down
32 changes: 32 additions & 0 deletions cli/compose/loader/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ func mergeServices(base, override []types.ServiceConfig) ([]types.ServiceConfig,
reflect.TypeOf([]types.ServiceSecretConfig{}): mergeSlice(toServiceSecretConfigsMap, toServiceSecretConfigsSlice),
reflect.TypeOf([]types.ServiceConfigObjConfig{}): mergeSlice(toServiceConfigObjConfigsMap, toSServiceConfigObjConfigsSlice),
reflect.TypeOf(&types.UlimitsConfig{}): mergeUlimitsConfig,
reflect.TypeOf([]types.ServiceVolumeConfig{}): mergeSlice(toServiceVolumeConfigsMap, toServiceVolumeConfigsSlice),
reflect.TypeOf(types.ShellCommand{}): mergeShellCommand,
reflect.TypeOf(&types.ServiceNetworkConfig{}): mergeServiceNetworkConfig,
},
}
Expand Down Expand Up @@ -116,6 +118,18 @@ func toServicePortConfigsMap(s interface{}) (map[interface{}]interface{}, error)
return m, nil
}

func toServiceVolumeConfigsMap(s interface{}) (map[interface{}]interface{}, error) {
volumes, ok := s.([]types.ServiceVolumeConfig)
if !ok {
return nil, errors.Errorf("not a serviceVolumeConfig slice: %v", s)
}
m := map[interface{}]interface{}{}
for _, v := range volumes {
m[v.Target] = v
}
return m, nil
}

func toServiceSecretConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
s := []types.ServiceSecretConfig{}
for _, v := range m {
Expand Down Expand Up @@ -146,6 +160,16 @@ func toServicePortConfigsSlice(dst reflect.Value, m map[interface{}]interface{})
return nil
}

func toServiceVolumeConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
s := []types.ServiceVolumeConfig{}
for _, v := range m {
s = append(s, v.(types.ServiceVolumeConfig))
}
sort.Slice(s, func(i, j int) bool { return s[i].Target < s[j].Target })
dst.Set(reflect.ValueOf(s))
return nil
}

type tomapFn func(s interface{}) (map[interface{}]interface{}, error)
type writeValueFromMapFn func(reflect.Value, map[interface{}]interface{}) error

Expand Down Expand Up @@ -211,6 +235,14 @@ func mergeUlimitsConfig(dst, src reflect.Value) error {
return nil
}

//nolint: unparam
func mergeShellCommand(dst, src reflect.Value) error {
if src.Len() != 0 {
dst.Set(src)
}
return nil
}

//nolint: unparam
func mergeServiceNetworkConfig(dst, src reflect.Value) error {
if src.Interface() != reflect.Zero(reflect.TypeOf(src.Interface())).Interface() {
Expand Down
Loading