diff --git a/go.mod b/go.mod index a5174c95..9c797fc6 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/asottile/dockerfile v3.1.0+incompatible github.com/blang/semver v3.5.1+incompatible github.com/briandowns/spinner v1.23.2 - github.com/codefly-dev/core v0.3.5 + github.com/codefly-dev/core v0.3.6 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go v0.1.65 diff --git a/go.sum b/go.sum index f9df9260..de6023ba 100644 --- a/go.sum +++ b/go.sum @@ -95,8 +95,8 @@ github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJ github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/cloudflare/circl v1.6.5 h1:O64F26HEqNhznd/hrC5KZXVKYuKM2rx4deZDTc4ihQA= github.com/cloudflare/circl v1.6.5/go.mod h1:h5LNyxAc5nTue9DS5jT+48en2PSDYt3zdGnz5OstK6c= -github.com/codefly-dev/core v0.3.5 h1:uZRZcuWD62KZMcL2u1zinfq1cGvdsuJJD1WvcM9OqxM= -github.com/codefly-dev/core v0.3.5/go.mod h1:ca5IAJnFJSC3OPl5DcoRIi41BMfO0As8BaDGciYZ7nI= +github.com/codefly-dev/core v0.3.6 h1:lt5EyL+uS1Z6Vzg+V/HhpnIV43Lu5rN9E0k+ZslIKHk= +github.com/codefly-dev/core v0.3.6/go.mod h1:ca5IAJnFJSC3OPl5DcoRIi41BMfO0As8BaDGciYZ7nI= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go new file mode 100644 index 00000000..842ac71b --- /dev/null +++ b/pkg/orchestration/build_plan.go @@ -0,0 +1,307 @@ +package orchestration + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + + coreservices "github.com/codefly-dev/core/agents/services" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/wool" +) + +const ( + // buildRecipeDir is the service-relative directory an agent emits its build + // recipes into. It is the committed, durable location a consumer rebuilds from + // (docker buildx -f services//builder/Dockerfile services/), and the + // Dockerfiles COPY builder/… paths relative to the service-directory context. + buildRecipeDir = "builder" + // buildxBuilderName is the dedicated docker-container buildx builder the CLI + // creates for multi-platform builds. The default buildx builder uses the + // "docker" driver, which cannot build multiple platforms. + buildxBuilderName = "codefly" +) + +// buildFromPlan owns the docker build the agent used to run in-process. It +// verifies the recipe tree the agent emitted, then runs docker buildx for each +// recipe — multi-arch and pushed as a manifest list when pushing — so the build +// recipe is a durable, first-class artifact and images are not tied to the +// builder's host architecture. +func (b *Builder) buildFromPlan(ctx context.Context, outputDir string, plan *builderv0.DockerBuildPlan) error { + w := wool.Get(ctx).In("Builder.buildFromPlan", wool.ThisField(b.instance)) + if err := coreservices.VerifyDockerBuildPlan(outputDir, plan); err != nil { + return w.Wrapf(err, "cannot verify build recipe for %s", b.instance.Unique()) + } + recipes := plan.GetRecipes() + if len(recipes) == 0 { + return w.NewError("build plan for %s contains no recipes", b.instance.Unique()) + } + shouldPush := push.Load() + if b.world.Mode == SnapshotMode { + if len(recipes) != 1 { + return w.NewError("snapshot build for %s emitted %d recipes; exactly one deployable image is required", b.instance.Unique(), len(recipes)) + } + if !shouldPush { + return w.NewError("snapshot build for %s requires push to resolve an immutable image digest", b.instance.Unique()) + } + } + serviceDir := b.instance.Service.Dir() + for _, recipe := range recipes { + if err := b.buildRecipe(ctx, w, outputDir, serviceDir, recipe, shouldPush); err != nil { + return err + } + } + return nil +} + +// buildRecipe builds and (when pushing) publishes one recipe. It refuses to push +// an image that omits the deployment architecture, applies the recipe's declared +// ignore file, provisions a multi-platform builder when required, and resolves +// the pushed manifest digest for snapshot builds. +func (b *Builder) buildRecipe( + ctx context.Context, + w *wool.Wool, + outputDir, serviceDir string, + recipe *builderv0.DockerBuildRecipe, + shouldPush bool, +) error { + if shouldPush && !platformsIncludeDeploymentArch(recipe.GetPlatforms()) { + return w.NewError( + "recipe %s of %s targets platforms %v but deployment nodes require linux/%s; the recipe must build %s", + recipe.GetName(), b.instance.Unique(), recipe.GetPlatforms(), deploymentImageArchitecture, deploymentImageArchitecture, + ) + } + dockerfile, err := recipeDockerfile(outputDir, recipe) + if err != nil { + return w.Wrapf(err, "cannot resolve dockerfile for recipe %s of %s", recipe.GetName(), b.instance.Unique()) + } + contextDir, err := recipeContext(serviceDir, recipe) + if err != nil { + return w.Wrapf(err, "cannot resolve build context for recipe %s of %s", recipe.GetName(), b.instance.Unique()) + } + + cleanupIgnore, err := applyRecipeIgnore(outputDir, dockerfile, recipe) + if err != nil { + return w.Wrapf(err, "cannot apply ignore file for recipe %s of %s", recipe.GetName(), b.instance.Unique()) + } + defer cleanupIgnore() + + multiArch := shouldPush && len(recipe.GetPlatforms()) > 1 + if multiArch { + if err := ensureBuildxBuilder(ctx); err != nil { + return w.Wrapf(err, "cannot provision multi-architecture builder for %s", b.instance.Unique()) + } + } + + var metadataFile string + if b.world.Mode == SnapshotMode { + file, err := os.CreateTemp("", "codefly-build-metadata-*.json") + if err != nil { + return w.Wrapf(err, "cannot stage build metadata for %s", b.instance.Unique()) + } + metadataFile = file.Name() + _ = file.Close() + defer os.Remove(metadataFile) + } + + args := buildxArgs(recipe, dockerfile, contextDir, shouldPush, multiArch, metadataFile) + w.Info("building image", wool.Field("image", recipe.GetImage()), wool.Field("push", shouldPush)) + command := exec.CommandContext(ctx, "docker", args...) + command.Stdout = os.Stderr + command.Stderr = os.Stderr + if err := command.Run(); err != nil { + return w.Wrapf(err, "cannot build %s", recipe.GetImage()) + } + + if b.world.Mode == SnapshotMode { + digest, err := readPushedImageDigest(metadataFile) + if err != nil { + return w.Wrapf(err, "cannot resolve immutable image for %s", b.instance.Unique()) + } + b.imageDigest = digest + } + return nil +} + +// buildxArgs renders the docker buildx argv for one recipe. A push builds every +// requested platform into one manifest list; a local build cannot materialize a +// multi-platform manifest list, so it targets a single platform and loads it +// into the daemon. Multi-platform builds run on the dedicated container-driver +// builder, and a metadata file captures the pushed manifest digest. +func buildxArgs(recipe *builderv0.DockerBuildRecipe, dockerfile, contextDir string, push, multiArch bool, metadataFile string) []string { + args := []string{"buildx", "build"} + if multiArch { + args = append(args, "--builder", buildxBuilderName) + } + platforms := recipe.GetPlatforms() + if push { + if len(platforms) > 0 { + args = append(args, "--platform", strings.Join(platforms, ",")) + } + args = append(args, "--push") + } else { + if len(platforms) > 0 { + args = append(args, "--platform", platforms[0]) + } + args = append(args, "--load") + } + if metadataFile != "" { + args = append(args, "--metadata-file", metadataFile) + } + if target := recipe.GetTarget(); target != "" { + args = append(args, "--target", target) + } + buildArgs := recipe.GetBuildArgs() + keys := make([]string, 0, len(buildArgs)) + for key := range buildArgs { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + args = append(args, "--build-arg", fmt.Sprintf("%s=%s", key, buildArgs[key])) + } + return append(args, "-t", recipe.GetImage(), "-f", dockerfile, contextDir) +} + +// platformsIncludeDeploymentArch reports whether the recipe builds the +// architecture deployment nodes run. An empty platform list also fails: without +// an explicit platform buildx builds only the builder's host architecture, which +// on Apple silicon is the arm64 image that cannot run on amd64 nodes. +func platformsIncludeDeploymentArch(platforms []string) bool { + for _, platform := range platforms { + fields := strings.Split(platform, "/") + if len(fields) >= 2 && fields[1] == deploymentImageArchitecture { + return true + } + } + return false +} + +// recipeDockerfile resolves a recipe's Dockerfile within the emitted recipe tree +// and rejects a path that escapes it. VerifyDockerBuildPlan digests only the file +// tree, not the recipe fields, so an unconstrained dockerfile could point +// buildx -f at an out-of-tree file — the same escape recipeContext guards for the +// build context. +func recipeDockerfile(outputDir string, recipe *builderv0.DockerBuildRecipe) (string, error) { + dockerfile := filepath.Join(outputDir, filepath.FromSlash(recipe.GetDockerfile())) + rel, err := filepath.Rel(outputDir, dockerfile) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("recipe dockerfile %q escapes the recipe directory", recipe.GetDockerfile()) + } + return dockerfile, nil +} + +// recipeContext resolves a recipe's build context and rejects a context that +// escapes the service directory. +func recipeContext(serviceDir string, recipe *builderv0.DockerBuildRecipe) (string, error) { + relative := recipe.GetContext() + if relative == "" || relative == "." { + return serviceDir, nil + } + contextDir := filepath.Join(serviceDir, filepath.FromSlash(relative)) + rel, err := filepath.Rel(serviceDir, contextDir) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("recipe context %q escapes the service directory", relative) + } + return contextDir, nil +} + +// applyRecipeIgnore makes the recipe's declared ignore file visible to buildx, +// which only discovers ".dockerignore" or "/.dockerignore" +// — never the "builder/dockerignore" name agents emit. It writes the ignore to +// the discovered sibling path for the duration of the build and returns a +// cleanup. It is a no-op when the recipe declares no ignore or already emits it +// at the discovered path. +func applyRecipeIgnore(outputDir, dockerfile string, recipe *builderv0.DockerBuildRecipe) (func(), error) { + ignore := recipe.GetDockerignore() + if ignore == "" { + return func() {}, nil + } + source := filepath.Join(outputDir, filepath.FromSlash(ignore)) + target := dockerfile + ".dockerignore" + if source == target { + return func() {}, nil + } + input, err := os.Open(source) + if err != nil { + return nil, err + } + defer input.Close() + output, err := os.OpenFile(target, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if err != nil { + return nil, fmt.Errorf("stage recipe ignore at %s: %w", target, err) + } + if _, err := io.Copy(output, input); err != nil { + _ = output.Close() + _ = os.Remove(target) + return nil, err + } + if err := output.Close(); err != nil { + _ = os.Remove(target) + return nil, err + } + return func() { _ = os.Remove(target) }, nil +} + +// ensureBuildxBuilder provisions the dedicated docker-container buildx builder +// used for multi-platform builds. It is idempotent and tolerates a concurrent +// creation racing another service's build. +func ensureBuildxBuilder(ctx context.Context) error { + if buildxBuilderExists(ctx) { + return nil + } + output, err := exec.CommandContext( + ctx, "docker", "buildx", "create", + "--name", buildxBuilderName, "--driver", "docker-container", "--bootstrap", + ).CombinedOutput() + if err != nil { + if buildxBuilderExists(ctx) { + return nil + } + return fmt.Errorf("create buildx builder %q: %w: %s", buildxBuilderName, err, strings.TrimSpace(string(output))) + } + return nil +} + +func buildxBuilderExists(ctx context.Context) bool { + return exec.CommandContext(ctx, "docker", "buildx", "inspect", buildxBuilderName).Run() == nil +} + +// readPushedImageDigest reads the registry manifest digest buildx recorded in +// its metadata file. A pushed multi-platform build never lands in the local +// image store, so the digest cannot be recovered with docker image inspect. +func readPushedImageDigest(metadataFile string) (string, error) { + input, err := os.Open(metadataFile) + if err != nil { + return "", err + } + defer input.Close() + data, err := io.ReadAll(input) + if err != nil { + return "", err + } + var metadata struct { + Digest string `json:"containerimage.digest"` + } + if err := json.Unmarshal(data, &metadata); err != nil { + return "", fmt.Errorf("decode build metadata: %w", err) + } + if !sha256Digest.MatchString(metadata.Digest) { + return "", fmt.Errorf("build produced no registry-backed sha256 digest; push the image before pinning it") + } + return metadata.Digest, nil +} + +// buildRecipeOutputDirectory is the absolute destination the caller asks the +// agent to emit recipes into: the committed builder/ directory under the +// service. It does not create the directory — the emitting agent owns writing +// there, so a legacy agent that ignores the field leaves no empty directory. +func buildRecipeOutputDirectory(serviceDir string) (string, error) { + return filepath.Abs(filepath.Join(serviceDir, buildRecipeDir)) +} diff --git a/pkg/orchestration/build_plan_test.go b/pkg/orchestration/build_plan_test.go new file mode 100644 index 00000000..4b32d13a --- /dev/null +++ b/pkg/orchestration/build_plan_test.go @@ -0,0 +1,158 @@ +package orchestration + +import ( + "os" + "path/filepath" + "strings" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +func TestBuildxArgsPushIsMultiArchManifestListOnContainerBuilder(t *testing.T) { + recipe := &builderv0.DockerBuildRecipe{ + Image: "repo/app:v1", + Platforms: []string{"linux/amd64", "linux/arm64"}, + Target: "final", + BuildArgs: map[string]string{"VERSION": "1", "COMMIT": "abc"}, + } + args := buildxArgs(recipe, "/svc/builder/Dockerfile", "/svc", true, true, "/tmp/meta.json") + joined := strings.Join(args, " ") + + require.Equal(t, []string{"buildx", "build"}, args[:2]) + // Multi-platform builds must run on the container-driver builder. + require.Contains(t, joined, "--builder codefly") + require.Contains(t, joined, "--platform linux/amd64,linux/arm64") + require.Contains(t, joined, "--push") + require.NotContains(t, joined, "--load") + require.Contains(t, joined, "--metadata-file /tmp/meta.json") + require.Contains(t, joined, "--target final") + // Build args are emitted in sorted key order for a stable command. + require.Contains(t, joined, "--build-arg COMMIT=abc --build-arg VERSION=1") + require.Equal(t, []string{"-t", "repo/app:v1", "-f", "/svc/builder/Dockerfile", "/svc"}, args[len(args)-5:]) +} + +func TestBuildxArgsLocalBuildIsSinglePlatformLoadOnDefaultBuilder(t *testing.T) { + recipe := &builderv0.DockerBuildRecipe{ + Image: "repo/app:v1", + Platforms: []string{"linux/amd64", "linux/arm64"}, + } + args := buildxArgs(recipe, "/svc/builder/Dockerfile", "/svc", false, false, "") + joined := strings.Join(args, " ") + + // A local load cannot materialize a multi-platform manifest list, and it + // uses the default builder (no dedicated container builder needed). + require.NotContains(t, joined, "--builder") + require.Contains(t, joined, "--platform linux/amd64") + require.NotContains(t, joined, "linux/amd64,linux/arm64") + require.Contains(t, joined, "--load") + require.NotContains(t, joined, "--push") + require.NotContains(t, joined, "--metadata-file") +} + +func TestPlatformsIncludeDeploymentArch(t *testing.T) { + require.True(t, platformsIncludeDeploymentArch([]string{"linux/arm64", "linux/amd64"})) + require.True(t, platformsIncludeDeploymentArch([]string{"linux/amd64/v2"})) + // An arm64-only recipe would deploy an image that cannot run on amd64 nodes. + require.False(t, platformsIncludeDeploymentArch([]string{"linux/arm64"})) + // An empty list builds only the host arch — the arm64-on-Apple-silicon bug. + require.False(t, platformsIncludeDeploymentArch(nil)) +} + +func TestRecipeDockerfileResolvesAndContains(t *testing.T) { + outputDir := filepath.FromSlash("/work/services/store/builder") + + got, err := recipeDockerfile(outputDir, &builderv0.DockerBuildRecipe{Dockerfile: "Dockerfile"}) + require.NoError(t, err) + require.Equal(t, filepath.Join(outputDir, "Dockerfile"), got) + + // A recipe must not point buildx -f at a file outside the recipe tree. + _, err = recipeDockerfile(outputDir, &builderv0.DockerBuildRecipe{Dockerfile: "../../../../etc/passwd"}) + require.Error(t, err) +} + +func TestRecipeContextResolvesAndContains(t *testing.T) { + serviceDir := "/work/services/store" + + got, err := recipeContext(serviceDir, &builderv0.DockerBuildRecipe{Context: ""}) + require.NoError(t, err) + require.Equal(t, serviceDir, got) + + got, err = recipeContext(serviceDir, &builderv0.DockerBuildRecipe{Context: "code"}) + require.NoError(t, err) + require.Equal(t, filepath.Join(serviceDir, "code"), got) + + _, err = recipeContext(serviceDir, &builderv0.DockerBuildRecipe{Context: "../other"}) + require.Error(t, err) +} + +func TestApplyRecipeIgnoreStagesDiscoverableSibling(t *testing.T) { + outputDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outputDir, "dockerignore"), []byte("code/node_modules\n"), 0o644)) + dockerfile := filepath.Join(outputDir, "Dockerfile") + require.NoError(t, os.WriteFile(dockerfile, []byte("FROM alpine\n"), 0o644)) + + cleanup, err := applyRecipeIgnore(outputDir, dockerfile, &builderv0.DockerBuildRecipe{Dockerignore: "dockerignore"}) + require.NoError(t, err) + + // buildx discovers ".dockerignore"; that sibling must now exist + // with the recipe's ignore content. + staged, err := os.ReadFile(dockerfile + ".dockerignore") + require.NoError(t, err) + require.Equal(t, "code/node_modules\n", string(staged)) + + cleanup() + _, err = os.Stat(dockerfile + ".dockerignore") + require.True(t, os.IsNotExist(err), "staged ignore must be cleaned up") +} + +func TestApplyRecipeIgnoreNoIgnoreIsNoOp(t *testing.T) { + outputDir := t.TempDir() + dockerfile := filepath.Join(outputDir, "Dockerfile") + cleanup, err := applyRecipeIgnore(outputDir, dockerfile, &builderv0.DockerBuildRecipe{}) + require.NoError(t, err) + cleanup() + _, err = os.Stat(dockerfile + ".dockerignore") + require.True(t, os.IsNotExist(err)) +} + +func TestApplyRecipeIgnoreRefusesToClobber(t *testing.T) { + outputDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outputDir, "dockerignore"), []byte("x\n"), 0o644)) + dockerfile := filepath.Join(outputDir, "Dockerfile") + require.NoError(t, os.WriteFile(dockerfile+".dockerignore", []byte("existing\n"), 0o644)) + + _, err := applyRecipeIgnore(outputDir, dockerfile, &builderv0.DockerBuildRecipe{Dockerignore: "dockerignore"}) + require.Error(t, err) + // The pre-existing sibling is left intact. + data, readErr := os.ReadFile(dockerfile + ".dockerignore") + require.NoError(t, readErr) + require.Equal(t, "existing\n", string(data)) +} + +func TestReadPushedImageDigest(t *testing.T) { + metadata := filepath.Join(t.TempDir(), "meta.json") + digest := "sha256:" + strings.Repeat("a", 64) + require.NoError(t, os.WriteFile(metadata, []byte(`{"containerimage.digest":"`+digest+`","image.name":"repo/app:v1"}`), 0o644)) + got, err := readPushedImageDigest(metadata) + require.NoError(t, err) + require.Equal(t, digest, got) + + bad := filepath.Join(t.TempDir(), "meta.json") + require.NoError(t, os.WriteFile(bad, []byte(`{"image.name":"repo/app:v1"}`), 0o644)) + _, err = readPushedImageDigest(bad) + require.Error(t, err) +} + +func TestBuildRecipeOutputDirectoryIsAbsoluteAndDoesNotCreate(t *testing.T) { + serviceDir := t.TempDir() + outputDir, err := buildRecipeOutputDirectory(serviceDir) + require.NoError(t, err) + require.True(t, filepath.IsAbs(outputDir)) + require.Equal(t, filepath.Join(serviceDir, buildRecipeDir), outputDir) + // The CLI must not create builder/ — a legacy agent that ignores the field + // must not have an empty directory left behind. + _, statErr := os.Stat(outputDir) + require.True(t, os.IsNotExist(statErr)) +} diff --git a/pkg/orchestration/builder.go b/pkg/orchestration/builder.go index 77f905e6..4bd99bf0 100644 --- a/pkg/orchestration/builder.go +++ b/pkg/orchestration/builder.go @@ -223,7 +223,15 @@ func (b *Builder) Build(ctx context.Context) (*OutputProperty, error) { return nil, w.Wrapf(err, "cannot create build context") } - resp, err := b.instance.Builder.Build(ctx, &builderv0.BuildRequest{BuildContext: builder.BuildContextFromDocker(dockerContext)}) + outputDir, err := buildRecipeOutputDirectory(b.instance.Service.Dir()) + if err != nil { + return nil, w.Wrapf(err, "cannot prepare build recipe directory") + } + + resp, err := b.instance.Builder.Build(ctx, &builderv0.BuildRequest{ + BuildContext: builder.BuildContextFromDocker(dockerContext), + OutputDirectory: outputDir, + }) if err != nil { return nil, w.Wrapf(err, "cannot call build") } @@ -245,7 +253,13 @@ func (b *Builder) Build(ctx context.Context) (*OutputProperty, error) { return nil, w.Wrapf(err, "cannot process outputProperty for build") } - if buildResult := dockerBuildResult(resp.Result); buildResult != nil { + // A build plan means the agent emitted recipes and the CLI owns the docker + // build; otherwise the agent built in-process (legacy) and the CLI only pushes. + if plan := resp.Result.GetDockerBuildPlan(); plan != nil { + if err = b.buildFromPlan(ctx, outputDir, plan); err != nil { + return nil, err + } + } else if buildResult := dockerBuildResult(resp.Result); buildResult != nil { if push.Load() { w.Info("Pushing docker image", wool.Field("result", resp.Result)) for _, im := range buildResult.Images {