Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use '-' as default separator and support of compatibility mode #294

Merged
merged 2 commits into from Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion cli/options.go
Expand Up @@ -350,7 +350,8 @@ func ProjectFromOptions(options *ProjectOptions) (*types.Project, error) {

options.loadOptions = append(options.loadOptions,
withNamePrecedenceLoad(absWorkingDir, options),
withConvertWindowsPaths(options))
withConvertWindowsPaths(options),
withSeparator(options))

project, err := loader.Load(types.ConfigDetails{
ConfigFiles: configs,
Expand Down Expand Up @@ -384,6 +385,15 @@ func withConvertWindowsPaths(options *ProjectOptions) func(*loader.Options) {
}
}

// withSeparator defines loader.Options separator used to define resource names
func withSeparator(options *ProjectOptions) func(*loader.Options) {
return func(o *loader.Options) {
if utils.StringToBool(options.Environment["COMPOSE_COMPATIBILITY"]) {
o.Separator = loader.CompatibilitySeparator
}
Comment on lines +391 to +393
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that this only fixes docker/compose#9618 when passing an environment variable. In the case of Docker Compose getting a --compatibility from the command line I'm not sure that will work (didn't test though).

Given this: https://github.com/docker/compose/blob/v2/cmd/compose/compose.go#L162-L169

Looks like compose just evaluates the command line option after this code is already evaluated.
One solution would be checking the command line option before and setting the EnvVar before calling ProjectFromOptions in the Docker Compose repo for this PR to work properly.

}
}

// getConfigPathsFromOptions retrieves the config files for project based on project options
func getConfigPathsFromOptions(options *ProjectOptions) ([]string, error) {
if len(options.ConfigPaths) != 0 {
Expand Down
26 changes: 26 additions & 0 deletions cli/options_test.go
Expand Up @@ -237,3 +237,29 @@ func TestEnvMap(t *testing.T) {
m = utils.GetAsEqualsMap(l)
assert.Equal(t, m["foo"], "bar")
}

func TestWithSeparator(t *testing.T) {
t.Run("With default separator", func(t *testing.T) {
opts, err := NewProjectOptions([]string{
"testdata/simple/compose-with-network-and-volume.yaml",
}, WithName("my-project"))
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Networks["test-network"].Name, "my-project-test-network")

})

t.Run("With compatibility separator", func(t *testing.T) {
os.Setenv("COMPOSE_COMPATIBILITY", "true") //nolint:errcheck
defer os.Unsetenv("COMPOSE_COMPATIBILITY") //nolint:errcheck
glours marked this conversation as resolved.
Show resolved Hide resolved
opts, err := NewProjectOptions([]string{
"testdata/simple/compose-with-network-and-volume.yaml",
}, WithName("my-project"), WithOsEnv)
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Networks["test-network"].Name, "my-project_test-network")

})
}
11 changes: 11 additions & 0 deletions cli/testdata/simple/compose-with-network-and-volume.yaml
@@ -0,0 +1,11 @@
services:
simple:
image: nginx
networks:
- test-network
volumes:
- test-volume
networks:
test-network:
volumes:
test-volume:
10 changes: 9 additions & 1 deletion loader/loader.go
Expand Up @@ -43,6 +43,11 @@ import (
"gopkg.in/yaml.v2"
)

const (
DefaultSeparator = "-"
CompatibilitySeparator = "_"
)

// Options supported by Load
type Options struct {
// Skip schema validation
Expand All @@ -67,6 +72,8 @@ type Options struct {
projectName string
// Indicates when the projectName was imperatively set or guessed from path
projectNameImperativelySet bool
// Set separator used for naming resources
Separator string
}

func (o *Options) SetProjectName(name string, imperativelySet bool) {
Expand Down Expand Up @@ -155,6 +162,7 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
LookupValue: configDetails.LookupEnv,
TypeCastMapping: interpolateTypeCastMapping,
},
Separator: DefaultSeparator,
}

for _, op := range options {
Expand Down Expand Up @@ -223,7 +231,7 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
}

if !opts.SkipNormalization {
err = normalize(project, opts.ResolvePaths)
err = normalize(project, opts.ResolvePaths, opts.Separator)
if err != nil {
return nil, err
}
Expand Down
14 changes: 7 additions & 7 deletions loader/normalize.go
Expand Up @@ -28,7 +28,7 @@ import (
)

// normalize compose project by moving deprecated attributes to their canonical position and injecting implicit defaults
func normalize(project *types.Project, resolvePaths bool) error {
func normalize(project *types.Project, resolvePaths bool, separator string) error {
absWorkingDir, err := filepath.Abs(project.WorkingDir)
if err != nil {
return err
Expand Down Expand Up @@ -110,7 +110,7 @@ func normalize(project *types.Project, resolvePaths bool) error {
project.Services[i] = s
}

setNameFromKey(project)
setNameFromKey(project, separator)

return nil
}
Expand Down Expand Up @@ -143,31 +143,31 @@ func absComposeFiles(composeFiles []string) ([]string, error) {
}

// Resources with no explicit name are actually named by their key in map
func setNameFromKey(project *types.Project) {
func setNameFromKey(project *types.Project, separator string) {
for i, n := range project.Networks {
if n.Name == "" {
n.Name = fmt.Sprintf("%s_%s", project.Name, i)
n.Name = fmt.Sprintf("%s%s%s", project.Name, separator, i)
project.Networks[i] = n
}
}

for i, v := range project.Volumes {
if v.Name == "" {
v.Name = fmt.Sprintf("%s_%s", project.Name, i)
v.Name = fmt.Sprintf("%s%s%s", project.Name, separator, i)
project.Volumes[i] = v
}
}

for i, c := range project.Configs {
if c.Name == "" {
c.Name = fmt.Sprintf("%s_%s", project.Name, i)
c.Name = fmt.Sprintf("%s%s%s", project.Name, separator, i)
project.Configs[i] = c
}
}

for i, s := range project.Secrets {
if s.Name == "" {
s.Name = fmt.Sprintf("%s_%s", project.Name, i)
s.Name = fmt.Sprintf("%s%s%s", project.Name, separator, i)
project.Secrets[i] = s
}
}
Expand Down
42 changes: 32 additions & 10 deletions loader/normalize_test.go
Expand Up @@ -73,16 +73,16 @@ services:
default: null
networks:
default:
name: myProject_default
name: myProject-default
myExternalnet:
name: myExternalnet
external: true
myNamedNet:
name: CustomName
mynet:
name: myProject_mynet
name: myProject-mynet
`
err := normalize(&project, false)
err := normalize(&project, false, DefaultSeparator)
assert.NilError(t, err)
marshal, err := yaml.Marshal(project)
assert.NilError(t, err)
Expand Down Expand Up @@ -116,9 +116,9 @@ services:
default: null
networks:
default:
name: myProject_default
name: myProject-default
`, filepath.Join(wd, "testdata"))
err := normalize(&project, true)
err := normalize(&project, true, DefaultSeparator)
assert.NilError(t, err)
marshal, err := yaml.Marshal(project)
assert.NilError(t, err)
Expand All @@ -138,11 +138,11 @@ func TestNormalizeAbsolutePaths(t *testing.T) {

expected := types.Project{
Name: "myProject",
Networks: types.Networks{"default": {Name: "myProject_default"}},
Networks: types.Networks{"default": {Name: "myProject-default"}},
WorkingDir: absWorkingDir,
ComposeFiles: []string{absComposeFile, absOverrideFile},
}
err := normalize(&project, false)
err := normalize(&project, false, DefaultSeparator)
assert.NilError(t, err)
assert.DeepEqual(t, expected, project)
}
Expand All @@ -166,21 +166,43 @@ func TestNormalizeVolumes(t *testing.T) {
absCwd, _ := filepath.Abs(".")
expected := types.Project{
Name: "myProject",
Networks: types.Networks{"default": {Name: "myProject_default"}},
Networks: types.Networks{"default": {Name: "myProject-default"}},
Volumes: types.Volumes{
"myExternalVol": {
Name: "myExternalVol",
External: types.External{External: true},
},
"myvol": {Name: "myProject_myvol"},
"myvol": {Name: "myProject-myvol"},
"myNamedVol": {
Name: "CustomName",
},
},
WorkingDir: absCwd,
ComposeFiles: []string{},
}
err := normalize(&project, false)
err := normalize(&project, false, DefaultSeparator)
assert.NilError(t, err)
assert.DeepEqual(t, expected, project)
}

func TestNormalizeWithCompatibilitySeparator(t *testing.T) {
project := types.Project{
Name: "myProject",
WorkingDir: "testdata",
Networks: types.Networks{},
ComposeFiles: []string{filepath.Join("testdata", "simple", "compose.yaml"), filepath.Join("testdata", "simple", "compose-with-overrides.yaml")},
}
absWorkingDir, _ := filepath.Abs("testdata")
absComposeFile, _ := filepath.Abs(filepath.Join("testdata", "simple", "compose.yaml"))
absOverrideFile, _ := filepath.Abs(filepath.Join("testdata", "simple", "compose-with-overrides.yaml"))

expected := types.Project{
Name: "myProject",
Networks: types.Networks{"default": {Name: "myProject_default"}},
WorkingDir: absWorkingDir,
ComposeFiles: []string{absComposeFile, absOverrideFile},
}
err := normalize(&project, false, "_")
assert.NilError(t, err)
assert.DeepEqual(t, expected, project)
}