feat(server): configure buildx driver via plugin configuration - #419
Merged
Conversation
The buildx driver and its --driver-opt values could only be set through the TRDL_BUILDX_DRIVER and TRDL_BUILDX_DRIVER_OPTS_* environment variables of the Vault process. The secret engine is also compiled into host processes whose environment the administrator does not control, and there those variables cannot be set at all, while `configure` is already the per-project channel for the git, s3 and quorum settings. Add the optional `buildx_driver` and `buildx_driver_opts` fields to `configure` and pass them down to `docker buildx create`. Each setting is resolved on its own: the plugin configuration takes precedence over the environment, and the environment over the previous default. With neither field set, the invocation is unchanged. `buildx_driver` is validated against the same allowlist at configure time, so an unsupported driver is rejected when it is written rather than on the next release. `buildx_driver_opts` is a list carrying one option per element, passed through as is, keeping the semantics of the environment form, where a value containing commas is not split. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Vasily Marmer <vasily.marmer@flant.com>
Both fields are optional, but nothing asserted it: every existing case sends a complete payload, so making either of them required, or giving buildx_driver_opts a non-empty default, would have gone unnoticed until an operator's next configure call failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Vasily Marmer <vasily.marmer@flant.com>
Three gaps found while reviewing the buildx configuration fields. Nothing asserted that the configured driver reaches the builder: dropping the assignment in either BuildReleaseArtifacts or NewBuilder left the whole suite green. Building with an unsupported driver now fails before any docker invocation, which pins the forwarding without a daemon. A configuration stored before these fields existed carries neither key, and nothing decoded such an entry. A rejected update also had no test proving the previously stored configuration survives it, so moving the validation below the write would have gone unnoticed. Also state, in the code and in both QUICKSTART pages, that an empty or omitted field means "not configured" and falls back to the environment rather than overriding it with an empty value: configure cannot tell an omitted field from an explicitly empty one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Vasily Marmer <vasily.marmer@flant.com>
A buildkitd address replaces the whole buildx path: no builder is created, so `buildx_driver` and `buildx_driver_opts` written next to it in the same `configure` call would silently do nothing. Reject the combination when the configuration is written, the way an unsupported driver and an unsupported address scheme are already rejected there, instead of leaving the operator to discover it from a build that ignores the driver. The environment variables are deliberately not part of this check: they are process-wide and can change after a project is configured, so a per-project write must not fail over them. Also pass a context to ValidateBuildxDriver, which AGENTS.md requires of an exported function, and drop buildkitd_address from the completeConfiguration fixture, which described a configuration that is now invalid. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
The rejection lives in the callback both create and update are registered on, but it was only exercised through create, so moving the check below the storage write would have kept the suite green. Assert that a rejected update leaves the stored configuration untouched, the way the unsupported-driver case already does. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
…nused ones The mutual-exclusion check read the raw field values while validation and resolution trim them, so a request carrying `buildkitd_address=" "` next to a driver was rejected even though the address means "not set" everywhere else, and an all-blank `buildx_driver_opts` counted as configured options. Compare the same way resolution does. The check also cannot see an address coming from TRDL_BUILDKITD_ADDRESS, which is process-wide and can change after a project is configured. In that shape the configured driver settings are unreachable rather than rejected, so the build now says so in the plugin log, and both QUICKSTART pages state it. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
…warding Two tests reached the real docker CLI. Both rely on driver validation stopping the build before `docker buildx create` runs, and nothing in the production code guarantees that: with the resolution precedence inverted, one of them provisioned a real BuildKit Deployment in the cluster of the machine running it, then deadlocked on an artifacts pipe nobody read. An empty PATH now makes such a regression fail the test, and the pipe is drained. Nothing covered the two lines in `pathRelease` that carry the stored settings into the release build: deleting both left the whole suite green while every release silently fell back to the environment. The new e2e job configures the working driver through `configure` while the environment names one that cannot work without a cluster, so the release only succeeds if the configured value still travels the whole way. Alongside that: the "settings unused" warning also goes to the task log the release operator reads, its condition treats blank values as unset like the rest of the resolution does, and the pre-existing builder test that constructs NewBuilderOpts without a logger now passes one, since that branch dereferences it. Adds a test for the full-replacement semantics of `configure`: omitting the buildx fields clears the stored ones. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
Member
Author
Verification
Review focus
Follow-up
|
alexey-igrychev
added a commit
that referenced
this pull request
Aug 5, 2026
…mines (#420) ## Summary `task server:test:unit paths="./pkg/docker/..."` now runs one suite in 4s instead of the whole module in 1m50s. The var was documented in AGENTS.md but the task hardcoded `./...`, so every scoped attempt silently ran everything. ## What - `server:test:unit`, `server:test:ai` and `e2e:test:e2e` pass `paths` through to ginkgo, defaulting to `./...`; `e2e:test:e2e` also passes `focusFilter` and `labelFilter`, which AGENTS.md documented and the task never used. - AGENTS.md states that namespaced targets exist only in the root Taskfile, so `task` runs from the repository root — `task server:test:unit` inside `server/` fails with "does not exist". - AGENTS.md states that `logboek.Context()` returns the default logger only for exactly `context.Background()`, and panics on any derived context with no logger bound. - `.agents/skills/test-the-tests` gains one mutation hazard: a test whose only barrier before a real daemon is a validation error inside the code being mutated provisions real infrastructure once that validation is inverted, and hangs instead of failing when nothing drains what the code writes. - Nothing changes for an unscoped run: the default keeps the previous argument list. ## Why All four items are this session's cost, not hypotheticals. The `paths` gap made every targeted check a full-module wait. The two AGENTS.md lines each burned a step — one failed invocation and one test that panicked inside logboek. The skill bullet is the mechanism behind an incident an independent reviewer hit while mutating #419: the mutated validation was the only thing keeping a unit test away from the machine's Docker, so the run created two BuildKit Deployments in a live cluster and deadlocked with no `FAIL` line at all. Signed-off-by: Aleksei Igrychev <aleksei.igrychev@palark.com>
alexey-igrychev
marked this pull request as ready for review
August 5, 2026 21:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A project can now choose its buildx driver and driver options through
configure, instead of only through the environment of the Vault process. That is the only route available when the secret engine is compiled into a host process whose environment the administrator cannot set, where thekubernetesdriver was unreachable until now.Continues #412 by @vmrm, rebased onto
mainafter #409.What
configureacceptsbuildx_driver—docker-containerorkubernetes, empty by default — and it wins overTRDL_BUILDX_DRIVER.configureacceptsbuildx_driver_opts— a list, one--driver-optper element, empty by default — and it wins overTRDL_BUILDX_DRIVER_OPTS_*. Elements are passed through verbatim, sonodeselector=disktype=ssd,zone=astays one option.docker-containerwith no options. A field omitted or set to a blank value means "not configured" and falls back to the environment instead of clearing it, so building with no options while the environment defines some requires unsetting those variables.buildx_driver=dockerfailsconfigureand stores nothing, while an unsupportedTRDL_BUILDX_DRIVERstill fails when the builder is created. The error names the setting the value came from.configurerejectsbuildx_driverorbuildx_driver_optswritten together withbuildkitd_address, and leaves an already stored configuration untouched: that address replaces the buildx path entirely, so the driver settings would have no effect. Blank values count as unset on both sides of that check.TRDL_BUILDKITD_ADDRESSinstead, the combination cannot be refused at write time — the variable is process-wide and can change after a project is configured — so the release reportsthe configured buildx driver settings are not usedin both the release task log and the plugin log.configurereplaces the whole document.docker buildx createis invoked exactly as before, down to the argument list, and a configuration stored before these fields existed reads back with both fields empty and builds as it did.buildx_driver_optsandbuildkitd_addressreach the release build at all — deleting their assignment inpathReleaseleaves every suite green. The driver now has an end-to-end guard; these two would need one job each, because the value has to break the build when it is lost.Why
Everything else a project needs already lives in
configure— the Git repository, the S3 credentials, the signature quorum — because Vault has no other per-project channel. The build backend was the exception: it could only be set through the environment of the process that hosts the plugin, and a host process that ships the engine as a built-in plugin may expose no way to set one, which left such an installation on the default driver permanently.Environment-only was the alternative, and #409 shows why it does not hold: the same argument produced
buildkitd_addressas aconfigurefield, so keeping the driver out ofconfigurewould have split one decision across two mechanisms.