From 00838ba403e40f5832be473ecfcc04022abb4da9 Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Mon, 19 Feb 2024 18:25:00 +0100 Subject: [PATCH 1/3] Values without a backing env var should not be expanded --- cmd/backup/config_provider.go | 38 +++++++++++-- cmd/backup/expand.go | 100 ++++++++++++++++++++++++++++++++++ test/confd/02backup.env | 1 + test/confd/run.sh | 2 +- 4 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 cmd/backup/expand.go diff --git a/cmd/backup/config_provider.go b/cmd/backup/config_provider.go index 4225d708..8414305f 100644 --- a/cmd/backup/config_provider.go +++ b/cmd/backup/config_provider.go @@ -4,6 +4,7 @@ package main import ( + "bufio" "fmt" "os" "path/filepath" @@ -99,11 +100,7 @@ func loadConfigsFromEnvFiles(directory string) ([]*Config, error) { continue } p := filepath.Join(directory, item.Name()) - f, err := os.ReadFile(p) - if err != nil { - return nil, errwrap.Wrap(err, fmt.Sprintf("error reading %s", item.Name())) - } - envFile, err := godotenv.Unmarshal(os.ExpandEnv(string(f))) + envFile, err := source(p) if err != nil { return nil, errwrap.Wrap(err, fmt.Sprintf("error reading config file %s", p)) } @@ -125,3 +122,34 @@ func loadConfigsFromEnvFiles(directory string) ([]*Config, error) { return configs, nil } + +func source(path string) (map[string]string, error) { + f, err := os.Open(path) + if err != nil { + return nil, errwrap.Wrap(err, fmt.Sprintf("error opening %s", path)) + } + + result := map[string]string{} + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := scanner.Text() + line = expand(line, os.LookupEnv) + m, err := godotenv.Unmarshal(line) + if err != nil { + return nil, errwrap.Wrap(err, fmt.Sprintf("error sourcing %s", path)) + } + for key, value := range m { + currentValue, currentOk := os.LookupEnv(key) + defer func() { + if currentOk { + os.Setenv(key, currentValue) + return + } + os.Unsetenv(key) + }() + result[key] = value + os.Setenv(key, value) + } + } + return result, nil +} diff --git a/cmd/backup/expand.go b/cmd/backup/expand.go new file mode 100644 index 00000000..d143f839 --- /dev/null +++ b/cmd/backup/expand.go @@ -0,0 +1,100 @@ +// This code comes from golang standard library: +// https://github.com/golang/go/blob/0e85fd7561de869add933801c531bf25dee9561c/src/os/env.go#L16-L96 +// This file is explicitly excluded from the top repository license. + +// Copyright 2010 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// General environment variables. + +package main + +import "fmt" + +// Expand replaces ${var} or $var in the string based on the mapping function. +func expand(s string, mapping func(string) (string, bool)) string { + var buf []byte + // ${} is all ASCII, so bytes are fine for this operation. + i := 0 + for j := 0; j < len(s); j++ { + if s[j] == '$' && j+1 < len(s) { + if buf == nil { + buf = make([]byte, 0, 2*len(s)) + } + buf = append(buf, s[i:j]...) + shellNameInput := s[j+1:] + name, w := getShellName(shellNameInput) + if name == "" && w > 0 { + // Encountered invalid syntax; eat the + // characters. + } else if name == "" { + // Valid syntax, but $ was not followed by a + // name. Leave the dollar character untouched. + buf = append(buf, s[j]) + } else { + replacement, ok := mapping(name) + if ok { + buf = append(buf, replacement...) + } else { + // preserve enclosing {} + if shellNameInput[0] == '{' { + buf = append(buf, fmt.Sprintf("${%s}", name)...) + } else { + buf = append(buf, fmt.Sprintf("$%s", name)...) + } + } + } + j += w + i = j + 1 + } + } + if buf == nil { + return s + } + return string(buf) + s[i:] +} + +// isShellSpecialVar reports whether the character identifies a special +// shell variable such as $*. +func isShellSpecialVar(c uint8) bool { + switch c { + case '*', '#', '$', '@', '!', '?', '-', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9': + return true + } + return false +} + +// isAlphaNum reports whether the byte is an ASCII letter, number, or underscore +func isAlphaNum(c uint8) bool { + return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' +} + +// getShellName returns the name that begins the string and the number of bytes +// consumed to extract it. If the name is enclosed in {}, it's part of a ${} +// expansion and two more bytes are needed than the length of the name. +func getShellName(s string) (string, int) { + switch { + case s[0] == '{': + if len(s) > 2 && isShellSpecialVar(s[1]) && s[2] == '}' { + return s[1:2], 3 + } + // Scan to closing brace + for i := 1; i < len(s); i++ { + if s[i] == '}' { + if i == 1 { + return "", 2 // Bad syntax; eat "${}" + } + return s[1:i], i + 1 + } + } + return "", 1 // Bad syntax; eat "${" + case isShellSpecialVar(s[0]): + return s[0:1], 1 + } + // Scan alphanumerics. + var i int + for i = 0; i < len(s) && isAlphaNum(s[i]); i++ { + } + return s[:i], i +} diff --git a/test/confd/02backup.env b/test/confd/02backup.env index 4278acb4..09bab41a 100644 --- a/test/confd/02backup.env +++ b/test/confd/02backup.env @@ -1,2 +1,3 @@ NAME="other" BACKUP_CRON_EXPRESSION="*/1 * * * *" +BACKUP_FILENAME="override-$NAME.tar.gz" diff --git a/test/confd/run.sh b/test/confd/run.sh index f81407a5..b722542e 100755 --- a/test/confd/run.sh +++ b/test/confd/run.sh @@ -20,7 +20,7 @@ if [ ! -f "$LOCAL_DIR/conf.tar.gz" ]; then fi pass "Config from file was used." -if [ ! -f "$LOCAL_DIR/other.tar.gz" ]; then +if [ ! -f "$LOCAL_DIR/override-other.tar.gz" ]; then fail "Run on same schedule did not succeed." fi pass "Run on same schedule succeeded." From 12dbfa41d177f7d26a8fba42e58dc8553f9ec97e Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Mon, 19 Feb 2024 20:57:40 +0100 Subject: [PATCH 2/3] Add unit tests for sourcing behavior --- .github/workflows/unit.yml | 21 ++++++++++ cmd/backup/config_provider.go | 2 + cmd/backup/config_provider_test.go | 67 ++++++++++++++++++++++++++++++ cmd/backup/testdata/braces.env | 2 + cmd/backup/testdata/default.env | 2 + cmd/backup/testdata/expansion.env | 4 ++ 6 files changed, 98 insertions(+) create mode 100644 .github/workflows/unit.yml create mode 100644 cmd/backup/config_provider_test.go create mode 100644 cmd/backup/testdata/braces.env create mode 100644 cmd/backup/testdata/default.env create mode 100644 cmd/backup/testdata/expansion.env diff --git a/.github/workflows/unit.yml b/.github/workflows/unit.yml new file mode 100644 index 00000000..04b35ecc --- /dev/null +++ b/.github/workflows/unit.yml @@ -0,0 +1,21 @@ +name: Run Unit Tests + +on: + push: + branches: + - main + pull_request: + +jobs: + build: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + - name: Setup Go + uses: actions/setup-go@v4 + with: + go-version: '1.22.x' + - name: Install dependencies + run: go mod download + - name: Test with the Go CLI + run: go test -v ./... diff --git a/cmd/backup/config_provider.go b/cmd/backup/config_provider.go index 8414305f..0d33b185 100644 --- a/cmd/backup/config_provider.go +++ b/cmd/backup/config_provider.go @@ -123,6 +123,8 @@ func loadConfigsFromEnvFiles(directory string) ([]*Config, error) { return configs, nil } +// source tries to mimic the pre v2.37.0 behavior of calling +// `set +a; source $path; set -a` and returns the env vars as a map func source(path string) (map[string]string, error) { f, err := os.Open(path) if err != nil { diff --git a/cmd/backup/config_provider_test.go b/cmd/backup/config_provider_test.go new file mode 100644 index 00000000..ab033688 --- /dev/null +++ b/cmd/backup/config_provider_test.go @@ -0,0 +1,67 @@ +package main + +import ( + "os" + "reflect" + "testing" +) + +func TestSource(t *testing.T) { + tests := []struct { + name string + input string + expectError bool + expectedOutput map[string]string + }{ + { + "default", + "testdata/default.env", + false, + map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + }, + { + "not found", + "testdata/nope.env", + true, + nil, + }, + { + "braces", + "testdata/braces.env", + false, + map[string]string{ + "FOO": "${bar:-qux}", + "BAR": "xxx", + }, + }, + { + "expansion", + "testdata/expansion.env", + false, + map[string]string{ + "BAR": "xxx", + "FOO": "xxx", + "BAZ": "xxx", + "QUX": "yyy", + }, + }, + } + + os.Setenv("QUX", "yyy") + defer os.Unsetenv("QUX") + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := source(test.input) + if (err != nil) != test.expectError { + t.Errorf("Unexpected error value %v", err) + } + if !reflect.DeepEqual(test.expectedOutput, result) { + t.Errorf("Expected %v, got %v", test.expectedOutput, result) + } + }) + } +} diff --git a/cmd/backup/testdata/braces.env b/cmd/backup/testdata/braces.env new file mode 100644 index 00000000..a13a2639 --- /dev/null +++ b/cmd/backup/testdata/braces.env @@ -0,0 +1,2 @@ +FOO=${bar:-qux} +BAR=xxx diff --git a/cmd/backup/testdata/default.env b/cmd/backup/testdata/default.env new file mode 100644 index 00000000..214ea78c --- /dev/null +++ b/cmd/backup/testdata/default.env @@ -0,0 +1,2 @@ +FOO=bar +BAZ=qux diff --git a/cmd/backup/testdata/expansion.env b/cmd/backup/testdata/expansion.env new file mode 100644 index 00000000..cba0e6d0 --- /dev/null +++ b/cmd/backup/testdata/expansion.env @@ -0,0 +1,4 @@ +BAR=xxx +FOO=${BAR} +BAZ=$BAR +QUX=${QUX} From a6a43aa3183fd6a61a64d46f3c3621eac383537c Mon Sep 17 00:00:00 2001 From: Frederik Ring Date: Mon, 19 Feb 2024 21:31:21 +0100 Subject: [PATCH 3/3] Replace godotenv with shell lib --- cmd/backup/config_provider.go | 31 ++------- cmd/backup/config_provider_test.go | 3 +- cmd/backup/expand.go | 100 ----------------------------- cmd/backup/testdata/braces.env | 1 + go.mod | 3 +- go.sum | 6 +- 6 files changed, 15 insertions(+), 129 deletions(-) delete mode 100644 cmd/backup/expand.go diff --git a/cmd/backup/config_provider.go b/cmd/backup/config_provider.go index 0d33b185..62da12be 100644 --- a/cmd/backup/config_provider.go +++ b/cmd/backup/config_provider.go @@ -4,14 +4,14 @@ package main import ( - "bufio" + "context" "fmt" "os" "path/filepath" - "github.com/joho/godotenv" "github.com/offen/docker-volume-backup/internal/errwrap" "github.com/offen/envconfig" + "mvdan.cc/sh/shell" ) type configStrategy string @@ -126,32 +126,13 @@ func loadConfigsFromEnvFiles(directory string) ([]*Config, error) { // source tries to mimic the pre v2.37.0 behavior of calling // `set +a; source $path; set -a` and returns the env vars as a map func source(path string) (map[string]string, error) { - f, err := os.Open(path) + vars, err := shell.SourceFile(context.Background(), path) if err != nil { - return nil, errwrap.Wrap(err, fmt.Sprintf("error opening %s", path)) + return nil, errwrap.Wrap(err, "error sourcing conf file") } - result := map[string]string{} - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := scanner.Text() - line = expand(line, os.LookupEnv) - m, err := godotenv.Unmarshal(line) - if err != nil { - return nil, errwrap.Wrap(err, fmt.Sprintf("error sourcing %s", path)) - } - for key, value := range m { - currentValue, currentOk := os.LookupEnv(key) - defer func() { - if currentOk { - os.Setenv(key, currentValue) - return - } - os.Unsetenv(key) - }() - result[key] = value - os.Setenv(key, value) - } + for key, value := range vars { + result[key] = value.String() } return result, nil } diff --git a/cmd/backup/config_provider_test.go b/cmd/backup/config_provider_test.go index ab033688..234de675 100644 --- a/cmd/backup/config_provider_test.go +++ b/cmd/backup/config_provider_test.go @@ -33,8 +33,9 @@ func TestSource(t *testing.T) { "testdata/braces.env", false, map[string]string{ - "FOO": "${bar:-qux}", + "FOO": "qux", "BAR": "xxx", + "BAZ": "", }, }, { diff --git a/cmd/backup/expand.go b/cmd/backup/expand.go deleted file mode 100644 index d143f839..00000000 --- a/cmd/backup/expand.go +++ /dev/null @@ -1,100 +0,0 @@ -// This code comes from golang standard library: -// https://github.com/golang/go/blob/0e85fd7561de869add933801c531bf25dee9561c/src/os/env.go#L16-L96 -// This file is explicitly excluded from the top repository license. - -// Copyright 2010 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// General environment variables. - -package main - -import "fmt" - -// Expand replaces ${var} or $var in the string based on the mapping function. -func expand(s string, mapping func(string) (string, bool)) string { - var buf []byte - // ${} is all ASCII, so bytes are fine for this operation. - i := 0 - for j := 0; j < len(s); j++ { - if s[j] == '$' && j+1 < len(s) { - if buf == nil { - buf = make([]byte, 0, 2*len(s)) - } - buf = append(buf, s[i:j]...) - shellNameInput := s[j+1:] - name, w := getShellName(shellNameInput) - if name == "" && w > 0 { - // Encountered invalid syntax; eat the - // characters. - } else if name == "" { - // Valid syntax, but $ was not followed by a - // name. Leave the dollar character untouched. - buf = append(buf, s[j]) - } else { - replacement, ok := mapping(name) - if ok { - buf = append(buf, replacement...) - } else { - // preserve enclosing {} - if shellNameInput[0] == '{' { - buf = append(buf, fmt.Sprintf("${%s}", name)...) - } else { - buf = append(buf, fmt.Sprintf("$%s", name)...) - } - } - } - j += w - i = j + 1 - } - } - if buf == nil { - return s - } - return string(buf) + s[i:] -} - -// isShellSpecialVar reports whether the character identifies a special -// shell variable such as $*. -func isShellSpecialVar(c uint8) bool { - switch c { - case '*', '#', '$', '@', '!', '?', '-', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9': - return true - } - return false -} - -// isAlphaNum reports whether the byte is an ASCII letter, number, or underscore -func isAlphaNum(c uint8) bool { - return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' -} - -// getShellName returns the name that begins the string and the number of bytes -// consumed to extract it. If the name is enclosed in {}, it's part of a ${} -// expansion and two more bytes are needed than the length of the name. -func getShellName(s string) (string, int) { - switch { - case s[0] == '{': - if len(s) > 2 && isShellSpecialVar(s[1]) && s[2] == '}' { - return s[1:2], 3 - } - // Scan to closing brace - for i := 1; i < len(s); i++ { - if s[i] == '}' { - if i == 1 { - return "", 2 // Bad syntax; eat "${}" - } - return s[1:i], i + 1 - } - } - return "", 1 // Bad syntax; eat "${" - case isShellSpecialVar(s[0]): - return s[0:1], 1 - } - // Scan alphanumerics. - var i int - for i = 0; i < len(s) && isAlphaNum(s[i]); i++ { - } - return s[:i], i -} diff --git a/cmd/backup/testdata/braces.env b/cmd/backup/testdata/braces.env index a13a2639..11b5bdc9 100644 --- a/cmd/backup/testdata/braces.env +++ b/cmd/backup/testdata/braces.env @@ -1,2 +1,3 @@ FOO=${bar:-qux} BAR=xxx +BAZ=$NOPE diff --git a/go.mod b/go.mod index cedbb00a..2d87cace 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/docker/cli v24.0.9+incompatible github.com/docker/docker v24.0.7+incompatible github.com/gofrs/flock v0.8.1 - github.com/joho/godotenv v1.5.1 github.com/klauspost/compress v1.17.6 github.com/leekchan/timeutil v0.0.0-20150802142658-28917288c48d github.com/minio/minio-go/v7 v7.0.67 @@ -22,6 +21,7 @@ require ( golang.org/x/crypto v0.19.0 golang.org/x/oauth2 v0.17.0 golang.org/x/sync v0.6.0 + mvdan.cc/sh v2.6.4+incompatible ) require ( @@ -29,6 +29,7 @@ require ( github.com/cloudflare/circl v1.3.7 // indirect github.com/golang-jwt/jwt/v5 v5.2.0 // indirect github.com/golang/protobuf v1.5.3 // indirect + golang.org/x/term v0.17.0 // indirect golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.31.0 // indirect diff --git a/go.sum b/go.sum index d4d4bacd..6f1464d8 100644 --- a/go.sum +++ b/go.sum @@ -443,8 +443,6 @@ github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1: github.com/inconshreveable/mousetrap v1.0.1/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jarcoal/httpmock v1.2.0 h1:gSvTxxFR/MEMfsGrvRbdfpRUMBStovlSRLw0Ep1bwwc= github.com/jarcoal/httpmock v1.2.0/go.mod h1:oCoTsnAz4+UoOUIf5lJOWV2QQIW5UoeUI6aM2YnWAZk= -github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= -github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= @@ -473,6 +471,7 @@ github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFB github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= @@ -599,6 +598,7 @@ github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= +github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc= github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= @@ -1259,6 +1259,8 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= +mvdan.cc/sh v2.6.4+incompatible h1:eD6tDeh0pw+/TOTI1BBEryZ02rD2nMcFsgcvde7jffM= +mvdan.cc/sh v2.6.4+incompatible/go.mod h1:IeeQbZq+x2SUGBensq/jge5lLQbS3XT2ktyp3wrt4x8= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=