From 49dd5f4f19f540e606736c18bdb7ce70cd7ae3b6 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Thu, 13 Jun 2024 09:43:09 -0700 Subject: [PATCH 1/6] Adds 'WARNING' prefix to the rule check output and update lint subrequest to return a status of 1 if an error occurred Signed-off-by: Talon Bowler (cherry picked from commit 09dafda017910ecc8ad68df65b5bdf705350d4ea) --- frontend/subrequests/lint/lint.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/subrequests/lint/lint.go b/frontend/subrequests/lint/lint.go index 402a704b9550..a2b567a62017 100644 --- a/frontend/subrequests/lint/lint.go +++ b/frontend/subrequests/lint/lint.go @@ -108,7 +108,7 @@ func (results *LintResults) ToResult() (*client.Result, error) { res.AddMeta("result.txt", b.Bytes()) status := 0 - if len(results.Warnings) > 0 { + if len(results.Warnings) > 0 || results.Error != nil { status = 1 } res.AddMeta("result.statuscode", []byte(fmt.Sprintf("%d", status))) @@ -169,7 +169,7 @@ func PrintLintViolations(dt []byte, w io.Writer) error { }) for _, warning := range results.Warnings { - fmt.Fprintf(w, "%s", warning.RuleName) + fmt.Fprintf(w, "\nWARNING: %s", warning.RuleName) if warning.URL != "" { fmt.Fprintf(w, " - %s", warning.URL) } @@ -187,8 +187,8 @@ func PrintLintViolations(dt []byte, w io.Writer) error { if err != nil { return err } - fmt.Fprintln(w) } + return nil } From 2bf5cbff6571375f48058e92e6c58a336f29c668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Fri, 14 Jun 2024 17:38:13 +0200 Subject: [PATCH 2/6] util/resolver: Make httpFallback concurrent safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The httpFallback mutates the `host` field, which needs a synchronization when the same httpFallback is used by multiple goroutines. This is happens with containerd push which uses `Dispatch` to walk the image recursively to push every blob. Signed-off-by: Paweł Gronowski (cherry picked from commit 02859508b5ab9f9867242cea9d479a76475e190b) --- util/resolver/resolver.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/util/resolver/resolver.go b/util/resolver/resolver.go index d6b7bc381eac..77250bf0b851 100644 --- a/util/resolver/resolver.go +++ b/util/resolver/resolver.go @@ -10,6 +10,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "syscall" "time" @@ -213,13 +214,18 @@ func newDefaultTransport() *http.Transport { } type httpFallback struct { - super http.RoundTripper - host string + super http.RoundTripper + host string + hostMut sync.Mutex } func (f *httpFallback) RoundTrip(r *http.Request) (*http.Response, error) { - // only fall back if the same host had previously fell back - if f.host != r.URL.Host { + f.hostMut.Lock() + // Skip the HTTPS call only if the same host had previously fell back + tryHTTPSFirst := f.host != r.URL.Host + f.hostMut.Unlock() + + if tryHTTPSFirst { resp, err := f.super.RoundTrip(r) if !isTLSError(err) && !isPortError(err, r.URL.Host) { return resp, err @@ -232,8 +238,13 @@ func (f *httpFallback) RoundTrip(r *http.Request) (*http.Response, error) { plainHTTPRequest := *r plainHTTPRequest.URL = &plainHTTPUrl - if f.host != r.URL.Host { + // We tried HTTPS first but it failed. + // Mark the host so we don't try HTTPS for this host next time + // and refresh the request body. + if tryHTTPSFirst { + f.hostMut.Lock() f.host = r.URL.Host + f.hostMut.Unlock() // update body on the second attempt if r.Body != nil && r.GetBody != nil { From 050e3b6f3b944f4c4175a0248dd2d1390e8e0da1 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Mon, 17 Jun 2024 07:07:16 -0700 Subject: [PATCH 3/6] Updates lint output to print detail instead of description Signed-off-by: Talon Bowler (cherry picked from commit 9305c60dd725ca2b456f574c804513aa23395c1f) --- frontend/dockerfile/linter/linter.go | 8 ++++++-- frontend/subrequests/lint/lint.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/frontend/dockerfile/linter/linter.go b/frontend/dockerfile/linter/linter.go index bb33a14229b3..a8f9e0ad0f83 100644 --- a/frontend/dockerfile/linter/linter.go +++ b/frontend/dockerfile/linter/linter.go @@ -91,8 +91,12 @@ func (rule *LinterRule[F]) Run(warn LintWarnFunc, location []parser.Range, txt . warn(rule.Name, rule.Description, rule.URL, short, location) } -func LintFormatShort(rulename, msg string, startLine int) string { - return fmt.Sprintf("%s: %s (line %d)", rulename, msg, startLine) +func LintFormatShort(rulename, msg string, line int) string { + msg = fmt.Sprintf("%s: %s", rulename, msg) + if line > 0 { + msg = fmt.Sprintf("%s (line %d)", msg, line) + } + return msg } type LintWarnFunc func(rulename, description, url, fmtmsg string, location []parser.Range) diff --git a/frontend/subrequests/lint/lint.go b/frontend/subrequests/lint/lint.go index a2b567a62017..d207fba3c082 100644 --- a/frontend/subrequests/lint/lint.go +++ b/frontend/subrequests/lint/lint.go @@ -173,7 +173,7 @@ func PrintLintViolations(dt []byte, w io.Writer) error { if warning.URL != "" { fmt.Fprintf(w, " - %s", warning.URL) } - fmt.Fprintf(w, "\n%s\n", warning.Description) + fmt.Fprintf(w, "\n%s\n", warning.Detail) if warning.Location.SourceIndex < 0 { continue From e34c21a7b0b63477e253dc654e15f536a09f5c42 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Mon, 17 Jun 2024 07:24:57 -0700 Subject: [PATCH 4/6] Consolidate instruction casing lint rules Signed-off-by: Talon Bowler (cherry picked from commit 8cbb98e4d9a2a52846f91f96a6979bc434fbab11) --- frontend/dockerfile/dockerfile2llb/convert.go | 23 +++---- frontend/dockerfile/dockerfile_lint_test.go | 61 +++++++++---------- frontend/dockerfile/docs/rules/_index.md | 4 -- .../rules/consistent-instruction-casing.md | 2 +- .../rules/file-consistent-command-casing.md | 61 ------------------- .../docs/FileConsistentCommandCasing.md | 53 ---------------- frontend/dockerfile/linter/ruleset.go | 12 +--- 7 files changed, 40 insertions(+), 176 deletions(-) delete mode 100644 frontend/dockerfile/docs/rules/file-consistent-command-casing.md delete mode 100644 frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 2ff6edb25f02..4a79eb19e5c8 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -2163,22 +2163,15 @@ func validateCommandCasing(dockerfile *parser.Result, lint *linter.Linter) { // Here, we check both if the command is consistent per command (ie, "CMD" or "cmd", not "Cmd") // as well as ensuring that the casing is consistent throughout the dockerfile by comparing the // command to the casing of the majority of commands. - if !isSelfConsistentCasing(node.Value) { - msg := linter.RuleConsistentInstructionCasing.Format(node.Value) + var correctCasing string + if isMajorityLower && strings.ToLower(node.Value) != node.Value { + correctCasing = "lowercase" + } else if !isMajorityLower && strings.ToUpper(node.Value) != node.Value { + correctCasing = "uppercase" + } + if correctCasing != "" { + msg := linter.RuleConsistentInstructionCasing.Format(node.Value, correctCasing) lint.Run(&linter.RuleConsistentInstructionCasing, node.Location(), msg) - } else { - var msg string - var needsLintWarn bool - if isMajorityLower && strings.ToUpper(node.Value) == node.Value { - msg = linter.RuleFileConsistentCommandCasing.Format(node.Value, "lowercase") - needsLintWarn = true - } else if !isMajorityLower && strings.ToLower(node.Value) == node.Value { - msg = linter.RuleFileConsistentCommandCasing.Format(node.Value, "uppercase") - needsLintWarn = true - } - if needsLintWarn { - lint.Run(&linter.RuleFileConsistentCommandCasing, node.Location(), msg) - } } } } diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index 1e30ddacb3d4..5e8e0b9b339d 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -27,7 +27,6 @@ var lintTests = integration.TestFuncs( testStageName, testNoEmptyContinuation, testConsistentInstructionCasing, - testFileConsistentCommandCasing, testDuplicateStageName, testReservedStageName, testJSONArgsRecommended, @@ -96,19 +95,19 @@ copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing,FromAsCasing + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing,FromAsCasing;error=true + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing,FromAsCasing;error=true FROM scratch as base copy Dockerfile . `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing FROM scratch as base copy Dockerfile . `) @@ -126,7 +125,7 @@ copy Dockerfile . }, }) - dockerfile = []byte(`#check=skip=FileConsistentCommandCasing;error=true + dockerfile = []byte(`#check=skip=ConsistentInstructionCasing;error=true FROM scratch as base copy Dockerfile . `) @@ -167,7 +166,7 @@ copy Dockerfile . UnmarshalBuildErr: "lint violation found for rules: FromAsCasing", BuildErrLocation: 2, FrontendAttrs: map[string]string{ - "build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=FileConsistentCommandCasing;error=true", + "build-arg:BUILDKIT_DOCKERFILE_CHECK": "skip=ConsistentInstructionCasing;error=true", }, }) @@ -274,9 +273,9 @@ FROM scratch AS base2 Warnings: []expectedLintWarning{ { RuleName: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", + Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Detail: "Command 'From' should be consistently cased", + Detail: "Command 'From' should match the case of the command majority (uppercase)", Level: 1, Line: 3, }, @@ -284,42 +283,40 @@ FROM scratch AS base2 }) dockerfile = []byte(` -# warning: 'FROM' should be either lowercased or uppercased -frOM scratch as base -from scratch as base2 +FROM scratch +# warning: 'copy' should match command majority's casing (uppercase) +copy Dockerfile /foo +COPY Dockerfile /bar `) + checkLinterWarnings(t, sb, &lintTestParams{ Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { RuleName: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", + Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Detail: "Command 'frOM' should be consistently cased", - Line: 3, + Detail: "Command 'copy' should match the case of the command majority (uppercase)", + Line: 4, Level: 1, }, }, }) -} -func testFileConsistentCommandCasing(t *testing.T, sb integration.Sandbox) { - dockerfile := []byte(` -FROM scratch -# warning: 'copy' should match command majority's casing (uppercase) -copy Dockerfile /foo -COPY Dockerfile /bar + dockerfile = []byte(` +# warning: 'frOM' should be either lowercased or uppercased +frOM scratch as base +from scratch as base2 `) - checkLinterWarnings(t, sb, &lintTestParams{ Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", - Detail: "Command 'copy' should match the case of the command majority (uppercase)", - Line: 4, + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", + Detail: "Command 'frOM' should match the case of the command majority (lowercase)", + Line: 3, Level: 1, }, }, @@ -335,9 +332,9 @@ copy Dockerfile /bar Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'COPY' should match the case of the command majority (lowercase)", Line: 4, Level: 1, @@ -356,9 +353,9 @@ COPY Dockerfile /baz Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'from' should match the case of the command majority (uppercase)", Line: 3, Level: 1, @@ -377,9 +374,9 @@ copy Dockerfile /baz Dockerfile: dockerfile, Warnings: []expectedLintWarning{ { - RuleName: "FileConsistentCommandCasing", + RuleName: "ConsistentInstructionCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Detail: "Command 'FROM' should match the case of the command majority (lowercase)", Line: 3, Level: 1, diff --git a/frontend/dockerfile/docs/rules/_index.md b/frontend/dockerfile/docs/rules/_index.md index 364f849772c3..87a3c5d6f56e 100644 --- a/frontend/dockerfile/docs/rules/_index.md +++ b/frontend/dockerfile/docs/rules/_index.md @@ -42,10 +42,6 @@ $ docker build --check . ConsistentInstructionCasing - Instructions should be in consistent casing (all lower or all upper) - - - FileConsistentCommandCasing All commands within the Dockerfile should use the same casing (either upper or lower) diff --git a/frontend/dockerfile/docs/rules/consistent-instruction-casing.md b/frontend/dockerfile/docs/rules/consistent-instruction-casing.md index f2432bede251..b080490d4736 100644 --- a/frontend/dockerfile/docs/rules/consistent-instruction-casing.md +++ b/frontend/dockerfile/docs/rules/consistent-instruction-casing.md @@ -1,6 +1,6 @@ --- title: ConsistentInstructionCasing -description: Instructions should be in consistent casing (all lower or all upper) +description: All commands within the Dockerfile should use the same casing (either upper or lower) aliases: - /go/dockerfile/rule/consistent-instruction-casing/ --- diff --git a/frontend/dockerfile/docs/rules/file-consistent-command-casing.md b/frontend/dockerfile/docs/rules/file-consistent-command-casing.md deleted file mode 100644 index 7d6188b1f578..000000000000 --- a/frontend/dockerfile/docs/rules/file-consistent-command-casing.md +++ /dev/null @@ -1,61 +0,0 @@ ---- -title: FileConsistentCommandCasing -description: All commands within the Dockerfile should use the same casing (either upper or lower) -aliases: - - /go/dockerfile/rule/file-consistent-command-casing/ ---- - -## Output - -Example warning: - -```text -Command 'foo' should match the case of the command majority (uppercase) -``` - -## Description - -Instructions within a Dockerfile should have consistent casing through out the -entire files. Instructions are not case-sensitive, but the convention is to use -uppercase for instruction keywords to make it easier to distinguish keywords -from arguments. - -Whether you prefer instructions to be uppercase or lowercase, you should make -sure you use consistent casing to help improve readability of the Dockerfile. - -## Examples - -❌ Bad: mixed uppercase and lowercase. - -```dockerfile -FROM alpine:latest AS builder -run apk --no-cache add build-base - -FROM builder AS build1 -copy source1.cpp source.cpp -``` - -✅ Good: all uppercase. - -```dockerfile -FROM alpine:latest AS builder -RUN apk --no-cache add build-base - -FROM builder AS build1 -COPY source1.cpp source.cpp -``` - -✅ Good: all lowercase. - -```dockerfile -from alpine:latest as builder -run apk --no-cache add build-base - -from builder as build1 -copy source1.cpp source.cpp -``` - -## Related errors - -- [`FromAsCasing`](./from-as-casing.md) - diff --git a/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md b/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md deleted file mode 100644 index 18872ffcb3a3..000000000000 --- a/frontend/dockerfile/linter/docs/FileConsistentCommandCasing.md +++ /dev/null @@ -1,53 +0,0 @@ -## Output - -Example warning: - -```text -Command 'foo' should match the case of the command majority (uppercase) -``` - -## Description - -Instructions within a Dockerfile should have consistent casing through out the -entire files. Instructions are not case-sensitive, but the convention is to use -uppercase for instruction keywords to make it easier to distinguish keywords -from arguments. - -Whether you prefer instructions to be uppercase or lowercase, you should make -sure you use consistent casing to help improve readability of the Dockerfile. - -## Examples - -❌ Bad: mixed uppercase and lowercase. - -```dockerfile -FROM alpine:latest AS builder -run apk --no-cache add build-base - -FROM builder AS build1 -copy source1.cpp source.cpp -``` - -✅ Good: all uppercase. - -```dockerfile -FROM alpine:latest AS builder -RUN apk --no-cache add build-base - -FROM builder AS build1 -COPY source1.cpp source.cpp -``` - -✅ Good: all lowercase. - -```dockerfile -from alpine:latest as builder -run apk --no-cache add build-base - -from builder as build1 -copy source1.cpp source.cpp -``` - -## Related errors - -- [`FromAsCasing`](./from-as-casing.md) diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go index 40b3b643cf39..6c70c8ee0c2e 100644 --- a/frontend/dockerfile/linter/ruleset.go +++ b/frontend/dockerfile/linter/ruleset.go @@ -29,18 +29,10 @@ var ( return "Empty continuation line" }, } - RuleConsistentInstructionCasing = LinterRule[func(string) string]{ + RuleConsistentInstructionCasing = LinterRule[func(string, string) string]{ Name: "ConsistentInstructionCasing", - Description: "Instructions should be in consistent casing (all lower or all upper)", - URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", - Format: func(command string) string { - return fmt.Sprintf("Command '%s' should be consistently cased", command) - }, - } - RuleFileConsistentCommandCasing = LinterRule[func(string, string) string]{ - Name: "FileConsistentCommandCasing", Description: "All commands within the Dockerfile should use the same casing (either upper or lower)", - URL: "https://docs.docker.com/go/dockerfile/rule/file-consistent-command-casing/", + URL: "https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/", Format: func(violatingCommand, correctCasing string) string { return fmt.Sprintf("Command '%s' should match the case of the command majority (%s)", violatingCommand, correctCasing) }, From b45ab30fc0809c4eaa42039f2d0945462d44bfd4 Mon Sep 17 00:00:00 2001 From: Talon Bowler Date: Mon, 17 Jun 2024 05:27:25 -0700 Subject: [PATCH 5/6] Initialize build args from stage base Signed-off-by: Talon Bowler (cherry picked from commit c7c2f0e97ee7a89046c1169d1528f03ebea21981) --- frontend/dockerfile/dockerfile2llb/convert.go | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 4a79eb19e5c8..0de62bc6dce4 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1035,6 +1035,7 @@ func (ds *dispatchState) init() { // the paths we use back to the base image. ds.paths = ds.base.paths ds.workdirSet = ds.base.workdirSet + ds.buildArgs = append(ds.buildArgs, ds.base.buildArgs...) } type dispatchStates struct { From dedaef0afe0d9a269c298fdb2773b9163626611b Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sun, 16 Jun 2024 21:32:19 -0700 Subject: [PATCH 6/6] shell: handle empty string for var replacements Signed-off-by: Tonis Tiigi (cherry picked from commit de3e9c4f07a2bcd07a8aed7fd1cb8eb3695153b0) --- frontend/dockerfile/shell/lex.go | 3 +- frontend/dockerfile/shell/lex_test.go | 107 +++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/frontend/dockerfile/shell/lex.go b/frontend/dockerfile/shell/lex.go index 306eb5e81939..86df5b6c4336 100644 --- a/frontend/dockerfile/shell/lex.go +++ b/frontend/dockerfile/shell/lex.go @@ -429,7 +429,8 @@ func (sw *shellWord) processDollar() (string, error) { case '%', '#': // %/# matches the shortest pattern expansion, %%/## the longest greedy := false - if word[0] == byte(ch) { + + if len(word) > 0 && word[0] == byte(ch) { greedy = true word = word[1:] } diff --git a/frontend/dockerfile/shell/lex_test.go b/frontend/dockerfile/shell/lex_test.go index 9e8f05a5b67c..ca34c1475c0d 100644 --- a/frontend/dockerfile/shell/lex_test.go +++ b/frontend/dockerfile/shell/lex_test.go @@ -313,6 +313,21 @@ func TestProcessWithMatches(t *testing.T) { matches: map[string]struct{}{"FOO": {}, "BAR": {}}, unmatched: map[string]struct{}{"BAZ": {}}, }, + { + input: "${FOO:-}", + envs: map[string]string{ + "FOO": "xxx", + "BAR": "", + }, + expected: "xxx", + matches: map[string]struct{}{"FOO": {}}, + }, + { + input: "${FOO:-}", + envs: map[string]string{}, + expected: "", + unmatched: map[string]struct{}{"FOO": {}}, + }, { input: "${FOO+aaa} ${BAR+bbb} ${BAZ+ccc}", @@ -388,6 +403,15 @@ func TestProcessWithMatches(t *testing.T) { expectedErr: true, unmatched: map[string]struct{}{"BAZ": {}}, }, + { + input: "${BAZ:?}", + envs: map[string]string{ + "FOO": "xxx", + "BAR": "", + }, + expectedErr: true, + unmatched: map[string]struct{}{"BAZ": {}}, + }, { input: "${FOO=aaa}", @@ -405,6 +429,11 @@ func TestProcessWithMatches(t *testing.T) { }, expectedErr: true, }, + { + input: "${FOO=}", + envs: map[string]string{}, + expectedErr: true, + }, { // special characters in regular expressions // } needs to be escaped so it doesn't match the @@ -426,12 +455,42 @@ func TestProcessWithMatches(t *testing.T) { expected: "y", matches: map[string]struct{}{"FOO": {}}, }, + { + input: "${FOO#*}", + envs: map[string]string{"FOO": "xxyy"}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}}, + }, + { + input: "${FOO#$BAR}", + envs: map[string]string{"FOO": "xxyy", "BAR": "x"}, + expected: "xyy", + matches: map[string]struct{}{"FOO": {}, "BAR": {}}, + }, + { + input: "${FOO#$BAR}", + envs: map[string]string{"FOO": "xxyy", "BAR": ""}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}, "BAR": {}}, + }, + { + input: "${FOO#}", + envs: map[string]string{"FOO": "xxyy"}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}}, + }, { input: "${FOO##*x}", envs: map[string]string{"FOO": "xxyy"}, expected: "yy", matches: map[string]struct{}{"FOO": {}}, }, + { + input: "${FOO##}", + envs: map[string]string{"FOO": "xxyy"}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}}, + }, { input: "${FOO#?\\?}", envs: map[string]string{"FOO": "???y"}, @@ -451,6 +510,18 @@ func TestProcessWithMatches(t *testing.T) { expected: "a", matches: map[string]struct{}{"FOO": {}}, }, + { + input: "${FOO%}", + envs: map[string]string{"FOO": "xxyy"}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}}, + }, + { + input: "${FOO%%$BAR}", + envs: map[string]string{"FOO": "xxyy", "BAR": ""}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}, "BAR": {}}, + }, { // test: wildcards input: "${FOO/$NEEDLE/.} - ${FOO//$NEEDLE/.}", @@ -484,6 +555,38 @@ func TestProcessWithMatches(t *testing.T) { expected: "\\/tmp\\/foo.txt", matches: map[string]struct{}{"FOO": {}}, }, + + // Following cases with empty/partial values are currently not + // guaranteed behavior. Tests are provided to make sure partial + // input does not cause runtime error. + { + input: "${FOO/$BAR/ww}", + envs: map[string]string{"FOO": "xxyy", "BAR": ""}, + expected: "wwxxyy", + matches: map[string]struct{}{"FOO": {}, "BAR": {}}, + }, + { + input: "${FOO//ww}", + envs: map[string]string{"FOO": "xxyy"}, + expectedErr: true, + }, + { + input: "${FOO//}", + envs: map[string]string{"FOO": "xxyy"}, + expectedErr: true, + }, + { + input: "${FOO///}", + envs: map[string]string{"FOO": "xxyy"}, + expected: "xxyy", + matches: map[string]struct{}{"FOO": {}}, + }, + { + input: "${FOO///}", + envs: map[string]string{}, + expected: "", + unmatched: map[string]struct{}{"FOO": {}}, + }, } for _, c := range tc { @@ -500,12 +603,12 @@ func TestProcessWithMatches(t *testing.T) { require.NoError(t, err) require.Equal(t, c.expected, w) - require.Equal(t, len(c.matches), len(matches)) + require.Len(t, matches, len(c.matches), c.matches) for k := range c.matches { require.Contains(t, matches, k) } - require.Equal(t, len(c.unmatched), len(unmatched)) + require.Len(t, unmatched, len(c.unmatched), c.unmatched) for k := range c.unmatched { require.Contains(t, unmatched, k) }