Merge pull request #364 from mbland/project-name-uniform-enforcement

Author: ndeloof

cli/options.go

@@ -18,7 +18,6 @@ package cli
 
 import (
 	"bytes"
-	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -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

cli/options_test.go

@@ -52,6 +52,23 @@ func TestProjectName(t *testing.T) {
 		assert.Equal(t, p.Name, "42my_project_env")
 	})
 
+	t.Run("by name must not be empty", func(t *testing.T) {
+		_, err := NewProjectOptions([]string{"testdata/simple/compose.yaml"}, WithName(""))
+		assert.ErrorContains(t, err, `project name must not be empty`)
+	})
+
+	t.Run("by name must not come from root directory", func(t *testing.T) {
+		opts, err := NewProjectOptions([]string{"testdata/simple/compose.yaml"},
+			WithWorkingDirectory("/"))
+		assert.NilError(t, err)
+		p, err := ProjectFromOptions(opts)
+
+		// On macOS and Linux, the message will start with "/". On Windows, it will
+		// start with "\\\\". So we leave that part of the error off here.
+		assert.ErrorContains(t, err, `is not a valid project name`)
+		assert.Assert(t, p == nil)
+	})
+
 	t.Run("by name start with invalid char '-'", 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`)
@@ -61,8 +78,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) {
@@ -74,8 +91,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) {
@@ -87,21 +104,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) {

cli/options_windows_test.go

@@ -28,13 +28,13 @@ func TestConvertWithEnvVar(t *testing.T) {
 	defer os.Unsetenv("COMPOSE_CONVERT_WINDOWS_PATHS")
 	opts, _ := NewProjectOptions([]string{"testdata/simple/compose-with-paths.yaml"},
 		WithOsEnv,
-		WithWorkingDirectory("C:\\\\"))
+		WithWorkingDirectory("C:\\project-dir\\"))
 
 	p, err := ProjectFromOptions(opts)
 
 	assert.NilError(t, err)
 	assert.Equal(t, len(p.Services[0].Volumes), 3)
 	assert.Equal(t, p.Services[0].Volumes[0].Source, "/c/docker/project")
-	assert.Equal(t, p.Services[0].Volumes[1].Source, "/c/relative")
-	assert.Equal(t, p.Services[0].Volumes[2].Source, "/c/relative2")
+	assert.Equal(t, p.Services[0].Volumes[1].Source, "/c/project-dir/relative")
+	assert.Equal(t, p.Services[0].Volumes[2].Source, "/c/project-dir/relative2")
 }

loader/full-struct_test.go

@@ -1050,6 +1050,7 @@ func fullExampleJSON(workingDir, homeDir string) string {
       "external": false
     }
   },
+  "name": "full_example_project_name",
   "networks": {
     "external-network": {
       "name": "external-network",

loader/loader.go

@@ -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) {
@@ -264,8 +267,20 @@ 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)
+	} else if normalized == "" {
+		return fmt.Errorf("project name must not be empty")
+	}
+	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 {
@@ -284,9 +299,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 != "" {