diff --git a/THIRD_PARTY_LICENSES.md b/THIRD_PARTY_LICENSES.md index 821841d04..2e80c698f 100644 --- a/THIRD_PARTY_LICENSES.md +++ b/THIRD_PARTY_LICENSES.md @@ -75,7 +75,7 @@ To regenerate this file after changing dependencies, run `task cli:licenses`. | [github.com/containerd/console](https://github.com/containerd/console) | `v1.0.5` | Apache-2.0 | | [github.com/containerd/containerd/api](https://github.com/containerd/containerd) | `v1.10.0` | Apache-2.0 | | [github.com/containerd/containerd/v2](https://github.com/containerd/containerd) | `v2.2.2` | Apache-2.0 | -| [github.com/containerd/continuity](https://github.com/containerd/continuity) | `v0.4.5` | Apache-2.0 | +| [github.com/containerd/continuity](https://github.com/containerd/continuity) | `v0.5.0` | Apache-2.0 | | [github.com/containerd/errdefs](https://github.com/containerd/errdefs) | `v1.0.0` | Apache-2.0 | | [github.com/containerd/errdefs/pkg](https://github.com/containerd/errdefs) | `v0.3.0` | Apache-2.0 | | [github.com/containerd/log](https://github.com/containerd/log) | `v0.1.0` | Apache-2.0 | @@ -243,7 +243,7 @@ To regenerate this file after changing dependencies, run `task cli:licenses`. | [github.com/tidwall/pretty](https://github.com/tidwall/pretty) | `v1.2.1` | MIT | | [github.com/tklauser/go-sysconf](https://github.com/tklauser/go-sysconf) | `v0.3.16` | BSD-3-Clause | | [github.com/tklauser/numcpus](https://github.com/tklauser/numcpus) | `v0.11.0` | Apache-2.0 | -| [github.com/tonistiigi/fsutil](https://github.com/tonistiigi/fsutil) | `v0.0.0-20251211185533-a2aa163d723f` | MIT | +| [github.com/tonistiigi/fsutil](https://github.com/tonistiigi/fsutil) | `v0.0.0-20260609174605-b61e79c0c046` | MIT | | [github.com/tonistiigi/go-csvvalue](https://github.com/tonistiigi/go-csvvalue) | `v0.0.0-20240814133006-030d3b2625d0` | MIT | | [github.com/tonistiigi/units](https://github.com/tonistiigi/units) | `v0.0.0-20180711220420-6950e57a87ea` | MIT | | [github.com/tonistiigi/vt100](https://github.com/tonistiigi/vt100) | `v0.0.0-20240514184818-90bafcd6abab` | MIT | diff --git a/cmd/internal/container_tunnel.go b/cmd/internal/container_tunnel.go index ede6ecdfa..aa8879265 100644 --- a/cmd/internal/container_tunnel.go +++ b/cmd/internal/container_tunnel.go @@ -93,7 +93,13 @@ func (cmd *ContainerTunnelCmd) Run(ctx context.Context) error { stdin io.Reader, stdout, stderr io.Writer, ) error { - return runner.Command(ctx, user, command, stdin, stdout, stderr) + return runner.Command(ctx, devcontainer.CommandParams{ + User: user, + Command: command, + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, + }) }, User: cmd.User, Stdin: os.Stdin, @@ -113,29 +119,36 @@ func startDevContainer( return err } - // start container if necessary + // start container if it is missing or not running if containerDetails == nil || containerDetails.State.Status != "running" { - // start container _, err = StartContainer(ctx, runner, workspaceConfig) - if err != nil { - return err - } - } else if encoding.IsLegacyUID(workspaceConfig.Workspace.UID) { - // make sure workspace result is in devcontainer - buf := &bytes.Buffer{} - err = runner.Command(ctx, "root", "cat "+pkgconfig.DevContainerResultPath, nil, buf, buf) - if err != nil { - // start container - _, err = StartContainer(ctx, runner, workspaceConfig) - if err != nil { - return err - } - } + return err + } + + // for legacy UIDs, ensure the workspace result is present in the container, + // restarting it when the result is missing + if encoding.IsLegacyUID(workspaceConfig.Workspace.UID) && + !hasDevContainerResult(ctx, runner) { + _, err = StartContainer(ctx, runner, workspaceConfig) + return err } return nil } +// hasDevContainerResult reports whether the devcontainer result file is readable +// inside the running container. +func hasDevContainerResult(ctx context.Context, runner devcontainer.Runner) bool { + buf := &bytes.Buffer{} + err := runner.Command(ctx, devcontainer.CommandParams{ + User: "root", + Command: "cat " + pkgconfig.DevContainerResultPath, + Stdout: buf, + Stderr: buf, + }) + return err == nil +} + func StartContainer( ctx context.Context, runner devcontainer.Runner, diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index bb45a4e53..79b0f632b 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -100,6 +100,8 @@ func NewBuildCmd(flags *flags.GlobalFlags) *cobra.Command { StringVar(&cmd.ImageName, "image-name", "", "Alternative name for the built image") buildCmd.Flags(). BoolVar(&cmd.NoBuild, "no-build", false, "Fail if the image must be built (enforce pre-built images only)") + buildCmd.Flags(). + BoolVar(&cmd.Pull, "pull", false, "Always attempt to pull a newer version of the base image when building") // TESTING buildCmd.Flags().BoolVar(&cmd.ForceBuild, "force-build", false, "TESTING ONLY") diff --git a/cmd/workspace/up/up_flags.go b/cmd/workspace/up/up_flags.go index 042e92cef..7d1c4f9ee 100644 --- a/cmd/workspace/up/up_flags.go +++ b/cmd/workspace/up/up_flags.go @@ -195,6 +195,12 @@ func (cmd *UpCmd) registerWorkspaceFlags(upCmd *cobra.Command) { upCmd.Flags(). BoolVar(&cmd.Prebuild, "prebuild", false, "If true will only run the prebuild lifecycle (onCreateCommand + updateContentCommand) then stop") + upCmd.Flags(). + BoolVar(&cmd.Pull, "pull", false, + "Always attempt to pull a newer version of the base image when building") + upCmd.Flags(). + BoolVar(&cmd.NoCache, "no-cache", false, + "Do not use the build cache when building the image") upCmd.Flags(). BoolVar(&cmd.Recreate, "recreate", false, "If true will remove any existing containers and recreate them") upCmd.Flags().BoolVar(&cmd.Recreate, "remove-existing-container", false, "Alias for --recreate") diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/.devcontainer.json b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/.devcontainer.json new file mode 100644 index 000000000..13a709164 --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/.devcontainer.json @@ -0,0 +1,11 @@ +{ + "name": "Go", + "dockerComposeFile": "./docker-compose.yaml", + "service": "app", + "workspaceFolder": "/workspaces", + "features": { + "ghcr.io/devsy-org/devcontainer-features/vcluster:1.0.1": { + "version": "v0.24.1" + } + } +} diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/Dockerfile b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/Dockerfile new file mode 100644 index 000000000..dcab83b93 --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/Dockerfile @@ -0,0 +1,9 @@ +FROM ghcr.io/devsy-org/test-images/go:1 AS base + +RUN echo "base-stage" > /stage-marker.txt + +FROM base AS dev + +RUN echo "dev-stage" > /stage-marker.txt +RUN mkdir -p /app +WORKDIR /app diff --git a/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/docker-compose.yaml b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/docker-compose.yaml new file mode 100644 index 000000000..2564d2ae1 --- /dev/null +++ b/e2e/tests/up-docker-compose/testdata/docker-compose-build-target-features/docker-compose.yaml @@ -0,0 +1,11 @@ +version: '3' + +services: + app: + build: + context: . + dockerfile: Dockerfile + target: dev + command: sleep infinity + volumes: + - .:/workspaces:cached diff --git a/e2e/tests/up-docker-compose/up_docker_compose.go b/e2e/tests/up-docker-compose/up_docker_compose.go index 6726b4f96..03136c68c 100644 --- a/e2e/tests/up-docker-compose/up_docker_compose.go +++ b/e2e/tests/up-docker-compose/up_docker_compose.go @@ -334,6 +334,43 @@ var _ = ginkgo.Describe( To(gomega.ContainSubstring("vcluster version 0.24.1")) }, ginkgo.SpecTimeout(framework.TimeoutLong())) + // Regression guard: a build-backed service with an explicit build.target + // and features must honor the real Dockerfile. Previously the Dockerfile + // contents were dropped when a target was set, producing a synthesized + // "FROM AS " that ignored the requested stage (and broke + // build-only services that have no top-level image). + ginkgo.It("features with build target", func(ctx context.Context) { + tempDir, workspace, err := tc.setupAndStartWorkspace( + ctx, + "tests/up-docker-compose/testdata/docker-compose-build-target-features", + "--debug", + ) + framework.ExpectNoError(err) + + ids, err := findComposeContainer( + ctx, + tc.dockerHelper, + tc.composeHelper, + workspace.UID, + "app", + ) + framework.ExpectNoError(err) + gomega.Expect(ids).To(gomega.HaveLen(1), "1 compose container to be created") + + // The "dev" stage overwrites the marker, so seeing "dev-stage" + // proves the real multi-stage Dockerfile (and its target) was built. + stageMarker, err := tc.execSSH(ctx, tempDir, "cat /stage-marker.txt") + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(stageMarker)). + To(gomega.Equal("dev-stage"), "the requested build target stage should be used") + + // The feature must still be installed on top of the targeted stage. + vclusterVersionOutput, err := tc.execSSH(ctx, tempDir, "vcluster --version") + framework.ExpectNoError(err) + gomega.Expect(vclusterVersionOutput). + To(gomega.ContainSubstring("vcluster version 0.24.1")) + }, ginkgo.SpecTimeout(framework.TimeoutLong())) + ginkgo.It( "does not retag shared image when applying features to image backed services", func(ctx context.Context) { diff --git a/pkg/devcontainer/build.go b/pkg/devcontainer/build.go index 5fd4560b5..6ed36784c 100644 --- a/pkg/devcontainer/build.go +++ b/pkg/devcontainer/build.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/devsy-org/devsy/pkg/compose" pkgconfig "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/devcontainer/build" "github.com/devsy-org/devsy/pkg/devcontainer/buildkit" @@ -31,11 +30,12 @@ func (r *runner) build( var buildInfo *config.BuildInfo var err error - if isDockerFileConfig(parsedConfig.Config) { + switch { + case isDockerFileConfig(parsedConfig.Config): buildInfo, err = r.buildAndExtendImage(ctx, parsedConfig, substitutionContext, options) - } else if isDockerComposeConfig(parsedConfig.Config) { + case isDockerComposeConfig(parsedConfig.Config): buildInfo, err = r.buildDevImageCompose(ctx, parsedConfig, substitutionContext, options) - } else { + default: buildInfo, err = r.extendImage(ctx, parsedConfig, substitutionContext, options) } @@ -95,56 +95,80 @@ func (r *runner) extendImage( } // build the image - return r.buildImage( - ctx, - parsedConfig, - substitutionContext, - imageBuildInfo, - extendedBuildInfo, - "", - "", - options, - ) + return r.buildImage(ctx, &buildImageParams{ + parsedConfig: parsedConfig, + substitutionContext: substitutionContext, + buildInfo: imageBuildInfo, + extendedBuildInfo: extendedBuildInfo, + options: options, + }) } -func (r *runner) buildAndExtendImage( - ctx context.Context, +// dockerfileBuildBase holds the resolved Dockerfile and the image base/target +// stage for a dockerfile-backed build. +type dockerfileBuildBase struct { + path string + content []byte + imageBase string +} + +// resolveDockerfileBuildBase locates and reads the Dockerfile and determines the +// image base/target stage, ensuring a final stage name when no target is set. +func (r *runner) resolveDockerfileBuildBase( parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - options provider.BuildOptions, -) (*config.BuildInfo, error) { +) (*dockerfileBuildBase, error) { dockerFilePath, err := r.getDockerfilePath(parsedConfig.Config) if err != nil { return nil, err } + // #nosec G304 -- dockerFilePath is derived from trusted devcontainer config. dockerFileContent, err := os.ReadFile(dockerFilePath) if err != nil { return nil, err } - // ensure there is a target to choose for us - var imageBase string - if parsedConfig.Config.GetTarget() != "" { - imageBase = parsedConfig.Config.GetTarget() - } else { - lastTargetName, modifiedDockerfileContents, err := dockerfile.EnsureFinalStageName( - string(dockerFileContent), - config.DockerfileDefaultTarget, - ) - if err != nil { - return nil, err - } else if modifiedDockerfileContents != "" { - dockerFileContent = []byte(modifiedDockerfileContents) - } + if target := parsedConfig.Config.GetTarget(); target != "" { + return &dockerfileBuildBase{ + path: dockerFilePath, + content: dockerFileContent, + imageBase: target, + }, nil + } - imageBase = lastTargetName + lastTargetName, modifiedDockerfileContents, err := dockerfile.EnsureFinalStageName( + string(dockerFileContent), + config.DockerfileDefaultTarget, + ) + if err != nil { + return nil, err + } + if modifiedDockerfileContents != "" { + dockerFileContent = []byte(modifiedDockerfileContents) + } + + return &dockerfileBuildBase{ + path: dockerFilePath, + content: dockerFileContent, + imageBase: lastTargetName, + }, nil +} + +func (r *runner) buildAndExtendImage( + ctx context.Context, + parsedConfig *config.SubstitutedConfig, + substitutionContext *config.SubstitutionContext, + options provider.BuildOptions, +) (*config.BuildInfo, error) { + base, err := r.resolveDockerfileBuildBase(parsedConfig) + if err != nil { + return nil, err } // get image build info imageBuildInfo, err := r.getImageBuildInfoFromDockerfile( substitutionContext, - string(dockerFileContent), + string(base.content), parsedConfig.Config.GetArgs(), parsedConfig.Config.GetTarget(), ) @@ -156,7 +180,7 @@ func (r *runner) buildAndExtendImage( extendedBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildParams{ Ctx: substitutionContext, ImageBuildInfo: imageBuildInfo, - Target: imageBase, + Target: base.imageBase, DevContainerConfig: parsedConfig, ForceBuild: options.ForceBuild, SecretOpts: featureSecretOpts(options), @@ -166,16 +190,15 @@ func (r *runner) buildAndExtendImage( } // build the image - return r.buildImage( - ctx, - parsedConfig, - substitutionContext, - imageBuildInfo, - extendedBuildInfo, - dockerFilePath, - string(dockerFileContent), - options, - ) + return r.buildImage(ctx, &buildImageParams{ + parsedConfig: parsedConfig, + substitutionContext: substitutionContext, + buildInfo: imageBuildInfo, + extendedBuildInfo: extendedBuildInfo, + dockerfilePath: base.path, + dockerfileContent: string(base.content), + options: options, + }) } func (r *runner) getDockerfilePath(parsedConfig *config.DevContainerConfig) (string, error) { @@ -252,12 +275,8 @@ func (r *runner) getImageBuildInfoFromDockerfile( return nil, fmt.Errorf("parse dockerfile: %w", err) } - // Check that the build target specified in the devcontainer.json exists in the Dockerfile - if target != "" && parsedDockerfile.StagesByTarget != nil { - _, ok := parsedDockerfile.StagesByTarget[target] - if !ok { - return nil, fmt.Errorf("build target does not exist") - } + if err := validateDockerfileTarget(parsedDockerfile, target); err != nil { + return nil, err } baseImage := parsedDockerfile.FindBaseImage(buildArgs, target) @@ -270,8 +289,42 @@ func (r *runner) getImageBuildInfoFromDockerfile( return nil, fmt.Errorf("inspect image %s: %w", baseImage, err) } - // find user - user := parsedDockerfile.FindUserStatement( + user := resolveDockerfileUser(parsedDockerfile, buildArgs, imageDetails, target) + + // parse metadata from image details + imageMetadataConfig, err := metadata.GetImageMetadata(imageDetails, substitutionContext) + if err != nil { + return nil, fmt.Errorf("get image metadata: %w", err) + } + + return &config.ImageBuildInfo{ + Dockerfile: parsedDockerfile, + User: user, + Metadata: imageMetadataConfig, + }, nil +} + +// validateDockerfileTarget ensures a non-empty build target exists in the +// parsed Dockerfile's stages. +func validateDockerfileTarget(parsed *dockerfile.Dockerfile, target string) error { + if target == "" || parsed.StagesByTarget == nil { + return nil + } + if _, ok := parsed.StagesByTarget[target]; !ok { + return fmt.Errorf("build target does not exist") + } + return nil +} + +// resolveDockerfileUser determines the build user, preferring a USER statement +// in the Dockerfile, then the base image's user, then "root". +func resolveDockerfileUser( + parsed *dockerfile.Dockerfile, + buildArgs map[string]string, + imageDetails *config.ImageDetails, + target string, +) string { + user := parsed.FindUserStatement( buildArgs, config.ListToObject(imageDetails.Config.Env), target, @@ -282,30 +335,87 @@ func (r *runner) getImageBuildInfoFromDockerfile( if user == "" { user = "root" } + return user +} - // parse metadata from image details - imageMetadataConfig, err := metadata.GetImageMetadata(imageDetails, substitutionContext) - if err != nil { - return nil, fmt.Errorf("get image metadata: %w", err) +// prebuildLookupParams groups the inputs for locating an existing prebuild image. +type prebuildLookupParams struct { + parsedConfig *config.SubstitutedConfig + extendedBuildInfo *feature.ExtendedBuildInfo + options provider.BuildOptions + prebuildHash string + targetArch string +} + +// findPrebuildImage searches the configured prebuild repositories for an image +// matching the prebuild hash and target architecture. It returns the resolved +// BuildInfo when found, or (nil, nil) when no prebuild image is available. +func (r *runner) findPrebuildImage( + ctx context.Context, + params *prebuildLookupParams, +) (*config.BuildInfo, error) { + options := params.options + devsyCustomizations := config.GetDevsyCustomizations(params.parsedConfig.Config) + if options.Repository != "" { + options.PrebuildRepositories = append(options.PrebuildRepositories, options.Repository) + } + options.PrebuildRepositories = append( + options.PrebuildRepositories, + devsyCustomizations.PrebuildRepository...) + + log.Debugf( + "Try to find prebuild image %s in repositories %s", + params.prebuildHash, + strings.Join(options.PrebuildRepositories, ","), + ) + for _, prebuildRepo := range options.PrebuildRepositories { + prebuildImage := prebuildRepo + ":" + params.prebuildHash + img, err := image.GetImageForArch(ctx, prebuildImage, params.targetArch) + if err != nil { + log.Debugf("Error trying to find prebuild image %s: %v", prebuildImage, err) + continue + } + if img == nil { + continue + } + + log.Infof("Found existing prebuilt image %s", prebuildImage) + imageDetails, err := r.inspectImage(ctx, prebuildImage) + if err != nil { + return nil, fmt.Errorf("get image details: %w", err) + } + + return &config.BuildInfo{ + ImageDetails: imageDetails, + ImageMetadata: params.extendedBuildInfo.MetadataConfig, + ImageName: prebuildImage, + PrebuildHash: params.prebuildHash, + RegistryCache: options.RegistryCache, + Tags: options.Tag, + }, nil } - return &config.ImageBuildInfo{ - Dockerfile: parsedDockerfile, - User: user, - Metadata: imageMetadataConfig, - }, nil + return nil, nil +} + +// buildImageParams groups the inputs for building a dockerfile-backed image. +type buildImageParams struct { + parsedConfig *config.SubstitutedConfig + substitutionContext *config.SubstitutionContext + buildInfo *config.ImageBuildInfo + extendedBuildInfo *feature.ExtendedBuildInfo + dockerfilePath string + dockerfileContent string + options provider.BuildOptions } func (r *runner) buildImage( ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - buildInfo *config.ImageBuildInfo, - extendedBuildInfo *feature.ExtendedBuildInfo, - dockerfilePath, - dockerfileContent string, - options provider.BuildOptions, + params *buildImageParams, ) (*config.BuildInfo, error) { + parsedConfig := params.parsedConfig + options := params.options + targetArch, err := r.Driver.TargetArchitecture(ctx, r.ID) if err != nil { return nil, err @@ -316,9 +426,9 @@ func (r *runner) buildImage( Platform: options.Platform, Architecture: targetArch, ContextPath: config.GetContextPath(parsedConfig.Config), - DockerfilePath: dockerfilePath, - DockerfileContent: dockerfileContent, - BuildInfo: buildInfo, + DockerfilePath: params.dockerfilePath, + DockerfileContent: params.dockerfileContent, + BuildInfo: params.buildInfo, }) if err != nil { return nil, err @@ -326,53 +436,41 @@ func (r *runner) buildImage( // check if there is a prebuild image if !options.ForceDockerless && !options.ForceBuild { - devsyCustomizations := config.GetDevsyCustomizations(parsedConfig.Config) - if options.Repository != "" { - options.PrebuildRepositories = append(options.PrebuildRepositories, options.Repository) + prebuilt, err := r.findPrebuildImage(ctx, &prebuildLookupParams{ + parsedConfig: parsedConfig, + extendedBuildInfo: params.extendedBuildInfo, + options: options, + prebuildHash: prebuildHash, + targetArch: targetArch, + }) + if err != nil { + return nil, err } - options.PrebuildRepositories = append( - options.PrebuildRepositories, - devsyCustomizations.PrebuildRepository...) - - log.Debugf( - "Try to find prebuild image %s in repositories %s", - prebuildHash, - strings.Join(options.PrebuildRepositories, ","), - ) - for _, prebuildRepo := range options.PrebuildRepositories { - prebuildImage := prebuildRepo + ":" + prebuildHash - img, err := image.GetImageForArch(ctx, prebuildImage, targetArch) - if err == nil && img != nil { - // prebuild image found - log.Infof("Found existing prebuilt image %s", prebuildImage) - - // inspect image - imageDetails, err := r.inspectImage(ctx, prebuildImage) - if err != nil { - return nil, fmt.Errorf("get image details: %w", err) - } - - return &config.BuildInfo{ - ImageDetails: imageDetails, - ImageMetadata: extendedBuildInfo.MetadataConfig, - ImageName: prebuildImage, - PrebuildHash: prebuildHash, - RegistryCache: options.RegistryCache, - Tags: options.Tag, - }, nil - } else if err != nil { - log.Debugf("Error trying to find prebuild image %s: %v", prebuildImage, err) - } + if prebuilt != nil { + return prebuilt, nil } } + return r.executeBuild(ctx, params, prebuildHash, targetArch) +} + +// executeBuild dispatches the actual image build to the appropriate backend: +// remote BuildKit (platform mode), the dockerless fallback (non-docker driver), +// or the docker driver. +func (r *runner) executeBuild( + ctx context.Context, + params *buildImageParams, + prebuildHash, targetArch string, +) (*config.BuildInfo, error) { + options := params.options + if options.CLIOptions.Platform.Enabled { buildInfo, err := buildkit.BuildRemote(ctx, buildkit.BuildRemoteOptions{ PrebuildHash: prebuildHash, - ParsedConfig: parsedConfig, - ExtendedBuildInfo: extendedBuildInfo, - DockerfilePath: dockerfilePath, - DockerfileContent: dockerfileContent, + ParsedConfig: params.parsedConfig, + ExtendedBuildInfo: params.extendedBuildInfo, + DockerfilePath: params.dockerfilePath, + DockerfileContent: params.dockerfileContent, LocalWorkspaceFolder: r.LocalWorkspaceFolder, Options: options, TargetArch: targetArch, @@ -394,23 +492,23 @@ func (r *runner) buildImage( ) } - return dockerlessFallback( - r.LocalWorkspaceFolder, - substitutionContext.ContainerWorkspaceFolder, - parsedConfig, - buildInfo, - extendedBuildInfo, - dockerfileContent, - options, - ) + return dockerlessFallback(&dockerlessFallbackParams{ + localWorkspaceFolder: r.LocalWorkspaceFolder, + containerWorkspaceFolder: params.substitutionContext.ContainerWorkspaceFolder, + parsedConfig: params.parsedConfig, + buildInfo: params.buildInfo, + extendedBuildInfo: params.extendedBuildInfo, + dockerfileContent: params.dockerfileContent, + options: options, + }) } return dockerDriver.BuildDevContainer(ctx, driver.BuildRequest{ PrebuildHash: prebuildHash, - ParsedConfig: parsedConfig, - ExtendedBuildInfo: extendedBuildInfo, - DockerfilePath: dockerfilePath, - DockerfileContent: dockerfileContent, + ParsedConfig: params.parsedConfig, + ExtendedBuildInfo: params.extendedBuildInfo, + DockerfilePath: params.dockerfilePath, + DockerfileContent: params.dockerfileContent, LocalWorkspaceFolder: r.LocalWorkspaceFolder, Options: options, }) @@ -427,64 +525,52 @@ func (r *runner) buildDevImageCompose( return nil, fmt.Errorf("find docker compose: %w", err) } - envFiles, err := r.getEnvFiles() - if err != nil { - return nil, fmt.Errorf("get env files: %w", err) - } - - composeFiles, err := r.getDockerComposeFilePaths(parsedConfig, envFiles) - if err != nil { - return nil, fmt.Errorf("get docker compose file paths: %w", err) - } - - var composeGlobalArgs []string - for _, configFile := range composeFiles { - composeGlobalArgs = append(composeGlobalArgs, "-f", configFile) - } - - for _, envFile := range envFiles { - composeGlobalArgs = append(composeGlobalArgs, "--env-file", envFile) - } - - log.Debugf("Loading docker compose project %+v", composeFiles) - project, err := compose.LoadDockerComposeProject(ctx, composeFiles, envFiles) + projFiles, err := r.dockerComposeProjectFiles(parsedConfig) if err != nil { - return nil, fmt.Errorf("load docker compose project: %w", err) + return nil, err } - project.Name = composeHelper.GetProjectName(r.ID) - log.Debugf("Loaded project %s", project.Name) - service := parsedConfig.Config.Service - composeService, err := project.GetService(service) + project, err := r.loadComposeProject(ctx, composeHelper, parsedConfig, projFiles) if err != nil { - return nil, fmt.Errorf( - "service %q configured in devcontainer.json not found in Docker Compose configuration", - service, - ) - } - - originalImageName := composeService.Image - if originalImageName == "" { - originalImageName, err = composeHelper.GetDefaultImage(project.Name, service) - if err != nil { - return nil, fmt.Errorf("get default image: %w", err) - } + return nil, err } - extendResult, err := r.buildAndExtendDockerCompose( - ctx, - parsedConfig, - substitutionContext, + composeService, originalImageName, err := resolveComposeServiceImage( project, composeHelper, - &composeService, - composeGlobalArgs, - options.FeatureSecretsFile, + parsedConfig.Config.Service, ) + if err != nil { + return nil, err + } + + extendResult, err := r.buildAndExtendDockerCompose(ctx, &buildAndExtendParams{ + parsedConfig: parsedConfig, + substitutionContext: substitutionContext, + project: project, + composeHelper: composeHelper, + composeService: &composeService, + globalArgs: projFiles.composeGlobalArgs, + featureSecretsFile: options.FeatureSecretsFile, + pull: options.Pull, + noCache: options.NoCache, + }) if err != nil { return nil, fmt.Errorf("build and extend docker-compose: %w", err) } + return r.composeBuildInfo(ctx, extendResult, originalImageName, options) +} + +// composeBuildInfo resolves the final image for a compose build and assembles +// the resulting BuildInfo. Compose builds do not compute a prebuild hash, so the +// image tag is used as the PrebuildHash fallback. +func (r *runner) composeBuildInfo( + ctx context.Context, + extendResult composeExtendResult, + originalImageName string, + options provider.BuildOptions, +) (*config.BuildInfo, error) { currentImageName := extendResult.buildImageName if currentImageName == "" { currentImageName = originalImageName @@ -495,9 +581,6 @@ func (r *runner) buildDevImageCompose( return nil, fmt.Errorf("inspect image: %w", err) } - // have a fallback value for PrebuildHash - // we don't calculate prebuild hash on docker compose builds - // let's use Images :tag then imageTag, err := r.getImageTag(ctx, imageDetails.ID) if err != nil { return nil, fmt.Errorf("inspect image: %w", err) @@ -506,22 +589,29 @@ func (r *runner) buildDevImageCompose( return &config.BuildInfo{ ImageDetails: imageDetails, ImageMetadata: extendResult.imageMetadata, - ImageName: extendResult.buildImageName, + ImageName: currentImageName, PrebuildHash: imageTag, RegistryCache: options.RegistryCache, Tags: options.Tag, }, nil } -func dockerlessFallback( - localWorkspaceFolder, - containerWorkspaceFolder string, - parsedConfig *config.SubstitutedConfig, - buildInfo *config.ImageBuildInfo, - extendedBuildInfo *feature.ExtendedBuildInfo, - dockerfileContent string, - options provider.BuildOptions, -) (*config.BuildInfo, error) { +// dockerlessFallbackParams groups the inputs for the dockerless build fallback. +type dockerlessFallbackParams struct { + localWorkspaceFolder string + containerWorkspaceFolder string + parsedConfig *config.SubstitutedConfig + buildInfo *config.ImageBuildInfo + extendedBuildInfo *feature.ExtendedBuildInfo + dockerfileContent string + options provider.BuildOptions +} + +func dockerlessFallback(params *dockerlessFallbackParams) (*config.BuildInfo, error) { + parsedConfig := params.parsedConfig + extendedBuildInfo := params.extendedBuildInfo + options := params.options + contextPath := config.GetContextPath(parsedConfig.Config) devsyInternalFolder := filepath.Join(contextPath, config.DevsyContextFeatureFolder) // #nosec G301 -- TODO Consider using a more secure permission setting and ownership if needed. @@ -531,12 +621,12 @@ func dockerlessFallback( } // build dockerfile - devsyDockerfile, err := build.RewriteDockerfile(dockerfileContent, extendedBuildInfo) + devsyDockerfile, err := build.RewriteDockerfile(params.dockerfileContent, extendedBuildInfo) if err != nil { return nil, fmt.Errorf("rewrite dockerfile: %w", err) } else if devsyDockerfile == "" { devsyDockerfile = filepath.Join(devsyInternalFolder, "Dockerfile-without-features") - err = os.WriteFile(devsyDockerfile, []byte(dockerfileContent), 0o600) + err = os.WriteFile(devsyDockerfile, []byte(params.dockerfileContent), 0o600) if err != nil { return nil, fmt.Errorf("write devsy dockerfile: %w", err) } @@ -544,8 +634,8 @@ func dockerlessFallback( // get build args and target containerContext, containerDockerfile := getContainerContextAndDockerfile( - localWorkspaceFolder, - containerWorkspaceFolder, + params.localWorkspaceFolder, + params.containerWorkspaceFolder, contextPath, devsyDockerfile, ) @@ -559,7 +649,7 @@ func dockerlessFallback( BuildArgs: buildArgs, Target: target, - User: buildInfo.User, + User: params.buildInfo.User, }, RegistryCache: options.RegistryCache, Tags: options.Tag, diff --git a/pkg/devcontainer/build/options.go b/pkg/devcontainer/build/options.go index 39eb5c1e3..2eb2c87f7 100644 --- a/pkg/devcontainer/build/options.go +++ b/pkg/devcontainer/build/options.go @@ -52,8 +52,10 @@ type BuildOptions struct { Push bool // Upload controls whether to upload the build context. Used for remote builds. Upload bool - // NoCache disables the Docker build cache entirely. + // NoCache disables the Docker build cache entirely (--no-cache). NoCache bool + // Pull always attempts to pull a newer version of base images (--pull). + Pull bool } // NewOptionsParams contains the parameters needed to create BuildOptions. @@ -96,6 +98,7 @@ func NewOptions(params NewOptionsParams) (*BuildOptions, error) { // directly to the registry. This is mutually exclusive with Load. Push: params.Options.PushDuringBuild, NoCache: params.Options.NoCache, + Pull: params.Options.Pull, } // get build args and target diff --git a/pkg/devcontainer/compose.go b/pkg/devcontainer/compose.go index 157947ac1..cbcae5484 100644 --- a/pkg/devcontainer/compose.go +++ b/pkg/devcontainer/compose.go @@ -6,28 +6,24 @@ import ( "os" "path" "path/filepath" - "reflect" - "regexp" - "strconv" "strings" - "time" composetypes "github.com/compose-spec/compose-go/v2/types" "github.com/devsy-org/devsy/pkg/compose" "github.com/devsy-org/devsy/pkg/devcontainer/config" - "github.com/devsy-org/devsy/pkg/devcontainer/feature" "github.com/devsy-org/devsy/pkg/devcontainer/metadata" - "github.com/devsy-org/devsy/pkg/dockerfile" "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/log" "github.com/joho/godotenv" - "gopkg.in/yaml.v3" ) const ( ConfigFilesLabel = "com.docker.compose.project.config_files" FeaturesBuildOverrideFilePrefix = "docker-compose.devcontainer.build" FeaturesStartOverrideFilePrefix = "docker-compose.devcontainer.containerFeatures" + + containerStatusRunning = "running" + composeProjectNameFlag = "--project-name" ) type composeProjectFiles struct { @@ -55,6 +51,49 @@ type persistedFileResult struct { filePath string } +// startContainerParams groups the inputs for starting (or recreating) the +// compose dev container. +type startContainerParams struct { + parsedConfig *config.SubstitutedConfig + substitutionContext *config.SubstitutionContext + project *composetypes.Project + composeHelper *compose.ComposeHelper + composeGlobalArgs []string + container *config.ContainerDetails + options UpOptions +} + +// buildAndExtendParams groups the inputs for building and feature-extending a +// compose service. +type buildAndExtendParams struct { + parsedConfig *config.SubstitutedConfig + substitutionContext *config.SubstitutionContext + project *composetypes.Project + composeHelper *compose.ComposeHelper + composeService *composetypes.ServiceConfig + globalArgs []string + featureSecretsFile string + // pull re-pulls base images during the compose build (--pull), set from + // CLIOptions.Pull. + pull bool + // noCache disables the build cache during the compose build (--no-cache), + // set from CLIOptions.NoCache. + noCache bool +} + +// composeUpParams groups the inputs shared by extendedDockerComposeUp and +// generateDockerComposeUpProject for producing the compose "up" override. +type composeUpParams struct { + parsedConfig *config.SubstitutedConfig + mergedConfig *config.MergedDevContainerConfig + composeHelper *compose.ComposeHelper + composeService *composetypes.ServiceConfig + originalImageName string + overrideImageName string + imageDetails *config.ImageDetails + additionalLabels map[string]string +} + func (r *runner) composeHelper() (*compose.ComposeHelper, error) { dockerDriver, ok := r.Driver.(driver.DockerDriver) if !ok { @@ -121,10 +160,7 @@ func (r *runner) deleteDockerCompose( func (r *runner) dockerComposeProjectFiles( parsedConfig *config.SubstitutedConfig, ) (composeProjectFiles, error) { - envFiles, err := r.getEnvFiles() - if err != nil { - return composeProjectFiles{}, fmt.Errorf("get env files: %w", err) - } + envFiles := r.getEnvFiles() composeFiles, err := r.getDockerComposeFilePaths(parsedConfig, envFiles) if err != nil { @@ -149,11 +185,10 @@ func (r *runner) dockerComposeProjectFiles( func (r *runner) runDockerCompose( ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - options UpOptions, - timeout time.Duration, + runParams *runContainerParams, ) (*config.Result, error) { + parsedConfig := runParams.parsedConfig + composeHelper, err := r.composeHelper() if err != nil { return nil, fmt.Errorf("find docker compose: %w", err) @@ -163,8 +198,33 @@ func (r *runner) runDockerCompose( if err != nil { return nil, fmt.Errorf("get compose/env files: %w", err) } - composeGlobalArgs := projFiles.composeGlobalArgs + project, err := r.loadComposeProject(ctx, composeHelper, parsedConfig, projFiles) + if err != nil { + return nil, err + } + + containerDetails, err := r.ensureComposeContainer(ctx, &composeContainerParams{ + runParams: runParams, + composeHelper: composeHelper, + project: project, + composeGlobalArgs: projFiles.composeGlobalArgs, + }) + if err != nil { + return nil, err + } + + return r.finalizeComposeContainer(ctx, runParams, project, containerDetails) +} + +// loadComposeProject loads the docker compose project from the resolved compose +// and env files and names it after the workspace. +func (r *runner) loadComposeProject( + ctx context.Context, + composeHelper *compose.ComposeHelper, + parsedConfig *config.SubstitutedConfig, + projFiles composeProjectFiles, +) (*composetypes.Project, error) { log.Debugf("Loading docker compose project %+v", projFiles.composeFiles) project, err := compose.LoadDockerComposeProject( ctx, @@ -181,6 +241,30 @@ func (r *runner) runDockerCompose( return nil, err } + return project, nil +} + +// composeContainerParams groups the inputs for ensuring a running compose dev +// container. +type composeContainerParams struct { + runParams *runContainerParams + composeHelper *compose.ComposeHelper + project *composetypes.Project + composeGlobalArgs []string +} + +// ensureComposeContainer finds the dev container and, when it is missing, +// stopped, or being recreated, starts it (reusing persisted project files when +// possible). It returns the resolved container details. +func (r *runner) ensureComposeContainer( + ctx context.Context, + params *composeContainerParams, +) (*config.ContainerDetails, error) { + parsedConfig := params.runParams.parsedConfig + options := params.runParams.options + composeHelper := params.composeHelper + project := params.project + containerDetails, err := composeHelper.FindDevContainer( ctx, project.Name, @@ -190,74 +274,54 @@ func (r *runner) runDockerCompose( return nil, fmt.Errorf("find dev container: %w", err) } - // does the container already exist or is it not running? - if containerDetails == nil || containerDetails.State.Status != "running" || options.Recreate { - didStartProject := false - // Try to find existing project first - existingProjectFiles, err := composeHelper.FindProjectFiles(ctx, project.Name) - if err != nil { - log.Errorf("Error finding project files: %s", err) - } else if len(existingProjectFiles) > 0 && !options.Recreate { - log.Debugf("Found existing project files: %s", existingProjectFiles) - // make sure all project files are still available - for _, file := range existingProjectFiles { - if _, err := os.Stat(file); err != nil { - log.Warnf("Project file %s does not exist anymore, recreating project", file) - containerDetails = nil - break - } - } - - // If project is found, we can call `up` with the project name - // If it fails, fall back to rebuilding - upArgs := []string{"--project-name", project.Name} - for _, existingProjectFiles := range existingProjectFiles { - upArgs = append(upArgs, "-f", existingProjectFiles) - } - upArgs = append(upArgs, "up", "-d") - upArgs = r.onlyRunServices(upArgs, parsedConfig) - - // Run docker-compose - writer := log.Writer(log.LevelInfo) - err = composeHelper.Run(ctx, upArgs, nil, writer, writer) - if err != nil { - log.Errorf("Error starting project: %s", err) - } else { - // wait for running and get container details - details, err := composeHelper.FindDevContainer( - ctx, - project.Name, - parsedConfig.Config.Service, - ) - if err != nil { - log.Errorf("Error finding dev container: %s", err) - } else { - containerDetails = details - didStartProject = true - } - } - } + // container already exists and is running, nothing to do + if containerDetails != nil && containerDetails.State.Status == containerStatusRunning && + !options.Recreate { + return containerDetails, nil + } - // Start container if not running - if !didStartProject { - containerDetails, err = r.startContainer( - ctx, - parsedConfig, - substitutionContext, - project, - composeHelper, - composeGlobalArgs, - containerDetails, - options, - ) - if err != nil { - return nil, fmt.Errorf("start container: %w", err) - } else if containerDetails == nil { - return nil, fmt.Errorf("couldn't find container after start") - } - } + containerDetails, didStartProject := r.tryStartExistingProject(ctx, &existingProjectParams{ + parsedConfig: parsedConfig, + composeHelper: composeHelper, + project: project, + container: containerDetails, + recreate: options.Recreate, + }) + if didStartProject { + return containerDetails, nil + } + + containerDetails, err = r.startContainer(ctx, &startContainerParams{ + parsedConfig: parsedConfig, + substitutionContext: params.runParams.substitutionContext, + project: project, + composeHelper: composeHelper, + composeGlobalArgs: params.composeGlobalArgs, + container: containerDetails, + options: options, + }) + if err != nil { + return nil, fmt.Errorf("start container: %w", err) + } + if containerDetails == nil { + return nil, fmt.Errorf("couldn't find container after start") } + return containerDetails, nil +} + +// finalizeComposeContainer merges the container's metadata config and sets up +// the running container. +func (r *runner) finalizeComposeContainer( + ctx context.Context, + runParams *runContainerParams, + project *composetypes.Project, + containerDetails *config.ContainerDetails, +) (*config.Result, error) { + parsedConfig := runParams.parsedConfig + substitutionContext := runParams.substitutionContext + options := runParams.options + imageMetadataConfig, err := metadata.GetImageMetadataFromContainer( containerDetails, substitutionContext, @@ -266,38 +330,16 @@ func (r *runner) runDockerCompose( return nil, fmt.Errorf("get image metadata from container: %w", err) } - if dockerDriver, ok := r.Driver.(driver.DockerDriver); ok { - err = dockerDriver.UpdateContainerUserUID( - ctx, - r.ID, - parsedConfig.Config, - log.Writer(log.LevelInfo), - ) - if err != nil { - log.Errorf("failed to update container user UID/GID: error=%v", err) - return nil, err - } - } - - if options.ExtraDevContainerPath != "" { - if imageMetadataConfig == nil { - imageMetadataConfig = &config.ImageMetadataConfig{} - } - extraConfig, parseErr := config.ParseDevContainerJSONFile(options.ExtraDevContainerPath) - if parseErr != nil { - return nil, parseErr - } - config.AddConfigToImageMetadata(extraConfig, imageMetadataConfig) + if err := r.updateContainerUserUID(ctx, parsedConfig); err != nil { + return nil, err } - mergedConfig, err := config.MergeConfiguration(parsedConfig.Config, imageMetadataConfig.Config) + mergedConfig, err := mergeImageMetadataConfig( + parsedConfig, + imageMetadataConfig, + options.ExtraDevContainerPath, + ) if err != nil { - return nil, fmt.Errorf("merge config: %w", err) - } - - if err := config.MergeExtraRemoteEnv( - mergedConfig, options.ExtraDevContainerPath, - ); err != nil { return nil, err } @@ -310,6 +352,52 @@ func (r *runner) runDockerCompose( composeAlias := project.Name mergedConfig.RemoteEnv["COMPOSE_PROJECT_NAME"] = &composeAlias + hostWarnings, err := r.composeHostWarnings(parsedConfig, substitutionContext, options) + if err != nil { + return nil, err + } + + return r.setupContainer(ctx, &setupContainerParams{ + rawConfig: parsedConfig.Raw, + containerDetails: containerDetails, + mergedConfig: mergedConfig, + substitutionContext: substitutionContext, + timeout: runParams.timeout, + hostWarnings: hostWarnings, + }) +} + +// updateContainerUserUID updates the container user's UID/GID on Docker drivers. +func (r *runner) updateContainerUserUID( + ctx context.Context, + parsedConfig *config.SubstitutedConfig, +) error { + dockerDriver, ok := r.Driver.(driver.DockerDriver) + if !ok { + return nil + } + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + if err := dockerDriver.UpdateContainerUserUID( + ctx, + r.ID, + parsedConfig.Config, + writer, + ); err != nil { + log.Errorf("failed to update container user UID/GID: error=%v", err) + return err + } + return nil +} + +// composeHostWarnings validates host requirements, returning warnings. When the +// requirements are unmet it errors unless SkipHostRequirements is set, in which +// case the error is downgraded to a warning. +func (r *runner) composeHostWarnings( + parsedConfig *config.SubstitutedConfig, + substitutionContext *config.SubstitutionContext, + options UpOptions, +) ([]string, error) { hostWarnings, hostErr := config.ValidateHostRequirements( parsedConfig.Config.HostRequirements, config.SystemHostInfo{}, @@ -321,15 +409,104 @@ func (r *runner) runDockerCompose( } hostWarnings = append(hostWarnings, hostErr.Error()) } + return hostWarnings, nil +} - return r.setupContainer(ctx, &setupContainerParams{ - rawConfig: parsedConfig.Raw, - containerDetails: containerDetails, - mergedConfig: mergedConfig, - substitutionContext: substitutionContext, - timeout: timeout, - hostWarnings: hostWarnings, - }) +// existingProjectParams groups the inputs for attempting to start a compose +// project from previously persisted project files. +type existingProjectParams struct { + parsedConfig *config.SubstitutedConfig + composeHelper *compose.ComposeHelper + project *composetypes.Project + container *config.ContainerDetails + recreate bool +} + +// tryStartExistingProject attempts to bring up the dev container from project +// files discovered for an existing compose project, avoiding a full rebuild. It +// returns the (possibly updated) container details and whether the project was +// started. A false return means the caller should fall back to startContainer. +func (r *runner) tryStartExistingProject( + ctx context.Context, + params *existingProjectParams, +) (*config.ContainerDetails, bool) { + composeHelper := params.composeHelper + project := params.project + containerDetails := params.container + + existingProjectFiles, err := composeHelper.FindProjectFiles(ctx, project.Name) + if err != nil { + log.Errorf("Error finding project files: %s", err) + return containerDetails, false + } + if len(existingProjectFiles) == 0 || params.recreate { + return containerDetails, false + } + + log.Debugf("Found existing project files: %s", existingProjectFiles) + if !allProjectFilesExist(existingProjectFiles) { + // A referenced file is gone, so `compose up -f ` would only + // fail; rebuild from scratch instead. + return containerDetails, false + } + + // The project files are present, so `up` can reuse them. If it fails, the + // caller falls back to rebuilding. + details, err := r.composeUpExistingProject(ctx, params, existingProjectFiles) + if err != nil || details == nil { + // Compose failed, or reported success but the dev container is not + // present; fall back to a full start rather than finalizing nil. + return containerDetails, false + } + + return details, true +} + +// composeUpExistingProject runs "compose up" using the persisted project files +// and returns the resulting dev container details. +func (r *runner) composeUpExistingProject( + ctx context.Context, + params *existingProjectParams, + existingProjectFiles []string, +) (*config.ContainerDetails, error) { + upArgs := []string{composeProjectNameFlag, params.project.Name} + for _, projectFile := range existingProjectFiles { + upArgs = append(upArgs, "-f", projectFile) + } + upArgs = append(upArgs, "up", "-d") + upArgs = r.onlyRunServices(upArgs, params.parsedConfig) + + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + if err := params.composeHelper.Run(ctx, upArgs, nil, writer, writer); err != nil { + log.Errorf("Error starting project: %s", err) + return nil, err + } + + // wait for running and get container details + details, err := params.composeHelper.FindDevContainer( + ctx, + params.project.Name, + params.parsedConfig.Config.Service, + ) + if err != nil { + log.Errorf("Error finding dev container: %s", err) + return nil, err + } + + return details, nil +} + +// allProjectFilesExist reports whether every persisted project file is still +// present on disk; a missing file means the project must be recreated. +func allProjectFilesExist(projectFiles []string) bool { + for _, file := range projectFiles { + if _, err := os.Stat(file); err != nil { + log.Warnf("Project file %s does not exist anymore, recreating project", file) + return false + } + } + return true } func validateRunServices(runServices []string, project *composetypes.Project) error { @@ -368,71 +545,81 @@ func (r *runner) getDockerComposeFilePaths( parsedConfig *config.SubstitutedConfig, envFiles []string, ) ([]string, error) { - configFileDir := filepath.Dir(parsedConfig.Config.Origin) - - // Use docker compose files from config - var composeFiles []string + // Prefer docker compose files declared in the devcontainer config. if len(parsedConfig.Config.DockerComposeFile) > 0 { - for _, composeFile := range parsedConfig.Config.DockerComposeFile { - absPath := composeFile - if !filepath.IsAbs(composeFile) { - absPath = filepath.Join(configFileDir, composeFile) - } - composeFiles = append(composeFiles, absPath) - } - - return composeFiles, nil + return absoluteComposeFiles( + filepath.Dir(parsedConfig.Config.Origin), + parsedConfig.Config.DockerComposeFile, + ), nil } - // Use docker compose files from $COMPOSE_FILE environment variable - envComposeFile := os.Getenv("COMPOSE_FILE") + // Otherwise fall back to $COMPOSE_FILE from the environment or .env files. + envComposeFile, err := composeFileFromEnv(envFiles) + if err != nil { + return nil, err + } + if envComposeFile != "" { + return filepath.SplitList(envComposeFile), nil + } - // Load docker compose files from $COMPOSE_FILE in .env file - if envComposeFile == "" { - for _, envFile := range envFiles { - env, err := godotenv.Read(envFile) - if err != nil { - return nil, err - } + return nil, nil +} - if env["COMPOSE_FILE"] != "" { - envComposeFile = env["COMPOSE_FILE"] - break - } +// absoluteComposeFiles resolves each compose file path against configFileDir +// unless it is already absolute. +func absoluteComposeFiles(configFileDir string, composeFiles []string) []string { + resolved := make([]string, 0, len(composeFiles)) + for _, composeFile := range composeFiles { + absPath := composeFile + if !filepath.IsAbs(composeFile) { + absPath = filepath.Join(configFileDir, composeFile) } + resolved = append(resolved, absPath) } + return resolved +} - if envComposeFile != "" { - return filepath.SplitList(envComposeFile), nil +// composeFileFromEnv returns the $COMPOSE_FILE value, preferring the process +// environment and falling back to the first .env file that defines it. +func composeFileFromEnv(envFiles []string) (string, error) { + if envComposeFile := os.Getenv("COMPOSE_FILE"); envComposeFile != "" { + return envComposeFile, nil } - return nil, nil + for _, envFile := range envFiles { + env, err := godotenv.Read(envFile) + if err != nil { + return "", err + } + if env["COMPOSE_FILE"] != "" { + return env["COMPOSE_FILE"], nil + } + } + + return "", nil } -func (r *runner) getEnvFiles() ([]string, error) { +func (r *runner) getEnvFiles() []string { var envFiles []string envFile := path.Join(r.LocalWorkspaceFolder, ".env") envFileStat, err := os.Stat(envFile) if err == nil && envFileStat.Mode().IsRegular() { envFiles = append(envFiles, envFile) } - return envFiles, nil + return envFiles } -func (r *runner) startContainer( - ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, +// resolveComposeServiceImage looks up the named devcontainer service in the +// compose project and determines its original image name, falling back to the +// compose default image when the service does not declare one. +func resolveComposeServiceImage( project *composetypes.Project, composeHelper *compose.ComposeHelper, - composeGlobalArgs []string, - container *config.ContainerDetails, - options UpOptions, -) (*config.ContainerDetails, error) { - service := parsedConfig.Config.Service + service string, +) (composetypes.ServiceConfig, string, error) { composeService, err := project.GetService(service) if err != nil { - return nil, fmt.Errorf( + return composetypes.ServiceConfig{}, "", fmt.Errorf( "service %q configured in devcontainer.json not found in Docker Compose configuration", service, ) @@ -442,894 +629,294 @@ func (r *runner) startContainer( if originalImageName == "" { originalImageName, err = composeHelper.GetDefaultImage(project.Name, service) if err != nil { - return nil, fmt.Errorf("get default image: %w", err) + return composetypes.ServiceConfig{}, "", fmt.Errorf("get default image: %w", err) } } - var didRestoreFromPersistedShare bool - if container != nil { - labels := container.Config.Labels - if labels[ConfigFilesLabel] != "" { - configFiles := strings.Split(labels[ConfigFilesLabel], ",") - - persistedBuildFile := checkForPersistedFile( - configFiles, - FeaturesBuildOverrideFilePrefix, - ) - - persistedStartFile := checkForPersistedFile( - configFiles, - FeaturesStartOverrideFilePrefix, - ) - - if (persistedBuildFile.fileExists || !persistedBuildFile.foundLabel) && - persistedStartFile.fileExists { - didRestoreFromPersistedShare = true - - if persistedBuildFile.fileExists { - composeGlobalArgs = append(composeGlobalArgs, "-f", persistedBuildFile.filePath) - } + return composeService, originalImageName, nil +} - if persistedStartFile.fileExists { - composeGlobalArgs = append(composeGlobalArgs, "-f", persistedStartFile.filePath) - } - } - } +func (r *runner) startContainer( + ctx context.Context, + params *startContainerParams, +) (*config.ContainerDetails, error) { + parsedConfig := params.parsedConfig + project := params.project + composeHelper := params.composeHelper + composeGlobalArgs := params.composeGlobalArgs + container := params.container + options := params.options + + composeService, originalImageName, err := resolveComposeServiceImage( + project, + composeHelper, + parsedConfig.Config.Service, + ) + if err != nil { + return nil, err } - if container == nil || !didRestoreFromPersistedShare { - extendResult, err := r.buildAndExtendDockerCompose( - ctx, - parsedConfig, - substitutionContext, - project, - composeHelper, - &composeService, - composeGlobalArgs, - options.FeatureSecretsFile, - ) - if err != nil { - return nil, fmt.Errorf("build and extend docker-compose: %w", err) - } - - if extendResult.composeBuildFilePath != "" { - composeGlobalArgs = append(composeGlobalArgs, "-f", extendResult.composeBuildFilePath) - } - - currentImageName := extendResult.buildImageName - if currentImageName == "" { - currentImageName = originalImageName - } - - imageDetails, err := r.inspectImage(ctx, currentImageName) - if err != nil { - return nil, fmt.Errorf("inspect image: %w", err) - } - - if options.ExtraDevContainerPath != "" { - if extendResult.imageMetadata == nil { - extendResult.imageMetadata = &config.ImageMetadataConfig{} - } - extraConfig, parseErr := config.ParseDevContainerJSONFile(options.ExtraDevContainerPath) - if parseErr != nil { - return nil, parseErr - } - config.AddConfigToImageMetadata(extraConfig, extendResult.imageMetadata) - } + composeGlobalArgs, didRestoreFromPersistedShare := restorePersistedComposeArgs( + container, + composeGlobalArgs, + ) - mergedConfig, err := config.MergeConfiguration( - parsedConfig.Config, - extendResult.imageMetadata.Config, - ) + if container == nil || !didRestoreFromPersistedShare { + composeGlobalArgs, err = r.buildComposeOverrideArgs(ctx, &composeOverrideParams{ + startParams: params, + composeService: &composeService, + originalImageName: originalImageName, + composeGlobalArgs: composeGlobalArgs, + }) if err != nil { - return nil, fmt.Errorf("merge configuration: %w", err) - } - - if err := config.MergeExtraRemoteEnv( - mergedConfig, options.ExtraDevContainerPath, - ); err != nil { return nil, err } - - additionalLabels := map[string]string{ - metadata.ImageMetadataLabel: extendResult.metadataLabel, - config.UserLabel: imageDetails.Config.User, - } - overrideComposeUpFilePath, err := r.extendedDockerComposeUp( - parsedConfig, - mergedConfig, - composeHelper, - &composeService, - originalImageName, - extendResult.buildImageName, - imageDetails, - additionalLabels, - ) - if err != nil { - return nil, fmt.Errorf("extend docker-compose up: %w", err) - } - - if overrideComposeUpFilePath != "" { - composeGlobalArgs = append(composeGlobalArgs, "-f", overrideComposeUpFilePath) - } } if container != nil && options.Recreate { - log.Debugf("Deleting dev container %s due to --recreate", container.ID) - - if err := r.Driver.StopDevContainer(ctx, r.ID); err != nil { - return nil, fmt.Errorf("stop dev container: %w", err) - } - - if err := r.Driver.DeleteDevContainer(ctx, r.ID); err != nil { - return nil, fmt.Errorf("delete dev container: %w", err) + if err := r.recreateDevContainer(ctx, container); err != nil { + return nil, err } } - upArgs := []string{"--project-name", project.Name} - upArgs = append(upArgs, composeGlobalArgs...) - upArgs = append(upArgs, "up", "-d") - if container != nil { - upArgs = append(upArgs, "--no-recreate") - } - upArgs = r.onlyRunServices(upArgs, parsedConfig) - - // start compose - writer := log.Writer(log.LevelInfo) - defer func() { _ = writer.Close() }() - err = composeHelper.Run(ctx, upArgs, nil, writer, writer) - if err != nil { - return nil, fmt.Errorf("docker-compose run: %w", err) - } - - // TODO wait for started event? - containerDetails, err := composeHelper.FindDevContainer(ctx, project.Name, composeService.Name) - if err != nil { - return nil, fmt.Errorf("find dev container: %w", err) - } - - return containerDetails, nil + return r.composeUpAndFindContainer(ctx, &composeUpRunParams{ + project: project, + composeService: &composeService, + composeHelper: composeHelper, + composeGlobalArgs: composeGlobalArgs, + parsedConfig: parsedConfig, + hasExistingContainer: container != nil, + }) } -// prepareComposeBuildInfo modifies a compose project's devcontainer Dockerfile -// to ensure it can be extended with features. If an Image is specified instead -// of a Build, the metadata from the Image is used to populate the build info. -func (r *runner) prepareComposeBuildInfo( - ctx context.Context, - subCtx *config.SubstitutionContext, - composeService *composetypes.ServiceConfig, - buildTarget string, -) (composeBuildInfo, error) { - var dockerFilePath, dockerfileContents string - var imageBuildInfo *config.ImageBuildInfo - var err error - if composeService.Build != nil { - // Read Dockerfile - if path.IsAbs(composeService.Build.Dockerfile) { - dockerFilePath = composeService.Build.Dockerfile - } else { - dockerFilePath = filepath.Join( - composeService.Build.Context, - composeService.Build.Dockerfile, - ) - } - - originalDockerfile, err := os.ReadFile(dockerFilePath) - if err != nil { - return composeBuildInfo{}, err - } - - // Determine build target. If a multi stage build is used, ensure it is - // valid and modify the Dockerfile if necessary. - originalTarget := composeService.Build.Target - if originalTarget != "" { - buildTarget = originalTarget - } else { - lastStageName, modifiedDockerfile, err := dockerfile.EnsureFinalStageName( - string(originalDockerfile), - config.DockerfileDefaultTarget, - ) - if err != nil { - return composeBuildInfo{}, err - } - - buildTarget = lastStageName - // Override Dockerfile if it was modified, otherwise use the original - if modifiedDockerfile != "" { - dockerfileContents = modifiedDockerfile - } else { - dockerfileContents = string(originalDockerfile) - } - } - imageBuildInfo, err = r.getImageBuildInfoFromDockerfile( - subCtx, - string(originalDockerfile), - mappingToMap(composeService.Build.Args), - originalTarget, - ) - if err != nil { - return composeBuildInfo{}, err - } - } else { - imageBuildInfo, err = r.getImageBuildInfoFromImage(ctx, subCtx, composeService.Image) - if err != nil { - return composeBuildInfo{}, err - } +// restorePersistedComposeArgs detects persisted feature override files recorded +// on an existing container and appends them as additional compose "-f" args. +// It reports whether a usable persisted share was found. +func restorePersistedComposeArgs( + container *config.ContainerDetails, + composeGlobalArgs []string, +) ([]string, bool) { + if container == nil { + return composeGlobalArgs, false } - return composeBuildInfo{ - imageBuildInfo: imageBuildInfo, - dockerfileContents: dockerfileContents, - buildTarget: buildTarget, - }, nil -} -// This extends the build information for docker compose containers. -func (r *runner) buildAndExtendDockerCompose( - ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - project *composetypes.Project, - composeHelper *compose.ComposeHelper, - composeService *composetypes.ServiceConfig, - globalArgs []string, - featureSecretsFile string, -) (composeExtendResult, error) { - var dockerFilePath, dockerfileContents, dockerComposeFilePath string - var imageBuildInfo *config.ImageBuildInfo - var err error - - buildTarget := "dev_container_auto_added_stage_label" - - // Determine base imageName for generated features build - buildInfo, err := r.prepareComposeBuildInfo( - ctx, - substitutionContext, - composeService, - buildTarget, - ) - if err != nil { - return composeExtendResult{}, err - } - imageBuildInfo = buildInfo.imageBuildInfo - dockerfileContents = buildInfo.dockerfileContents - buildTarget = buildInfo.buildTarget - - secretOpts := &feature.SecretOptions{ - SecretsFile: featureSecretsFile, - Prompter: &feature.TerminalSecretPrompter{}, - } - extendImageBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildParams{ - Ctx: substitutionContext, - ImageBuildInfo: imageBuildInfo, - Target: buildTarget, - DevContainerConfig: parsedConfig, - ForceBuild: false, - SecretOpts: secretOpts, - }) - if err != nil { - return composeExtendResult{}, err + labels := container.Config.Labels + if labels[ConfigFilesLabel] == "" { + return composeGlobalArgs, false } - hasFeatures := extendImageBuildInfo != nil && extendImageBuildInfo.FeaturesBuildInfo != nil - buildImageName, err := composeBuildImageName( - composeHelper, - project.Name, - composeService, - hasFeatures, - ) - if err != nil { - return composeExtendResult{}, err - } - - if hasFeatures { - // If the dockerfile is empty (because an Image was used), reference that - // image as the build target after the features / modified contents. - if dockerfileContents == "" { - if composeService.Image == "" && composeService.Build == nil { - return composeExtendResult{}, fmt.Errorf( - "compose service %q has no image or build configuration", - composeService.Name, - ) - } - sanitizedImage := strings.ReplaceAll( - strings.ReplaceAll(composeService.Image, "\n", ""), - "\r", - "", - ) - dockerfileContents = fmt.Sprintf("FROM %s AS %s\n", sanitizedImage, buildTarget) - } - - // Write the final Dockerfile with features - extendedDockerfilePath, extendedDockerfileContent := r.extendedDockerfile( - extendImageBuildInfo.FeaturesBuildInfo, - dockerFilePath, - dockerfileContents, - ) - - log.Debugf( - "Creating extended Dockerfile %s with content: \n %s", - extendedDockerfilePath, - extendedDockerfileContent, - ) - - defer func() { _ = os.RemoveAll(filepath.Dir(extendedDockerfilePath)) }() + configFiles := strings.Split(labels[ConfigFilesLabel], ",") + persistedBuildFile := checkForPersistedFile(configFiles, FeaturesBuildOverrideFilePrefix) + persistedStartFile := checkForPersistedFile(configFiles, FeaturesStartOverrideFilePrefix) - // Write the final docker-compose referencing the modified Dockerfile or Image - dockerComposeFilePath, err = r.extendedDockerComposeBuild( - composeService, - buildImageName, - extendedDockerfilePath, - extendedDockerfileContent, - extendImageBuildInfo.FeaturesBuildInfo, - ) - if err != nil { - return composeExtendResult{buildImageName: buildImageName}, err - } + usablePersistedShare := (persistedBuildFile.fileExists || !persistedBuildFile.foundLabel) && + persistedStartFile.fileExists + if !usablePersistedShare { + return composeGlobalArgs, false } - // Prepare the docker-compose build arguments - buildArgs := []string{"--project-name", project.Name} - buildArgs = append(buildArgs, globalArgs...) - if dockerComposeFilePath != "" { - buildArgs = append(buildArgs, "-f", dockerComposeFilePath) + if persistedBuildFile.fileExists { + composeGlobalArgs = append(composeGlobalArgs, "-f", persistedBuildFile.filePath) } - buildArgs = append(buildArgs, "build") - if extendImageBuildInfo == nil { - buildArgs = append(buildArgs, "--pull") + if persistedStartFile.fileExists { + composeGlobalArgs = append(composeGlobalArgs, "-f", persistedStartFile.filePath) } + return composeGlobalArgs, true +} - // Only run the services defined in .devcontainer.json runServices - if len(parsedConfig.Config.RunServices) > 0 { - buildArgs = append(buildArgs, composeService.Name) - for _, service := range parsedConfig.Config.RunServices { - if service == composeService.Name { - continue - } - buildArgs = append(buildArgs, service) - } - } +// composeOverrideParams groups the inputs for building the feature build/up +// override files and the resulting compose "-f" arguments. +type composeOverrideParams struct { + startParams *startContainerParams + composeService *composetypes.ServiceConfig + originalImageName string + composeGlobalArgs []string +} - // build image - writer := log.Writer(log.LevelInfo) - defer func() { _ = writer.Close() }() - log.Debugf("Run %s %s", composeHelper.Command, strings.Join(buildArgs, " ")) - err = composeHelper.Run(ctx, buildArgs, nil, writer, writer) +// buildComposeOverrideArgs builds and feature-extends the compose project, then +// generates the "up" override, returning composeGlobalArgs extended with any +// generated override files. +func (r *runner) buildComposeOverrideArgs( + ctx context.Context, + params *composeOverrideParams, +) ([]string, error) { + start := params.startParams + composeGlobalArgs := params.composeGlobalArgs + + extendResult, err := r.buildAndExtendDockerCompose(ctx, &buildAndExtendParams{ + parsedConfig: start.parsedConfig, + substitutionContext: start.substitutionContext, + project: start.project, + composeHelper: start.composeHelper, + composeService: params.composeService, + globalArgs: composeGlobalArgs, + featureSecretsFile: start.options.FeatureSecretsFile, + pull: start.options.Pull, + noCache: start.options.NoCache, + }) if err != nil { - return composeExtendResult{buildImageName: buildImageName}, err + return nil, fmt.Errorf("build and extend docker-compose: %w", err) } - imageMetadata, err := metadata.GetDevContainerMetadata( - substitutionContext, - imageBuildInfo.Metadata, - parsedConfig, - extendImageBuildInfo.Features, - ) - if err != nil { - return composeExtendResult{buildImageName: buildImageName}, err + if extendResult.composeBuildFilePath != "" { + composeGlobalArgs = append(composeGlobalArgs, "-f", extendResult.composeBuildFilePath) } - return composeExtendResult{ - buildImageName: buildImageName, - composeBuildFilePath: dockerComposeFilePath, - imageMetadata: imageMetadata, - metadataLabel: extendImageBuildInfo.MetadataLabel, - }, nil -} - -func (r *runner) extendedDockerfile( - featureBuildInfo *feature.BuildInfo, - dockerfilePath, dockerfileContent string, -) (string, string) { - // extra args? - finalDockerfilePath := dockerfilePath - finalDockerfileContent := dockerfileContent - - // get extended build info - if featureBuildInfo != nil { - // rewrite dockerfile path - finalDockerfilePath = filepath.Join( - featureBuildInfo.FeaturesFolder, - "Dockerfile-with-features", - ) - - // rewrite dockerfile - finalDockerfileContent = dockerfile.RemoveSyntaxVersion(dockerfileContent) - finalDockerfileContent = strings.TrimSpace(strings.Join([]string{ - featureBuildInfo.DockerfilePrefixContent, - strings.TrimSpace(finalDockerfileContent), - featureBuildInfo.DockerfileContent, - }, "\n")) + currentImageName := extendResult.buildImageName + if currentImageName == "" { + currentImageName = params.originalImageName } - return finalDockerfilePath, finalDockerfileContent -} - -func (r *runner) setBuildPathsForContext( - originalContext, dockerFilePath, dockerfileContent, featuresFolder string, -) (relDockerfilePath string, modifiedDockerfileContent string, err error) { - absBuildContext, err := filepath.Abs(originalContext) + imageDetails, err := r.inspectImage(ctx, currentImageName) if err != nil { - return "", "", err + return nil, fmt.Errorf("inspect image: %w", err) } - absDockerFilePath, err := filepath.Abs(dockerFilePath) - if err != nil { - return "", "", err - } - relDockerfilePath, err = filepath.Rel(absBuildContext, absDockerFilePath) + overrideComposeUpFilePath, err := r.generateComposeUpOverride( + params, + extendResult, + imageDetails, + ) if err != nil { - return "", "", err + return nil, err } - absFeatureFolder, err := filepath.Abs(featuresFolder) - if err != nil { - return "", "", err + if overrideComposeUpFilePath != "" { + composeGlobalArgs = append(composeGlobalArgs, "-f", overrideComposeUpFilePath) } - relFeaturePath, err := filepath.Rel(absBuildContext, absFeatureFolder) - if err != nil { - return "", "", err - } - - // Rewrite COPY/ADD directives that reference the features folder to use the relative path - // from the custom build context. This ensures that the features folder is referenced in the - // Dockerfile. - pattern := regexp.MustCompile( - `(COPY|ADD)(\s+)\./` + regexp.QuoteMeta(config.DevsyContextFeatureFolder) + `/`, - ) - modifiedDockerfileContent = pattern.ReplaceAllString( - dockerfileContent, - "${1}${2}./"+filepath.ToSlash(relFeaturePath)+"/", - ) - - return relDockerfilePath, modifiedDockerfileContent, nil -} -type buildContextResult struct { - context string - dockerfilePathInContext string - dockerfileContent string + return composeGlobalArgs, nil } -func (r *runner) extendedDockerComposeBuild( - composeService *composetypes.ServiceConfig, - buildImageName string, - dockerFilePath string, - dockerfileContent string, - featuresBuildInfo *feature.BuildInfo, +// generateComposeUpOverride merges the image metadata into the devcontainer +// config and writes the compose "up" override file, returning its path. +func (r *runner) generateComposeUpOverride( + params *composeOverrideParams, + extendResult composeExtendResult, + imageDetails *config.ImageDetails, ) (string, error) { - result, err := r.prepareBuildContext( - composeService, dockerFilePath, dockerfileContent, featuresBuildInfo, + start := params.startParams + + mergedConfig, err := mergeImageMetadataConfig( + start.parsedConfig, + extendResult.imageMetadata, + start.options.ExtraDevContainerPath, ) if err != nil { return "", err } - if err := os.WriteFile(dockerFilePath, []byte(result.dockerfileContent), 0o600); err != nil { - return "", err + additionalLabels := map[string]string{ + metadata.ImageMetadataLabel: extendResult.metadataLabel, + config.UserLabel: imageDetails.Config.User, } - - service := r.createComposeService( - composeService, - buildImageName, - result.dockerfilePathInContext, - result.context, - featuresBuildInfo, - ) - return r.writeComposeFile(service) -} - -func (r *runner) prepareBuildContext( - composeService *composetypes.ServiceConfig, - dockerFilePath, dockerfileContent string, - featuresBuildInfo *feature.BuildInfo, -) (*buildContextResult, error) { - buildContext := filepath.Dir(featuresBuildInfo.FeaturesFolder) - relDockerFilePath, err := filepath.Rel(buildContext, dockerFilePath) + overrideComposeUpFilePath, err := r.extendedDockerComposeUp(&composeUpParams{ + parsedConfig: start.parsedConfig, + mergedConfig: mergedConfig, + composeHelper: start.composeHelper, + composeService: params.composeService, + originalImageName: params.originalImageName, + overrideImageName: extendResult.buildImageName, + imageDetails: imageDetails, + additionalLabels: additionalLabels, + }) if err != nil { - return nil, err + return "", fmt.Errorf("extend docker-compose up: %w", err) } - result := &buildContextResult{ - context: buildContext, - dockerfilePathInContext: relDockerFilePath, - dockerfileContent: dockerfileContent, - } + return overrideComposeUpFilePath, nil +} - if composeService.Build != nil && composeService.Build.Context != "" { - relDockerFilePath, modifiedDockerfileContent, err := r.setBuildPathsForContext( - composeService.Build.Context, - dockerFilePath, - dockerfileContent, - featuresBuildInfo.FeaturesFolder, - ) +// mergeImageMetadataConfig folds any extra devcontainer config into the image +// metadata, merges it with the parsed config, and applies extra remote env, +// returning the resulting merged devcontainer config. +func mergeImageMetadataConfig( + parsedConfig *config.SubstitutedConfig, + imageMetadata *config.ImageMetadataConfig, + extraDevContainerPath string, +) (*config.MergedDevContainerConfig, error) { + if extraDevContainerPath != "" { + if imageMetadata == nil { + imageMetadata = &config.ImageMetadataConfig{} + } + extraConfig, err := config.ParseDevContainerJSONFile(extraDevContainerPath) if err != nil { return nil, err } - log.Debugf( - "modified Dockerfile path in context to %s and content for extended compose build context %s", - relDockerFilePath, - composeService.Build.Context, - ) - result.context = composeService.Build.Context - result.dockerfilePathInContext = relDockerFilePath - result.dockerfileContent = modifiedDockerfileContent - } - - return result, nil -} - -func (r *runner) createComposeService( - composeService *composetypes.ServiceConfig, - buildImageName string, - dockerfilePathInContext, buildContext string, - featuresBuildInfo *feature.BuildInfo, -) *composetypes.ServiceConfig { - service := &composetypes.ServiceConfig{ - Name: composeService.Name, - Build: &composetypes.BuildConfig{ - Dockerfile: dockerfilePathInContext, - Context: buildContext, - }, - } - if buildImageName != "" { - service.Image = stripDigestFromImageRef(buildImageName) - } - - if composeService.Build != nil && composeService.Build.Target != "" { - service.Build.Target = featuresBuildInfo.OverrideTarget - } - - service.Build.Args = composetypes.NewMappingWithEquals([]string{"BUILDKIT_INLINE_CACHE=1"}) - for k, v := range featuresBuildInfo.BuildArgs { - service.Build.Args[k] = &v - } - - return service -} - -func composeBuildImageName( - composeHelper *compose.ComposeHelper, - projectName string, - composeService *composetypes.ServiceConfig, - hasFeatures bool, -) (string, error) { - if hasFeatures && composeService.Image != "" && composeService.Build == nil { - return composeHelper.GetDefaultImage(projectName, composeService.Name) - } - - if composeService.Image != "" { - return composeService.Image, nil + config.AddConfigToImageMetadata(extraConfig, imageMetadata) } - return composeHelper.GetDefaultImage(projectName, composeService.Name) -} - -func (r *runner) writeComposeFile(service *composetypes.ServiceConfig) (string, error) { - project := &composetypes.Project{ - Services: map[string]composetypes.ServiceConfig{ - service.Name: *service, - }, - } - - dockerComposeFolder := getDockerComposeFolder(r.WorkspaceConfig.Origin) - if err := os.MkdirAll(dockerComposeFolder, 0o750); err != nil { - return "", err - } - - dockerComposeData, err := yaml.Marshal(project) + mergedConfig, err := config.MergeConfiguration(parsedConfig.Config, imageMetadata.Config) if err != nil { - return "", err + return nil, fmt.Errorf("merge configuration: %w", err) } - dockerComposePath := filepath.Join( - dockerComposeFolder, - fmt.Sprintf("%s-%d.yml", FeaturesBuildOverrideFilePrefix, time.Now().Second()), - ) - - log.Debugf( - "Creating docker-compose build %s with content:\n %s", - dockerComposePath, - string(dockerComposeData), - ) - - if err := os.WriteFile(dockerComposePath, dockerComposeData, 0o600); err != nil { - return "", err - } - - return dockerComposePath, nil -} - -func stripDigestFromImageRef(imageRef string) string { - baseRef, _, found := strings.Cut(imageRef, "@") - if !found { - return imageRef + if err := config.MergeExtraRemoteEnv(mergedConfig, extraDevContainerPath); err != nil { + return nil, err } - return baseRef -} - -// mountToServiceVolumeConfig forwards Mount.Other options into the -// override Compose volume so they survive override generation; without -// this, bind/volume/tmpfs options are silently dropped. -func mountToServiceVolumeConfig(m *config.Mount) composetypes.ServiceVolumeConfig { - v := composetypes.ServiceVolumeConfig{ - Type: m.Type, - Source: m.Source, - Target: m.Target, - ReadOnly: m.IsReadOnly(), - Consistency: m.Consistency(), - } - switch m.Type { - case composetypes.VolumeTypeBind, "": - v.Bind = bindOptionsFromMount(m) - case composetypes.VolumeTypeVolume: - v.Volume = volumeOptionsFromMount(m) - case composetypes.VolumeTypeTmpfs: - v.Tmpfs = tmpfsOptionsFromMount(m) - } - return v -} - -func bindOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeBind { - propagation := m.BindPropagation() - nonRecursive := m.IsBindNonRecursive() - if propagation == "" && !nonRecursive { - return nil - } - bind := &composetypes.ServiceVolumeBind{Propagation: propagation} - if nonRecursive { - bind.Recursive = "disabled" - } - return bind + return mergedConfig, nil } -func volumeOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeVolume { - nocopy := m.VolumeNoCopy() - subpath := m.VolumeSubpath() - if !nocopy && subpath == "" { - return nil - } - return &composetypes.ServiceVolumeVolume{NoCopy: nocopy, Subpath: subpath} -} +// recreateDevContainer stops and deletes an existing dev container so it can be +// recreated, in response to the --recreate option. +func (r *runner) recreateDevContainer( + ctx context.Context, + container *config.ContainerDetails, +) error { + log.Debugf("Deleting dev container %s due to --recreate", container.ID) -func tmpfsOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeTmpfs { - size, hasSize := parseTmpfsSize(m.TmpfsSize(), m.Target) - mode, hasMode := parseTmpfsMode(m.TmpfsMode(), m.Target) - if !hasSize && !hasMode { - return nil + if err := r.Driver.StopDevContainer(ctx, r.ID); err != nil { + return fmt.Errorf("stop dev container: %w", err) } - return &composetypes.ServiceVolumeTmpfs{Size: size, Mode: mode} -} -func parseTmpfsSize(raw, target string) (composetypes.UnitBytes, bool) { - if raw == "" { - return 0, false - } - parsed, err := strconv.ParseInt(raw, 10, 64) - if err != nil { - log.Warnf("ignoring tmpfs-size %q on mount %s: %s", raw, target, err) - return 0, false + if err := r.Driver.DeleteDevContainer(ctx, r.ID); err != nil { + return fmt.Errorf("delete dev container: %w", err) } - return composetypes.UnitBytes(parsed), true + return nil } -func parseTmpfsMode(raw, target string) (uint32, bool) { - if raw == "" { - return 0, false - } - parsed, err := strconv.ParseUint(raw, 8, 32) - if err != nil { - log.Warnf("ignoring tmpfs-mode %q on mount %s: %s", raw, target, err) - return 0, false - } - return uint32(parsed), true +// composeUpRunParams groups the inputs for the final "docker compose up" step. +type composeUpRunParams struct { + project *composetypes.Project + composeService *composetypes.ServiceConfig + composeHelper *compose.ComposeHelper + composeGlobalArgs []string + parsedConfig *config.SubstitutedConfig + hasExistingContainer bool } -func (r *runner) extendedDockerComposeUp( - parsedConfig *config.SubstitutedConfig, - mergedConfig *config.MergedDevContainerConfig, - composeHelper *compose.ComposeHelper, - composeService *composetypes.ServiceConfig, - originalImageName, - overrideImageName string, - imageDetails *config.ImageDetails, - additionalLabels map[string]string, -) (string, error) { - dockerComposeUpProject := r.generateDockerComposeUpProject( - parsedConfig, - mergedConfig, - composeHelper, - composeService, - originalImageName, - overrideImageName, - imageDetails, - additionalLabels, - ) - dockerComposeData, err := yaml.Marshal(dockerComposeUpProject) - if err != nil { - return "", err +// composeUpAndFindContainer runs "docker compose up -d" with the assembled +// arguments and returns the resulting dev container details. +func (r *runner) composeUpAndFindContainer( + ctx context.Context, + params *composeUpRunParams, +) (*config.ContainerDetails, error) { + upArgs := []string{composeProjectNameFlag, params.project.Name} + upArgs = append(upArgs, params.composeGlobalArgs...) + upArgs = append(upArgs, "up", "-d") + if params.hasExistingContainer { + upArgs = append(upArgs, "--no-recreate") } + upArgs = r.onlyRunServices(upArgs, params.parsedConfig) - dockerComposeFolder := getDockerComposeFolder(r.WorkspaceConfig.Origin) - err = os.MkdirAll(dockerComposeFolder, 0o750) - if err != nil { - return "", err + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + if err := params.composeHelper.Run(ctx, upArgs, nil, writer, writer); err != nil { + return nil, fmt.Errorf("docker-compose run: %w", err) } - dockerComposePath := filepath.Join( - dockerComposeFolder, - fmt.Sprintf("%s-%d.yml", FeaturesStartOverrideFilePrefix, time.Now().Second()), - ) - - log.Debugf( - "Creating docker-compose up %s with content:\n %s", - dockerComposePath, - string(dockerComposeData), + // TODO wait for started event? + containerDetails, err := params.composeHelper.FindDevContainer( + ctx, + params.project.Name, + params.composeService.Name, ) - - err = os.WriteFile(dockerComposePath, dockerComposeData, 0o600) if err != nil { - return "", err - } - return dockerComposePath, nil -} - -func (r *runner) generateDockerComposeUpProject( - parsedConfig *config.SubstitutedConfig, - mergedConfig *config.MergedDevContainerConfig, - composeHelper *compose.ComposeHelper, - composeService *composetypes.ServiceConfig, - originalImageName, - overrideImageName string, - imageDetails *config.ImageDetails, - additionalLabels map[string]string, -) *composetypes.Project { - // Configure overridden service - userEntrypoint := composeService.Entrypoint - userCommand := composeService.Command - if mergedConfig.OverrideCommand != nil && *mergedConfig.OverrideCommand { - userEntrypoint = []string{} - userCommand = []string{} - } else { - if len(userEntrypoint) == 0 { - userEntrypoint = imageDetails.Config.Entrypoint - } - - if len(userCommand) == 0 { - userCommand = imageDetails.Config.Cmd - } - } - - entrypoint := composetypes.ShellCommand{ - "/bin/sh", - "-c", - `echo Container started -trap "exit 0" 15 -` + strings.Join(mergedConfig.Entrypoints, "\n") + ` -exec "$$@" -` + DefaultEntrypoint, - "-", - } - entrypoint = append(entrypoint, userEntrypoint...) - - labels := composetypes.Labels{} - if len(r.IDLabels) > 0 { - for _, l := range r.IDLabels { - k, v, _ := strings.Cut(l, "=") - v = regexp.MustCompile(`\$`).ReplaceAllString(v, "$$$$") - v = regexp.MustCompile(`'`).ReplaceAllString(v, `\'\'`) - labels[k] = v - } - } else { - labels[config.DockerIDLabel] = r.ID - } - for k, v := range additionalLabels { - // Escape $ and ' to prevent substituting local environment variables! - label := regexp.MustCompile(`\$`).ReplaceAllString(v, "$$$$") - label = regexp.MustCompile(`'`).ReplaceAllString(label, `\'\'`) - labels.Add(k, label) - } - - overrideService := &composetypes.ServiceConfig{ - Name: composeService.Name, - Entrypoint: entrypoint, - Environment: mappingFromMap(mergedConfig.ContainerEnv), - Init: mergedConfig.Init, - CapAdd: mergedConfig.CapAdd, - SecurityOpt: mergedConfig.SecurityOpt, - Labels: labels, - } - - if originalImageName != overrideImageName { - overrideService.Image = overrideImageName - } - - if !reflect.DeepEqual(userCommand, composeService.Command) { - overrideService.Command = userCommand - } - - if mergedConfig.ContainerUser != "" { - overrideService.User = mergedConfig.ContainerUser - } - - if mergedConfig.Privileged != nil { - overrideService.Privileged = *mergedConfig.Privileged - } - - gpuSupportEnabled := r.resolveComposeGPUAvailability(composeHelper) - r.configureGPUResources(parsedConfig, gpuSupportEnabled, overrideService) - - for _, mount := range mergedConfig.Mounts { - overrideService.Volumes = append( - overrideService.Volumes, - mountToServiceVolumeConfig(mount), - ) - } - - project := &composetypes.Project{} - project.Services = map[string]composetypes.ServiceConfig{ - overrideService.Name: *overrideService, - } - - // Configure volumes - var volumeMounts []composetypes.VolumeConfig - for _, m := range mergedConfig.Mounts { - if m.Type == "volume" { - volumeMounts = append(volumeMounts, composetypes.VolumeConfig{ - Name: m.Source, - External: composetypes.External(m.External), - }) - } - } - - if len(volumeMounts) > 0 { - project.Volumes = map[string]composetypes.VolumeConfig{} - } - for _, volumeMount := range volumeMounts { - project.Volumes[volumeMount.Name] = volumeMount - } - - return project -} - -func (r *runner) resolveComposeGPUAvailability(composeHelper *compose.ComposeHelper) bool { - switch r.WorkspaceConfig.CLIOptions.GPUAvailability { - case stringTrue: - return true - case stringFalse: - return false - default: - available, _ := composeHelper.Docker.GPUSupportEnabled() - return available + return nil, fmt.Errorf("find dev container: %w", err) } -} -func (r *runner) configureGPUResources( - parsedConfig *config.SubstitutedConfig, - gpuSupportEnabled bool, - overrideService *composetypes.ServiceConfig, -) { - if parsedConfig.Config.HostRequirements != nil { - enableGPU, warnIfMissing := parsedConfig.Config.HostRequirements.ShouldEnableGPU( - gpuSupportEnabled, - ) - if enableGPU { - overrideService.Deploy = &composetypes.DeployConfig{ - Resources: composetypes.Resources{ - Reservations: &composetypes.Resource{ - Devices: []composetypes.DeviceRequest{ - { - Capabilities: []string{"gpu"}, - }, - }, - }, - }, - } - } - if warnIfMissing { - log.Warn("GPU required but not available on host") - } - } + return containerDetails, nil } func checkForPersistedFile( @@ -1356,6 +943,29 @@ func getDockerComposeFolder(workspaceOriginFolder string) string { return filepath.Join(workspaceOriginFolder, ".docker-compose") } +// writeComposeOverrideFile writes a compose override file into the workspace's +// docker-compose folder using a collision-safe unique name that retains the +// given prefix (so checkForPersistedFile can still match it by prefix). +func (r *runner) writeComposeOverrideFile(prefix string, data []byte) (string, error) { + dockerComposeFolder := getDockerComposeFolder(r.WorkspaceConfig.Origin) + if err := os.MkdirAll(dockerComposeFolder, 0o750); err != nil { + return "", err + } + + f, err := os.CreateTemp(dockerComposeFolder, prefix+"-*.yml") + if err != nil { + return "", err + } + defer func() { _ = f.Close() }() + + if _, err := f.Write(data); err != nil { + _ = os.Remove(f.Name()) + return "", err + } + + return f.Name(), nil +} + func mappingFromMap(m map[string]string) composetypes.MappingWithEquals { if len(m) == 0 { return nil diff --git a/pkg/devcontainer/compose_build.go b/pkg/devcontainer/compose_build.go new file mode 100644 index 000000000..575a1f1fd --- /dev/null +++ b/pkg/devcontainer/compose_build.go @@ -0,0 +1,616 @@ +package devcontainer + +import ( + "context" + "fmt" + "os" + "path" + "path/filepath" + "regexp" + "strings" + + composetypes "github.com/compose-spec/compose-go/v2/types" + "github.com/devsy-org/devsy/pkg/compose" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/devcontainer/feature" + "github.com/devsy-org/devsy/pkg/devcontainer/metadata" + "github.com/devsy-org/devsy/pkg/dockerfile" + "github.com/devsy-org/devsy/pkg/log" + "gopkg.in/yaml.v3" +) + +// featureFolderCopyPattern matches COPY/ADD directives that reference the +// devsy feature folder so they can be rewritten to a path relative to a custom +// build context. Compiled once because the folder name is a build-time constant. +var featureFolderCopyPattern = regexp.MustCompile( + `(COPY|ADD)(\s+)\./` + regexp.QuoteMeta(config.DevsyContextFeatureFolder) + `/`, +) + +// prepareComposeBuildInfo modifies a compose project's devcontainer Dockerfile +// to ensure it can be extended with features. If an Image is specified instead +// of a Build, the metadata from the Image is used to populate the build info. +func (r *runner) prepareComposeBuildInfo( + ctx context.Context, + subCtx *config.SubstitutionContext, + composeService *composetypes.ServiceConfig, + buildTarget string, +) (composeBuildInfo, error) { + if composeService.Build == nil { + imageBuildInfo, err := r.getImageBuildInfoFromImage(ctx, subCtx, composeService.Image) + if err != nil { + return composeBuildInfo{}, err + } + return composeBuildInfo{imageBuildInfo: imageBuildInfo, buildTarget: buildTarget}, nil + } + + return r.prepareComposeDockerfileBuildInfo(subCtx, composeService) +} + +// prepareComposeDockerfileBuildInfo handles the Build-backed branch of +// prepareComposeBuildInfo: it reads the service Dockerfile, resolves the build +// target (ensuring a final stage name for multi-stage builds), and extracts the +// image build info. +func (r *runner) prepareComposeDockerfileBuildInfo( + subCtx *config.SubstitutionContext, + composeService *composetypes.ServiceConfig, +) (composeBuildInfo, error) { + dockerFilePath := composeService.Build.Dockerfile + if !path.IsAbs(dockerFilePath) { + dockerFilePath = filepath.Join(composeService.Build.Context, dockerFilePath) + } + + // #nosec G304 -- dockerFilePath is derived from trusted devcontainer config. + originalDockerfile, err := os.ReadFile(dockerFilePath) + if err != nil { + return composeBuildInfo{}, err + } + + // Determine build target. If a multi stage build is used, ensure it is + // valid and modify the Dockerfile if necessary. + originalTarget := composeService.Build.Target + var dockerfileContents string + var buildTarget string + if originalTarget != "" { + buildTarget = originalTarget + // Preserve the Dockerfile contents so a build-backed service with an + // explicit target is not later misclassified as image-based. + dockerfileContents = string(originalDockerfile) + } else { + lastStageName, modifiedDockerfile, stageErr := dockerfile.EnsureFinalStageName( + string(originalDockerfile), + config.DockerfileDefaultTarget, + ) + if stageErr != nil { + return composeBuildInfo{}, stageErr + } + + buildTarget = lastStageName + // Override Dockerfile if it was modified, otherwise use the original + if modifiedDockerfile != "" { + dockerfileContents = modifiedDockerfile + } else { + dockerfileContents = string(originalDockerfile) + } + } + + imageBuildInfo, err := r.getImageBuildInfoFromDockerfile( + subCtx, + string(originalDockerfile), + mappingToMap(composeService.Build.Args), + originalTarget, + ) + if err != nil { + return composeBuildInfo{}, err + } + + return composeBuildInfo{ + imageBuildInfo: imageBuildInfo, + dockerfileContents: dockerfileContents, + buildTarget: buildTarget, + }, nil +} + +// This extends the build information for docker compose containers. +func (r *runner) buildAndExtendDockerCompose( + ctx context.Context, + params *buildAndExtendParams, +) (composeExtendResult, error) { + prepared, err := r.prepareExtendedComposeBuild(ctx, params) + if err != nil { + return composeExtendResult{}, err + } + extendImageBuildInfo := prepared.extendImageBuildInfo + + buildImageName, err := composeBuildImageName( + params.composeHelper, + params.project.Name, + params.composeService, + hasFeatureBuildInfo(extendImageBuildInfo), + ) + if err != nil { + return composeExtendResult{}, err + } + + dockerComposeFilePath, cleanup, err := r.composeFeatureOverride( + prepared, + params.composeService, + buildImageName, + ) + defer cleanup() + if err != nil { + return composeExtendResult{buildImageName: buildImageName}, err + } + + buildArgs := composeBuildArgs(&composeBuildArgsParams{ + projectName: params.project.Name, + globalArgs: params.globalArgs, + overrideComposeFilePath: dockerComposeFilePath, + pull: params.pull, + noCache: params.noCache, + serviceName: params.composeService.Name, + runServices: params.parsedConfig.Config.RunServices, + }) + + if err := r.runComposeBuild(ctx, params.composeHelper, buildArgs); err != nil { + return composeExtendResult{buildImageName: buildImageName}, err + } + + imageMetadata, err := metadata.GetDevContainerMetadata( + params.substitutionContext, + prepared.imageBuildInfo.Metadata, + params.parsedConfig, + extendImageBuildInfo.Features, + ) + if err != nil { + return composeExtendResult{buildImageName: buildImageName}, err + } + + return composeExtendResult{ + buildImageName: buildImageName, + composeBuildFilePath: dockerComposeFilePath, + imageMetadata: imageMetadata, + metadataLabel: extendImageBuildInfo.MetadataLabel, + }, nil +} + +// composeFeatureOverride builds the feature override compose file when the build +// has features, returning the override path (empty when none) and a cleanup +// function that is always safe to defer. +func (r *runner) composeFeatureOverride( + prepared preparedComposeBuild, + composeService *composetypes.ServiceConfig, + buildImageName string, +) (string, func(), error) { + if !hasFeatureBuildInfo(prepared.extendImageBuildInfo) { + return "", func() {}, nil + } + return r.writeFeatureBuildOverride(&featureBuildOverrideParams{ + extendImageBuildInfo: prepared.extendImageBuildInfo, + composeService: composeService, + buildImageName: buildImageName, + dockerfileContents: prepared.dockerfileContents, + buildTarget: prepared.buildTarget, + }) +} + +// preparedComposeBuild holds the resolved base build info and feature-extended +// build info used to assemble the compose build. +type preparedComposeBuild struct { + imageBuildInfo *config.ImageBuildInfo + extendImageBuildInfo *feature.ExtendedBuildInfo + dockerfileContents string + buildTarget string +} + +// prepareExtendedComposeBuild resolves the base image build info for the compose +// service and computes the feature-extended build info on top of it. +func (r *runner) prepareExtendedComposeBuild( + ctx context.Context, + params *buildAndExtendParams, +) (preparedComposeBuild, error) { + const defaultBuildTarget = "dev_container_auto_added_stage_label" + + buildInfo, err := r.prepareComposeBuildInfo( + ctx, + params.substitutionContext, + params.composeService, + defaultBuildTarget, + ) + if err != nil { + return preparedComposeBuild{}, err + } + + extendImageBuildInfo, err := feature.GetExtendedBuildInfo(&feature.ExtendedBuildParams{ + Ctx: params.substitutionContext, + ImageBuildInfo: buildInfo.imageBuildInfo, + Target: buildInfo.buildTarget, + DevContainerConfig: params.parsedConfig, + ForceBuild: false, + SecretOpts: &feature.SecretOptions{ + SecretsFile: params.featureSecretsFile, + Prompter: &feature.TerminalSecretPrompter{}, + }, + }) + if err != nil { + return preparedComposeBuild{}, err + } + + return preparedComposeBuild{ + imageBuildInfo: buildInfo.imageBuildInfo, + extendImageBuildInfo: extendImageBuildInfo, + dockerfileContents: buildInfo.dockerfileContents, + buildTarget: buildInfo.buildTarget, + }, nil +} + +// hasFeatureBuildInfo reports whether the extended build info carries a feature +// build that requires generating an override Dockerfile and compose file. +func hasFeatureBuildInfo(extendImageBuildInfo *feature.ExtendedBuildInfo) bool { + return extendImageBuildInfo != nil && extendImageBuildInfo.FeaturesBuildInfo != nil +} + +// runComposeBuild runs "docker compose ... build" with the given arguments, +// streaming output to the info log. +func (r *runner) runComposeBuild( + ctx context.Context, + composeHelper *compose.ComposeHelper, + buildArgs []string, +) error { + writer := log.Writer(log.LevelInfo) + defer func() { _ = writer.Close() }() + log.Debugf("Run %s %s", composeHelper.Command, strings.Join(buildArgs, " ")) + if err := composeHelper.Run(ctx, buildArgs, nil, writer, writer); err != nil { + return err + } + return nil +} + +// featureBuildOverrideParams groups the inputs for writing the extended +// Dockerfile and compose build override used when features are present. +type featureBuildOverrideParams struct { + extendImageBuildInfo *feature.ExtendedBuildInfo + composeService *composetypes.ServiceConfig + buildImageName string + dockerfileContents string + buildTarget string +} + +// writeFeatureBuildOverride writes the extended Dockerfile and the compose build +// override file referencing it. It returns the override compose file path and a +// cleanup function that removes the temporary extended Dockerfile directory. The +// cleanup function is always non-nil and safe to defer even on error. +func (r *runner) writeFeatureBuildOverride( + params *featureBuildOverrideParams, +) (composeFilePath string, cleanup func(), err error) { + cleanup = func() {} + + dockerfileContents := params.dockerfileContents + // If the dockerfile is empty (because an Image was used), reference that + // image as the build target after the features / modified contents. + if dockerfileContents == "" { + if params.composeService.Image == "" && params.composeService.Build == nil { + return "", cleanup, fmt.Errorf( + "compose service %q has no image or build configuration", + params.composeService.Name, + ) + } + sanitizedImage := strings.ReplaceAll( + strings.ReplaceAll(params.composeService.Image, "\n", ""), + "\r", + "", + ) + dockerfileContents = fmt.Sprintf("FROM %s AS %s\n", sanitizedImage, params.buildTarget) + } + + // The feature build info rewrites the Dockerfile path, so the original path + // is intentionally empty here. + extendedDockerfilePath, extendedDockerfileContent := r.extendedDockerfile( + params.extendImageBuildInfo.FeaturesBuildInfo, + "", + dockerfileContents, + ) + + log.Debugf( + "Creating extended Dockerfile %s with content: \n %s", + extendedDockerfilePath, + extendedDockerfileContent, + ) + + cleanup = func() { _ = os.RemoveAll(filepath.Dir(extendedDockerfilePath)) } + + composeFilePath, err = r.extendedDockerComposeBuild(&extendedComposeBuildParams{ + composeService: params.composeService, + buildImageName: params.buildImageName, + dockerFilePath: extendedDockerfilePath, + dockerfileContent: extendedDockerfileContent, + featuresBuildInfo: params.extendImageBuildInfo.FeaturesBuildInfo, + }) + if err != nil { + return "", cleanup, err + } + + return composeFilePath, cleanup, nil +} + +// composeBuildArgsParams groups the inputs for assembling compose build args. +type composeBuildArgsParams struct { + projectName string + globalArgs []string + overrideComposeFilePath string + pull bool + noCache bool + serviceName string + runServices []string +} + +// composeBuildArgs assembles the "docker compose ... build" argument list, +// adding the override file, --pull/--no-cache build modifiers, and any +// explicitly requested run services. +func composeBuildArgs(params *composeBuildArgsParams) []string { + buildArgs := []string{composeProjectNameFlag, params.projectName} + buildArgs = append(buildArgs, params.globalArgs...) + if params.overrideComposeFilePath != "" { + buildArgs = append(buildArgs, "-f", params.overrideComposeFilePath) + } + buildArgs = append(buildArgs, "build") + if params.pull { + buildArgs = append(buildArgs, "--pull") + } + if params.noCache { + buildArgs = append(buildArgs, "--no-cache") + } + + // Only run the services defined in .devcontainer.json runServices + if len(params.runServices) > 0 { + buildArgs = append(buildArgs, params.serviceName) + for _, service := range params.runServices { + if service == params.serviceName { + continue + } + buildArgs = append(buildArgs, service) + } + } + return buildArgs +} + +func (r *runner) extendedDockerfile( + featureBuildInfo *feature.BuildInfo, + dockerfilePath, dockerfileContent string, +) (string, string) { + // extra args? + finalDockerfilePath := dockerfilePath + finalDockerfileContent := dockerfileContent + + // get extended build info + if featureBuildInfo != nil { + // rewrite dockerfile path + finalDockerfilePath = filepath.Join( + featureBuildInfo.FeaturesFolder, + "Dockerfile-with-features", + ) + + // rewrite dockerfile + finalDockerfileContent = dockerfile.RemoveSyntaxVersion(dockerfileContent) + finalDockerfileContent = strings.TrimSpace(strings.Join([]string{ + featureBuildInfo.DockerfilePrefixContent, + strings.TrimSpace(finalDockerfileContent), + featureBuildInfo.DockerfileContent, + }, "\n")) + } + + return finalDockerfilePath, finalDockerfileContent +} + +func (r *runner) setBuildPathsForContext( + originalContext, dockerFilePath, dockerfileContent, featuresFolder string, +) (relDockerfilePath string, modifiedDockerfileContent string, err error) { + absBuildContext, err := filepath.Abs(originalContext) + if err != nil { + return "", "", err + } + + absDockerFilePath, err := filepath.Abs(dockerFilePath) + if err != nil { + return "", "", err + } + relDockerfilePath, err = filepath.Rel(absBuildContext, absDockerFilePath) + if err != nil { + return "", "", err + } + + absFeatureFolder, err := filepath.Abs(featuresFolder) + if err != nil { + return "", "", err + } + relFeaturePath, err := filepath.Rel(absBuildContext, absFeatureFolder) + if err != nil { + return "", "", err + } + + // Rewrite COPY/ADD directives that reference the features folder to use the relative path + // from the custom build context. This ensures that the features folder is referenced in the + // Dockerfile. + modifiedDockerfileContent = featureFolderCopyPattern.ReplaceAllString( + dockerfileContent, + "${1}${2}./"+filepath.ToSlash(relFeaturePath)+"/", + ) + + return relDockerfilePath, modifiedDockerfileContent, nil +} + +type buildContextResult struct { + context string + dockerfilePathInContext string + dockerfileContent string +} + +// extendedComposeBuildParams groups the inputs for writing the extended compose +// build file referencing a feature-augmented Dockerfile. +type extendedComposeBuildParams struct { + composeService *composetypes.ServiceConfig + buildImageName string + dockerFilePath string + dockerfileContent string + featuresBuildInfo *feature.BuildInfo +} + +func (r *runner) extendedDockerComposeBuild(params *extendedComposeBuildParams) (string, error) { + result, err := r.prepareBuildContext( + params.composeService, + params.dockerFilePath, + params.dockerfileContent, + params.featuresBuildInfo, + ) + if err != nil { + return "", err + } + + if err := os.WriteFile( + params.dockerFilePath, + []byte(result.dockerfileContent), + 0o600, + ); err != nil { + return "", err + } + + service := r.createComposeService(&composeServiceParams{ + composeService: params.composeService, + buildImageName: params.buildImageName, + dockerfilePathInContext: result.dockerfilePathInContext, + buildContext: result.context, + featuresBuildInfo: params.featuresBuildInfo, + }) + return r.writeComposeFile(service) +} + +func (r *runner) prepareBuildContext( + composeService *composetypes.ServiceConfig, + dockerFilePath, dockerfileContent string, + featuresBuildInfo *feature.BuildInfo, +) (*buildContextResult, error) { + buildContext := filepath.Dir(featuresBuildInfo.FeaturesFolder) + relDockerFilePath, err := filepath.Rel(buildContext, dockerFilePath) + if err != nil { + return nil, err + } + + result := &buildContextResult{ + context: buildContext, + dockerfilePathInContext: relDockerFilePath, + dockerfileContent: dockerfileContent, + } + + if composeService.Build != nil && composeService.Build.Context != "" { + relDockerFilePath, modifiedDockerfileContent, err := r.setBuildPathsForContext( + composeService.Build.Context, + dockerFilePath, + dockerfileContent, + featuresBuildInfo.FeaturesFolder, + ) + if err != nil { + return nil, err + } + log.Debugf( + "modified Dockerfile path in context to %s and content for extended compose build context %s", + relDockerFilePath, + composeService.Build.Context, + ) + result.context = composeService.Build.Context + result.dockerfilePathInContext = relDockerFilePath + result.dockerfileContent = modifiedDockerfileContent + } + + return result, nil +} + +// composeServiceParams groups the inputs for building the override compose +// service definition. +type composeServiceParams struct { + composeService *composetypes.ServiceConfig + buildImageName string + dockerfilePathInContext string + buildContext string + featuresBuildInfo *feature.BuildInfo +} + +func (r *runner) createComposeService(params *composeServiceParams) *composetypes.ServiceConfig { + composeService := params.composeService + featuresBuildInfo := params.featuresBuildInfo + + service := &composetypes.ServiceConfig{ + Name: composeService.Name, + Build: &composetypes.BuildConfig{ + Dockerfile: params.dockerfilePathInContext, + Context: params.buildContext, + }, + } + if params.buildImageName != "" { + service.Image = stripDigestFromImageRef(params.buildImageName) + } + + if composeService.Build != nil && composeService.Build.Target != "" { + service.Build.Target = featuresBuildInfo.OverrideTarget + } + + service.Build.Args = composetypes.NewMappingWithEquals([]string{"BUILDKIT_INLINE_CACHE=1"}) + for k, v := range featuresBuildInfo.BuildArgs { + service.Build.Args[k] = &v + } + + return service +} + +func composeBuildImageName( + composeHelper *compose.ComposeHelper, + projectName string, + composeService *composetypes.ServiceConfig, + hasFeatures bool, +) (string, error) { + if hasFeatures && composeService.Image != "" && composeService.Build == nil { + return composeHelper.GetDefaultImage(projectName, composeService.Name) + } + + if composeService.Image != "" { + return composeService.Image, nil + } + + return composeHelper.GetDefaultImage(projectName, composeService.Name) +} + +func (r *runner) writeComposeFile(service *composetypes.ServiceConfig) (string, error) { + project := &composetypes.Project{ + Services: map[string]composetypes.ServiceConfig{ + service.Name: *service, + }, + } + + dockerComposeData, err := yaml.Marshal(project) + if err != nil { + return "", err + } + + dockerComposePath, err := r.writeComposeOverrideFile( + FeaturesBuildOverrideFilePrefix, + dockerComposeData, + ) + if err != nil { + return "", err + } + + log.Debugf( + "Creating docker-compose build %s with content:\n %s", + dockerComposePath, + string(dockerComposeData), + ) + + return dockerComposePath, nil +} + +func stripDigestFromImageRef(imageRef string) string { + baseRef, _, found := strings.Cut(imageRef, "@") + if !found { + return imageRef + } + + return baseRef +} diff --git a/pkg/devcontainer/compose_mounts.go b/pkg/devcontainer/compose_mounts.go new file mode 100644 index 000000000..679201067 --- /dev/null +++ b/pkg/devcontainer/compose_mounts.go @@ -0,0 +1,86 @@ +package devcontainer + +import ( + "strconv" + + composetypes "github.com/compose-spec/compose-go/v2/types" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/log" +) + +// mountToServiceVolumeConfig forwards Mount.Other options into the +// override Compose volume so they survive override generation; without +// this, bind/volume/tmpfs options are silently dropped. +func mountToServiceVolumeConfig(m *config.Mount) composetypes.ServiceVolumeConfig { + v := composetypes.ServiceVolumeConfig{ + Type: m.Type, + Source: m.Source, + Target: m.Target, + ReadOnly: m.IsReadOnly(), + Consistency: m.Consistency(), + } + switch m.Type { + case composetypes.VolumeTypeBind, "": + v.Bind = bindOptionsFromMount(m) + case composetypes.VolumeTypeVolume: + v.Volume = volumeOptionsFromMount(m) + case composetypes.VolumeTypeTmpfs: + v.Tmpfs = tmpfsOptionsFromMount(m) + } + return v +} + +func bindOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeBind { + propagation := m.BindPropagation() + nonRecursive := m.IsBindNonRecursive() + if propagation == "" && !nonRecursive { + return nil + } + bind := &composetypes.ServiceVolumeBind{Propagation: propagation} + if nonRecursive { + bind.Recursive = "disabled" + } + return bind +} + +func volumeOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeVolume { + nocopy := m.VolumeNoCopy() + subpath := m.VolumeSubpath() + if !nocopy && subpath == "" { + return nil + } + return &composetypes.ServiceVolumeVolume{NoCopy: nocopy, Subpath: subpath} +} + +func tmpfsOptionsFromMount(m *config.Mount) *composetypes.ServiceVolumeTmpfs { + size, hasSize := parseTmpfsSize(m.TmpfsSize(), m.Target) + mode, hasMode := parseTmpfsMode(m.TmpfsMode(), m.Target) + if !hasSize && !hasMode { + return nil + } + return &composetypes.ServiceVolumeTmpfs{Size: size, Mode: mode} +} + +func parseTmpfsSize(raw, target string) (composetypes.UnitBytes, bool) { + if raw == "" { + return 0, false + } + parsed, err := strconv.ParseInt(raw, 10, 64) + if err != nil || parsed < 0 { + log.Warnf("ignoring invalid tmpfs-size %q on mount %s", raw, target) + return 0, false + } + return composetypes.UnitBytes(parsed), true +} + +func parseTmpfsMode(raw, target string) (uint32, bool) { + if raw == "" { + return 0, false + } + parsed, err := strconv.ParseUint(raw, 8, 32) + if err != nil { + log.Warnf("ignoring tmpfs-mode %q on mount %s: %s", raw, target, err) + return 0, false + } + return uint32(parsed), true +} diff --git a/pkg/devcontainer/compose_test.go b/pkg/devcontainer/compose_test.go index 66176f6bd..ea0c08516 100644 --- a/pkg/devcontainer/compose_test.go +++ b/pkg/devcontainer/compose_test.go @@ -2,6 +2,7 @@ package devcontainer import ( "path/filepath" + "slices" "strings" "testing" @@ -145,22 +146,22 @@ func (s *ComposeSuite) TestComposeBuildImageName() { func (s *ComposeSuite) TestCreateComposeServiceUsesBuildImageName() { r := &runner{} - service := r.createComposeService( - &composetypes.ServiceConfig{ + service := r.createComposeService(&composeServiceParams{ + composeService: &composetypes.ServiceConfig{ Name: "app", Image: "ghcr.io/example/shared-base:latest", Build: &composetypes.BuildConfig{Target: "original-target"}, }, - "workspace-app:latest", - "Dockerfile-with-features", - "/tmp/context", - &feature.BuildInfo{ + buildImageName: "workspace-app:latest", + dockerfilePathInContext: "Dockerfile-with-features", + buildContext: "/tmp/context", + featuresBuildInfo: &feature.BuildInfo{ OverrideTarget: "dev_containers_target_stage", BuildArgs: map[string]string{ "FEATURE_FLAG": "true", }, }, - ) + }) s.Equal("workspace-app:latest", service.Image) s.Require().NotNil(service.Build) @@ -457,6 +458,11 @@ func TestTmpfsOptionsFromMount(t *testing.T) { in: &config.Mount{Other: []string{"tmpfs-size=oops", "tmpfs-mode=oops"}}, want: nil, }, + { + name: "negative size dropped", + in: &config.Mount{Other: []string{"tmpfs-size=-1"}}, + want: nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -470,3 +476,261 @@ func TestTmpfsOptionsFromMount(t *testing.T) { }) } } + +func TestEscapeComposeLabelValue(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {name: "plain value untouched", in: "plain-value", want: "plain-value"}, + {name: "empty value", in: "", want: ""}, + + // "$" is the only character Compose interpolates, so it is doubled. + {name: "single dollar doubled", in: "$HOME", want: "$$HOME"}, + {name: "leading dollar", in: "$", want: "$$"}, + {name: "trailing dollar", in: "a$", want: "a$$"}, + {name: "multiple dollars", in: "$a$b$", want: "$$a$$b$$"}, + {name: "already doubled dollar is doubled again", in: "$$", want: "$$$$"}, + {name: "braced variable reference", in: "${FOO}", want: "$${FOO}"}, + + // Characters that are NOT special to Compose interpolation must pass + // through verbatim. Escaping them would corrupt the stored payload. + {name: "single quote preserved", in: "it's", want: "it's"}, + {name: "double quote preserved", in: `say "hi"`, want: `say "hi"`}, + {name: "backslash preserved", in: `a\b`, want: `a\b`}, + {name: "backtick preserved", in: "a`b", want: "a`b"}, + {name: "newline preserved", in: "line1\nline2", want: "line1\nline2"}, + {name: "unicode preserved", in: "café—naïve 🚀", want: "café—naïve 🚀"}, + + // Realistic metadata: JSON containing an apostrophe must survive so it + // can be json.Unmarshal'd back on read. Only "$" is altered. + { + name: "json metadata with apostrophe and dollar", + in: `[{"id":"it's-a-feature","cmd":"echo $X"}]`, + want: `[{"id":"it's-a-feature","cmd":"echo $$X"}]`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := escapeComposeLabelValue(tt.in); got != tt.want { + t.Errorf("escapeComposeLabelValue(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +// TestEscapeComposeLabelValueRoundTrip documents the contract that motivates the +// escaping: doubling "$" survives Compose's interpolation (which un-doubles +// "$$" back to "$"), and non-"$" characters are returned unchanged so a +// JSON-encoded metadata label can be unmarshalled back without corruption. +func TestEscapeComposeLabelValueRoundTrip(t *testing.T) { + original := `{"name":"it's a test","entrypoint":"run $CMD"}` + + escaped := escapeComposeLabelValue(original) + + // Compose interpolation collapses "$$" -> "$"; emulate that to recover the + // value a consumer would see. + interpolated := strings.ReplaceAll(escaped, "$$", "$") + if interpolated != original { + t.Errorf( + "round-trip mismatch:\n escaped = %q\n interpolated = %q\n original = %q", + escaped, + interpolated, + original, + ) + } + + // The apostrophe (and every non-"$" byte) must be untouched in the escaped + // form so json.Unmarshal succeeds after interpolation. + if strings.Contains(escaped, `\'`) { + t.Errorf("apostrophe was escaped, corrupting the payload: %q", escaped) + } +} + +func TestBuildServiceLabels(t *testing.T) { + t.Run("uses default ID label when no ID labels", func(t *testing.T) { + r := &runner{} + r.ID = "workspace-id" + + labels := r.buildServiceLabels(nil) + + if labels[config.DockerIDLabel] != "workspace-id" { + t.Errorf("default ID label = %q, want %q", labels[config.DockerIDLabel], "workspace-id") + } + }) + + t.Run("escapes dollars but preserves other characters", func(t *testing.T) { + r := &runner{} + r.IDLabels = []string{"id.label=$value"} + + labels := r.buildServiceLabels(map[string]string{"extra": "it's $here"}) + + if labels["id.label"] != "$$value" { + t.Errorf("id.label = %q, want %q", labels["id.label"], "$$value") + } + // The apostrophe must be preserved; only "$" is doubled. + if labels["extra"] != "it's $$here" { + t.Errorf("extra = %q, want %q", labels["extra"], "it's $$here") + } + }) + + t.Run("splits ID label only on first equals sign", func(t *testing.T) { + r := &runner{} + r.IDLabels = []string{"id.label=a=b=c"} + + labels := r.buildServiceLabels(nil) + + if labels["id.label"] != "a=b=c" { + t.Errorf("id.label = %q, want %q", labels["id.label"], "a=b=c") + } + }) + + t.Run("additional labels merge alongside ID labels", func(t *testing.T) { + r := &runner{} + r.IDLabels = []string{"id.label=v"} + + labels := r.buildServiceLabels(map[string]string{"k": "plain"}) + + if labels["id.label"] != "v" { + t.Errorf("id.label = %q, want %q", labels["id.label"], "v") + } + if labels["k"] != "plain" { + t.Errorf("k = %q, want %q", labels["k"], "plain") + } + }) +} + +func TestResolveServiceEntrypoint(t *testing.T) { + override := true + t.Run("override command clears entrypoint and command", func(t *testing.T) { + entry, cmd := resolveServiceEntrypoint( + &config.MergedDevContainerConfig{ + DevContainerConfigBase: config.DevContainerConfigBase{OverrideCommand: &override}, + }, + &composetypes.ServiceConfig{Entrypoint: []string{"a"}, Command: []string{"b"}}, + &config.ImageDetails{}, + ) + if len(entry) != 0 || len(cmd) != 0 { + t.Errorf("expected empty entrypoint/command, got %v / %v", entry, cmd) + } + }) + + t.Run("falls back to image entrypoint and command", func(t *testing.T) { + entry, cmd := resolveServiceEntrypoint( + &config.MergedDevContainerConfig{}, + &composetypes.ServiceConfig{}, + &config.ImageDetails{Config: config.ImageDetailsConfig{ + Entrypoint: []string{"img-entry"}, + Cmd: []string{"img-cmd"}, + }}, + ) + if len(entry) != 1 || entry[0] != "img-entry" { + t.Errorf("entrypoint = %v, want [img-entry]", entry) + } + if len(cmd) != 1 || cmd[0] != "img-cmd" { + t.Errorf("command = %v, want [img-cmd]", cmd) + } + }) +} + +func TestNamedVolumesFromMounts(t *testing.T) { + t.Run("nil when no volume mounts", func(t *testing.T) { + if got := namedVolumesFromMounts([]*config.Mount{{Type: mountTypeBind}}); got != nil { + t.Errorf("expected nil, got %+v", got) + } + }) + + t.Run("collects volume mounts", func(t *testing.T) { + got := namedVolumesFromMounts([]*config.Mount{ + {Type: mountTypeVolume, Source: "data", External: true}, + {Type: mountTypeBind, Source: "/host"}, + }) + if len(got) != 1 { + t.Fatalf("expected 1 volume, got %d", len(got)) + } + v := got["data"] + if v.Name != "data" || !bool(v.External) { + t.Errorf("volume = %+v, want name=data external=true", v) + } + }) + + t.Run("skips anonymous volumes with empty source", func(t *testing.T) { + got := namedVolumesFromMounts([]*config.Mount{ + {Type: mountTypeVolume, Target: "/anon"}, + {Type: mountTypeVolume, Source: "named"}, + }) + if len(got) != 1 { + t.Fatalf("expected 1 named volume, got %d: %+v", len(got), got) + } + if _, ok := got[""]; ok { + t.Error("anonymous volume with empty source should not be declared") + } + }) +} + +func TestComposeBuildArgs(t *testing.T) { + modifierTests := []struct { + name string + pull bool + noCache bool + wantPull bool + wantNoCache bool + }{ + {name: "no modifiers"}, + {name: "pull only", pull: true, wantPull: true}, + {name: "no-cache only", noCache: true, wantNoCache: true}, + {name: "both", pull: true, noCache: true, wantPull: true, wantNoCache: true}, + } + for _, tt := range modifierTests { + t.Run(tt.name, func(t *testing.T) { + args := composeBuildArgs(&composeBuildArgsParams{ + projectName: "ws", + serviceName: "app", + pull: tt.pull, + noCache: tt.noCache, + }) + if got := slices.Contains(args, "--pull"); got != tt.wantPull { + t.Errorf("--pull present = %v, want %v (args=%v)", got, tt.wantPull, args) + } + if got := slices.Contains(args, "--no-cache"); got != tt.wantNoCache { + t.Errorf("--no-cache present = %v, want %v (args=%v)", got, tt.wantNoCache, args) + } + }) + } +} + +func TestComposeBuildArgsIncludesCoreArgs(t *testing.T) { + args := composeBuildArgs(&composeBuildArgsParams{ + projectName: "ws", + serviceName: "app", + overrideComposeFilePath: "/tmp/override.yml", + }) + + for _, want := range []string{composeProjectNameFlag, "ws", "build", "/tmp/override.yml"} { + if !slices.Contains(args, want) { + t.Errorf("expected %q in %v", want, args) + } + } +} + +func TestComposeBuildArgsRunServicesNoDuplicate(t *testing.T) { + args := composeBuildArgs(&composeBuildArgsParams{ + projectName: "ws", + serviceName: "app", + runServices: []string{"app", "db"}, + }) + + appCount := 0 + for _, a := range args { + if a == "app" { + appCount++ + } + } + if appCount != 1 { + t.Errorf("main service should appear once, got %d in %v", appCount, args) + } + if !slices.Contains(args, "db") { + t.Errorf("expected run service db in %v", args) + } +} diff --git a/pkg/devcontainer/compose_up.go b/pkg/devcontainer/compose_up.go new file mode 100644 index 000000000..885607aef --- /dev/null +++ b/pkg/devcontainer/compose_up.go @@ -0,0 +1,228 @@ +package devcontainer + +import ( + "reflect" + "strings" + + composetypes "github.com/compose-spec/compose-go/v2/types" + "github.com/devsy-org/devsy/pkg/compose" + "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/log" + "gopkg.in/yaml.v3" +) + +// composeLabelEscaper doubles "$" so Compose does not interpolate it as a +// variable reference. "$" is the only special character: labels are written as +// YAML and the compose CLI runs via argv (no shell), so escaping anything else +// (e.g. "'") would corrupt the stored value. +var composeLabelEscaper = strings.NewReplacer("$", "$$") + +func escapeComposeLabelValue(value string) string { + return composeLabelEscaper.Replace(value) +} + +func (r *runner) extendedDockerComposeUp(params *composeUpParams) (string, error) { + dockerComposeUpProject := r.generateDockerComposeUpProject(params) + dockerComposeData, err := yaml.Marshal(dockerComposeUpProject) + if err != nil { + return "", err + } + + dockerComposePath, err := r.writeComposeOverrideFile( + FeaturesStartOverrideFilePrefix, + dockerComposeData, + ) + if err != nil { + return "", err + } + + log.Debugf( + "Creating docker-compose up %s with content:\n %s", + dockerComposePath, + string(dockerComposeData), + ) + + return dockerComposePath, nil +} + +func (r *runner) generateDockerComposeUpProject(params *composeUpParams) *composetypes.Project { + mergedConfig := params.mergedConfig + composeService := params.composeService + + userEntrypoint, userCommand := resolveServiceEntrypoint( + mergedConfig, + composeService, + params.imageDetails, + ) + + overrideService := &composetypes.ServiceConfig{ + Name: composeService.Name, + Entrypoint: buildOverrideEntrypoint(mergedConfig, userEntrypoint), + Environment: mappingFromMap(mergedConfig.ContainerEnv), + Init: mergedConfig.Init, + CapAdd: mergedConfig.CapAdd, + SecurityOpt: mergedConfig.SecurityOpt, + Labels: r.buildServiceLabels(params.additionalLabels), + } + + if params.originalImageName != params.overrideImageName { + overrideService.Image = params.overrideImageName + } + + if !reflect.DeepEqual(userCommand, composeService.Command) { + overrideService.Command = userCommand + } + + if mergedConfig.ContainerUser != "" { + overrideService.User = mergedConfig.ContainerUser + } + + if mergedConfig.Privileged != nil { + overrideService.Privileged = *mergedConfig.Privileged + } + + gpuSupportEnabled := r.resolveComposeGPUAvailability(params.composeHelper) + r.configureGPUResources(params.parsedConfig, gpuSupportEnabled, overrideService) + + for _, mount := range mergedConfig.Mounts { + overrideService.Volumes = append( + overrideService.Volumes, + mountToServiceVolumeConfig(mount), + ) + } + + project := &composetypes.Project{ + Services: map[string]composetypes.ServiceConfig{ + overrideService.Name: *overrideService, + }, + Volumes: namedVolumesFromMounts(mergedConfig.Mounts), + } + + return project +} + +// resolveServiceEntrypoint determines the effective entrypoint and command for +// the overridden service. When OverrideCommand is set both are cleared so the +// devcontainer entrypoint takes over; otherwise the service values fall back to +// the image's own entrypoint and command. +func resolveServiceEntrypoint( + mergedConfig *config.MergedDevContainerConfig, + composeService *composetypes.ServiceConfig, + imageDetails *config.ImageDetails, +) (entrypoint, command []string) { + entrypoint = composeService.Entrypoint + command = composeService.Command + + if mergedConfig.OverrideCommand != nil && *mergedConfig.OverrideCommand { + return []string{}, []string{} + } + + if len(entrypoint) == 0 { + entrypoint = imageDetails.Config.Entrypoint + } + if len(command) == 0 { + command = imageDetails.Config.Cmd + } + return entrypoint, command +} + +// buildOverrideEntrypoint wraps the user entrypoint in the devcontainer startup +// shim that runs the configured entrypoint scripts before exec'ing the user +// command. +func buildOverrideEntrypoint( + mergedConfig *config.MergedDevContainerConfig, + userEntrypoint []string, +) composetypes.ShellCommand { + entrypoint := composetypes.ShellCommand{ + "/bin/sh", + "-c", + `echo Container started +trap "exit 0" 15 +` + strings.Join(mergedConfig.Entrypoints, "\n") + ` +exec "$$@" +` + DefaultEntrypoint, + "-", + } + return append(entrypoint, userEntrypoint...) +} + +// buildServiceLabels assembles the labels for the overridden service. ID labels +// (or the default ID label) identify the workspace, and any additional labels +// are merged in. All values are escaped to prevent compose variable expansion. +func (r *runner) buildServiceLabels(additionalLabels map[string]string) composetypes.Labels { + labels := composetypes.Labels{} + if len(r.IDLabels) > 0 { + for _, l := range r.IDLabels { + k, v, _ := strings.Cut(l, "=") + labels[k] = escapeComposeLabelValue(v) + } + } else { + labels[config.DockerIDLabel] = r.ID + } + for k, v := range additionalLabels { + labels.Add(k, escapeComposeLabelValue(v)) + } + return labels +} + +// namedVolumesFromMounts collects the named volumes referenced by the merged +// mounts so they can be declared at the project level. Returns nil when no +// volume-type mounts are present. +func namedVolumesFromMounts(mounts []*config.Mount) map[string]composetypes.VolumeConfig { + var volumes map[string]composetypes.VolumeConfig + for _, m := range mounts { + // Only named volumes are declared at the project level; anonymous + // volumes (empty source) stay service-scoped. + if m.Type != composetypes.VolumeTypeVolume || m.Source == "" { + continue + } + if volumes == nil { + volumes = map[string]composetypes.VolumeConfig{} + } + volumes[m.Source] = composetypes.VolumeConfig{ + Name: m.Source, + External: composetypes.External(m.External), + } + } + return volumes +} + +func (r *runner) resolveComposeGPUAvailability(composeHelper *compose.ComposeHelper) bool { + switch r.WorkspaceConfig.CLIOptions.GPUAvailability { + case stringTrue: + return true + case stringFalse: + return false + default: + available, _ := composeHelper.Docker.GPUSupportEnabled() + return available + } +} + +func (r *runner) configureGPUResources( + parsedConfig *config.SubstitutedConfig, + gpuSupportEnabled bool, + overrideService *composetypes.ServiceConfig, +) { + if parsedConfig.Config.HostRequirements != nil { + enableGPU, warnIfMissing := parsedConfig.Config.HostRequirements.ShouldEnableGPU( + gpuSupportEnabled, + ) + if enableGPU { + overrideService.Deploy = &composetypes.DeployConfig{ + Resources: composetypes.Resources{ + Reservations: &composetypes.Resource{ + Devices: []composetypes.DeviceRequest{ + { + Capabilities: []string{"gpu"}, + }, + }, + }, + }, + } + } + if warnIfMissing { + log.Warn("GPU required but not available on host") + } + } +} diff --git a/pkg/devcontainer/run.go b/pkg/devcontainer/run.go index 1ace5901e..d13287a54 100644 --- a/pkg/devcontainer/run.go +++ b/pkg/devcontainer/run.go @@ -28,14 +28,7 @@ type Runner interface { Find(ctx context.Context) (*config.ContainerDetails, error) - Command( - ctx context.Context, - user string, - command string, - stdin io.Reader, - stdout io.Writer, - stderr io.Writer, - ) error + Command(ctx context.Context, params CommandParams) error Stop(ctx context.Context) error @@ -48,6 +41,16 @@ type DeleteOptions struct { RemoveVolumes bool } +// CommandParams groups the inputs for running a command inside the dev +// container. +type CommandParams struct { + User string + Command string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer +} + func NewRunner( agentPath, agentDownloadURL string, workspaceConfig *provider2.AgentWorkspaceInfo, @@ -86,11 +89,34 @@ type runner struct { type UpOptions struct { provider2.CLIOptions - NoBuild bool - ForceBuild bool + // NoBuild is set by the container tunnel to force pre-built images; it is + // distinct from the embedded CLIOptions.NoBuild used by the build command. + NoBuild bool + // RegistryCache is sourced from AgentWorkspaceInfo.RegistryCache (a provider + // context option), not from CLIOptions, so it is a separate field. RegistryCache string } +// toBuildOptions derives the BuildOptions for an up-triggered build. It carries +// the full embedded CLIOptions (so build flags like Pull/ForceBuild are never +// dropped) and applies the up-specific NoBuild/RegistryCache overrides. +func (o UpOptions) toBuildOptions() provider2.BuildOptions { + return provider2.BuildOptions{ + CLIOptions: o.CLIOptions, + RegistryCache: o.RegistryCache, + NoBuild: o.NoBuild, + } +} + +// runContainerParams groups the inputs shared by the runSingleContainer, +// runDockerCompose, and runDefaultContainer dispatch methods. +type runContainerParams struct { + parsedConfig *config.SubstitutedConfig + substitutionContext *config.SubstitutionContext + options UpOptions + timeout time.Duration +} + func (r *runner) Up( ctx context.Context, options UpOptions, @@ -121,31 +147,55 @@ func (r *runner) Up( log.Info("Skipping initializeCommand on platform") } + runParams := &runContainerParams{ + parsedConfig: substitutedConfig, + substitutionContext: substitutionContext, + options: options, + timeout: timeout, + } + switch { case isDockerFileConfig(substitutedConfig.Config), substitutedConfig.Config.Image != "", substitutedConfig.Config.ContainerID != "": - return r.runSingleContainer( - ctx, - substitutedConfig, - substitutionContext, - options, - timeout, - ) + return r.runSingleContainer(ctx, runParams) case isDockerComposeConfig(substitutedConfig.Config): - return r.runDockerCompose(ctx, substitutedConfig, substitutionContext, options, timeout) + return r.runDockerCompose(ctx, runParams) default: - return r.runDefaultContainer(ctx, options, substitutedConfig, substitutionContext, timeout) + return r.runDefaultContainer(ctx, runParams) } } +func (r *runner) Command(ctx context.Context, params CommandParams) error { + return r.Driver.CommandDevContainer(ctx, &driver.CommandParams{ + WorkspaceID: r.ID, + User: params.User, + Command: params.Command, + Stdin: params.Stdin, + Stdout: params.Stdout, + Stderr: params.Stderr, + }) +} + +func (r *runner) Find(ctx context.Context) (*config.ContainerDetails, error) { + containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + if err != nil { + return nil, fmt.Errorf("find dev container: %w", err) + } + + return containerDetails, nil +} + +func (r *runner) Logs(ctx context.Context, writer io.Writer) error { + return r.Driver.GetDevContainerLogs(ctx, r.ID, writer, writer) +} + func (r *runner) runDefaultContainer( ctx context.Context, - options UpOptions, - substitutedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - timeout time.Duration, + params *runContainerParams, ) (*config.Result, error) { + options := params.options + substitutedConfig := params.parsedConfig if options.FallbackImage != "" { log.Warn( "dev container config is missing one of \"image\", \"dockerFile\" or \"dockerComposeFile\" properties, " + @@ -178,38 +228,7 @@ func (r *runner) runDefaultContainer( substitutedConfig.Config.ImageContainer = language.MapConfig[lang].ImageContainer } - return r.runSingleContainer(ctx, substitutedConfig, substitutionContext, options, timeout) -} - -func (r *runner) Command( - ctx context.Context, - user string, - command string, - stdin io.Reader, - stdout io.Writer, - stderr io.Writer, -) error { - return r.Driver.CommandDevContainer(ctx, &driver.CommandParams{ - WorkspaceID: r.ID, - User: user, - Command: command, - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - }) -} - -func (r *runner) Find(ctx context.Context) (*config.ContainerDetails, error) { - containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) - if err != nil { - return nil, fmt.Errorf("find dev container: %w", err) - } - - return containerDetails, nil -} - -func (r *runner) Logs(ctx context.Context, writer io.Writer) error { - return r.Driver.GetDevContainerLogs(ctx, r.ID, writer, writer) + return r.runSingleContainer(ctx, params) } func isDockerFileConfig(config *config.DevContainerConfig) bool { diff --git a/pkg/devcontainer/run_test.go b/pkg/devcontainer/run_test.go index 444673a28..76ed9038c 100644 --- a/pkg/devcontainer/run_test.go +++ b/pkg/devcontainer/run_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/devsy-org/devsy/pkg/devcontainer/config" + provider2 "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/types" ) @@ -294,3 +295,41 @@ func TestMountSetConsistency(t *testing.T) { }) } } + +func TestUpOptionsToBuildOptions(t *testing.T) { + up := UpOptions{ + CLIOptions: provider2.CLIOptions{ + Pull: true, + ForceBuild: true, + ForceDockerless: true, + ExtraDevContainerPath: "/extra.json", + }, + NoBuild: true, + RegistryCache: "registry.example.com/cache", + } + + got := up.toBuildOptions() + + // Build flags carried via the embedded CLIOptions must survive the + // conversion (regression guard against the old lossy field-by-field copy). + if !got.Pull { + t.Error("Pull should be carried through to BuildOptions") + } + if !got.ForceBuild { + t.Error("ForceBuild should be carried through to BuildOptions") + } + if !got.ForceDockerless { + t.Error("ForceDockerless should be carried through to BuildOptions") + } + if got.ExtraDevContainerPath != "/extra.json" { + t.Errorf("ExtraDevContainerPath = %q, want /extra.json", got.ExtraDevContainerPath) + } + + // Up-specific overrides. + if !got.NoBuild { + t.Error("NoBuild override should be applied") + } + if got.RegistryCache != "registry.example.com/cache" { + t.Errorf("RegistryCache = %q, want registry.example.com/cache", got.RegistryCache) + } +} diff --git a/pkg/devcontainer/single.go b/pkg/devcontainer/single.go index 34230f4e5..4dfe350f5 100644 --- a/pkg/devcontainer/single.go +++ b/pkg/devcontainer/single.go @@ -6,7 +6,6 @@ import ( "fmt" "maps" "strings" - "time" "github.com/devsy-org/devsy/pkg/agent/delivery" "github.com/devsy-org/devsy/pkg/command" @@ -16,7 +15,6 @@ import ( "github.com/devsy-org/devsy/pkg/devcontainer/metadata" "github.com/devsy-org/devsy/pkg/driver" "github.com/devsy-org/devsy/pkg/log" - provider2 "github.com/devsy-org/devsy/pkg/provider" ) var dockerlessImage = "ghcr.io/devsy-org/dockerless:0.2.0" @@ -52,55 +50,32 @@ type resolveParams struct { func (r *runner) runSingleContainer( ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, - options UpOptions, - timeout time.Duration, + runParams *runContainerParams, ) (*config.Result, error) { + parsedConfig := runParams.parsedConfig + substitutionContext := runParams.substitutionContext + options := runParams.options + timeout := runParams.timeout + log.Debugf("starting devcontainer for workspace %s", r.ID) substitutionContext.Userns = options.Userns substitutionContext.UidMap = options.UidMap substitutionContext.GidMap = options.GidMap - // Check if Docker exists before trying to find containers - var containerDetails *config.ContainerDetails - var err error - dockerCmd := "docker" - if r.WorkspaceConfig.Agent.Docker.Path != "" { - dockerCmd = r.WorkspaceConfig.Agent.Docker.Path - } - if command.Exists(dockerCmd) { - containerDetails, err = r.Driver.FindDevContainer(ctx, r.ID) - if err != nil { - return nil, fmt.Errorf("find dev container: %w", err) - } + containerDetails, err := r.findExistingDevContainer(ctx) + if err != nil { + return nil, err } - // Resolve container: ensure we have a running container with merged config. - var resolved *resolvedContainer - params := &resolveParams{ parsedConfig: parsedConfig, substitutionContext: substitutionContext, options: options, } - if options.Recreate && parsedConfig.Config.ContainerID != "" { - return nil, fmt.Errorf("cannot recreate container not created by Devsy") - } else if !options.Recreate && containerDetails != nil { - if actual := workspaceMountDestination(containerDetails); actual != "" && - actual != substitutionContext.ContainerWorkspaceFolder { - log.Infof( - "container workspace mount is %s, updating from computed %s", - actual, substitutionContext.ContainerWorkspaceFolder, - ) - substitutionContext.ContainerWorkspaceFolder = actual - } - resolved, err = r.resolveExistingContainer(ctx, containerDetails, params) - } else { - resolved, err = r.resolveNewContainer(ctx, params) - } + // Resolve container: ensure we have a running container with merged config. + resolved, err := r.resolveContainer(ctx, params, containerDetails) if err != nil { return nil, err } @@ -115,6 +90,57 @@ func (r *runner) runSingleContainer( }) } +// resolveContainer ensures a running container with merged config, either by +// reusing the existing container or creating a new one. Recreating a container +// not created by Devsy (i.e. one with an explicit ContainerID) is rejected. +func (r *runner) resolveContainer( + ctx context.Context, + params *resolveParams, + containerDetails *config.ContainerDetails, +) (*resolvedContainer, error) { + options := params.options + + if options.Recreate && params.parsedConfig.Config.ContainerID != "" { + return nil, fmt.Errorf("cannot recreate container not created by Devsy") + } + + if options.Recreate || containerDetails == nil { + return r.resolveNewContainer(ctx, params) + } + + substitutionContext := params.substitutionContext + if actual := workspaceMountDestination(containerDetails); actual != "" && + actual != substitutionContext.ContainerWorkspaceFolder { + log.Infof( + "container workspace mount is %s, updating from computed %s", + actual, substitutionContext.ContainerWorkspaceFolder, + ) + substitutionContext.ContainerWorkspaceFolder = actual + } + return r.resolveExistingContainer(ctx, containerDetails, params) +} + +// findExistingDevContainer looks up the dev container, first checking that the +// configured docker command exists. Returns nil details (without error) when +// docker is unavailable. +func (r *runner) findExistingDevContainer( + ctx context.Context, +) (*config.ContainerDetails, error) { + dockerCmd := "docker" + if r.WorkspaceConfig.Agent.Docker.Path != "" { + dockerCmd = r.WorkspaceConfig.Agent.Docker.Path + } + if !command.Exists(dockerCmd) { + return nil, nil + } + + containerDetails, err := r.Driver.FindDevContainer(ctx, r.ID) + if err != nil { + return nil, fmt.Errorf("find dev container: %w", err) + } + return containerDetails, nil +} + // resolveExistingContainer handles the case where a container already exists. // It starts the container if stopped, merges configuration from container // metadata, and optionally reprovisions. Returns fresh container details. @@ -155,7 +181,7 @@ func (r *runner) ensureRunning( ctx context.Context, containerDetails *config.ContainerDetails, ) (*config.ContainerDetails, error) { - if strings.ToLower(containerDetails.State.Status) == "running" { + if strings.ToLower(containerDetails.State.Status) == containerStatusRunning { return containerDetails, nil } @@ -231,50 +257,13 @@ func (r *runner) resolveNewContainer( ctx context.Context, p *resolveParams, ) (*resolvedContainer, error) { - hostWarnings, hostErr := config.ValidateHostRequirements( - p.parsedConfig.Config.HostRequirements, - config.SystemHostInfo{}, - p.substitutionContext.LocalWorkspaceFolder, - ) - if hostErr != nil { - if !p.options.SkipHostRequirements { - return nil, hostErr - } - hostWarnings = append(hostWarnings, hostErr.Error()) - } - - buildInfo, err := r.build(ctx, p.parsedConfig, p.substitutionContext, provider2.BuildOptions{ - CLIOptions: provider2.CLIOptions{ - PrebuildRepositories: p.options.PrebuildRepositories, - ForceDockerless: p.options.ForceDockerless, - Platform: p.options.Platform, - ExtraDevContainerPath: p.options.ExtraDevContainerPath, - }, - NoBuild: p.options.NoBuild, - RegistryCache: p.options.RegistryCache, - ExportCache: false, - }) + hostWarnings, err := r.newContainerHostWarnings(p) if err != nil { - return nil, fmt.Errorf("build image: %w", err) - } - - if p.options.Recreate { - if err := r.deleteForRecreate(ctx); err != nil { - return nil, err - } + return nil, err } - mergedConfig, err := config.MergeConfiguration( - p.parsedConfig.Config, - buildInfo.ImageMetadata.Config, - ) + buildInfo, mergedConfig, err := r.buildNewContainerConfig(ctx, p) if err != nil { - return nil, fmt.Errorf("merge config: %w", err) - } - - if err := config.MergeExtraRemoteEnv( - mergedConfig, p.options.ExtraDevContainerPath, - ); err != nil { return nil, err } @@ -287,7 +276,7 @@ func (r *runner) resolveNewContainer( } } - err = r.runContainer(ctx, p.parsedConfig, p.substitutionContext, mergedConfig, buildInfo) + err = r.runContainer(ctx, p, mergedConfig, buildInfo) if err != nil { return nil, fmt.Errorf("runner run container: %w", err) } @@ -304,6 +293,65 @@ func (r *runner) resolveNewContainer( }, nil } +// buildNewContainerConfig builds the image (deleting the existing container +// first when recreating) and produces the merged devcontainer config from the +// build's image metadata. +func (r *runner) buildNewContainerConfig( + ctx context.Context, + p *resolveParams, +) (*config.BuildInfo, *config.MergedDevContainerConfig, error) { + buildInfo, err := r.build( + ctx, + p.parsedConfig, + p.substitutionContext, + p.options.toBuildOptions(), + ) + if err != nil { + return nil, nil, fmt.Errorf("build image: %w", err) + } + + if p.options.Recreate { + if err := r.deleteForRecreate(ctx); err != nil { + return nil, nil, err + } + } + + mergedConfig, err := config.MergeConfiguration( + p.parsedConfig.Config, + buildInfo.ImageMetadata.Config, + ) + if err != nil { + return nil, nil, fmt.Errorf("merge config: %w", err) + } + + if err := config.MergeExtraRemoteEnv( + mergedConfig, + p.options.ExtraDevContainerPath, + ); err != nil { + return nil, nil, err + } + + return buildInfo, mergedConfig, nil +} + +// newContainerHostWarnings validates host requirements for a new container, +// returning warnings. Unmet requirements error unless SkipHostRequirements is +// set, in which case the error is downgraded to a warning. +func (r *runner) newContainerHostWarnings(p *resolveParams) ([]string, error) { + hostWarnings, hostErr := config.ValidateHostRequirements( + p.parsedConfig.Config.HostRequirements, + config.SystemHostInfo{}, + p.substitutionContext.LocalWorkspaceFolder, + ) + if hostErr != nil { + if !p.options.SkipHostRequirements { + return nil, hostErr + } + hostWarnings = append(hostWarnings, hostErr.Error()) + } + return hostWarnings, nil +} + // deleteForRecreate removes the existing container before recreating it. // Docker containers are fully deleted; other drivers stop the container. func (r *runner) deleteForRecreate(ctx context.Context) error { @@ -416,8 +464,7 @@ func (r *runner) deliverPreStart(ctx context.Context, runOptions *driver.RunOpti func (r *runner) runContainer( ctx context.Context, - parsedConfig *config.SubstitutedConfig, - substitutionContext *config.SubstitutionContext, + p *resolveParams, mergedConfig *config.MergedDevContainerConfig, buildInfo *config.BuildInfo, ) error { @@ -426,13 +473,13 @@ func (r *runner) runContainer( // build run options for dockerless mode var runOptions *driver.RunOptions if buildInfo.Dockerless != nil { - runOptions, err = r.getDockerlessRunOptions(mergedConfig, substitutionContext, buildInfo) + runOptions, err = r.getDockerlessRunOptions(mergedConfig, p.substitutionContext, buildInfo) if err != nil { return fmt.Errorf("build dockerless run options: %w", err) } } else { // build run options - runOptions, err = r.getRunOptions(mergedConfig, substitutionContext, buildInfo) + runOptions, err = r.getRunOptions(mergedConfig, p.substitutionContext, buildInfo) if err != nil { return fmt.Errorf("build run options: %w", err) } @@ -446,7 +493,7 @@ func (r *runner) runContainer( return dockerDriver.RunDockerDevContainer(ctx, &driver.RunDockerDevContainerParams{ WorkspaceID: r.ID, Options: runOptions, - ParsedConfig: parsedConfig.Config, + ParsedConfig: p.parsedConfig.Config, IDE: r.WorkspaceConfig.Workspace.IDE.Name, IDEOptions: r.WorkspaceConfig.Workspace.IDE.Options, LocalWorkspaceFolder: r.LocalWorkspaceFolder, @@ -458,25 +505,32 @@ func (r *runner) runContainer( return r.Driver.RunDevContainer(ctx, r.ID, runOptions) } -func (r *runner) getDockerlessRunOptions( - mergedConfig *config.MergedDevContainerConfig, - substitutionContext *config.SubstitutionContext, - buildInfo *config.BuildInfo, -) (*driver.RunOptions, error) { - // parse workspace mount — nil when suppressed via workspaceMount: "" - var workspaceMountPtr *config.Mount - if substitutionContext.WorkspaceMount != "" { - parsed := config.ParseMount(substitutionContext.WorkspaceMount) - workspaceMountPtr = &parsed +// parseWorkspaceMount parses the substituted workspace mount, returning nil when +// it has been suppressed via an empty workspaceMount. +func parseWorkspaceMount(substitutionContext *config.SubstitutionContext) *config.Mount { + if substitutionContext.WorkspaceMount == "" { + return nil } + parsed := config.ParseMount(substitutionContext.WorkspaceMount) + return &parsed +} - // add metadata as label here - marshalled, err := metadata.MarshalImageMetadata(buildInfo.ImageMetadata.Raw) - if err != nil { - return nil, fmt.Errorf("marshal config: %w", err) +// workspaceUID returns the workspace UID, or an empty string when unavailable. +func (r *runner) workspaceUID() string { + if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil { + return r.WorkspaceConfig.Workspace.UID } + return "" +} + +// dockerlessEnv builds the environment for a dockerless build container, +// combining the kaniko/dockerless settings with the merged container env. +func (r *runner) dockerlessEnv( + mergedConfig *config.MergedDevContainerConfig, + buildInfo *config.BuildInfo, +) (map[string]string, error) { env := map[string]string{ - "DOCKERLESS": "true", + "DOCKERLESS": stringTrue, "DOCKERLESS_CONTEXT": buildInfo.Dockerless.Context, "DOCKERLESS_DOCKERFILE": buildInfo.Dockerless.Dockerfile, "GODEBUG": "http2client=0", // https://github.com/GoogleContainerTools/kaniko/issues/875 @@ -490,9 +544,28 @@ func (r *runner) getDockerlessRunOptions( if err != nil { return nil, fmt.Errorf("marshal build args: %w", err) } - env["DOCKERLESS_BUILD_ARGS"] = string(out) } + return env, nil +} + +func (r *runner) getDockerlessRunOptions( + mergedConfig *config.MergedDevContainerConfig, + substitutionContext *config.SubstitutionContext, + buildInfo *config.BuildInfo, +) (*driver.RunOptions, error) { + workspaceMountPtr := parseWorkspaceMount(substitutionContext) + + // add metadata as label here + marshalled, err := metadata.MarshalImageMetadata(buildInfo.ImageMetadata.Raw) + if err != nil { + return nil, fmt.Errorf("marshal config: %w", err) + } + + env, err := r.dockerlessEnv(mergedConfig, buildInfo) + if err != nil { + return nil, err + } image := dockerlessImage if r.WorkspaceConfig != nil && r.WorkspaceConfig.Agent.Dockerless.Image != "" { @@ -507,16 +580,11 @@ func (r *runner) getDockerlessRunOptions( Target: "/workspaces/.dockerless", }) - uid := "" - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil { - uid = r.WorkspaceConfig.Workspace.UID - } - // build run options return &driver.RunOptions{ - UID: uid, + UID: r.workspaceUID(), Image: image, - User: "root", + User: containerRootUser, Entrypoint: "/.dockerless/dockerless", Cmd: []string{ "start", @@ -548,12 +616,7 @@ func (r *runner) getRunOptions( substitutionContext *config.SubstitutionContext, buildInfo *config.BuildInfo, ) (*driver.RunOptions, error) { - // parse workspace mount — nil when suppressed via workspaceMount: "" - var workspaceMountPtr *config.Mount - if substitutionContext.WorkspaceMount != "" { - parsed := config.ParseMount(substitutionContext.WorkspaceMount) - workspaceMountPtr = &parsed - } + workspaceMountPtr := parseWorkspaceMount(substitutionContext) // add metadata as label here marshalled, err := metadata.MarshalImageMetadata(buildInfo.ImageMetadata.Raw) @@ -580,11 +643,6 @@ func (r *runner) getRunOptions( user = mergedConfig.ContainerUser } - uid := "" - if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil { - uid = r.WorkspaceConfig.Workspace.UID - } - // Resolve any ${containerEnv:VAR} references in containerEnv values using // the image's inspected environment. ${containerWorkspaceFolder} and // ${containerWorkspaceFolderBasename} are already substituted upstream in @@ -602,7 +660,7 @@ func (r *runner) getRunOptions( mergedConfig.ContainerEnv = resolvedContainerEnv return &driver.RunOptions{ - UID: uid, + UID: r.workspaceUID(), Image: buildInfo.ImageName, User: user, Entrypoint: entrypoint, @@ -629,8 +687,8 @@ func (r *runner) addExtraEnvVars(env map[string]string) map[string]string { env = make(map[string]string) } - env[DevsyExtraEnvVar] = "true" - env[RemoteContainersExtraEnvVar] = "true" + env[DevsyExtraEnvVar] = stringTrue + env[RemoteContainersExtraEnvVar] = stringTrue if r.WorkspaceConfig != nil && r.WorkspaceConfig.Workspace != nil && r.WorkspaceConfig.Workspace.ID != "" { env[pkgconfig.EnvWorkspaceID] = r.WorkspaceConfig.Workspace.ID diff --git a/pkg/driver/docker/build.go b/pkg/driver/docker/build.go index 9df78524e..184f5f559 100644 --- a/pkg/driver/docker/build.go +++ b/pkg/driver/docker/build.go @@ -101,6 +101,9 @@ func buildDockerBuildxArgs(options *build.BuildOptions, platform string) []strin if options.NoCache { args = append(args, "--no-cache") } + if options.Pull { + args = append(args, "--pull") + } args = appendImageTags(args, options.Images) args = appendBuildArgsAndContexts(args, options.BuildArgs, options.Contexts) args = appendLabels(args, options.Labels) diff --git a/pkg/driver/docker/build_test.go b/pkg/driver/docker/build_test.go index 896b9e728..a6442b937 100644 --- a/pkg/driver/docker/build_test.go +++ b/pkg/driver/docker/build_test.go @@ -4,8 +4,10 @@ import ( "os" "os/exec" "path/filepath" + "slices" "testing" + "github.com/devsy-org/devsy/pkg/devcontainer/build" "github.com/devsy-org/devsy/pkg/docker" "github.com/devsy-org/devsy/pkg/provider" "github.com/stretchr/testify/assert" @@ -109,3 +111,29 @@ func TestSelectStrategy_DockerRuntime_BuildxWhenAvailable(t *testing.T) { t.Fatalf("unexpected strategy type: %T", strategy) } } + +func TestBuildDockerBuildxArgs_PullAndNoCache(t *testing.T) { + tests := []struct { + name string + opts *build.BuildOptions + wantPull bool + wantNoCach bool + }{ + {name: "neither", opts: &build.BuildOptions{}}, + {name: "pull only", opts: &build.BuildOptions{Pull: true}, wantPull: true}, + {name: "no-cache only", opts: &build.BuildOptions{NoCache: true}, wantNoCach: true}, + { + name: "both", + opts: &build.BuildOptions{Pull: true, NoCache: true}, + wantPull: true, + wantNoCach: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := buildDockerBuildxArgs(tt.opts, "") + assert.Equal(t, tt.wantPull, slices.Contains(args, "--pull"), "args=%v", args) + assert.Equal(t, tt.wantNoCach, slices.Contains(args, "--no-cache"), "args=%v", args) + }) + } +} diff --git a/pkg/provider/workspace.go b/pkg/provider/workspace.go index ba3b3d240..d3862f423 100644 --- a/pkg/provider/workspace.go +++ b/pkg/provider/workspace.go @@ -296,6 +296,9 @@ type CLIOptions struct { // ForceBuild forces a rebuild even if a cached image exists. ForceBuild bool `json:"forceBuild,omitempty"` + // Pull re-pulls base images during the build (docker build --pull) instead + // of reusing locally cached layers. + Pull bool `json:"pull,omitempty"` // ForceDockerless forces the use of a dockerless build approach. ForceDockerless bool `json:"forceDockerless,omitempty"` // ForceInternalBuildKit forces the use of internal BuildKit instead of docker buildx.