From e112eeea4d96cee562c56c88058af147127686b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 20 Aug 2025 14:55:28 +0200 Subject: [PATCH] opts: deprecate ParseEnvFile It was a wrapper around kvfile.Load, which should be used instead. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit e650803f091a1e6fc30c5d71b2aae743c22a73db) Signed-off-by: Sebastiaan van Stijn --- {opts => cli/compose/loader}/envfile.go | 14 ++++++++------ {opts => cli/compose/loader}/envfile_test.go | 10 +++++----- cli/compose/loader/loader.go | 2 +- opts/env.go | 11 ++++++----- opts/envfile_deprecated.go | 14 ++++++++++++++ 5 files changed, 34 insertions(+), 17 deletions(-) rename {opts => cli/compose/loader}/envfile.go (54%) rename {opts => cli/compose/loader}/envfile_test.go (79%) create mode 100644 opts/envfile_deprecated.go diff --git a/opts/envfile.go b/cli/compose/loader/envfile.go similarity index 54% rename from opts/envfile.go rename to cli/compose/loader/envfile.go index 3a16e6c189bc..6430f4b5f5f8 100644 --- a/opts/envfile.go +++ b/cli/compose/loader/envfile.go @@ -1,4 +1,4 @@ -package opts +package loader import ( "os" @@ -6,19 +6,21 @@ import ( "github.com/docker/cli/pkg/kvfile" ) -// ParseEnvFile reads a file with environment variables enumerated by lines +// parseEnvFile 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 +// 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 +// As of [moby-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. -func ParseEnvFile(filename string) ([]string, error) { +// +// [IEEE Std 1003.1-2001]: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html +// [moby-16585]: https://github.com/moby/moby/issues/16585 +func parseEnvFile(filename string) ([]string, error) { return kvfile.Parse(filename, os.LookupEnv) } diff --git a/opts/envfile_test.go b/cli/compose/loader/envfile_test.go similarity index 79% rename from opts/envfile_test.go rename to cli/compose/loader/envfile_test.go index 2c01de4fd520..723b1e2269e7 100644 --- a/opts/envfile_test.go +++ b/cli/compose/loader/envfile_test.go @@ -1,4 +1,4 @@ -package opts +package loader import ( "os" @@ -17,13 +17,13 @@ func tmpFileWithContent(t *testing.T, content string) string { return fileName } -// Test ParseEnvFile for a non existent file +// Test parseEnvFile for a non existent file func TestParseEnvFileNonExistentFile(t *testing.T) { - _, err := ParseEnvFile("no_such_file") + _, err := parseEnvFile("no_such_file") assert.Check(t, is.ErrorType(err, os.IsNotExist)) } -// ParseEnvFile with environment variable import definitions +// parseEnvFile with environment variable import definitions func TestParseEnvVariableDefinitionsFile(t *testing.T) { content := `# comment= UNDEFINED_VAR @@ -32,7 +32,7 @@ DEFINED_VAR tmpFile := tmpFileWithContent(t, content) t.Setenv("DEFINED_VAR", "defined-value") - variables, err := ParseEnvFile(tmpFile) + variables, err := parseEnvFile(tmpFile) assert.NilError(t, err) expectedLines := []string{"DEFINED_VAR=defined-value"} diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 839a258f0b6e..3b788ca048f3 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -468,7 +468,7 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l for _, file := range serviceConfig.EnvFile { filePath := absPath(workingDir, file) - fileVars, err := opts.ParseEnvFile(filePath) + fileVars, err := parseEnvFile(filePath) if err != nil { return err } diff --git a/opts/env.go b/opts/env.go index 675ddda96229..2a21394ba5ba 100644 --- a/opts/env.go +++ b/opts/env.go @@ -7,13 +7,14 @@ import ( ) // ValidateEnv validates an environment variable and returns it. -// If no value is specified, it obtains its value from the current environment +// If no value is specified, it obtains its value from the current environment. // -// As on ParseEnvFile and related to #16585, environment variable names -// are not validated, and it's up to the application inside the container -// to validate them or not. +// Environment variable names are not validated, and it's up to the application +// inside the container to validate them (see [moby-16585]). The only validation +// here is to check if name is empty, per [moby-25099]. // -// The only validation here is to check if name is empty, per #25099 +// [moby-16585]: https://github.com/moby/moby/issues/16585 +// [moby-25099]: https://github.com/moby/moby/issues/25099 func ValidateEnv(val string) (string, error) { k, _, hasValue := strings.Cut(val, "=") if k == "" { diff --git a/opts/envfile_deprecated.go b/opts/envfile_deprecated.go new file mode 100644 index 000000000000..f2f10b1816fe --- /dev/null +++ b/opts/envfile_deprecated.go @@ -0,0 +1,14 @@ +package opts + +import ( + "os" + + "github.com/docker/cli/pkg/kvfile" +) + +// ParseEnvFile reads a file with environment variables enumerated by lines +// +// Deprecated: use [kvfile.Parse] and pass [os.LookupEnv] to lookup env-vars from the current environment. +func ParseEnvFile(filename string) ([]string, error) { + return kvfile.Parse(filename, os.LookupEnv) +}