Repository: compose-spec/compose-go
Pull request #128 (2021-04-22 08:17:03)

Normalize absolute paths for working dir & compose files
These paths will be included in container labels, used to resolve compose files if required in further compose commands on running projects

Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>

gtardif: I'll test this in the downstream PR before merging...

Patch:
diff --git a/cli/options.go b/cli/options.go
index 4eff1e1..fcd6cf2 100644
--- a/cli/options.go
+++ b/cli/options.go
@@ -285,6 +285,13 @@ func getConfigPathsFromOptions(options *ProjectOptions) ([]string, error) {
 				logrus.Warnf("Found multiple config files with supported names: %s", strings.Join(candidates, ", "))
 				logrus.Warnf("Using %s", winner)
 			}
+			if !filepath.IsAbs(winner) {
+				fAbs, err := filepath.Abs(winner)
+				if err != nil {
+					return nil, err
+				}
+				winner = fAbs
+			}
 			return []string{winner}, nil
 		}
 		parent := filepath.Dir(pwd)
diff --git a/compatibility/services.go b/compatibility/services.go
index 39bca64..88f2fa7 100644
--- a/compatibility/services.go
+++ b/compatibility/services.go
@@ -668,8 +668,8 @@ func (c *AllowList) CheckSecurityOpt(service *types.ServiceConfig) {
 }
 
 func (c *AllowList) CheckShmSize(service *types.ServiceConfig) {
-	if !c.supported("services.shm_size") && service.ShmSize != 0 {
-		service.ShmSize = 0
+	if !c.supported("services.shm_size") && service.ShmSize != "" {
+		service.ShmSize = ""
 		c.Unsupported("services.shm_size")
 	}
 }
diff --git a/envfile/envfile.go b/envfile/envfile.go
new file mode 100644
index 0000000..839f04c
--- /dev/null
+++ b/envfile/envfile.go
@@ -0,0 +1,104 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package envfile
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"os"
+	"strings"
+	"unicode"
+	"unicode/utf8"
+
+	"github.com/compose-spec/compose-go/types"
+)
+
+const whiteSpaces = " \t"
+
+// ErrBadKey typed error for bad environment variable
+type ErrBadKey struct {
+	msg string
+}
+
+func (e ErrBadKey) Error() string {
+	return fmt.Sprintf("poorly formatted environment: %s", e.msg)
+}
+
+// Parse reads a file with environment variables enumerated by lines
+//
+// ``Environment variable names used by the utilities in the Shell and
+// Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase
+// letters, digits, and the '_' (underscore) from the characters defined in
+// Portable Character Set and do not begin with a digit. *But*, other
+// characters may be permitted by an implementation; applications shall
+// tolerate the presence of such names.''
+// -- http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
+//
+// As of #16585, it's up to application inside docker to validate or not
+// environment variables, that's why we just strip leading whitespace and
+// nothing more.
+// Converts ["key=value"] to {"key":"value"} but set unset keys - the ones with no "=" in them - to nil
+// We use this in cases where we need to distinguish between FOO=  and FOO
+// where the latter case just means FOO was mentioned but not given a value
+func Parse(filename string) (types.MappingWithEquals, error) {
+	vars := types.MappingWithEquals{}
+	fh, err := os.Open(filename)
+	if err != nil {
+		return vars, err
+	}
+	defer fh.Close()
+
+	scanner := bufio.NewScanner(fh)
+	currentLine := 0
+	utf8bom := []byte{0xEF, 0xBB, 0xBF}
+	for scanner.Scan() {
+		scannedBytes := scanner.Bytes()
+		if !utf8.Valid(scannedBytes) {
+			return vars, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine+1, scannedBytes)
+		}
+		// We trim UTF8 BOM
+		if currentLine == 0 {
+			scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
+		}
+		// trim the line from all leading whitespace first
+		line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace)
+		currentLine++
+		// line is not empty, and not starting with '#'
+		if len(line) > 0 && !strings.HasPrefix(line, "#") {
+			data := strings.SplitN(line, "=", 2)
+
+			// trim the front of a variable, but nothing else
+			variable := strings.TrimLeft(data[0], whiteSpaces)
+			if strings.ContainsAny(variable, whiteSpaces) {
+				return vars, ErrBadKey{fmt.Sprintf("variable '%s' contains whitespaces", variable)}
+			}
+			if len(variable) == 0 {
+				return vars, ErrBadKey{fmt.Sprintf("no variable name on line '%s'", line)}
+			}
+
+			if len(data) > 1 {
+				// pass the value through, no trimming
+				vars[variable] = &data[1]
+			} else {
+				// variable was not given a value but declared
+				vars[strings.TrimSpace(line)] = nil
+			}
+		}
+	}
+	return vars, scanner.Err()
+}
diff --git a/loader/loader.go b/loader/loader.go
index 65d3ab9..232cec0 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -27,13 +27,14 @@ import (
 	"strings"
 	"time"
 
+	"github.com/imdario/mergo"
+
+	"github.com/compose-spec/compose-go/envfile"
 	interp "github.com/compose-spec/compose-go/interpolation"
 	"github.com/compose-spec/compose-go/schema"
 	"github.com/compose-spec/compose-go/template"
 	"github.com/compose-spec/compose-go/types"
 	units "github.com/docker/go-units"
-	"github.com/imdario/mergo"
-	"github.com/joho/godotenv"
 	shellwords "github.com/mattn/go-shellwords"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
@@ -527,21 +528,11 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l
 	if len(serviceConfig.EnvFile) > 0 {
 		for _, file := range serviceConfig.EnvFile {
 			filePath := absPath(workingDir, file)
-			file, err := os.Open(filePath)
+			fileVars, err := envfile.Parse(filePath)
 			if err != nil {
 				return err
 			}
-			defer file.Close()
-			fileVars, err := godotenv.Parse(file)
-			if err != nil {
-				return err
-			}
-			env := types.MappingWithEquals{}
-			for k, v := range fileVars {
-				v := v
-				env[k] = &v
-			}
-			environment.OverrideBy(env.Resolve(lookupEnv).RemoveEmpty())
+			environment.OverrideBy(fileVars.Resolve(lookupEnv).RemoveEmpty())
 		}
 	}
 
diff --git a/loader/normalize.go b/loader/normalize.go
index 4ec1811..c3e9baf 100644
--- a/loader/normalize.go
+++ b/loader/normalize.go
@@ -29,12 +29,24 @@ import (
 
 // normalize compose project by moving deprecated attributes to their canonical position and injecting implicit defaults
 func normalize(project *types.Project) error {
+	absWorkingDir, err := filepath.Abs(project.WorkingDir)
+	if err != nil {
+		return err
+	}
+	project.WorkingDir = absWorkingDir
+
+	absComposeFiles, err := absComposeFiles(project.ComposeFiles)
+	if err != nil {
+		return err
+	}
+	project.ComposeFiles = absComposeFiles
+
 	// If not declared explicitly, Compose model involves an implicit "default" network
 	if _, ok := project.Networks["default"]; !ok {
 		project.Networks["default"] = types.NetworkConfig{}
 	}
 
-	err := relocateExternalName(project)
+	err = relocateExternalName(project)
 	if err != nil {
 		return err
 	}
@@ -94,6 +106,18 @@ func normalize(project *types.Project) error {
 	return nil
 }
 
+func absComposeFiles(composeFiles []string) ([]string, error) {
+	absComposeFiles := make([]string, len(composeFiles))
+	for i, composeFile := range composeFiles {
+		absComposefile, err := filepath.Abs(composeFile)
+		if err != nil {
+			return nil, err
+		}
+		absComposeFiles[i] = absComposefile
+	}
+	return absComposeFiles, nil
+}
+
 // Resources with no explicit name are actually named by their key in map
 func setNameFromKey(project *types.Project) {
 	for i, n := range project.Networks {
diff --git a/types/types.go b/types/types.go
index 752911a..3d445c2 100644
--- a/types/types.go
+++ b/types/types.go
@@ -155,7 +155,7 @@ type ServiceConfig struct {
 	Scale           int                              `yaml:",omitempty" json:"scale,omitempty"`
 	Secrets         []ServiceSecretConfig            `yaml:",omitempty" json:"secrets,omitempty"`
 	SecurityOpt     []string                         `mapstructure:"security_opt" yaml:"security_opt,omitempty" json:"security_opt,omitempty"`
-	ShmSize         UnitBytes                        `mapstructure:"shm_size" yaml:"shm_size,omitempty" json:"shm_size,omitempty"`
+	ShmSize         string                           `mapstructure:"shm_size" yaml:"shm_size,omitempty" json:"shm_size,omitempty"`
 	StdinOpen       bool                             `mapstructure:"stdin_open" yaml:"stdin_open,omitempty" json:"stdin_open,omitempty"`
 	StopGracePeriod *Duration                        `mapstructure:"stop_grace_period" yaml:"stop_grace_period,omitempty" json:"stop_grace_period,omitempty"`
 	StopSignal      string                           `mapstructure:"stop_signal" yaml:"stop_signal,omitempty" json:"stop_signal,omitempty"`


Test patch:
diff --git a/cli/options_test.go b/cli/options_test.go
index fb2dafd..b8101f6 100644
--- a/cli/options_test.go
+++ b/cli/options_test.go
@@ -77,7 +77,8 @@ func TestProjectComposefilesFromSetOfFiles(t *testing.T) {
 	assert.NilError(t, err)
 	p, err := ProjectFromOptions(opts)
 	assert.NilError(t, err)
-	assert.DeepEqual(t, p.ComposeFiles, []string{filepath.Join("testdata", "simple", "compose.yaml")})
+	absPath, _ := filepath.Abs(filepath.Join("testdata", "simple", "compose.yaml"))
+	assert.DeepEqual(t, p.ComposeFiles, []string{absPath})
 }
 
 func TestProjectComposefilesFromWorkingDir(t *testing.T) {
diff --git a/envfile/envfile_test.go b/envfile/envfile_test.go
new file mode 100644
index 0000000..b050df4
--- /dev/null
+++ b/envfile/envfile_test.go
@@ -0,0 +1,203 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package envfile
+
+import (
+	"bufio"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/compose-spec/compose-go/types"
+	is "gotest.tools/v3/assert/cmp"
+
+	"gotest.tools/v3/assert"
+)
+
+func tmpFileWithContent(content string, t *testing.T) string {
+	tmpFile, err := ioutil.TempFile("", "envfile-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer tmpFile.Close()
+
+	_, err = tmpFile.WriteString(content)
+	assert.NilError(t, err)
+	return tmpFile.Name()
+}
+
+// Test Parse for a file with a few well formatted lines
+func TestParseEnvFileGoodFile(t *testing.T) {
+	content := `foo=bar
+    baz=quux
+# comment
+
+_foobar=foobaz
+with.dots=working
+and_underscore=working too
+`
+	// Adding a newline + a line with pure whitespace.
+	// This is being done like this instead of the block above
+	// because it's common for editors to trim trailing whitespace
+	// from lines, which becomes annoying since that's the
+	// exact thing we need to test.
+	content += "\n    \t  "
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	lines, err := Parse(tmpFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedLines := types.MappingWithEquals{
+		"foo":            strPtr("bar"),
+		"baz":            strPtr("quux"),
+		"_foobar":        strPtr("foobaz"),
+		"with.dots":      strPtr("working"),
+		"and_underscore": strPtr("working too"),
+	}
+
+	assert.Check(t, is.DeepEqual(expectedLines, lines), "lines not equal to expectedLines")
+}
+
+func strPtr(s string) *string {
+	return &s
+}
+
+// Test Parse for an empty file
+func TestParseEnvFileEmptyFile(t *testing.T) {
+	tmpFile := tmpFileWithContent("", t)
+	defer os.Remove(tmpFile)
+
+	lines, err := Parse(tmpFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(lines) != 0 {
+		t.Fatal("lines not empty; expected empty")
+	}
+}
+
+// Test Parse for a non existent file
+func TestParseEnvFileNonExistentFile(t *testing.T) {
+	_, err := Parse("foo_bar_baz")
+	if err == nil {
+		t.Fatal("Parse succeeded; expected failure")
+	}
+	if _, ok := err.(*os.PathError); !ok {
+		t.Fatalf("Expected a PathError, got [%v]", err)
+	}
+}
+
+// Test Parse for a badly formatted file
+func TestParseEnvFileBadlyFormattedFile(t *testing.T) {
+	content := `foo=bar
+    f   =quux
+`
+
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	_, err := Parse(tmpFile)
+	if err == nil {
+		t.Fatalf("Expected an ErrBadKey, got nothing")
+	}
+	if _, ok := err.(ErrBadKey); !ok {
+		t.Fatalf("Expected an ErrBadKey, got [%v]", err)
+	}
+	expectedMessage := "poorly formatted environment: variable 'f   ' contains whitespaces"
+	if err.Error() != expectedMessage {
+		t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error())
+	}
+}
+
+// Test Parse for a file with a line exceeding bufio.MaxScanTokenSize
+func TestParseEnvFileLineTooLongFile(t *testing.T) {
+	content := strings.Repeat("a", bufio.MaxScanTokenSize+42)
+	content = fmt.Sprint("foo=", content)
+
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	_, err := Parse(tmpFile)
+	if err == nil {
+		t.Fatal("Parse succeeded; expected failure")
+	}
+}
+
+// Parse with a random file, pass through
+func TestParseEnvFileRandomFile(t *testing.T) {
+	content := `first line
+another invalid line`
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	_, err := Parse(tmpFile)
+	if err == nil {
+		t.Fatalf("Expected an ErrBadKey, got nothing")
+	}
+	if _, ok := err.(ErrBadKey); !ok {
+		t.Fatalf("Expected an ErrBadKey, got [%v]", err)
+	}
+	expectedMessage := "poorly formatted environment: variable 'first line' contains whitespaces"
+	if err.Error() != expectedMessage {
+		t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error())
+	}
+}
+
+// Parse with environment variable import definitions
+func TestParseEnvVariableDefinitionsFile(t *testing.T) {
+	content := `# comment=
+UNDEFINED_VAR
+DEFINED_VAR
+`
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	variables, err := Parse(tmpFile)
+	if nil != err {
+		t.Fatal("There must not be any error")
+	}
+	variables.Resolve(func(key string) (string, bool) {
+		if key == "DEFINED_VAR" {
+			return "VALUE", true
+		}
+		return "", false
+	})
+
+	if variables["DEFINED_VAR"] == nil || *variables["DEFINED_VAR"] != "VALUE" {
+		t.Fatal("the DEFINED_VAR variable is not properly imported as the first variable (but it is the only one to import)")
+	}
+}
+
+// Parse with empty variable name
+func TestParseEnvVariableWithNoNameFile(t *testing.T) {
+	content := `# comment=
+=blank variable names are an error case
+`
+	tmpFile := tmpFileWithContent(content, t)
+	defer os.Remove(tmpFile)
+
+	_, err := Parse(tmpFile)
+	if nil == err {
+		t.Fatal("if a variable has no name parsing an environment file must fail")
+	}
+}
diff --git a/loader/loader_test.go b/loader/loader_test.go
index 4ded0e0..7917586 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -556,7 +556,6 @@ services:
         soft: $theint
     privileged: $thebool
     read_only: $thebool
-    shm_size: 2gb
     stdin_open: ${thebool}
     tty: $thebool
     volumes:
@@ -648,7 +647,6 @@ networks:
 				},
 				Privileged: true,
 				ReadOnly:   true,
-				ShmSize:    types.UnitBytes(2 * 1024 * 1024 * 1024),
 				StdinOpen:  true,
 				Tty:        true,
 				Volumes: []types.ServiceVolumeConfig{
diff --git a/loader/normalize_test.go b/loader/normalize_test.go
index d145fb7..79ffd7a 100644
--- a/loader/normalize_test.go
+++ b/loader/normalize_test.go
@@ -57,7 +57,8 @@ func TestNormalizeNetworkNames(t *testing.T) {
 		},
 	}
 
-	expected := strings.ReplaceAll(`services:
+	absFolder, _ := filepath.Abs("/some/path/foo")
+	expected := strings.ReplaceAll(strings.ReplaceAll(`services:
   foo:
     build:
       context: /some/path/foo
@@ -77,7 +78,7 @@ networks:
     name: CustomName
   mynet:
     name: myProject_mynet
-`, "/", string(filepath.Separator))
+`, "/some/path/foo", absFolder), "/", string(filepath.Separator))
 	err := normalize(&project)
 	assert.NilError(t, err)
 	marshal, err := yaml.Marshal(project)
@@ -85,6 +86,28 @@ networks:
 	assert.DeepEqual(t, expected, string(marshal))
 }
 
+func TestNormalizeAbsolutePaths(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)
+	assert.NilError(t, err)
+	assert.DeepEqual(t, expected, project)
+}
+
 func TestNormalizeVolumes(t *testing.T) {
 	project := types.Project{
 		Name:     "myProject",
@@ -101,6 +124,7 @@ func TestNormalizeVolumes(t *testing.T) {
 		},
 	}
 
+	absCwd, _ := filepath.Abs(".")
 	expected := types.Project{
 		Name:     "myProject",
 		Networks: types.Networks{"default": {Name: "myProject_default"}},
@@ -114,6 +138,8 @@ func TestNormalizeVolumes(t *testing.T) {
 				Name: "CustomName",
 			},
 		},
+		WorkingDir:   absCwd,
+		ComposeFiles: []string{},
 	}
 	err := normalize(&project)
 	assert.NilError(t, err)