loader: fix overly aggressive project name validation

There were cases where the loader would infer a perfectly valid
project name by normalizing a directory, but would then re-validate
the un-normalized project directory and throw an error.

This moves all the validation into a single spot during the load,
which is the only point that it has all the context.

Some of this logic needs to be pushed out of here: it should NOT
be reading the config files, for example.

For now, this resolves the main issues.

Signed-off-by: Milas Bowman <[email protected]>

Author: milas

cli/options.go

@@ -103,11 +103,8 @@ func WithName(name string) ProjectOptionsFn {
 		// however, on the options object, the name is optional: if unset,
 		// a name will be inferred by the loader, so it's legal to set the
 		// name to an empty string here
-		if name != "" {
-			normalized := loader.NormalizeProjectName(name)
-			if err := loader.CheckOriginalProjectNameIsNormalized(name, normalized); err != nil {
-				return err
-			}
+		if name != loader.NormalizeProjectName(name) {
+			return loader.InvalidProjectNameErr(name)
 		}
 		o.Name = name
 		return nil
@@ -439,7 +436,10 @@ func withNamePrecedenceLoad(absWorkingDir string, options *ProjectOptions) func(
 		} else if nameFromEnv, ok := options.Environment[consts.ComposeProjectName]; ok && nameFromEnv != "" {
 			opts.SetProjectName(nameFromEnv, true)
 		} else {
-			opts.SetProjectName(filepath.Base(absWorkingDir), false)
+			opts.SetProjectName(
+				loader.NormalizeProjectName(filepath.Base(absWorkingDir)),
+				false,
+			)
 		}
 	}
 }

cli/options_test.go

@@ -82,9 +82,9 @@ func TestProjectName(t *testing.T) {
 		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`)
+		// root directory will resolve to an empty project name since there
+		// IS no directory name!
+		assert.ErrorContains(t, err, `project name must not be empty`)
 		assert.Assert(t, p == nil)
 	})
 
@@ -156,6 +156,14 @@ func TestProjectName(t *testing.T) {
 		assert.Equal(t, p.Name, "simple")
 	})
 
+	t.Run("by compose file parent dir special", func(t *testing.T) {
+		opts, err := NewProjectOptions([]string{"testdata/UNNORMALIZED PATH/compose.yaml"})
+		assert.NilError(t, err)
+		p, err := ProjectFromOptions(opts)
+		assert.NilError(t, err)
+		assert.Equal(t, p.Name, "unnormalizedpath")
+	})
+
 	t.Run("by COMPOSE_PROJECT_NAME", func(t *testing.T) {
 		os.Setenv("COMPOSE_PROJECT_NAME", "my_project_from_env") //nolint:errcheck
 		defer os.Unsetenv("COMPOSE_PROJECT_NAME")                //nolint:errcheck

cli/testdata/UNNORMALIZED PATH/compose.yaml

@@ -0,0 +1,3 @@
+services:
+  nginx:
+    image: nginx

loader/loader.go

@@ -64,16 +64,13 @@ 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.projectName = name
 	o.projectNameImperativelySet = imperativelySet
-	o.projectNameBeforeNormalization = name
 }
 
 func (o Options) GetProjectName() (string, bool) {
@@ -267,49 +264,54 @@ 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 InvalidProjectNameErr(v string) error {
+	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]", v,
+	)
 }
 
 func projectName(details types.ConfigDetails, opts *Options) (string, error) {
 	projectName, projectNameImperativelySet := opts.GetProjectName()
-	projectNameBeforeNormalization := opts.projectNameBeforeNormalization
 
-	var pjNameFromConfigFile string
-
-	for _, configFile := range details.ConfigFiles {
-		yml, err := ParseYAML(configFile.Content)
-		if err != nil {
-			return "", nil
+	// if user did NOT provide a name explicitly, then see if one is defined
+	// in any of the config files
+	if !projectNameImperativelySet {
+		var pjNameFromConfigFile string
+		for _, configFile := range details.ConfigFiles {
+			yml, err := ParseYAML(configFile.Content)
+			if err != nil {
+				return "", nil
+			}
+			if val, ok := yml["name"]; ok && val != "" {
+				pjNameFromConfigFile = yml["name"].(string)
+			}
 		}
-		if val, ok := yml["name"]; ok && val != "" {
-			pjNameFromConfigFile = yml["name"].(string)
+		if !opts.SkipInterpolation {
+			interpolated, err := interp.Interpolate(
+				map[string]interface{}{"name": pjNameFromConfigFile},
+				*opts.Interpolate,
+			)
+			if err != nil {
+				return "", err
+			}
+			pjNameFromConfigFile = interpolated["name"].(string)
 		}
-	}
-	if !opts.SkipInterpolation {
-		interpolated, err := interp.Interpolate(map[string]interface{}{"name": pjNameFromConfigFile}, *opts.Interpolate)
-		if err != nil {
-			return "", err
+		pjNameFromConfigFile = NormalizeProjectName(pjNameFromConfigFile)
+		if pjNameFromConfigFile != "" {
+			projectName = pjNameFromConfigFile
 		}
-		pjNameFromConfigFile = interpolated["name"].(string)
 	}
-	pjNameFromConfigFileNormalized := NormalizeProjectName(pjNameFromConfigFile)
-	if !projectNameImperativelySet && pjNameFromConfigFileNormalized != "" {
-		projectName = pjNameFromConfigFileNormalized
-		projectNameBeforeNormalization = pjNameFromConfigFile
+
+	if projectName == "" {
+		return "", errors.New("project name must not be empty")
 	}
 
-	if err := CheckOriginalProjectNameIsNormalized(
-		projectNameBeforeNormalization, projectName); err != nil {
-		return "", err
+	if NormalizeProjectName(projectName) != projectName {
+		return "", InvalidProjectNameErr(projectName)
 	}
 
+	// TODO(milas): this should probably ALWAYS set (overriding any existing)
 	if _, ok := details.Environment[consts.ComposeProjectName]; !ok && projectName != "" {
 		details.Environment[consts.ComposeProjectName] = projectName
 	}