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..979fd69 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -248,6 +248,18 @@ 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 test ./internal/devtools -run '^TestTSOracleGoldensMatchTheReference$' -count=1 -v + reference: desc: Install and compile the TypeScript parity oracle cmds: @@ -267,6 +279,8 @@ tasks: # Oracle flag-surface gate: every flag the TS oracle declares per command # must be in the inventory (needs reference/ checked out, so it lives here). - go test ./internal/cli -run TestOracleFlagCoverage -count=1 + # The ts-oracle goldens must be what the reference's functions return. + - go test ./internal/devtools -run '^TestTSOracleGoldensMatchTheReference$' -count=1 - PARITY_REPORT_FILE={{.ROOT_DIR}}/artifacts/parity-contract.json PARITY_STRICT=true PARITY_LANE=contract PARITY_SKIP_DOCKER=true PARITY_SKIP_NETWORK=true go test ./internal/cli -run TestParityMatrix -count=1 -timeout 10m -v {{.CLI_ARGS}} parity:semantic: @@ -282,10 +296,10 @@ tasks: - PARITY_REPORT_FILE={{.ROOT_DIR}}/artifacts/parity-network.json PARITY_STRICT=true PARITY_LANE=all PARITY_NETWORK_ONLY=true PARITY_SKIP_DOCKER=true go test ./internal/cli -run TestParityMatrix -count=1 -timeout 15m -v {{.CLI_ARGS}} parity:differential: - desc: Compare read-configuration with the TS oracle over every upstream fixture (reference/src/test/configs); needs Docker and the network + desc: Compare read-configuration, up and build with the TS oracle over every upstream fixture (reference/src/test/configs); needs Docker and the network deps: [build, reference:compile] cmds: - - PARITY_DIFFERENTIAL=1 go test ./internal/cli -run TestDifferentialReadConfiguration -count=1 -timeout 30m -v {{.CLI_ARGS}} + - PARITY_DIFFERENTIAL=1 go test ./internal/cli -run '^TestDifferential' -count=1 -timeout 180m -v {{.CLI_ARGS}} parity:runtime: desc: Run the runtime parity matrix with Docker (set PARITY_COMMAND=up,build to scope it) diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index 0a580b0..a2235e6 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -20,6 +20,20 @@ touches a compared surface, reflected in the parity matrix. `stop`/`down` (pause, or stop+remove, a workspace's container — Compose-aware; the upstream CLI leaves teardown to the editor), and the automatic credential bridge that hands the CLI's resolved auth to `docker build`. +- **A final `FROM` line continued with a backslash keeps its stage name.** TS + `ensureDockerfileHasFinalStageName` reads only the physical line, so for + `FROM \` followed by `alpine AS base` it writes `FROM \ AS dev`, a Dockerfile + no builder accepts. This CLI asks BuildKit for that stage's name and leaves the + Dockerfile as it is (`TestEnsureFinalStageNameReadsAContinuedFrom`, + `internal/docker/testdata/ts-oracle/dockerfile.json`). Every other FROM line is + handled exactly as TS handles it. +- **Compose builds may read the Features Dockerfile outside the build context.** Compose + builds through bake, and bake on buildx 0.38 and later refuses to read a file outside the + context unless the caller passes `--allow=fs.read`, which `docker compose build` cannot. + The reference writes the Features Dockerfile under its cache folder and fails there with + "additional privileges requested". This CLI starts its `docker compose` processes with + `BUILDX_BAKE_ENTITLEMENTS_FS=0` unless the variable is already set + (`TestWithBakeFSEntitlement`, `TestE2E_ComposeBuildLabel`). - **`--override-config` deep-merges** the override onto the base config, whereas TS replaces the config wholesale (`readDocument(overrideConfigFile ?? configFile)`). With no readable base, the override stands alone — identical to TS. This lets an orchestrator @@ -45,6 +59,28 @@ 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`). +- **The `devcontainer.metadata` label writes object values with their keys sorted.** The + label is TS's `JSON.stringify` of the entries: each entry's properties in TS's pick + order (`pickFeatureProperties` for a Feature, `pickConfigProperties` for the + configuration) and `&`, `<`, `>` unescaped, as in TS. Within a property's object value + (a mount object, `customizations`, `portsAttributes`, …) the keys are sorted, because a + decoded Go map keeps no order, where TS keeps the order they were written in; an entry + given in another order (a base image's label written by another tool) is likewise + rewritten in pick order. The parsed metadata is the same; only the label's bytes differ + (`internal/imagemeta/testdata/ts-oracle/imagemeta.json` records both cases). ## Firm decisions & scope diff --git a/docs/go-only-features.md b/docs/go-only-features.md index b3866b1..1c7c190 100644 --- a/docs/go-only-features.md +++ b/docs/go-only-features.md @@ -135,8 +135,11 @@ Flags: `--workspace-folder`, `--config`, `--id-label`, `--container-id`, Boot the container from an already-built image (features baked in), **skipping the image build and feature installation entirely**. The merged configuration is recovered from the -image's `devcontainer.metadata` label — exactly as an image-based config would — while -`remoteUser`, mounts and lifecycle hooks still come from `devcontainer.json`. +image's `devcontainer.metadata` label, which a `devcontainer build` of the configuration +writes with the configuration's entry (its mounts, lifecycle hooks, …), as for any +container the CLI creates; `remoteUser`, `userEnvProbe` and `remoteEnv` come from the +current `devcontainer.json`. An image without that label is used as an image-based +config's image is: the whole configuration comes from `devcontainer.json`. ```sh devcontainer up --workspace-folder . \ @@ -184,6 +187,10 @@ Each `KEY` becomes `--secret id=KEY,env=KEY`; the value is delivered through the subprocess environment, never as an argument. Requires buildx (ignored with a warning on the legacy builder). +A Docker Compose configuration is built by `docker compose build`, as the reference CLI +builds it, which takes no build secrets from the command line: there the flag is ignored +with a warning. + ## Automatic credential bridge Not a flag — a behavior. The upstream CLI's credential chain applies only to its *own* diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index a2c08c4..d55d6da 100644 --- a/docs/parity/parity-matrix.yaml +++ b/docs/parity/parity-matrix.yaml @@ -856,6 +856,30 @@ initial_cases: current_status: match notes: "The compose service's Features Dockerfile (appended to `FROM AS dev_container_auto_added_stage_label`) has the same structure as the image build's: builtin env, consecutiveId folders, install wrapper, the Feature's containerEnv and, like TS getFeaturesBuildOptions, the config's containerEnv as ENV lines (CFG_VAR). Go used the _dev_container_feature_ layout and left the config's containerEnv out of the compose image." + - id: read-configuration.container-id-missing-error + lane: runtime + command: read-configuration + priority: p1 + docker_required: true + ts_cmd: "read-configuration --container-id devcontainer-cli-parity-no-such-container" + asserts: [exit_code, stdout_normalized] + class: container-runtime + current_status: match + notes: "A --container-id that cannot be inspected fails read-configuration: TS findContainerAndIdLabels runs inspectContainer, whose `docker inspect` failure throws, so the command exits 1 with nothing on stdout. Go used to ignore the inspect failure and print the configuration." + + - id: exec.config-remote-user-probe-success + lane: runtime + command: exec + priority: p1 + docker_required: true + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; IMG=parity-exec-user-${PARITY_CASE_ID}-${PARITY_SIDE}; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/img $WORKSPACE/.devcontainer; printf 'FROM debian:latest\\nRUN useradd --create-home foo\\n' > $WORKSPACE/img/Dockerfile; docker build -q -t $IMG $WORKSPACE/img >/dev/null; printf '{\\042image\\042:\\042%s\\042}' $IMG > $WORKSPACE/.devcontainer/devcontainer.json; if [ \"${PARITY_SIDE}\" = \"ts\" ]; then ${PARITY_CLI_TS} up --workspace-folder $WORKSPACE --skip-post-create --update-remote-user-uid-default never --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null; else ${PARITY_CLI_GO} up --workspace-folder $WORKSPACE --skip-post-create --update-remote-user-uid-default never --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null; fi; printf '{\\042image\\042:\\042%s\\042,\\042remoteUser\\042:\\042foo\\042}' $IMG > $WORKSPACE/.devcontainer/devcontainer.json; echo WORKSPACE=$WORKSPACE" + ts_cmd: "exec --workspace-folder ${WORKSPACE} --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE} sh -c 'printf %s:%s:%s \"$HOME\" \"$(id -un)\" \"$PATH\"'" + cleanup_cmd: "IDS=$(docker ps -aq --filter label=parity.case=${PARITY_CASE_ID}-${PARITY_SIDE}); [ -n \"$IDS\" ] && docker rm -f $IDS >/dev/null 2>&1; docker rmi -f parity-exec-user-${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null 2>&1; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true" + asserts: [exit_code, stdout_normalized] + class: container-runtime + current_status: match + notes: "The container is created without a remoteUser and the config then names foo. exec probes the user environment as the merged configuration's remoteUser (devContainersSpecCLI.ts exec: getImageMetadataFromContainer adds the config's remoteUser, createContainerProperties(..., mergedConfig.remoteUser)), so the probed HOME and PATH are foo's. Go probed as the container's metadata user (root here) and only ran the command as foo, passing root's HOME=/root and PATH." + - id: upgrade.write-lockfile-to-disk-success lane: runtime command: upgrade diff --git a/internal/cli/build.go b/internal/cli/build.go index dff3294..32953d0 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "github.com/devcontainers/cli/internal/config" @@ -108,6 +109,9 @@ type buildRunner struct { docker *docker.Client engine *docker.EngineClient opts *buildOpts + // configLabel is the configuration's entry in the devcontainer.metadata + // label: its properties as written (TS pick(config.raw, ...)). + configLabel imagemeta.Entry } func runBuild(ctx context.Context, out Output, opts *buildOpts) error { @@ -186,6 +190,9 @@ func runBuild(ctx context.Context, out Output, opts *buildOpts) error { defer engine.Close() run := &buildRunner{log: logger, docker: dockerClient, engine: engine, opts: opts} + if run.configLabel, err = configLabelEntry(loadResult.RawConfig, false); err != nil { + return writeErrorResult(out, err.Error()) + } // Detect BuildKit var useBuildx bool @@ -203,19 +210,14 @@ func runBuild(ctx context.Context, out Output, opts *buildOpts) error { if cfg.IsDockerfileConfig() { imageNameResult, err = run.buildDockerfile(ctx, cfg, loadResult, useBuildx) } else if cfg.IsComposeConfig() { - imageNameResult, err = run.buildCompose(ctx, cfg, useBuildx) + imageNameResult, err = run.buildCompose(ctx, cfg, loadResult, useBuildx) } else { // Image-based config imageNameResult, err = run.buildImage(ctx, cfg, loadResult, useBuildx) } if err != nil { - if isUserFacingBuildError(err) { - return writeErrorResult(out, err.Error()) - } - return writeErrorJSON(out, coreerrors.ToErrorOutput(&coreerrors.ContainerError{ - Description: fmt.Sprintf("An error occurred building the container: %v", err), - })) + return writeErrorJSON(out, commandErrorOutput(err, "An error occurred building the container")) } return writeSuccessJSON(out, map[string]any{ @@ -314,7 +316,10 @@ func prepareDockerfileBuild(cfg *config.DevContainer) (*dockerfileBuildPrep, err if cfg.Build != nil && cfg.Build.Target != "" { prep.StageName = cfg.Build.Target } else { - stageName, modified := docker.EnsureFinalStageName(string(content), "dev_container_auto_added_stage_label") + stageName, modified, err := docker.EnsureFinalStageName(string(content), "dev_container_auto_added_stage_label") + if err != nil { + return nil, err + } prep.StageName = stageName if modified != "" { tmpDockerfile := dockerfilePath + ".devcontainer.build" @@ -356,7 +361,14 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai stageName := prep.StageName configDir := filepath.Dir(cfg.ConfigFilePath) - baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), stageName) + // The base image is the FROM of the config's target, else of the last stage + // (TS getImageBuildInfoFromDockerfile with config.build.target), not of the + // stage name added to the Dockerfile. + findTarget := "" + if cfg.Build != nil { + findTarget = cfg.Build.Target + } + baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), findTarget, buildxPlatformArgs(opts.platform)) logger.Write(fmt.Sprintf("Base image: %s", baseImage), log.LevelInfo) // Generate base-image metadata. Ensure the base image is available locally so @@ -364,7 +376,6 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage) baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger) stageMetadata := imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger) - metadataLabel := imagemeta.GenerateMetadataLabel(dockerfileImageMetadata(cfg, baseMetadata, stageMetadata)) // Determine image names (match TS getFolderImageName: vsc-{basename}-{hash}) imageNames := imageNamesOr(opts.imageNames, folderImageName(resolvePath(opts.workspaceFolder))) @@ -379,7 +390,7 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai buildArgs := buildArgsFromConfig(cfg) // Add metadata label - allLabels := append(opts.labels, fmt.Sprintf("%s=%s", imagemeta.MetadataLabel, metadataLabel)) + allLabels := append(slices.Clone(opts.labels), imageLabelArgs(dockerfileImageMetadata(cfg, baseMetadata, stageMetadata, r.configLabel))...) baseOutput, basePush := dockerfileBaseExport(opts, cfg) @@ -438,7 +449,7 @@ func (r *buildRunner) buildDockerfile(ctx context.Context, cfg *config.DevContai // If config has features, extend the built image with features if len(cfg.Features) > 0 { baseImageName := imageNames[0] - return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, r.featureBuildOptions(cfg, &loadResult.HostSub, configToMetadataEntry(cfg))) + return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, r.featureBuildOptions(cfg, &loadResult.HostSub)) } return imageNames, nil @@ -472,13 +483,23 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer, featureImageNames := imageNamesOr(opts.imageNames, folderImage+"-features") // Ensure image exists locally - _, err := engine.InspectImage(ctx, cfg.Image) + info, err := engine.InspectImage(ctx, cfg.Image) if err != nil { logger.Write(fmt.Sprintf("Pulling image %s...", cfg.Image), log.LevelInfo) if pullErr := engine.PullImage(ctx, cfg.Image); pullErr != nil { return nil, fmt.Errorf("image %q not found locally and pull failed: %w", cfg.Image, pullErr) } + if info, err = engine.InspectImage(ctx, cfg.Image); err != nil { + return nil, fmt.Errorf("inspect image %q: %w", cfg.Image, err) + } + } + // The label of an image build without Features (TS getImageBuildOptions): + // the image's metadata, then the configuration's entry as written. + var imageMetadata []imagemeta.Entry + if info.Config != nil { + imageMetadata = imagemeta.ReadMetadataFromLabels(info.Config.Labels, logger) } + imageMetadata = append(imageMetadata, r.configLabel) switch imageBuildFor(cfg, opts) { case imageWithFeatures: @@ -486,10 +507,8 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer, case imageExported: // --platform, --push, or --output still need a build: a minimal FROM - // Dockerfile carrying the config metadata. - metadata := []imagemeta.Entry{configToMetadataEntry(cfg)} - metadataLabel := imagemeta.GenerateMetadataLabel(metadata) - allLabels := append(opts.labels, fmt.Sprintf("%s=%s", imagemeta.MetadataLabel, metadataLabel)) + // Dockerfile carrying the metadata (TS getImageBuildOptions). + allLabels := append(slices.Clone(opts.labels), imageLabelArgs(imageMetadata)...) tmpDir, tmpErr := os.MkdirTemp("", "devcontainer-image-build-") if tmpErr != nil { @@ -532,9 +551,7 @@ func (r *buildRunner) buildImage(ctx context.Context, cfg *config.DevContainer, case imageRelabeled: // No platform/push — use temp Dockerfile when imageNames require labeling - metadata := []imagemeta.Entry{configToMetadataEntry(cfg)} - metadataLabel := imagemeta.GenerateMetadataLabel(metadata) - allLabels := append(opts.labels, fmt.Sprintf("%s=%s", imagemeta.MetadataLabel, metadataLabel)) + allLabels := append(slices.Clone(opts.labels), imageLabelArgs(imageMetadata)...) tmpDir, tmpErr := os.MkdirTemp("", "devcontainer-image-build-") if tmpErr != nil { @@ -606,7 +623,7 @@ func checkLegacyFeatures(feats map[string]any) error { name = id[:i] } if !features.IsKnownLegacyFeature(name) { - return fmt.Errorf(msgLegacyFeature, id) + return &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, id)} } } return nil @@ -623,12 +640,12 @@ func imageNamesOr(names []string, fallback string) []string { // dockerfileImageMetadata is the metadata a Dockerfile build stamps on its // image: the base image's, then a `LABEL devcontainer.metadata` the user's // Dockerfile declares (#1225; the CLI's own label would otherwise clobber it), -// then the config's entry. With Features the Feature build appends that entry -// instead. -func dockerfileImageMetadata(cfg *config.DevContainer, base, stage []imagemeta.Entry) []imagemeta.Entry { +// then the config's entry as written (configLabel). With Features the Feature +// build appends that entry instead. +func dockerfileImageMetadata(cfg *config.DevContainer, base, stage []imagemeta.Entry, configLabel imagemeta.Entry) []imagemeta.Entry { metadata := append(append([]imagemeta.Entry{}, base...), stage...) if len(cfg.Features) == 0 { - metadata = append(metadata, configToMetadataEntry(cfg)) + metadata = append(metadata, configLabel) } return metadata } @@ -727,21 +744,30 @@ func writeErrorJSON(out Output, errOutput coreerrors.ErrorOutput) error { return &coreerrors.ExitCodeError{Code: 1, Err: errors.New(errOutput.Description)} } -func isUserFacingBuildError(err error) bool { - msg := err.Error() - return strings.HasPrefix(msg, legacyFeaturePrefix) +// commandErrorOutput is the error result of up and build: a ContainerError (TS +// throws one with its own description, which the commands report as it is) +// as it is, any other error described as ": ". +func commandErrorOutput(err error, description string) coreerrors.ErrorOutput { + if ce, ok := coreerrors.AsContainerError(err); ok { + return coreerrors.ToErrorOutput(ce) + } + return coreerrors.ToErrorOutput(&coreerrors.ContainerError{Description: fmt.Sprintf("%s: %v", description, err)}) } -// buildCompose builds a compose config's service image, then its Features on -// top, or re-tags it to apply --label and --image-name. +// buildCompose is TS build for a Docker Compose configuration: the service's +// image is built by `docker compose build` through the build override TS +// buildAndExtendDockerCompose writes, with the metadata label on the image +// (with or without Features, an image-only service included), so the image +// carries compose's project and service labels; then it is tagged with each +// --image-name. // // mutate-lane: TestParityMatrix (build.compose-dockerfile-multi-image-success, // build.compose-image-without-features-minimal-success, // build.compose-image-without-features-minimal-metadata-label-success, // build.compose-image-with-features-success, // build.compose-image-extra-cache-from-success), also run by -// TestE2E_ComposeBuildLabel. -func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer, useBuildx bool) ([]string, error) { +// TestE2E_ComposeBuildLabel and TestDifferentialBuild. +func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer, loadResult *config.LoadResult, useBuildx bool) ([]string, error) { logger, dockerClient, engine, opts := r.log, r.docker, r.engine, r.opts if cfg.Service == "" { return nil, fmt.Errorf("dockerComposeFile config requires 'service' property") @@ -763,114 +789,108 @@ func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer if err != nil { return nil, fmt.Errorf("compose config: %w", err) } - serviceConfig := composeServiceConfig(composeConfig, cfg.Service) - - // Compute project name BEFORE build so compose image names are deterministic - projectName := docker.ToProjectName( - filepath.Base(resolvePath(opts.workspaceFolder))+"_devcontainer", - composeClient.UsesNewProjectNames(), - ) - - baseImageName := composeBaseImageName(projectName, cfg.Service, serviceConfig) - _, hasBuild := serviceConfig["build"] - - if hasBuild { - logger.Write(fmt.Sprintf("Building compose service %s...", cfg.Service), log.LevelInfo) - buildErr := composeClient.Build(ctx, composeFiles, "", []string{"--project-name", projectName}, []string{cfg.Service}, opts.noCache) - if buildErr != nil { - return nil, fmt.Errorf("compose build: %w", buildErr) - } + projectName := resolveComposeProjectName(ctx, cfg, env, composeFiles, composeClient) + svc, ok := composeService(composeConfig, cfg.Service) + if !ok { + return nil, fmt.Errorf("Service '%s' configured in devcontainer.json not found in Docker Compose configuration.", cfg.Service) } - - if len(cfg.Features) > 0 { - if !hasBuild { - if _, err := engine.InspectImage(ctx, baseImageName); err != nil { - logger.Write(fmt.Sprintf("Pulling image %s...", baseImageName), log.LevelInfo) - if pullErr := engine.PullImage(ctx, baseImageName); pullErr != nil { - return nil, fmt.Errorf("pull image %q: %w", baseImageName, pullErr) - } - } - } - imageNames := imageNamesOr(opts.imageNames, baseImageName+"-features") - return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, r.featureBuildOptions(cfg, nil)) + versionPrefix, err := readComposeVersionPrefix(composeFiles) + if err != nil { + return nil, err } - - imageName := baseImageName - - if tags, labels, ok := composeRetag(cfg, opts, imageName); ok { - tmpDir, tmpErr := os.MkdirTemp("", "devcontainer-compose-image-build-") - if tmpErr != nil { - return nil, fmt.Errorf("create temp dir: %w", tmpErr) - } - defer os.RemoveAll(tmpDir) - dfPath := filepath.Join(tmpDir, "Dockerfile") - if err := os.WriteFile(dfPath, []byte(fmt.Sprintf("FROM %s\n", imageName)), 0644); err != nil { - return nil, fmt.Errorf("write temp Dockerfile: %w", err) - } - - result, buildErr := dockerClient.Build(ctx, docker.BuildOptions{ - Dockerfile: dfPath, - ContextPath: tmpDir, - Tags: tags, - Labels: labels, - UseBuildx: useBuildx, - }) - if buildErr != nil { - return nil, fmt.Errorf("build image: %w", buildErr) - } - if result.ExitCode != 0 { - return nil, fmt.Errorf(msgDockerBuildFailed, result.ExitCode, string(result.Stderr)) - } - return tags, nil + if opts.secretsFile != "" { + logger.Write("--secrets-file does not apply to a Docker Compose configuration: `docker compose build` takes no build secrets.", log.LevelWarning) + } + + run := &upRunner{ + log: logger, + docker: dockerClient, + engine: engine, + opts: &upOpts{ + cacheFrom: opts.cacheFrom, + buildNoCache: opts.noCache, + skipFeatureAutoMapping: opts.skipFeatureAutoMapping, + experimentalLockfile: opts.experimentalLockfile, + experimentalFrozenLockfile: opts.experimentalFrozenLockfile, + noLockfile: opts.noLockfile, + lockfileExcludeIDs: opts.lockfileExcludeIDs, + }, + substitute: hostMetadataSubstitution(loadResult.HostSub, nil), + configLabel: r.configLabel, + } + ext, err := run.buildAndExtendCompose(ctx, composeStart{ + cfg: cfg, + loadResult: loadResult, + workspaceFolder: resolvePath(opts.workspaceFolder), + useBuildx: useBuildx, + projectName: projectName, + composeFiles: composeFiles, + composeConfig: composeConfig, + client: composeClient, + metadataOnImage: true, + buildServices: []string{cfg.Service}, + skipPersistCustoms: opts.skipPersistCustoms, + }, cacheFolder(), versionPrefix, false) + if err != nil { + return nil, err } - return []string{imageName}, nil -} - -// composeBaseImageName is the image of the compose service: its `image`, or -// compose's own - name for a service that only builds. -func composeBaseImageName(projectName, service string, serviceConfig map[string]any) string { - if image, ok := serviceConfig["image"].(string); ok && image != "" { - return image + imageName := ext.overrideImageName + if imageName == "" { + imageName, _ = svc["image"].(string) } - return fmt.Sprintf("%s-%s", projectName, service) -} - -// composeRetag is the FROM re-tag that applies image labels to a compose image, -// and ok is false when no flag asks for one. compose build itself takes no -// --label, so without it a --label with no --image-name is silently dropped -// (upstream #930). With --image-name the devcontainer.metadata label is stamped -// onto the distinct output image too; a bare re-tag under the same name carries -// only the user's --label. -func composeRetag(cfg *config.DevContainer, opts *buildOpts, imageName string) (tags, labels []string, ok bool) { - if len(opts.imageNames) == 0 && len(opts.labels) == 0 { - return nil, nil, false - } - labels = append([]string{}, opts.labels...) - if len(opts.imageNames) == 0 { - return []string{imageName}, labels, true - } - metadataLabel := imagemeta.GenerateMetadataLabel([]imagemeta.Entry{configToMetadataEntry(cfg)}) - return opts.imageNames, append(labels, fmt.Sprintf("%s=%s", imagemeta.MetadataLabel, metadataLabel)), true + if imageName == "" { + imageName = composeClient.DefaultImageName(projectName, cfg.Service) + } + tags := imageNamesOr(opts.imageNames, imageName) + if len(opts.labels) > 0 { + // compose build takes no --label: a FROM build applies them (upstream + // #930), under the --image-name values or the image's own name. + return tags, labelComposeImage(ctx, dockerClient, imageName, tags, opts.labels, useBuildx) + } + for _, name := range opts.imageNames { + if err := dockerClient.Tag(ctx, imageName, name); err != nil { + return nil, fmt.Errorf("tag image %q: %w", name, err) + } + } + return tags, nil } -func composeServiceConfig(composeConfig map[string]any, service string) map[string]any { - services, ok := composeConfig["services"].(map[string]any) - if !ok { - return nil +// labelComposeImage builds `FROM imageName` with labels, tagged tags. +// +// mutate-lane: TestE2E_ComposeBuildLabel - builds a compose image with --label +// and reads the label back. +func labelComposeImage(ctx context.Context, dockerClient *docker.Client, imageName string, tags, labels []string, useBuildx bool) error { + tmpDir, err := os.MkdirTemp("", "devcontainer-compose-image-build-") + if err != nil { + return fmt.Errorf("create temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + dfPath := filepath.Join(tmpDir, "Dockerfile") + if err := os.WriteFile(dfPath, []byte(fmt.Sprintf("FROM %s\n", imageName)), 0o644); err != nil { + return fmt.Errorf("write temp Dockerfile: %w", err) + } + result, err := dockerClient.Build(ctx, docker.BuildOptions{ + Dockerfile: dfPath, + ContextPath: tmpDir, + Tags: tags, + Labels: labels, + UseBuildx: useBuildx, + }) + if err != nil { + return fmt.Errorf("build image: %w", err) } - svc, ok := services[service].(map[string]any) - if !ok { - return nil + if result.ExitCode != 0 { + return fmt.Errorf(msgDockerBuildFailed, result.ExitCode, string(result.Stderr)) } - return svc + return nil } // featureBuildOptions are the Feature install options every build path passes: // the build flags and the config's Feature settings. hostSub resolves the -// Features' ${localEnv:…} references (nil leaves them as written); -// additionalMetadata is appended to the image's metadata label. -func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *config.HostSubContext, additionalMetadata ...imagemeta.Entry) *FeatureBuildOptions { +// Features' ${localEnv:…} references (nil leaves them as written). The +// configuration's entry follows the Features' in the image's metadata label. +func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *config.HostSubContext) *FeatureBuildOptions { opts := r.opts return &FeatureBuildOptions{ Secrets: buildSecretsFromFile(opts.secretsFile), @@ -892,6 +912,7 @@ func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *con HostSub: hostSub, SkipPersistCustoms: opts.skipPersistCustoms, FeaturesBasePath: filepath.Dir(cfg.ConfigFilePath), - AdditionalMetadata: additionalMetadata, + AdditionalMetadata: []imagemeta.Entry{configToMetadataEntry(cfg)}, + LabelMetadata: []imagemeta.Entry{r.configLabel}, } } diff --git a/internal/cli/build_compose_path_test.go b/internal/cli/build_compose_path_test.go index f3b1213..e88bc63 100644 --- a/internal/cli/build_compose_path_test.go +++ b/internal/cli/build_compose_path_test.go @@ -51,7 +51,7 @@ func TestBuildComposePathWiring(t *testing.T) { DockerComposeFile: config.StringOrStrings{"docker-compose.yml"}, } - _, err := r.buildCompose(t.Context(), cfg, false) + _, err := r.buildCompose(t.Context(), cfg, &config.LoadResult{}, false) if err == nil { t.Fatal("expected compose-client detection error, got nil") } diff --git a/internal/cli/build_held_test.go b/internal/cli/build_held_test.go index a993468..ffe6f27 100644 --- a/internal/cli/build_held_test.go +++ b/internal/cli/build_held_test.go @@ -145,12 +145,13 @@ func TestDockerfileImageMetadata(t *testing.T) { } cfg := &config.DevContainer{RemoteUser: "dev"} - if got, want := ids(dockerfileImageMetadata(cfg, base, stage)), []string{"base/", "stage/", "/dev"}; !slices.Equal(got, want) { - t.Errorf("without features: %v, want %v", got, want) + configLabel := imagemeta.Entry{RemoteUser: "${localEnv:USER}"} + if got, want := ids(dockerfileImageMetadata(cfg, base, stage, configLabel)), []string{"base/", "stage/", "/${localEnv:USER}"}; !slices.Equal(got, want) { + t.Errorf("without features: %v, want %v (the config's entry as written)", got, want) } cfg.Features = map[string]any{"ghcr.io/devcontainers/features/go:1": true} - if got, want := ids(dockerfileImageMetadata(cfg, base, stage)), []string{"base/", "stage/"}; !slices.Equal(got, want) { + if got, want := ids(dockerfileImageMetadata(cfg, base, stage, configLabel)), []string{"base/", "stage/"}; !slices.Equal(got, want) { t.Errorf("with features (the Feature build adds the config entry): %v, want %v", got, want) } } @@ -185,49 +186,6 @@ func TestBuildxSecrets(t *testing.T) { } } -func TestComposeBaseImageName(t *testing.T) { - for _, tc := range []struct { - name string - service map[string]any - want string - }{ - {"service image", map[string]any{"image": "alpine:3"}, "alpine:3"}, - {"empty image", map[string]any{"image": ""}, "proj-app"}, - {"build only", map[string]any{"build": map[string]any{}}, "proj-app"}, - {"no service config", nil, "proj-app"}, - } { - t.Run(tc.name, func(t *testing.T) { - if got := composeBaseImageName("proj", "app", tc.service); got != tc.want { - t.Errorf("composeBaseImageName = %q, want %q", got, tc.want) - } - }) - } -} - -func TestComposeRetag(t *testing.T) { - cfg := &config.DevContainer{RemoteUser: "dev"} - isMetadata := func(l string) bool { return strings.HasPrefix(l, imagemeta.MetadataLabel+"=") } - - if _, _, ok := composeRetag(cfg, &buildOpts{}, "proj-app"); ok { - t.Error("no --label or --image-name: want no re-tag") - } - - tags, labels, ok := composeRetag(cfg, &buildOpts{labels: []string{"a=1"}}, "proj-app") - if !ok || !slices.Equal(tags, []string{"proj-app"}) || !slices.Equal(labels, []string{"a=1"}) { - t.Errorf("--label only: (%v, %v, %v), want the service image re-tagged with the user's label", tags, labels, ok) - } - - tags, labels, ok = composeRetag(cfg, &buildOpts{imageNames: []string{"out:1"}}, "proj-app") - if !ok || !slices.Equal(tags, []string{"out:1"}) || len(labels) != 1 || !isMetadata(labels[0]) || !strings.Contains(labels[0], `"remoteUser":"dev"`) { - t.Errorf("--image-name only: (%v, %v, %v), want the name with the config's metadata label", tags, labels, ok) - } - - tags, labels, ok = composeRetag(cfg, &buildOpts{imageNames: []string{"out:1", "out:2"}, labels: []string{"a=1"}}, "proj-app") - if !ok || !slices.Equal(tags, []string{"out:1", "out:2"}) || len(labels) != 2 || labels[0] != "a=1" || !isMetadata(labels[1]) { - t.Errorf("both: (%v, %v, %v), want the names, the user's label then the metadata label", tags, labels, ok) - } -} - // --buildkit never skips BuildKit detection, so the build runs as if buildx // were absent even when the Docker CLI has it. The fake --docker-path reports // buildx, and both cases stop at the flag check before any Docker build. @@ -267,3 +225,33 @@ func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) { }) } } + +// The Dockerfile build targets the config's target when it names one, else +// the final stage, named when it has no name; a Dockerfile without FROM is +// TS's error. +func TestPrepareDockerfileBuildStage(t *testing.T) { + ws := writeConfigFixture(t, map[string]string{ + ".devcontainer/Dockerfile": "FROM alpine:3 AS base\nFROM base\n", + ".devcontainer/Empty": "RUN true\n", + }) + cfgFile := filepath.Join(ws, ".devcontainer", "devcontainer.json") + prep, err := prepareDockerfileBuild(&config.DevContainer{ConfigFilePath: cfgFile, Build: &config.Build{Dockerfile: "Dockerfile", Target: "base"}}) + if err != nil || prep.StageName != "base" { + t.Fatalf("with a target: %+v, %v; want stage base", prep, err) + } + prep.Cleanup() + prep, err = prepareDockerfileBuild(&config.DevContainer{ConfigFilePath: cfgFile, Build: &config.Build{Dockerfile: "Dockerfile"}}) + if err != nil || prep.StageName != "dev_container_auto_added_stage_label" { + t.Fatalf("without a target: %+v, %v", prep, err) + } + if _, err := os.Stat(prep.Path); err != nil { + t.Fatalf("named Dockerfile: %v", err) + } + prep.Cleanup() + if _, err := os.Stat(prep.Path); !os.IsNotExist(err) { + t.Errorf("named Dockerfile %s left after Cleanup: %v", prep.Path, err) + } + if _, err := prepareDockerfileBuild(&config.DevContainer{ConfigFilePath: cfgFile, Build: &config.Build{Dockerfile: "Empty"}}); err == nil || !strings.Contains(err.Error(), "no FROM instructions") { + t.Errorf("no FROM: err = %v", err) + } +} diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index ccda926..81baa46 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -161,24 +161,6 @@ func TestReadConfiguration_AllFixtures(t *testing.T) { t.Logf("read-configuration: %d passed, %d skipped", passed, skipped) } -func TestComposeServiceConfig(t *testing.T) { - cfg := map[string]any{ - "services": map[string]any{ - "app": map[string]any{ - "image": "example/app:latest", - }, - }, - } - - got := composeServiceConfig(cfg, "app") - if got == nil { - t.Fatal("composeServiceConfig() = nil") - } - if got["image"] != "example/app:latest" { - t.Fatalf("image = %v", got["image"]) - } -} - func TestConfigToMetadataEntry_IncludesMountsAndUIDSetting(t *testing.T) { trueValue := true cfg := &config.DevContainer{ 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/compose_up.go b/internal/cli/compose_up.go index 9ee5ace..f6c41f7 100644 --- a/internal/cli/compose_up.go +++ b/internal/cli/compose_up.go @@ -42,6 +42,14 @@ type composeStart struct { client *docker.ComposeClient // container is the existing (stopped) service container, nil for none. container *container.InspectResponse + // The build command's settings (zero for up): metadataOnImage puts the + // metadata label on an image-only service's image even without Features (TS + // canAddLabelsToContainer false), buildServices are the services `compose + // build` builds (nil for TS up's runServices filter), skipPersistCustoms + // leaves the Features' customizations out of the metadata. + metadataOnImage bool + buildServices []string + skipPersistCustoms bool } // persistedFolder is TS common.persistedFolder: --user-data-folder, or @@ -377,14 +385,17 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe if svcBuild.Target != "" { baseName = svcBuild.Target } else { - name, modified := docker.EnsureFinalStageName(dockerfile, baseName) + name, modified, err := docker.EnsureFinalStageName(dockerfile, baseName) + if err != nil { + return nil, err + } baseName = name if modified != "" { dockerfile = modified } } parsed = docker.ExtractDockerfile(string(raw)) - if baseImage := docker.FindBaseImage(parsed, svcBuild.Args, svcBuild.Target); baseImage != "" { + if baseImage := docker.FindBaseImage(parsed, svcBuild.Args, svcBuild.Target, buildxPlatformArgs("")); baseImage != "" { info, err := r.inspectImagePulling(ctx, baseImage) if err != nil { return nil, err @@ -415,12 +426,19 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe return nil, err } - configEntry := configMetadataEntry(cfg, opts.omitConfigRemoteEnvFromMeta) - metadata := slices.Clone(baseEntries) + // TS getDevcontainerMetadata: the label records the image's entries, the + // Features' and the configuration's as written (labelMetadata); the merged + // configuration reads them with their variables substituted (metadata). + var featureEntries []imagemeta.Entry for _, fs := range featureSets { - metadata = append(metadata, featureMetadataEntry(fs, false)) + featureEntries = append(featureEntries, featureMetadataEntry(fs, s.skipPersistCustoms)) } - metadata = append(metadata, configEntry) + labelMetadata := slices.Concat(baseEntries, featureEntries, []imagemeta.Entry{r.configLabel}) + metadata := slices.Clone(baseEntries) + if err := r.substitute(&metadata); err != nil { + return nil, fmt.Errorf("substitute the image's metadata: %w", err) + } + metadata = slices.Concat(metadata, featureEntries, []imagemeta.Entry{configMetadataEntry(cfg, false)}) result := &composeExtendResult{imageMetadata: metadata} userSyntax := "" @@ -431,10 +449,10 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe var info *imagemeta.ExtendImageBuildInfo dstFolder := featuresFolder switch { - case len(featureSets) == 0 && !svcBuild.HasBuild: + case len(featureSets) == 0 && !svcBuild.HasBuild && !s.metadataOnImage: // TS getExtendImageBuildInfo: no Features and an image (no Dockerfile) → // only the metadata label, added to the container. - result.labels = []string{imagemeta.MetadataLabel + "=" + imagemeta.GenerateMetadataLabel(metadata)} + result.labels = []string{imagemeta.MetadataLabel + "=" + containerMetadataLabel(labelMetadata)} case len(featureSets) == 0: // TS getImageBuildOptions: extend the Dockerfile with the metadata label. syntax := userSyntax @@ -445,7 +463,7 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe if err := os.MkdirAll(dstFolder, 0o755); err != nil { return nil, fmt.Errorf("create features folder: %w", err) } - info = imagemeta.GenerateComposeLabelBuild(baseName, metadata, syntax) + info = imagemeta.GenerateComposeLabelBuild(baseName, labelMetadata, syntax) default: // TS findContainerUsers over the image + Features + config metadata. containerUser, remoteUser := findContainerUsers(metadata, svcUser, imageUser) @@ -468,7 +486,7 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe info = imagemeta.GenerateFeaturesBuildOptions(imagemeta.FeaturesBuildParams{ BaseName: baseName, FeatureSets: featureSets, - Metadata: metadata, + Metadata: labelMetadata, ContainerUser: containerUser, RemoteUser: remoteUser, ImageUser: imageUser, @@ -545,7 +563,11 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe if !noBuild { files := append(slices.Clone(s.composeFiles), result.overrideFiles...) - if err := s.client.Build(ctx, files, "", []string{"--project-name", s.projectName}, composeServicesFilter(cfg), opts.buildNoCache); err != nil { + services := s.buildServices + if services == nil { + services = composeServicesFilter(cfg) + } + if err := s.client.Build(ctx, files, "", []string{"--project-name", s.projectName}, services, opts.buildNoCache); err != nil { return nil, fmt.Errorf("An error occurred building the Docker Compose images: %w", err) } } diff --git a/internal/cli/compose_up_held_test.go b/internal/cli/compose_up_held_test.go index 7d34ab9..6c7a40b 100644 --- a/internal/cli/compose_up_held_test.go +++ b/internal/cli/compose_up_held_test.go @@ -295,6 +295,8 @@ func newComposeHarness(t *testing.T) *composeHarness { docker: &docker.Client{DockerPath: "docker", Log: logger, Runner: run}, engine: &docker.EngineClient{API: engine, Log: logger}, opts: &upOpts{userDataFolder: persisted, updateRemoteUserUIDDefault: "off", gpuAvailability: "none"}, + // The fixtures' metadata names no variables. + substitute: func(*[]imagemeta.Entry) error { return nil }, }, } } @@ -1228,3 +1230,48 @@ func TestStartComposeContainerImageServiceWithFeatures(t *testing.T) { t.Errorf("compose up %q, want the compose file and both overrides", ups) } } + +// For the build command (TS canAddLabelsToContainer false) an image-only +// service without Features is still built, with the metadata label on its +// image: the image's entries and the configuration's as written, while the +// merged metadata has it substituted; compose builds the dev container +// service alone. +func TestBuildAndExtendComposeForTheBuildCommand(t *testing.T) { + h := newComposeHarness(t) + h.r.configLabel = imagemeta.Entry{RemoteUser: "${localEnv:ME}"} + s := h.start(&config.DevContainer{RemoteUser: "me", RunServices: []string{"db"}}, map[string]any{"image": "alpine:3"}) + s.metadataOnImage, s.buildServices = true, []string{"app"} + res, err := h.r.buildAndExtendCompose(context.Background(), s, h.persisted, "", false) + if err != nil { + t.Fatal(err) + } + if res.labels != nil || res.overrideImageName != folderImageName(h.ws) { + t.Errorf("labels %q, override image %q; want no container label and the folder image", res.labels, res.overrideImageName) + } + content := readFile(t, overrideDockerfile(t, readFile(t, res.overrideFiles[0]))) + if !strings.Contains(content, `LABEL devcontainer.metadata="[`) || !strings.Contains(content, `\"remoteUser\":\"\${localEnv:ME}\"`) || !strings.Contains(content, `\"remoteUser\":\"node\"`) { + t.Errorf("final Dockerfile lacks the label with the image's and the raw config's entries:\n%s", content) + } + if last := res.imageMetadata[len(res.imageMetadata)-1]; last.RemoteUser != "me" { + t.Errorf("merged metadata ends with %+v, want the substituted config entry", last) + } + if got, want := h.run.compose("build"), [][]string{{"-f", h.compose, "-f", res.overrideFiles[0], "--project-name", "proj", "build", "app"}}; !slices.EqualFunc(got, want, slices.Equal) { + t.Errorf("compose build runs %q, want %q", got, want) + } + + // --skip-persisting-customizations-from-features leaves a Feature's + // customizations out of the metadata. + h.write("custom/devcontainer-feature.json", `{"id":"custom","version":"1.0.0","customizations":{"vscode":{"extensions":["x"]}}}`) + h.write("custom/install.sh", "#!/bin/sh\n") + for _, skip := range []bool{false, true} { + s := h.start(&config.DevContainer{Features: map[string]any{"./custom": map[string]any{}}}, map[string]any{"image": "alpine:3"}) + s.skipPersistCustoms = skip + res, err := h.r.buildAndExtendCompose(context.Background(), s, h.persisted, "", true) + if err != nil { + t.Fatal(err) + } + if got := res.imageMetadata[1].Customizations != nil; got == skip { + t.Errorf("skip=%t: Feature entry %+v", skip, res.imageMetadata[1]) + } + } +} diff --git a/internal/cli/differential_runtime_test.go b/internal/cli/differential_runtime_test.go new file mode 100644 index 0000000..fbafa3f --- /dev/null +++ b/internal/cli/differential_runtime_test.go @@ -0,0 +1,393 @@ +package cli + +import ( + "bytes" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "reflect" + "regexp" + "slices" + "sort" + "strings" + "testing" +) + +// differentialRuntimeKnown lists, per command, the fixtures whose result still +// differs from the oracle's, each with the reason. Like differentialKnown it can +// only shrink: a listed fixture that matches fails the test, so a fix removes +// its line. +var differentialRuntimeKnown = map[string]map[string]string{ + // docs/DIVERGENCES.md: Features backed by GitHub Releases (v1, owner/repo/id) + // are not implemented; the configuration fails instead of resolving them. + "up": { + "dockerfile-with-syntax": "legacy GitHub Release Feature (accepted limitation)", + "dockerfile-with-target": "legacy GitHub Release Feature (accepted limitation)", + "compose-Dockerfile-with-features": "legacy GitHub Release Feature (accepted limitation)", + "compose-Dockerfile-with-target": "legacy GitHub Release Feature (accepted limitation)", + "compose-image-with-features": "legacy GitHub Release Feature (accepted limitation)", + }, + "build": { + "dockerfile-with-syntax": "legacy GitHub Release Feature (accepted limitation)", + "dockerfile-with-target": "legacy GitHub Release Feature (accepted limitation)", + "compose-Dockerfile-with-features": "legacy GitHub Release Feature (accepted limitation)", + "compose-Dockerfile-with-target": "legacy GitHub Release Feature (accepted limitation)", + "compose-image-with-features": "legacy GitHub Release Feature (accepted limitation)", + }, +} + +// TestDifferentialUp runs `up` on every upstream fixture with the TS oracle and +// with this CLI, one after the other on copies of the fixture, and compares what +// a user of the container sees: the command's JSON result, the container's and +// its image's configuration (user, env, entrypoint, command, labels with the +// parsed devcontainer.metadata, mounts, init, privileged, capabilities, security +// options) and the remote user's `id` inside the container. Hand-written parity +// cases compare what someone thought of; this compares everything a fixture +// exercises. Like TestDifferentialReadConfiguration it needs the compiled +// oracle, Docker and the network, and runs when PARITY_DIFFERENTIAL is set. +func TestDifferentialUp(t *testing.T) { + runDifferentialRuntime(t, "up", differentialUpSnapshot) +} + +// TestDifferentialBuild runs `build` on every upstream fixture with both CLIs +// and compares the command's JSON result and the built image's configuration. +func TestDifferentialBuild(t *testing.T) { + runDifferentialRuntime(t, "build", differentialBuildSnapshot) +} + +// differentialSnapshot runs one CLI's command on the workspace ws and returns +// what is compared, normalized for that workspace. +type differentialSnapshot func(t *testing.T, cli []string, ws string) map[string]any + +func runDifferentialRuntime(t *testing.T, command string, snapshot differentialSnapshot) { + if os.Getenv("PARITY_DIFFERENTIAL") == "" { + t.Skip("set PARITY_DIFFERENTIAL=1 (task parity:differential): needs the oracle, Docker and the network") + } + repoRoot := findRepoRoot(t) + if _, err := os.Stat(filepath.Join(repoRoot, "reference", "dist", "spec-node", "devContainersSpecCLI.js")); err != nil { + t.Fatalf("TS reference oracle not compiled (task reference): %v", err) + } + if err := exec.Command("docker", "info").Run(); err != nil { + t.Fatalf("Docker is required: %v", err) + } + ts := []string{"node", filepath.Join(repoRoot, "reference", "devcontainer.js")} + goCLI := os.Getenv("PARITY_CLI_GO") + if goCLI == "" { + goCLI = filepath.Join(repoRoot, "devcontainer") + } + + configs := filepath.Join(repoRoot, "reference", "src", "test", "configs") + fixtures := differentialFixtures(t, configs) + known := differentialRuntimeKnown[command] + for name := range known { + if !slices.Contains(fixtures, name) { + t.Errorf("differentialRuntimeKnown[%q] lists %q, which is no fixture", command, name) + } + } + for _, name := range fixtures { + t.Run(name, func(t *testing.T) { + if strings.Contains(name, "arm64") && os.Getenv("PARITY_ARM64") != "true" { + t.Skip("arm64 fixture (set PARITY_ARM64=true; needs arm64 emulation)") + } + src := filepath.Join(configs, name) + tsOut := snapshot(t, ts, differentialWorkspace(t, src, "ts")) + goOut := snapshot(t, []string{goCLI}, differentialWorkspace(t, src, "go")) + same := reflect.DeepEqual(tsOut, goOut) + reason, isKnown := known[name] + switch { + case same && isKnown: + t.Errorf("%s %s no longer differs (%s): remove it from differentialRuntimeKnown", command, name, reason) + case !same && isKnown: + t.Skipf("known difference: %s", reason) + case !same: + t.Errorf("%s differs\n%s", command, differentialDiff(tsOut, goOut)) + } + }) + } +} + +// differentialWorkspace copies the fixture to //ws: both sides see a +// workspace folder named ws, so names derived from its basename only differ by +// the path hash, which the snapshots normalize. +func differentialWorkspace(t *testing.T, fixture, side string) string { + t.Helper() + ws := filepath.Join(t.TempDir(), side, "ws") + if err := copyDir(fixture, ws); err != nil { + t.Fatal(err) + } + return ws +} + +func differentialUpSnapshot(t *testing.T, cli []string, ws string) map[string]any { + t.Helper() + defer differentialCleanup(t, ws) + code, result := runDifferentialCLI(t, cli, "up", "--workspace-folder", ws) + snap := map[string]any{"exitCode": code, "result": normalizeDifferential(result, ws)} + containerID, _ := asMap(result)["containerId"].(string) + if code != 0 || containerID == "" { + return snap + } + container := dockerInspect(t, "container", containerID) + snap["container"] = normalizeDifferential(containerView(container), ws) + if image, _ := container["Image"].(string); image != "" { + snap["image"] = normalizeDifferential(imageView(dockerInspect(t, "image", image)), ws) + } + if user, _ := asMap(result)["remoteUser"].(string); user != "" { + out, err := exec.CommandContext(t.Context(), "docker", "exec", "-u", user, containerID, "id").CombinedOutput() + snap["remoteUserId"] = strings.TrimSpace(string(out)) + if err != nil { + snap["remoteUserIdError"] = err.Error() + } + } + return snap +} + +// differentialImageName is the --image-name both sides build, one after the +// other: the image is removed between them. +const differentialImageName = "devcontainer-differential:latest" + +func differentialBuildSnapshot(t *testing.T, cli []string, ws string) map[string]any { + t.Helper() + defer differentialCleanup(t, ws) + defer exec.Command("docker", "rmi", "-f", differentialImageName).Run() + code, result := runDifferentialCLI(t, cli, "build", "--workspace-folder", ws, "--image-name", differentialImageName) + snap := map[string]any{"exitCode": code, "result": normalizeDifferential(result, ws)} + if code == 0 { + snap["image"] = normalizeDifferential(imageView(dockerInspect(t, "image", differentialImageName)), ws) + } + return snap +} + +// runDifferentialCLI runs the CLI and returns its exit code and the last stdout +// line decoded as JSON (the raw line when it is not JSON). +func runDifferentialCLI(t *testing.T, cli []string, args ...string) (int, any) { + t.Helper() + cmd := exec.CommandContext(t.Context(), cli[0], append(cli[1:], args...)...) + cmd.Dir = findRepoRoot(t) + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + err := cmd.Run() + code := 0 + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("run %v: %v", cli, err) + } + lines := strings.Split(strings.TrimSpace(stdout.String()), "\n") + last := lines[len(lines)-1] + var out any + if err := json.Unmarshal([]byte(last), &out); err != nil { + out = last + } + if code != 0 && testing.Verbose() { + t.Logf("%s %s: exit %d\n%s", filepath.Base(cli[len(cli)-1]), args[0], code, tail(stderr.String(), 2000)) + } + return code, out +} + +// differentialCleanup removes what a run left behind for the workspace: its +// containers, their compose projects (containers, networks, volumes) and the +// images built for them, so the other side starts from the same state. +func differentialCleanup(t *testing.T, ws string) { + t.Helper() + out, _ := exec.Command("docker", "ps", "-aq", "--filter", "label=devcontainer.local_folder="+ws).Output() + ids := strings.Fields(string(out)) + images := map[string]bool{} + projects := map[string]bool{} + for _, id := range ids { + c := dockerInspect(t, "container", id) + if img, _ := c["Image"].(string); img != "" { + images[img] = true + } + labels := asMap(asMap(c["Config"])["Labels"]) + if p, _ := labels["com.docker.compose.project"].(string); p != "" { + projects[p] = true + } + } + if len(ids) > 0 { + exec.Command("docker", append([]string{"rm", "-f", "-v"}, ids...)...).Run() + } + for p := range projects { + exec.Command("docker", "compose", "-p", p, "down", "-v", "--remove-orphans").Run() + exec.Command("sh", "-c", "docker images --format '{{.Repository}}:{{.Tag}}' | grep -E '^"+regexp.QuoteMeta(p)+"[-_]' | xargs -r docker rmi -f").Run() + } + for img := range images { + exec.Command("docker", "rmi", "-f", img).Run() + } + exec.Command("sh", "-c", "docker images --format '{{.Repository}}:{{.Tag}}' | grep -E '^vsc-ws-' | xargs -r docker rmi -f").Run() +} + +func dockerInspect(t *testing.T, kind, id string) map[string]any { + t.Helper() + out, err := exec.Command("docker", kind, "inspect", id).Output() + if err != nil { + t.Fatalf("docker %s inspect %s: %v", kind, id, err) + } + var v []map[string]any + if err := json.Unmarshal(out, &v); err != nil || len(v) != 1 { + t.Fatalf("docker %s inspect %s: %v", kind, id, err) + } + return v[0] +} + +// containerView is the part of a container's configuration a dev container's +// user depends on. +func containerView(c map[string]any) map[string]any { + cfg, host := asMap(c["Config"]), asMap(c["HostConfig"]) + var mounts []any + for _, m := range asSlice(c["Mounts"]) { + mm := asMap(m) + source := mm["Source"] + if mm["Type"] == "volume" { + source = mm["Name"] + } + mounts = append(mounts, map[string]any{"type": mm["Type"], "source": source, "destination": mm["Destination"], "rw": mm["RW"]}) + } + sortByJSON(mounts) + return map[string]any{ + "user": cfg["User"], + "env": sortedStrings(cfg["Env"]), + "entrypoint": cfg["Entrypoint"], + "cmd": cfg["Cmd"], + "workingDir": cfg["WorkingDir"], + "labels": viewLabels(asMap(cfg["Labels"])), + "mounts": mounts, + "init": host["Init"], + "privileged": host["Privileged"], + "capAdd": sortedStrings(host["CapAdd"]), + "securityOpt": sortedStrings(host["SecurityOpt"]), + } +} + +// imageView is the part of an image's configuration the container inherits. +func imageView(img map[string]any) map[string]any { + cfg := asMap(img["Config"]) + return map[string]any{ + "user": cfg["User"], + "env": sortedStrings(cfg["Env"]), + "entrypoint": cfg["Entrypoint"], + "cmd": cfg["Cmd"], + "labels": viewLabels(asMap(cfg["Labels"])), + } +} + +// viewLabels keeps the labels with the devcontainer.metadata JSON parsed, and +// drops compose's bookkeeping labels whose values are hashes, versions or +// absolute paths of compose's own. +func viewLabels(labels map[string]any) map[string]any { + out := map[string]any{} + for k, v := range labels { + switch k { + case "com.docker.compose.config-hash", "com.docker.compose.version", "com.docker.compose.container-number", + "com.docker.compose.oneoff", "com.docker.compose.depends_on", "com.docker.compose.image", + "com.docker.compose.replace", "desktop.docker.io/binds/0/Source", "desktop.docker.io/binds/0/SourceKind": + continue + } + if s, ok := v.(string); ok && k == "devcontainer.metadata" { + var parsed any + if json.Unmarshal([]byte(s), &parsed) == nil { + v = parsed + } + } + out[k] = v + } + return out +} + +var ( + differentialHashRe = regexp.MustCompile(`(vsc-ws-)[0-9a-f]{64}`) + differentialDigest = regexp.MustCompile(`sha256:[0-9a-f]{64}`) + differentialHexIDRe = regexp.MustCompile(`\b[0-9a-f]{64}\b`) + // The compose override files both CLIs write are named + // docker-compose.devcontainer.build-.yml and + // docker-compose.devcontainer.containerFeatures--.yml. + differentialOverrideRe = regexp.MustCompile(`(docker-compose\.devcontainer\.\w+)-\d+(?:-[0-9a-f-]{36})?`) + // ${devcontainerId} hashes the id labels, which hold the workspace path. + differentialDevcontainerIDRe = regexp.MustCompile(`(dind-var-lib-docker-)[0-9a-v]{52}`) +) + +// normalizeDifferential replaces what legitimately differs between the two +// runs: the workspace path, the path hash in image names, the features folder's +// version-and-time segment, the compose override file's time and uuid, the +// devcontainerId (a hash of the workspace's id labels), and digests and ids. +func normalizeDifferential(v any, ws string) any { + data, err := json.Marshal(v) + if err != nil { + return v + } + // The workspace first, then its parent (/), which a fixture can + // mount through ${localWorkspaceFolder}/.. + s := strings.ReplaceAll(string(data), ws, "") + s = strings.ReplaceAll(s, filepath.Dir(ws), "/..") + s = differentialHashRe.ReplaceAllString(s, "${1}") + s = featuresFolderRe.ReplaceAllString(s, "${1}-") + s = differentialDigest.ReplaceAllString(s, "sha256:") + s = differentialHexIDRe.ReplaceAllString(s, "") + s = differentialOverrideRe.ReplaceAllString(s, "${1}-") + s = differentialDevcontainerIDRe.ReplaceAllString(s, "${1}") + var out any + if err := json.Unmarshal([]byte(s), &out); err != nil { + return s + } + return out +} + +func differentialDiff(ts, goOut map[string]any) string { + var b strings.Builder + keys := map[string]bool{} + for k := range ts { + keys[k] = true + } + for k := range goOut { + keys[k] = true + } + sorted := make([]string, 0, len(keys)) + for k := range keys { + sorted = append(sorted, k) + } + sort.Strings(sorted) + for _, k := range sorted { + if !reflect.DeepEqual(ts[k], goOut[k]) { + b.WriteString("--- " + k + " (TS)\n" + pretty(ts[k]) + "\n--- " + k + " (Go)\n" + pretty(goOut[k]) + "\n") + } + } + return b.String() +} + +func asMap(v any) map[string]any { + m, _ := v.(map[string]any) + return m +} + +func asSlice(v any) []any { + s, _ := v.([]any) + return s +} + +func sortedStrings(v any) []string { + var out []string + for _, e := range asSlice(v) { + if s, ok := e.(string); ok { + out = append(out, s) + } + } + sort.Strings(out) + return out +} + +func sortByJSON(items []any) { + sort.Slice(items, func(i, j int) bool { + a, _ := json.Marshal(items[i]) + b, _ := json.Marshal(items[j]) + return string(a) < string(b) + }) +} + +func tail(s string, n int) string { + if len(s) <= n { + return s + } + return s[len(s)-n:] +} diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 62918c9..9f16fb8 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -113,7 +113,8 @@ func newExecCmd() *cobra.Command { // mutate-lane: TestParityMatrix (exec.workspace-image-success, // exec.workspace-compose-success, exec.container-config-success, // exec.container-metadata-success, exec.workspace-image-json-raw-output-success, -// exec.workspace-image-nonzero-exit, exec.container-not-found). +// exec.workspace-image-nonzero-exit, exec.container-not-found, +// exec.config-remote-user-probe-success). func runExec(ctx context.Context, opts *execOpts, cmdArgs []string) error { if err := validateIDLabels(opts.idLabels); err != nil { return err @@ -191,18 +192,26 @@ func runExec(ctx context.Context, opts *execOpts, cmdArgs []string) error { // 4-6. The container's env, and its image metadata from the labels. containerEnv := config.EnvMap(containerCfg.Env) - merged := imagemeta.MergeConfiguration(imagemeta.ReadMetadataFromLabels(containerCfg.Labels, logger)) var cfg *config.DevContainer + hostSub, configFile := config.EnvHostSubContext(), "" if loadResult != nil { - cfg = loadResult.Config + cfg, hostSub, configFile = loadResult.Config, loadResult.HostSub, loadResult.Config.ConfigFilePath } - // The environment is probed as the image metadata's user (else the - // container's, else root); the configuration's remoteUser applies after. - remoteUser := cmp.Or(merged.RemoteUser, containerCfg.User, "root") + // TS getImageMetadataFromContainer, with the host substitutions alone. + idLabels := containerIDLabels(opts.idLabels, containerCfg.Labels, resolvePath(opts.workspaceFolder), configFile) + entries, err := imageMetadataFromContainer(containerCfg.Labels, cfg, nil, idLabels, hostMetadataSubstitution(hostSub, nil), logger) + if err != nil { + return err + } + merged := imagemeta.MergeConfiguration(entries) + // The user is the merged configuration's remoteUser - the configuration's, + // else the image metadata's - else the container's, else root; the + // environment is probed as that user (TS createContainerProperties with + // mergedConfig.remoteUser). + remoteUser := execRemoteUser(cmp.Or(merged.RemoteUser, containerCfg.User, "root"), cfg) // 7-8. Probe the user environment, then merge and substitute it. probed := lifecycle.ProbedRemoteEnv(ctx, engine, logger, containerID, remoteUser, execProbeStrategy(opts.defaultUserEnvProbe, cfg), "") - remoteUser = execRemoteUser(remoteUser, cfg) resolvedRemoteEnv, err := execRemoteEnv(probed, merged.RemoteEnv, cfg, containerEnv) if err != nil { return err @@ -255,13 +264,13 @@ func containerRunning(state *container.State) bool { return state != nil && state.Running } -// execRemoteUser is the user exec runs as: the configuration's remoteUser, -// else the user the environment was probed as. -func execRemoteUser(probeUser string, cfg *config.DevContainer) string { +// execRemoteUser is the user exec probes the environment as and runs as: the +// configuration's remoteUser, else metadataUser. +func execRemoteUser(metadataUser string, cfg *config.DevContainer) string { if cfg != nil && cfg.RemoteUser != "" { return cfg.RemoteUser } - return probeUser + return metadataUser } // execProbeStrategy is the configuration's userEnvProbe, else the flag's. diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index e8a7cfb..50fd843 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -24,6 +24,7 @@ import ( "github.com/devcontainers/cli/internal/config" "github.com/devcontainers/cli/internal/docker" + coreerrors "github.com/devcontainers/cli/internal/errors" "github.com/devcontainers/cli/internal/features" "github.com/devcontainers/cli/internal/httpx" "github.com/devcontainers/cli/internal/imagemeta" @@ -54,7 +55,12 @@ type FeatureBuildOptions struct { SkipFeatureAutoMapping bool SkipPersistCustoms bool FeaturesBasePath string - AdditionalMetadata []imagemeta.Entry + // AdditionalMetadata follows the Features' entries in the metadata the + // Features' users are found in (TS findContainerUsers), variables + // substituted; LabelMetadata follows them in the image's + // devcontainer.metadata label, as written (TS getDevcontainerMetadata's raw). + AdditionalMetadata []imagemeta.Entry + LabelMetadata []imagemeta.Entry // Lockfile writes devcontainer-lock.json with the resolved feature digests; // FrozenLockfile validates the resolved features against the committed // lockfile and aborts on drift. ConfigPath is the devcontainer.json path @@ -229,7 +235,7 @@ func processInstallFeature( name = id[:idx] } if !features.IsKnownLegacyFeature(name) { - return nil, fmt.Errorf(msgLegacyFeature, id) + return nil, &coreerrors.ContainerError{Description: fmt.Sprintf(msgLegacyFeature, id)} } // gradle/maven/jupyterlab folded into java/python with an option // (installGradle, ...) — matching the TS deprecatedFeaturesIntoOptions. @@ -686,6 +692,7 @@ func extendImageWithFeatures( for _, fs := range featureSets { metadata = append(metadata, featureMetadataEntry(fs, fbOpts.SkipPersistCustoms)) } + labelMetadata := append(slices.Clone(metadata), fbOpts.LabelMetadata...) metadata = append(metadata, fbOpts.AdditionalMetadata...) // TS findContainerUsers over the image, Features and config metadata. @@ -714,7 +721,7 @@ func extendImageWithFeatures( buildInfo := imagemeta.GenerateFeaturesBuildOptions(imagemeta.FeaturesBuildParams{ BaseName: baseImage, FeatureSets: featureSets, - Metadata: metadata, + Metadata: labelMetadata, ContainerUser: containerUser, RemoteUser: remoteUser, ImageUser: imageUser, diff --git a/internal/cli/image_build_info.go b/internal/cli/image_build_info.go index eda2bbe..08e3e09 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 } @@ -234,6 +233,19 @@ func imageBuildInfoFromDockerfile(ctx context.Context, insp *imageInspector, doc return info, nil } +// buildxPlatformArgs are the automatic platform ARGs of a build on this host +// for platform, os/arch[/variant] as --platform takes it, the host's when +// empty (TS devContainers.ts targetPlatformInfo). +func buildxPlatformArgs(platform string) map[string]string { + target := targetPlatform() + if platform != "" { + os, rest, _ := strings.Cut(platform, "/") + arch, variant, _ := strings.Cut(rest, "/") + target = ocispec.Platform{OS: os, Architecture: arch, Variant: variant} + } + return globalBuildxPlatformArgs(targetPlatform(), target) +} + // globalBuildxPlatformArgs are BuildKit's automatic platform ARGs in the global // scope, as TS internalGetImageBuildInfoFromDockerfile defines them. func globalBuildxPlatformArgs(build, target ocispec.Platform) map[string]string { @@ -251,172 +263,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..1a93cb1 100644 --- a/internal/cli/image_build_info_test.go +++ b/internal/cli/image_build_info_test.go @@ -3,14 +3,15 @@ package cli import ( "context" "errors" + "maps" "os" "path/filepath" + "runtime" "testing" 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 +98,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) } } @@ -246,3 +212,20 @@ func TestImageBuildInfoImagePullFailurePropagates(t *testing.T) { t.Fatalf("got %+v, %v; want the pull error", info, err) } } + +// The platform ARGs up and build resolve a Dockerfile's FROM with target the +// --platform value (os/arch[/variant]), else this host, as TS +// devContainers.ts targetPlatformInfo; the build platform is always the host. +func TestBuildxPlatformArgs(t *testing.T) { + host := runtime.GOOS + "/" + runtime.GOARCH + if got := buildxPlatformArgs(""); got["TARGETPLATFORM"] != host || got["BUILDPLATFORM"] != host || got["TARGETARCH"] != runtime.GOARCH { + t.Errorf("no --platform: %v, want the host's", got) + } + got := buildxPlatformArgs("linux/arm64/v8") + if got["TARGETPLATFORM"] != "linux/arm64/v8" || got["TARGETOS"] != "linux" || got["TARGETARCH"] != "arm64" || got["TARGETVARIANT"] != "v8" || got["BUILDPLATFORM"] != host { + t.Errorf("--platform linux/arm64/v8: %v", got) + } + if got := buildxPlatformArgs("linux/amd64"); got["TARGETARCH"] != "amd64" || got["TARGETVARIANT"] != "" { + t.Errorf("--platform linux/amd64: %v", got) + } +} diff --git a/internal/cli/messages.go b/internal/cli/messages.go index 2c00aac..539f74b 100644 --- a/internal/cli/messages.go +++ b/internal/cli/messages.go @@ -10,10 +10,6 @@ const ( // msgLegacyFeature is returned when a bare-id (legacy) Feature is requested. msgLegacyFeature = "Legacy feature '%s' not supported. Please check https://containers.dev/features for replacements.\nIf you were hoping to use local Features, remember to prepend your Feature name with \"./\". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information." - // legacyFeaturePrefix is the stable prefix of msgLegacyFeature, used to - // recognize that message as user-facing when classifying build errors. - legacyFeaturePrefix = "Legacy feature '" - // msgDockerBuildFailed is returned when `docker build` exits non-zero. msgDockerBuildFailed = "Command failed: docker build (exit %d): %s" ) diff --git a/internal/cli/metadata_label.go b/internal/cli/metadata_label.go new file mode 100644 index 0000000..1ddf683 --- /dev/null +++ b/internal/cli/metadata_label.go @@ -0,0 +1,138 @@ +package cli + +import ( + "encoding/json" + "fmt" + + "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/imagemeta" + "github.com/devcontainers/cli/internal/log" +) + +// labelConfigProperties is TS pickConfigProperties: what of devcontainer.json +// the devcontainer.metadata label records, in that order. +var labelConfigProperties = []string{ + "onCreateCommand", "updateContentCommand", "postCreateCommand", "postStartCommand", "postAttachCommand", + "waitFor", "customizations", "mounts", "containerEnv", "containerUser", "init", "privileged", "capAdd", + "securityOpt", "remoteUser", "userEnvProbe", "remoteEnv", "overrideCommand", "portsAttributes", + "otherPortsAttributes", "forwardPorts", "shutdownAction", "updateRemoteUserUID", "hostRequirements", +} + +// configLabelEntry is the configuration's entry in the devcontainer.metadata +// label: TS pick(config.raw, pickConfigProperties), the properties as written +// with their variables unresolved (raw is config.LoadResult.RawConfig). The +// merged configuration reads the substituted entry instead (configMetadataEntry). +// omitRemoteEnv is --omit-config-remote-env-from-metadata. +func configLabelEntry(raw map[string]any, omitRemoteEnv bool) (imagemeta.Entry, error) { + picked := map[string]any{} + for _, k := range labelConfigProperties { + if v, ok := raw[k]; ok && (!omitRemoteEnv || k != "remoteEnv") { + picked[k] = v + } + } + var entry imagemeta.Entry + data, err := json.Marshal(picked) + if err == nil { + err = json.Unmarshal(data, &entry) + } + if err != nil { + return imagemeta.Entry{}, fmt.Errorf("devcontainer.metadata entry of the configuration: %w", err) + } + return entry, nil +} + +// containerMetadataLabel is the devcontainer.metadata label TS adds to a +// container created from an image with no Features: JSON.stringify of the +// entries, "[]" without any. +func containerMetadataLabel(entries []imagemeta.Entry) string { + if label := imagemeta.GenerateMetadataLabel(entries); label != "" { + return label + } + return "[]" +} + +// imageLabelArgs are the --label values that put the metadata label on a built +// image: none without entries, where TS's Dockerfile has no LABEL line. +func imageLabelArgs(entries []imagemeta.Entry) []string { + if label := imagemeta.GenerateMetadataLabel(entries); label != "" { + return []string{imagemeta.MetadataLabel + "=" + label} + } + return nil +} + +// metadataSubstitution resolves the variables of metadata entries read from a +// devcontainer.metadata label, which records them as written (TS +// SubstitutedConfig.substitute applied to the label's raw entries). +type metadataSubstitution func(entries *[]imagemeta.Entry) error + +// hostMetadataSubstitution substitutes the host variables (${localEnv:…}, +// ${localWorkspaceFolder}, …) of hostSub and, with idLabels, ${devcontainerId}. +func hostMetadataSubstitution(hostSub config.HostSubContext, idLabels []string) metadataSubstitution { + return func(entries *[]imagemeta.Entry) error { + resolver := config.NewVariableResolver() + ctx := config.SubstitutionContext{HostSubContext: hostSub} + if idLabels == nil { + return resolver.ResolveInto(ctx, config.PhaseHost, entries) + } + ctx.DevContainerID = config.ComputeDevContainerID(config.EnvMap(idLabels)) + return resolver.BeforeContainerInto(ctx, entries) + } +} + +// imageMetadataFromContainer is TS getImageMetadataFromContainer(...).config: +// the entries of the container's devcontainer.metadata label (its own, or its +// image's, which the container inherits) with their variables substituted. +// For a container that carries every id label, which the CLI created with the +// configuration's entry in its metadata, the configuration's updateable +// properties (TS pickUpdateableConfigProperties: remoteUser, userEnvProbe, +// remoteEnv) follow; for any other, the features entries and the +// configuration's entry. A container without the +// label has those alone. cfg nil adds nothing of a configuration. +func imageMetadataFromContainer(labels map[string]string, cfg *config.DevContainer, features []imagemeta.Entry, idLabels []string, substitute metadataSubstitution, logger log.Logger) ([]imagemeta.Entry, error) { + var entries []imagemeta.Entry + hasLabel := labels[imagemeta.MetadataLabel] != "" + if hasLabel { + entries = imagemeta.ReadMetadataFromLabels(labels, logger) + if err := substitute(&entries); err != nil { + return nil, fmt.Errorf("substitute the container's metadata: %w", err) + } + } + if cfg == nil { + return entries, nil + } + if hasLabel && hasEveryLabel(labels, idLabels) { + entry := configMetadataEntry(cfg, false) + return append(entries, imagemeta.Entry{RemoteUser: entry.RemoteUser, UserEnvProbe: entry.UserEnvProbe, RemoteEnv: entry.RemoteEnv}), nil + } + entries = append(entries, features...) + return append(entries, configMetadataEntry(cfg, false)), nil +} + +// containerIDLabels is TS findContainerAndIdLabels' idLabels for a container +// found without the CLI's own lookup: the --id-label values, else the +// workspace folder's and the config file's labels, the config file's only when +// the container has one (an older container has the workspace folder's alone). +func containerIDLabels(provided []string, containerLabels map[string]string, workspaceFolder, configFile string) []string { + if len(provided) > 0 { + return provided + } + labels := []string{"devcontainer.local_folder=" + workspaceFolder} + if containerLabels["devcontainer.config_file"] != "" { + labels = append(labels, "devcontainer.config_file="+configFile) + } + return labels +} + +// hasEveryLabel reports whether labels has every "name=value" of idLabels (TS +// hasIdLabels: every name is set); false for nil id labels (none known). +func hasEveryLabel(labels map[string]string, idLabels []string) bool { + if idLabels == nil { + return false + } + for name := range config.EnvMap(idLabels) { + if labels[name] == "" { + return false + } + } + return true +} diff --git a/internal/cli/metadata_label_test.go b/internal/cli/metadata_label_test.go new file mode 100644 index 0000000..ebf3199 --- /dev/null +++ b/internal/cli/metadata_label_test.go @@ -0,0 +1,156 @@ +package cli + +import ( + "encoding/json" + "errors" + "reflect" + "testing" + + "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/imagemeta" +) + +// The configuration's label entry is TS pick(config.raw, pickConfigProperties): +// variables stay unresolved, properties the label does not record (runArgs, +// image, features) are left out, and --omit-config-remote-env-from-metadata drops +// remoteEnv. +func TestConfigLabelEntryPicksTheRawConfig(t *testing.T) { + var raw map[string]any + if err := json.Unmarshal([]byte(`{ + "image": "debian", + "runArgs": ["-e", "X=1"], + "features": {"ghcr.io/a/b/c:1": {}}, + "remoteUser": "vscode", + "containerEnv": {"LOCAL_PATH": "${localEnv:PATH}"}, + "remoteEnv": {"TEST": "ENV"}, + "postCreateCommand": "echo hi", + "forwardPorts": [8080, "db:5432"] + }`), &raw); err != nil { + t.Fatal(err) + } + entry, err := configLabelEntry(raw, false) + if err != nil { + t.Fatal(err) + } + got, _ := json.Marshal(entry) + want := `{"postCreateCommand":"echo hi","containerEnv":{"LOCAL_PATH":"${localEnv:PATH}"},"remoteUser":"vscode","remoteEnv":{"TEST":"ENV"},"forwardPorts":[8080,"db:5432"]}` + if string(got) != want { + t.Errorf("entry = %s\nwant %s", got, want) + } + omitted, err := configLabelEntry(raw, true) + if err != nil { + t.Fatal(err) + } + if omitted.RemoteEnv != nil { + t.Errorf("remoteEnv kept with omitRemoteEnv: %v", omitted.RemoteEnv) + } + if _, err := configLabelEntry(map[string]any{"init": "yes"}, false); err == nil { + t.Error("an init that is no boolean: want an error") + } +} + +// A container gets "[]" without metadata (TS JSON.stringify of no entries); an +// image build gets no label at all, as TS's Dockerfile has no LABEL line. +func TestMetadataLabelsWithoutEntries(t *testing.T) { + if got := containerMetadataLabel(nil); got != "[]" { + t.Errorf("container label = %q, want []", got) + } + if got := containerMetadataLabel([]imagemeta.Entry{{RemoteUser: "a"}}); got != `[{"remoteUser":"a"}]` { + t.Errorf("container label = %q", got) + } + if got := imageLabelArgs([]imagemeta.Entry{{}}); got != nil { + t.Errorf("image labels = %q, want none", got) + } + if got := imageLabelArgs([]imagemeta.Entry{{RemoteUser: "a"}}); !reflect.DeepEqual(got, []string{`devcontainer.metadata=[{"remoteUser":"a"}]`}) { + t.Errorf("image labels = %q", got) + } +} + +// TS getImageMetadataFromContainer: the container's label entries, substituted; +// for a container with every id label, which records the configuration's entry, +// only the configuration's updateable properties follow, so its hooks do not +// run twice; for any other the Features' entries and the configuration's; no +// label gives the configuration's entry alone. +func TestImageMetadataFromContainer(t *testing.T) { + cfg := &config.DevContainer{RemoteUser: "me", UserEnvProbe: "none", RemoteEnv: map[string]*string{"A": strPtr("b")}} + cfg.PostCreateCommand = config.LifecycleCommand{} + if err := json.Unmarshal([]byte(`"cfg-pc"`), &cfg.PostCreateCommand); err != nil { + t.Fatal(err) + } + labels := map[string]string{ + imagemeta.MetadataLabel: `[{"id":"feat","postCreateCommand":"feat-pc"},{"postCreateCommand":"cfg-pc","containerEnv":{"P":"${localEnv:HOME}"}}]`, + "devcontainer.local_folder": "/ws", + } + substitute := hostMetadataSubstitution(config.HostSubContext{Env: map[string]string{"HOME": "/home/me"}}, nil) + features := []imagemeta.Entry{{ID: "f2"}} + + got, err := imageMetadataFromContainer(labels, cfg, features, []string{"devcontainer.local_folder=/ws"}, substitute, quietLogger()) + if err != nil { + t.Fatal(err) + } + merged := imagemeta.MergeConfiguration(got) + if !reflect.DeepEqual(merged.PostCreateCommands, []any{"feat-pc", "cfg-pc"}) { + t.Errorf("with the id labels: postCreateCommands = %v, want the label's alone", merged.PostCreateCommands) + } + if merged.ContainerEnv["P"] != "/home/me" || merged.RemoteUser != "me" || merged.UserEnvProbe != "none" || *merged.RemoteEnv["A"] != "b" { + t.Errorf("with the id labels: merged = %+v, want the label substituted and the updateable properties", merged) + } + + got, err = imageMetadataFromContainer(labels, cfg, features, []string{"other=label"}, substitute, quietLogger()) + if err != nil { + t.Fatal(err) + } + if len(got) != 4 || got[2].ID != "f2" || got[3].PostCreateCommand != "cfg-pc" { + t.Errorf("without the id labels: %+v, want the label, the Features and the config's entry", got) + } + + got, err = imageMetadataFromContainer(map[string]string{}, cfg, features, nil, substitute, quietLogger()) + if err != nil || len(got) != 2 || got[0].ID != "f2" || got[1].RemoteUser != "me" { + t.Errorf("without a label: %+v, %v; want the Features and the config's entry", got, err) + } + + if got, err := imageMetadataFromContainer(labels, nil, features, nil, substitute, quietLogger()); err != nil || len(got) != 2 { + t.Errorf("without a config: %+v, %v; want the label's entries", got, err) + } + + failing := func(*[]imagemeta.Entry) error { return errors.New("boom") } + if _, err := imageMetadataFromContainer(labels, cfg, nil, nil, failing, quietLogger()); err == nil { + t.Error("a failing substitution: want the error") + } +} + +// TS findContainerAndIdLabels: the --id-label values, else the workspace +// folder's and config file's labels, the config file's only for a container +// that has one. +func TestContainerIDLabels(t *testing.T) { + if got := containerIDLabels([]string{"a=b"}, nil, "/ws", "/ws/c.json"); !reflect.DeepEqual(got, []string{"a=b"}) { + t.Errorf("provided = %v", got) + } + withConfig := map[string]string{"devcontainer.config_file": "/ws/c.json"} + if got := containerIDLabels(nil, withConfig, "/ws", "/ws/c.json"); !reflect.DeepEqual(got, []string{"devcontainer.local_folder=/ws", "devcontainer.config_file=/ws/c.json"}) { + t.Errorf("container with a config file label = %v", got) + } + if got := containerIDLabels(nil, map[string]string{}, "/ws", "/ws/c.json"); !reflect.DeepEqual(got, []string{"devcontainer.local_folder=/ws"}) { + t.Errorf("older container = %v", got) + } +} + +// The label's entries get the host substitutions, and ${devcontainerId} only +// with id labels to compute it from. +func TestHostMetadataSubstitution(t *testing.T) { + hostSub := config.HostSubContext{Env: map[string]string{"U": "me"}} + entries := []imagemeta.Entry{{RemoteUser: "${localEnv:U}", ContainerEnv: map[string]string{"ID": "${devcontainerId}"}}} + if err := hostMetadataSubstitution(hostSub, nil)(&entries); err != nil { + t.Fatal(err) + } + if entries[0].RemoteUser != "me" || entries[0].ContainerEnv["ID"] != "${devcontainerId}" { + t.Errorf("without id labels: %+v", entries[0]) + } + idLabels := []string{"devcontainer.local_folder=/ws"} + if err := hostMetadataSubstitution(hostSub, idLabels)(&entries); err != nil { + t.Fatal(err) + } + if want := config.ComputeDevContainerID(config.EnvMap(idLabels)); entries[0].ContainerEnv["ID"] != want { + t.Errorf("with id labels: ID = %q, want %q", entries[0].ContainerEnv["ID"], want) + } +} diff --git a/internal/cli/pure_helpers_test.go b/internal/cli/pure_helpers_test.go index e8c0299..f0aaf8c 100644 --- a/internal/cli/pure_helpers_test.go +++ b/internal/cli/pure_helpers_test.go @@ -2,6 +2,7 @@ package cli import ( "encoding/json" + "fmt" "os" "path/filepath" "strings" @@ -264,12 +265,20 @@ func TestBuildArgsAndOptionsFromConfig(t *testing.T) { } } -func TestIsUserFacingBuildError(t *testing.T) { - if !isUserFacingBuildError(errorString(legacyFeaturePrefix + "foo' ...")) { - t.Error("a legacy-feature error must be user-facing") - } - if isUserFacingBuildError(errorString("some internal failure")) { - t.Error("an arbitrary error must not be user-facing") +// up and build report a ContainerError as it is, as TS does for the errors it +// throws with their own description (a legacy Feature id, a failing compose +// config), wherever it is wrapped; any other error gets the command's +// description with the error appended. +func TestCommandErrorOutputKeepsAContainerError(t *testing.T) { + legacy := checkLegacyFeatures(map[string]any{"myfeature": true}) + out := commandErrorOutput(fmt.Errorf("install features: %w", legacy), "An error occurred setting up the container") + want := fmt.Sprintf(msgLegacyFeature, "myfeature") + if out.Description != want || out.Message != want { + t.Errorf("legacy Feature error = %+v", out) + } + out = commandErrorOutput(errorString("some internal failure"), "An error occurred building the container") + if want := "An error occurred building the container: some internal failure"; out.Description != want || out.Message != want { + t.Errorf("other error = %+v", out) } } diff --git a/internal/cli/read_configuration.go b/internal/cli/read_configuration.go index 5226a6c..b792049 100644 --- a/internal/cli/read_configuration.go +++ b/internal/cli/read_configuration.go @@ -142,7 +142,10 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) } containerID := readConfigurationContainerID(ctx, engine, opts, workspaceFolder, result) - containerCfg := inspectContainerConfig(ctx, engine, containerID) + containerCfg, inspectErr := inspectContainerConfig(ctx, engine, containerID) + if inspectErr != nil { + return readConfigurationFailure(lgr, inspectErr) + } cfgMap, subErr := substituteContainerEnv(containerCfg, cfgMap) if subErr != nil { return writeErrorResult(out, fmt.Sprintf("resolve configuration container variables: %v", subErr)) @@ -174,8 +177,7 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) if featErr != nil { // TS readConfiguration writes the error to the log and exits 1 with no // result; a Feature that cannot be resolved is not an empty one. - lgr.Write(featErr.Error(), log.LevelError) - return &coreerrors.ExitCodeError{Code: 1, Err: featErr} + return readConfigurationFailure(lgr, featErr) } if featResult != nil { defer os.RemoveAll(featResult.TmpDir) @@ -200,17 +202,21 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) // featuresConfiguration above, as in TS. buildInfo, biErr := readConfigurationImageBuildInfo(ctx, lgr, engine, opts.dockerPath, result) if biErr != nil { - lgr.Write(biErr.Error(), log.LevelError) - return &coreerrors.ExitCodeError{Code: 1, Err: biErr} + return readConfigurationFailure(lgr, biErr) } base = append(base, buildInfo.Metadata...) - if featResult != nil { - for _, fs := range featResult.FeatureSets { - base = append(base, featureMetadataEntry(fs, false)) - } + } + var featureEntries []imagemeta.Entry + if featResult != nil { + for _, fs := range featResult.FeatureSets { + featureEntries = append(featureEntries, featureMetadataEntry(fs, false)) } } - merged, mergeErr := mergedConfigurationOutput(base, containerCfg, result, cfgMap, lgr) + entries, entriesErr := readConfigurationMetadata(append(base, featureEntries...), featureEntries, containerCfg, result, opts.idLabels, workspaceFolder, lgr) + if entriesErr != nil { + return readConfigurationFailure(lgr, entriesErr) + } + merged, mergeErr := mergedConfigurationOutput(entries, containerCfg, cfgMap) if mergeErr != nil { return mergeErr } @@ -230,6 +236,14 @@ func runReadConfiguration(ctx context.Context, out Output, opts *readConfigOpts) return nil } +// readConfigurationFailure is how read-configuration fails, as TS +// readConfiguration does: the error is logged and the command exits 1 with no +// result. +func readConfigurationFailure(lgr log.Logger, err error) error { + lgr.Write(err.Error(), log.LevelError) + return &coreerrors.ExitCodeError{Code: 1, Err: err} +} + // readConfigurationPaths resolves --workspace-folder, --config and // --override-config against cwd. Without --container-id, --id-label or // --workspace-folder, the workspace folder is cwd (0.88). @@ -329,16 +343,20 @@ func readConfigurationContainerID(ctx context.Context, engine *docker.EngineClie } // inspectContainerConfig is the config of container cid, nil when there is no -// container to inspect or it cannot be inspected. -func inspectContainerConfig(ctx context.Context, engine *docker.EngineClient, cid string) *container.Config { - if cid == "" || engine == nil { - return nil +// container to inspect. A container that cannot be inspected is an error, as +// TS findContainerAndIdLabels' inspectContainer throws. +func inspectContainerConfig(ctx context.Context, engine *docker.EngineClient, cid string) (*container.Config, error) { + if cid == "" { + return nil, nil + } + if engine == nil { + return nil, fmt.Errorf("inspect container %s: no Docker engine", cid) } inspect, err := engine.InspectContainer(ctx, cid) if err != nil { - return nil + return nil, fmt.Errorf("inspect container %s: %w", cid, err) } - return inspect.Config + return inspect.Config, nil } // substituteContainerEnv resolves ${containerEnv:...} in cfgMap with the @@ -384,21 +402,35 @@ func needsFeaturesConfiguration(opts *readConfigOpts, containerID string, result return wanted && result != nil && len(result.Config.Features) > 0 } -// mergedConfigurationOutput is read-configuration's "mergedConfiguration". The -// metadata entries are the container's devcontainer.metadata label when there is -// a container, else base (the image's and the Features' metadata); the config is -// the last, highest-priority entry. With a container, ${containerEnv:...} is -// resolved with its environment. TS builds mergedConfiguration by spreading the -// config, so the properties mergeConfiguration does not own (image, features, -// build, configFilePath, ...) come from cfgMap. -func mergedConfigurationOutput(base []imagemeta.Entry, containerCfg *container.Config, result *config.LoadResult, cfgMap map[string]any, lgr log.Logger) (map[string]any, error) { - entries := base - if containerCfg != nil { - entries = imagemeta.ReadMetadataFromLabels(containerCfg.Labels, lgr) - } +// readConfigurationMetadata is the metadata read-configuration merges. With a +// container it is TS getImageMetadataFromContainer: the container's +// devcontainer.metadata entries, substituted, then the configuration's +// updateable properties for a container with every id label, else the +// Features' entries and the configuration's entry. Without one it is base (the +// image's and the Features' metadata) and the configuration's entry. +func readConfigurationMetadata(base, featureEntries []imagemeta.Entry, containerCfg *container.Config, result *config.LoadResult, providedIDLabels []string, workspaceFolder string, lgr log.Logger) ([]imagemeta.Entry, error) { + var cfg *config.DevContainer + hostSub, configFile := config.EnvHostSubContext(), "" if result != nil { - entries = append(entries, configToMetadataEntry(result.Config)) + cfg, hostSub, configFile = result.Config, result.HostSub, result.Config.ConfigFilePath } + if containerCfg == nil { + if cfg == nil { + return base, nil + } + return append(base, configToMetadataEntry(cfg)), nil + } + idLabels := containerIDLabels(providedIDLabels, containerCfg.Labels, workspaceFolder, configFile) + return imageMetadataFromContainer(containerCfg.Labels, cfg, featureEntries, idLabels, hostMetadataSubstitution(hostSub, idLabels), lgr) +} + +// mergedConfigurationOutput is read-configuration's "mergedConfiguration": the +// merge of entries (readConfigurationMetadata). With a container, +// ${containerEnv:...} is resolved with its environment. TS builds +// mergedConfiguration by spreading the config, so the properties +// mergeConfiguration does not own (image, features, build, configFilePath, ...) +// come from cfgMap. +func mergedConfigurationOutput(entries []imagemeta.Entry, containerCfg *container.Config, cfgMap map[string]any) (map[string]any, error) { merged := imagemeta.MergeConfiguration(entries) if containerCfg != nil { @@ -468,7 +500,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/read_configuration_held_test.go b/internal/cli/read_configuration_held_test.go index 994934a..82738ed 100644 --- a/internal/cli/read_configuration_held_test.go +++ b/internal/cli/read_configuration_held_test.go @@ -178,6 +178,8 @@ func TestReadConfigurationContainerID(t *testing.T) { } } +// A container read-configuration cannot inspect fails it, as TS's +// inspectContainer throws; with no container there is nothing to inspect. func TestInspectContainerConfig(t *testing.T) { ctx := context.Background() api := &fakeEngineAPI{containers: map[string]string{ @@ -185,19 +187,20 @@ func TestInspectContainerConfig(t *testing.T) { "noconfig": `{"Id":"noconfig"}`, }} engine := &docker.EngineClient{API: api, Log: log.Null} - if got := inspectContainerConfig(ctx, engine, "c1"); got == nil || !slices.Equal(got.Env, []string{"A=b"}) { - t.Errorf("c1 = %+v, want its config", got) - } - for name, got := range map[string]*container.Config{ - "no container": inspectContainerConfig(ctx, engine, ""), - "no engine": inspectContainerConfig(ctx, nil, "c1"), - "missing container": inspectContainerConfig(ctx, engine, "missing"), - "container w/o config": inspectContainerConfig(ctx, engine, "noconfig"), - } { - if got != nil { - t.Errorf("%s = %+v, want nil", name, got) + if got, err := inspectContainerConfig(ctx, engine, "c1"); err != nil || got == nil || !slices.Equal(got.Env, []string{"A=b"}) { + t.Errorf("c1 = %+v, %v; want its config", got, err) + } + for name, cid := range map[string]string{"no container": "", "container w/o config": "noconfig"} { + if got, err := inspectContainerConfig(ctx, engine, cid); got != nil || err != nil { + t.Errorf("%s = %+v, %v; want nil", name, got, err) } } + if _, err := inspectContainerConfig(ctx, engine, "missing"); err == nil || !strings.Contains(err.Error(), "inspect container missing") { + t.Errorf("missing container: err = %v, want the inspect error", err) + } + if _, err := inspectContainerConfig(ctx, nil, "c1"); err == nil { + t.Error("no engine: want an error") + } } func TestSubstituteContainerEnv(t *testing.T) { @@ -270,16 +273,26 @@ func TestMergedConfigurationOutput(t *testing.T) { "image": "img:1", "remoteUser": "config-user", "remoteEnv": {"P": "${containerEnv:PATH}"}, + "postStartCommand": "from-config", "x-vendor": true }`, nil) cfgMap := configurationOutput(result, &readConfigOpts{}) base := []imagemeta.Entry{{RemoteUser: "image-user", ContainerUser: "image-container-user"}} - - t.Run("without a container", func(t *testing.T) { - got, err := mergedConfigurationOutput(base, nil, result, cfgMap, log.Null) + merge := func(t *testing.T, base, features []imagemeta.Entry, cc *container.Config, result *config.LoadResult, idLabels []string, cfgMap map[string]any) map[string]any { + t.Helper() + entries, err := readConfigurationMetadata(base, features, cc, result, idLabels, "/ws", log.Null) if err != nil { t.Fatal(err) } + got, err := mergedConfigurationOutput(entries, cc, cfgMap) + if err != nil { + t.Fatal(err) + } + return got + } + + t.Run("without a container", func(t *testing.T) { + got := merge(t, base, nil, nil, result, nil, cfgMap) want := map[string]any{"remoteUser": "config-user", "containerUser": "image-container-user", "image": "img:1", "x-vendor": true, "configFilePath": cfgMap["configFilePath"]} for k, v := range want { if !reflect.DeepEqual(got[k], v) { @@ -294,27 +307,40 @@ func TestMergedConfigurationOutput(t *testing.T) { t.Run("with a container", func(t *testing.T) { cc := &container.Config{ Env: []string{"PATH=/container/bin"}, - Labels: map[string]string{"devcontainer.metadata": `[{"containerUser":"label-user","postStartCommand":"from-label"}]`}, - } - got, err := mergedConfigurationOutput(base, cc, result, cfgMap, log.Null) - if err != nil { - t.Fatal(err) + Labels: map[string]string{"devcontainer.metadata": `[{"containerUser":"label-user","postStartCommand":"from-label","containerEnv":{"H":"${localEnv:HOME}"}}]`}, } + got := merge(t, nil, []imagemeta.Entry{{ID: "feat", PostStartCommand: "from-feature"}}, cc, result, nil, cfgMap) if got["containerUser"] != "label-user" || got["remoteUser"] != "config-user" { t.Errorf("users = %v, %v; want the label's containerUser and the config's remoteUser", got["containerUser"], got["remoteUser"]) } - if !reflect.DeepEqual(got["postStartCommands"], []any{"from-label"}) { - t.Errorf("postStartCommands = %v, want the label's", got["postStartCommands"]) + if !reflect.DeepEqual(got["postStartCommands"], []any{"from-label", "from-feature", "from-config"}) { + t.Errorf("postStartCommands = %v, want the label's, the Features' and the config's", got["postStartCommands"]) } if env := got["remoteEnv"]; !reflect.DeepEqual(env, map[string]any{"P": "/container/bin"}) { t.Errorf("remoteEnv = %v, want ${containerEnv:PATH} resolved with the container's env", env) } + if env, _ := got["containerEnv"].(map[string]any); env["H"] != result.HostSub.Env["HOME"] { + t.Errorf("containerEnv = %v, want the label's ${localEnv:HOME} substituted", got["containerEnv"]) + } + }) + + t.Run("with a container the CLI created", func(t *testing.T) { + // Its label records the configuration's entry: only the configuration's + // updateable properties follow, so its commands are not merged twice. + cc := &container.Config{Labels: map[string]string{ + "devcontainer.metadata": `[{"postStartCommand":"from-config"}]`, + "devcontainer.local_folder": "/ws", + }} + got := merge(t, nil, nil, cc, result, nil, cfgMap) + if !reflect.DeepEqual(got["postStartCommands"], []any{"from-config"}) || got["remoteUser"] != "config-user" { + t.Errorf("merged = %v, want the label's command once and the config's remoteUser", got) + } }) t.Run("without a config", func(t *testing.T) { - got, err := mergedConfigurationOutput(base, nil, nil, map[string]any{}, log.Null) - if err != nil || got["remoteUser"] != "image-user" { - t.Fatalf("got %v, %v; want the base entries alone", got, err) + got := merge(t, base, nil, nil, nil, nil, map[string]any{}) + if got["remoteUser"] != "image-user" { + t.Fatalf("got %v; want the base entries alone", got) } }) } @@ -409,17 +435,12 @@ func TestReadConfigurationCommandWithoutConfigExitsOne(t *testing.T) { } func TestReadConfigurationCommandContainerIDWithoutWorkspace(t *testing.T) { - // --container-id alone loads no configuration: there is no workspace folder - // or config file to read one from. - out, err := runReadConfigurationCmd(t, "--container-id", "devcontainer-cli-test-no-such-container") - if err != nil { - t.Fatal(err) - } - if cfg, ok := out["configuration"].(map[string]any); !ok || len(cfg) != 0 { - t.Errorf("configuration = %v, want an empty object", out["configuration"]) - } - if ws, ok := out["workspace"]; ok { - t.Errorf("workspace = %v, want none without a configuration", ws) + // --container-id alone loads no configuration; a container that cannot be + // inspected fails the command with no result, as TS's inspectContainer throws. + _, err := runReadConfigurationCmd(t, "--container-id", "devcontainer-cli-test-no-such-container") + wantExitCode(t, err, 1) + if !strings.Contains(err.Error(), "inspect container devcontainer-cli-test-no-such-container") { + t.Errorf("err = %v, want the inspect failure (no configuration is loaded)", err) } } diff --git a/internal/cli/run_user_commands.go b/internal/cli/run_user_commands.go index 2813947..5b0e5ef 100644 --- a/internal/cli/run_user_commands.go +++ b/internal/cli/run_user_commands.go @@ -180,21 +180,21 @@ func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCo if err != nil { return writeErrorResult(out, fmt.Sprintf("Failed to inspect container: %v", err)) } - // Build merged config. With a config loaded, read the image's metadata - // (features are baked into the image) + the config entry — reading the - // container label (which now carries the config metadata) and re-adding the - // config entry would double the lifecycle commands. Without a config - // (container-id path) the container metadata is authoritative. - var allEntries []imagemeta.Entry - if cfg != nil { - if inspect.Image != "" { - if img, imgErr := engine.InspectImage(ctx, inspect.Image); imgErr == nil && img.Config != nil { - allEntries = imagemeta.ReadMetadataFromLabels(img.Config.Labels, logger) - } - } - allEntries = append(allEntries, configToMetadataEntry(cfg)) - } else { - allEntries = imagemeta.ReadMetadataFromLabels(inspect.Config.Labels, logger) + // The merged configuration is the container's metadata (TS + // getImageMetadataFromContainer), which records the configuration's entry + // for a container the CLI created. + hostSub, configFile := config.EnvHostSubContext(), "" + if loadResult != nil { + hostSub, configFile = loadResult.HostSub, loadResult.Config.ConfigFilePath + } + var containerLabels map[string]string + if inspect.Config != nil { + containerLabels = inspect.Config.Labels + } + idLabels := containerIDLabels(opts.idLabels, containerLabels, resolvePath(opts.workspaceFolder), configFile) + allEntries, err := imageMetadataFromContainer(containerLabels, cfg, nil, idLabels, hostMetadataSubstitution(hostSub, idLabels), logger) + if err != nil { + return writeErrorResult(out, err.Error()) } merged := imagemeta.MergeConfiguration(allEntries) 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/cli/up.go b/internal/cli/up.go index b561659..5c65da7 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -90,6 +90,14 @@ type upRunner struct { docker *docker.Client engine *docker.EngineClient opts *upOpts + // idLabels are the labels the dev container is found by. + idLabels []string + // substitute resolves the variables of entries read from a + // devcontainer.metadata label (TS configWithRaw.substitute). + substitute metadataSubstitution + // configLabel is the configuration's entry in the devcontainer.metadata + // label: its properties as written (TS pick(config.raw, ...)). + configLabel imagemeta.Entry } func newUpCmd() *cobra.Command { @@ -331,6 +339,11 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error { if err := resolver.BeforeContainerInto(idCtx, loadResult.WorkspaceConfig); err != nil { return writeErrorResult(out, fmt.Sprintf("apply workspace pre-container substitutions: %v", err)) } + run.idLabels = idLabels + run.substitute = func(entries *[]imagemeta.Entry) error { return resolver.BeforeContainerInto(idCtx, entries) } + if run.configLabel, err = configLabelEntry(loadResult.RawConfig, opts.omitConfigRemoteEnvFromMeta); err != nil { + return writeErrorResult(out, err.Error()) + } // Apply --remove-existing-container up-front, BEFORE initializeCommand, so a // failed initializeCommand (or rebuild) does not leave the old container @@ -409,9 +422,7 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error { containerID, err = run.fromImage(ctx, cfg, loadResult, workspaceFolder, idLabels, useBuildx) } if err != nil { - return writeErrorJSON(out, coreerrors.ToErrorOutput(&coreerrors.ContainerError{ - Description: fmt.Sprintf("An error occurred setting up the container: %v", err), - })) + return writeErrorJSON(out, commandErrorOutput(err, "An error occurred setting up the container")) } } @@ -449,7 +460,7 @@ func runUp(ctx context.Context, out Output, opts *upOpts) error { result["composeProjectName"] = opts.composeProjectName } - if err := addUpConfiguration(result, opts, cfg, inspectResp, imageLabels, logger); err != nil { + if err := addUpConfiguration(result, opts, cfg, inspectResp, run.metadataOfContainer, logger); err != nil { return writeErrorResult(out, err.Error()) } @@ -505,13 +516,10 @@ func upRemoteUser(cfgUser string, ctr container.InspectResponse, imageLabels fun // mergedConfiguration (--include-merged-configuration) to up's result, with // ${containerEnv:…} resolved from the container's environment as TS does. // -// mergedConfiguration reads the image's metadata (features are baked into the -// image), not the container's: the container carries the full config metadata -// as a label, so reading it and re-adding the config entry would double the -// lifecycle commands. Its config entry uses the config's remoteEnv regardless of -// --omit-config-remote-env-from-metadata (that flag only affects the persisted -// label). -func addUpConfiguration(result map[string]any, opts *upOpts, cfg *config.DevContainer, ctr container.InspectResponse, imageLabels func(ref string) map[string]string, logger log.Logger) error { +// mergedConfiguration merges the container's metadata (metadata, TS +// getImageMetadataFromContainer), which records the configuration's entry, as +// TS's up does for an existing container. +func addUpConfiguration(result map[string]any, opts *upOpts, cfg *config.DevContainer, ctr container.InspectResponse, metadata func(ctr container.InspectResponse, cfg *config.DevContainer) ([]imagemeta.Entry, error), logger log.Logger) error { if !opts.includeConfig && !opts.includeMergedConfig { return nil } @@ -557,8 +565,11 @@ func addUpConfiguration(result map[string]any, opts *upOpts, cfg *config.DevCont } if opts.includeMergedConfig { - entries := imagemeta.ReadMetadataFromLabels(imageLabels(ctr.Image), logger) - merged := imagemeta.MergeConfiguration(append(entries, configToMetadataEntry(cfg))) + entries, err := metadata(ctr, cfg) + if err != nil { + return err + } + merged := imagemeta.MergeConfiguration(entries) // image is spread from the config into mergedConfiguration (TS); it is // not part of the metadata entries. merged.Image = cfg.Image @@ -614,19 +625,15 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer, if cfg.Build != nil { findTarget = cfg.Build.Target } - baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), findTarget) + baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), findTarget, buildxPlatformArgs(opts.platform)) - metadata := []imagemeta.Entry{} // Read the FROM image's baked devcontainer.metadata (e.g. remoteUser=node on // the mcr devcontainers images). The metadata is computed before the build, so // on a fresh run the base image is otherwise absent and its metadata would be // silently lost — making exec/lifecycle fall back to root. baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage) - metadata = append(metadata, imagemeta.ReadMetadataFromLabels(baseLabels, logger)...) - // Preserve a `LABEL devcontainer.metadata` from the user's Dockerfile (#1225). - metadata = append(metadata, imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger)...) - metadata = append(metadata, imagemeta.Entry{RemoteUser: cfg.RemoteUser, ContainerUser: cfg.ContainerUser}) - metadataLabel := imagemeta.GenerateMetadataLabel(metadata) + baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger) + stageMetadata := imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger) contextPath := filepath.Join(configDir, cfg.BuildContext()) @@ -644,7 +651,7 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer, BuildArgs: buildArgsFromConfig(cfg), ExtraArgs: buildOptionsFromConfig(cfg), CacheFrom: upCacheFrom, - Labels: []string{fmt.Sprintf("%s=%s", imagemeta.MetadataLabel, metadataLabel)}, + Labels: imageLabelArgs(dockerfileImageMetadata(cfg, baseMetadata, stageMetadata, r.configLabel)), NoCache: opts.buildNoCache, UseBuildx: useBuildx, CacheTo: opts.cacheTo, @@ -679,7 +686,7 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer, return "", err } - return r.runContainer(ctx, imageName, cfg, loadResult, workspaceFolder, idLabels) + return r.runContainer(ctx, imageName, cfg, loadResult, workspaceFolder, idLabels, true) } // fromImage pulls the config's image when it is missing, installs its @@ -728,7 +735,9 @@ func (r *upRunner) fromImage(ctx context.Context, cfg *config.DevContainer, load return "", err } - return r.runContainer(ctx, imageName, cfg, loadResult, workspaceFolder, idLabels) + // TS getExtendImageBuildInfo: an image with no Features is not built, so the + // metadata label, with the configuration's entry, goes on the container. + return r.runContainer(ctx, imageName, cfg, loadResult, workspaceFolder, idLabels, len(cfg.Features) > 0) } // fromCacheImage starts the container from a prebuilt image (--cache-image), @@ -757,7 +766,11 @@ func (r *upRunner) fromCacheImage(ctx context.Context, cfg *config.DevContainer, if err != nil { return "", err } - return r.runContainer(ctx, image, cfg, loadResult, workspaceFolder, idLabels) + // A prebuilt image with a metadata label is a build of the configuration, + // whose label records the configuration's entry; one without is used as an + // image config's image is. + _, configInImage := r.imageLabels(ctx, image)[imagemeta.MetadataLabel] + return r.runContainer(ctx, image, cfg, loadResult, workspaceFolder, idLabels, configInImage) } // singleContainerRemoteUserUID is TS singleContainer's updateRemoteUserUID call: @@ -797,24 +810,29 @@ func findUserArg(runArgs []string) string { // up.override-command-false-success, up.workspace-mount-custom-success, // up.image-with-mounts-success and up.config-without-workspace-success create // their container through it; also TestE2E_ReadonlyMountObject. -func (r *upRunner) runContainer(ctx context.Context, imageName string, cfg *config.DevContainer, loadResult *config.LoadResult, workspaceFolder string, idLabels []string) (string, error) { +func (r *upRunner) runContainer(ctx context.Context, imageName string, cfg *config.DevContainer, loadResult *config.LoadResult, workspaceFolder string, idLabels []string, configInImage bool) (string, error) { logger, dockerClient, engine, opts := r.log, r.docker, r.engine, r.opts // Inspect image once — used for metadata and containerUser resolution. imageInspect, _ := engine.InspectImage(ctx, imageName) - var metaEntries []imagemeta.Entry + var labelEntries []imagemeta.Entry if imageInspect.Config != nil { - metaEntries = append(metaEntries, imagemeta.ReadMetadataFromLabels(imageInspect.Config.Labels, logger)...) + labelEntries = imagemeta.ReadMetadataFromLabels(imageInspect.Config.Labels, logger) + } + metaEntries := slices.Clone(labelEntries) + if err := r.substitute(&metaEntries); err != nil { + return "", fmt.Errorf("substitute the image's metadata: %w", err) + } + containerLabels := append([]string{}, idLabels...) + // A Dockerfile or Features build put the metadata, with the configuration's + // entry, on the image (configInImage); otherwise TS adds the label, the + // image's entries and the configuration's as written, to the container. + if !configInImage { + metaEntries = append(metaEntries, configMetadataEntry(cfg, opts.omitConfigRemoteEnvFromMeta)) + containerLabels = append(containerLabels, imagemeta.MetadataLabel+"="+containerMetadataLabel(append(labelEntries, r.configLabel))) } - metaEntries = append(metaEntries, configMetadataEntry(cfg, opts.omitConfigRemoteEnvFromMeta)) merged := imagemeta.MergeConfiguration(metaEntries) - // The container carries the full devcontainer.metadata (image + config) so the - // CLI/VS Code can read remoteUser/lifecycle/customizations back on reattach — - // image-based containers previously had no metadata label. The label matches - // what the TS CLI adds with `-l`. - containerMetadataLabel := imagemeta.GenerateMetadataLabel(metaEntries) - // Build CMD script with feature entrypoints cmdScript := docker.BuildEntrypointScript(merged.Entrypoints) @@ -861,11 +879,6 @@ func (r *upRunner) runContainer(ctx context.Context, imageName string, cfg *conf // appPort → publish flags (127.0.0.1:N:N for numbers, verbatim for strings). runArgs = append(runArgs, appPortPublishArgs(cfg.AppPort)...) - containerLabels := append([]string{}, idLabels...) - if containerMetadataLabel != "" { - containerLabels = append(containerLabels, imagemeta.MetadataLabel+"="+containerMetadataLabel) - } - // Command: the keep-alive script wrapping any feature entrypoints. When // overrideCommand is false, append the image's own entrypoint+cmd so the // script's `exec "$@"` runs them (e.g. dind, databases) instead of just @@ -1171,15 +1184,15 @@ func (r *upRunner) runLifecycleForUp(ctx context.Context, containerID string, cf executor := &lifecycle.ShellExecutor{Server: shellServer, Log: logger, WorkDir: workDir} - // Read metadata from the image (features' lifecycle commands are baked into - // the image), NOT the container: the container now carries the config metadata - // as a label, so reading it and then re-adding the config commands below would - // run every hook twice. - // A container the engine cannot inspect has no image: its hooks are the - // config's alone. + // The hooks are the container's metadata's (TS getImageMetadataFromContainer), + // which records the configuration's entry; a container the engine cannot + // inspect has the configuration's alone. inspectResp, _ := engine.InspectContainer(ctx, containerID) - merged := imagemeta.MergeConfiguration(imagemeta.ReadMetadataFromLabels(r.imageLabels(ctx, inspectResp.Image), logger)) - appendConfigHooks(merged, cfg) + entries, err := r.metadataOfContainer(inspectResp, cfg) + if err != nil { + return hookContainerError(err, containerID, "An error occurred setting up the container.") + } + merged := imagemeta.MergeConfiguration(entries) applyLifecycleMarkers(shellServer, merged, inspectResp, opts.prebuild) @@ -1233,26 +1246,6 @@ func hookRemoteEnv(probed map[string]string, extra []string, cfgRemoteEnv map[st return env } -// appendConfigHooks adds the config's lifecycle commands after the image -// metadata's. It keeps each raw value so object-form commands still run in -// parallel like the TS CLI. -func appendConfigHooks(merged *imagemeta.MergedConfig, cfg *config.DevContainer) { - for _, h := range []struct { - list *[]any - cmd *config.LifecycleCommand - }{ - {&merged.OnCreateCommands, &cfg.OnCreateCommand}, - {&merged.UpdateContentCommands, &cfg.UpdateContentCommand}, - {&merged.PostCreateCommands, &cfg.PostCreateCommand}, - {&merged.PostStartCommands, &cfg.PostStartCommand}, - {&merged.PostAttachCommands, &cfg.PostAttachCommand}, - } { - if !h.cmd.IsEmpty() { - *h.list = append(*h.list, h.cmd.Raw()) - } - } -} - // hookContainerError is TS runLifecycleCommand's ContainerError for a failed // lifecycle command: the description names the command and where it comes from, // the message is the command's own error, and up reports the container. Any @@ -1367,5 +1360,28 @@ func (r *upRunner) featureBuildOptions(cfg *config.DevContainer, loadResult *con ConfigPath: cfg.ConfigFilePath, LockfileExcludeIDs: opts.lockfileExcludeIDs, HostSub: &loadResult.HostSub, + // TS getFeaturesBuildOptions writes the config's containerEnv into the + // Features image as ENV lines (#{containerEnvMetadata}). + ContainerEnv: cfg.ContainerEnv, + // The configuration's entry: substituted for the Features' users, as + // written in the image's metadata label. + AdditionalMetadata: []imagemeta.Entry{configMetadataEntry(cfg, false)}, + LabelMetadata: []imagemeta.Entry{r.configLabel}, + } +} + +// metadataOfContainer is the container's metadata, TS +// getImageMetadataFromContainer with up's id labels and substitutions. A +// container without a config (the engine could not inspect it) has the +// configuration's entry alone. +// +// mutate-lane: TestParityMatrix - up.feature-hook-failure-error and +// up.dockerfile-with-parallel-commands-success run the hooks it reads from a +// real container; also TestDifferentialUp. +func (r *upRunner) metadataOfContainer(ctr container.InspectResponse, cfg *config.DevContainer) ([]imagemeta.Entry, error) { + var labels map[string]string + if ctr.Config != nil { + labels = ctr.Config.Labels } + return imageMetadataFromContainer(labels, cfg, nil, r.idLabels, r.substitute, r.log) } diff --git a/internal/cli/up_held_test.go b/internal/cli/up_held_test.go index 0989683..2686137 100644 --- a/internal/cli/up_held_test.go +++ b/internal/cli/up_held_test.go @@ -112,12 +112,20 @@ func TestUpRemoteUserFallsBackInTSOrder(t *testing.T) { } } +// containerMetadata reads a container's metadata labels without substitution +// or id labels: its entries, then the configuration's. +func containerMetadata(labels map[string]string) func(container.InspectResponse, *config.DevContainer) ([]imagemeta.Entry, error) { + return func(_ container.InspectResponse, cfg *config.DevContainer) ([]imagemeta.Entry, error) { + return imageMetadataFromContainer(labels, cfg, nil, nil, func(*[]imagemeta.Entry) error { return nil }, quietLogger()) + } +} + // upConfigurationJSON is what addUpConfiguration adds to up's result, as up -// prints it. -func upConfigurationJSON(t *testing.T, opts *upOpts, cfg *config.DevContainer, ctr container.InspectResponse, imageLabels map[string]string) map[string]any { +// prints it, for a container whose metadata labels are containerLabels. +func upConfigurationJSON(t *testing.T, opts *upOpts, cfg *config.DevContainer, ctr container.InspectResponse, containerLabels map[string]string) map[string]any { t.Helper() result := map[string]any{} - if err := addUpConfiguration(result, opts, cfg, ctr, func(string) map[string]string { return imageLabels }, quietLogger()); err != nil { + if err := addUpConfiguration(result, opts, cfg, ctr, containerMetadata(containerLabels), quietLogger()); err != nil { t.Fatal(err) } b, err := json.Marshal(result) @@ -209,7 +217,7 @@ func TestAddUpConfigurationAddsOnlyWhatWasAskedFor(t *testing.T) { func TestAddUpConfigurationReportsAContainerEnvReferenceWithoutAName(t *testing.T) { cfg := &config.DevContainer{RemoteEnv: map[string]*string{"X": strPtr("${containerEnv}")}} - labels := func(string) map[string]string { return nil } + labels := containerMetadata(nil) err := addUpConfiguration(map[string]any{}, &upOpts{includeConfig: true}, cfg, container.InspectResponse{}, labels, quietLogger()) if err == nil || !strings.HasPrefix(err.Error(), "resolve configuration container variables: ") { t.Errorf("configuration error = %v, want the substitution error", err) @@ -453,37 +461,6 @@ func TestHookRemoteEnvOrdersLaterSourcesLast(t *testing.T) { } } -func TestAppendConfigHooksAddsTheConfigsCommandsAfterTheImages(t *testing.T) { - merged := imagemeta.MergeConfiguration([]imagemeta.Entry{{OnCreateCommand: "img-oc", PostStartCommand: "img-ps"}}) - var cfg config.DevContainer - if err := json.Unmarshal([]byte(`{ - "onCreateCommand": "oc", - "updateContentCommand": ["uc", "arg"], - "postCreateCommand": {"a": "pc"}, - "postAttachCommand": "pa" - }`), &cfg); err != nil { - t.Fatal(err) - } - - appendConfigHooks(merged, &cfg) - - for _, tc := range []struct { - name string - got []any - want []any - }{ - {"onCreate", merged.OnCreateCommands, []any{"img-oc", "oc"}}, - {"updateContent", merged.UpdateContentCommands, []any{[]any{"uc", "arg"}}}, - {"postCreate", merged.PostCreateCommands, []any{map[string]any{"a": "pc"}}}, - {"postStart", merged.PostStartCommands, []any{"img-ps"}}, - {"postAttach", merged.PostAttachCommands, []any{"pa"}}, - } { - if !reflect.DeepEqual(tc.got, tc.want) { - t.Errorf("%s = %v, want %v", tc.name, tc.got, tc.want) - } - } -} - func TestDropRanHooksAsksEveryMarkerAndDropsWhatRan(t *testing.T) { inspect := container.InspectResponse{Created: "created", State: &container.State{StartedAt: "started"}} allHooks := func() *imagemeta.MergedConfig { @@ -659,3 +636,13 @@ func TestAddUpConfigurationDropsAConfigFilePathThatResolvesToNothing(t *testing. t.Errorf("mergedConfiguration.configFilePath = %v, want it left out", v) } } + +// up's Features build writes the configuration's containerEnv into the image +// as ENV lines, as TS getFeaturesBuildOptions does and build already did. +func TestUpFeatureBuildOptionsCarryTheContainerEnv(t *testing.T) { + cfg := &config.DevContainer{ContainerEnv: map[string]string{"VAR_WITH_SPACES": "value with spaces"}} + got := (&upRunner{opts: &upOpts{}}).featureBuildOptions(cfg, &config.LoadResult{}, "").ContainerEnv + if !reflect.DeepEqual(got, cfg.ContainerEnv) { + t.Errorf("ContainerEnv = %v, want the config's %v", got, cfg.ContainerEnv) + } +} 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/loader.go b/internal/config/loader.go index 6612966..3b53c76 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -588,6 +588,13 @@ func detectGitRootUntil(dir, stopAt string) string { } } +// EnvHostSubContext is the host substitution context without a configuration +// file: the platform and this process's environment, which TS substitutes a +// command's empty configuration with ({ platform, env }). +func EnvHostSubContext() HostSubContext { + return HostSubContext{Platform: currentPlatform(), Env: EnvMap(os.Environ())} +} + func currentPlatform() string { if IsWindows() { return "win32" 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..fb3eafb 100644 --- a/internal/config/varsub_test.go +++ b/internal/config/varsub_test.go @@ -2,7 +2,7 @@ package config import ( "errors" - "reflect" + "fmt" "strings" "testing" ) @@ -35,18 +35,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 +49,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 +88,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 } @@ -288,3 +119,30 @@ func TestSubstituteTags(t *testing.T) { t.Errorf("error prop = %v", err) } } + +// The id is the hash in base 32 padded to 52 digits, also when the hash's +// leading digits are zeros. +func TestComputeDevContainerIDIsPaddedTo52Digits(t *testing.T) { + padded := false + for i := 0; i < 200; i++ { + labels := map[string]string{"devcontainer.local_folder": fmt.Sprintf("/ws/%d", i)} + id := ComputeDevContainerID(labels) + if len(id) != 52 { + t.Fatalf("id for %v = %q, %d digits", labels, id, len(id)) + } + padded = padded || id[0] == '0' + } + if !padded { + t.Error("no id needed padding: widen the search") + } +} + +// Only a letter followed by a colon is a drive (node path.win32.basename): +// '{' lowercases past 'z', so `{:foo` keeps its colon. +func TestWinBasenameDrive(t *testing.T) { + for in, want := range map[string]string{`C:foo`: "foo", `c:\a\b\`: "b", `1:foo`: "1:foo", `C:`: "", `{:foo`: "{:foo", "@:foo": "@:foo"} { + if got := winBasename(in); got != want { + t.Errorf("winBasename(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/internal/devtools/command.go b/internal/devtools/command.go index 05b3cd2..c37d4c1 100644 --- a/internal/devtools/command.go +++ b/internal/devtools/command.go @@ -2,6 +2,7 @@ package devtools import ( "bufio" + "bytes" "encoding/json" "fmt" "io" @@ -47,6 +48,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(".", false) default: fmt.Fprintf(stderr, "devtool: unknown command %q\n", cmd) return 2 @@ -547,3 +550,69 @@ 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 under root +// with the answers the reference's own functions give to the golden's +// requests; with check it writes nothing and fails on a stale golden. +// +// mutate-lane: TestTSOracleGoldensMatchTheReference +func (t tool) tsOracle(root string, check bool) 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", filepath.Join(root, "tools/tsoracle/build.mjs"), filepath.Join(root, "reference"), bundle); err != nil { + return fmt.Errorf("bundle tools/tsoracle/oracle.ts: %w", err) + } + return refreshTSOracleFiles(root, func(request []byte) ([]byte, error) { + cmd := exec.Command("node", bundle) + cmd.Stdin = bytes.NewReader(request) + cmd.Stderr = t.errw + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("run the oracle: %w", err) + } + return out, nil + }, check, t.errw) +} + +// refreshTSOracleFiles asks oracle for every golden under root and rewrites +// the ones whose answers changed, naming each on errw; with check it writes +// nothing and fails naming the stale ones. +func refreshTSOracleFiles(root string, oracle func(request []byte) ([]byte, error), check bool, errw io.Writer) error { + paths, err := TSOracleGoldens(os.DirFS(root)) + if err != nil { + return err + } + goldens := map[string][]byte{} + for _, p := range paths { + if goldens[p], err = os.ReadFile(filepath.Join(root, p)); err != nil { + return err + } + } + refreshed, err := RefreshTSOracleGoldens(goldens, oracle) + if err != nil { + return err + } + var stale []string + for _, p := range paths { + if bytes.Equal(refreshed[p], goldens[p]) { + continue + } + stale = append(stale, p) + if check { + continue + } + if err := os.WriteFile(filepath.Join(root, p), refreshed[p], 0o644); err != nil { + return err + } + fmt.Fprintln(errw, "updated", p) + } + if check && len(stale) > 0 { + return fmt.Errorf("stale ts-oracle goldens, run `go tool task testdata:ts-oracle`: %s", strings.Join(stale, ", ")) + } + fmt.Fprintf(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..f157b1e --- /dev/null +++ b/internal/devtools/tsoracle_test.go @@ -0,0 +1,232 @@ +package devtools + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "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) + } + }) + } +} + +// With no cases the oracle is still asked with a cases array, which the +// tools/tsoracle protocol requires, not with null. +func TestRefreshTSOracleGoldensAsksWithAnEmptyCasesArray(t *testing.T) { + var asked string + _, err := RefreshTSOracleGoldens(map[string][]byte{"g.json": []byte(`{"cases":[]}`)}, func(request []byte) ([]byte, error) { + asked = string(request) + return []byte(`{"cases":[]}`), nil + }) + if err != nil { + t.Fatal(err) + } + if asked != `{"cases":[]}` { + t.Errorf("request = %s, want an empty cases array", asked) + } +} + +// refreshTSOracleFiles rewrites only the goldens whose answers changed and +// names them; with check it writes nothing and fails naming the stale ones. +func TestRefreshTSOracleFiles(t *testing.T) { + root := t.TempDir() + write := func(p, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(filepath.Join(root, p)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, p), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + const fresh = "{\n \"cases\": [\n {\n \"fn\": \"len\",\n \"args\": [\n \"ab\"\n ],\n \"result\": 2\n }\n ]\n}\n" + const stale = `{"cases":[{"fn":"len","args":["abc"]}]}` + write("internal/a/testdata/ts-oracle/fresh.json", fresh) + write("internal/b/testdata/ts-oracle/stale.json", stale) + var sent []byte + + var errw strings.Builder + err := refreshTSOracleFiles(root, fakeOracle(&sent), true, &errw) + if err == nil || !strings.Contains(err.Error(), "internal/b/testdata/ts-oracle/stale.json") || strings.Contains(err.Error(), "fresh.json") { + t.Fatalf("check = %v, want the stale golden named alone", err) + } + if got, _ := os.ReadFile(filepath.Join(root, "internal/b/testdata/ts-oracle/stale.json")); string(got) != stale { + t.Errorf("check rewrote the stale golden: %s", got) + } + + errw.Reset() + if err := refreshTSOracleFiles(root, fakeOracle(&sent), false, &errw); err != nil { + t.Fatal(err) + } + if want := "updated internal/b/testdata/ts-oracle/stale.json\n2 ts-oracle goldens checked\n"; errw.String() != want { + t.Errorf("stderr = %q, want %q", errw.String(), want) + } + if got, _ := os.ReadFile(filepath.Join(root, "internal/b/testdata/ts-oracle/stale.json")); !strings.Contains(string(got), `"result": 3`) { + t.Errorf("stale golden not refreshed: %s", got) + } + if err := refreshTSOracleFiles(root, fakeOracle(&sent), true, &errw); err != nil { + t.Errorf("check after a refresh = %v, want nil", err) + } + if err := refreshTSOracleFiles(root, func([]byte) ([]byte, error) { return nil, errors.New("node failed") }, true, &errw); err == nil || !strings.Contains(err.Error(), "node failed") { + t.Errorf("oracle failure = %v", err) + } +} + +// The recorded goldens are what the reference's functions return today. It +// runs where `task reference` has installed the reference (the parity CI job, +// through task parity:contract). +func TestTSOracleGoldensMatchTheReference(t *testing.T) { + const root = "../.." + if _, err := os.Stat(filepath.Join(root, "reference/node_modules/esbuild")); err != nil { + t.Skipf("reference not installed (%v); run `task reference`", err) + } + var out strings.Builder + if err := (tool{out: &out, errw: &out}).tsOracle(root, true); err != nil { + t.Fatalf("%v\n%s", err, out.String()) + } +} 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/argbuilders_test.go b/internal/docker/argbuilders_test.go index f18faa8..49dcfce 100644 --- a/internal/docker/argbuilders_test.go +++ b/internal/docker/argbuilders_test.go @@ -301,7 +301,7 @@ func TestComposeConfig_ArgsAndParse(t *testing.T) { want := []string{"compose", "-f", "c.yml", "--env-file", ".env", - "config", "--format", "json", + "--profile", "*", "config", } if !reflect.DeepEqual(rr.last().args, want) { t.Errorf("config args =\n %v\nwant\n %v", rr.last().args, want) diff --git a/internal/docker/compose.go b/internal/docker/compose.go index e045fb7..fdf54c2 100644 --- a/internal/docker/compose.go +++ b/internal/docker/compose.go @@ -5,10 +5,15 @@ import ( "encoding/json" "fmt" "io" + "os" osexec "os/exec" + "slices" "strconv" "strings" + "gopkg.in/yaml.v3" + + coreerrors "github.com/devcontainers/cli/internal/errors" "github.com/devcontainers/cli/internal/exec" "github.com/devcontainers/cli/internal/log" ) @@ -41,7 +46,7 @@ func NewComposeClient(dockerPath, composePath string, env []string, logger log.L return &ComposeClient{ Command: []string{dockerPath, "compose"}, Version: parseComposeVersion(strings.TrimSpace(string(out))), - Env: env, + Env: withBakeFSEntitlement(env, os.LookupEnv), Log: logger, }, nil } @@ -51,6 +56,24 @@ func NewComposeClient(dockerPath, composePath string, env []string, logger log.L return nil, fmt.Errorf("'docker compose' (v2) not found") } +// bakeFSEntitlement turns off bake's filesystem entitlement check for the +// compose processes this CLI starts. Compose builds through bake, and bake on +// buildx 0.38 and later refuses to read a Dockerfile outside the build context +// unless the caller passes --allow=fs.read, which `docker compose build` cannot; +// the Features Dockerfile this CLI writes lives in its cache folder, outside the +// context. The reference passes nothing and fails there ("additional privileges +// requested"); docs/DIVERGENCES.md records the difference. +const bakeFSEntitlement = "BUILDX_BAKE_ENTITLEMENTS_FS" + +// withBakeFSEntitlement is env plus BUILDX_BAKE_ENTITLEMENTS_FS=0, unless env or +// the process environment (lookup) already sets it. +func withBakeFSEntitlement(env []string, lookup func(string) (string, bool)) []string { + if _, ok := lookup(bakeFSEntitlement); ok || slices.ContainsFunc(env, func(kv string) bool { return strings.HasPrefix(kv, bakeFSEntitlement+"=") }) { + return env + } + return append(slices.Clone(env), bakeFSEntitlement+"=0") +} + // Run executes a compose command and captures output (no live streaming). func (c *ComposeClient) Run(ctx context.Context, args ...string) (*ExecResult, error) { return c.run(ctx, false, args...) @@ -90,26 +113,83 @@ func (c *ComposeClient) run(ctx context.Context, stream bool, args ...string) (* }, nil } -// Config runs `docker compose config` and returns parsed YAML. +// Config runs `docker compose config` with every profile and returns the +// parsed project, as TS readDockerComposeConfig does (it reads the YAML output, +// a missing document being an empty project). A failure is TS's ContainerError: +// "An error occurred retrieving the Docker Compose configuration.", the +// message naming the command that failed. Compose's own output is logged. func (c *ComposeClient) Config(ctx context.Context, composeFiles []string, envFile string) (map[string]any, error) { args := c.buildGlobalArgs(composeFiles, envFile) - args = append(args, "config", "--format", "json") + args = append(args, "--profile", "*", "config") res, err := c.Run(ctx, args...) - if err != nil { - return nil, fmt.Errorf("compose config: %w", err) + if err == nil && res.ExitCode != 0 { + c.Log.Write(string(res.Stdout)+string(res.Stderr), log.LevelError) + err = &CommandFailedError{Command: strings.Join(append(slices.Clone(c.Command), args...), " "), ExitCode: res.ExitCode} } - if res.ExitCode != 0 { - return nil, fmt.Errorf("compose config failed (exit %d): %s", res.ExitCode, string(res.Stderr)) + var config map[string]any + if err == nil { + config, err = parseComposeConfig(res.Stdout) + } + if err != nil { + return nil, &coreerrors.ContainerError{Description: "An error occurred retrieving the Docker Compose configuration.", OriginalError: err} } + return config, nil +} - var config map[string]any - if err := json.Unmarshal(res.Stdout, &config); err != nil { +// parseComposeConfig reads `docker compose config`'s YAML into the JSON value +// types (float64 numbers, map[string]any objects) the callers read. +func parseComposeConfig(out []byte) (map[string]any, error) { + var doc any + if err := yaml.Unmarshal(out, &doc); err != nil { + return nil, fmt.Errorf("parse compose config: %w", err) + } + data, err := json.Marshal(stringKeys(doc)) + if err != nil { return nil, fmt.Errorf("parse compose config: %w", err) } + config := map[string]any{} + if doc != nil { + if err := json.Unmarshal(data, &config); err != nil { + return nil, fmt.Errorf("parse compose config: %w", err) + } + } return config, nil } +// stringKeys turns the map[any]any YAML gives a mapping with a non-string key +// into a map[string]any, which JSON can represent. +func stringKeys(v any) any { + switch v := v.(type) { + case map[string]any: + for k, e := range v { + v[k] = stringKeys(e) + } + case map[any]any: + m := make(map[string]any, len(v)) + for k, e := range v { + m[fmt.Sprint(k)] = stringKeys(e) + } + return m + case []any: + for i, e := range v { + v[i] = stringKeys(e) + } + } + return v +} + +// CommandFailedError is a command that exited non-zero; its message is TS +// runCommandNoPty's, "Command failed: ". +type CommandFailedError struct { + Command string + ExitCode int +} + +func (e *CommandFailedError) Error() string { + return "Command failed: " + e.Command +} + // Build runs `docker compose build`. func (c *ComposeClient) Build(ctx context.Context, composeFiles []string, envFile string, globalArgs, services []string, noCache bool) error { args := c.buildGlobalArgs(composeFiles, envFile) diff --git a/internal/docker/compose_test.go b/internal/docker/compose_test.go index 9a9aac4..731abcc 100644 --- a/internal/docker/compose_test.go +++ b/internal/docker/compose_test.go @@ -1,6 +1,7 @@ package docker import ( + "slices" "testing" ) @@ -135,3 +136,24 @@ func TestBuildGlobalArgs_NoEnvFile(t *testing.T) { t.Errorf("args = %v", args) } } + +// The compose processes get BUILDX_BAKE_ENTITLEMENTS_FS=0 so bake reads the +// Features Dockerfile outside the build context, unless the user set it. +func TestWithBakeFSEntitlement(t *testing.T) { + unset := func(string) (string, bool) { return "", false } + if got := withBakeFSEntitlement([]string{"A=1"}, unset); !slices.Equal(got, []string{"A=1", "BUILDX_BAKE_ENTITLEMENTS_FS=0"}) { + t.Errorf("unset: %q", got) + } + if got := withBakeFSEntitlement(nil, func(k string) (string, bool) { return "1", k == "BUILDX_BAKE_ENTITLEMENTS_FS" }); len(got) != 0 { + t.Errorf("set in the process: %q, want it left alone", got) + } + if got := withBakeFSEntitlement([]string{"BUILDX_BAKE_ENTITLEMENTS_FS=1"}, unset); !slices.Equal(got, []string{"BUILDX_BAKE_ENTITLEMENTS_FS=1"}) { + t.Errorf("set in env: %q", got) + } + env := make([]string, 1, 4) + env[0] = "A=1" + withBakeFSEntitlement(env, unset) + if env[:2][1] != "" { + t.Error("the caller's env was written to") + } +} diff --git a/internal/docker/dockerfile.go b/internal/docker/dockerfile.go index 779b76c..8aa1d05 100644 --- a/internal/docker/dockerfile.go +++ b/internal/docker/dockerfile.go @@ -1,13 +1,12 @@ package docker import ( - "maps" - "slices" + "errors" + "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 @@ -16,7 +15,6 @@ type Dockerfile struct { Preamble Preamble Stages []Stage StagesByLabel map[string]*Stage - escapeToken rune } // Preamble is the content before any FROM statement. @@ -54,14 +52,12 @@ func ExtractDockerfile(content string) *Dockerfile { df := &Dockerfile{ Preamble: Preamble{Directives: map[string]string{}}, StagesByLabel: map[string]*Stage{}, - escapeToken: '\\', } res, err := parser.Parse(strings.NewReader(content)) if err != nil { return df } - df.escapeToken = res.EscapeToken if syntax, _, _, ok := parser.DetectSyntax([]byte(content)); ok { df.Preamble.Directives["syntax"] = syntax df.Preamble.Version = syntaxVersion(syntax) @@ -82,7 +78,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 +107,17 @@ 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. On +// a FROM line BuildKit accepts (continuations joined), the FROM-line +// expression reads the label BuildKit names. +func stageLabel(s *instructions.Stage) string { + if m := fromLineLabel.FindStringSubmatch(s.SourceCode); m != nil { + 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,71 @@ 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. -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 "" +// FindBaseImage is TS findBaseImage: the image the target stage (or the last +// one) is built from, with the --build-arg values and the automatic BUILD*/ +// TARGET* platform ARGs (platformArgs, TS globalBuildxPlatformArgs), 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, platformArgs map[string]string) string { + return NewResolver(df, buildArgs, platformArgs).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+(?