From c517a4c1c340542f402cc794c6508bd4370b8a3b Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 25 Oct 2026 16:21:53 -0300 Subject: [PATCH 01/17] fix: match the reference where its own functions disagree Tests that claimed TS behaviour carried expected values typed by hand, so a typo or a reference upgrade went unseen, and tests written against Go's behaviour missed real divergences. tools/tsoracle runs the real TS functions from reference/src over JSON cases; `task testdata:ts-oracle` records their results in internal/*/testdata/ts-oracle and `testdata:ts-oracle:check` fails on a stale one. 483 cases cover parseVersion, isEarlierVersion, the Dockerfile helpers, mergeConfiguration and the metadata label, the feature build helpers, variable substitution and devcontainerIdForLabels, getRef and getCollectionRef, the compose helpers, escapeQuotesForShell and qualifyImageName. A case marked as a divergence must keep differing, so a fix removes its mark. They found, and this fixes: - ${devcontainerId} escaped &, < and > and U+2028/U+2029 and sorted keys by bytes, so some id labels hashed to another id; - a stage name lost its case (`AS Final`), CRLF Dockerfiles got `AS` after the '\r', and a target `Dev` was not found; - ${IMG:-${REG}/app} and other nested defaults resolved differently; - varsub's unresolved variable error, ${...Basename} of '/' and win32 trailing separators; - devcontainers-contrib refs were not redirected to devcontainers-extra; - the features label escaped && and <>; - readComposeVersionPrefix kept '\r', parseMount cut `source=/a=b` wrong, and `external=false` is external as in TS; - mergeConfiguration took runArgs from metadata entries; TS takes them from the config only. docs/DIVERGENCES.md records the BuildKit Dockerfile parsing and the upstream #1213 and #881 ports, which stay as they are. Co-Authored-By: Claude Opus 5.5 --- .github/deadcode-allow.txt | 1 + Taskfile.yml | 13 + docs/DIVERGENCES.md | 13 + internal/cli/compose_override.go | 27 +- internal/cli/compose_override_held_test.go | 8 +- internal/cli/compose_override_test.go | 20 - internal/cli/image_build_info.go | 176 +- internal/cli/image_build_info_held_test.go | 54 - internal/cli/image_build_info_test.go | 55 +- internal/cli/read_configuration.go | 1 - internal/cli/testdata/ts-oracle/compose.json | 409 +++ .../cli/testdata/ts-oracle/dockerfile.json | 1160 +++++++++ internal/cli/ts_oracle_test.go | 108 + internal/config/devcontainerid_parity_test.go | 100 +- .../config/testdata/ts-oracle/varsub.json | 1073 ++++++++ internal/config/varsub.go | 91 +- internal/config/varsub_test.go | 170 -- internal/devtools/command.go | 50 + internal/devtools/tsoracle.go | 110 + internal/devtools/tsoracle_test.go | 154 ++ internal/devtools/versions_gate_test.go | 9 +- internal/docker/dockerfile.go | 177 +- internal/docker/dockerfile_held_test.go | 60 - internal/docker/dockerfile_oracle_test.go | 374 +-- internal/docker/dockerfile_test.go | 241 -- internal/docker/reference_oracle_test.go | 29 - internal/docker/resolve.go | 175 ++ internal/docker/resolve_held_test.go | 54 + .../docker/testdata/ts-oracle/dockerfile.json | 841 +++++++ internal/features/advisory_test.go | 58 - .../features/testdata/ts-oracle/version.json | 356 +++ internal/features/ts_oracle_test.go | 29 + internal/imagemeta/extend.go | 8 +- internal/imagemeta/extend_test.go | 20 - internal/imagemeta/merge.go | 4 - internal/imagemeta/merge_held_test.go | 9 + .../imagemeta/merge_nodocker_held_test.go | 9 +- internal/imagemeta/merge_test.go | 5 +- internal/imagemeta/metadata.go | 1 - internal/imagemeta/reference_oracle_test.go | 162 +- .../testdata/ts-oracle/imagemeta.json | 2221 +++++++++++++++++ internal/oci/image_inspect_test.go | 15 - internal/oci/ref.go | 5 + internal/oci/ref_test.go | 193 +- internal/oci/testdata/ts-oracle/ref.json | 971 +++++++ internal/tsoracle/oracletest/oracletest.go | 171 ++ .../tsoracle/oracletest/oracletest_test.go | 91 + internal/tsoracle/tsoracle.go | 64 + internal/tsoracle/tsoracle_test.go | 40 + tools/tsoracle/build.mjs | 45 + tools/tsoracle/oracle.ts | 102 + 51 files changed, 8661 insertions(+), 1671 deletions(-) create mode 100644 internal/cli/testdata/ts-oracle/compose.json create mode 100644 internal/cli/testdata/ts-oracle/dockerfile.json create mode 100644 internal/cli/ts_oracle_test.go create mode 100644 internal/config/testdata/ts-oracle/varsub.json create mode 100644 internal/devtools/tsoracle.go create mode 100644 internal/devtools/tsoracle_test.go delete mode 100644 internal/docker/reference_oracle_test.go create mode 100644 internal/docker/resolve.go create mode 100644 internal/docker/resolve_held_test.go create mode 100644 internal/docker/testdata/ts-oracle/dockerfile.json create mode 100644 internal/features/testdata/ts-oracle/version.json create mode 100644 internal/features/ts_oracle_test.go create mode 100644 internal/imagemeta/testdata/ts-oracle/imagemeta.json create mode 100644 internal/oci/testdata/ts-oracle/ref.json create mode 100644 internal/tsoracle/oracletest/oracletest.go create mode 100644 internal/tsoracle/oracletest/oracletest_test.go create mode 100644 internal/tsoracle/tsoracle.go create mode 100644 internal/tsoracle/tsoracle_test.go create mode 100644 tools/tsoracle/build.mjs create mode 100644 tools/tsoracle/oracle.ts diff --git a/.github/deadcode-allow.txt b/.github/deadcode-allow.txt index ed6e995..4413cef 100644 --- a/.github/deadcode-allow.txt +++ b/.github/deadcode-allow.txt @@ -1,2 +1,3 @@ # Functions no binary reaches that are correct for ever, one ` # why` a line (go tool deadgate -allow). # An entry that matches nothing fails, so the list only shrinks. +package internal/tsoracle/oracletest # test helper: the Go tests assert their ports against the testdata/ts-oracle goldens with it; no binary should link it diff --git a/Taskfile.yml b/Taskfile.yml index a20a805..2659ee3 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -248,6 +248,19 @@ tasks: - curl -fsSL https://raw.githubusercontent.com/devcontainers/spec/main/schemas/devContainer.base.schema.json -o internal/config/testdata/devContainer.base.schema.json - 'echo "Schema refreshed. Run ''task spec:compliance'' — any new spec property fails until modeled in internal/config/types.go."' + testdata:ts-oracle: + desc: Rewrite every internal/**/testdata/ts-oracle/*.json with the results of the reference TypeScript functions (tools/tsoracle) + deps: [reference:compile] + cmds: + - go run ./cmd/devtool ts-oracle + + testdata:ts-oracle:check: + desc: Fail when a testdata/ts-oracle golden differs from what the reference TypeScript functions return + deps: [reference:compile] + cmds: + - go run ./cmd/devtool ts-oracle + - git diff --exit-code -- '*/testdata/ts-oracle/*.json' + reference: desc: Install and compile the TypeScript parity oracle cmds: diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index 0a580b0..5bdc1f4 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -45,6 +45,19 @@ touches a compared surface, reflected in the parity matrix. - **`BUILDKIT_INLINE_CACHE=1`** is omitted when `--cache-to` is an inline exporter (`/type\s*=\s*inline/i`), matching TS `isBuildxCacheToInline` — a parity fix over the earlier unconditional build-arg. +- **Two upstream fixes newer than the pinned reference are already ported.** A + `${localEnv:VAR:default}` or `${containerEnv:VAR:default}` default keeps its colons + (`my.registry.io:5000/img`; upstream #1213), where v0.89.0 keeps the text up to the next + colon. A read-only mount keeps `:ro` in the Compose override (upstream #881), where + v0.89.0's `convertMountToVolume` drops `readonly`. The `testdata/ts-oracle` goldens + record both as divergence cases. +- **Dockerfile instructions are read with BuildKit's parser**, not with the reference's + regular expressions, when resolving the base image and the `USER` of a Dockerfile. + BuildKit unquotes `'...'` values, reads every `NAME=value` of one `ARG`/`ENV` line and + takes `ENV A = B` as `A` = `= B`, as Docker does; the reference keeps the single quotes, + reads only the first pair and takes `B`. Variable expansion, stage lookup and the final + stage name otherwise follow the reference (recorded in + `internal/{docker,cli}/testdata/ts-oracle/dockerfile.json`). ## Firm decisions & scope diff --git a/internal/cli/compose_override.go b/internal/cli/compose_override.go index 1e35295..dc0e0e5 100644 --- a/internal/cli/compose_override.go +++ b/internal/cli/compose_override.go @@ -20,8 +20,11 @@ const ( composeFeaturesOverridePrefix = "docker-compose.devcontainer.containerFeatures" ) -// composeVersionLineRe is TS readVersionPrefix's /^\s*(version:.*)$/m. -var composeVersionLineRe = regexp.MustCompile(`(?m)^\s*(version:.*)$`) +// composeVersionLineRe is TS readVersionPrefix's /^\s*(version:.*)$/m with +// JavaScript's semantics: a line also starts after \r, \u2028 or \u2029, \s +// includes \v and the Unicode spaces, and . stops at any line terminator (a +// CRLF file's version line has no trailing \r). +var composeVersionLineRe = regexp.MustCompile(`(?m)(?:^|[\r\x{2028}\x{2029}])[\t\n\v\f\r \x{a0}\x{1680}\x{2000}-\x{200a}\x{2028}\x{2029}\x{202f}\x{205f}\x{3000}\x{feff}]*(version:[^\n\r\x{2028}\x{2029}]*)`) // readComposeVersionPrefix returns the first compose file's `version:` line // followed by a blank line, or "" (TS readVersionPrefix). @@ -64,6 +67,16 @@ func composeVolumeSpec(m composeMount) string { return volume } +// composeTopLevelVolume is TS convertMountToVolumeTopLevelElement: a named +// volume's entry under the override's top-level volumes. +func composeTopLevelVolume(m composeMount) string { + volume := "\n " + m.Source + ":" + if m.External { + volume += "\n external: true" + } + return volume +} + // composeMountsFromMetadata converts merged-config mounts (strings or objects) // and --mount values the way TS generateFeaturesComposeOverrideContent does: // strings go through parseMount (`key=value` pairs, src/dst/destination @@ -108,7 +121,9 @@ func composeMountsFromMetadata(entries []any, resolve func(string) (string, erro func parseComposeMountString(spec string) composeMount { var m composeMount for part := range strings.SplitSeq(spec, ",") { + // TS splits on every "=" and keeps the second piece only. key, value, _ := strings.Cut(part, "=") + value, _, _ = strings.Cut(value, "=") switch key { case "type": m.Type = value @@ -117,7 +132,8 @@ func parseComposeMountString(spec string) composeMount { case "target", "dst", "destination": m.Target = value case "external": - m.External = value == "true" + // TS tests the string's truthiness: "false" is external too. + m.External = value != "" case "readonly", "ro": m.ReadOnly = value == "" || value == "true" || value == "1" } @@ -274,10 +290,7 @@ func generateComposeFeaturesOverride(in composeFeaturesOverride) string { if len(named) > 0 { b.WriteString("\nvolumes:") for _, m := range named { - fmt.Fprintf(&b, "\n \n %s:", m.Source) - if m.External { - b.WriteString("\n external: true") - } + b.WriteString("\n " + composeTopLevelVolume(m)) } } b.WriteString("\n") diff --git a/internal/cli/compose_override_held_test.go b/internal/cli/compose_override_held_test.go index 36096cb..3d918ea 100644 --- a/internal/cli/compose_override_held_test.go +++ b/internal/cli/compose_override_held_test.go @@ -8,15 +8,17 @@ import ( "github.com/devcontainers/cli/internal/imagemeta" ) -// TS parseMount keeps every key=value pair; the boolean ones are true only for -// the spellings TS accepts. +// TS parseMount keeps every key=value pair. external is a JavaScript string, so +// any value, "false" included, makes the volume external +// (testdata/ts-oracle/compose.json); readonly is true only for the spellings TS +// accepts. func TestParseComposeMountStringBooleans(t *testing.T) { tests := []struct { spec string want composeMount }{ {"type=volume,source=v,target=/v,external=true", composeMount{Type: "volume", Source: "v", Target: "/v", External: true}}, - {"type=volume,source=v,target=/v,external=false", composeMount{Type: "volume", Source: "v", Target: "/v"}}, + {"type=volume,source=v,target=/v,external=false", composeMount{Type: "volume", Source: "v", Target: "/v", External: true}}, {"source=/h,destination=/c,ro", composeMount{Source: "/h", Target: "/c", ReadOnly: true}}, {"source=/h,target=/c,readonly=true", composeMount{Source: "/h", Target: "/c", ReadOnly: true}}, {"source=/h,target=/c,ro=1", composeMount{Source: "/h", Target: "/c", ReadOnly: true}}, diff --git a/internal/cli/compose_override_test.go b/internal/cli/compose_override_test.go index c703b30..e21b845 100644 --- a/internal/cli/compose_override_test.go +++ b/internal/cli/compose_override_test.go @@ -1,8 +1,6 @@ package cli import ( - "os" - "path/filepath" "strings" "testing" @@ -182,24 +180,6 @@ func TestComposeMountsFromMetadata(t *testing.T) { } } -func TestReadComposeVersionPrefix(t *testing.T) { - dir := t.TempDir() - withVersion := filepath.Join(dir, "a.yml") - if err := os.WriteFile(withVersion, []byte("# c\n version: '3.8'\nservices: {}\n"), 0o644); err != nil { - t.Fatal(err) - } - without := filepath.Join(dir, "b.yml") - if err := os.WriteFile(without, []byte("services: {}\n"), 0o644); err != nil { - t.Fatal(err) - } - if got, _ := readComposeVersionPrefix([]string{withVersion, without}); got != "version: '3.8'\n\n" { - t.Errorf("got %q", got) - } - if got, _ := readComposeVersionPrefix([]string{without}); got != "" { - t.Errorf("got %q", got) - } -} - // The compose Features Dockerfile is appended to the service's Dockerfile, so // the global ARG must be in the prefix (before the service's first FROM), and // the Features' content comes from the additional build context or from the diff --git a/internal/cli/image_build_info.go b/internal/cli/image_build_info.go index eda2bbe..05d6288 100644 --- a/internal/cli/image_build_info.go +++ b/internal/cli/image_build_info.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path/filepath" - "regexp" "runtime" "slices" "strings" @@ -208,9 +207,9 @@ func imageBuildInfoFromImage(ctx context.Context, insp *imageInspector, name str // statement in the Dockerfile takes precedence over the image's user. func imageBuildInfoFromDockerfile(ctx context.Context, insp *imageInspector, dockerfileText string, buildArgs map[string]string, target string, lgr log.Logger) (*imageBuildInfo, error) { df := docker.ExtractDockerfile(dockerfileText) - r := newDockerfileResolver(df, buildArgs, globalBuildxPlatformArgs(targetPlatform(), targetPlatform())) + r := docker.NewResolver(df, buildArgs, globalBuildxPlatformArgs(targetPlatform(), targetPlatform())) var img *oci.ImageConfig - if baseImage := r.findBaseImage(target); baseImage != "" { + if baseImage := r.FindBaseImage(target); baseImage != "" { var err error if img, err = insp.inspect(ctx, baseImage); err != nil { return nil, err @@ -220,7 +219,7 @@ func imageBuildInfoFromDockerfile(ctx context.Context, insp *imageInspector, doc if img != nil { baseEnv = config.EnvMap(img.Env) } - user := r.findUserStatement(baseEnv, target) + user := r.FindUserStatement(baseEnv, target) if user == "" && img != nil { user = img.User } @@ -251,172 +250,3 @@ func globalBuildxPlatformArgs(build, target ocispec.Platform) map[string]string "TARGETVARIANT": target.Variant, } } - -// argumentExpression is TS dockerfileUtils.ts argumentExpression. -var argumentExpression = regexp.MustCompile(`\$\{?([a-zA-Z0-9_]+)(:(-|\+)([^}]+))?\}?`) - -// dfScope is a TS Stage or the preamble ({ from?, instructions }). -type dfScope struct { - from *docker.From - instructions []docker.Instruction -} - -// dockerfileResolver ports the variable resolution of TS dockerfileUtils.ts -// (replaceVariables / findValue), which findBaseImage and findUserStatement use. -// docker.FindBaseImage expands only the declared global ARGs, so it misses the -// automatic platform ARGs that TS (and BuildKit) provide. -type dockerfileResolver struct { - preamble *dfScope - stages []*dfScope - byLabel map[string]*dfScope - buildArgs map[string]string - platform map[string]string -} - -func newDockerfileResolver(df *docker.Dockerfile, buildArgs, platformArgs map[string]string) *dockerfileResolver { - r := &dockerfileResolver{ - preamble: &dfScope{instructions: df.Preamble.Instructions}, - byLabel: map[string]*dfScope{}, - buildArgs: buildArgs, - platform: platformArgs, - } - for i := range df.Stages { - s := &dfScope{from: &df.Stages[i].From, instructions: df.Stages[i].Instructions} - r.stages = append(r.stages, s) - if s.from.Label != "" { - r.byLabel[s.from.Label] = s - } - } - return r -} - -func (r *dockerfileResolver) startStage(target string) *dfScope { - if target != "" { - return r.byLabel[target] - } - if len(r.stages) == 0 { - return nil - } - return r.stages[len(r.stages)-1] -} - -// fromImage resolves a stage's FROM image in the global scope. -func (r *dockerfileResolver) fromImage(s *dfScope, baseEnv map[string]string) string { - return r.replace(baseEnv, s.from.Image, r.preamble, len(r.preamble.instructions)) -} - -// findBaseImage is TS findBaseImage: follow FROM to the first image -// that is not a stage. The base image's env is not available in FROM. -func (r *dockerfileResolver) findBaseImage(target string) string { - seen := map[*dfScope]bool{} - for s := r.startStage(target); s != nil; { - if seen[s] { - return "" - } - seen[s] = true - image := r.fromImage(s, nil) - next, ok := r.byLabel[image] - if !ok { - return image - } - s = next - } - return "" -} - -// findUserStatement is TS findUserStatement: the last USER of the target stage -// (or of the stage it is built FROM), with its variables resolved. -func (r *dockerfileResolver) findUserStatement(baseEnv map[string]string, target string) string { - seen := map[*dfScope]bool{} - for s := r.startStage(target); s != nil; { - if seen[s] { - return "" - } - seen[s] = true - for i := len(s.instructions) - 1; i >= 0; i-- { - if s.instructions[i].Instruction == "USER" { - return r.replace(baseEnv, s.instructions[i].Name, s, i) - } - } - s = r.byLabel[r.fromImage(s, baseEnv)] - } - return "" -} - -// replace is TS replaceVariables. -func (r *dockerfileResolver) replace(baseEnv map[string]string, str string, s *dfScope, before int) string { - var b strings.Builder - last := 0 - for _, m := range argumentExpression.FindAllStringSubmatchIndex(str, -1) { // mutate-exempt: any n < 0 is every match - b.WriteString(str[last:m[0]]) - last = m[1] - value := r.findValue(baseEnv, str[m[2]:m[3]], s, before) - varExp := m[4] // mutate-exempt: m[4] and m[5] are both -1 exactly when the :-word/:+word group is unmatched - if varExp != -1 { // ${var:-word} / ${var:+word} - option, word := str[m[6]], str[m[8]:m[9]] // the option is one character, - or + - isSet := value != "" - if (option == '-' && !isSet) || (option == '+' && isSet) { - value = word - } - value = trimOneQuote(value) - } - b.WriteString(value) - } - b.WriteString(str[last:]) - return b.String() -} - -// trimOneQuote strips one leading and one trailing quote, as TS getExpressionValue does. -func trimOneQuote(s string) string { - if s != "" && (s[0] == '"' || s[0] == '\'') { - s = s[1:] - } - if s != "" && (s[len(s)-1] == '"' || s[len(s)-1] == '\'') { - s = s[:len(s)-1] - } - return s -} - -// findValue is TS findValue. An ARG without a default (and no build arg) does -// not define the variable, so the lookup continues outward. -func (r *dockerfileResolver) findValue(baseEnv map[string]string, variable string, s *dfScope, before int) string { - considerArg := true - seen := map[*dfScope]bool{} - for { - if seen[s] { - return "" - } - seen[s] = true - for i := min(before, len(s.instructions)) - 1; i >= 0; i-- { - ins := s.instructions[i] - if ins.Name != variable { - continue - } - if ins.Instruction == "ENV" { - return r.replace(baseEnv, ins.Value, s, i) - } - if ins.Instruction == "ARG" && considerArg { - if v, ok := r.buildArgs[ins.Name]; ok { - return r.replace(baseEnv, v, s, i) - } - if ins.Value != "" { - return r.replace(baseEnv, ins.Value, s, i) - } - } - } - if s.from == nil { - if v, ok := baseEnv[variable]; ok { - return v - } - return r.platform[variable] - } - image := r.fromImage(s, baseEnv) - next, ok := r.byLabel[image] - if !ok { - next = r.preamble - } - s = next - before = len(s.instructions) - considerArg = s == r.preamble - } -} diff --git a/internal/cli/image_build_info_held_test.go b/internal/cli/image_build_info_held_test.go index ec67c00..0bcea1b 100644 --- a/internal/cli/image_build_info_held_test.go +++ b/internal/cli/image_build_info_held_test.go @@ -10,7 +10,6 @@ import ( "github.com/moby/moby/api/types/container" mobyclient "github.com/moby/moby/client" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/devcontainers/cli/internal/config" "github.com/devcontainers/cli/internal/docker" @@ -153,56 +152,3 @@ func TestImageBuildInfoDockerfileUserFromBaseImageEnv(t *testing.T) { t.Fatalf("info = %+v, %v; want the USER resolved with the base image's env", info, err) } } - -func TestDockerfileResolverVariableExpressions(t *testing.T) { - platform := globalBuildxPlatformArgs(ocispec.Platform{OS: "linux", Architecture: "amd64"}, ocispec.Platform{OS: "linux", Architecture: "amd64"}) - for _, tc := range []struct { - name, dockerfile string - want string - }{ - {"default word, variable set", "ARG V=set\nFROM img:${V:-fallback}\n", "img:set"}, - {"default word, variable unset", "ARG V\nFROM img:${V:-fallback}\n", "img:fallback"}, - {"alternate word, variable set", "ARG V=set\nFROM img:${V:+alt}\n", "img:alt"}, - {"alternate word, variable unset", "ARG V\nFROM img:x${V:+alt}\n", "img:x"}, - {"quoted word", "ARG V\nFROM img:${V:-\"quoted\"}\n", "img:quoted"}, - {"plain variable", "ARG V=1\nFROM img:$V-${V}\n", "img:1-1"}, - } { - t.Run(tc.name, func(t *testing.T) { - r := newDockerfileResolver(docker.ExtractDockerfile(tc.dockerfile), nil, platform) - if got := r.findBaseImage(""); got != tc.want { - t.Errorf("findBaseImage = %q, want %q", got, tc.want) - } - }) - } -} - -func TestDockerfileResolverParentStageArgIsNotInherited(t *testing.T) { - // An ARG of the stage a stage is built FROM is not in scope, its ENV is, and - // so is a global ARG (TS findValue considers ARGs only in the stage itself - // and in the global scope). - platform := globalBuildxPlatformArgs(ocispec.Platform{OS: "linux", Architecture: "amd64"}, ocispec.Platform{OS: "linux", Architecture: "amd64"}) - r := newDockerfileResolver(docker.ExtractDockerfile("ARG G=global\nFROM img AS base\nARG U=parent\nENV E=env\nFROM base\nUSER ${U:-none}-${E}-${G}\n"), nil, platform) - if got := r.findUserStatement(nil, ""); got != "none-env-global" { - t.Fatalf("findUserStatement = %q, want %q", got, "none-env-global") - } -} - -func TestTrimOneQuote(t *testing.T) { - for in, want := range map[string]string{ - ``: ``, - `"`: ``, - `'`: ``, - `a`: `a`, - `"ab"`: `ab`, - `'ab'`: `ab`, - `"ab`: `ab`, - `ab"`: `ab`, - `'ab`: `ab`, - `ab'`: `ab`, - `""ab""`: `"ab"`, - } { - if got := trimOneQuote(in); got != want { - t.Errorf("trimOneQuote(%q) = %q, want %q", in, got, want) - } - } -} diff --git a/internal/cli/image_build_info_test.go b/internal/cli/image_build_info_test.go index de024e5..21d6c80 100644 --- a/internal/cli/image_build_info_test.go +++ b/internal/cli/image_build_info_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "errors" + "maps" "os" "path/filepath" "testing" @@ -10,7 +11,6 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/devcontainers/cli/internal/config" - "github.com/devcontainers/cli/internal/docker" "github.com/devcontainers/cli/internal/log" "github.com/devcontainers/cli/internal/oci" ) @@ -97,51 +97,16 @@ func TestImageInspectorFallbackChain(t *testing.T) { }) } -func TestDockerfileResolverBaseImage(t *testing.T) { - platform := globalBuildxPlatformArgs(ocispec.Platform{OS: "linux", Architecture: "amd64"}, ocispec.Platform{OS: "linux", Architecture: "arm64"}) - for _, tc := range []struct { - name, dockerfile, target string - args map[string]string - want string - }{ - {"global arg default", "ARG V=1\nFROM img:${V}\n", "", nil, "img:1"}, - {"build arg override", "ARG V=1\nFROM img:${V}\n", "", map[string]string{"V": "2"}, "img:2"}, - {"automatic platform arg", "FROM img-$TARGETARCH:${BUILDOS}\n", "", nil, "img-arm64:linux"}, - {"declared platform arg without default", "ARG TARGETPLATFORM\nFROM --platform=$BUILDPLATFORM img:${TARGETPLATFORM}\n", "", nil, "img:linux/arm64"}, - {"var expression", "ARG V\nFROM img:${V:-fallback}\n", "", nil, "img:fallback"}, - {"stage chain", "FROM base:1 AS one\nFROM one AS two\nFROM other:2\n", "two", nil, "base:1"}, - {"last stage by default", "FROM a:1 AS one\nFROM one\n", "", nil, "a:1"}, - {"unknown target", "FROM a:1\n", "nope", nil, ""}, - } { - t.Run(tc.name, func(t *testing.T) { - r := newDockerfileResolver(docker.ExtractDockerfile(tc.dockerfile), tc.args, platform) - if got := r.findBaseImage(tc.target); got != tc.want { - t.Errorf("findBaseImage = %q, want %q", got, tc.want) - } - }) +// globalBuildxPlatformArgs feeds docker.Resolver the automatic platform ARGs +// (its resolution is asserted against TS in testdata/ts-oracle/dockerfile.json). +func TestGlobalBuildxPlatformArgs(t *testing.T) { + got := globalBuildxPlatformArgs(ocispec.Platform{OS: "linux", Architecture: "amd64"}, ocispec.Platform{OS: "linux", Architecture: "arm", Variant: "v7"}) + want := map[string]string{ + "BUILDPLATFORM": "linux/amd64", "BUILDOS": "linux", "BUILDARCH": "amd64", "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm/v7", "TARGETOS": "linux", "TARGETARCH": "arm", "TARGETVARIANT": "v7", } -} - -func TestDockerfileResolverUserStatement(t *testing.T) { - platform := globalBuildxPlatformArgs(ocispec.Platform{OS: "linux", Architecture: "amd64"}, ocispec.Platform{OS: "linux", Architecture: "amd64"}) - for _, tc := range []struct { - name, dockerfile, target string - args, baseEnv map[string]string - want string - }{ - {"plain", "FROM img\nUSER node\n", "", nil, nil, "node"}, - {"stage arg", "FROM img\nARG U=dev\nUSER $U\n", "", nil, nil, "dev"}, - {"build arg", "FROM img\nARG U=dev\nUSER $U\n", "", map[string]string{"U": "ci"}, nil, "ci"}, - {"base image env", "FROM img\nUSER ${IMG_USER}\n", "", nil, map[string]string{"IMG_USER": "vscode"}, "vscode"}, - {"parent stage", "FROM img AS base\nUSER app\nFROM base\nRUN true\n", "", nil, nil, "app"}, - {"none", "FROM img\nRUN true\n", "", nil, nil, ""}, - } { - t.Run(tc.name, func(t *testing.T) { - r := newDockerfileResolver(docker.ExtractDockerfile(tc.dockerfile), tc.args, platform) - if got := r.findUserStatement(tc.baseEnv, tc.target); got != tc.want { - t.Errorf("findUserStatement = %q, want %q", got, tc.want) - } - }) + if !maps.Equal(got, want) { + t.Errorf("globalBuildxPlatformArgs = %v, want %v", got, want) } } diff --git a/internal/cli/read_configuration.go b/internal/cli/read_configuration.go index 5226a6c..5ec738e 100644 --- a/internal/cli/read_configuration.go +++ b/internal/cli/read_configuration.go @@ -468,7 +468,6 @@ func configMetadataEntry(cfg *config.DevContainer, omitRemoteEnv bool) imagemeta WaitFor: cfg.WaitFor, ShutdownAction: cfg.ShutdownAction, UpdateRemoteUserUID: cfg.UpdateRemoteUserUID, - RunArgs: cfg.RunArgs, ContainerEnv: cfg.ContainerEnv, CapAdd: cfg.CapAdd, SecurityOpt: cfg.SecurityOpt, diff --git a/internal/cli/testdata/ts-oracle/compose.json b/internal/cli/testdata/ts-oracle/compose.json new file mode 100644 index 0000000..47c9475 --- /dev/null +++ b/internal/cli/testdata/ts-oracle/compose.json @@ -0,0 +1,409 @@ +{ + "comment": "containerFeaturesOCI.ts getFeatureIdWithoutVersion, dockerCompose.ts readVersionPrefix (given the compose files' contents), convertMountToVolume and convertMountToVolumeTopLevelElement (a string mount through parseMount first, as generateFeaturesComposeOverrideContent does); ported in feature_install.go and compose_override.go.", + "cases": [ + { + "name": "'ghcr.io/devcontainers/features/node:1'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "ghcr.io/devcontainers/features/node:1" + ], + "result": "ghcr.io/devcontainers/features/node" + }, + { + "name": "'ghcr.io/devcontainers/features/node@sha256:ab'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "ghcr.io/devcontainers/features/node@sha256:ab" + ], + "result": "ghcr.io/devcontainers/features/node" + }, + { + "name": "'localhost:5000/o/f/node'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "localhost:5000/o/f/node" + ], + "result": "localhost:5000/o/f/node" + }, + { + "name": "'localhost:5000/o/f/node:2'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "localhost:5000/o/f/node:2" + ], + "result": "localhost:5000/o/f/node" + }, + { + "name": "'ghcr.io/devcontainers/features/node'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "ghcr.io/devcontainers/features/node" + ], + "result": "ghcr.io/devcontainers/features/node" + }, + { + "name": "'node'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "node" + ], + "result": "node" + }, + { + "name": "'node:1'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "node:1" + ], + "result": "node" + }, + { + "name": "'./local-feature'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "./local-feature" + ], + "result": "./local-feature" + }, + { + "name": "'../up/feature:x'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "../up/feature:x" + ], + "result": "../up/feature" + }, + { + "name": "'https://example.com/devcontainer-feature-go.tgz'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "https://example.com/devcontainer-feature-go.tgz" + ], + "result": "https://example.com/devcontainer-feature-go.tgz" + }, + { + "name": "'octocat/myfeatures/helloworld@v0.0.4'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "octocat/myfeatures/helloworld@v0.0.4" + ], + "result": "octocat/myfeatures/helloworld" + }, + { + "name": "'ghcr.io/a/b/c:1@sha256:abc'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "ghcr.io/a/b/c:1@sha256:abc" + ], + "result": "ghcr.io/a/b/c" + }, + { + "name": "'localhost:5000/o/f/node@sha256:abc'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "localhost:5000/o/f/node@sha256:abc" + ], + "result": "localhost:5000/o/f/node" + }, + { + "name": "''", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "" + ], + "result": "" + }, + { + "name": "'a:b/c'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "a:b/c" + ], + "result": "a:b/c" + }, + { + "name": "'ghcr.io/a/b/ñ:1'", + "fn": "getFeatureIdWithoutVersion", + "args": [ + "ghcr.io/a/b/ñ:1" + ], + "result": "ghcr.io/a/b/ñ" + }, + { + "name": "version line after a comment", + "fn": "readVersionPrefix", + "args": [ + [ + "# c\n version: '3.8'\nservices: {}\n", + "services: {}\n" + ] + ], + "result": "version: '3.8'\n\n" + }, + { + "name": "no version", + "fn": "readVersionPrefix", + "args": [ + [ + "services: {}\n" + ] + ], + "result": "" + }, + { + "name": "no files", + "fn": "readVersionPrefix", + "args": [ + [] + ], + "result": "" + }, + { + "name": "version first", + "fn": "readVersionPrefix", + "args": [ + [ + "version: \"3\"\nservices: {}\n" + ] + ], + "result": "version: \"3\"\n\n" + }, + { + "name": "CRLF", + "fn": "readVersionPrefix", + "args": [ + [ + "version: \"3\"\r\nservices: {}\r\n" + ] + ], + "result": "version: \"3\"\n\n" + }, + { + "name": "version in a later file only", + "fn": "readVersionPrefix", + "args": [ + [ + "services: {}\n", + "version: '2'\n" + ] + ], + "result": "" + }, + { + "name": "indented under a key", + "fn": "readVersionPrefix", + "args": [ + [ + "services:\n app:\n version: 1\n" + ] + ], + "result": "version: 1\n\n" + }, + { + "name": "version without space", + "fn": "readVersionPrefix", + "args": [ + [ + "version:'3'\n" + ] + ], + "result": "version:'3'\n\n" + }, + { + "name": "versions key", + "fn": "readVersionPrefix", + "args": [ + [ + "versions: x\n" + ] + ], + "result": "" + }, + { + "name": "tab indented", + "fn": "readVersionPrefix", + "args": [ + [ + "\tversion: '3.9'\n" + ] + ], + "result": "version: '3.9'\n\n" + }, + { + "name": "empty file", + "fn": "readVersionPrefix", + "args": [ + [ + "" + ] + ], + "result": "" + }, + { + "name": "comment after value", + "fn": "readVersionPrefix", + "args": [ + [ + "version: '3' # old\n" + ] + ], + "result": "version: '3' # old\n\n" + }, + { + "name": "volume string", + "fn": "convertMountToVolume", + "args": [ + "type=volume,src=data,dst=/data" + ], + "result": "data:/data" + }, + { + "name": "bind string", + "fn": "convertMountToVolume", + "args": [ + "source=/h,target=/c,type=bind" + ], + "result": "/h:/c" + }, + { + "name": "destination key", + "fn": "convertMountToVolume", + "args": [ + "source=s,destination=/d,type=volume" + ], + "result": "s:/d" + }, + { + "name": "no source", + "fn": "convertMountToVolume", + "args": [ + "type=tmpfs,target=/tmp" + ], + "result": "/tmp" + }, + { + "name": "readonly string", + "fn": "convertMountToVolume", + "args": [ + "source=/h,target=/c,type=bind,readonly" + ], + "divergence": "docs/DIVERGENCES.md: a read-only mount keeps :ro in the compose override (upstream devcontainers/cli#881, newer than the pinned reference, whose convertMountToVolume drops readonly)", + "result": "/h:/c" + }, + { + "name": "value with equals", + "fn": "convertMountToVolume", + "args": [ + "type=bind,source=/a=b,target=/c" + ], + "result": "/a:/c" + }, + { + "name": "volume object", + "fn": "convertMountToVolume", + "args": [ + { + "type": "volume", + "source": "ext", + "target": "/e", + "external": true + } + ], + "result": "ext:/e" + }, + { + "name": "object without source", + "fn": "convertMountToVolume", + "args": [ + { + "type": "tmpfs", + "target": "/t" + } + ], + "result": "/t" + }, + { + "name": "readonly object", + "fn": "convertMountToVolume", + "args": [ + { + "type": "bind", + "source": "/h", + "target": "/c", + "readonly": true + } + ], + "divergence": "docs/DIVERGENCES.md: a read-only mount keeps :ro in the compose override (upstream devcontainers/cli#881, newer than the pinned reference, whose convertMountToVolume drops readonly)", + "result": "/h:/c" + }, + { + "name": "unicode", + "fn": "convertMountToVolume", + "args": [ + { + "type": "bind", + "source": "/home/usuário", + "target": "/w ✓" + } + ], + "result": "/home/usuário:/w ✓" + }, + { + "name": "named volume string", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + "type=volume,src=data,dst=/data" + ], + "result": "\n data:" + }, + { + "name": "external object", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + { + "type": "volume", + "source": "ext", + "target": "/e", + "external": true + } + ], + "result": "\n ext:\n external: true" + }, + { + "name": "external=true string", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + "type=volume,source=ext,target=/e,external=true" + ], + "result": "\n ext:\n external: true" + }, + { + "name": "external=false string", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + "type=volume,source=ext,target=/e,external=false" + ], + "result": "\n ext:\n external: true" + }, + { + "name": "external without value", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + "type=volume,source=ext,target=/e,external" + ], + "result": "\n ext:" + }, + { + "name": "not external", + "fn": "convertMountToVolumeTopLevelElement", + "args": [ + { + "type": "volume", + "source": "v", + "target": "/v" + } + ], + "result": "\n v:" + } + ] +} diff --git a/internal/cli/testdata/ts-oracle/dockerfile.json b/internal/cli/testdata/ts-oracle/dockerfile.json new file mode 100644 index 0000000..fc09540 --- /dev/null +++ b/internal/cli/testdata/ts-oracle/dockerfile.json @@ -0,0 +1,1160 @@ +{ + "comment": "dockerfileUtils.ts findBaseImage with platform ARGs, findUserStatement and supportsBuildContexts, ported in image_build_info.go (dockerfileResolver) and compose_up.go (dockerfileSupportsBuildContexts).", + "cases": [ + { + "name": "automatic platform ARG", + "fn": "findBaseImage", + "args": [ + "FROM img-$TARGETARCH:${BUILDOS}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img-arm64:linux" + }, + { + "name": "declared platform ARG without default", + "fn": "findBaseImage", + "args": [ + "ARG TARGETPLATFORM\nFROM --platform=$BUILDPLATFORM img:${TARGETPLATFORM}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img:linux/arm64" + }, + { + "name": "declared platform ARG with default", + "fn": "findBaseImage", + "args": [ + "ARG TARGETARCH=riscv64\nFROM img:${TARGETARCH}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img:riscv64" + }, + { + "name": "build arg beats platform ARG", + "fn": "findBaseImage", + "args": [ + "ARG TARGETARCH\nFROM img:${TARGETARCH}\n", + { + "TARGETARCH": "s390x" + }, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img:s390x" + }, + { + "name": "variant", + "fn": "findBaseImage", + "args": [ + "FROM img:${TARGETVARIANT:-none}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm/v7", + "TARGETOS": "linux", + "TARGETARCH": "arm", + "TARGETVARIANT": "v7" + } + ], + "result": "img:v7" + }, + { + "name": "empty variant", + "fn": "findBaseImage", + "args": [ + "FROM img:${TARGETVARIANT:-none}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img:none" + }, + { + "name": "var expression fallback", + "fn": "findBaseImage", + "args": [ + "ARG V\nFROM img:${V:-fallback}\n", + {}, + null, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "img:fallback" + }, + { + "name": "stage chain", + "fn": "findBaseImage", + "args": [ + "FROM base:1 AS one\nFROM one AS two\nFROM other:2\n", + {}, + "two", + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + } + ], + "result": "base:1" + }, + { + "name": "ts: simple USER", + "fn": "findUserStatement", + "args": [ + "FROM debian\nUSER user1\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: arg USER", + "fn": "findUserStatement", + "args": [ + "FROM debian\nARG IMAGE_USER=user2\nUSER $IMAGE_USER\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user2" + }, + { + "name": "ts: arg USER overwritten", + "fn": "findUserStatement", + "args": [ + "FROM debian\nARG IMAGE_USER=user2\nUSER $IMAGE_USER\n", + { + "IMAGE_USER": "user3" + }, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user3" + }, + { + "name": "ts: multistage", + "fn": "findUserStatement", + "args": [ + "\nFROM image1 as stage1\nUSER user1\nFROM stage3 as stage2\nFROM image3 as stage3\nUSER user3_1\nUSER user3_2\nFROM image4 as stage4\nUSER user4\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + "stage2" + ], + "result": "user3_2" + }, + { + "name": "ts: ARG after ENV", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nENV USERNAME=user1\nARG USERNAME=user2\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user2" + }, + { + "name": "ts: ARG after ENV in preceding stage", + "fn": "findUserStatement", + "args": [ + "\nFROM debian as one\nENV USERNAME=user1\nARG USERNAME=user2\n\nFROM one as two\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: ARG in preamble", + "fn": "findUserStatement", + "args": [ + "\nARG USERNAME=user1\nFROM debian\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: unbound ARG after ENV", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nENV USERNAME=user1\nARG USERNAME\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: unbound", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": null + }, + { + "name": "ts: ENV after ARG", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nARG USERNAME=user1\nENV USERNAME=user2\nUSER ${USERNAME}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user2" + }, + { + "name": "ts: ENV set from ARG", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nARG USERNAME1=user1\nENV USERNAME2=${USERNAME1}\nUSER ${USERNAME2}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: multiple variables", + "fn": "findUserStatement", + "args": [ + "\nFROM debian\nARG USERNAME1=user1\nENV USERNAME2=user2\nUSER A${USERNAME1}A${USERNAME2}A\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "Auser1Auser2A" + }, + { + "name": "ts: ENV in base image", + "fn": "findUserStatement", + "args": [ + "\nFROM mybase\nUSER ${USERNAME}\n", + {}, + { + "USERNAME": "user1" + }, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "user1" + }, + { + "name": "ts: lowercase instructions", + "fn": "findUserStatement", + "args": [ + "from E\nenv A=B\narg C\nuser D\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "D" + }, + { + "name": "ENV with spaces around equals", + "fn": "findUserStatement", + "args": [ + "FROM debian\nENV A = B\nUSER $A\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "divergence": "docs/DIVERGENCES.md: Dockerfile instructions are read with BuildKit's parser, which unquotes '...' values, reads every NAME=value of an ARG or ENV and takes 'ENV A = B' as A='= B', where TS extractDockerfile's regular expression does not", + "result": "B" + }, + { + "name": "ENV without equals", + "fn": "findUserStatement", + "args": [ + "FROM debian\nENV A B\nUSER $A\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "B" + }, + { + "name": "parent stage USER", + "fn": "findUserStatement", + "args": [ + "FROM img AS base\nUSER app\nFROM base\nRUN true\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "app" + }, + { + "name": "no USER", + "fn": "findUserStatement", + "args": [ + "FROM img\nRUN true\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": null + }, + { + "name": "uid and gid", + "fn": "findUserStatement", + "args": [ + "FROM img\nUSER 1000:1000\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "1000:1000" + }, + { + "name": "quoted USER", + "fn": "findUserStatement", + "args": [ + "FROM img\nUSER \"app\"\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "\"app\"" + }, + { + "name": "quoted ARG default", + "fn": "findUserStatement", + "args": [ + "FROM img\nARG U=\"dev\"\nUSER $U\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "dev" + }, + { + "name": "build arg for stage ARG", + "fn": "findUserStatement", + "args": [ + "FROM img\nARG U=dev\nUSER $U\n", + { + "U": "ci" + }, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "ci" + }, + { + "name": "base env shadowed by ENV", + "fn": "findUserStatement", + "args": [ + "FROM img\nENV U=mine\nUSER $U\n", + {}, + { + "U": "image" + }, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "mine" + }, + { + "name": "variable expression default", + "fn": "findUserStatement", + "args": [ + "FROM img\nARG U\nUSER ${U:-vscode}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "vscode" + }, + { + "name": "variable expression alternative", + "fn": "findUserStatement", + "args": [ + "FROM img\nARG U=x\nUSER ${U:+node}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "node" + }, + { + "name": "unknown target", + "fn": "findUserStatement", + "args": [ + "FROM img\nUSER a\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + "nope" + ], + "result": null + }, + { + "name": "cycle", + "fn": "findUserStatement", + "args": [ + "FROM s AS s\nRUN true\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + "s" + ], + "result": null + }, + { + "name": "empty Dockerfile", + "fn": "findUserStatement", + "args": [ + "", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": null + }, + { + "name": "CRLF", + "fn": "findUserStatement", + "args": [ + "FROM img\r\nUSER app\r\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "app" + }, + { + "name": "unicode user", + "fn": "findUserStatement", + "args": [ + "FROM img\nUSER usuário\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "usuário" + }, + { + "name": "platform ARG in USER", + "fn": "findUserStatement", + "args": [ + "FROM img\nARG TARGETARCH\nUSER u-${TARGETARCH}\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "u-arm64" + }, + { + "name": "global ARG not redeclared", + "fn": "findUserStatement", + "args": [ + "ARG U=g\nFROM img\nUSER $U\n", + {}, + {}, + { + "BUILDPLATFORM": "linux/amd64", + "BUILDOS": "linux", + "BUILDARCH": "amd64", + "BUILDVARIANT": "", + "TARGETPLATFORM": "linux/arm64", + "TARGETOS": "linux", + "TARGETARCH": "arm64", + "TARGETVARIANT": "" + }, + null + ], + "result": "g" + }, + { + "name": "ts: no syntax directive", + "fn": "supportsBuildContexts", + "args": [ + "FROM debian" + ], + "result": false + }, + { + "name": "ts: matching", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "ts: matching with docker.io", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "ts: unknown frontend", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=mycompany/myimage:1.4\nFROM debian" + ], + "result": "unknown" + }, + { + "name": "ts: tag 0", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:0\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.2", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.2\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1.2.3", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.2.3\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1.4", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.4\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.4.5", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.4.5\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.5", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.5\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.5.0", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.5.0\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 2", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:2\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag ", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag latest", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:latest\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 0-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:0-labs\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.2-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.2-labs\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1.2.3-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.2.3-labs\n FROM debian" + ], + "result": false + }, + { + "name": "ts: tag 1.4-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.4-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.4.5-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.4.5-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.5-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.5-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 1.5.0-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:1.5.0-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag 2-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:2-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag -labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:-labs\n FROM debian" + ], + "result": true + }, + { + "name": "ts: tag latest-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker.io/docker/dockerfile:latest-labs\n FROM debian" + ], + "result": true + }, + { + "name": "no space after hash", + "fn": "supportsBuildContexts", + "args": [ + "#syntax=docker/dockerfile:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "spaces around equals", + "fn": "supportsBuildContexts", + "args": [ + "# syntax = docker/dockerfile:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "1.10", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:1.10\nFROM debian" + ], + "result": true + }, + { + "name": "1.3-labs", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:1.3-labs\nFROM debian" + ], + "result": false + }, + { + "name": "digest-pinned", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:1.7@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef\nFROM debian" + ], + "result": true + }, + { + "name": "uppercase frontend", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=DOCKER/DOCKERFILE:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "upstream frontend", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile-upstream:master\nFROM debian" + ], + "result": true + }, + { + "name": "after a check directive", + "fn": "supportsBuildContexts", + "args": [ + "# check=skip=all\n# syntax=docker/dockerfile:1.4\nFROM debian" + ], + "result": true + }, + { + "name": "after a comment line", + "fn": "supportsBuildContexts", + "args": [ + "# my image\n# syntax=docker/dockerfile:1.4\nFROM debian" + ], + "result": false + }, + { + "name": "after a blank line", + "fn": "supportsBuildContexts", + "args": [ + "\n# syntax=docker/dockerfile:1.4\nFROM debian" + ], + "result": false + }, + { + "name": "CRLF", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:1.4\r\nFROM debian\r\n" + ], + "result": true + }, + { + "name": "empty Dockerfile", + "fn": "supportsBuildContexts", + "args": [ + "" + ], + "result": false + }, + { + "name": "v-prefixed tag", + "fn": "supportsBuildContexts", + "args": [ + "# syntax=docker/dockerfile:v1.4\nFROM debian" + ], + "result": true + } + ] +} diff --git a/internal/cli/ts_oracle_test.go b/internal/cli/ts_oracle_test.go new file mode 100644 index 0000000..9e385a8 --- /dev/null +++ b/internal/cli/ts_oracle_test.go @@ -0,0 +1,108 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strconv" + "testing" + + "github.com/devcontainers/cli/internal/docker" + "github.com/devcontainers/cli/internal/tsoracle/oracletest" +) + +// The ts-oracle goldens are the results of the reference functions +// themselves (go tool task testdata:ts-oracle). + +func tsFindBaseImage(args []json.RawMessage) (any, error) { + var content, target string + var buildArgs, platformArgs map[string]string + if err := oracletest.Args(args, &content, &buildArgs, &target, &platformArgs); err != nil { + return nil, err + } + r := docker.NewResolver(docker.ExtractDockerfile(content), buildArgs, platformArgs) + return oracletest.OptionalString(r.FindBaseImage(target)), nil +} + +func TestTSOracleDockerfile(t *testing.T) { + impls := map[string]oracletest.Func{ + "findBaseImage": tsFindBaseImage, + "findUserStatement": func(args []json.RawMessage) (any, error) { + var content, target string + var buildArgs, baseEnv, platformArgs map[string]string + if err := oracletest.Args(args, &content, &buildArgs, &baseEnv, &platformArgs, &target); err != nil { + return nil, err + } + r := docker.NewResolver(docker.ExtractDockerfile(content), buildArgs, platformArgs) + return oracletest.OptionalString(r.FindUserStatement(baseEnv, target)), nil + }, + "supportsBuildContexts": func(args []json.RawMessage) (any, error) { + var content string + if err := oracletest.Args(args, &content); err != nil { + return nil, err + } + // TS answers true, false or 'unknown'; every caller tests it for truthiness. + return oracletest.Truthiness(dockerfileSupportsBuildContexts(docker.ExtractDockerfile(content))), nil + }, + // internal/docker's test asserts these. + "ensureDockerfileHasFinalStageName": nil, + "isBuildxCacheToInline": nil, + } + oracletest.Run(t, "testdata/ts-oracle/dockerfile.json", impls) + // The same findBaseImage cases internal/docker's FindBaseImage answers. + oracletest.Run(t, "../docker/testdata/ts-oracle/dockerfile.json", impls) +} + +func TestTSOracleCompose(t *testing.T) { + // mount is a TS Mount or mount string, as composeMountsFromMetadata takes + // the merged configuration's mounts. + mount := func(args []json.RawMessage) (composeMount, error) { + var m any + if err := oracletest.Args(args, &m); err != nil { + return composeMount{}, err + } + mounts, err := composeMountsFromMetadata([]any{m}, func(s string) (string, error) { return s, nil }) + if err != nil { + return composeMount{}, err + } + return mounts[0], nil + } + oracletest.Run(t, "testdata/ts-oracle/compose.json", map[string]oracletest.Func{ + "getFeatureIdWithoutVersion": func(args []json.RawMessage) (any, error) { + var id string + if err := oracletest.Args(args, &id); err != nil { + return nil, err + } + return featureIDWithoutVersion(id), nil + }, + "readVersionPrefix": func(args []json.RawMessage) (any, error) { + var contents []string + if err := oracletest.Args(args, &contents); err != nil { + return nil, err + } + dir := t.TempDir() + files := make([]string, len(contents)) + for i, c := range contents { + files[i] = filepath.Join(dir, strconv.Itoa(i)+".yml") + if err := os.WriteFile(files[i], []byte(c), 0o644); err != nil { + return nil, err + } + } + return readComposeVersionPrefix(files) + }, + "convertMountToVolume": func(args []json.RawMessage) (any, error) { + m, err := mount(args) + if err != nil { + return nil, err + } + return composeVolumeSpec(m), nil + }, + "convertMountToVolumeTopLevelElement": func(args []json.RawMessage) (any, error) { + m, err := mount(args) + if err != nil { + return nil, err + } + return composeTopLevelVolume(m), nil + }, + }) +} diff --git a/internal/config/devcontainerid_parity_test.go b/internal/config/devcontainerid_parity_test.go index 4a94ef2..bfe84ac 100644 --- a/internal/config/devcontainerid_parity_test.go +++ b/internal/config/devcontainerid_parity_test.go @@ -1,45 +1,71 @@ package config -import "testing" +import ( + "encoding/json" + "testing" -// TestComputeDevContainerID_TSParity pins Go's ${devcontainerId} algorithm to the -// TS oracle (v0.88.0, spec-common/variableSubstitution.ts devcontainerIdForLabels: -// JSON.stringify(labels, sortedKeys) → sha256 → BigInt.toString(32).padStart(52,'0')). -// The expected values were computed with the reference implementation. A cross- -// command substitution divergence here would silently break every ${devcontainerId} -// use in up/build/exec — and the parity matrix can't catch it because each side runs -// with different --id-labels (so the resolved IDs legitimately differ per side). -func TestComputeDevContainerID_TSParity(t *testing.T) { - tests := []struct { - name string - labels map[string]string - want string - }{ - { - name: "two labels", - labels: map[string]string{"foo": "bar", "parity.test": "devid"}, - want: "0eqk35tddobe0171rebd7mvh8npbq2f49ctp543uiimslnjgumv5", - }, - { - name: "devcontainer local_folder + config_file", - labels: map[string]string{"devcontainer.local_folder": "/w", "devcontainer.config_file": "/w/.devcontainer.json"}, - want: "0c27kmi3t5h3anj4psh9usfultojkmt45hfb3mf98sm13ug01diu", + "github.com/devcontainers/cli/internal/tsoracle/oracletest" +) + +// tsSubstitutionContext is a TS SubstitutionContext as the golden writes it. +type tsSubstitutionContext struct { + Platform string `json:"platform"` + ConfigFile string `json:"configFile"` + LocalWorkspaceFolder string `json:"localWorkspaceFolder"` + ContainerWorkspaceFolder string `json:"containerWorkspaceFolder"` + Env map[string]string `json:"env"` +} + +// TestTSOracleVariableSubstitution asserts varsub.go against the reference +// functions' results (go tool task testdata:ts-oracle). A divergence in +// ${devcontainerId} would break every use of it in up/build/exec, and the +// parity matrix cannot see it: each side runs with its own --id-labels. +func TestTSOracleVariableSubstitution(t *testing.T) { + resolve := func(ctx SubstitutionContext, phase SubstitutionPhase, value any) (any, error) { + got, err := NewVariableResolver().Resolve(ctx, phase, value) + if err != nil { + return nil, err + } + return got, nil + } + oracletest.Run(t, "testdata/ts-oracle/varsub.json", map[string]oracletest.Func{ + "devcontainerIdForLabels": func(args []json.RawMessage) (any, error) { + var labels map[string]string + if err := oracletest.Args(args, &labels); err != nil { + return nil, err + } + return ComputeDevContainerID(labels), nil }, - { - name: "single label", - labels: map[string]string{"a": "1"}, - want: "16num3pb40viagoiti6uqh0t0ccbfgqcavs6inmu8b922mhgo2b0", + "substitute": func(args []json.RawMessage) (any, error) { + var c tsSubstitutionContext + var value any + if err := oracletest.Args(args, &c, &value); err != nil { + return nil, err + } + return resolve(SubstitutionContext{HostSubContext: HostSubContext{ + Platform: c.Platform, LocalWorkspaceFolder: c.LocalWorkspaceFolder, + ContainerWorkspaceFolder: c.ContainerWorkspaceFolder, Env: c.Env, ConfigFilePath: c.ConfigFile, + }}, PhaseHost, value) }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got := ComputeDevContainerID(tc.labels) - if got != tc.want { - t.Errorf("ComputeDevContainerID(%v)\n got: %s\n want: %s (TS oracle)", tc.labels, got, tc.want) + "beforeContainerSubstitute": func(args []json.RawMessage) (any, error) { + var labels map[string]string + var value any + if err := oracletest.Args(args, &labels, &value); err != nil { + return nil, err } - if len(got) != 52 { - t.Errorf("devcontainerId length = %d, want 52 (padStart)", len(got)) + return resolve(SubstitutionContext{IDLabels: labels}, PhaseIdentity, value) + }, + "containerSubstitute": func(args []json.RawMessage) (any, error) { + var platform, configFile string + var env map[string]string + var value any + if err := oracletest.Args(args, &platform, &configFile, &env, &value); err != nil { + return nil, err } - }) - } + return resolve(SubstitutionContext{ + HostSubContext: HostSubContext{Platform: platform, ConfigFilePath: configFile}, + ContainerEnv: env, + }, PhaseContainer, value) + }, + }) } diff --git a/internal/config/testdata/ts-oracle/varsub.json b/internal/config/testdata/ts-oracle/varsub.json new file mode 100644 index 0000000..688173c --- /dev/null +++ b/internal/config/testdata/ts-oracle/varsub.json @@ -0,0 +1,1073 @@ +{ + "comment": "variableSubstitution.ts substitute, beforeContainerSubstitute, containerSubstitute and devcontainerIdForLabels (through ${devcontainerId}), ported in varsub.go (Resolve with PhaseHost, PhaseIdentity, PhaseContainer; ComputeDevContainerID).", + "cases": [ + { + "name": "two labels", + "fn": "devcontainerIdForLabels", + "args": [ + { + "foo": "bar", + "parity.test": "devid" + } + ], + "result": "0eqk35tddobe0171rebd7mvh8npbq2f49ctp543uiimslnjgumv5" + }, + { + "name": "local_folder and config_file", + "fn": "devcontainerIdForLabels", + "args": [ + { + "devcontainer.local_folder": "/w", + "devcontainer.config_file": "/w/.devcontainer.json" + } + ], + "result": "0c27kmi3t5h3anj4psh9usfultojkmt45hfb3mf98sm13ug01diu" + }, + { + "name": "single label", + "fn": "devcontainerIdForLabels", + "args": [ + { + "a": "1" + } + ], + "result": "16num3pb40viagoiti6uqh0t0ccbfgqcavs6inmu8b922mhgo2b0" + }, + { + "name": "ts: a=b", + "fn": "devcontainerIdForLabels", + "args": [ + { + "a": "b" + } + ], + "result": "1mqafr5h2iu6dhh3k1m4vtnuhml2uiec4s7bntt1v0f25ao63i1n" + }, + { + "name": "ts: two labels", + "fn": "devcontainerIdForLabels", + "args": [ + { + "a": "b", + "c": "d" + } + ], + "result": "1e2sfmkjt1sga649hgk0s5f3u6nlqhlv9al481r91s7gkeo32p3o" + }, + { + "name": "ts: unsorted labels", + "fn": "devcontainerIdForLabels", + "args": [ + { + "c": "d", + "a": "b" + } + ], + "result": "1e2sfmkjt1sga649hgk0s5f3u6nlqhlv9al481r91s7gkeo32p3o" + }, + { + "name": "empty", + "fn": "devcontainerIdForLabels", + "args": [ + {} + ], + "result": "0h0jduhlbcr7h88kdb8muvk697kkvd7s47v7fq1h1g30uoealvsa" + }, + { + "name": "HTML characters", + "fn": "devcontainerIdForLabels", + "args": [ + { + "devcontainer.local_folder": "/home/me/a&b " + } + ], + "result": "06ilodbvfsb63itnb0brs4scnijm066eg4pjk1lrr98h1j9181r9" + }, + { + "name": "quotes and backslashes", + "fn": "devcontainerIdForLabels", + "args": [ + { + "k": "C:\\Users\\me\\\"q\"" + } + ], + "result": "1rvqfcgbnscs3ese18e25juciac2cs25jbc9bl4vfresrsgqj31q" + }, + { + "name": "control characters", + "fn": "devcontainerIdForLabels", + "args": [ + { + "k": "tab\tnl\ncr\rbs\bff\fnul\u0000esc\u001b" + } + ], + "result": "1fj8klj379vnjkefd2j3qabtqmear9tfp8qavtg8d99oh0bn9sir" + }, + { + "name": "line and paragraph separators", + "fn": "devcontainerIdForLabels", + "args": [ + { + "k": "a\u2028b\u2029c" + } + ], + "result": "1jog83ncrnadnokl6cu3petc00uf821vkk2ousopop6i0glah7rn" + }, + { + "name": "unicode keys sorted by UTF-16", + "fn": "devcontainerIdForLabels", + "args": [ + { + "": "1", + "😀": "2", + "é": "3", + "z": "4" + } + ], + "result": "1g2hhjejhkapd890kq688ohq023bss46ghsj7qq1rntns9ftod2n" + }, + { + "name": "unicode values", + "fn": "devcontainerIdForLabels", + "args": [ + { + "devcontainer.local_folder": "/home/usuário/projeto ✓" + } + ], + "result": "1m61do3vis1sn18nsboulsb2oqnc0i3kkq5vo4q0i4coqc29nvea" + }, + { + "name": "ts: environment variables", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": { + "baz": "somevalue" + } + }, + { + "foo": "bar${env:baz}bar" + } + ], + "result": { + "foo": "barsomevaluebar" + } + }, + { + "name": "ts: localWorkspaceFolder", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": { + "baz": "somevalue" + } + }, + { + "foo": "bar${localWorkspaceFolder}bar" + } + ], + "result": { + "foo": "bar/foo/barbar" + } + }, + { + "name": "ts: containerWorkspaceFolder", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": { + "baz": "somevalue" + } + }, + { + "foo": "bar${containerWorkspaceFolder}bar" + } + ], + "result": { + "foo": "bar/baz/bluebar" + } + }, + { + "name": "ts: containerWorkspaceFolder from localWorkspaceFolderBasename", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/red", + "containerWorkspaceFolder": "/baz/${localWorkspaceFolderBasename}", + "configFile": "/foo/bar/baz.json", + "env": { + "baz": "somevalue" + } + }, + { + "foo": "bar${containerWorkspaceFolder}bar" + } + ], + "result": { + "foo": "bar/baz/redbar" + } + }, + { + "name": "ts: default when missing", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": {} + }, + { + "foo": "bar${localEnv:baz:default}bar" + } + ], + "result": { + "foo": "bardefaultbar" + } + }, + { + "name": "ts: missing without default", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": {} + }, + { + "foo": "bar${localEnv:baz}bar" + } + ], + "result": { + "foo": "barbar" + } + }, + { + "name": "ts: default ignored when present", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": { + "baz": "somevalue" + } + }, + { + "foo": "bar${localEnv:baz:default}bar" + } + ], + "result": { + "foo": "barsomevaluebar" + } + }, + { + "name": "ts: default with colons", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/foo/bar", + "containerWorkspaceFolder": "/baz/blue", + "configFile": "/foo/bar/baz.json", + "env": {} + }, + { + "foo": "bar${localEnv:baz:default:a:b:c}bar" + } + ], + "divergence": "docs/DIVERGENCES.md: a ${localEnv:VAR:default} / ${containerEnv:VAR:default} default keeps its colons (upstream devcontainers/cli#1213, newer than the pinned reference, which keeps only the text up to the next colon)", + "result": { + "foo": "bardefaultbar" + } + }, + { + "name": "localEnv HOME", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:HOME}" + ], + "result": "/home/user" + }, + { + "name": "env alias", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${env:HOME}" + ], + "result": "/home/user" + }, + { + "name": "localEnv with suffix", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:HOME}/project" + ], + "result": "/home/user/project" + }, + { + "name": "localEnv missing", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:MISSING}" + ], + "result": "" + }, + { + "name": "localEnv missing with fallback", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:MISSING:fallback}" + ], + "result": "fallback" + }, + { + "name": "localEnv fallback with colons", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:MISSING:my.registry.io:5000/img:tag}" + ], + "divergence": "docs/DIVERGENCES.md: a ${localEnv:VAR:default} / ${containerEnv:VAR:default} default keeps its colons (upstream devcontainers/cli#1213, newer than the pinned reference, which keeps only the text up to the next colon)", + "result": "my.registry.io" + }, + { + "name": "localEnv present ignores colon fallback", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:HOME:a:b:c}" + ], + "result": "/home/user" + }, + { + "name": "no variables", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "no vars here" + ], + "result": "no vars here" + }, + { + "name": "two variables", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:USER}@${localEnv:HOME}" + ], + "result": "test@/home/user" + }, + { + "name": "empty variable name", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:}" + ], + "result": "" + }, + { + "name": "empty default", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:MISSING:}" + ], + "result": "" + }, + { + "name": "empty value", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "E": "" + } + }, + "${localEnv:E:fallback}" + ], + "result": "" + }, + { + "name": "names are case-sensitive on linux", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:home}" + ], + "result": "" + }, + { + "name": "localWorkspaceFolder", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project", + "containerWorkspaceFolder": "/workspaces/project", + "env": {} + }, + "${localWorkspaceFolder}" + ], + "result": "/home/user/project" + }, + { + "name": "localWorkspaceFolderBasename", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project", + "containerWorkspaceFolder": "/workspaces/project", + "env": {} + }, + "${localWorkspaceFolderBasename}" + ], + "result": "project" + }, + { + "name": "containerWorkspaceFolder", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project", + "containerWorkspaceFolder": "/workspaces/project", + "env": {} + }, + "${containerWorkspaceFolder}" + ], + "result": "/workspaces/project" + }, + { + "name": "containerWorkspaceFolderBasename", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project", + "containerWorkspaceFolder": "/workspaces/project", + "env": {} + }, + "${containerWorkspaceFolderBasename}" + ], + "result": "project" + }, + { + "name": "unset folders kept", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + "${localWorkspaceFolder}|${localWorkspaceFolderBasename}|${containerWorkspaceFolder}|${containerWorkspaceFolderBasename}" + ], + "result": "${localWorkspaceFolder}|${localWorkspaceFolderBasename}|${containerWorkspaceFolder}|${containerWorkspaceFolderBasename}" + }, + { + "name": "trailing slash basename", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project/", + "containerWorkspaceFolder": "/workspaces/x/", + "env": {} + }, + "${localWorkspaceFolderBasename}|${containerWorkspaceFolderBasename}" + ], + "result": "project|x" + }, + { + "name": "root basename", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/", + "containerWorkspaceFolder": "/", + "env": {} + }, + "[${localWorkspaceFolderBasename}|${containerWorkspaceFolderBasename}]" + ], + "result": "[|]" + }, + { + "name": "unknown variable", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + "${unknownVar}" + ], + "result": "${unknownVar}" + }, + { + "name": "devcontainerId left for later", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + "${devcontainerId}" + ], + "result": "${devcontainerId}" + }, + { + "name": "containerEnv left for later", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + "${containerEnv:PATH}" + ], + "result": "${containerEnv:PATH}" + }, + { + "name": "number", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + 42 + ], + "result": 42 + }, + { + "name": "boolean", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + true + ], + "result": true + }, + { + "name": "null", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + null + ], + "result": null + }, + { + "name": "nested map and array", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "localWorkspaceFolder": "/home/user/project", + "env": { + "HOME": "/home/user" + } + }, + { + "workspaceFolder": "${localWorkspaceFolder}", + "env": { + "HOME": "${localEnv:HOME}" + }, + "ports": [ + "${localEnv:HOME}:8080", + 3000, + null, + false + ] + } + ], + "result": { + "workspaceFolder": "/home/user/project", + "env": { + "HOME": "/home/user" + }, + "ports": [ + "/home/user:8080", + 3000, + null, + false + ] + } + }, + { + "name": "unterminated", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "before ${localEnv:HOME" + ], + "result": "before ${localEnv:HOME" + }, + { + "name": "nested braces", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "${localEnv:${localEnv:HOME}}" + ], + "result": "HOME}" + }, + { + "name": "dollar escape is not special", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "HOME": "/home/user", + "USER": "test" + } + }, + "$${localEnv:HOME}" + ], + "result": "$/home/user" + }, + { + "name": "unicode", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": { + "SALUDO": "¡hola ✓!" + } + }, + "${localEnv:SALUDO}" + ], + "result": "¡hola ✓!" + }, + { + "name": "missing variable name", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {}, + "configFile": "/w/.devcontainer/devcontainer.json" + }, + "${localEnv}" + ], + "error": "'${localEnv}' in devcontainer.json can not be resolved because no environment variable name is given." + }, + { + "name": "missing env name", + "fn": "substitute", + "args": [ + { + "platform": "linux", + "env": {} + }, + "x ${env} y" + ], + "error": "'${env}' can not be resolved because no environment variable name is given." + }, + { + "name": "windows: case-insensitive names", + "fn": "substitute", + "args": [ + { + "platform": "win32", + "env": { + "Path": "C:\\bin" + } + }, + "${localEnv:PATH}|${env:path}" + ], + "result": "C:\\bin|C:\\bin" + }, + { + "name": "windows: basename", + "fn": "substitute", + "args": [ + { + "platform": "win32", + "localWorkspaceFolder": "C:\\Users\\me\\proj", + "containerWorkspaceFolder": "/workspaces/proj", + "env": {} + }, + "${localWorkspaceFolderBasename}|${containerWorkspaceFolderBasename}" + ], + "result": "proj|proj" + }, + { + "name": "windows: forward slashes", + "fn": "substitute", + "args": [ + { + "platform": "win32", + "localWorkspaceFolder": "C:/Users/me/proj", + "env": {} + }, + "${localWorkspaceFolderBasename}" + ], + "result": "proj" + }, + { + "name": "windows: drive root", + "fn": "substitute", + "args": [ + { + "platform": "win32", + "localWorkspaceFolder": "C:\\", + "env": {} + }, + "[${localWorkspaceFolderBasename}]" + ], + "result": "[]" + }, + { + "name": "windows: trailing separator", + "fn": "substitute", + "args": [ + { + "platform": "win32", + "localWorkspaceFolder": "C:\\Users\\me\\proj\\", + "env": {} + }, + "${localWorkspaceFolderBasename}" + ], + "result": "proj" + }, + { + "name": "ts: devcontainerId", + "fn": "beforeContainerSubstitute", + "args": [ + { + "a": "b" + }, + { + "test": "${devcontainerId}" + } + ], + "result": { + "test": "1mqafr5h2iu6dhh3k1m4vtnuhml2uiec4s7bntt1v0f25ao63i1n" + } + }, + { + "name": "devcontainerId twice", + "fn": "beforeContainerSubstitute", + "args": [ + { + "devcontainer.local_folder": "/w" + }, + "cache-${devcontainerId}/${devcontainerId}" + ], + "result": "cache-0ntkv7h0lfm4ou0de4rjg3vak3nbb5pk116v2bq1jj3hm6kho6n5/0ntkv7h0lfm4ou0de4rjg3vak3nbb5pk116v2bq1jj3hm6kho6n5" + }, + { + "name": "unknown variables kept", + "fn": "beforeContainerSubstitute", + "args": [ + { + "a": "b" + }, + "${unknown}:${devcontainerId}:${localEnv:HOME}" + ], + "result": "${unknown}:1mqafr5h2iu6dhh3k1m4vtnuhml2uiec4s7bntt1v0f25ao63i1n:${localEnv:HOME}" + }, + { + "name": "similar name kept", + "fn": "beforeContainerSubstitute", + "args": [ + { + "a": "b" + }, + "${devcontainerIdExtra}" + ], + "result": "${devcontainerIdExtra}" + }, + { + "name": "no id labels", + "fn": "beforeContainerSubstitute", + "args": [ + null, + "cache-${devcontainerId}" + ], + "result": "cache-${devcontainerId}" + }, + { + "name": "nested", + "fn": "beforeContainerSubstitute", + "args": [ + { + "a": "b" + }, + { + "mounts": [ + "source=v-${devcontainerId},target=/v" + ], + "n": 1 + } + ], + "result": { + "mounts": [ + "source=v-1mqafr5h2iu6dhh3k1m4vtnuhml2uiec4s7bntt1v0f25ao63i1n,target=/v" + ], + "n": 1 + } + }, + { + "name": "ts: default when missing", + "fn": "containerSubstitute", + "args": [ + "linux", + "/foo/bar/baz.json", + {}, + { + "foo": "bar${containerEnv:baz:default}bar" + } + ], + "result": { + "foo": "bardefaultbar" + } + }, + { + "name": "PATH", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv:PATH}" + ], + "result": "/usr/bin:/usr/local/bin" + }, + { + "name": "SHELL", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv:SHELL}" + ], + "result": "/bin/bash" + }, + { + "name": "missing", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv:MISSING}" + ], + "result": "" + }, + { + "name": "no variables", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "no vars" + ], + "result": "no vars" + }, + { + "name": "localEnv left alone", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${localEnv:HOME}" + ], + "result": "${localEnv:HOME}" + }, + { + "name": "default with colons", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv:NOPE:a:b}" + ], + "divergence": "docs/DIVERGENCES.md: a ${localEnv:VAR:default} / ${containerEnv:VAR:default} default keeps its colons (upstream devcontainers/cli#1213, newer than the pinned reference, which keeps only the text up to the next colon)", + "result": "a" + }, + { + "name": "missing variable name", + "fn": "containerSubstitute", + "args": [ + "linux", + "/w/.devcontainer.json", + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv}" + ], + "error": "'${containerEnv}' in .devcontainer.json can not be resolved because no environment variable name is given." + }, + { + "name": "missing variable name without config", + "fn": "containerSubstitute", + "args": [ + "linux", + null, + { + "PATH": "/usr/bin:/usr/local/bin", + "SHELL": "/bin/bash" + }, + "${containerEnv}" + ], + "error": "'${containerEnv}' can not be resolved because no environment variable name is given." + }, + { + "name": "windows: case-insensitive", + "fn": "containerSubstitute", + "args": [ + "win32", + null, + { + "Path": "x" + }, + "${containerEnv:PATH}" + ], + "result": "x" + } + ] +} diff --git a/internal/config/varsub.go b/internal/config/varsub.go index a5b6770..cc6c002 100644 --- a/internal/config/varsub.go +++ b/internal/config/varsub.go @@ -4,10 +4,13 @@ import ( "crypto/sha256" "encoding/json" "fmt" + "maps" "math/big" "path" "runtime" + "slices" "strings" + "unicode/utf16" ) type SubstitutionPhase uint8 @@ -201,7 +204,7 @@ func resolveHostTag(ctx HostSubContext, variable string, args []string, tag stri if isWin { return winBasename(ctx.LocalWorkspaceFolder), true, nil } - return path.Base(ctx.LocalWorkspaceFolder), true, nil + return posixBasename(ctx.LocalWorkspaceFolder), true, nil } case "containerWorkspaceFolder": if ctx.ContainerWorkspaceFolder != "" { @@ -209,7 +212,7 @@ func resolveHostTag(ctx HostSubContext, variable string, args []string, tag stri } case "containerWorkspaceFolderBasename": if ctx.ContainerWorkspaceFolder != "" { - return path.Base(ctx.ContainerWorkspaceFolder), true, nil + return posixBasename(ctx.ContainerWorkspaceFolder), true, nil } } return "", false, nil @@ -221,7 +224,7 @@ func resolveEnvTag(isWin bool, env map[string]string, args []string, tag, config if configFilePath != "" { location = " in " + path.Base(configFilePath) } - return "", false, fmt.Errorf("'${%s}'%s cannot be resolved because no environment variable name is given", tag, location) + return "", false, fmt.Errorf("'${%s}'%s can not be resolved because no environment variable name is given.", tag, location) } name := args[0] if isWin { @@ -249,21 +252,67 @@ type HostSubContext struct { ConfigFilePath string // for error messages } -// ComputeDevContainerID produces a deterministic ID from id-labels, -// matching the TS implementation exactly: SHA256 → BigInt → base32 padded to 52. +// ComputeDevContainerID produces a deterministic ID from id-labels, as TS +// devcontainerIdForLabels does: the labels' JSON.stringify with sorted keys, +// SHA-256, then the hash as a base-32 number padded to 52 digits. func ComputeDevContainerID(idLabels map[string]string) string { - // encoding/json writes a map's keys sorted, as the TS JSON.stringify of the - // sorted labels does. - data, _ := json.Marshal(idLabels) - hash := sha256.Sum256(data) - - // Convert hash to BigInt, then to base32 string, padded to 52 chars - n := new(big.Int).SetBytes(hash[:]) - str := n.Text(32) + hash := sha256.Sum256([]byte(jsonStringifySorted(idLabels))) + str := new(big.Int).SetBytes(hash[:]).Text(32) // A 256-bit hash has at most 52 base-32 digits. return strings.Repeat("0", 52-len(str)) + str } +// jsonStringifySorted is JSON.stringify(m, Object.keys(m).sort()): keys in +// UTF-16 code unit order (JS's sort), strings escaped as JSON.stringify does +// (encoding/json would also escape &, <, >, U+2028 and U+2029). +func jsonStringifySorted(m map[string]string) string { + keys := slices.Collect(maps.Keys(m)) + slices.SortFunc(keys, func(a, b string) int { + return slices.Compare(utf16.Encode([]rune(a)), utf16.Encode([]rune(b))) + }) + var b strings.Builder + b.WriteByte('{') + for i, k := range keys { + if i > 0 { + b.WriteByte(',') + } + writeJSString(&b, k) + b.WriteByte(':') + writeJSString(&b, m[k]) + } + b.WriteByte('}') + return b.String() +} + +func writeJSString(b *strings.Builder, s string) { + b.WriteByte('"') + for _, r := range s { + switch r { + case '"': + b.WriteString(`\"`) + case '\\': + b.WriteString(`\\`) + case '\b': + b.WriteString(`\b`) + case '\f': + b.WriteString(`\f`) + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\t': + b.WriteString(`\t`) + default: + if r < 0x20 { + fmt.Fprintf(b, `\u%04x`, r) + } else { + b.WriteRune(r) + } + } + } + b.WriteByte('"') +} + func normalizeEnvKeys(env map[string]string) map[string]string { m := make(map[string]string, len(env)) for k, v := range env { @@ -272,9 +321,21 @@ func normalizeEnvKeys(env map[string]string) map[string]string { return m } +// posixBasename is node's path.posix.basename: trailing slashes are ignored, +// and "/" has none. +func posixBasename(p string) string { + p = strings.TrimRight(p, "/") + return p[strings.LastIndexByte(p, '/')+1:] +} + +// winBasename is node's path.win32.basename: both / and \ separate, a drive +// letter is no name, trailing separators are ignored. func winBasename(p string) string { - // Both / and \ separate on Windows; no separator leaves p whole. - return p[strings.LastIndexAny(p, "/\\")+1:] + if len(p) >= 2 && p[1] == ':' && ('a' <= p[0]|0x20 && p[0]|0x20 <= 'z') { + p = p[2:] + } + p = strings.TrimRight(p, `/\`) + return p[strings.LastIndexAny(p, `/\`)+1:] } // IsWindows returns true if the current platform is Windows. diff --git a/internal/config/varsub_test.go b/internal/config/varsub_test.go index 1626e87..19298c0 100644 --- a/internal/config/varsub_test.go +++ b/internal/config/varsub_test.go @@ -2,7 +2,6 @@ package config import ( "errors" - "reflect" "strings" "testing" ) @@ -35,18 +34,6 @@ func TestVariableResolverBeforeContainerPhases(t *testing.T) { } } -func TestVariableResolverEnvironmentErrorIncludesConfig(t *testing.T) { - resolver := NewVariableResolver() - _, err := resolver.Resolve(SubstitutionContext{ - ConfigFilePath: "/work/.devcontainer/devcontainer.json"}, PhaseHost, "${localEnv}") - if err == nil { - t.Fatal("expected missing environment variable name to fail") - } - if !strings.Contains(err.Error(), "devcontainer.json") { - t.Errorf("error does not identify config file: %v", err) - } -} - func TestVariableResolverResolveInto(t *testing.T) { type value struct { Env map[string]string `json:"env"` @@ -61,131 +48,6 @@ func TestVariableResolverResolveInto(t *testing.T) { } } -func TestVariableResolverHostPhase(t *testing.T) { - localEnvCtx := HostSubContext{ - Platform: "linux", - Env: map[string]string{"HOME": "/home/user", "USER": "test"}, - } - workspaceCtx := HostSubContext{ - Platform: "linux", - LocalWorkspaceFolder: "/home/user/project", - ContainerWorkspaceFolder: "/workspaces/project", - Env: map[string]string{}, - } - emptyCtx := HostSubContext{Platform: "linux", Env: map[string]string{}} - recursiveCtx := HostSubContext{ - Platform: "linux", - LocalWorkspaceFolder: "/home/user/project", - Env: map[string]string{"HOME": "/home/user"}, - } - - tests := []struct { - name string - ctx HostSubContext - input any - want any - }{ - // localEnv substitution - {"localEnv HOME", localEnvCtx, "${localEnv:HOME}", "/home/user"}, - {"env alias", localEnvCtx, "${env:HOME}", "/home/user"}, - {"localEnv with suffix", localEnvCtx, "${localEnv:HOME}/project", "/home/user/project"}, - {"localEnv missing", localEnvCtx, "${localEnv:MISSING}", ""}, - {"localEnv missing with fallback", localEnvCtx, "${localEnv:MISSING:fallback}", "fallback"}, - // A default value may itself contain colons (URLs, image:tag, host:port) — - // only the first colon separates the var name from its default (#1213). - {"localEnv fallback with colons", localEnvCtx, "${localEnv:MISSING:my.registry.io:5000/img:tag}", "my.registry.io:5000/img:tag"}, - {"localEnv present ignores colon fallback", localEnvCtx, "${localEnv:HOME:a:b:c}", "/home/user"}, - {"no vars", localEnvCtx, "no vars here", "no vars here"}, - {"multiple localEnv", localEnvCtx, "${localEnv:USER}@${localEnv:HOME}", "test@/home/user"}, - - // workspace folder substitution - {"localWorkspaceFolder", workspaceCtx, "${localWorkspaceFolder}", "/home/user/project"}, - {"localWorkspaceFolderBasename", workspaceCtx, "${localWorkspaceFolderBasename}", "project"}, - {"containerWorkspaceFolder", workspaceCtx, "${containerWorkspaceFolder}", "/workspaces/project"}, - {"containerWorkspaceFolderBasename", workspaceCtx, "${containerWorkspaceFolderBasename}", "project"}, - - // unknown variables pass through unchanged - {"unknown variable", emptyCtx, "${unknownVar}", "${unknownVar}"}, - - // non-string values pass through unchanged - {"number passthrough", emptyCtx, 42.0, 42.0}, - {"bool passthrough", emptyCtx, true, true}, - {"nil passthrough", emptyCtx, nil, nil}, - - // recursive substitution into nested maps and slices - { - "recursive map and slice", - recursiveCtx, - map[string]any{ - "workspaceFolder": "${localWorkspaceFolder}", - "env": map[string]any{ - "HOME": "${localEnv:HOME}", - }, - "ports": []any{"${localEnv:HOME}:8080"}, - }, - map[string]any{ - "workspaceFolder": "/home/user/project", - "env": map[string]any{ - "HOME": "/home/user", - }, - "ports": []any{"/home/user:8080"}, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := NewVariableResolver().Resolve(SubstitutionContext{HostSubContext: tt.ctx}, PhaseHost, tt.input) - if err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("host Resolve(%v) = %v, want %v", tt.input, got, tt.want) - } - }) - } -} - -func TestVariableResolverIdentityPhase(t *testing.T) { - labels := map[string]string{ - "devcontainer.local_folder": "/home/user/project", - } - id := ComputeDevContainerID(labels) - if id == "" { - t.Fatal("expected non-empty id") - } - if len(id) != 52 { - t.Errorf("id length = %d, want 52", len(id)) - } - - // Deterministic - id2 := ComputeDevContainerID(labels) - if id != id2 { - t.Errorf("id not deterministic: %q != %q", id, id2) - } - - // Substitute into value - result, err := NewVariableResolver().Resolve(SubstitutionContext{IDLabels: labels}, PhaseIdentity, "${devcontainerId}") - if err != nil { - t.Fatal(err) - } - if result != id { - t.Errorf("substitution = %q, want %q", result, id) - } -} - -func TestComputeDevContainerIDSortedKeys(t *testing.T) { - labels1 := map[string]string{"a": "1", "b": "2"} - labels2 := map[string]string{"b": "2", "a": "1"} - - id1 := ComputeDevContainerID(labels1) - id2 := ComputeDevContainerID(labels2) - - if id1 != id2 { - t.Errorf("key order should not matter: %q != %q", id1, id2) - } -} - func TestVariableResolverIdentityStrings(t *testing.T) { tests := []struct { name string @@ -225,38 +87,6 @@ func TestSubstituteTemplateOptions(t *testing.T) { } } -func TestVariableResolverContainerPhase(t *testing.T) { - containerEnv := map[string]string{ - "PATH": "/usr/bin:/usr/local/bin", - "SHELL": "/bin/bash", - } - - tests := []struct { - input string - want string - }{ - {"${containerEnv:PATH}", "/usr/bin:/usr/local/bin"}, - {"${containerEnv:SHELL}", "/bin/bash"}, - {"${containerEnv:MISSING}", ""}, - {"no vars", "no vars"}, - // Non-container vars should pass through - {"${localEnv:HOME}", "${localEnv:HOME}"}, - } - - for _, tt := range tests { - got, err := NewVariableResolver().AfterContainer(SubstitutionContext{ - Platform: "linux", - ContainerEnv: containerEnv, - }, tt.input) - if err != nil { - t.Fatal(err) - } - if got != tt.want { - t.Errorf("container Resolve(%q) = %q, want %q", tt.input, got, tt.want) - } - } -} - func TestSubstituteTags(t *testing.T) { upper := func(tag string) (string, bool, error) { return strings.ToUpper(tag), true, nil } diff --git a/internal/devtools/command.go b/internal/devtools/command.go index 05b3cd2..9bdd6ac 100644 --- a/internal/devtools/command.go +++ b/internal/devtools/command.go @@ -47,6 +47,8 @@ func Main(args []string, stdin io.Reader, stdout, stderr io.Writer) int { err = t.releaseResolve() case "metrics": err = t.metrics(rest) + case "ts-oracle": + err = t.tsOracle() default: fmt.Fprintf(stderr, "devtool: unknown command %q\n", cmd) return 2 @@ -547,3 +549,51 @@ func (t tool) metrics(args []string) error { fmt.Fprintf(t.out, "metrics: wrote %s\n%s\n", out, data) return nil } + +// tsOracle rewrites every internal/**/testdata/ts-oracle/*.json with the +// answers the reference's own functions give to the golden's requests. +func (t tool) tsOracle() error { + dir, err := os.MkdirTemp("", "ts-oracle-") + if err != nil { + return err + } + defer os.RemoveAll(dir) + bundle := filepath.Join(dir, "oracle.js") + if err := t.run("node", "tools/tsoracle/build.mjs", "reference", bundle); err != nil { + return fmt.Errorf("bundle tools/tsoracle/oracle.ts: %w", err) + } + paths, err := TSOracleGoldens(os.DirFS(".")) + if err != nil { + return err + } + goldens := map[string][]byte{} + for _, p := range paths { + if goldens[p], err = os.ReadFile(p); err != nil { + return err + } + } + refreshed, err := RefreshTSOracleGoldens(goldens, func(request []byte) ([]byte, error) { + cmd := exec.Command("node", bundle) + cmd.Stdin = strings.NewReader(string(request)) + cmd.Stderr = t.errw + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("run the oracle: %w", err) + } + return out, nil + }) + if err != nil { + return err + } + for _, p := range paths { + if string(refreshed[p]) == string(goldens[p]) { + continue + } + if err := os.WriteFile(p, refreshed[p], 0o644); err != nil { + return err + } + fmt.Fprintln(t.errw, "updated", p) + } + fmt.Fprintf(t.errw, "%d ts-oracle goldens checked\n", len(paths)) + return nil +} diff --git a/internal/devtools/tsoracle.go b/internal/devtools/tsoracle.go new file mode 100644 index 0000000..000d8eb --- /dev/null +++ b/internal/devtools/tsoracle.go @@ -0,0 +1,110 @@ +package devtools + +import ( + "bytes" + "encoding/json" + "fmt" + "io/fs" + "maps" + "path" + "slices" + + "github.com/devcontainers/cli/internal/tsoracle" +) + +// TSOracleGoldens lists every internal/**/testdata/ts-oracle/*.json of fsys +// (the repository root), sorted. +func TSOracleGoldens(fsys fs.FS) ([]string, error) { + var files []string + err := fs.WalkDir(fsys, "internal", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && path.Ext(p) == ".json" && path.Base(path.Dir(p)) == "ts-oracle" && + path.Base(path.Dir(path.Dir(p))) == "testdata" { + files = append(files, p) + } + return nil + }) + slices.Sort(files) + return files, err +} + +// RefreshTSOracleGoldens sends the cases of every golden (path -> content) to +// oracle in one request and returns each golden rewritten with the results +// oracle recorded. oracle reads and writes the tools/tsoracle protocol: +// {"cases":[{"fn","args"}]} in, the same cases with "result" or "error" out. +func RefreshTSOracleGoldens(goldens map[string][]byte, oracle func(request []byte) ([]byte, error)) (map[string][]byte, error) { + paths := slices.Sorted(maps.Keys(goldens)) + type call struct { + Fn string `json:"fn"` + Args []json.RawMessage `json:"args"` + } + var request struct { + Cases []call `json:"cases"` + } + request.Cases = []call{} + files := make([]*tsoracle.File, len(paths)) + for i, p := range paths { + f, err := tsoracle.Decode(goldens[p]) + if err != nil { + return nil, fmt.Errorf("%s: %w", p, err) + } + files[i] = f + for _, c := range f.Cases { + args := c.Args + if args == nil { + args = []json.RawMessage{} + } + request.Cases = append(request.Cases, call{Fn: c.Fn, Args: args}) + } + } + var body bytes.Buffer + enc := json.NewEncoder(&body) + enc.SetEscapeHTML(false) + if err := enc.Encode(request); err != nil { + return nil, err + } + out, err := oracle(bytes.TrimSuffix(body.Bytes(), []byte("\n"))) + if err != nil { + return nil, err + } + var response struct { + Cases []struct { + Fn string `json:"fn"` + Result json.RawMessage `json:"result"` + Error *string `json:"error"` + } `json:"cases"` + } + if err := json.Unmarshal(out, &response); err != nil { + return nil, fmt.Errorf("oracle response: %w", err) + } + if len(response.Cases) != len(request.Cases) { + return nil, fmt.Errorf("oracle answered %d cases, %d were sent", len(response.Cases), len(request.Cases)) + } + refreshed := make(map[string][]byte, len(paths)) + next := 0 + for i, p := range paths { + for j := range files[i].Cases { + c, r := &files[i].Cases[j], response.Cases[next] + next++ + if r.Fn != c.Fn { + return nil, fmt.Errorf("%s: case %d: oracle answered %s for %s", p, j, r.Fn, c.Fn) + } + switch { + case r.Error != nil: + c.Result, c.Error = nil, *r.Error + case r.Result != nil: + c.Result, c.Error = r.Result, "" + default: + return nil, fmt.Errorf("%s: case %d: oracle recorded neither a result nor an error", p, j) + } + } + data, err := tsoracle.Encode(files[i]) + if err != nil { + return nil, fmt.Errorf("%s: %w", p, err) + } + refreshed[p] = data + } + return refreshed, nil +} diff --git a/internal/devtools/tsoracle_test.go b/internal/devtools/tsoracle_test.go new file mode 100644 index 0000000..8c9c57f --- /dev/null +++ b/internal/devtools/tsoracle_test.go @@ -0,0 +1,154 @@ +package devtools + +import ( + "encoding/json" + "errors" + "slices" + "strings" + "testing" + "testing/fstest" +) + +func TestTSOracleGoldens(t *testing.T) { + fsys := fstest.MapFS{ + "internal/b/testdata/ts-oracle/x.json": {}, + "internal/a/testdata/ts-oracle/y.json": {}, + "internal/a/testdata/ts-oracle/notes.txt": {}, + "internal/a/testdata/other/z.json": {}, + "internal/a/ts-oracle/w.json": {}, + "internal/a/testdata/ts-oracle/sub/v.json": {}, + } + got, err := TSOracleGoldens(fsys) + if err != nil { + t.Fatal(err) + } + want := []string{"internal/a/testdata/ts-oracle/y.json", "internal/b/testdata/ts-oracle/x.json"} + if !slices.Equal(got, want) { + t.Errorf("goldens = %v, want %v", got, want) + } +} + +// fakeOracle answers parseVersion with its argument's length and throws for +// "boom", recording the request it was sent. +func fakeOracle(sent *[]byte) func([]byte) ([]byte, error) { + return func(req []byte) ([]byte, error) { + *sent = req + var in struct { + Cases []struct { + Fn string `json:"fn"` + Args []any `json:"args"` + } `json:"cases"` + } + if err := json.Unmarshal(req, &in); err != nil { + return nil, err + } + out := []map[string]any{} + for _, c := range in.Cases { + r := map[string]any{"fn": c.Fn, "args": c.Args} + if c.Fn == "boom" { + r["error"] = "it threw" + } else { + r["result"] = len(c.Args[0].(string)) + } + out = append(out, r) + } + return json.Marshal(map[string]any{"cases": out}) + } +} + +func TestRefreshTSOracleGoldens(t *testing.T) { + goldens := map[string][]byte{ + "internal/b/testdata/ts-oracle/b.json": []byte(`{"cases":[{"fn":"boom","args":["x"],"result":1}]}`), + "internal/a/testdata/ts-oracle/a.json": []byte(`{"comment":"kept","cases":[ + {"name":"n","fn":"len","args":["<&>"],"divergence":"why","error":"stale"}, + {"fn":"len","args":["ab"]}]}`), + } + var sent []byte + got, err := RefreshTSOracleGoldens(goldens, fakeOracle(&sent)) + if err != nil { + t.Fatal(err) + } + // One request, the files' cases in path order, only fn and args. + if want := `{"cases":[{"fn":"len","args":["<&>"]},{"fn":"len","args":["ab"]},{"fn":"boom","args":["x"]}]}`; string(sent) != want { + t.Errorf("request = %s, want %s", sent, want) + } + wantA := `{ + "comment": "kept", + "cases": [ + { + "name": "n", + "fn": "len", + "args": [ + "<&>" + ], + "divergence": "why", + "result": 3 + }, + { + "fn": "len", + "args": [ + "ab" + ], + "result": 2 + } + ] +} +` + if s := string(got["internal/a/testdata/ts-oracle/a.json"]); s != wantA { + t.Errorf("a.json =\n%s\nwant\n%s", s, wantA) + } + if s := string(got["internal/b/testdata/ts-oracle/b.json"]); !strings.Contains(s, `"error": "it threw"`) || strings.Contains(s, `"result"`) { + t.Errorf("b.json = %s, want the thrown error in place of the stale result", s) + } + // Refreshing a refreshed golden changes nothing. + again, err := RefreshTSOracleGoldens(got, fakeOracle(&sent)) + if err != nil { + t.Fatal(err) + } + for p := range got { + if string(again[p]) != string(got[p]) { + t.Errorf("%s is not stable:\n%s\nthen\n%s", p, got[p], again[p]) + } + } +} + +func TestRefreshTSOracleGoldensNullResult(t *testing.T) { + goldens := map[string][]byte{"g.json": []byte(`{"cases":[{"fn":"f","args":[]}]}`)} + got, err := RefreshTSOracleGoldens(goldens, func([]byte) ([]byte, error) { + return []byte(`{"cases":[{"fn":"f","args":[],"result":null}]}`), nil + }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got["g.json"]), `"result": null`) { + t.Errorf("g.json = %s, want the null result recorded", got["g.json"]) + } +} + +func TestRefreshTSOracleGoldensRejects(t *testing.T) { + oracleErr := errors.New("node failed") + tests := []struct { + name string + golden string + answer string + err error + wantErr string + }{ + {"bad golden", `{"cases":[{"args":[]}]}`, ``, nil, "no fn"}, + {"unknown golden field", `{"cases":[],"extra":1}`, ``, nil, "unknown field"}, + {"oracle fails", `{"cases":[{"fn":"f","args":[]}]}`, ``, oracleErr, "node failed"}, + {"missing case", `{"cases":[{"fn":"f","args":[]}]}`, `{"cases":[]}`, nil, "answered 0 cases, 1 were sent"}, + {"other function", `{"cases":[{"fn":"f","args":[]}]}`, `{"cases":[{"fn":"g","result":1}]}`, nil, "answered g for f"}, + {"no result", `{"cases":[{"fn":"f","args":[]}]}`, `{"cases":[{"fn":"f"}]}`, nil, "neither a result nor an error"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := RefreshTSOracleGoldens(map[string][]byte{"g.json": []byte(tc.golden)}, func([]byte) ([]byte, error) { + return []byte(tc.answer), tc.err + }) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("err = %v, want it to mention %q", err, tc.wantErr) + } + }) + } +} diff --git a/internal/devtools/versions_gate_test.go b/internal/devtools/versions_gate_test.go index 4dfe1fd..fff337a 100644 --- a/internal/devtools/versions_gate_test.go +++ b/internal/devtools/versions_gate_test.go @@ -1,6 +1,7 @@ package devtools import ( + "os" "testing" "github.com/spin-stack/go-tools/versions/gate" @@ -13,11 +14,17 @@ import ( // digests are what the parity tests compare; updateUID.Dockerfile is upstream's, // shipped in the binary, and its BASE_IMAGE is the user's image, handed in at run // time (docker.BuildUIDImage), not a pin; internal/imagemeta/testdata/ holds -// Dockerfiles the TS CLI generated, whose ARG defaults are its output, not pins. +// Dockerfiles the TS CLI generated, whose ARG defaults are its output, not pins; +// the testdata/ts-oracle goldens hold digests as inputs of reference functions. func TestVersionsYAMLIsTheOnlyPin(t *testing.T) { g := gate.Gate{Root: "../..", Elsewhere: []string{ "go.sum", "reference/", "src/test/", "internal/docker/updateUID.Dockerfile", "internal/imagemeta/testdata/", }} + goldens, err := TSOracleGoldens(os.DirFS("../..")) + if err != nil { + t.Fatal(err) + } + g.Elsewhere = append(g.Elsewhere, goldens...) if err := g.Check(); err != nil { t.Error(err) } diff --git a/internal/docker/dockerfile.go b/internal/docker/dockerfile.go index 779b76c..1a60462 100644 --- a/internal/docker/dockerfile.go +++ b/internal/docker/dockerfile.go @@ -1,13 +1,11 @@ package docker import ( - "maps" - "slices" + "regexp" "strings" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/moby/buildkit/frontend/dockerfile/parser" - "github.com/moby/buildkit/frontend/dockerfile/shell" ) // Dockerfile is a parsed Dockerfile projected from BuildKit's AST into the small @@ -82,7 +80,7 @@ func ExtractDockerfile(content string) *Dockerfile { for i := range stages { s := &stages[i] - st := Stage{From: From{Platform: s.Platform, Image: trimQuotes(s.BaseName), Label: s.Name}} + st := Stage{From: From{Platform: s.Platform, Image: trimQuotes(s.BaseName), Label: stageLabel(s)}} for _, cmd := range s.Commands { switch c := cmd.(type) { case *instructions.EnvCommand: @@ -111,6 +109,15 @@ func ExtractDockerfile(content string) *Dockerfile { return df } +// stageLabel is the stage's name as the FROM line spells it: BuildKit +// lowercases it, TS extractDockerfile keeps it, and looks stages up by it. +func stageLabel(s *instructions.Stage) string { + if m := fromLineLabel.FindStringSubmatch(s.SourceCode); m != nil && strings.EqualFold(m[1], s.Name) { + return m[1] + } + return s.Name +} + // StageLabels returns the LABEL key/value pairs declared in the named build // stage (or the final stage when name is ""), with later labels overriding // earlier ones. Used to recover a user's `LABEL devcontainer.metadata` from the @@ -160,144 +167,46 @@ func derefTrim(p *string) string { return trimQuotes(*p) } -// mapEnv is a shell.EnvGetter backed by a plain map. -type mapEnv map[string]string - -func (m mapEnv) Get(k string) (string, bool) { v, ok := m[k]; return v, ok } -func (m mapEnv) Keys() []string { return slices.Collect(maps.Keys(m)) } - -// FindBaseImage resolves the base image for a target stage (or the last stage), -// following FROM references to the root image. ${VAR} / ${VAR:+x} / -// ${VAR:-x} in the FROM line are expanded with BuildKit's shell lexer against the -// global (preamble) ARGs plus --build-arg overrides — the same env Docker uses to -// resolve a FROM line. +// FindBaseImage is TS findBaseImage without platform ARGs: the image the +// target stage (or the last one) is built from, following FROM to the +// first image that is no stage; "" when the target is unknown or the stages +// form a cycle. func FindBaseImage(df *Dockerfile, buildArgs map[string]string, target string) string { - var stage *Stage - if target != "" { - stage = df.StagesByLabel[target] - } else if len(df.Stages) > 0 { - stage = &df.Stages[len(df.Stages)-1] - } - - env := mapEnv{} - for _, ins := range df.Preamble.Instructions { - if ins.Instruction == "ARG" { - v := ins.Value - if ov, ok := buildArgs[ins.Name]; ok { - v = ov - } - env[ins.Name] = v - } - } - - lex := shell.NewLex(df.escapeToken) - seen := make(map[*Stage]bool) - for stage != nil { - if seen[stage] { - return "" - } - seen[stage] = true - - image := stage.From.Image - if out, _, err := lex.ProcessWord(image, env); err == nil { - image = trimQuotes(out) - } - if next, ok := df.StagesByLabel[image]; ok { - stage = next - } else { - return image - } - } - return "" + return NewResolver(df, buildArgs, nil).FindBaseImage(target) } -// EnsureFinalStageName returns the name of the final build stage, adding an -// "AS " to its FROM line when it has none (so features can extend a -// known stage). The modified Dockerfile is returned only when a name was added; -// otherwise modifiedContent is empty. -func EnsureFinalStageName(content, defaultName string) (stageName string, modifiedContent string) { - // Primary detection via BuildKit (continuation-safe). Trust a positive name; - // on a parse error (e.g. a trailing inline comment the strict parser rejects - // but the TS tolerates) or no name, fall through to the tolerant scan below. - if res, err := parser.Parse(strings.NewReader(content)); err == nil { - // instructions.Parse returns no stages with its error, so an error lands in - // the tolerant scan like "no stages" does. - if stages, _, _ := instructions.Parse(res.AST, nil); len(stages) > 0 { - if name := stages[len(stages)-1].Name; name != "" { - return name, "" - } - } - } +// jsSpace is the JavaScript regular expression class \s (Go's \s lacks \v and +// the Unicode spaces), so the ports of TS regular expressions match the same text. +const jsSpace = `\t\n\v\f\r \x{a0}\x{1680}\x{2000}-\x{200a}\x{2028}\x{2029}\x{202f}\x{205f}\x{3000}\x{feff}` + +var ( + // findFromLines is TS dockerfileUtils' /^(?\s*FROM.*)/gmi; JS's . stops + // at \r, \u2028 and \u2029 as well as \n. + findFromLines = regexp.MustCompile(`(?mi)^[` + jsSpace + `]*FROM[^\n\r\x{2028}\x{2029}]*`) + // fromLineLabel is TS dockerfileUtils' parseFromLine: + // /FROM\s+(?--platform=\S+\s+)?(?"?[^\s]+"?)(\s+AS\s+(?