diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index 4f9d049..01bc17d 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -157,14 +157,14 @@ upstream issue it fixes. in the build context (upstream #969). This CLI copies the ignore file next to every Dockerfile it generates (`TestCopyDockerignore`, `TestBuildAndExtendComposeDockerfileService` in `internal/cli/compose_up_held_test.go`, - and `TestE2E_SiblingDockerignore` in `internal/cli/e2e_fixes_test.go`). + `TestBuildDockerfileKeepsTheSiblingDockerignore`, and `TestE2E_SiblingDockerignore` in `internal/cli/e2e_fixes_test.go`). - **A `LABEL devcontainer.metadata` in the Dockerfile is kept.** TS builds the image's metadata label from the base image's label and the configuration and writes it over the image, so a `LABEL devcontainer.metadata=…` written in the built stage of the user's Dockerfile is lost (upstream #1225). This CLI merges that stage's entries between the base image's and the configuration's (`TestStageLabelsDevcontainerMetadata` and - `TestStageLabelsReadsTheNamedStage` in `internal/docker`, `TestDockerfileImageMetadata` - in `internal/cli/build_held_test.go`). + `TestStageLabelsReadsTheNamedStage` in `internal/docker`, `TestBuildDockerfileMetadataLabel` + in `internal/cli/dockerfile_build_held_test.go`). - **`build --label` applies to a Docker Compose configuration.** `docker compose build` takes no labels, and TS drops `--label` for a Compose configuration without a word (upstream #930). This CLI applies the labels with one more build, `FROM `, tagged diff --git a/docs/migration.md b/docs/migration.md index c1ac1aa..9417054 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -101,6 +101,16 @@ runs the official CLI is unaffected: matching container. - Features are staged under `/devcontainercli-/container-features/`, one folder per build, and that folder is kept as the official CLI keeps it. +- A Dockerfile configuration's image is made by one `docker build`, as the + official CLI makes it: the Dockerfile followed by the stage that installs the + Features and writes the `devcontainer.metadata` label, written as + `Dockerfile-with-features` into that Features folder and built with + `--target dev_containers_target_stage`. Earlier releases built the Dockerfile + first, wrote `.devcontainer.build` next to it when its last stage had + no name (failing in a read-only workspace), and added the Features in a second + build. `up` of a Dockerfile configuration with Features now tags the image + `vsc--`, not `vsc---features`; a script that looked + the image up by the old name should use the new one. - When the workspace is a Git worktree and the configuration sets both `workspaceMount` and `workspaceFolder`, `up` no longer mounts the main repository's Git common directory, and `read-configuration` no longer reports diff --git a/docs/parity/parity-matrix.yaml b/docs/parity/parity-matrix.yaml index 312ffd2..72833cf 100644 --- a/docs/parity/parity-matrix.yaml +++ b/docs/parity/parity-matrix.yaml @@ -1779,6 +1779,34 @@ initial_cases: current_status: match notes: "Without build.context the context is the Dockerfile's folder (utils.ts getDockerContextPath: parentURI(getDockerfilePath)), not the configuration's: COPY finds the file next to sub/Dockerfile. Go used the configuration's folder and the COPY failed." + - id: build.read-only-workspace-dockerfile-success + lane: runtime + command: build + priority: p1 + docker_required: true + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; chmod -R u+w $WORKSPACE 2>/dev/null; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s\\n' 'FROM alpine:3' 'RUN echo ok > /marker' > $WORKSPACE/.devcontainer/Dockerfile; printf '%s' '{\"build\":{\"dockerfile\":\"Dockerfile\"},\"remoteUser\":\"root\"}' > $WORKSPACE/.devcontainer/devcontainer.json; chmod -R a-w $WORKSPACE; echo WORKSPACE=$WORKSPACE" + ts_cmd: "build --workspace-folder ${WORKSPACE} --image-name parity-${PARITY_CASE_ID}-${PARITY_SIDE}" + verify_cmd: "cd ${WORKSPACE} && find . | sort; docker history --no-trunc --format '{{.CreatedBy}}' ${IMAGE_NAME}" + cleanup_cmd: "docker rmi -f parity-${PARITY_CASE_ID}-${PARITY_SIDE} >/dev/null 2>&1; chmod -R u+w /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE} 2>/dev/null; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true" + asserts: [exit_code, stdout_normalized] + class: build-runtime + current_status: match + notes: "TS buildAndExtendImage (singleContainer.ts) builds a Dockerfile configuration in one docker build of Dockerfile-with-features, written into the Features folder under the cache folder: the user's Dockerfile with its final stage named, then FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage with the devcontainer.metadata LABEL, built with --target dev_containers_target_stage. A read-only workspace builds, nothing is written into it, and the image history ends with that LABEL instruction. Go wrote .devcontainer.build next to the Dockerfile (failing here with permission denied), built it with --target and --label devcontainer.metadata=..., and added Features in a second build." + + - id: up.read-only-workspace-dockerfile-success + lane: runtime + command: up + priority: p1 + docker_required: true + setup_cmd: "WORKSPACE=/tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; chmod -R u+w $WORKSPACE 2>/dev/null; rm -rf $WORKSPACE; mkdir -p $WORKSPACE/.devcontainer; printf '%s\\n' 'FROM alpine:3' 'RUN echo ok > /marker' > $WORKSPACE/.devcontainer/Dockerfile; printf '%s' '{\"build\":{\"dockerfile\":\"Dockerfile\"},\"remoteUser\":\"root\"}' > $WORKSPACE/.devcontainer/devcontainer.json; chmod -R a-w $WORKSPACE; echo WORKSPACE=$WORKSPACE" + ts_cmd: "up --workspace-folder ${WORKSPACE} --id-label parity.case=${PARITY_CASE_ID}-${PARITY_SIDE}" + verify_cmd: "cd ${WORKSPACE} && find . | sort; docker exec ${CONTAINER_ID} cat /marker; I=$(docker inspect -f '{{.Image}}' ${CONTAINER_ID}); docker history --no-trunc --format '{{.CreatedBy}}' $I" + cleanup_cmd: "I=$(docker inspect -f '{{.Image}}' ${CONTAINER_ID} 2>/dev/null); docker rm -f ${CONTAINER_ID} >/dev/null 2>&1; [ -n \"$I\" ] && docker rmi -f $I >/dev/null 2>&1; chmod -R u+w /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE} 2>/dev/null; rm -rf /tmp/parity-${PARITY_CASE_ID}-${PARITY_SIDE}; true" + asserts: [exit_code, stdout_normalized] + class: up-runtime + current_status: match + notes: "As build.read-only-workspace-dockerfile-success, for up: one build of Dockerfile-with-features from the cache folder, so a read-only workspace starts and holds only what it held. Go failed writing .devcontainer.build into the workspace." + - id: upgrade.write-lockfile-to-disk-success lane: runtime command: upgrade diff --git a/internal/cli/build.go b/internal/cli/build.go index 04e3b1e..ff88b05 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -44,6 +44,7 @@ type buildOpts struct { secretsFile string secrets fileSecrets caBundle string + omitSyntaxDirective bool lockfileExcludeIDs map[string]bool } @@ -92,7 +93,7 @@ func newBuildCmd() *cobra.Command { f.BoolVar(&opts.frozenLockfile, "frozen-lockfile", false, "Ensure lockfile exists and remains unchanged; fail otherwise.") f.StringVar(&opts.secretsFile, "secrets-file", "", "Path to a JSON file with build secrets ({\"KEY\":\"VALUE\"}); each is passed to BuildKit as a build secret (requires buildx).") f.StringVar(&opts.caBundle, "ca-bundle", "", caBundleFlagUsage) - f.Bool("omit-syntax-directive", false, "") + f.BoolVar(&opts.omitSyntaxDirective, "omit-syntax-directive", false, "") cmd.Flags().MarkHidden("skip-feature-auto-mapping") cmd.Flags().MarkHidden("skip-persisting-customizations-from-features") cmd.Flags().MarkHidden("experimental-lockfile") @@ -267,15 +268,6 @@ func buildFlagConflict(opts *buildOpts, useBuildx, compose bool) string { return "" } -type dockerfileBuildPrep struct { - Path string - StageName string - Parsed *docker.Dockerfile - cleanup func() -} - -func (p *dockerfileBuildPrep) Cleanup() { p.cleanup() } - // copyDockerignore copies a Dockerfile's sibling `.dockerignore` next to a // renamed/generated build Dockerfile, so BuildKit's per-Dockerfile ignore lookup // (`.dockerignore`) still finds the user's rules instead of leaking @@ -307,132 +299,21 @@ func dockerContextPath(cfg *config.DevContainer) string { return filepath.Dir(dockerfileConfigPath(cfg)) } -func prepareDockerfileBuild(cfg *config.DevContainer) (*dockerfileBuildPrep, error) { - dockerfilePath := cfg.Dockerfile() - if dockerfilePath == "" { - return nil, fmt.Errorf("no Dockerfile specified") - } - dockerfilePath = dockerfileConfigPath(cfg) - - content, err := os.ReadFile(dockerfilePath) - if err != nil { - return nil, fmt.Errorf("read Dockerfile: %w", err) - } - - prep := &dockerfileBuildPrep{ - Parsed: docker.ExtractDockerfile(string(content)), - cleanup: func() {}, - } - - if cfg.Build != nil && cfg.Build.Target != "" { - prep.StageName = cfg.Build.Target - } else { - 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" - // mutate-exempt: only the docker build this process starts reads the file, as its owner, and Cleanup removes it - if err := os.WriteFile(tmpDockerfile, []byte(modified), 0644); err != nil { - return nil, fmt.Errorf("write build dockerfile: %w", err) - } - copyDockerignore(dockerfilePath, tmpDockerfile) - prep.cleanup = func() { - os.Remove(tmpDockerfile) - os.Remove(tmpDockerfile + ".dockerignore") - } - dockerfilePath = tmpDockerfile - } - } - - prep.Path = dockerfilePath - return prep, nil -} - -// mutate-lane: TestParityMatrix (build.dockerfile-multi-image-success, -// build.subfolder-config-success, build.apply-build-options-success, -// build.output-oci-success, build.cache-to-local-success, -// build.platform-supported-dockerfile-success), also run by +// mutate-lane: TestBuildDockerfileBuildsOnce and TestParityMatrix +// (build.dockerfile-multi-image-success, build.subfolder-config-success, +// build.apply-build-options-success, build.output-oci-success, +// build.cache-to-local-success, build.platform-supported-dockerfile-success, +// build.read-only-workspace-dockerfile-success), also run by // TestE2E_BuildUpExec_Dockerfile, TestE2E_SiblingDockerignore and // TestE2E_CABundle (an opted-in RUN reads the bundle). func (r *buildRunner) buildDockerfile(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 - - prep, err := prepareDockerfileBuild(cfg) - if err != nil { - return nil, err - } - defer prep.Cleanup() - dockerfilePath := prep.Path - stageName := prep.StageName - - 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) - - baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage) - baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger) - stageMetadata := imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger) - + opts := r.opts + fb := r.featureBuildOptions(cfg, &loadResult.HostSub) + fb.Secrets = opts.secrets.env() imageNames := imageNamesOr(opts.imageNames, folderImageName(resolvePath(opts.workspaceFolder))) - - contextPath := dockerContextPath(cfg) - - buildArgs := buildArgsFromConfig(cfg) - - allLabels := append(slices.Clone(opts.labels), imageLabelArgs(dockerfileImageMetadata(cfg, baseMetadata, stageMetadata, r.configLabel))...) - - baseOutput, basePush := dockerfileBaseExport(opts, cfg) - - cacheFrom := cacheFromForDockerfileBuild(opts.cacheFrom, cfg) - authEnv, authCleanup := bridgeBuildAuth(logger, baseImage, imageNames, cacheFrom, opts.cacheTo) - defer authCleanup() - - buildSecrets := buildxSecrets(opts.secrets.env(), useBuildx, logger) - caSecrets := caBundleBuildSecrets(opts.caBundle, useBuildx, logger) - - buildOpts := docker.BuildOptions{ - Dockerfile: dockerfilePath, - ContextPath: contextPath, - Tags: imageNames, - Target: stageName, - BuildArgs: buildArgs, - CacheFrom: cacheFrom, - Labels: allLabels, - NoCache: opts.noCache, - ExtraArgs: buildOptionsFromConfig(cfg), - UseBuildx: useBuildx, - Platform: opts.platform, - Push: basePush, - Output: baseOutput, - CacheTo: opts.cacheTo, - Env: authEnv, - Secrets: buildSecrets, - FileSecrets: caSecrets, - } - - result, err := dockerClient.Build(ctx, buildOpts) - if err != nil { + if err := buildDockerfileImage(ctx, r.log, r.docker, r.engine, cfg, useBuildx, imageNames, opts.platform, fb); err != nil { return nil, err } - if result.ExitCode != 0 { - return nil, fmt.Errorf(msgDockerBuildFailed, result.ExitCode, string(result.Stderr)) - } - - if dockerClient.ProgressWriter == nil { - logger.Write(string(result.Stderr), log.LevelInfo) - } - - if len(cfg.Features) > 0 { - baseImageName := imageNames[0] - return extendImageWithFeatures(ctx, logger, dockerClient, engine, baseImageName, cfg.Features, useBuildx, imageNames, r.featureBuildOptions(cfg, &loadResult.HostSub)) - } - return imageNames, nil } @@ -593,26 +474,6 @@ func imageNamesOr(names []string, fallback string) []string { return names } -// 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 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, configLabel) - } - return metadata -} - -func dockerfileBaseExport(opts *buildOpts, cfg *config.DevContainer) (output string, push bool) { - if len(cfg.Features) > 0 { - return "", false - } - return opts.output, opts.push -} - func buildxSecrets(secrets []string, useBuildx bool, logger log.Logger) []string { if useBuildx { return secrets @@ -746,6 +607,7 @@ func (r *buildRunner) buildCompose(ctx context.Context, cfg *config.DevContainer noLockfile: opts.noLockfile, lockfileExcludeIDs: opts.lockfileExcludeIDs, caBundle: opts.caBundle, + omitSyntaxDirective: opts.omitSyntaxDirective, }, substitute: hostMetadataSubstitution(loadResult.HostSub, nil), configLabel: r.configLabel, @@ -840,5 +702,6 @@ func (r *buildRunner) featureBuildOptions(cfg *config.DevContainer, hostSub *con WorkspaceFolder: opts.workspaceFolder, AdditionalMetadata: []imagemeta.Entry{configToMetadataEntry(cfg)}, LabelMetadata: []imagemeta.Entry{r.configLabel}, + OmitSyntaxDirective: opts.omitSyntaxDirective, } } diff --git a/internal/cli/build_held_test.go b/internal/cli/build_held_test.go index 225dcae..9337ae7 100644 --- a/internal/cli/build_held_test.go +++ b/internal/cli/build_held_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/devcontainers/cli/internal/config" - "github.com/devcontainers/cli/internal/imagemeta" "github.com/devcontainers/cli/internal/log" ) @@ -132,40 +131,6 @@ func TestImageNamesOr(t *testing.T) { } } -func TestDockerfileImageMetadata(t *testing.T) { - base := []imagemeta.Entry{{ID: "base"}} - stage := []imagemeta.Entry{{ID: "stage"}} - ids := func(entries []imagemeta.Entry) []string { - var out []string - for _, e := range entries { - out = append(out, e.ID+"/"+e.RemoteUser) - } - return out - } - - cfg := &config.DevContainer{RemoteUser: "dev"} - 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, configLabel)), []string{"base/", "stage/"}; !slices.Equal(got, want) { - t.Errorf("with features (the Feature build adds the config entry): %v, want %v", got, want) - } -} - -func TestDockerfileBaseExport(t *testing.T) { - opts := &buildOpts{output: "type=oci,dest=x.tar", push: true} - if output, push := dockerfileBaseExport(opts, &config.DevContainer{}); output != opts.output || !push { - t.Errorf("without features: (%q, %v), want the flags", output, push) - } - withFeature := &config.DevContainer{Features: map[string]any{"ghcr.io/devcontainers/features/go:1": true}} - if output, push := dockerfileBaseExport(opts, withFeature); output != "" || push { - t.Errorf("with features: (%q, %v), want them left to the Feature build", output, push) - } -} - func TestBuildxSecrets(t *testing.T) { secrets := []string{"TOKEN=x"} var logged bytes.Buffer @@ -222,33 +187,6 @@ func TestRunBuildBuildKitNeverIgnoresBuildx(t *testing.T) { } } -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) - } -} - func TestBuildResultImageName(t *testing.T) { for _, tc := range []struct { name string diff --git a/internal/cli/build_repo2_held_test.go b/internal/cli/build_repo2_held_test.go deleted file mode 100644 index 0491a50..0000000 --- a/internal/cli/build_repo2_held_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package cli - -import ( - "errors" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/devcontainers/cli/internal/config" -) - -func TestPrepareDockerfileBuildCleanupRemovesTheWrittenFiles(t *testing.T) { - dir := t.TempDir() - dockerfile := filepath.Join(dir, "Dockerfile") - if err := os.WriteFile(dockerfile, []byte("FROM alpine\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(dockerfile+".dockerignore", []byte("secret\n"), 0o644); err != nil { - t.Fatal(err) - } - prep, err := prepareDockerfileBuild(&config.DevContainer{ - ConfigFilePath: filepath.Join(dir, "devcontainer.json"), - Build: &config.Build{Dockerfile: "Dockerfile"}, - }) - if err != nil { - t.Fatal(err) - } - written, err := os.ReadFile(prep.Path) - if err != nil || !strings.Contains(string(written), "AS dev_container_auto_added_stage_label") { - t.Fatalf("written Dockerfile = %q, %v; want the final stage named", written, err) - } - if ignore, err := os.ReadFile(prep.Path + ".dockerignore"); err != nil || string(ignore) != "secret\n" { - t.Fatalf("written .dockerignore = %q, %v; want the user's copied", ignore, err) - } - prep.Cleanup() - for _, p := range []string{prep.Path, prep.Path + ".dockerignore"} { - if _, err := os.Stat(p); !errors.Is(err, os.ErrNotExist) { - t.Errorf("%s after Cleanup: %v, want it removed", p, err) - } - } - for _, p := range []string{dockerfile, dockerfile + ".dockerignore"} { - if _, err := os.Stat(p); err != nil { - t.Errorf("user's %s after Cleanup: %v", p, err) - } - } -} diff --git a/internal/cli/build_repo_held_test.go b/internal/cli/build_repo_held_test.go index 72d0335..37117e6 100644 --- a/internal/cli/build_repo_held_test.go +++ b/internal/cli/build_repo_held_test.go @@ -3,11 +3,8 @@ package cli import ( "bytes" "encoding/json" - "os" "path/filepath" "testing" - - "github.com/devcontainers/cli/internal/config" ) func buildConfigNotFound(t *testing.T, args ...string) string { @@ -40,48 +37,3 @@ func TestBuildPositionalPathIsTheWorkspace(t *testing.T) { t.Errorf("message with --workspace-folder = %q, want %q", got, want) } } - -func TestPrepareDockerfileBuildUsesTheConfiguredTarget(t *testing.T) { - dir := t.TempDir() - dockerfile := filepath.Join(dir, "Dockerfile") - if err := os.WriteFile(dockerfile, []byte("FROM alpine AS base\nFROM base\n"), 0o644); err != nil { - t.Fatal(err) - } - cfg := &config.DevContainer{ - ConfigFilePath: filepath.Join(dir, "devcontainer.json"), - Build: &config.Build{Dockerfile: "Dockerfile", Target: "base"}, - } - prep, err := prepareDockerfileBuild(cfg) - if err != nil { - t.Fatal(err) - } - defer prep.Cleanup() - if prep.StageName != "base" || prep.Path != dockerfile { - t.Errorf("stage %q at %q, want base at %q", prep.StageName, prep.Path, dockerfile) - } -} - -func TestPrepareDockerfileBuildNamesTheFinalStageWithoutATarget(t *testing.T) { - for name, cfg := range map[string]config.DevContainer{ - "build": {Build: &config.Build{Dockerfile: "Dockerfile"}}, - "top level": {DockerFile: "Dockerfile"}, - } { - t.Run(name, func(t *testing.T) { - dir := t.TempDir() - dockerfile := filepath.Join(dir, "Dockerfile") - if err := os.WriteFile(dockerfile, []byte("FROM alpine\n"), 0o644); err != nil { - t.Fatal(err) - } - cfg.ConfigFilePath = filepath.Join(dir, "devcontainer.json") - prep, err := prepareDockerfileBuild(&cfg) - if err != nil { - t.Fatal(err) - } - defer prep.Cleanup() - const stage = "dev_container_auto_added_stage_label" - if prep.StageName != stage || prep.Path != dockerfile+".devcontainer.build" { - t.Errorf("stage %q at %q, want %s at the written Dockerfile", prep.StageName, prep.Path, stage) - } - }) - } -} diff --git a/internal/cli/compose_up.go b/internal/cli/compose_up.go index 630d738..4043991 100644 --- a/internal/cli/compose_up.go +++ b/internal/cli/compose_up.go @@ -431,16 +431,7 @@ func (r *upRunner) buildAndExtendCompose(ctx context.Context, s composeStart, pe // TS: BuildKit build contexts need Compose >= 2.17.0 (the CLI variant is // always Docker: Podman is unsupported, see docs/DIVERGENCES.md). useContexts := s.useBuildx && s.client.SupportsAdditionalContexts() - syntax := "" - switch { - case opts.omitSyntaxDirective: - case engineAtLeast23(ctx, r.engine): - syntax = userSyntax - case useContexts && !dockerfileSupportsBuildContexts(parsed): - syntax = "docker/dockerfile:1.4" - default: - syntax = userSyntax - } + syntax := featuresSyntaxDirective(ctx, r.engine, opts.omitSyntaxDirective, useContexts, parsed) if err := stageFeatureBuildFiles(featuresFolder, featureSets, containerUser, remoteUser); err != nil { return nil, err } diff --git a/internal/cli/dockerfile_build.go b/internal/cli/dockerfile_build.go new file mode 100644 index 0000000..946e8f9 --- /dev/null +++ b/internal/cli/dockerfile_build.go @@ -0,0 +1,208 @@ +package cli + +import ( + "context" + "fmt" + "os" + "path/filepath" + "slices" + + "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/docker" + "github.com/devcontainers/cli/internal/imagemeta" + "github.com/devcontainers/cli/internal/log" +) + +const autoAddedStageLabel = "dev_container_auto_added_stage_label" + +const featuresDockerfileName = "Dockerfile-with-features" + +// mutate-lane: TestBuildDockerfileBuildsOnce, TestBuildDockerfileWritesTheReferenceDockerfile, +// TestUpDockerfileBuildsOnce and TestParityMatrix (build.dockerfile-multi-image-success, +// build.read-only-workspace-dockerfile-success, up.read-only-workspace-dockerfile-success), +// also run by TestE2E_BuildUpExec_Dockerfile, TestE2E_SiblingDockerignore, +// TestE2E_UpSecretsFile and TestE2E_CABundle. +func buildDockerfileImage(ctx context.Context, logger log.Logger, dockerClient *docker.Client, engine *docker.EngineClient, cfg *config.DevContainer, useBuildx bool, imageNames []string, basePlatform string, fb *FeatureBuildOptions) error { + if cfg.Dockerfile() == "" { + return fmt.Errorf("no Dockerfile specified") + } + dockerfilePath := dockerfileConfigPath(cfg) + content, err := os.ReadFile(dockerfilePath) + if err != nil { + return fmt.Errorf("read Dockerfile: %w", err) + } + dockerfile := string(content) + target := "" + if cfg.Build != nil { + target = cfg.Build.Target + } + baseName := target + if baseName == "" { + name, modified, err := docker.EnsureFinalStageName(dockerfile, autoAddedStageLabel) + if err != nil { + return err + } + baseName = name + if modified != "" { + dockerfile = modified + } + } + + parsed := docker.ExtractDockerfile(string(content)) + configArgs := buildArgsFromConfig(cfg) + resolver := docker.NewResolver(parsed, configArgs, buildxPlatformArgs(basePlatform)) + baseImage := resolver.FindBaseImage(target) + logger.Write(fmt.Sprintf("Base image: %s", baseImage), log.LevelInfo) + + baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage) + var baseEnv map[string]string + imageUser := "" + if info, err := engine.InspectImage(ctx, baseImage); err == nil && info.Config != nil { + baseEnv, imageUser = config.EnvMap(info.Config.Env), info.Config.User + } + if user := resolver.FindUserStatement(baseEnv, target); user != "" { + imageUser = user + } + if imageUser == "" { + imageUser = "root" + } + imageEntries := slices.Concat( + imagemeta.ReadMetadataFromLabels(baseLabels, logger), + imagemeta.ReadMetadataFromLabels(parsed.StageLabels(baseName), logger), + ) + + fetched, err := fetchBuildFeatures(ctx, logger, cfg.Features, fb) + if err != nil { + return err + } + + userSyntax := parsed.Preamble.Directives["syntax"] + var info *imagemeta.ExtendImageBuildInfo + var folder string + var secretIDs []string + caSecrets := caBundleBuildSecrets(fb.CABundle, useBuildx, logger) + if fetched == nil { + if folder, err = createFeaturesFolder(); err != nil { + return err + } + syntax := userSyntax + if fb.OmitSyntaxDirective { + syntax = "" + } + info = imagemeta.GenerateComposeLabelBuild(baseName, slices.Concat(imageEntries, fb.LabelMetadata), syntax) + } else { + folder = fetched.TmpDir + var featureEntries []imagemeta.Entry + for _, fs := range fetched.FeatureSets { + featureEntries = append(featureEntries, featureMetadataEntry(fs, fb.SkipPersistCustoms)) + } + containerUser, remoteUser := findContainerUsers(slices.Concat(imageEntries, featureEntries, fb.AdditionalMetadata), "", imageUser) + if err := stageFeatureBuildFiles(folder, fetched.FeatureSets, containerUser, remoteUser); err != nil { + return err + } + if useBuildx { + secretIDs = buildSecretIDs(fb.Secrets) + } + info = imagemeta.GenerateFeaturesBuildOptions(imagemeta.FeaturesBuildParams{ + BaseName: baseName, + FeatureSets: fetched.FeatureSets, + Metadata: slices.Concat(imageEntries, featureEntries, fb.LabelMetadata), + ContainerUser: containerUser, + RemoteUser: remoteUser, + ImageUser: imageUser, + UseBuildKitContexts: useBuildx, + Syntax: featuresSyntaxDirective(ctx, engine, fb.OmitSyntaxDirective, useBuildx, parsed), + ConfigContainerEnv: fb.ContainerEnv, + SecretIDs: secretIDs, + CABundle: caSecrets != nil, + }) + if !useBuildx { + if err := buildFeatureContentImage(ctx, dockerClient, folder); err != nil { + return err + } + } + } + + if loc := composeSyntaxLineRe.FindStringIndex(dockerfile); loc != nil { + dockerfile = dockerfile[loc[1]:] + } + finalPath := filepath.Join(folder, featuresDockerfileName) + final := info.DockerfilePrefixContent + dockerfile + "\n" + info.DockerfileContent + if err := os.WriteFile(finalPath, []byte(final), 0o644); err != nil { + return fmt.Errorf("write %s: %w", featuresDockerfileName, err) + } + copyDockerignore(dockerfilePath, finalPath) + logger.Write("Generated Dockerfile:\n"+final, log.LevelTrace) + + extraArgs := slices.Clone(buildOptionsFromConfig(cfg)) + for _, k := range sortedStringKeys(info.BuildKitContexts) { + v := info.BuildKitContexts[k] + if v == "." { + v = folder + } + extraArgs = append(extraArgs, "--build-context", k+"="+v) + } + for _, k := range featureBuildArgOrder(info.BuildArgs) { + extraArgs = append(extraArgs, "--build-arg", k+"="+info.BuildArgs[k]) + } + + cacheFrom := cacheFromForDockerfileBuild(fb.CacheFrom, cfg) + authEnv, authCleanup := bridgeBuildAuth(logger, baseImage, imageNames, cacheFrom, fb.CacheTo) + defer authCleanup() + + result, err := dockerClient.Build(ctx, docker.BuildOptions{ + Dockerfile: finalPath, + ContextPath: dockerContextPath(cfg), + Tags: imageNames, + Target: info.OverrideTarget, + BuildArgs: configArgs, + CacheFrom: cacheFrom, + Labels: fb.Labels, + NoCache: fb.NoCache, + ExtraArgs: extraArgs, + UseBuildx: useBuildx, + Platform: fb.Platform, + Push: fb.Push, + Output: fb.Output, + CacheTo: fb.CacheTo, + Env: authEnv, + Secrets: buildxSecrets(fb.Secrets, useBuildx, logger), + FileSecrets: caSecrets, + }) + if err != nil { + return err + } + if result.ExitCode != 0 { + return fmt.Errorf(msgDockerBuildFailed, result.ExitCode, string(result.Stderr)) + } + if dockerClient.ProgressWriter == nil { + logger.Write(string(result.Stderr), log.LevelInfo) + } + return nil +} + +func featureBuildArgOrder(args map[string]string) []string { + var keys []string + for _, k := range []string{"_DEV_CONTAINERS_BASE_IMAGE", "_DEV_CONTAINERS_IMAGE_USER", "_DEV_CONTAINERS_FEATURE_CONTENT_SOURCE"} { + if _, ok := args[k]; ok { + keys = append(keys, k) + } + } + return keys +} + +func featuresSyntaxDirective(ctx context.Context, engine *docker.EngineClient, omit, useContexts bool, parsed *docker.Dockerfile) string { + userSyntax := "" + if parsed != nil { + userSyntax = parsed.Preamble.Directives["syntax"] + } + switch { + case omit: + return "" + case engineAtLeast23(ctx, engine): + return userSyntax + case useContexts && !dockerfileSupportsBuildContexts(parsed): + return "docker/dockerfile:1.4" + } + return userSyntax +} diff --git a/internal/cli/dockerfile_build_held_test.go b/internal/cli/dockerfile_build_held_test.go new file mode 100644 index 0000000..dbba53a --- /dev/null +++ b/internal/cli/dockerfile_build_held_test.go @@ -0,0 +1,562 @@ +package cli + +import ( + "errors" + "os" + "path/filepath" + "reflect" + "slices" + "strings" + "testing" + + dockerspec "github.com/moby/docker-image-spec/specs-go/v1" + + "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/docker" + "github.com/devcontainers/cli/internal/imagemeta" + "github.com/devcontainers/cli/internal/log" +) + +func isImageBuild(c dockerCall) bool { + return len(c) > 0 && c[0] == "build" || len(c) > 1 && c[0] == "buildx" && c[1] == "build" +} + +func imageBuilds(calls []dockerCall) []dockerCall { + var builds []dockerCall + for _, c := range calls { + if isImageBuild(c) { + builds = append(builds, c) + } + } + return builds +} + +func argAfter(c dockerCall, flag string) string { + if i := slices.Index(c, flag); i >= 0 && i+1 < len(c) { + return c[i+1] + } + return "" +} + +func readOnlyWorkspace(t *testing.T, files map[string]string) (string, []string) { + t.Helper() + if os.Geteuid() == 0 { + t.Skip("root writes into read-only folders") + } + ws := writeFixture(t, files) + var dirs []string + listing := workspaceListing(t, ws) + if err := filepath.WalkDir(ws, func(p string, d os.DirEntry, err error) error { + if err == nil && d.IsDir() { + dirs = append(dirs, p) + } + return err + }); err != nil { + t.Fatal(err) + } + for _, d := range dirs { + if err := os.Chmod(d, 0o555); err != nil { + t.Fatal(err) + } + } + t.Cleanup(func() { + for _, d := range dirs { + os.Chmod(d, 0o755) + } + }) + return ws, listing +} + +func workspaceListing(t *testing.T, ws string) []string { + t.Helper() + var files []string + if err := filepath.WalkDir(ws, func(p string, _ os.DirEntry, err error) error { + files = append(files, p) + return err + }); err != nil { + t.Fatal(err) + } + return files +} + +func TestBuildDockerfileBuildsOnce(t *testing.T) { + for name, features := range map[string]string{ + "without Features": "", + "with a Feature": `,"features":{"./hello":{}}`, + } { + t.Run(name, func(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + ws, listing := readOnlyWorkspace(t, map[string]string{ + ".devcontainer/devcontainer.json": `{"build":{"dockerfile":"Dockerfile"}` + features + `}`, + ".devcontainer/Dockerfile": "FROM alpine:3\nRUN true\n", + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\n", + }) + loadResult, err := config.LoadDevContainerConfig(ws, "", "") + if err != nil { + t.Fatal(err) + } + r := &fakeDockerRunner{} + run := &buildRunner{ + log: log.Null, + docker: fakeDockerClient(r), + engine: &docker.EngineClient{API: &fakeDockerAPI{version: "25.0.0", imageConfig: &dockerspec.DockerOCIImageConfig{}}}, + opts: &buildOpts{workspaceFolder: ws, imageNames: []string{"img"}, noLockfile: true}, + } + names, err := run.buildDockerfile(t.Context(), loadResult.Config, loadResult, true) + if err != nil { + t.Fatalf("build of a read-only workspace: %v", err) + } + if !slices.Equal(names, []string{"img"}) { + t.Errorf("image names = %v, want [img]", names) + } + builds := imageBuilds(r.calls) + if len(builds) != 1 { + t.Fatalf("docker builds = %v, want one", builds) + } + b := builds[0] + if f := argAfter(b, "-f"); filepath.Base(f) != featuresDockerfileName || !strings.HasPrefix(f, filepath.Join(os.Getenv("TMPDIR"), "devcontainercli")) { + t.Errorf("-f %s, want the cache folder's %s", f, featuresDockerfileName) + } + if argAfter(b, "--target") != "dev_containers_target_stage" || !slices.Equal(tagsOf(b), []string{"img"}) { + t.Errorf("build = %v, want --target dev_containers_target_stage -t img", b) + } + if got := workspaceListing(t, ws); !slices.Equal(got, listing) { + t.Errorf("workspace holds %v after the build, want %v", got, listing) + } + }) + } +} + +var errStopAfterBuild = errors.New("stop after the build") + +func TestUpDockerfileBuildsOnce(t *testing.T) { + for name, features := range map[string]string{ + "without Features": "", + "with a Feature": `,"features":{"./hello":{}}`, + } { + t.Run(name, func(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + ws, listing := readOnlyWorkspace(t, map[string]string{ + ".devcontainer/devcontainer.json": `{"build":{"dockerfile":"Dockerfile"}` + features + `}`, + ".devcontainer/Dockerfile": "FROM alpine:3\nRUN true\n", + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\n", + }) + loadResult, err := config.LoadDevContainerConfig(ws, "", "") + if err != nil { + t.Fatal(err) + } + r := &fakeDockerRunner{} + run := &upRunner{ + log: log.Null, + docker: fakeDockerClient(r), + engine: &docker.EngineClient{API: &fakeDockerAPI{version: "25.0.0", imageConfig: &dockerspec.DockerOCIImageConfig{}}}, + opts: &upOpts{workspaceFolder: ws, noLockfile: true, updateRemoteUserUIDDefault: "never"}, + substitute: func(*[]imagemeta.Entry) error { return errStopAfterBuild }, + } + if _, err := run.fromDockerfile(t.Context(), loadResult.Config, loadResult, ws, nil, true); !errors.Is(err, errStopAfterBuild) { + t.Fatalf("up of a read-only workspace: %v, want it to reach the container", err) + } + builds := imageBuilds(r.calls) + if len(builds) != 1 { + t.Fatalf("docker builds = %v, want one", builds) + } + b := builds[0] + if got, want := tagsOf(b), []string{folderImageName(ws)}; !slices.Equal(got, want) { + t.Errorf("tags = %v, want %v", got, want) + } + if f := argAfter(b, "-f"); filepath.Base(f) != featuresDockerfileName || argAfter(b, "--target") != "dev_containers_target_stage" { + t.Errorf("build = %v, want -f the cache folder's %s --target dev_containers_target_stage", b, featuresDockerfileName) + } + if got := workspaceListing(t, ws); !slices.Equal(got, listing) { + t.Errorf("workspace holds %v after the build, want %v", got, listing) + } + }) + } +} + +type dockerfileBuildRun struct { + files map[string]string + config config.DevContainer + useBuildx bool + names []string + fb FeatureBuildOptions + imageConfig *dockerspec.DockerOCIImageConfig + reply func([]string) (string, int, error) +} + +type dockerfileBuildResult struct { + ws string + err error + calls []dockerCall + build dockerCall + dockerfile string + path string +} + +func (d dockerfileBuildRun) run(t *testing.T) dockerfileBuildResult { + t.Helper() + t.Setenv("TMPDIR", t.TempDir()) + ws := writeFixture(t, d.files) + cfg := d.config + cfg.ConfigFilePath = filepath.Join(ws, ".devcontainer", "devcontainer.json") + fb := d.fb + fb.FeaturesBasePath, fb.WorkspaceFolder, fb.NoLockfile = filepath.Dir(cfg.ConfigFilePath), ws, true + names := d.names + if names == nil { + names = []string{"img"} + } + imageConfig := d.imageConfig + if imageConfig == nil { + imageConfig = &dockerspec.DockerOCIImageConfig{} + } + r := &fakeDockerRunner{reply: d.reply} + err := buildDockerfileImage(t.Context(), log.Null, fakeDockerClient(r), &docker.EngineClient{API: &fakeDockerAPI{version: "25.0.0", imageConfig: imageConfig}}, + &cfg, d.useBuildx, names, fb.Platform, &fb) + res := dockerfileBuildResult{ws: ws, err: err, calls: r.calls} + if builds := imageBuilds(r.calls); len(builds) > 0 { + res.build = builds[len(builds)-1] + res.path = argAfter(res.build, "-f") + data, rerr := os.ReadFile(res.path) + if rerr != nil { + t.Fatal(rerr) + } + res.dockerfile = string(data) + } + return res +} + +func TestBuildDockerfileWritesTheReferenceDockerfile(t *testing.T) { + t.Run("without Features", func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{ + ".devcontainer/Dockerfile": "# syntax=docker/dockerfile:1\nFROM alpine:3\nRUN echo hi > /hi\n", + }, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile", Args: map[string]string{"A": "1"}}}, + useBuildx: true, + names: []string{"tsparity-ws1"}, + fb: FeatureBuildOptions{LabelMetadata: []imagemeta.Entry{{RemoteUser: "root"}}}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + const want = "# syntax=docker/dockerfile:1\n" + + " ARG _DEV_CONTAINERS_BASE_IMAGE=scratch\n" + + "FROM alpine:3 AS dev_container_auto_added_stage_label\n" + + "RUN echo hi > /hi\n" + + "\n" + + "\n" + + "FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage\n" + + "LABEL devcontainer.metadata=\"[ \\\n" + + "{\\\"remoteUser\\\":\\\"root\\\"} \\\n" + + "]\"\n" + if res.dockerfile != want { + t.Errorf("Dockerfile =\n%q\nwant\n%q", res.dockerfile, want) + } + wantArgs := dockerCall{"buildx", "build", "--load", "--build-arg", "BUILDKIT_INLINE_CACHE=1", + "-f", res.path, "-t", "tsparity-ws1", "--target", "dev_containers_target_stage", + "--build-arg", "A=1", "--build-arg", "_DEV_CONTAINERS_BASE_IMAGE=dev_container_auto_added_stage_label", + filepath.Join(res.ws, ".devcontainer")} + if !reflect.DeepEqual(res.build, wantArgs) { + t.Errorf("build =\n%v\nwant\n%v", res.build, wantArgs) + } + }) + t.Run("with a Feature", func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{ + ".devcontainer/Dockerfile": "FROM alpine:3\nRUN echo hi > /hi\n", + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0","name":"Hello"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\necho installed > /hello\n", + }, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}, Features: map[string]any{"./hello": map[string]any{}}}, + useBuildx: true, + names: []string{"tsparity-ws2"}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + const want = "\n" + + "ARG _DEV_CONTAINERS_BASE_IMAGE=scratch\n" + + "FROM alpine:3 AS dev_container_auto_added_stage_label\n" + + "RUN echo hi > /hi\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "ARG _DEV_CONTAINERS_BASE_IMAGE=scratch\n" + + "\n" + + "FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize\n" + + "USER root\n" + + "COPY --from=dev_containers_feature_content_source devcontainer-features.builtin.env /tmp/build-features/\n" + + "RUN chmod -R 0755 /tmp/build-features/\n" + + "\n" + + "FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage\n" + + "\n" + + "USER root\n" + + "\n" + + "RUN mkdir -p /tmp/dev-container-features\n" + + "COPY --from=dev_containers_feature_content_normalize /tmp/build-features/ /tmp/dev-container-features\n" + + "\n" + + "RUN \\\n" + + "echo \"_CONTAINER_USER_HOME=$( (command -v getent >/dev/null 2>&1 && getent passwd 'root' || grep -E '^root|^[^:]*:[^:]*:root:' /etc/passwd || true) | cut -d: -f6)\" >> /tmp/dev-container-features/devcontainer-features.builtin.env && \\\n" + + "echo \"_REMOTE_USER_HOME=$( (command -v getent >/dev/null 2>&1 && getent passwd 'root' || grep -E '^root|^[^:]*:[^:]*:root:' /etc/passwd || true) | cut -d: -f6)\" >> /tmp/dev-container-features/devcontainer-features.builtin.env\n" + + "\n" + + "\n" + + "RUN --mount=type=bind,from=dev_containers_feature_content_source,source=hello_0,target=/tmp/build-features-src/hello_0 \\\n" + + " cp -ar /tmp/build-features-src/hello_0 /tmp/dev-container-features \\\n" + + " && chmod -R 0755 /tmp/dev-container-features/hello_0 \\\n" + + " && cd /tmp/dev-container-features/hello_0 \\\n" + + " && chmod +x ./devcontainer-features-install.sh \\\n" + + " && ./devcontainer-features-install.sh \\\n" + + " && rm -rf /tmp/dev-container-features/hello_0\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "\n" + + "ARG _DEV_CONTAINERS_IMAGE_USER=root\n" + + "USER $_DEV_CONTAINERS_IMAGE_USER\n" + + "\n" + + "LABEL devcontainer.metadata=\"[ \\\n" + + "{\\\"id\\\":\\\"./hello\\\"} \\\n" + + "]\"\n" + + "\n" + + "\n" + if res.dockerfile != want { + t.Errorf("Dockerfile =\n%q\nwant\n%q", res.dockerfile, want) + } + folder := filepath.Dir(res.path) + wantArgs := dockerCall{"buildx", "build", "--load", "--build-arg", "BUILDKIT_INLINE_CACHE=1", + "-f", res.path, "-t", "tsparity-ws2", "--target", "dev_containers_target_stage", + "--build-context", "dev_containers_feature_content_source=" + folder, + "--build-arg", "_DEV_CONTAINERS_BASE_IMAGE=dev_container_auto_added_stage_label", + "--build-arg", "_DEV_CONTAINERS_IMAGE_USER=root", + "--build-arg", "_DEV_CONTAINERS_FEATURE_CONTENT_SOURCE=dev_container_feature_content_temp", + filepath.Join(res.ws, ".devcontainer")} + if !reflect.DeepEqual(res.build, wantArgs) { + t.Errorf("build =\n%v\nwant\n%v", res.build, wantArgs) + } + }) +} + +func TestBuildDockerfileStage(t *testing.T) { + t.Run("a configured target is the base stage, the Dockerfile kept", func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{".devcontainer/Dockerfile": "FROM alpine AS base\nFROM base\n"}, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile", Target: "base"}}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + if !strings.Contains(res.dockerfile, "\nFROM alpine AS base\nFROM base\n\n") || !slices.Contains(res.build, "_DEV_CONTAINERS_BASE_IMAGE=base") { + t.Errorf("Dockerfile %q, build %v; want the user's Dockerfile as is on base", res.dockerfile, res.build) + } + if argAfter(res.build, "--target") != "dev_containers_target_stage" { + t.Errorf("build %v, want --target dev_containers_target_stage", res.build) + } + }) + for name, cfg := range map[string]config.DevContainer{ + "build": {Build: &config.Build{Dockerfile: "Dockerfile"}}, + "top level": {DockerFile: "Dockerfile"}, + } { + t.Run("the unnamed final stage is named, "+name, func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{".devcontainer/Dockerfile": "FROM alpine\n"}, + config: cfg, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + if !strings.Contains(res.dockerfile, "FROM alpine AS dev_container_auto_added_stage_label\n") { + t.Errorf("Dockerfile = %q, want the final stage named", res.dockerfile) + } + if data, err := os.ReadFile(filepath.Join(res.ws, ".devcontainer", "Dockerfile")); err != nil || string(data) != "FROM alpine\n" { + t.Errorf("user's Dockerfile = %q, %v; want it untouched", data, err) + } + }) + } + t.Run("a Dockerfile without FROM", func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{".devcontainer/Dockerfile": "RUN true\n"}, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}}, + }.run(t) + if res.err == nil || !strings.Contains(res.err.Error(), "no FROM instructions") || res.build != nil { + t.Errorf("err = %v, build = %v; want the parse error and no build", res.err, res.build) + } + }) +} + +func TestBuildDockerfileKeepsTheSiblingDockerignore(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{ + ".devcontainer/Dockerfile": "FROM alpine\n", + ".devcontainer/Dockerfile.dockerignore": "secret\n", + }, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + if data, err := os.ReadFile(res.path + ".dockerignore"); err != nil || string(data) != "secret\n" { + t.Errorf("%s.dockerignore = %q, %v; want the user's", res.path, data, err) + } +} + +func TestBuildDockerfileMetadataLabel(t *testing.T) { + base := &dockerspec.DockerOCIImageConfig{} + base.Labels = map[string]string{imagemeta.MetadataLabel: `[{"id":"base"}]`} + for name, tc := range map[string]struct { + features map[string]any + want []string + }{ + "without Features": {nil, []string{`"id":"base"`, `"id":"stage"`, `"remoteUser":"dev"`}}, + "with a Feature": {map[string]any{"./hello": map[string]any{}}, []string{`"id":"base"`, `"id":"stage"`, `"id":"./hello"`, `"remoteUser":"dev"`}}, + } { + t.Run(name, func(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{ + ".devcontainer/Dockerfile": "FROM alpine\nLABEL devcontainer.metadata='[{\"id\":\"stage\"}]'\n", + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\n", + }, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}, Features: tc.features}, + imageConfig: base, + fb: FeatureBuildOptions{LabelMetadata: []imagemeta.Entry{{RemoteUser: "dev"}}}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + label := res.dockerfile[strings.LastIndex(res.dockerfile, "LABEL "+imagemeta.MetadataLabel):] + label = strings.ReplaceAll(label, `\"`, `"`) + at := 0 + for _, w := range tc.want { + i := strings.Index(label[at:], w) + if i < 0 { + t.Fatalf("label %s: %s missing or out of order, want %v", label, w, tc.want) + } + at += i + len(w) + } + }) + } +} + +func TestBuildDockerfileBuildFlags(t *testing.T) { + bundle := filepath.Join(t.TempDir(), "ca.pem") + res := dockerfileBuildRun{ + files: map[string]string{".devcontainer/Dockerfile": "FROM alpine AS dev\n"}, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile", Options: []string{"--network=host"}, + CacheFrom: config.StringOrStrings{"cfg-cache"}}}, + useBuildx: true, + fb: FeatureBuildOptions{Push: true, Platform: "linux/arm64", CacheFrom: []string{"flag-cache"}, CacheTo: "type=local,dest=c", + Labels: []string{"a=b"}, Secrets: []string{"TOKEN=v"}, CABundle: bundle, NoCache: true}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + b := res.build + for _, want := range [][]string{ + {"--platform", "linux/arm64"}, {"--push"}, {"--cache-to", "type=local,dest=c"}, + {"--secret", "id=TOKEN,env=TOKEN"}, {"--secret", "id=" + imagemeta.CABundleSecretID + ",src=" + bundle}, + {"--no-cache", "--pull"}, {"--cache-from", "flag-cache", "--cache-from", "cfg-cache"}, + {"--label", "a=b", "--network=host", "--build-arg", "_DEV_CONTAINERS_BASE_IMAGE=dev"}, + } { + if !containsRun(b, want) { + t.Errorf("build %v lacks %v", b, want) + } + } + if slices.Contains(b, "--load") || slices.Contains(b, "--output") { + t.Errorf("build %v: --push takes the place of --load", b) + } + output := dockerfileBuildRun{ + files: map[string]string{".devcontainer/Dockerfile": "FROM alpine AS dev\n"}, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}, Features: map[string]any{"./hello": map[string]any{}}}, + useBuildx: true, + fb: FeatureBuildOptions{Output: "type=oci,dest=out.tar"}, + } + output.files[".devcontainer/hello/devcontainer-feature.json"] = `{"id":"hello","version":"1.0.0"}` + output.files[".devcontainer/hello/install.sh"] = "#!/bin/sh\n" + if res := output.run(t); res.err != nil || !containsRun(res.build, []string{"--output", "type=oci,dest=out.tar"}) || len(imageBuilds(res.calls)) != 1 { + t.Errorf("with a Feature: err %v, builds %v; want one build with --output", res.err, imageBuilds(res.calls)) + } +} + +func containsRun(c dockerCall, run []string) bool { + for i := 0; i+len(run) <= len(c); i++ { + if slices.Equal(c[i:i+len(run)], run) { + return true + } + } + return false +} + +func TestBuildDockerfileWithoutBuildKit(t *testing.T) { + res := dockerfileBuildRun{ + files: map[string]string{ + ".devcontainer/Dockerfile": "FROM alpine\n", + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\n", + }, + config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}, Features: map[string]any{"./hello": map[string]any{}}}, + fb: FeatureBuildOptions{Secrets: []string{"TOKEN=v"}}, + }.run(t) + if res.err != nil { + t.Fatal(res.err) + } + builds := imageBuilds(res.calls) + if len(builds) != 2 || !slices.Contains(builds[0], imagemeta.ComposeFeatureContentImage) { + t.Fatalf("builds = %v, want the content image then the image", builds) + } + for _, flag := range []string{"--build-context", "--secret", "--load", "BUILDKIT_INLINE_CACHE=1"} { + if slices.Contains(res.build, flag) { + t.Errorf("build %v has %s", res.build, flag) + } + } + if !strings.Contains(res.dockerfile, "FROM "+imagemeta.ComposeFeatureContentImage+" as dev_containers_feature_content_source") { + t.Errorf("Dockerfile = %q, want the content image stage", res.dockerfile) + } +} + +func TestBuildDockerfileFailures(t *testing.T) { + files := map[string]string{".devcontainer/Dockerfile": "FROM alpine\n"} + for name, tc := range map[string]struct { + run dockerfileBuildRun + want string + }{ + "no Dockerfile": {dockerfileBuildRun{files: files, config: config.DevContainer{}}, "no Dockerfile specified"}, + "missing Dockerfile": {dockerfileBuildRun{files: files, config: config.DevContainer{Build: &config.Build{Dockerfile: "Missing"}}}, "read Dockerfile"}, + "docker fails": {dockerfileBuildRun{files: files, config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}}, reply: func([]string) (string, int, error) { return "", 0, errors.New("no docker") }}, "exec docker: no docker"}, + "build fails": {dockerfileBuildRun{files: files, config: config.DevContainer{Build: &config.Build{Dockerfile: "Dockerfile"}}, reply: func([]string) (string, int, error) { return "", 3, nil }}, "Command failed: docker build (exit 3)"}, + } { + t.Run(name, func(t *testing.T) { + if res := tc.run.run(t); res.err == nil || !strings.Contains(res.err.Error(), tc.want) { + t.Errorf("err = %v, want %q", res.err, tc.want) + } + }) + } +} + +func TestFeaturesSyntaxDirective(t *testing.T) { + withSyntax := docker.ExtractDockerfile("# syntax=docker/dockerfile:1.3\nFROM alpine\n") + engine := func(v string) *docker.EngineClient { return &docker.EngineClient{API: &fakeDockerAPI{version: v}} } + for name, tc := range map[string]struct { + engine *docker.EngineClient + omit, ctxs bool + parsed *docker.Dockerfile + want string + }{ + "omitted": {engine("25.0.0"), true, true, withSyntax, ""}, + "engine 23 keeps the user's": {engine("23.0.0"), false, true, withSyntax, "docker/dockerfile:1.3"}, + "old engine needs 1.4": {engine("20.10.7"), false, true, withSyntax, "docker/dockerfile:1.4"}, + "old engine without contexts": {engine("20.10.7"), false, false, withSyntax, "docker/dockerfile:1.3"}, + "no Dockerfile, old engine": {engine("20.10.7"), false, true, nil, "docker/dockerfile:1.4"}, + "no Dockerfile, no contexts": {engine("20.10.7"), false, false, nil, ""}, + } { + t.Run(name, func(t *testing.T) { + if got := featuresSyntaxDirective(t.Context(), tc.engine, tc.omit, tc.ctxs, tc.parsed); got != tc.want { + t.Errorf("syntax = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/cli/dockerfile_path_test.go b/internal/cli/dockerfile_path_test.go index d1e874a..0ceb060 100644 --- a/internal/cli/dockerfile_path_test.go +++ b/internal/cli/dockerfile_path_test.go @@ -3,9 +3,14 @@ package cli import ( "os" "path/filepath" + "slices" "testing" + dockerspec "github.com/moby/docker-image-spec/specs-go/v1" + "github.com/devcontainers/cli/internal/config" + "github.com/devcontainers/cli/internal/docker" + "github.com/devcontainers/cli/internal/log" ) func TestDockerfileAndContextPaths(t *testing.T) { @@ -42,13 +47,14 @@ func TestAbsoluteDockerfileIsReadWhereItIs(t *testing.T) { t.Fatal(err) } cfg := &config.DevContainer{ConfigFilePath: filepath.Join(t.TempDir(), "devcontainer.json"), Build: &config.Build{Dockerfile: dockerfile}} - prep, err := prepareDockerfileBuild(cfg) - if err != nil { + t.Setenv("TMPDIR", t.TempDir()) + r := &fakeDockerRunner{} + if err := buildDockerfileImage(t.Context(), log.Null, fakeDockerClient(r), &docker.EngineClient{API: &fakeDockerAPI{version: "25.0.0", imageConfig: &dockerspec.DockerOCIImageConfig{}}}, cfg, true, []string{"img"}, "", &FeatureBuildOptions{}); err != nil { t.Fatal(err) } - defer prep.Cleanup() - if prep.Path != dockerfile || prep.StageName != "base" { - t.Errorf("prep = %q, stage %q; want %q, stage base", prep.Path, prep.StageName, dockerfile) + builds := imageBuilds(r.calls) + if len(builds) != 1 || builds[0][len(builds[0])-1] != filepath.Dir(dockerfile) || !slices.Contains(builds[0], "_DEV_CONTAINERS_BASE_IMAGE=base") { + t.Errorf("builds = %v; want one, on base, in %s", builds, filepath.Dir(dockerfile)) } key, err := computeCacheKey(&config.LoadResult{Config: cfg}, nil) if err != nil || key == "" { diff --git a/internal/cli/feature_install.go b/internal/cli/feature_install.go index 20287bd..1a54518 100644 --- a/internal/cli/feature_install.go +++ b/internal/cli/feature_install.go @@ -57,6 +57,7 @@ type FeatureBuildOptions struct { ConfigPath string LockfileExcludeIDs map[string]bool OverrideFeatureInstallOrder []string + OmitSyntaxDirective bool } type fetchFeatureResult struct { @@ -570,17 +571,10 @@ func substituteFeatureHostVars(sets []*features.Set, hs config.HostSubContext) e return nil } -func extendImageWithFeatures( - ctx context.Context, - logger log.Logger, - dockerClient *docker.Client, - engine *docker.EngineClient, - baseImage string, - featuresCfg map[string]any, - useBuildx bool, - imageNames []string, - fbOpts *FeatureBuildOptions, -) ([]string, error) { +func fetchBuildFeatures(ctx context.Context, logger log.Logger, featuresCfg map[string]any, fbOpts *FeatureBuildOptions) (*fetchFeatureResult, error) { + if len(featuresCfg) == 0 { + return nil, nil + } var lockfile *features.Lockfile if fbOpts.ConfigPath != "" && !fbOpts.NoLockfile { var err error @@ -592,24 +586,40 @@ func extendImageWithFeatures( if err != nil { return nil, err } - if result == nil { - return imageNames, nil - } - featureSets := result.FeatureSets - tmpDir := result.TmpDir - if fbOpts.HostSub != nil { - if err := substituteFeatureHostVars(featureSets, *fbOpts.HostSub); err != nil { + if err := substituteFeatureHostVars(result.FeatureSets, *fbOpts.HostSub); err != nil { return nil, err } } - if !fbOpts.NoLockfile && fbOpts.ConfigPath != "" { - lf := features.GenerateLockfile(&features.Config{FeatureSets: featureSets}, fbOpts.LockfileExcludeIDs) + lf := features.GenerateLockfile(&features.Config{FeatureSets: result.FeatureSets}, fbOpts.LockfileExcludeIDs) if err := features.WriteLockfile(fbOpts.ConfigPath, lf, features.WriteOptions{Frozen: fbOpts.FrozenLockfile}); err != nil { return nil, fmt.Errorf("lockfile: %w", err) } } + return result, nil +} + +func extendImageWithFeatures( + ctx context.Context, + logger log.Logger, + dockerClient *docker.Client, + engine *docker.EngineClient, + baseImage string, + featuresCfg map[string]any, + useBuildx bool, + imageNames []string, + fbOpts *FeatureBuildOptions, +) ([]string, error) { + result, err := fetchBuildFeatures(ctx, logger, featuresCfg, fbOpts) + if err != nil { + return nil, err + } + if result == nil { + return imageNames, nil + } + featureSets := result.FeatureSets + tmpDir := result.TmpDir baseImageInfo, _ := engine.InspectImage(ctx, baseImage) diff --git a/internal/cli/up.go b/internal/cli/up.go index 9333308..9daa99a 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -550,77 +550,24 @@ func addUpConfiguration(result map[string]any, opts *upOpts, fsPathRead bool, cf // mutate-lane: TestParityMatrix - up.update-uid-remote-user-from-image-metadata-success, // up.dockerfile-without-features-omit-config-remote-env-from-metadata-success, -// up.dockerfile-with-parallel-commands-success and up.update-uid-amd64-dockerfile-platform-success -// build through it; also TestE2E_BuildUpExec_Dockerfile and +// up.dockerfile-with-parallel-commands-success, up.update-uid-amd64-dockerfile-platform-success +// and up.read-only-workspace-dockerfile-success build through it; also TestUpDockerfileBuildsOnce, +// TestE2E_BuildUpExec_Dockerfile and // TestE2E_UpSecretsFile (a Dockerfile RUN reads a --secrets-file secret). func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer, loadResult *config.LoadResult, workspaceFolder string, idLabels []string, useBuildx bool) (string, error) { - logger, dockerClient, engine, opts := r.log, r.docker, r.engine, r.opts - - prep, err := prepareDockerfileBuild(cfg) - if err != nil { - return "", err - } - defer prep.Cleanup() - dockerfilePath := prep.Path - stageName := prep.StageName - + opts := r.opts imageName := folderImageName(workspaceFolder) - - findTarget := "" - if cfg.Build != nil { - findTarget = cfg.Build.Target - } - baseImage := docker.FindBaseImage(prep.Parsed, buildArgsFromConfig(cfg), findTarget, buildxPlatformArgs(opts.platform)) - - baseLabels := engine.ImageLabelsEnsuringPresent(ctx, baseImage) - baseMetadata := imagemeta.ReadMetadataFromLabels(baseLabels, logger) - stageMetadata := imagemeta.ReadMetadataFromLabels(prep.Parsed.StageLabels(stageName), logger) - - contextPath := dockerContextPath(cfg) - - upCacheFrom := cacheFromForDockerfileBuild(opts.cacheFrom, cfg) - authEnv, authCleanup := bridgeBuildAuth(logger, baseImage, []string{imageName}, upCacheFrom, opts.cacheTo) - defer authCleanup() - - buildResult, err := dockerClient.Build(ctx, docker.BuildOptions{ - Secrets: buildxSecrets(opts.secrets.env(), useBuildx, logger), - FileSecrets: caBundleBuildSecrets(opts.caBundle, useBuildx, logger), - Dockerfile: dockerfilePath, - ContextPath: contextPath, - Tags: []string{imageName}, - Target: stageName, - BuildArgs: buildArgsFromConfig(cfg), - ExtraArgs: buildOptionsFromConfig(cfg), - CacheFrom: upCacheFrom, - Labels: imageLabelArgs(dockerfileImageMetadata(cfg, baseMetadata, stageMetadata, r.configLabel)), - NoCache: opts.buildNoCache, - UseBuildx: useBuildx, - CacheTo: opts.cacheTo, - Env: authEnv, - }) - if err != nil { - return "", err - } - if buildResult.ExitCode != 0 { - return "", fmt.Errorf(msgDockerBuildFailed, buildResult.ExitCode, string(buildResult.Stderr)) - } - - if dockerClient.ProgressWriter == nil { - logger.Write(string(buildResult.Stderr), log.LevelInfo) - } - if len(cfg.Features) > 0 { r.lockfileRead = !opts.noLockfile - names, err := extendImageWithFeatures(ctx, logger, dockerClient, engine, imageName, cfg.Features, useBuildx, nil, r.featureBuildOptions(cfg, loadResult, "")) - if err != nil { - return "", fmt.Errorf("install features: %w", err) - } - if len(names) > 0 { - imageName = names[0] - } + } + fb := r.featureBuildOptions(cfg, loadResult, "") + fb.NoCache, fb.CacheFrom, fb.CacheTo = opts.buildNoCache, opts.cacheFrom, opts.cacheTo + fb.OmitSyntaxDirective = opts.omitSyntaxDirective + if err := buildDockerfileImage(ctx, r.log, r.docker, r.engine, cfg, useBuildx, []string{imageName}, opts.platform, fb); err != nil { + return "", err } - imageName, err = r.singleContainerRemoteUserUID(ctx, imageName, cfg, workspaceFolder) + imageName, err := r.singleContainerRemoteUserUID(ctx, imageName, cfg, workspaceFolder) if err != nil { return "", err }