Skip to content

Commit

Permalink
Apply package name constraints to env vars, dirs
Browse files Browse the repository at this point in the history
Applies the constraints articulated in compose-spec/compose-spec#314 for
project names from `COMPOSE_PROJECT_NAME`, the project dir, or the
working dir.

Before this change, the constraints applied only when using `docker
compose -p` or the `name:` config file property. Non-normalized project
names from `COMPOSE_PROJECT_NAME`, the project dir, or the working dir
were still allowed, which `docker compose` would then normalize.

The constraints are enforced by:

- `cli.ProjectFromOptions()` setting `withNamePrecedenceLoad()` as an
  option to `loader.Load()`.
- `loader.Load()` applies `withNamePrecedenceLoad()`, which tries to get
  the project name from the command line options, then
  `COMPOSE_PROJECT_NAME`, then the project or working dir.
- `withNamePrecedenceLoad()` calls `loader.Options.SetProjectName()`,
  which now also saves the original name as well as the normalized name.
- `loader.Load()` then calls `loader.projectName()`, which reads the
  project name from the config file(s) if it's not already set.
- `loader.projectName()` then applies the new
  `CheckOriginalProjectNameIsNormalized()` function extracted from
  `cli.WithName()`.

Only a handful of test cases required updating in `cli/options_test.go`.
These test cases were checking that invalid project names were rejected
by `cli.WithName()`, but that `cli.ProjectFromOptions()` would normalize
them. Now `cli.ProjectFromOptions()` rejects them with the same error.

There's no need for new test cases specifically for
`COMPOSE_PROJECT_NAME` or directory-based project names, because the
`CheckOriginalProjectNameIsNormalized()` function is applied regardless
of the source. Other existing test cases are validating that
`cli.ProjectFromOptions` does also use `COMPOSE_PROJECT_NAME` or
directory names when necessary.

Closes compose-spec#363.

Signed-off-by: Mike Bland <[email protected]>
  • Loading branch information
mbland committed Mar 3, 2023
1 parent 37ba724 commit b22d7d5
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 16 deletions.
7 changes: 3 additions & 4 deletions cli/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package cli

import (
"bytes"
"fmt"
"io"
"os"
"path/filepath"
Expand Down Expand Up @@ -64,9 +63,9 @@ func NewProjectOptions(configs []string, opts ...ProjectOptionsFn) (*ProjectOpti
// WithName defines ProjectOptions' name
func WithName(name string) ProjectOptionsFn {
return func(o *ProjectOptions) error {
if name != loader.NormalizeProjectName(name) {
return fmt.Errorf("%q is not a valid project name: it must contain "+
"only characters from [a-z0-9_-] and start with [a-z0-9]", name)
normalized := loader.NormalizeProjectName(name)
if err := loader.CheckOriginalProjectNameIsNormalized(name, normalized); err != nil {
return err
}
o.Name = name
return nil
Expand Down
18 changes: 9 additions & 9 deletions cli/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ func TestProjectName(t *testing.T) {
}))
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Name, "my_project")
assert.ErrorContains(t, err, `"-my_project" is not a valid project name`)
assert.Assert(t, p == nil)
})

t.Run("by name start with invalid char '_'", func(t *testing.T) {
Expand All @@ -74,8 +74,8 @@ func TestProjectName(t *testing.T) {
}))
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Name, "my_project")
assert.ErrorContains(t, err, `"_my_project" is not a valid project name`)
assert.Assert(t, p == nil)
})

t.Run("by name contains dots", func(t *testing.T) {
Expand All @@ -87,21 +87,21 @@ func TestProjectName(t *testing.T) {
}))
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Name, "wwwmyproject")
assert.ErrorContains(t, err, `"www.my.project" is not a valid project name`)
assert.Assert(t, p == nil)
})

t.Run("by name uppercase", func(t *testing.T) {
_, err := NewProjectOptions([]string{"testdata/simple/compose.yaml"}, WithName("MY_PROJECT"))
assert.ErrorContains(t, err, `"MY_PROJECT" is not a valid project name`)

opts, err := NewProjectOptions([]string{"testdata/simple/compose.yaml"}, WithEnv([]string{
fmt.Sprintf("%s=%s", consts.ComposeProjectName, "_my_project"),
fmt.Sprintf("%s=%s", consts.ComposeProjectName, "MY_PROJECT"),
}))
assert.NilError(t, err)
p, err := ProjectFromOptions(opts)
assert.NilError(t, err)
assert.Equal(t, p.Name, "my_project")
assert.ErrorContains(t, err, `"MY_PROJECT" is not a valid project name`)
assert.Assert(t, p == nil)
})

t.Run("by working dir", func(t *testing.T) {
Expand Down
25 changes: 22 additions & 3 deletions loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,16 @@ type Options struct {
projectName string
// Indicates when the projectName was imperatively set or guessed from path
projectNameImperativelySet bool
// The value of projectName before normalization
projectNameBeforeNormalization string
// Profiles set profiles to enable
Profiles []string
}

func (o *Options) SetProjectName(name string, imperativelySet bool) {
o.projectName = NormalizeProjectName(name)
o.projectNameImperativelySet = imperativelySet
o.projectNameBeforeNormalization = name
}

func (o Options) GetProjectName() (string, bool) {
Expand Down Expand Up @@ -264,8 +267,18 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
return project, err
}

func CheckOriginalProjectNameIsNormalized(original, normalized string) error {
if original != normalized {
return fmt.Errorf("%q is not a valid project name: it must contain only "+
"characters from [a-z0-9_-] and start with [a-z0-9]", original)
}
return nil
}

func projectName(details types.ConfigDetails, opts *Options) (string, error) {
projectName, projectNameImperativelySet := opts.GetProjectName()
projectNameBeforeNormalization := opts.projectNameBeforeNormalization

var pjNameFromConfigFile string

for _, configFile := range details.ConfigFiles {
Expand All @@ -284,9 +297,15 @@ func projectName(details types.ConfigDetails, opts *Options) (string, error) {
}
pjNameFromConfigFile = interpolated["name"].(string)
}
pjNameFromConfigFile = NormalizeProjectName(pjNameFromConfigFile)
if !projectNameImperativelySet && pjNameFromConfigFile != "" {
projectName = pjNameFromConfigFile
pjNameFromConfigFileNormalized := NormalizeProjectName(pjNameFromConfigFile)
if !projectNameImperativelySet && pjNameFromConfigFileNormalized != "" {
projectName = pjNameFromConfigFileNormalized
projectNameBeforeNormalization = pjNameFromConfigFile
}

if err := CheckOriginalProjectNameIsNormalized(
projectNameBeforeNormalization, projectName); err != nil {
return "", err
}

if _, ok := details.Environment[consts.ComposeProjectName]; !ok && projectName != "" {
Expand Down

0 comments on commit b22d7d5

Please sign in to comment.