diff --git a/.github/workflows/build-cache.yml b/.github/workflows/build-cache.yml index 4304549e2..f14c98feb 100644 --- a/.github/workflows/build-cache.yml +++ b/.github/workflows/build-cache.yml @@ -29,11 +29,9 @@ jobs: - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version: "1.27.0" - - name: Verify in-agent Buildx selection and cache export + - name: Verify Buildx selection and cache arguments reach the docker CLI if: matrix.language == 'go' - env: - CODEFLY_TEST_REGISTRY_CACHE: "1" - run: go test ./runners/golang -run '^TestCustomContextBuildUsesRequestedBuildxAndExportsCache$' -count=1 -v -timeout=5m + run: go test ./agents/helpers/docker -run '^TestBuildx|^TestBuildOmits|^TestBuildCarries' -count=1 -v -timeout=5m - name: Reject hidden inputs in exported cache layers env: CODEFLY_TEST_REGISTRY_CACHE: "1" diff --git a/agents/helpers/docker/buildx_selection_test.go b/agents/helpers/docker/buildx_selection_test.go new file mode 100644 index 000000000..4dd51b82a --- /dev/null +++ b/agents/helpers/docker/buildx_selection_test.go @@ -0,0 +1,107 @@ +package docker + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// recordedDocker puts a fake `docker` on PATH that appends each invocation's +// arguments to a log and succeeds, so the argv this package builds can be +// asserted without a daemon, a registry, or a real buildx builder. +func recordedDocker(t *testing.T) func() []string { + t.Helper() + bin := t.TempDir() + log := filepath.Join(t.TempDir(), "argv") + script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> " + log + "\nexit 0\n" + require.NoError(t, os.WriteFile(filepath.Join(bin, "docker"), []byte(script), 0o755)) + t.Setenv("PATH", bin) + return func() []string { + contents, err := os.ReadFile(log) + require.NoError(t, err) + var invocations []string + for _, line := range strings.Split(strings.TrimSpace(string(contents)), "\n") { + if line != "" { + invocations = append(invocations, line) + } + } + return invocations + } +} + +func buildInvocation(t *testing.T, invocations []string) string { + t.Helper() + for _, invocation := range invocations { + if strings.HasPrefix(invocation, "buildx build") { + return invocation + } + } + require.FailNow(t, "no `docker buildx build` invocation was recorded", "recorded: %v", invocations) + return "" +} + +func backendRequest() backendBuildRequest { + return backendBuildRequest{ + Platform: "linux/amd64", + Dockerfile: "/tmp/svc/builder/Dockerfile", + Tag: "repo/app:test", + Context: "/tmp/svc", + Output: &bytes.Buffer{}, + } +} + +// The caller provisions its own Buildx builder and names it in the request; the +// build must run on that builder and not on whatever ambient builder the host +// has selected. Nothing else in this repository asserts that --builder reaches +// the docker CLI, so a regression here would otherwise surface only as a build +// running on the wrong builder — silently succeeding against the wrong registry +// network, or failing to export cache at all. +func TestBuildxBuilderSelectionReachesTheDockerCLI(t *testing.T) { + recorded := recordedDocker(t) + + request := backendRequest() + request.BuildxBuilder = "codefly-provisioned" + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), request)) + + invocation := buildInvocation(t, recorded()) + require.Contains(t, invocation, "--builder codefly-provisioned") + require.Contains(t, invocation, "--platform linux/amd64") + require.Contains(t, invocation, "-t repo/app:test") +} + +// Without an explicit selection the flag must be absent entirely: passing +// `--builder ""` would fail, and passing a default would silently override the +// operator's own buildx selection. +func TestBuildOmitsBuilderFlagWithoutSelection(t *testing.T) { + recorded := recordedDocker(t) + + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), backendRequest())) + + require.NotContains(t, buildInvocation(t, recorded()), "--builder") +} + +// Cache policy and builder selection are independent: selecting a builder must +// not drop the cache arguments, which is the combination the registry-cache +// conformance job exercises against a real registry. +func TestBuildCarriesCacheArgumentsAlongsideBuilderSelection(t *testing.T) { + recorded := recordedDocker(t) + + request := backendRequest() + request.BuildxBuilder = "codefly-provisioned" + request.Cache = &builderv0.BuildCacheOptions{ + Backend: "registry", Scope: "workspace/service/app", + Imports: []string{"registry.example.com/cache"}, + Exports: []string{"registry.example.com/cache"}, + } + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), request)) + + invocation := buildInvocation(t, recorded()) + require.Contains(t, invocation, "--builder codefly-provisioned") + require.Contains(t, invocation, "--cache-from") + require.Contains(t, invocation, "--cache-to") +} diff --git a/agents/helpers/docker/cache_integration_test.go b/agents/helpers/docker/cache_integration_test.go index 2c187ad9f..ff01a4d5e 100644 --- a/agents/helpers/docker/cache_integration_test.go +++ b/agents/helpers/docker/cache_integration_test.go @@ -129,7 +129,12 @@ func TestLanguageRegistryCacheAcrossCleanBuilders(t *testing.T) { for i := first; i <= last; i++ { builderName := fmt.Sprintf("%s-%d", name, i) run("buildx", "create", "--name", builderName, "--driver", "docker-container", "--driver-opt", "network="+name, "--buildkitd-config", config) - t.Setenv("BUILDX_BUILDER", builderName) + // Point the ambient selection at a builder that does not exist and pass + // the real one explicitly, so this run proves BuilderConfiguration. + // BuildxBuilder actually reaches docker buildx against a live registry. + // Falling back to the ambient builder would fail outright rather than + // quietly exporting cache from the wrong one. + t.Setenv("BUILDX_BUILDER", "must-not-use-ambient-builder") t.Cleanup(func() { out, err := exec.Command("docker", "buildx", "rm", builderName).CombinedOutput() require.NoError(t, err, "%s", out) @@ -165,7 +170,7 @@ func TestLanguageRegistryCacheAcrossCleanBuilders(t *testing.T) { out, err := exec.Command("docker", "image", "rm", tag).CombinedOutput() require.NoError(t, err, "%s", out) }) - builder, err := NewBuilder(BuilderConfiguration{Root: root, Dockerfile: "Dockerfile", Destination: resources.NewDockerImage(tag), Output: &output, Platform: "linux/amd64", Cache: policy}) + builder, err := NewBuilder(BuilderConfiguration{Root: root, Dockerfile: "Dockerfile", Destination: resources.NewDockerImage(tag), Output: &output, Platform: "linux/amd64", Cache: policy, BuildxBuilder: builderName}) require.NoError(t, err) result, err := builder.Build(ctx) require.NoError(t, err, "%s", output.String()) diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index d576f0674..77d8ba310 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -259,16 +259,13 @@ func (s *BuilderWrapper) WithBuildPlan(plan *builderv0.DockerBuildPlan) { } } -// SingleImageBuildResponse renders the conventional single-image recipe over the -// caller's output_directory, records it as the build result, and returns the build -// response. A language runner calls this — instead of building the image in-process -// — when BuildPlanRequested(req) is true. The conventional layout it emits -// (a Dockerfile in output_directory, context = the service directory, an optional -// dockerignore) is language-agnostic, so Go, Rust, Python, Node, and every -// other agent built on the shared builder move to CLI-owned, multi-arch builds -// through this one path — just by re-pinning core. -func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string) (*builderv0.BuildResponse, error) { - plan, err := SingleImageBuildPlan(req.GetOutputDirectory(), image, RecipeBuildPlatforms()) +// SingleImageBuildResponse records a single-image recipe over the caller's +// output_directory and returns the build response. The CLI executes the plan. +// emitted is the path set the caller just rendered, from PrepareRecipeDestination: +// it scopes the plan's inventory to the files this build wrote and decides whether +// the recipe references a dockerignore at all. +func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string, emitted []string) (*builderv0.BuildResponse, error) { + plan, err := SingleImageBuildPlan(req.GetOutputDirectory(), image, RecipeBuildPlatforms(), emitted) if err != nil { return s.BuildError(err) } diff --git a/agents/services/builder_cache_test.go b/agents/services/builder_cache_test.go index 1a684c93c..f5c5f3d23 100644 --- a/agents/services/builder_cache_test.go +++ b/agents/services/builder_cache_test.go @@ -51,8 +51,8 @@ func TestBuildCacheRequiresAgentAcknowledgement(t *testing.T) { } func TestRecipeAgentsDoNotAcknowledgeCallerOwnedCacheExecution(t *testing.T) { - directory := writeRecipeTree(t, true) - plan, err := SingleImageBuildPlan(directory, "repo/app:v1", RecipeBuildPlatforms()) + directory, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(directory, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) client := &cacheBuilderClient{plan: plan} agent := &BuilderAgent{BuilderClient: client} diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index 50c0fe640..ccdb45821 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -5,11 +5,15 @@ import ( "encoding/hex" "fmt" "io" + "io/fs" "os" + "path" "path/filepath" "sort" "strings" + "github.com/codefly-dev/core/templates" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" ) @@ -18,12 +22,16 @@ import ( // the aggregate-digest algorithm changes, so a digest produced by an older core // is reported as a contract mismatch (a clear, actionable error) rather than as // a digest mismatch (indistinguishable from tampering). v2 covers the recipes -// and per-file mode in the digest; v1 covered only file paths and content. -const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v2" +// and per-file mode in the digest; v1 covered only file paths and content. v3 +// makes each plan declare what its inventory covers (RecipeInventoryScope) and +// digests that declaration, so a plan rendered into a directory shared with the +// repository is not failed by unrelated content while a plan covering a whole +// assembled build context still rejects files added to it. +const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v3" // ValidateBuildRequestOutputDirectory enforces the BuildRequest.output_directory // contract: when set, the destination must be an absolute path the caller owns. -// Empty is valid and selects the legacy in-agent build. A relative path is +// Empty is valid for requests that do not build an image. A relative path is // rejected with a clear error rather than silently resolved against whatever // working directory the agent happens to run in — the agent and the caller would // otherwise resolve it against different directories and the recipe handshake @@ -44,6 +52,84 @@ func ValidateBuildRequestOutputDirectory(req *builderv0.BuildRequest) error { return nil } +// BuilderTemplateRoot is the subtree of an agent's embedded template filesystem +// that WithBuilder renders. PrepareRecipeDestination and WithBuilder must agree +// on it, or the prepared path set would not match the rendered one. +const BuilderTemplateRoot = "templates/builder" + +// PrepareRecipeDestination unlinks every path the builder template set is about +// to render into outputDirectory and returns that path set, destination-relative +// and sorted. +// +// It must run before the templates are rendered. The template writer opens each +// destination with os.OpenFile(O_WRONLY|O_CREATE|O_TRUNC) and tests existence +// with os.Stat, and both follow symlinks: rendering over a symlinked destination +// truncates the symlink's target, which can sit anywhere on disk. Because +// output_directory is the service's committed builder/ directory, a symlink +// there is ordinary repository content, not an attack precondition. Unlinking +// first (os.Remove never follows the final component) makes every render land +// on a fresh regular file inside the caller-owned directory. +// +// Inventorying the returned set rather than walking the destination is what +// keeps a plan's digest a function of the recipe: unrelated files in a committed +// directory — editor backups, .DS_Store, a symlink to shared config — are +// neither hashed nor rejected, because none of them is a file this build wrote. +func PrepareRecipeDestination(builderFS fs.FS, outputDirectory string) ([]string, error) { + var emitted []string + namer := templates.CutTemplateSuffix{} + err := fs.WalkDir(builderFS, BuilderTemplateRoot, func(entry string, info fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if info.IsDir() { + return nil + } + // fs.WalkDir always yields slash-separated paths, so the template-relative + // name is derived with path, not filepath, and converted once on the way to + // the destination below. + relative, found := strings.CutPrefix(entry, BuilderTemplateRoot+"/") + if !found { + return fmt.Errorf("template entry %q is outside %q", entry, BuilderTemplateRoot) + } + name := namer.NewName(path.Clean(relative)) + emitted = append(emitted, name) + if removeErr := os.Remove(filepath.Join(outputDirectory, filepath.FromSlash(name))); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("cannot replace recipe file %q: %w", name, removeErr) + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("prepare recipe destination: %w", err) + } + if len(emitted) == 0 { + return nil, fmt.Errorf("builder template set %q is empty; nothing to emit", BuilderTemplateRoot) + } + sort.Strings(emitted) + return emitted, nil +} + +// BuildEmittedDockerBuildPlan builds a plan whose inventory is exactly the files +// this build emitted — the set PrepareRecipeDestination returned — instead of +// whatever the destination directory currently holds. Use it from any runner +// that renders a known template set; BuildDockerBuildPlan remains for callers +// that assemble a destination tree by other means and want all of it covered. +func BuildEmittedDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRecipe, emitted []string) (*builderv0.DockerBuildPlan, error) { + files, err := inventoryRecipePaths(destination, emitted) + if err != nil { + return nil, fmt.Errorf("inventory recipe files: %w", err) + } + if err := validateRecipes(destination, recipes, files); err != nil { + return nil, err + } + return &builderv0.DockerBuildPlan{ + Recipes: recipes, + Files: files, + Digest: aggregateRecipeDigest(recipes, files, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED), + ContractVersion: DockerBuildRecipeContractVersion, + Scope: builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED, + }, nil +} + // BuildDockerBuildPlan inventories the recipe tree an agent wrote to destination // and returns a build plan: the ordered recipes plus the canonical sorted file // inventory with per-file sha256 digests and an aggregate digest that is a @@ -52,6 +138,13 @@ func ValidateBuildRequestOutputDirectory(req *builderv0.BuildRequest) error { // returned, so a plan that passes is buildable. The caller (the CLI) verifies the // on-disk tree against the plan before running docker buildx, so the recipe is a // durable, first-class artifact rather than an image built inside the agent. +// +// This inventories everything under destination, including entries the caller did +// not write, and rejects any symlink it finds. That is the right contract for a +// caller that assembles the whole destination itself and is claiming all of it — +// there, an unexpected entry is drift. A caller that renders a known template set +// into a directory it shares with the repository should use +// BuildEmittedDockerBuildPlan instead, which claims only the files it wrote. func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRecipe) (*builderv0.DockerBuildPlan, error) { files, err := inventoryRecipeFiles(destination) if err != nil { @@ -63,8 +156,9 @@ func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRe return &builderv0.DockerBuildPlan{ Recipes: recipes, Files: files, - Digest: aggregateRecipeDigest(recipes, files), + Digest: aggregateRecipeDigest(recipes, files, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE), ContractVersion: DockerBuildRecipeContractVersion, + Scope: builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE, }, nil } @@ -82,12 +176,8 @@ func RecipeBuildPlatforms() []string { return []string{"linux/amd64", "linux/arm64"} } -// BuildPlanRequested reports whether the caller (the CLI) owns the build for this -// request — a non-empty BuildRequest.output_directory means the runner should emit -// a recipe into that directory instead of building the image in-process. This is -// the single negotiation point every language runner checks, so the recipe path is -// language-agnostic: Go, Rust, Python, Node — any agent built on the shared builder -// switches to CLI-owned builds by consulting this and nothing else. +// BuildPlanRequested reports whether a request supplies a recipe destination. +// Image-building runners must reject requests without one before preparation. func BuildPlanRequested(req *builderv0.BuildRequest) bool { return req.GetOutputDirectory() != "" } @@ -102,14 +192,20 @@ func BuildPlanRequested(req *builderv0.BuildRequest) bool { // caller requested recipe emission (a non-empty BuildRequest.output_directory) // instead of building the image in-process, so the build recipe becomes a durable // artifact the caller builds. -func SingleImageBuildPlan(outputDirectory, image string, platforms []string) (*builderv0.DockerBuildPlan, error) { +// +// emitted is the path set this build rendered, from PrepareRecipeDestination. The +// recipe references a dockerignore only when that set contains one. Probing the +// destination for a dockerignore instead would adopt a stale file: the executor +// copies the referenced ignore to the path buildx discovers and applies it, so an +// ignore left behind by an older template set would silently keep files out of +// the image with no error and no diff. +func SingleImageBuildPlan(outputDirectory, image string, platforms []string, emitted []string) (*builderv0.DockerBuildPlan, error) { dockerignore := "" - // Lstat, not Stat: inventoryRecipeFiles rejects symlinks outright, so a - // symlinked dockerignore that Stat would follow-and-accept must not enter the - // recipe — it would hard-fail the inventory with an error unrelated to the - // dockerignore reference. - if info, err := os.Lstat(filepath.Join(outputDirectory, "dockerignore")); err == nil && info.Mode().IsRegular() { - dockerignore = "dockerignore" + for _, name := range emitted { + if name == "dockerignore" { + dockerignore = "dockerignore" + break + } } recipe := &builderv0.DockerBuildRecipe{ Name: "app", @@ -119,15 +215,77 @@ func SingleImageBuildPlan(outputDirectory, image string, platforms []string) (*b Image: image, Platforms: platforms, } - return BuildDockerBuildPlan(outputDirectory, []*builderv0.DockerBuildRecipe{recipe}) + return BuildEmittedDockerBuildPlan(outputDirectory, []*builderv0.DockerBuildRecipe{recipe}, emitted) } -// VerifyDockerBuildPlan re-inventories the recipe tree at destination and checks -// it against plan. The caller (the CLI) runs this before docker buildx so it -// never builds from a tree that drifted from the inventory the agent validated, -// and never builds recipes whose metadata (image, args, paths) was tampered with -// after the plan was emitted: the digest covers the recipes as well as the files, -// and every recipe is re-validated to reference real, contained tree entries. +// inventoryRecipePaths inventories exactly the destination-relative paths given, +// in canonical sorted order. Each path must be relative, stay inside destination, +// and resolve to a regular file. Symlinks are rejected by Lstat rather than +// followed: fileDigest would otherwise hash an out-of-tree target and buildx +// would read it, so containment is enforced on the real file type, not only on +// the lexical path. +func inventoryRecipePaths(destination string, paths []string) ([]*builderv0.RecipeFile, error) { + files := make([]*builderv0.RecipeFile, 0, len(paths)) + seen := make(map[string]struct{}, len(paths)) + for _, raw := range paths { + relative, err := recipeRelPath(destination, raw) + if err != nil { + return nil, fmt.Errorf("recipe file %q: %w", raw, err) + } + if _, duplicate := seen[relative]; duplicate { + return nil, fmt.Errorf("recipe file %q is listed more than once", relative) + } + seen[relative] = struct{}{} + full := filepath.Join(destination, filepath.FromSlash(relative)) + info, err := os.Lstat(full) + if err != nil { + return nil, err + } + if info.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("recipe file %q is a symlink; symlinks are not permitted in the recipe tree", relative) + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("recipe file %q is not a regular file", relative) + } + digest, err := fileDigest(full) + if err != nil { + return nil, err + } + files = append(files, &builderv0.RecipeFile{ + Path: relative, + Digest: digest, + Mode: uint32(info.Mode().Perm()), + }) + } + sort.Slice(files, func(i, j int) bool { return files[i].GetPath() < files[j].GetPath() }) + return files, nil +} + +// VerifyDockerBuildPlan re-inventories a plan's files and checks them against +// plan. The caller (the CLI) runs this before docker buildx so it never builds +// from a tree that drifted from the inventory the agent validated, and never +// builds recipes whose metadata (image, args, paths) was tampered with after the +// plan was emitted: the digest covers the recipes as well as the files, and every +// recipe is re-validated to reference real, contained tree entries. +// +// How much of destination counts as "the plan's files" is the plan's own +// declaration, not a guess here, because the two possible answers are not +// interchangeable: +// +// - TREE: re-walk destination and compare the whole inventory. The emitter +// assembled the entire destination — service agents copy the build context +// there and build "." — so a file ADDED after emission is a build input that +// would be baked into the image, and must fail verification. +// - EMITTED: re-hash exactly the declared paths. The emitter rendered a known +// template set into the service's committed builder/ directory, which it +// shares with the repository, so unrelated content there is outside the claim +// and must neither move the digest nor abort the build. +// +// Verifying a TREE plan as if it were EMITTED silently stops detecting injected +// build-context files; verifying an EMITTED plan as if it were TREE fails builds +// over an editor backup. A plan that declares no scope is rejected rather than +// verified under a default, and the scope is covered by the digest so it cannot +// be rewritten to weaken the check. func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) error { if plan == nil { return fmt.Errorf("build plan is nil") @@ -135,26 +293,49 @@ func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) if plan.GetContractVersion() != DockerBuildRecipeContractVersion { return fmt.Errorf("build plan contract %q, expected %q", plan.GetContractVersion(), DockerBuildRecipeContractVersion) } - files, err := inventoryRecipeFiles(destination) + + var files []*builderv0.RecipeFile + var err error + switch scope := plan.GetScope(); scope { + case builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE: + files, err = inventoryRecipeFiles(destination) + case builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED: + declared := make([]string, 0, len(plan.GetFiles())) + for _, file := range plan.GetFiles() { + declared = append(declared, file.GetPath()) + } + files, err = inventoryRecipePaths(destination, declared) + default: + return fmt.Errorf("build plan declares no inventory scope (%s); it cannot be verified", scope) + } if err != nil { - return fmt.Errorf("inventory recipe tree: %w", err) + return fmt.Errorf("inventory recipe files: %w", err) } + if err := validateRecipes(destination, plan.GetRecipes(), files); err != nil { return err } - if digest := aggregateRecipeDigest(plan.GetRecipes(), files); digest != plan.GetDigest() { + if digest := aggregateRecipeDigest(plan.GetRecipes(), files, plan.GetScope()); digest != plan.GetDigest() { return fmt.Errorf("recipe tree digest %s does not match plan digest %s", digest, plan.GetDigest()) } return nil } // validateRecipes checks that every recipe carries a non-empty name that is -// unique within the service, and references paths that are relative, contained -// within destination, and present in the inventoried tree: the Dockerfile and -// (optional) dockerignore must be files in the inventory, and the context must -// be an existing directory. A plan that passes this is buildable by docker -// buildx and cannot point the build context or Dockerfile outside the -// caller-owned output directory. +// unique within the service, and references paths that are relative and stay +// inside the root they are resolved against: the Dockerfile and (optional) +// dockerignore must be files in the inventory, so they cannot point buildx -f at +// anything outside the caller-owned output directory. +// +// The context is checked lexically only. dockerfile and dockerignore are +// output_directory-relative, but context is resolved by the executor against the +// SERVICE directory — that is what makes "." mean "build the service" while the +// Dockerfile lives in the service's builder/ subdirectory. destination is +// therefore the wrong root to resolve the context against, and statting it here +// proved nothing: it accepted "." because output_directory trivially exists, and +// it rejected a perfectly valid context such as "code" whenever the service had +// no builder/code directory. Existence and containment of the context belong to +// the executor, which is the only party that knows the service directory. func validateRecipes(destination string, recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile) error { inventory := make(map[string]struct{}, len(files)) for _, file := range files { @@ -177,17 +358,9 @@ func validateRecipes(destination string, recipes []*builderv0.DockerBuildRecipe, if _, ok := inventory[dockerfile]; !ok { return fmt.Errorf("recipe %q dockerfile %q is not present in the recipe tree", recipe.GetName(), recipe.GetDockerfile()) } - context, err := recipeRelPath(destination, recipe.GetContext()) - if err != nil { + if _, err := recipeRelPath(destination, recipe.GetContext()); err != nil { return fmt.Errorf("recipe %q context: %w", recipe.GetName(), err) } - info, statErr := os.Stat(filepath.Join(destination, filepath.FromSlash(context))) - if statErr != nil { - return fmt.Errorf("recipe %q context %q: %w", recipe.GetName(), recipe.GetContext(), statErr) - } - if !info.IsDir() { - return fmt.Errorf("recipe %q context %q is not a directory", recipe.GetName(), recipe.GetContext()) - } if ignore := recipe.GetDockerignore(); ignore != "" { dockerignore, err := recipeRelPath(destination, ignore) if err != nil { @@ -294,8 +467,13 @@ func fileDigest(path string) (string, error) { // permission bits are covered too, so flipping a file's executable bit — which // buildx carries into the image — is detected even when its content is // unchanged. -func aggregateRecipeDigest(recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile) string { +func aggregateRecipeDigest(recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile, scope builderv0.RecipeInventoryScope) string { hasher := sha256.New() + // The scope decides how strictly the inventory is verified, so it is covered + // here: rewriting a TREE plan's scope to EMITTED would otherwise silently stop + // added files from being detected without disturbing the digest. + hashField(hasher, "scope") + hashField(hasher, scope.String()) hashField(hasher, "recipes") hashCount(hasher, len(recipes)) for _, recipe := range recipes { diff --git a/agents/services/docker_recipe_inventory_scope_test.go b/agents/services/docker_recipe_inventory_scope_test.go new file mode 100644 index 000000000..dd397592f --- /dev/null +++ b/agents/services/docker_recipe_inventory_scope_test.go @@ -0,0 +1,95 @@ +package services + +import ( + "os" + "path/filepath" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// wholeTreePlan mirrors what a service agent emits when it copies its build +// context into the destination and builds "." — service-go's buildRecipe does +// exactly this, so every file under destination is a real build input. +func wholeTreePlan(t *testing.T) (string, *builderv0.DockerBuildPlan) { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "code"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "code", "main.go"), []byte("package main\n"), 0o644)) + plan, err := BuildDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{{ + Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1", + Platforms: RecipeBuildPlatforms(), + }}) + require.NoError(t, err) + require.Equal(t, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE, plan.GetScope()) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) + return dir, plan +} + +// A whole-tree emitter claims the entire destination, so a file added after +// emission is an injected build input that buildx would copy into the image. +// Verifying such a plan by re-hashing only its declared paths would not see it. +func TestVerifyRejectsFilesAddedToAWholeTreeBuildContext(t *testing.T) { + dir, plan := wholeTreePlan(t) + + require.NoError(t, os.WriteFile(filepath.Join(dir, "code", "injected.go"), []byte("package main\n"), 0o644)) + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "digest") +} + +func TestVerifyRejectsSymlinksAddedToAWholeTreeBuildContext(t *testing.T) { + dir, plan := wholeTreePlan(t) + + outside := filepath.Join(t.TempDir(), "secret") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "code", "secret"))) + require.Error(t, VerifyDockerBuildPlan(dir, plan)) +} + +// The emitted-scope claim is the opposite case and must stay tolerant: the +// destination is the service's committed builder/ directory, shared with the +// repository, so content the build did not write is outside the claim. +func TestVerifyToleratesUnclaimedContentInAnEmittedScopePlan(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.Equal(t, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED, plan.GetScope()) + + require.NoError(t, os.WriteFile(filepath.Join(dir, ".DS_Store"), []byte("mac"), 0o644)) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) +} + +// Downgrading the scope would silently turn the strict check into the tolerant +// one, so the scope is covered by the aggregate digest. +func TestScopeCannotBeDowngradedWithoutBreakingTheDigest(t *testing.T) { + dir, plan := wholeTreePlan(t) + + plan.Scope = builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "digest") +} + +// Identical recipes and files under different scopes are different claims, so +// they must not share a digest. +func TestScopeParticipatesInTheAggregateDigest(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + recipes := []*builderv0.DockerBuildRecipe{{Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1"}} + + tree, err := BuildDockerBuildPlan(dir, recipes) + require.NoError(t, err) + emitted, err := BuildEmittedDockerBuildPlan(dir, recipes, []string{"Dockerfile"}) + require.NoError(t, err) + + require.Equal(t, tree.GetFiles()[0].GetDigest(), emitted.GetFiles()[0].GetDigest()) + require.NotEqual(t, tree.GetDigest(), emitted.GetDigest()) +} + +// A plan carrying no scope predates the declaration and cannot be verified under +// either reading; defaulting would pick one silently. +func TestVerifyRejectsAPlanWithNoDeclaredScope(t *testing.T) { + dir, plan := wholeTreePlan(t) + + plan.Scope = builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "no inventory scope") +} diff --git a/agents/services/docker_recipe_prepare_test.go b/agents/services/docker_recipe_prepare_test.go new file mode 100644 index 000000000..6b00bf7ef --- /dev/null +++ b/agents/services/docker_recipe_prepare_test.go @@ -0,0 +1,80 @@ +package services + +import ( + "embed" + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// A two-file template set: the guard has to cover every path the renderer writes, +// not just the Dockerfile. +// +//go:embed testdata/recipetemplates +var recipeTemplates embed.FS + +func builderTemplateSet(t *testing.T) fs.FS { + t.Helper() + sub, err := fs.Sub(recipeTemplates, "testdata/recipetemplates") + require.NoError(t, err) + return sub +} + +func TestPrepareRecipeDestinationReportsEveryEmittedPath(t *testing.T) { + emitted, err := PrepareRecipeDestination(builderTemplateSet(t), t.TempDir()) + require.NoError(t, err) + require.Equal(t, []string{"Dockerfile", "dockerignore"}, emitted) +} + +// The template renderer opens each destination with O_CREATE|O_TRUNC and tests +// existence with os.Stat, both of which follow symlinks — so a symlinked +// destination is written *through*, truncating a file anywhere on disk. Because +// output_directory is the service's committed builder/ directory, such a symlink +// is ordinary repository content. Unlinking every emitted path first is what +// keeps the write inside the caller-owned directory; guarding only "Dockerfile" +// left every other file in the template set escaping. +func TestPrepareRecipeDestinationUnlinksSymlinksWithoutFollowingThem(t *testing.T) { + root := t.TempDir() + output := filepath.Join(root, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + outside := map[string]string{"Dockerfile": "keep-dockerfile-target", "dockerignore": "keep-ignore-target"} + for name, content := range outside { + target := filepath.Join(root, name+".target") + require.NoError(t, os.WriteFile(target, []byte(content), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(output, name))) + } + + emitted, err := PrepareRecipeDestination(builderTemplateSet(t), output) + require.NoError(t, err) + require.Equal(t, []string{"Dockerfile", "dockerignore"}, emitted) + + for name, content := range outside { + require.NoFileExists(t, filepath.Join(output, name), "the symlink itself must be gone") + got, readErr := os.ReadFile(filepath.Join(root, name+".target")) + require.NoError(t, readErr) + require.Equal(t, content, string(got), "%s target was written through", name) + } +} + +// Content the template set does not own is left alone: the guard unlinks what it +// is about to write, never the directory's other contents. +func TestPrepareRecipeDestinationLeavesUnrelatedContentAlone(t *testing.T) { + output := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(output, "notes.md"), []byte("hand written"), 0o644)) + + _, err := PrepareRecipeDestination(builderTemplateSet(t), output) + require.NoError(t, err) + + got, err := os.ReadFile(filepath.Join(output, "notes.md")) + require.NoError(t, err) + require.Equal(t, "hand written", string(got)) +} + +func TestPrepareRecipeDestinationRejectsAnEmptyTemplateSet(t *testing.T) { + _, err := PrepareRecipeDestination(os.DirFS(t.TempDir()), t.TempDir()) + require.Error(t, err) +} diff --git a/agents/services/docker_recipe_scope_test.go b/agents/services/docker_recipe_scope_test.go new file mode 100644 index 000000000..8c364a458 --- /dev/null +++ b/agents/services/docker_recipe_scope_test.go @@ -0,0 +1,123 @@ +package services + +import ( + "os" + "path/filepath" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// output_directory is the service's committed builder/ directory, so unrelated +// content lands there routinely: an OS sidecar file, an editor backup, a symlink +// to shared configuration. None of it is a build input — the build context is the +// service directory, not this one — so none of it may perturb the plan or fail +// the build. Scoping the inventory to the emitted set is what guarantees that. +func TestPlanIgnoresContentThisBuildDidNotEmit(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + clean, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(filepath.Join(dir, ".DS_Store"), []byte("mac"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile~"), []byte("backup"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(dir), "shared.conf"), []byte("x"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join(filepath.Dir(dir), "shared.conf"), filepath.Join(dir, "shared.conf"))) + + dirty, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err, "an unrelated symlink in the committed recipe directory must not fail the build") + require.Equal(t, clean.GetDigest(), dirty.GetDigest(), "unrelated files must not perturb the recipe digest") + + // Verification must tolerate the same content for the same reason. + require.NoError(t, VerifyDockerBuildPlan(dir, dirty)) +} + +// The drift the plan exists to detect must still be caught, on every axis the +// digest covers: content, permission bits, and a declared file replaced by a +// symlink or removed outright. +func TestVerifyStillCatchesDriftInDeclaredFiles(t *testing.T) { + for name, mutate := range map[string]func(t *testing.T, dir string){ + "content": func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM evil\n"), 0o644)) + }, + "mode": func(t *testing.T, dir string) { + require.NoError(t, os.Chmod(filepath.Join(dir, "Dockerfile"), 0o755)) + }, + "symlinked": func(t *testing.T, dir string) { + outside := filepath.Join(filepath.Dir(dir), "outside") + require.NoError(t, os.WriteFile(outside, []byte("FROM alpine\nCOPY . .\n"), 0o644)) + require.NoError(t, os.Remove(filepath.Join(dir, "Dockerfile"))) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "Dockerfile"))) + }, + "removed": func(t *testing.T, dir string) { + require.NoError(t, os.Remove(filepath.Join(dir, "dockerignore"))) + }, + } { + t.Run(name, func(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) + mutate(t, dir) + require.Error(t, VerifyDockerBuildPlan(dir, plan)) + }) + } +} + +// A dockerignore this build did not emit must not be adopted into the recipe: the +// executor copies the referenced ignore to the path buildx discovers and applies +// it, so a stale file left by an older template set would silently keep files out +// of the image with no error and no diff. +func TestSingleImageBuildPlanIgnoresStaleDockerignore(t *testing.T) { + dir, _ := writeRecipeTree(t, true) + + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), []string{"Dockerfile"}) + require.NoError(t, err) + require.Empty(t, plan.GetRecipes()[0].GetDockerignore()) + require.Len(t, plan.GetFiles(), 1) + require.Equal(t, "Dockerfile", plan.GetFiles()[0].GetPath()) +} + +// The executor resolves context against the SERVICE directory, not against +// output_directory. Core must not reject a context that is valid there — the old +// os.Stat resolved "code" to /code and refused every service +// whose context was a subdirectory. +func TestRecipeContextIsNotResolvedAgainstTheOutputDirectory(t *testing.T) { + dir, emitted := writeRecipeTree(t, false) + recipe := &builderv0.DockerBuildRecipe{ + Name: "app", Dockerfile: "Dockerfile", Context: "code", + Image: "repo/app:v1", Platforms: RecipeBuildPlatforms(), + } + plan, err := BuildEmittedDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{recipe}, emitted) + require.NoError(t, err) + require.Equal(t, "code", plan.GetRecipes()[0].GetContext()) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) +} + +// Lexical containment still holds: a context or Dockerfile that escapes is +// rejected, so a plan can never point buildx outside the roots it is given. +func TestRecipePathsStillRejectEscapes(t *testing.T) { + dir, emitted := writeRecipeTree(t, false) + for name, recipe := range map[string]*builderv0.DockerBuildRecipe{ + "context escapes": {Name: "app", Dockerfile: "Dockerfile", Context: "../..", Image: "i"}, + "context absolute": {Name: "app", Dockerfile: "Dockerfile", Context: "/etc", Image: "i"}, + "dockerfile escapes": {Name: "app", Dockerfile: "../Dockerfile", Context: ".", Image: "i"}, + "empty name": {Dockerfile: "Dockerfile", Context: ".", Image: "i"}, + } { + t.Run(name, func(t *testing.T) { + _, err := BuildEmittedDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{recipe}, emitted) + require.Error(t, err) + }) + } +} + +// A plan emitted by an older core carries an older contract version and must be +// reported as a contract mismatch, not silently verified or read as tampering. +func TestVerifyRejectsSupersededContractVersion(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.Equal(t, "codefly.dev/docker-build-recipe/v3", plan.GetContractVersion()) + plan.ContractVersion = "codefly.dev/docker-build-recipe/v2" + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "contract") +} diff --git a/agents/services/docker_recipe_singleimage_test.go b/agents/services/docker_recipe_singleimage_test.go index 73a195bff..7a1c5b959 100644 --- a/agents/services/docker_recipe_singleimage_test.go +++ b/agents/services/docker_recipe_singleimage_test.go @@ -9,23 +9,27 @@ import ( "github.com/stretchr/testify/require" ) -func writeRecipeTree(t *testing.T, withIgnore bool) string { +// writeRecipeTree returns the output_directory and the emitted path set a runner +// would have gotten back from PrepareRecipeDestination for this template set. +func writeRecipeTree(t *testing.T, withIgnore bool) (string, []string) { t.Helper() // dir is the output_directory the caller (the CLI) passes: the service's // committed builder/ recipe directory, with the Dockerfile — and the optional // dockerignore — directly inside it, exactly as the runner renders them there. dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) + emitted := []string{"Dockerfile"} if withIgnore { require.NoError(t, os.WriteFile(filepath.Join(dir, "dockerignore"), []byte("code/node_modules\n"), 0o644)) + emitted = append(emitted, "dockerignore") } - return dir + return dir, emitted } func TestSingleImageBuildPlanConventionalLayout(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) - plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms()) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) require.Len(t, plan.GetRecipes(), 1) @@ -42,9 +46,9 @@ func TestSingleImageBuildPlanConventionalLayout(t *testing.T) { } func TestSingleImageBuildPlanOmitsAbsentDockerignore(t *testing.T) { - dir := writeRecipeTree(t, false) + dir, emitted := writeRecipeTree(t, false) - plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms()) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) require.Equal(t, "", plan.GetRecipes()[0].GetDockerignore()) require.NoError(t, VerifyDockerBuildPlan(dir, plan)) @@ -64,12 +68,12 @@ func TestBuildPlanRequested(t *testing.T) { // language runner returns SingleImageBuildResponse and gets the same DockerBuildPlan // result, verifiable against the caller's tree. func TestSingleImageBuildResponseEmitsPlan(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) base := &Base{loaded: true} wrapper := &BuilderWrapper{Base: base} base.Builder = wrapper // WithBuildPlan records onto s.Builder, as production wires it. - resp, err := wrapper.SingleImageBuildResponse(&builderv0.BuildRequest{OutputDirectory: dir}, "repo/app:v1") + resp, err := wrapper.SingleImageBuildResponse(&builderv0.BuildRequest{OutputDirectory: dir}, "repo/app:v1", emitted) require.NoError(t, err) require.Equal(t, builderv0.BuildStatus_SUCCESS, resp.GetState().GetState()) @@ -81,16 +85,16 @@ func TestSingleImageBuildResponseEmitsPlan(t *testing.T) { } func TestSingleImageBuildResponseLeavesCacheExecutionWithCaller(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) base := &Base{loaded: true} wrapper := &BuilderWrapper{Base: base} base.Builder = wrapper req := &builderv0.BuildRequest{OutputDirectory: dir} - cold, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1") + cold, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1", emitted) require.NoError(t, err) cache := &builderv0.BuildCacheOptions{Backend: "registry", Scope: "workspace/service/app", Imports: []string{"ghcr.io/org/cache"}} req.BuildContext = &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: &builderv0.DockerBuildContext{Cache: cache}}} - warm, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1") + warm, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1", emitted) require.NoError(t, err) require.Empty(t, warm.CacheContractVersion) require.Equal(t, cold.GetResult().GetDockerBuildPlan().Digest, warm.GetResult().GetDockerBuildPlan().Digest) diff --git a/agents/services/docker_recipe_test.go b/agents/services/docker_recipe_test.go index 2fd041974..d9b1a5792 100644 --- a/agents/services/docker_recipe_test.go +++ b/agents/services/docker_recipe_test.go @@ -64,10 +64,10 @@ func TestBuildDockerBuildPlanInventoriesRecipeTree(t *testing.T) { } func TestValidateBuildRequestOutputDirectory(t *testing.T) { - // A nil request carries no directory and selects the legacy in-agent build. + // A nil request carries no directory and requires no recipe destination. require.NoError(t, ValidateBuildRequestOutputDirectory(nil)) - // Empty selects the legacy in-agent build and is valid. + // Empty requires no recipe destination and is valid. require.NoError(t, ValidateBuildRequestOutputDirectory(&builderv0.BuildRequest{})) // An absolute destination honors the contract. @@ -104,7 +104,7 @@ func TestDockerBuildRequestEnforcesAbsoluteOutputDirectory(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) - // An empty output_directory (legacy in-agent build) passes through. + // No-build requests may omit output_directory. got, err = wrapper.DockerBuildRequest(context.Background(), &builderv0.BuildRequest{BuildContext: dockerContext}) require.NoError(t, err) require.NotNil(t, got) diff --git a/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl b/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl new file mode 100644 index 000000000..12c97b01d --- /dev/null +++ b/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl @@ -0,0 +1,2 @@ +FROM scratch +COPY app /app diff --git a/agents/services/testdata/recipetemplates/templates/builder/dockerignore b/agents/services/testdata/recipetemplates/templates/builder/dockerignore new file mode 100644 index 000000000..3c3629e64 --- /dev/null +++ b/agents/services/testdata/recipetemplates/templates/builder/dockerignore @@ -0,0 +1 @@ +node_modules diff --git a/docs/build-cache.md b/docs/build-cache.md index 5dba3b2f2..9e9ba9d00 100644 --- a/docs/build-cache.md +++ b/docs/build-cache.md @@ -2,8 +2,7 @@ Core supports the `registry-v1` cache contract through `DockerBuildContext.cache`. This is optional layer reuse: every build still -executes BuildKit, verifies the requested architecture on the in-agent path, -and produces its own image. Cache references never substitute for artifact +executes BuildKit in the CLI and produces its own image. Cache references never substitute for artifact manifest digests, recipe verification, tests, or release authorization. A caller can supply this protobuf JSON fragment in its Docker build context: @@ -49,13 +48,9 @@ build instead of claiming publication succeeded. ## Integration through Codefly -The Go and Rust shared in-agent builders forward cache policy to Buildx. -`SingleImageBuildResponse` returns only recipes. Recipe agents need not claim -cache execution: the CLI validates their plan and applies its own policy. -`BuilderAgent.Build` requires `registry-v1` acknowledgement for successful -in-agent image results when cache was requested, rejecting legacy agents that -silently ignore it. Generated gRPC clients used directly must enforce the same -result-kind distinction. +The Go and Rust shared runners return only recipes through +`SingleImageBuildResponse`. The CLI validates their plan and applies its own +cache policy when executing Buildx. CLI recipe executors should verify the recipe tree as usual, then append `docker.CacheArguments(callerCache, recipe.Platforms)` to their existing Buildx @@ -135,27 +130,49 @@ edits, rebuilt it for lockfile/base changes, and produced the expected changed outputs. These are conformance measurements, not a speedup claim; startup and host load affect wall time. -### In-agent executor selection - -`DockerBuildContext.buildx_builder` carries the caller's Buildx builder name -through Go and Rust in-agent builds, including Go services with a custom context -root. It is passed as `--builder` on that build only; the selected global builder -and process environment are unchanged. The caller provisions its builder before -issuing Build. Registry exports require a container-driver builder or a Docker -driver configured with the containerd image store. - -Before dispatching a Build with an explicit selection, `BuilderAgent.Build` -calls the read-only `BuildCapabilities` RPC and requires `buildx_selection=true`. -Older agents, unsupported agents, and failed probes are rejected before Build -can transfer source, change image tags, or export cache. This also applies when -`output_directory` requests a recipe: an older agent can fall back to executing -the build. Requests without explicit selection do not require the probe. - -Agents must explicitly implement the capability RPC after verifying that every -in-agent path honors selection (for example, through the updated `BuildGoDocker` -or `BuildRustDocker` helper). Recipe-only agents may opt in as well. The default -server leaves the RPC unimplemented so a custom Build cannot accidentally claim -support just by updating Core: +### CLI executor selection + +The CLI alone builds and publishes application images. Shared Go and Rust +runners require an absolute `output_directory` and return a DockerBuildPlan. +Missing destinations fail before template preparation. Go custom context roots +and workspace-root contexts are both rejected before preparation because the +single-image recipe cannot represent them — `Workspace` carries the same +requirement as `ContextRoot`, so rejecting only one would defer the failure to +an opaque `COPY` error inside the executor. Managed images and runtime-only +agents remain no-build. + +`output_directory` is the service's committed `builder/` directory, so it holds +whatever the repository and the developer's OS put there. A plan from the shared +runners therefore inventories only the files the emitting build wrote — the set +`PrepareRecipeDestination` returns — and declares +`RECIPE_INVENTORY_SCOPE_EMITTED`. `VerifyDockerBuildPlan` re-hashes exactly those +paths: unrelated content cannot perturb the digest or fail the build, while drift +in a declared file (content, mode, replacement by a symlink, removal) is still +rejected. + +An agent that assembles the whole destination itself — copying its build context +there and building `"."`, as the Go agent does — calls `BuildDockerBuildPlan` and +declares `RECIPE_INVENTORY_SCOPE_TREE` instead. There the inventory covers every +file, and verification re-walks the destination, because a file *added* after +emission is an injected build input that buildx would copy into the image. The +scope is part of the aggregate digest, so a plan cannot be rewritten to the +weaker check, and a plan declaring no scope is rejected rather than verified +under a default. Because that directory is committed, every emitted +path is unlinked before rendering: the template writer opens destinations with +`O_CREATE|O_TRUNC` and would otherwise write *through* a symlink to a file +outside the caller-owned directory. + +Recipe path bases differ and the distinction is load-bearing: `dockerfile` and +`dockerignore` are relative to `output_directory`, while `context` is relative +to the **service** directory, which is why `"."` means "build the service". Core +checks the context lexically only; the executor resolves it and enforces +containment, because only the executor knows the service directory. + +`DockerBuildContext.buildx_builder` identifies the builder the CLI selected, +including selection for registry cache exports. The agent does not invoke it. +Before dispatching an explicit selection, `BuilderAgent.Build` requires the +read-only `BuildCapabilities` RPC to report `buildx_selection=true`. Recipe-only +agents should implement this capability after verifying their Build path: ```go func (*Builder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { @@ -163,7 +180,6 @@ func (*Builder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesR } ``` -Successful in-agent responses additionally acknowledge the selection in -`BuildResponse.buildx_builder`. `BuilderAgent.Build` rejects an absent or different -acknowledgement when a builder was requested. Recipe responses need no executor -acknowledgement because the caller executes the plan itself. +Recipe responses need no executor acknowledgement because the CLI executes the +plan itself. The Docker execution and registry-cache helpers remain available +for the CLI executor; service agents must not call them to build images. diff --git a/generated/go/codefly/services/builder/v0/builder.pb.go b/generated/go/codefly/services/builder/v0/builder.pb.go index 506bbe8e9..51137376a 100644 --- a/generated/go/codefly/services/builder/v0/builder.pb.go +++ b/generated/go/codefly/services/builder/v0/builder.pb.go @@ -1884,8 +1884,8 @@ type BuildRequest struct { // build_context is the container build context supplied to the builder. BuildContext *BuildContext `protobuf:"bytes,1,opt,name=build_context,json=buildContext,proto3" json:"build_context,omitempty"` // output_directory is an absolute destination owned by the caller where the - // builder writes reproducible build recipes. Empty means the builder does not - // emit recipes and performs the legacy in-agent build. + // builder writes reproducible build recipes. Required for application image + // builds; managed-image and runtime-only agents may omit it. OutputDirectory string `protobuf:"bytes,2,opt,name=output_directory,json=outputDirectory,proto3" json:"output_directory,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/generated/go/codefly/services/builder/v0/docker.pb.go b/generated/go/codefly/services/builder/v0/docker.pb.go index 07c429585..1d1a3c524 100644 --- a/generated/go/codefly/services/builder/v0/docker.pb.go +++ b/generated/go/codefly/services/builder/v0/docker.pb.go @@ -22,6 +22,70 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +// RecipeInventoryScope states what a plan's file inventory claims to cover, so +// the verifier knows what "complete" means for that plan. Without it the +// verifier has to guess, and the two answers are not interchangeable: guessing +// EMITTED for a TREE plan stops detecting files ADDED to a build context after +// emission, and guessing TREE for an EMITTED plan fails a build over unrelated +// content in the service's committed builder/ directory. +type RecipeInventoryScope int32 + +const ( + // RECIPE_INVENTORY_SCOPE_UNSPECIFIED does not authorize verification; a plan + // that omits the scope is rejected rather than verified under a default. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED RecipeInventoryScope = 0 + // RECIPE_INVENTORY_SCOPE_EMITTED claims only the files the emitting build + // wrote. Other entries under output_directory are outside the claim and are + // neither digested nor rejected. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED RecipeInventoryScope = 1 + // RECIPE_INVENTORY_SCOPE_TREE claims every file under output_directory. The + // emitter assembled the whole destination — typically because it copied the + // build context there and a recipe builds "." — so an entry the inventory does + // not list is drift in the emitter's own output and fails verification. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE RecipeInventoryScope = 2 +) + +// Enum value maps for RecipeInventoryScope. +var ( + RecipeInventoryScope_name = map[int32]string{ + 0: "RECIPE_INVENTORY_SCOPE_UNSPECIFIED", + 1: "RECIPE_INVENTORY_SCOPE_EMITTED", + 2: "RECIPE_INVENTORY_SCOPE_TREE", + } + RecipeInventoryScope_value = map[string]int32{ + "RECIPE_INVENTORY_SCOPE_UNSPECIFIED": 0, + "RECIPE_INVENTORY_SCOPE_EMITTED": 1, + "RECIPE_INVENTORY_SCOPE_TREE": 2, + } +) + +func (x RecipeInventoryScope) Enum() *RecipeInventoryScope { + p := new(RecipeInventoryScope) + *p = x + return p +} + +func (x RecipeInventoryScope) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (RecipeInventoryScope) Descriptor() protoreflect.EnumDescriptor { + return file_codefly_services_builder_v0_docker_proto_enumTypes[0].Descriptor() +} + +func (RecipeInventoryScope) Type() protoreflect.EnumType { + return &file_codefly_services_builder_v0_docker_proto_enumTypes[0] +} + +func (x RecipeInventoryScope) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use RecipeInventoryScope.Descriptor instead. +func (RecipeInventoryScope) EnumDescriptor() ([]byte, []int) { + return file_codefly_services_builder_v0_docker_proto_rawDescGZIP(), []int{0} +} + // DockerBuildContext contains Docker-specific image build inputs. type DockerBuildContext struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -218,7 +282,12 @@ type DockerBuildRecipe struct { Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // dockerfile is the output_directory-relative path to the Dockerfile. Dockerfile string `protobuf:"bytes,2,opt,name=dockerfile,proto3" json:"dockerfile,omitempty"` - // context is the build-context directory the Dockerfile is evaluated against. + // context is the build-context directory the Dockerfile is evaluated against, + // relative to the SERVICE directory — not to output_directory, which the + // dockerfile and dockerignore paths are relative to. "." therefore means "build + // the service" while the Dockerfile itself lives in the service's builder/ + // subdirectory. The executor resolves it and is the party that enforces it + // stays inside the service. Context string `protobuf:"bytes,3,opt,name=context,proto3" json:"context,omitempty"` // dockerignore is the optional output_directory-relative ignore file. Dockerignore string `protobuf:"bytes,4,opt,name=dockerignore,proto3" json:"dockerignore,omitempty"` @@ -329,15 +398,22 @@ type DockerBuildPlan struct { state protoimpl.MessageState `protogen:"open.v1"` // recipes is the ordered set of image build recipes for the service. Recipes []*DockerBuildRecipe `protobuf:"bytes,1,rep,name=recipes,proto3" json:"recipes,omitempty"` - // files is the canonical, sorted inventory of recipe files with digests. + // files is the canonical, sorted inventory of the files the emitting build + // wrote, with digests. It is not an inventory of everything in + // output_directory: that directory is the service's committed builder/, so + // unrelated content there must neither perturb the digest nor fail the build. Files []*RecipeFile `protobuf:"bytes,2,rep,name=files,proto3" json:"files,omitempty"` // digest is the aggregate content digest over files, formatted as // "sha256:". Identical recipe trees yield an identical digest. Digest string `protobuf:"bytes,3,opt,name=digest,proto3" json:"digest,omitempty"` // contract_version identifies the recipe contract the caller validates. ContractVersion string `protobuf:"bytes,4,opt,name=contract_version,json=contractVersion,proto3" json:"contract_version,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // scope states what the files inventory covers. It is part of the aggregate + // digest, so a plan cannot be downgraded to a weaker verification by + // rewriting this field. + Scope RecipeInventoryScope `protobuf:"varint,5,opt,name=scope,proto3,enum=codefly.services.builder.v0.RecipeInventoryScope" json:"scope,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *DockerBuildPlan) Reset() { @@ -398,6 +474,13 @@ func (x *DockerBuildPlan) GetContractVersion() string { return "" } +func (x *DockerBuildPlan) GetScope() RecipeInventoryScope { + if x != nil { + return x.Scope + } + return RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED +} + // BuildCacheOptions is caller-owned transport policy. Registry credentials are // resolved by Docker, never serialized into a request or recipe. type BuildCacheOptions struct { @@ -513,18 +596,23 @@ const file_codefly_services_builder_v0_docker_proto_rawDesc = "" + "\x06target\x18\b \x01(\tR\x06target\x1a<\n" + "\x0eBuildArgsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xdd\x01\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xa6\x02\n" + "\x0fDockerBuildPlan\x12H\n" + "\arecipes\x18\x01 \x03(\v2..codefly.services.builder.v0.DockerBuildRecipeR\arecipes\x12=\n" + "\x05files\x18\x02 \x03(\v2'.codefly.services.builder.v0.RecipeFileR\x05files\x12\x16\n" + "\x06digest\x18\x03 \x01(\tR\x06digest\x12)\n" + - "\x10contract_version\x18\x04 \x01(\tR\x0fcontractVersion\"\x8b\x01\n" + + "\x10contract_version\x18\x04 \x01(\tR\x0fcontractVersion\x12G\n" + + "\x05scope\x18\x05 \x01(\x0e21.codefly.services.builder.v0.RecipeInventoryScopeR\x05scope\"\x8b\x01\n" + "\x11BuildCacheOptions\x12\x18\n" + "\aimports\x18\x01 \x03(\tR\aimports\x12\x18\n" + "\aexports\x18\x02 \x03(\tR\aexports\x12\x14\n" + "\x05scope\x18\x03 \x01(\tR\x05scope\x12\x12\n" + "\x04mode\x18\x04 \x01(\tR\x04mode\x12\x18\n" + - "\abackend\x18\x05 \x01(\tR\abackendB\x84\x02\n" + + "\abackend\x18\x05 \x01(\tR\abackend*\x83\x01\n" + + "\x14RecipeInventoryScope\x12&\n" + + "\"RECIPE_INVENTORY_SCOPE_UNSPECIFIED\x10\x00\x12\"\n" + + "\x1eRECIPE_INVENTORY_SCOPE_EMITTED\x10\x01\x12\x1f\n" + + "\x1bRECIPE_INVENTORY_SCOPE_TREE\x10\x02B\x84\x02\n" + "\x1fcom.codefly.services.builder.v0B\vDockerProtoP\x01ZDgithub.com/codefly-dev/core/generated/go/codefly/services/builder/v0\xa2\x02\x04CSBV\xaa\x02\x1bCodefly.Services.Builder.V0\xca\x02\x1bCodefly\\Services\\Builder\\V0\xe2\x02'Codefly\\Services\\Builder\\V0\\GPBMetadata\xea\x02\x1eCodefly::Services::Builder::V0b\x06proto3" var ( @@ -539,26 +627,29 @@ func file_codefly_services_builder_v0_docker_proto_rawDescGZIP() []byte { return file_codefly_services_builder_v0_docker_proto_rawDescData } +var file_codefly_services_builder_v0_docker_proto_enumTypes = make([]protoimpl.EnumInfo, 1) var file_codefly_services_builder_v0_docker_proto_msgTypes = make([]protoimpl.MessageInfo, 7) var file_codefly_services_builder_v0_docker_proto_goTypes = []any{ - (*DockerBuildContext)(nil), // 0: codefly.services.builder.v0.DockerBuildContext - (*DockerBuildResult)(nil), // 1: codefly.services.builder.v0.DockerBuildResult - (*RecipeFile)(nil), // 2: codefly.services.builder.v0.RecipeFile - (*DockerBuildRecipe)(nil), // 3: codefly.services.builder.v0.DockerBuildRecipe - (*DockerBuildPlan)(nil), // 4: codefly.services.builder.v0.DockerBuildPlan - (*BuildCacheOptions)(nil), // 5: codefly.services.builder.v0.BuildCacheOptions - nil, // 6: codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry + (RecipeInventoryScope)(0), // 0: codefly.services.builder.v0.RecipeInventoryScope + (*DockerBuildContext)(nil), // 1: codefly.services.builder.v0.DockerBuildContext + (*DockerBuildResult)(nil), // 2: codefly.services.builder.v0.DockerBuildResult + (*RecipeFile)(nil), // 3: codefly.services.builder.v0.RecipeFile + (*DockerBuildRecipe)(nil), // 4: codefly.services.builder.v0.DockerBuildRecipe + (*DockerBuildPlan)(nil), // 5: codefly.services.builder.v0.DockerBuildPlan + (*BuildCacheOptions)(nil), // 6: codefly.services.builder.v0.BuildCacheOptions + nil, // 7: codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry } var file_codefly_services_builder_v0_docker_proto_depIdxs = []int32{ - 5, // 0: codefly.services.builder.v0.DockerBuildContext.cache:type_name -> codefly.services.builder.v0.BuildCacheOptions - 6, // 1: codefly.services.builder.v0.DockerBuildRecipe.build_args:type_name -> codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry - 3, // 2: codefly.services.builder.v0.DockerBuildPlan.recipes:type_name -> codefly.services.builder.v0.DockerBuildRecipe - 2, // 3: codefly.services.builder.v0.DockerBuildPlan.files:type_name -> codefly.services.builder.v0.RecipeFile - 4, // [4:4] is the sub-list for method output_type - 4, // [4:4] is the sub-list for method input_type - 4, // [4:4] is the sub-list for extension type_name - 4, // [4:4] is the sub-list for extension extendee - 0, // [0:4] is the sub-list for field type_name + 6, // 0: codefly.services.builder.v0.DockerBuildContext.cache:type_name -> codefly.services.builder.v0.BuildCacheOptions + 7, // 1: codefly.services.builder.v0.DockerBuildRecipe.build_args:type_name -> codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry + 4, // 2: codefly.services.builder.v0.DockerBuildPlan.recipes:type_name -> codefly.services.builder.v0.DockerBuildRecipe + 3, // 3: codefly.services.builder.v0.DockerBuildPlan.files:type_name -> codefly.services.builder.v0.RecipeFile + 0, // 4: codefly.services.builder.v0.DockerBuildPlan.scope:type_name -> codefly.services.builder.v0.RecipeInventoryScope + 5, // [5:5] is the sub-list for method output_type + 5, // [5:5] is the sub-list for method input_type + 5, // [5:5] is the sub-list for extension type_name + 5, // [5:5] is the sub-list for extension extendee + 0, // [0:5] is the sub-list for field type_name } func init() { file_codefly_services_builder_v0_docker_proto_init() } @@ -571,13 +662,14 @@ func file_codefly_services_builder_v0_docker_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_codefly_services_builder_v0_docker_proto_rawDesc), len(file_codefly_services_builder_v0_docker_proto_rawDesc)), - NumEnums: 0, + NumEnums: 1, NumMessages: 7, NumExtensions: 0, NumServices: 0, }, GoTypes: file_codefly_services_builder_v0_docker_proto_goTypes, DependencyIndexes: file_codefly_services_builder_v0_docker_proto_depIdxs, + EnumInfos: file_codefly_services_builder_v0_docker_proto_enumTypes, MessageInfos: file_codefly_services_builder_v0_docker_proto_msgTypes, }.Build() File_codefly_services_builder_v0_docker_proto = out.File diff --git a/proto/codefly/services/builder/v0/builder.proto b/proto/codefly/services/builder/v0/builder.proto index 6e3042fff..8dc8db9d4 100644 --- a/proto/codefly/services/builder/v0/builder.proto +++ b/proto/codefly/services/builder/v0/builder.proto @@ -254,8 +254,8 @@ message BuildRequest { // build_context is the container build context supplied to the builder. BuildContext build_context = 1; // output_directory is an absolute destination owned by the caller where the - // builder writes reproducible build recipes. Empty means the builder does not - // emit recipes and performs the legacy in-agent build. + // builder writes reproducible build recipes. Required for application image + // builds; managed-image and runtime-only agents may omit it. string output_directory = 2; } diff --git a/proto/codefly/services/builder/v0/docker.proto b/proto/codefly/services/builder/v0/docker.proto index bb4ba225c..859bb5f4c 100644 --- a/proto/codefly/services/builder/v0/docker.proto +++ b/proto/codefly/services/builder/v0/docker.proto @@ -42,7 +42,12 @@ message DockerBuildRecipe { string name = 1; // dockerfile is the output_directory-relative path to the Dockerfile. string dockerfile = 2; - // context is the build-context directory the Dockerfile is evaluated against. + // context is the build-context directory the Dockerfile is evaluated against, + // relative to the SERVICE directory — not to output_directory, which the + // dockerfile and dockerignore paths are relative to. "." therefore means "build + // the service" while the Dockerfile itself lives in the service's builder/ + // subdirectory. The executor resolves it and is the party that enforces it + // stays inside the service. string context = 3; // dockerignore is the optional output_directory-relative ignore file. string dockerignore = 4; @@ -57,6 +62,27 @@ message DockerBuildRecipe { string target = 8; } +// RecipeInventoryScope states what a plan's file inventory claims to cover, so +// the verifier knows what "complete" means for that plan. Without it the +// verifier has to guess, and the two answers are not interchangeable: guessing +// EMITTED for a TREE plan stops detecting files ADDED to a build context after +// emission, and guessing TREE for an EMITTED plan fails a build over unrelated +// content in the service's committed builder/ directory. +enum RecipeInventoryScope { + // RECIPE_INVENTORY_SCOPE_UNSPECIFIED does not authorize verification; a plan + // that omits the scope is rejected rather than verified under a default. + RECIPE_INVENTORY_SCOPE_UNSPECIFIED = 0; + // RECIPE_INVENTORY_SCOPE_EMITTED claims only the files the emitting build + // wrote. Other entries under output_directory are outside the claim and are + // neither digested nor rejected. + RECIPE_INVENTORY_SCOPE_EMITTED = 1; + // RECIPE_INVENTORY_SCOPE_TREE claims every file under output_directory. The + // emitter assembled the whole destination — typically because it copied the + // build context there and a recipe builds "." — so an entry the inventory does + // not list is drift in the emitter's own output and fails verification. + RECIPE_INVENTORY_SCOPE_TREE = 2; +} + // DockerBuildPlan is the reproducible recipe set a builder emits to the caller's // output_directory. The caller owns running docker buildx from the recipes, so // the recipes are a durable, first-class artifact rather than an image built @@ -64,13 +90,20 @@ message DockerBuildRecipe { message DockerBuildPlan { // recipes is the ordered set of image build recipes for the service. repeated DockerBuildRecipe recipes = 1; - // files is the canonical, sorted inventory of recipe files with digests. + // files is the canonical, sorted inventory of the files the emitting build + // wrote, with digests. It is not an inventory of everything in + // output_directory: that directory is the service's committed builder/, so + // unrelated content there must neither perturb the digest nor fail the build. repeated RecipeFile files = 2; // digest is the aggregate content digest over files, formatted as // "sha256:". Identical recipe trees yield an identical digest. string digest = 3; // contract_version identifies the recipe contract the caller validates. string contract_version = 4; + // scope states what the files inventory covers. It is part of the aggregate + // digest, so a plan cannot be downgraded to a weaker verification by + // rewriting this field. + RecipeInventoryScope scope = 5; } // BuildCacheOptions is caller-owned transport policy. Registry credentials are diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index 0c15b1dd9..e0c19aa36 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -4,15 +4,12 @@ import ( "context" "embed" "fmt" - "io" - "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" "github.com/codefly-dev/core/agents/services" "github.com/codefly-dev/core/builders" "github.com/codefly-dev/core/resources" - "github.com/codefly-dev/core/shared" "github.com/codefly-dev/core/wool" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -31,8 +28,8 @@ type DockerTemplating struct { SourceDir string // e.g. "code/cmd/server" — the Go main package location ModuleRoot string // e.g. "code" — where go.mod lives BuildTarget string // e.g. "./cmd/server" — package to build (relative to ModuleRoot) - ContextRoot string // optional exact Docker context root; defaults to the service directory - Workspace bool // template hint for workspace-aware source copying + ContextRoot string // non-empty custom contexts are rejected before preparation + Workspace bool // workspace-root source copying; rejected before preparation } // DockerEnv is a key-value pair for Docker environment variables. @@ -41,21 +38,25 @@ type DockerEnv struct { Value string } -// BuildGoDocker generates templates and builds a Docker image for a Go service. +// BuildGoDocker emits a Docker build recipe for a Go service. func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, location string, + req *builderv0.BuildRequest, requirements *builders.Dependencies, builderFS embed.FS, goVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { w := wool.Get(ctx).In("golang.BuildGoDocker") + if !services.BuildPlanRequested(req) { + return builder.BuildError(fmt.Errorf("BuildRequest.output_directory is required for image recipes")) + } + dockerRequest, err := builder.DockerBuildRequest(ctx, req) if err != nil { return nil, w.Wrapf(err, "docker build request") } image := builder.DockerImage(dockerRequest) - w.Debug("building docker image", wool.Field("image", image.FullName())) + w.Debug("preparing docker image recipe", wool.Field("image", image.FullName())) if !dockerhelpers.IsValidDockerImageName(image.Name) { return builder.BuildError(fmt.Errorf("invalid docker image name: %s", image.Name)) @@ -70,87 +71,32 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } - _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - - err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) - if err != nil { - return builder.BuildError(err) + // ContextRoot and Workspace express one requirement — build from a directory + // above the service — and agents set them together. Rejecting only ContextRoot + // would let Workspace through, and a workspace template renders COPY paths + // relative to the workspace root while the recipe pins the context to the + // service directory: the build would then fail inside the CLI's buildx run + // with an opaque "COPY failed: file not found" instead of here, where the + // error can name the unsupported setting. + if docker.ContextRoot != "" { + return builder.BuildError(fmt.Errorf("custom Docker context root %q is not supported by image recipes", docker.ContextRoot)) } - // When the caller owns the build (output_directory set), emit the recipe and - // let the caller run docker buildx instead of building the image in-process. - // A custom ContextRoot builds from a directory other than the service dir, so - // the "context is the service directory" recipe model does not hold — fall - // through to the in-process build, and the caller uses its legacy push path. - if services.BuildPlanRequested(req) { - if docker.ContextRoot == "" { - return builder.SingleImageBuildResponse(req, image.FullName()) - } - // The caller asked for a recipe but a custom ContextRoot forces the legacy - // in-process build; surface it so a caller expecting a plan isn't left - // wondering why it got a DockerBuildResult instead. - w.Warn("recipe emission requested but skipped: service uses a custom Docker context root; building in-process", - wool.Field("context_root", docker.ContextRoot)) + if docker.Workspace { + return builder.BuildError(fmt.Errorf("workspace-root Docker contexts are not supported by image recipes")) } - configuration, err := goDockerBuilderConfiguration(location, image, w, docker) + emitted, err := services.PrepareRecipeDestination(builderFS, req.GetOutputDirectory()) if err != nil { return builder.BuildError(err) } - configuration.Cache = dockerRequest.GetCache() - configuration.BuildxBuilder = dockerRequest.GetBuildxBuilder() - b, err := dockerhelpers.NewBuilder(configuration) - if err != nil { - return builder.BuildError(err) - } - _, err = b.Build(ctx) + + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) } - builder.WithDockerImages(image) - resp, err := builder.BuildResponse() - if resp != nil { - resp.BuildxBuilder = dockerRequest.GetBuildxBuilder() - } - if resp != nil && dockerRequest.GetCache() != nil { - resp.CacheContractVersion = dockerhelpers.CacheContractVersion - } - return resp, err -} -func goDockerBuilderConfiguration( - location string, - image *resources.DockerImage, - output io.Writer, - docker DockerTemplating, -) (dockerhelpers.BuilderConfiguration, error) { - contextRoot := docker.ContextRoot - if contextRoot == "" { - contextRoot = location - } - resolvedRoot, err := filepath.EvalSymlinks(contextRoot) - if err != nil { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf("resolve Docker context root: %w", err) - } - resolvedLocation, err := filepath.EvalSymlinks(location) - if err != nil { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf("resolve service directory: %w", err) - } - relative, err := filepath.Rel(resolvedRoot, resolvedLocation) - if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf( - "service directory %q is outside Docker context root %q", - resolvedLocation, - resolvedRoot, - ) - } - return dockerhelpers.BuilderConfiguration{ - Root: resolvedRoot, - Dockerfile: filepath.ToSlash(filepath.Join(relative, "builder", "Dockerfile")), - Ignorefile: filepath.ToSlash(filepath.Join(relative, "builder", "dockerignore")), - Destination: image, - Output: output, - }, nil + return builder.SingleImageBuildResponse(req, image.FullName(), emitted) } // DeployGoKubernetes deploys a Go service to Kubernetes. diff --git a/runners/golang/agent_builder_cache_test.go b/runners/golang/agent_builder_cache_test.go deleted file mode 100644 index 71771088d..000000000 --- a/runners/golang/agent_builder_cache_test.go +++ /dev/null @@ -1,133 +0,0 @@ -package golang - -import ( - "bytes" - "context" - "embed" - "encoding/json" - "fmt" - "net" - "net/http" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/codefly-dev/core/agents/services" - "github.com/codefly-dev/core/builders" - basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" - builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" - "github.com/codefly-dev/core/resources" - "github.com/stretchr/testify/require" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" -) - -//go:embed templates/builder -var cacheBuilderTemplates embed.FS - -type legacyGoCacheBuilder struct { - *services.DefaultBuilder - wrapper *services.BuilderWrapper - root string - location string -} - -func (builder *legacyGoCacheBuilder) Build(ctx context.Context, request *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { - return BuildGoDocker(ctx, builder.wrapper, request, builder.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "", func(options *DockerTemplating) { options.ContextRoot = builder.root }) -} - -func (*legacyGoCacheBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { - return &builderv0.BuildCapabilitiesResponse{BuildxSelection: true}, nil -} - -func TestCustomContextBuildUsesRequestedBuildxAndExportsCache(t *testing.T) { - if os.Getenv("CODEFLY_TEST_REGISTRY_CACHE") != "1" { - t.Skip("set CODEFLY_TEST_REGISTRY_CACHE=1 to build against a disposable registry") - } - ctx, cancel := context.WithTimeout(t.Context(), 4*time.Minute) - defer cancel() - run := func(args ...string) string { - t.Helper() - cmd := exec.CommandContext(ctx, "docker", args...) - var stderr bytes.Buffer - cmd.Stderr = &stderr - output, err := cmd.Output() - require.NoError(t, err, "%v: %s", args, stderr.String()) - return strings.TrimSpace(string(output)) - } - cleanup := func(args ...string) { - t.Helper() - cleanupCtx, stop := context.WithTimeout(context.Background(), time.Minute) - defer stop() - output, err := exec.CommandContext(cleanupCtx, "docker", args...).CombinedOutput() - require.NoError(t, err, "%s", output) - } - name := fmt.Sprintf("codefly-builder-%d", time.Now().UnixNano()) - run("network", "create", name) - t.Cleanup(func() { cleanup("network", "rm", name) }) - registry := name + ".local" - run("run", "-d", "--name", registry, "--network", name, "-p", "127.0.0.1::5000", "registry:2") - t.Cleanup(func() { cleanup("rm", "-f", registry) }) - address := run("port", registry, "5000/tcp") - client := &http.Client{Timeout: time.Second} - require.Eventually(t, func() bool { - response, err := client.Get("http://" + address + "/v2/") - if err != nil { - return false - } - closeErr := response.Body.Close() - return response.StatusCode == http.StatusOK && closeErr == nil - }, 20*time.Second, 100*time.Millisecond) - config := filepath.Join(t.TempDir(), "buildkitd.toml") - require.NoError(t, os.WriteFile(config, []byte(fmt.Sprintf("[registry.\"%s:5000\"]\n http = true\n", registry)), 0o600)) - run("buildx", "create", "--name", name, "--driver", "docker-container", "--driver-opt", "network="+name, "--buildkitd-config", config, "--bootstrap") - t.Cleanup(func() { cleanup("buildx", "rm", name) }) - t.Setenv("BUILDX_BUILDER", "must-not-use-ambient-builder") - root := t.TempDir() - location := filepath.Join(root, "service") - require.NoError(t, os.Mkdir(location, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(root, "app"), []byte("legacy cached image"), 0o644)) - base := &services.Base{} - require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) - wrapper := &services.BuilderWrapper{Base: base} - base.Builder = wrapper - image := &resources.DockerImage{Name: name, Tag: "test"} - base.SetDockerImage(image) - request := &builderv0.BuildRequest{ - OutputDirectory: filepath.Join(location, "builder"), - BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: &builderv0.DockerBuildContext{ - BuildxBuilder: name, - Cache: &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{registry + ":5000/cache"}}, - }}}, - } - server := grpc.NewServer() - builderv0.RegisterBuilderServer(server, &legacyGoCacheBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper, root: root, location: location}) - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - go func() { _ = server.Serve(listener) }() - t.Cleanup(server.Stop) - connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, connection.Close()) }) - response, err := services.NewBuilderAgentClient(connection).Build(ctx, request) - require.NoError(t, err) - require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState()) - t.Cleanup(func() { cleanup("image", "rm", image.FullName()) }) - require.Nil(t, response.GetResult().GetDockerBuildPlan()) - require.Equal(t, name, response.GetBuildxBuilder()) - require.Equal(t, "registry-v1", response.GetCacheContractVersion()) - require.Equal(t, "amd64", run("image", "inspect", "--format", "{{.Architecture}}", image.FullName())) - tags, err := client.Get("http://" + address + "/v2/cache/tags/list") - require.NoError(t, err) - defer func() { require.NoError(t, tags.Body.Close()) }() - require.Equal(t, http.StatusOK, tags.StatusCode) - var manifest struct { - Tags []string `json:"tags"` - } - require.NoError(t, json.NewDecoder(tags.Body).Decode(&manifest)) - require.Len(t, manifest.Tags, 1) - require.True(t, strings.HasPrefix(manifest.Tags[0], "codefly-")) -} diff --git a/runners/golang/agent_builder_recipe_test.go b/runners/golang/agent_builder_recipe_test.go new file mode 100644 index 000000000..975ea9651 --- /dev/null +++ b/runners/golang/agent_builder_recipe_test.go @@ -0,0 +1,246 @@ +package golang + +import ( + "context" + "embed" + "net" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/codefly-dev/core/agents/services" + "github.com/codefly-dev/core/builders" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +//go:embed templates/builder +var recipeBuilderTemplates embed.FS + +type recipeBuilder struct { + *services.DefaultBuilder + wrapper *services.BuilderWrapper + contextRoot string + workspace bool +} + +func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { + return BuildGoDocker(ctx, b.wrapper, req, &builders.Dependencies{}, recipeBuilderTemplates, "", "", + func(options *DockerTemplating) { + options.ContextRoot = b.contextRoot + options.Workspace = b.workspace + }) +} + +func (*recipeBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { + return &builderv0.BuildCapabilitiesResponse{BuildxSelection: true}, nil +} + +// recipeAgent stands up the real gRPC Builder surface with Docker and Buildx +// absent from PATH, so nothing here can accidentally execute a build. +type recipeAgent struct { + client builderv0.BuilderClient + service string + output string + agent *recipeBuilder +} + +func newRecipeAgent(t *testing.T, ctx context.Context) *recipeAgent { + t.Helper() + t.Setenv("PATH", t.TempDir()) + for _, executable := range []string{"docker", "buildx"} { + _, err := exec.LookPath(executable) + require.Error(t, err) + } + + root := t.TempDir() + location := filepath.Join(root, "service") + output := filepath.Join(location, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + base := &services.Base{} + require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{ + Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) + wrapper := &services.BuilderWrapper{Base: base} + base.Builder = wrapper + base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) + + agent := &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper} + server := grpc.NewServer() + builderv0.RegisterBuilderServer(server, agent) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, connection.Close()) }) + + return &recipeAgent{client: builderv0.NewBuilderClient(connection), service: location, output: output, agent: agent} +} + +func (a *recipeAgent) request(dockerContext *builderv0.DockerBuildContext) *builderv0.BuildRequest { + return &builderv0.BuildRequest{ + OutputDirectory: a.output, + BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}, + } +} + +func selectedContext() *builderv0.DockerBuildContext { + return &builderv0.DockerBuildContext{BuildxBuilder: "selected"} +} + +func testContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + t.Cleanup(cancel) + return ctx +} + +func TestImageRecipeEmitsAVerifiablePlan(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + response, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + plan := response.GetResult().GetDockerBuildPlan() + require.NotNil(t, plan) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, plan)) + require.Len(t, plan.GetRecipes(), 1) + require.Equal(t, "app:test", plan.GetRecipes()[0].GetImage()) + require.Equal(t, services.RecipeBuildPlatforms(), plan.GetRecipes()[0].GetPlatforms()) + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) + + // This template set emits no dockerignore, so the recipe must not claim one. + require.Empty(t, plan.GetRecipes()[0].GetDockerignore()) +} + +// A stale recipe from an earlier build is replaced, not left in place. +func TestImageRecipeReplacesStaleDockerfile(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("FROM stale-image\n"), 0o644)) + + response, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) +} + +// The recipe directory is committed, so it carries whatever the repository and +// the developer's OS put there. None of it is a build input — the build context +// is the service directory — so it must neither fail the build nor move the +// digest. A symlink here used to abort the build naming a file unrelated to the +// image; a stray sidecar file used to change the digest between macOS and CI. +func TestImageRecipeIsUnaffectedByUnrelatedContentInTheRecipeDirectory(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + clean, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, clean.GetState().GetState(), clean.GetState().GetMessage()) + + require.NoError(t, os.WriteFile(filepath.Join(agent.service, "shared.conf"), []byte("x"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join(agent.service, "shared.conf"), filepath.Join(agent.output, "shared.conf"))) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, ".DS_Store"), []byte("mac"), 0o644)) + + dirty, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, dirty.GetState().GetState(), dirty.GetState().GetMessage()) + require.Equal(t, + clean.GetResult().GetDockerBuildPlan().GetDigest(), + dirty.GetResult().GetDockerBuildPlan().GetDigest()) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, dirty.GetResult().GetDockerBuildPlan())) +} + +// Caller transport policy (which Buildx builder, which cache) is the executor's +// concern. It must not reach the recipe, or the same source would emit different +// durable artifacts depending on how the caller happened to invoke the build. +func TestImageRecipeIsIndependentOfCallerTransportPolicy(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + plain, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + + cached := selectedContext() + cached.BuildxBuilder = "cache-selected" + cached.Cache = &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{"registry.example.com/cache"}} + selected, err := agent.client.Build(ctx, agent.request(cached)) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, selected.GetState().GetState(), selected.GetState().GetMessage()) + + require.Equal(t, + plain.GetResult().GetDockerBuildPlan().GetDigest(), + selected.GetResult().GetDockerBuildPlan().GetDigest()) + // The agent executes nothing, so it acknowledges no executor selection. + require.Empty(t, selected.GetBuildxBuilder()) + require.Empty(t, selected.GetCacheContractVersion()) +} + +func TestImageRecipeRejectsUnsupportedRequests(t *testing.T) { + for name, scenario := range map[string]struct { + mutate func(*recipeAgent, *builderv0.BuildRequest) + expected string + }{ + "missing output_directory": { + func(_ *recipeAgent, r *builderv0.BuildRequest) { r.OutputDirectory = "" }, + "output_directory is required", + }, + "relative output_directory": { + func(_ *recipeAgent, r *builderv0.BuildRequest) { r.OutputDirectory = "builder" }, + "must be absolute", + }, + "custom context root": { + func(a *recipeAgent, _ *builderv0.BuildRequest) { a.agent.contextRoot = a.service }, + "custom Docker context root", + }, + // Workspace carries the same requirement as ContextRoot and agents set the + // two together. Letting it through would emit a recipe whose context is the + // service directory while the template copies workspace-relative paths — + // failing later, inside the executor, with an opaque COPY error. + "workspace context": { + func(a *recipeAgent, _ *builderv0.BuildRequest) { a.agent.workspace = true }, + "workspace-root Docker contexts are not supported", + }, + } { + t.Run(name, func(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("original recipe"), 0o644)) + + request := agent.request(selectedContext()) + scenario.mutate(agent, request) + + response, err := agent.client.Build(ctx, request) + if err != nil { + require.ErrorContains(t, err, scenario.expected) + } else { + require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) + require.Contains(t, response.GetState().GetMessage(), scenario.expected) + } + + // A rejected request must not have touched the committed recipe. + contents, readErr := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(contents)) + entries, readErr := os.ReadDir(agent.output) + require.NoError(t, readErr) + require.Len(t, entries, 1) + }) + } +} diff --git a/runners/golang/agent_builder_test.go b/runners/golang/agent_builder_test.go deleted file mode 100644 index a3f207e77..000000000 --- a/runners/golang/agent_builder_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package golang - -import ( - "io" - "os" - "path/filepath" - "testing" - - "github.com/codefly-dev/core/resources" - "github.com/stretchr/testify/require" -) - -func TestGoDockerBuilderConfigurationUsesExactWorkspaceContext(t *testing.T) { - workspace := t.TempDir() - service := filepath.Join(workspace, "modules", "users", "services", "forge-edge") - require.NoError(t, os.MkdirAll(service, 0o755)) - - configuration, err := goDockerBuilderConfiguration( - service, - &resources.DockerImage{Name: "registry.example.com/forge-edge", Tag: "test"}, - io.Discard, - DockerTemplating{ContextRoot: workspace, Workspace: true}, - ) - require.NoError(t, err) - resolvedWorkspace, err := filepath.EvalSymlinks(workspace) - require.NoError(t, err) - require.Equal(t, resolvedWorkspace, configuration.Root) - require.Equal(t, "modules/users/services/forge-edge/builder/Dockerfile", configuration.Dockerfile) - require.Equal(t, "modules/users/services/forge-edge/builder/dockerignore", configuration.Ignorefile) -} - -func TestGoDockerBuilderConfigurationRejectsServiceOutsideContext(t *testing.T) { - workspace := t.TempDir() - service := t.TempDir() - - _, err := goDockerBuilderConfiguration( - service, - &resources.DockerImage{Name: "registry.example.com/forge-edge", Tag: "test"}, - io.Discard, - DockerTemplating{ContextRoot: workspace, Workspace: true}, - ) - require.ErrorContains(t, err, "outside Docker context root") -} diff --git a/runners/rust/agent_builder.go b/runners/rust/agent_builder.go index 055f94020..3efc0d052 100644 --- a/runners/rust/agent_builder.go +++ b/runners/rust/agent_builder.go @@ -10,7 +10,6 @@ import ( "github.com/codefly-dev/core/agents/services" "github.com/codefly-dev/core/builders" "github.com/codefly-dev/core/resources" - "github.com/codefly-dev/core/shared" "github.com/codefly-dev/core/wool" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -34,22 +33,25 @@ type DockerEnv struct { Value string } -// BuildRustDocker generates templates and builds a Docker image for a Rust -// service. Mirrors golang.BuildGoDocker. +// BuildRustDocker emits a Docker build recipe for a Rust service. func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, location string, + req *builderv0.BuildRequest, requirements *builders.Dependencies, builderFS embed.FS, rustVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { w := wool.Get(ctx).In("rust.BuildRustDocker") + if !services.BuildPlanRequested(req) { + return builder.BuildError(fmt.Errorf("BuildRequest.output_directory is required for image recipes")) + } + dockerRequest, err := builder.DockerBuildRequest(ctx, req) if err != nil { return nil, w.Wrapf(err, "docker build request") } image := builder.DockerImage(dockerRequest) - w.Debug("building docker image", wool.Field("image", image.FullName())) + w.Debug("preparing docker image recipe", wool.Field("image", image.FullName())) if !dockerhelpers.IsValidDockerImageName(image.Name) { return builder.BuildError(fmt.Errorf("invalid docker image name: %s", image.Name)) @@ -64,44 +66,17 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } - _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - - err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) + emitted, err := services.PrepareRecipeDestination(builderFS, req.GetOutputDirectory()) if err != nil { return builder.BuildError(err) } - // When the caller owns the build (output_directory set), emit the recipe and - // let the caller run docker buildx instead of building the image in-process. - if services.BuildPlanRequested(req) { - return builder.SingleImageBuildResponse(req, image.FullName()) - } - - b, err := dockerhelpers.NewBuilder(dockerhelpers.BuilderConfiguration{ - Root: location, - Cache: dockerRequest.GetCache(), - BuildxBuilder: dockerRequest.GetBuildxBuilder(), - Dockerfile: "builder/Dockerfile", - Ignorefile: "builder/dockerignore", - Destination: image, - Output: w, - }) + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) } - _, err = b.Build(ctx) - if err != nil { - return builder.BuildError(err) - } - builder.WithDockerImages(image) - resp, err := builder.BuildResponse() - if resp != nil { - resp.BuildxBuilder = dockerRequest.GetBuildxBuilder() - } - if resp != nil && dockerRequest.GetCache() != nil { - resp.CacheContractVersion = dockerhelpers.CacheContractVersion - } - return resp, err + + return builder.SingleImageBuildResponse(req, image.FullName(), emitted) } // DeployRustKubernetes deploys a Rust service to Kubernetes. Identical in diff --git a/runners/rust/agent_builder_recipe_test.go b/runners/rust/agent_builder_recipe_test.go new file mode 100644 index 000000000..386d3c5f3 --- /dev/null +++ b/runners/rust/agent_builder_recipe_test.go @@ -0,0 +1,169 @@ +package rust + +import ( + "context" + "embed" + "net" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/codefly-dev/core/agents/services" + "github.com/codefly-dev/core/builders" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +// A two-file template set — Dockerfile and dockerignore — so the runner is +// exercised with a recipe that emits more than one file, which is where a guard +// scoped to a single filename leaks. +// +//go:embed templates/builder +var recipeBuilderTemplates embed.FS + +type recipeBuilder struct { + *services.DefaultBuilder + wrapper *services.BuilderWrapper +} + +func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { + return BuildRustDocker(ctx, b.wrapper, req, &builders.Dependencies{}, recipeBuilderTemplates, "", "") +} + +type recipeAgent struct { + client builderv0.BuilderClient + service string + output string +} + +func newRecipeAgent(t *testing.T, ctx context.Context) *recipeAgent { + t.Helper() + t.Setenv("PATH", t.TempDir()) + for _, executable := range []string{"docker", "buildx"} { + _, err := exec.LookPath(executable) + require.Error(t, err) + } + + root := t.TempDir() + location := filepath.Join(root, "service") + output := filepath.Join(location, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + base := &services.Base{} + require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{ + Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) + wrapper := &services.BuilderWrapper{Base: base} + base.Builder = wrapper + base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) + + server := grpc.NewServer() + builderv0.RegisterBuilderServer(server, &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper}) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, connection.Close()) }) + + return &recipeAgent{client: builderv0.NewBuilderClient(connection), service: location, output: output} +} + +func (a *recipeAgent) request() *builderv0.BuildRequest { + return &builderv0.BuildRequest{ + OutputDirectory: a.output, + BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{ + DockerBuildContext: &builderv0.DockerBuildContext{BuildxBuilder: "selected"}}}, + } +} + +func testContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + t.Cleanup(cancel) + return ctx +} + +func TestImageRecipeEmitsEveryTemplateFile(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + response, err := agent.client.Build(ctx, agent.request()) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + plan := response.GetResult().GetDockerBuildPlan() + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, plan)) + require.Equal(t, "dockerignore", plan.GetRecipes()[0].GetDockerignore()) + + inventory := make([]string, 0, len(plan.GetFiles())) + for _, file := range plan.GetFiles() { + inventory = append(inventory, file.GetPath()) + } + require.Equal(t, []string{"Dockerfile", "dockerignore"}, inventory) +} + +// The renderer follows symlinks on write, so any emitted path that is a symlink +// gets written *through*, truncating a file outside the caller-owned directory. +// A guard scoped to "Dockerfile" left every other file in the template set +// exposed; the recipe directory is committed, so a symlinked dockerignore is +// ordinary repository content rather than an attack precondition. +func TestImageRecipeNeverWritesThroughASymlinkedDestination(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + targets := map[string]string{"Dockerfile": "dockerfile target", "dockerignore": "ignore target"} + for name, content := range targets { + outside := filepath.Join(agent.service, name+".outside") + require.NoError(t, os.WriteFile(outside, []byte(content), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(agent.output, name))) + } + + response, err := agent.client.Build(ctx, agent.request()) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, response.GetResult().GetDockerBuildPlan())) + + for name, content := range targets { + got, readErr := os.ReadFile(filepath.Join(agent.service, name+".outside")) + require.NoError(t, readErr) + require.Equal(t, content, string(got), "%s target was written through the symlink", name) + } + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) +} + +func TestImageRecipeRequiresAnAbsoluteDestination(t *testing.T) { + for name, scenario := range map[string]struct{ directory, expected string }{ + "missing": {"", "output_directory is required"}, + "relative": {"builder", "must be absolute"}, + } { + t.Run(name, func(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("original recipe"), 0o644)) + + request := agent.request() + request.OutputDirectory = scenario.directory + response, err := agent.client.Build(ctx, request) + if err != nil { + require.ErrorContains(t, err, scenario.expected) + } else { + require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) + require.Contains(t, response.GetState().GetMessage(), scenario.expected) + } + + contents, readErr := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(contents)) + }) + } +} diff --git a/runners/rust/templates/builder/Dockerfile.tmpl b/runners/rust/templates/builder/Dockerfile.tmpl new file mode 100644 index 000000000..12c97b01d --- /dev/null +++ b/runners/rust/templates/builder/Dockerfile.tmpl @@ -0,0 +1,2 @@ +FROM scratch +COPY app /app diff --git a/runners/rust/templates/builder/dockerignore b/runners/rust/templates/builder/dockerignore new file mode 100644 index 000000000..eb5a316cb --- /dev/null +++ b/runners/rust/templates/builder/dockerignore @@ -0,0 +1 @@ +target diff --git a/version/info.codefly.yaml b/version/info.codefly.yaml index 63d3b5b3b..3a0891ffb 100644 --- a/version/info.codefly.yaml +++ b/version/info.codefly.yaml @@ -1 +1 @@ -version: 0.3.27 +version: 0.3.28