feat(server): build releases in kubernetes without the docker binary

[vmrm]

Summary

A project can now build releases with no docker binary next to the plugin and no long-lived BuildKit daemon: buildkitd_driver=kubernetes has the plugin create a buildkitd Pod for one build, talk to it with the BuildKit client over pods/exec, and delete it afterwards. Until now those two properties were mutually exclusive — the buildx drivers shell out to the docker CLI, while buildkitd_address needs a daemon somebody else keeps running and shares between projects and builds.

Continues #409 and #419.

What

• configure accepts buildkitd_driver — kubernetes, empty by default — and buildkitd_driver_opts, a list of name=value pairs.
• With buildkitd_driver=kubernetes a release creates one Pod trdl-builder-<build id> in the target namespace, waits for its readiness probe, streams the build through the API server's exec channel, and deletes the Pod when the build ends. No external binary is executed on any part of that path.
• The permissions needed in that namespace are create, get and delete on pods, plus create on pods/exec. A namespaced Role is enough and no apps API group is touched. The plugin needs no network route to the builder pod.
• The Pod is deleted on every exit path the plugin controls: build success, build failure, a canceled release, a builder that never becomes ready, and a create whose response was lost after the API server had already persisted the pod. VERIFIED: a release through this driver against a kind cluster left no pod behind, and each cleanup path is held by a mutation rather than by a passing test alone — see the comment.
• The Pod is not deleted if the plugin's own process dies outright. The deadline option caps how long an abandoned builder keeps running, via activeDeadlineSeconds; it is a whole number of seconds of at least 1s, because anything smaller truncates to zero and anything fractional loses its remainder. UNVERIFIED: that the kubelet enforces it as expected here — a dispatch of the opt-in workflow with a short deadline would settle it.
• Driver options are validated when the configuration is written: buildkitd_driver_opts=replicas=3, a negative quantity such as requests.cpu=-1, and a truncating deadline=500ms all fail configure and store nothing, instead of failing a release hours later. Accepted names are annotations, deadline, image, labels, limits.cpu, limits.ephemeral-storage, limits.memory, namespace, nodeselector, requests.cpu, requests.ephemeral-storage, requests.memory, rootless, serviceaccount and timeout.
• configure rejects buildkitd_driver written together with buildkitd_address, buildx_driver or buildx_driver_opts, and a rejected write leaves an already stored configuration untouched.
• With TRDL_BUILDKITD_ADDRESS set on the Vault process the combination cannot be refused at write time, so the build states that the buildkitd driver settings are unused, in the release log and the plugin log — the treatment buildx_driver already gets.
• The builder is a bare Pod with restartPolicy: Never, not a Deployment: a replacement pod would be a builder the release is not connected to, and apps/v1 cannot carry activeDeadlineSeconds at all, so a Deployment builder survives a crashed plugin forever.
• The builder container runs privileged by default, because BuildKit needs it; rootless=true trades that for seccomp Unconfined and the unconfined AppArmor annotation. Either way the namespace has to admit what baseline PodSecurity refuses, and whoever can write a project's configure chooses the namespace, the ServiceAccount and the image of that pod — configure write access is pod-create access wherever the plugin's Role reaches. Both QUICKSTART locales state this as the administrator's responsibility.
• Defaults: image moby/buildkit:buildx-stable-1, or its -rootless variant when rootless=true; readiness wait 2m (timeout), the same default buildx uses for the same wait; no deadline unless configured.
• The two fields have no TRDL_* counterpart, unlike every other build-backend setting. That is deliberate: an environment twin is what makes a value impossible to reject at write time, and rejecting bad options at write time is the point of a separate vocabulary.
• New dependencies: k8s.io/api, k8s.io/apimachinery, k8s.io/client-go. Measured on this branch as unique modules in server/go.sum after go mod tidy and a successful go build ./...: 240 → 278, so 38 new modules, 117 new linked packages, and the plugin binary goes 48.8 MB → 61.1 MB.
• Nothing changes for a project that sets neither field: the docker CLI path and buildkitd_address behave exactly as before, and buildx_driver/buildx_driver_opts keep meaning what they meant.

Why

buildkitd_address was added in #409 for a plugin embedded in a distroless image, where docker cannot exist. It removes the binary but not the daemon: trdl connects to a buildkitd somebody else runs, so that daemon outlives the build and is shared by whatever else points at it. Where the build executes untrusted instructions, a daemon that survives the build and reaches the next project is the thing being designed against — and the buildx kubernetes driver, which does provision one builder per build, is unreachable there because it goes through the docker CLI. The ephemeral-builder property and the no-docker property could not be had together.

They are separable, because the CLI is not what talks to Kubernetes. The buildx kubernetes driver is pure Go: it creates its objects with client-go and dials the builder with remotecommand over pods/exec, executing no external process anywhere on that path. The docker requirement comes only from trdl driving that driver through the CLI wrapper.

Which leaves a choice this PR should not make alone.

Import github.com/docker/buildx/driver/kubernetes. Least new code, and behaviour already proven in the wild. Two things argue against it, both measured rather than estimated. Cost: v0.31.0 adds 88 modules to server/go.sum and takes the plugin binary to 97.6 MB; v0.36.1 adds 183, forces go 1.26.3 on the module, and pulls buildkit, docker/cli and containerd up with it. For scale, #409 accepted 46 modules and +8.9 MB — that is a precedent, not a rule, but 88 is twice it. Blocker: buildx's kubernetes factory cannot be handed a rest.Config. driver.Register(&factory{}) registers a zero value and its cc ClientConfig field is unexported, so the in-cluster path is reachable only through an undocumented EndpointAddr = "kubernetes://" with a nil ContextStore, and any other endpoint nil-panics inside ConfigFromEndpoint.

A minimal implementation on client-go, which is what this PR does: 38 modules, +12.3 MB, and roughly 400 lines. The pod calls go through a plain REST client rather than the generated typed client, which reaches every API group and its apply configurations — the same trim buildx made for itself in v0.33.0 (5a7f7c28, "kubernetes: trim client-go dependency surface"), and worth 23 MB of binary here. The cost is that the Deployment/StatefulSet manifest matrix, the pod chooser and the driver-opt surface are now trdl's to maintain, and the option vocabulary is a documented subset rather than all of buildx's.

If the answer is the first one, the conversation is worth having about driver factories generally rather than the kubernetes one alone: buildx's remote driver would come with it, and it already implements TLS to an external buildkitd through cacert/cert/key/servername driver-opts — which buildkitd_address has no answer for today. The honest caveat is that it reads that material from files by absolute path, which a plugin holding its configuration in Vault does not have.

On naming: buildx_driver_mode=in-process reusing buildx_driver_opts was the first shape tried and was dropped. It keeps the buildx name for a path with no buildx in it; ten of buildx's documented kubernetes driver-opts would be accepted by configure and rejected only at release time, which is what ff76158 refused to allow for this exact field pair; and a mode field valid for exactly one value of a sibling field encodes three states in four cells. A buildkitd_* field also follows the precedent already in this surface — an externally managed daemon was not modelled as buildx_driver=remote either.

[vmrm]

Verification

A first run against a real cluster failed, and the failure was a real defect rather than a flaky job — that run is the reason for the fourth commit.

• VERIFIED against kind, Tests. In-cluster Kubernetes builders: a release through buildkitd_driver=kubernetes supplied by configure creates default/trdl-builder-<uuid>, reports Build is successful after 6.96s, the suite passes, and the job's own post-check prints pods still in the namespace: 0. The buildx job in the same workflow stays green alongside it.

• The run before the fix failed with rpc error: code = Unavailable desc = error reading from server: context canceled, seconds after the builder pod came up. Cause: the exec stream was scoped to the context gRPC hands a dialer, and gRPC cancels that context once the transport is established, so the stream the connection is made of was torn down on every successful dial. driver/kubernetes/driver.go in buildx sidesteps this by discarding the dialer's context — its dialer literally takes func(context.Context, string) with both parameters unnamed — and using the one Client was given. This driver now does the same.

• Mutation: dialerFor handing the stream gRPC's dialer context again → TestKubernetesBuilderClientIgnoresDialerContext failed, and nothing else did. The test drives the dialer with an already-canceled context and asserts the exec request still reaches the API server.

• Mutation: the cleanup dropped from the failed-create path → TestKubernetesBuilderBootstrapRemovesPodAfterFailedCreate failed. The fake records the pod before writing the error status, which is the case where the API server persisted it and the client saw only a failure.

• Mutation: the sign check dropped from setResourceQuantity → TestParseKubernetesDriverOptsRejections failed. resource.ParseQuantity accepts signed quantities, verified by running it: ParseQuantity("-1") returns no error and Sign() == -1.

• deadline=500ms was accepted and produced activeDeadlineSeconds: 0, and 1500ms truncated to 1, both verified by running int64(d.Seconds()). Whole seconds of at least 1s are now required.

• Mutation: the cleanup reused the build context — context.WithTimeout(ctx, …) instead of context.WithTimeout(context.WithoutCancel(ctx), …) → TestKubernetesBuilderBootstrapRemovesPodOnCancelledContext failed alone. On a canceled release the delete never leaves, and a test asserting only that remove was called would stay green.

• Mutation: bootstrap's remove call deleted → the three removal tests failed.

• Mutation: isPodReady returning true regardless of the Ready condition → survived at first, and the reason was the suite's. This is test-the-tests' "the assertion holds under the bug too": the fake only ever served Pending for an unready pod, so no input made the correct and the broken implementation disagree. TestKubernetesBuilderBootstrapWaitsForReadinessNotJustRunning now serves a Running pod carrying no Ready condition, and kills it.

• Hermeticity: the pod tests drive an httptest API server and run with an empty PATH, so a mutation that removes a guard reaches the fake rather than a cluster, and cannot find a real docker or kubectl.

• Dependency figures were measured as unique modules in server/go.sum after go mod tidy and a successful go build ./... — 240 → 278. Measuring before tidy gives 3 and is wrong; it counts only the three named modules and none of the transitive tail. Binary sizes are task server:build output, same toolchain both sides.

• Two vulnerability scans, both as a delta against origin/main rather than as absolutes, because the base carries findings of its own. govulncheck -show verbose ./... in server/: the same 15 advisory ids on both sides. osv-scanner --lockfile=go.mod, which reports the whole module graph with no reachability filter and so is the closer analogue of what Snyk does: 28 findings on both sides, delta empty. That empty delta is only worth something with a positive control, so one was run — downgrading github.com/gogo/protobuf to v1.3.1 in a copy of go.mod moves the count to 30 and surfaces GHSA-c3h9-896r-86jm, which shows the scanner really does read the indirect requirements the new dependencies live in.

• No go/toolchain directive changed in any of the four modules, so nothing here asks Snyk for a newer Go than main does.

• govulncheck as a delta, not an absolute: -show verbose ./... in server/ on this branch and on origin/main return the same 15 advisory ids. Licenses of the 38 new modules are Apache-2.0, MIT and BSD only. After the REST-client trim, kube-openapi, gnostic-models, go-openapi/* and go-restful are in go.sum but not linked; of k8s.io/api exactly one package (core/v1) is compiled in.

• The buildx comparison numbers come from building this plugin against docker/buildx v0.31.0 and v0.36.1 in a scratch copy: 88 and 183 new modules, 97.6 MB and 77.3 MB. The rest.Config blocker was established by running the factory, not only by reading it: with any endpoint other than kubernetes:// and a nil ContextStore it panics inside ConfigFromEndpoint.

• Not run: the deadline option against a cluster. Nothing here shows the kubelet enforcing activeDeadlineSeconds; only that the value reaches the pod spec.

• The forwarding of buildkitd_driver is guarded in the required checks, cluster-free, as the mirror image of e2e_buildx_config: the configuration names the kubernetes driver, which cannot come up without a cluster, while the environment names a buildx driver that works, so the release has to fail with the driver's own error. Deleting the two lines in pathRelease makes it fall back to the docker CLI and succeed, which the job refuses. Mutation: moving the driver validation below putConfiguration → the five TestUpdate_RejectedBuildkitdDriverKeepsConfiguration subtests failed; the create-path tests alone did not cover that the stored configuration survives a rejected update.

• UNVERIFIED: that the guard specs skip themselves in the default e2e jobs. They carry a Skip on TRDL_TEST_BUILDKITD_DRIVER, and the label filter selecting them was checked with ginkgo --dry-run (1 of 2 specs), but a dry run does not execute BeforeEach. The first run of this workflow settles it in both directions: the default jobs go red if the skip misfires, and the guard job's own step fails if the skip empties it.

Review focus

• security/snyk and license/snyk fail and I cannot see why. The results are private to the werf Snyk org. The control is PR chore: release 0.14.0 #415, which reports "No manifest changes detected in 3 projects" and passes — so these checks really did scan here, because this PR changes server/go.mod and e2e/go.mod. What I could establish independently is above: no new advisory in either the Go vulnerability database or OSV, no non-permissive license among the 38 new modules, and no toolchain change. Both checks report the same count, 2 tests have failed, which reads more like two projects Snyk could not process than like findings — this PR changes exactly two go.mod files, server/ and e2e/. I cannot get further from outside: Snyk posts these as commit statuses with no output payload, so GitHub carries no detail. Please paste what it says and I will pin around it or drop the dependency.
• execConn in server/pkg/docker/kubernetes_exec.go. CloseWrite/CloseRead are load-bearing rather than boilerplate: the BuildKit client half-closes when it finishes sending, and a plain io.Pipe wrapper without them waits for an EOF that never arrives. Both ends are also closed with the stream's error so a caller blocked on the connection fails with it instead of hanging. Worth reading against driver/kubernetes/execconn in buildx, which it follows deliberately.
• The cross-field validation in path_configure.go moved from an inline if/else chain to isConfigurationFieldSet + firstSetConfigurationField. That is a refactor of existing code, done because the chain now spans four fields across two mutually exclusive rules rather than two fields across one. The existing rule's behaviour is unchanged and its tests were extended rather than rewritten.

Follow-up

• Decide whether trdl should drive buildx as a library instead, which is the question the Why section puts rather than answers. If yes, this driver is replaced by buildx's own and buildkitd_driver becomes an alias to retire; the option names and values were kept identical to buildx's where they overlap so that migration stays a rename.
• Guard the pathRelease forwarding of buildx_driver_opts and buildkitd_address the way e2e_buildx_config guards the driver, from feat(server): configure buildx driver via plugin configuration #419's own follow-up list. buildkitd_driver now has such a guard in the required checks (e2e_buildkitd_driver_forwarding); those two are still uncovered, and deleting their assignment leaves every suite green.
• Unrelated to this diff: AGENTS.md points at .agents/skills/rigorous-review/SKILL.md, which does not exist — the file is .agents/skills/review/SKILL.md.

[alexey-igrychev]

Code Review Report

Base: main @ 443fdd352d8a43c35ce0c1771fbe5610de6cbbdf
Diff: 21 files, +2115/-76

Verdict

Request changes. Kubernetes driver does not support the documented in-cluster ServiceAccount path; the optional cluster workflow runs incompatible specs; and the required forwarding guard does not prove forwarding of buildkitd_driver_opts.

Issues

1. Major — documented in-cluster ServiceAccount resolution is absent.
   server/pkg/docker/kubernetes.go:87-104 only loads a kubeconfig through clientcmd; it never uses rest.InClusterConfig(). A Vault plugin running inside a Pod with only its default ServiceAccount credentials fails with unable to configure the kubernetes client, and cannot default to the ServiceAccount namespace. This contradicts docs/pages_en/QUICKSTART.md:107,119 and the Russian counterpart.
   Fix: prefer in-cluster configuration when available, fall back to kubeconfig, derive the namespace from the ServiceAccount namespace file, and add coverage for both sources.

2. Major — the Kubernetes dispatch workflow cannot complete its intended suite.
   .github/workflows/tests_buildx_kubernetes_driver.yaml:95-96 enables TRDL_TEST_BUILDKITD_DRIVER=kubernetes, which registers the negative forwarding spec in e2e/tests/flow_vault/buildkitd_driver_forwarding_test.go:26-32. The workflow then runs the whole suite (tests_buildx_kubernetes_driver.yaml:144-145). After the first of two specs, AfterEach removes the plugin binary (e2e/tests/flow_vault/suite_test.go:54-59; server/Taskfile.yaml:182-192) built only once in BeforeSuite, so the second spec cannot enable the plugin.
   Fix: run only the complete-cycle spec, e.g. task --yes e2e:test:e2e paths='./tests/flow_vault' labelFilter='flow'; alternatively register the forwarding guard behind a dedicated environment variable used only by its required job.

3. Major — required forwarding CI does not test buildkitd_driver_opts.
   It passes only namespace=default (.github/workflows/tests.yml:155-158), equal to the normal fallback; no-kubeconfig execution also fails before options are parsed (server/pkg/docker/kubernetes.go:87-101). I removed only BuildkitdDriverOpts: cfg.BuildkitdDriverOpts from server/path_release.go:177: the required forwarding test still passed.
   Fix: pass a non-default namespace via a minimal kubeconfig pointed at an unreachable API server, and require that namespace in the builder pod <namespace>/... error. This makes the options observable and kills the mutation.

4. Major — ambiguous pod creation can leak a privileged pod without an actionable error.
   In server/pkg/docker/kubernetes.go:128-135, the create-error path discards removeAfterFailure's error. If POST persisted the pod but its response was lost, and DELETE also fails, users see only the POST error. The readiness-error path correctly reports a left-behind pod at :140-143.
   Fix: append/log the cleanup error in the create-error branch, matching the readiness branch, and add POST persist-then-fail + DELETE-fail coverage.

5. Major — ServiceAccount security guidance gives a false isolation guarantee.
   docs/pages_en/QUICKSTART.md:129 says one ServiceAccount without its own RoleBinding prevents access to another principal's permissions. ClusterRoleBindings, including bindings through system:serviceaccounts groups, still grant privileges. The generated pod uses the selected ServiceAccount and does not disable token automount (server/pkg/docker/kubernetes.go:438-447).
   Fix: require a dedicated ServiceAccount whose direct, group, namespaced, and cluster-wide bindings have been audited; do not state that omitting a RoleBinding is sufficient.

6. Minor — generated configure reference contradicts the new backend.
   docs/_includes/reference/vault_plugin/configure.md:12 says Docker CLI is used whenever buildkitd_address is unset, ignoring buildkitd_driver.
   Fix: say Docker CLI is used only when neither buildkitd_address nor buildkitd_driver is set.

7. Minor — E2E option transport splits valid comma-valued options.
   e2e/tests/flow_vault/utils.go:241-245 splits TRDL_TEST_BUILDKITD_DRIVER_OPTS by comma, although documented options include nodeselector=disktype=ssd,zone=a.
   Fix: use indexed variables or an unambiguous separator, preserving option values verbatim.

Verification

• task --yes --taskfile Taskfile.dist.yaml server:test:unit paths=. — passed.
• task --yes --taskfile Taskfile.dist.yaml server:test:unit paths=./pkg/docker — passed.
• Linux task --yes --taskfile Taskfile.dist.yaml server:build — passed.
• Real kind-cluster E2E passed: three Kubernetes-driver releases, BuildKit secrets, and no remaining trdl-builder-* pods.
• Mutation checks killed configure validation, cleanup, readiness, dialer-context, negative quantity, rootless, defaults, ServiceAccount, driver/options validation, and configured driver forwarding.
• Disabling Builder.Remove left three builder pods, which the workflow leftover-pod assertion detected.
• Failed mutation: deleting only BuildkitdDriverOpts forwarding left the required forwarding guard green.

Not verified

• Real kubelet enforcement of deadline / activeDeadlineSeconds remains unverified.
• GitHub link checks are red because of a pre-existing GnuPG URL already present in main, not this PR.
• External Snyk status reports two failed checks, but its diagnostic detail was not available through GitHub API.

Recommendation

Do not merge before resolving the Major issues above.

[alexey-igrychev]

Review methodology

This report covers PR head d3e6afc8ac6aa0eb2e534ff78fbbce4b05ed9de2 against base main 443fdd352d8a43c35ce0c1771fbe5610de6cbbdf: 21 changed files, +2115/-76.

1. Retrieved PR metadata, description, commits, changed-file list, and check results through GitHub API. The base was resolved from the PR's actual baseRefName, not assumed.
2. Fetched refs/pull/422/head, read the complete diff and every changed file. The review was split into three independent inspection passes: configure/release forwarding; Kubernetes BuildKit driver; E2E/CI/docs. Each reported finding was then rechecked against source and execution evidence before inclusion.
3. Traced the configuration path end to end: configure → stored configuration → pathRelease → BuildReleaseArtifacts → NewBuilder → Kubernetes REST/exec. Persistent configuration, BuildKit session/secrets, pod lifecycle, and workflow selection were examined separately.
4. Ran git diff --check. Every mutation used a source-file copy; source was restored and the relevant suite was rerun before the next mutation.

Models and skills

Models

• Primary synthesis: active Pi coding-agent session. Its exact underlying model identifier was not exposed through the session environment.
• Independent configuration/forwarding pass: OpenAI GPT-5.6 Sol High.
• Independent Kubernetes implementation pass: Anthropic Claude Fable 5 Medium.
• Independent E2E/CI/docs pass: OpenAI GPT-5.6 Sol High.

The independent passes were used for adversarial inspection, not as proof by assertion. Findings were accepted only after direct source inspection and, where applicable, runtime or mutation evidence.

Skills and project rules

• review: PR contract recovery, changed-file inspection, technical/product/risk reporting.
• agent-code-review: adversarial verification and check-gaming review for changed tests and CI.
• test-the-tests: real implementation mutations, failure confirmation, source restoration, and green reruns.
• remote-linux: Linux/CGO build and Docker/kind E2E procedure.
• pull-request: review publication and PR-comment conventions.
• Project AGENTS.md: task-only build/test commands, LSP-first navigation, test rules, and security constraints.
• Active session modes: Ponytail (full) for minimum necessary changes and Caveman (full) for concise communication. Neither changed review severity or verification criteria.

Verification

• task --yes --taskfile Taskfile.dist.yaml server:test:unit paths=. — passed.
• task --yes --taskfile Taskfile.dist.yaml server:test:unit paths=./pkg/docker — passed.
• On an isolated Linux x86_64 host: task --yes --taskfile Taskfile.dist.yaml server:build — passed.
• On a real kind cluster: task --yes --taskfile Taskfile.dist.yaml e2e:test:e2e paths='./tests/flow_vault' labelFilter='flow' with TRDL_TEST_BUILDKITD_DRIVER=kubernetes and intentionally invalid TRDL_BUILDX_DRIVER — passed. It performed three pod-backed BuildKit releases, including BuildKit secrets; no trdl-builder-* pods remained after the suite.
• The required forwarding guard passed separately under its CI environment with the buildkitd-driver-forwarding label.

Mutation evidence

Every mutation was applied to an implementation copy, followed by immediate restoration and a passing rerun of the relevant suite.

Mutation	Expected evidence	Result
Remove ValidateBuildkitdDriver on the configure path	Unsupported-driver and atomic-update tests fail	Killed
Disable removeAfterFailure	never-ready, cancellation, failed-create, and terminated-pod cleanup tests fail	Killed
Treat PodRunning as ready	readiness test fails	Killed
Pass gRPC dialer context to exec stream	dialer-context test never observes /exec	Killed
Allow negative resource quantities	rejection table fails	Killed
Disable rootless pod adaptation or custom rootless image handling	rootless pod tests fail	Killed
Change default timeout, restart policy, or ServiceAccount	corresponding pod/options tests fail	Killed
Remove IsNotFound handling	idempotent-remove test fails	Killed
Accept an unknown driver or options without a driver	validation tests fail	Killed
Remove forwarding of both buildkitd fields from path_release.go	required forwarding guard fails	Killed
Remove only BuildkitdDriverOpts: cfg.BuildkitdDriverOpts	guard should fail	Survived: the guard remained green; this is Issue 3 in the review
Disable Builder.Remove on the Kubernetes path	workflow leftover-pod shell check finds pods	Killed: successful E2E left three trdl-builder-* pods, which the check detected; the pods were then removed

CI and review limitations

• Current GitHub unit/E2E/lint checks are green. EN/RU link checks fail on an old external GnuPG URL already present in base main; this is not a PR regression.
• External Snyk status reports two failed checks, but GitHub API did not expose their diagnostics, so it was not turned into an unverified defect.
• Real kubelet enforcement of deadline / activeDeadlineSeconds was not run. The author explicitly marks it unverified; a real short-deadline workflow is needed.
• The official Kubernetes workflow_dispatch job was not run end to end because static analysis found the lifecycle defect in Issue 2: it starts two incompatible specs without a label filter. This conclusion is based on environment registration, Ginkgo suite lifecycle, and the cleanup task, not workflow naming.

Review focus

• server/pkg/docker/kubernetes.go:87-104: documentation promises in-cluster ServiceAccount resolution, but implementation invokes only the kubeconfig loader and never rest.InClusterConfig().
• server/pkg/docker/kubernetes.go:128-145: create-error branch suppresses cleanup errors while readiness-error branch reports them.
• .github/workflows/tests.yml:155-210: required guard proves driver forwarding but not the options list.
• .github/workflows/tests_buildx_kubernetes_driver.yaml:85-165: dispatcher must select only the complete-cycle scenario.

[vmrm]

Thanks — six of the seven are fixed in 5054e6a, and finding 3 was the one worth the whole review: the guard I wrote to catch a lost forwarding did not catch losing the options, and your mutation proved it while my reasoning did not.

• 3. The job now writes a kubeconfig pointing at https://127.0.0.1:1 with namespace: kubeconfig-namespace, and passes namespace=trdl-forwarding-guard through the options. The driver therefore gets past client configuration and fails at pod creation, and the spec requires builder pod trdl-forwarding-guard/ in the output. Dropping the options anywhere leaves the kubeconfig's namespace in that message. A unit test now pins the message shape the guard greps for; mutation: removing the namespace from the create error fails TestKubernetesBuilderBootstrapRemovesPodAfterFailedCreate alone. UNVERIFIED here: the guard's discrimination end to end — the container tooling this workstation reaches was down, so CI on this push is the first real run.
• 2. The dispatch workflow runs labelFilter='flow', so the buildkitd job executes only the complete-cycle spec.
• 4. The create path reports the cleanup error the way the readiness path does, covered by a failCreate + failDelete test.
• 7. Options come one per TRDL_TEST_BUILDKITD_DRIVER_OPTS_<SUFFIX>, mirroring TRDL_BUILDX_DRIVER_OPTS_*, so comma-valued options survive.
• 5. Both locales now say a workload creator can run as any ServiceAccount of the namespace, that a ServiceAccount carries direct, namespaced, cluster-wide and group bindings including system:serviceaccounts, that the absence of a RoleBinding is not an isolation guarantee, and that the pod mounts its token.
• 6. Fixed at the field description and regenerated.

On 1, I think the finding is incorrect, and I would rather say so than quietly rewrite working code. In-cluster ServiceAccount resolution is present — through clientcmd, not through a direct rest.InClusterConfig() call:

• NewNonInteractiveDeferredLoadingClientConfig returns &DeferredLoadingClientConfig{loader: loader, overrides: overrides, icc: &inClusterClientConfig{overrides: overrides}} (tools/clientcmd/merged_client_builder.go).
• Its ClientConfig() ends with if config.icc.Possible() { klog.V(4).Infof("Using in-cluster configuration"); return config.icc.ClientConfig() }, taken whenever the kubeconfig is absent or is the default one.
• inClusterClientConfig.ClientConfig() calls restclient.InClusterConfig as its provider, and Possible() is exactly KUBERNETES_SERVICE_HOST/_PORT plus a readable /var/run/secrets/kubernetes.io/serviceaccount/token.
• Namespace() reads POD_NAMESPACE, then /var/run/secrets/kubernetes.io/serviceaccount/namespace, then falls back to default — which is where the documented ServiceAccount-namespace default comes from.

So a plugin in a Pod with only its ServiceAccount does get a working client and the ServiceAccount namespace. What your finding does identify correctly is precedence: clientcmd prefers a non-default kubeconfig over in-cluster, so a Pod that also has a kubeconfig mounted uses the kubeconfig. If you want in-cluster to win in that case, that is a deliberate behaviour change and I will make it — say the word and I will add it with a test per source. I did not change it on my own because it would silently override an operator's explicit kubeconfig.

Two things from your report I am carrying rather than closing: the deadline enforcement is still unverified against a kubelet, and TestPublisher in pkg/publisher fails on origin/main too — it is time.Now()-based date arithmetic, unrelated to this PR, but it will keep failing local full runs.

[vmrm]

The guard from finding 3 is now verified in both directions, so the UNVERIFIED I left on it a few comments ago is withdrawn.

Positive control — PR head 5054e6a, job End-to-end tests (buildkitd driver forwarding): passed (3m23s).

Negative control — a throwaway branch carrying exactly the mutation that survived your review, BuildkitdDriverOpts: cfg.BuildkitdDriverOpts deleted from server/path_release.go and nothing else: the job failed, and on the right assertion rather than incidentally:

to contain substring
    <string>: builder pod trdl-forwarding-guard/

The release under mutation reported unable to create builder pod kubeconfig-namespace/trdl-builder-<uuid> — the namespace from the kubeconfig instead of the one the options carried, which is precisely what the reworked job makes observable. The branch has been deleted.

That run also exercised finding 4 in the wild, which I had only unit-tested: the message now reads ... connection refused (a pod may have been created and was not removed: unable to delete builder pod ...).

Two notes on the environment while getting there, in case they save you time:

• The Tests workflow cannot run on a fork branch at all: notification uses werf/common-ci/.github/workflows/notification.yml@main and passes secrets.LOOP_*, which a fork does not have, and that fails the whole workflow at startup before any job runs. common-ci itself is public; it is the secrets mapping. I dropped that job on the throwaway branch to get the control.
• TestPublisher in pkg/publisher fails on origin/main too, in a clean worktree — tuf_repo_rotator_test.go is built on time.Now() and month arithmetic, so it breaks on end-of-month dates. Your report says the current unit checks are green; as of today they are not, and the red Tests on this PR is that, not this diff.

[alexey-igrychev]

Review: changes requested

1. Major — rootless builders are not compatible with Kubernetes 1.36+. server/pkg/docker/kubernetes.go:483-498 sets only the deprecated container.apparmor.security.beta.kubernetes.io/...=unconfined annotation. Kubernetes 1.36 removed annotation-to-field conversion; kind v0.32.0 used by this PR defaults to Kubernetes 1.36.1. Rootless builders therefore do not receive the required AppArmor Unconfined profile. Set container.SecurityContext.AppArmorProfile to corev1.AppArmorProfileTypeUnconfined, retain the annotation only for older-cluster compatibility, and add a rootless in-cluster case.

2. Major — the manual in-cluster job does not pass its declared driver option. .github/workflows/tests_buildx_kubernetes_driver.yaml:95-96 sets TRDL_TEST_BUILDKITD_DRIVER_OPTS, while e2e/tests/flow_vault/utils.go:247-255 accepts only TRDL_TEST_BUILDKITD_DRIVER_OPTS_<SUFFIX>. The option is ignored and the run succeeds using its default fallback, so this job cannot prove live option handling. Use a suffixed variable (for example, TRDL_TEST_BUILDKITD_DRIVER_OPTS_NAMESPACE) and make the option's effect observable; alternatively remove the ineffective variable and leave forwarding solely to the dedicated guard job.

3. Major — kind is downloaded and installed as root without integrity verification. .github/workflows/tests_buildx_kubernetes_driver.yaml:139-140 downloads a mutable release asset then calls sudo install. Pin and verify its SHA-256 before installing (or use a commit-pinned setup action that does so); restrict workflow permissions and disable checkout credential persistence if no push is required.

Minor documentation gaps:

• docs/pages_{en,ru}/QUICKSTART.md:76-81 says the new backends cannot be combined with buildx settings, but configured buildkitd_* intentionally overrides TRDL_BUILDX_* environment fallbacks.
• docs/_includes/reference/vault_plugin/configure.md:12-15 should state that buildkitd_driver_opts requires a driver and that process-level TRDL_BUILDKITD_ADDRESS takes precedence over a stored driver.

[alexey-igrychev]

Review verification

• Reviewed origin/main...PR (merge-base 443fdd3; current origin/main f0eb3b8): 21 files, +2196/-78; git diff --check is clean.
• Read all changed files and traced configuration → release → builder plumbing plus Kubernetes pod/exec lifecycle.
• task server:build succeeded on Linux (buildah-commons).
• Relevant PR-head tests passed through project task targets:
  • task server:test:unit paths="./pkg/docker/..."
  • task server:test:unit paths="./" for the server configure package.
• Mutation checks were falsifiable:
  • removing create-failure cleanup failed TestKubernetesBuilderBootstrapRemovesPodAfterFailedCreate and TestKubernetesBuilderBootstrapReportsAPodItCouldNotRemove;
  • removing ValidateBuildkitdDriverOpts before persistence failed the invalid-option and rejected-update configure tests.
• Full task server:test:unit was not green, but failures were reproduced on origin/main: 39 pkg/git failures because git signatures is absent, plus time-sensitive pkg/publisher TUF rotator failure.

Not run: a real Kubernetes release/exec/cleanup cycle, rootless Kubernetes 1.36+ flow, and lint. The in-cluster workflow is manual-only; the rootless failure and ineffective option wiring above prevent treating it as adequate evidence.

[alexey-igrychev]

Review: changes requested

Independent second pass over head 5054e6a (21 files, +2196/-78 vs merge-base 443fdd3). Findings are left as line comments: two Majors (an unpinned, never-refreshed builder image; a ServiceAccount token mounted into the builder), three Minors (readiness-wait fail-fast, dial returning before the exec stream exists, and the pod cleanup having no required-CI coverage), and one note on a silent environment override.

This is a fresh pass, not an adjudication: the earlier findings were re-checked only for whether they still hold at this head. They do — nothing has been pushed since — and they are not renumbered here. Still open: the rootless AppArmor annotation without the securityContext.appArmorProfile field (server/pkg/docker/kubernetes.go:498); the dead TRDL_TEST_BUILDKITD_DRIVER_OPTS in the manual job (.github/workflows/tests_buildx_kubernetes_driver.yaml:96 versus the _<SUFFIX> form e2e/tests/flow_vault/utils.go:247-255 reads); the unverified kind download (:139-140, which copies the pre-existing step at :65-66); and the two documentation items.

One more note that did not warrant a line comment: empty image=, empty map keys, and label/annotation names that are not valid Kubernetes keys all pass parseKubernetesDriverOpts and fail at pod creation instead. Duplicate scalar options silently take the last value; duplicate map options replace rather than merge.

Architecture

Keep the hand-rolled client-go driver. buildx's kubernetes factory cannot be handed a rest.Config — cc ClientConfig is unexported and driver.Register registers a zero value — so importing it means an undocumented endpoint string or upstream work, for 88+ modules against 38, to make four REST calls against one bare Pod. Confirmed independently by a second review pass on a different model family. The three new direct dependencies (k8s.io/api, k8s.io/apimachinery, k8s.io/client-go) are accepted on that basis.

Review methodology

1. Resolved the PR through the GitHub API — metadata, description, the existing review thread and the changed-file list. The base was taken as the actual merge-base with main (443fdd3), not assumed. This is a fork PR, so the head was fetched as refs/pull/422/head and checked out locally at 5054e6a before anything was read.
2. Read the complete diff and every changed file — Go, workflows, e2e, documentation — and traced the configuration path end to end: configure → stored configuration → pathRelease → BuildReleaseArtifacts → NewBuilder → pod create / pods/exec / delete. Pod lifecycle, the exec transport, driver-option parsing and backend selection were examined separately.
3. Two parallel exploration passes on a small model were commissioned for the CI/e2e wiring and for the documentation-versus-code check. Both were discarded — one returned no output, the other reported literal matches as mismatches — and both areas were then re-done by hand. Nothing from those passes reached this report.
4. Every finding was rechecked against the source before inclusion, and the two external claims the report leans on (the AppArmor annotation transition and the kind default node image) were verified against upstream sources rather than the PR description.
5. Mutations were named rather than executed: this pass was read-only and modified no source. Where a mutation is the evidence, the predicted outcome and the reasoning behind it are stated so it can be run and falsified.

Models and skills

• Primary synthesis: Anthropic Claude Opus, this session.
• Independent adversarial pass: OpenAI GPT-5.6 Sol Pro (high). Used to challenge severity and to hunt for what the primary pass missed, not as proof by assertion — its findings were accepted only after direct source inspection.
• Discarded exploration passes: Anthropic Claude Haiku 4.5.
• Skills applied: agent-code-review (the diff's author is an agent and it touches tests and CI) and test-the-tests (falsifiability of the new suites). Project AGENTS.md for task-only build/test commands, LSP-first navigation and the security constraints on dependencies and user-facing changes.
• Active session modes: Ponytail (full) and Caveman (full). Neither changed severity or verification criteria.

Review verification

• Scope: head 5054e6a, merge-base 443fdd3 — 21 files, +2196/-78; git diff --check clean. Read every changed Go, workflow and documentation file, and traced configure → stored configuration → pathRelease → BuildReleaseArtifacts → NewBuilder → pod create / pods/exec / delete.
• task server:test:unit paths="./pkg/docker/..." — Test Suite Passed (Ginkgo, 5.97s).
• task server:lint — 0 issues, Prettier clean.
• External claims checked rather than taken from the diff: KEP-24 phase 3 lands in v1.36 and is exactly "API server stops copying annotations to fields", and kind v0.32.0 — the version tests_buildx_kubernetes_driver.yaml:13 pins — defaults to Kubernetes v1.36.1. The rootless AppArmor gap therefore fails on the very cluster this PR's own job provisions, and that job cannot detect it, because it never sets rootless=true.
• Mutations were named, not executed: this pass modified no source. The load-bearing one is deleting the kubernetesBuilder branch from Builder.Remove; the predicted outcome — from the absence of any test constructing that Builder, and from the guard job failing inside bootstrap — is that every unit suite and every required job stays green. Worth running before that finding is dismissed.
• A second, independently prompted pass on a different model family reviewed the same head. It confirmed the two Majors and two of the Minors, argued the environment-override item down to an observability nit, and contributed the dial establishment race. It found no lifetime bug in the execConn / bkclient.Close() / deferred-delete ordering, and no stale-field hole in configure — each write rebuilds the whole configuration from the request.
• Checked and not findings: in-cluster ServiceAccount resolution is present through clientcmd's deferred-loading in-cluster fallback; cleanup ordering is correct on every path the plugin controls; SetDeadline returning nil mirrors buildx's own exec connection; and the forwarding guard discriminates in both directions as claimed.
• Not run: any real cluster, the rootless path, the e2e suites, and the full server unit suite (pkg/git needs git signatures; pkg/publisher's TUF rotator test is date-dependent and red on main too).

(generated by pi-pi)

[alexey-igrychev]

Major — the builder image is unpinned. The default is a mutable tag, and the rootless default is derived from it.

This is the container that executes project-supplied build instructions and receives every build secret — the project secrets and, when mac signing is configured, the signing certificate, its password and the notary key. Meanwhile trdl refuses to build at all unless the project's own build image carries a digest (server/pkg/config/trdl.go:35 → server/pkg/docker/util.go:13, ErrImageNameWithoutRequiredDigest). The plugin now provisions a privileged, secret-bearing image under a weaker rule than the one it enforces on the project one image over.

Honest counterweight: buildx's own kubernetes driver defaults to the same tag, and the existing docker-CLI path already runs a buildkitd image buildx chose. That explains where the value came from; it does not carry over, because buildx is not the component holding the release keys, and until this PR trdl did not write the pod spec.

Pin both defaults by digest — the rootless default needs its own constant, since -rootless cannot be appended to a digest reference.

(generated by pi-pi)

[alexey-igrychev]

image= is accepted as an arbitrary string with no validation — an empty value included, which then fails at pod creation instead of at configure.

Require a digest here the way dockerImage is required to carry one (server/pkg/config/trdl.go:35). Early rejection of options the driver cannot honor is the stated point of a separate option vocabulary, and an unpinned or empty builder image is exactly that class of value.

(generated by pi-pi)

[alexey-igrychev]

No imagePullPolicy is set, so Kubernetes applies IfNotPresent for a non-latest tag: a node that pulled buildx-stable-1 once keeps that layer indefinitely, and the builder silently ages into a buildkit release nobody chose.

imagePullPolicy: Always buys freshness but not reproducibility, and does not protect against tag replacement — a digest-pinned default does both.

(generated by pi-pi)

[alexey-igrychev]

Major — the builder pod mounts a ServiceAccount token it never uses. AutomountServiceAccountToken is left nil, so the pod receives the namespace's default ServiceAccount token — or the token of whatever serviceaccount= names — inside a privileged container running untrusted build instructions.

buildkitd makes no Kubernetes API calls: every API call this driver makes is made by the plugin through its own restClient. The QUICKSTART already identifies the exposure in prose — "the absence of a RoleBinding in the namespace is not by itself an isolation guarantee, and the builder pod does mount its token" — where one field would remove it.

Set automountServiceAccountToken: false by default, including when a ServiceAccount is named: an SA is still needed for its imagePullSecrets and its PodSecurity identity, and neither requires the token to be projected. If a workload-identity case genuinely needs one, make it an explicit opt-in driver option rather than the default.

(generated by pi-pi)

[alexey-igrychev]

Minor — one API-server blip during the readiness wait kills the release. The first failed GET ends the wait as long as the context is alive, so a 429, a 500 or an etcd timeout at any point in a two-minute readiness wait aborts the release and deletes a builder that may have been seconds from ready.

The buildx kubernetes driver keeps the last error and retries until its timeout. Suggested: retry the retryable classes (429, 5xx, transport timeouts) until the readiness timeout expires, and keep the immediate failure for permanent ones (403, 404) — better than a retry-everything loop, which would hide a permanent 403 for the whole timeout.

(generated by pi-pi)

[alexey-igrychev]

Minor — dial reports success before the exec stream exists. The stream is started in a goroutine and a net.Conn is returned immediately, so a failure to establish the SPDY upgrade — API server to kubelet — never reaches the dialer. It surfaces later as an opaque gRPC read/write error naming no pod and no namespace, and gRPC's dial-level retry has nothing to act on. The same structural race has been reported against buildx's own exec connection.

Suggested: have dial wait for the stream to be established, or for the first error, before returning the connection.

Related gap: nothing tests execConn at all. CloseWrite/CloseRead are exactly what keeps the gRPC transport from waiting on an EOF that never comes, and the only test touching this file asserts that the dialer's context is ignored.

(generated by pi-pi)

[alexey-igrychev]

Minor — required CI cannot catch the loss of the pod cleanup. No test constructs a Builder with a non-nil kubernetesBuilder (the only occurrence in the test tree is assert.Nil(t, builder.kubernetesBuilder, ...) in buildkit_test.go:264), so both routing branches — Build at builder.go:301 and this one — execute only in the workflow_dispatch-only cluster job.

Named mutation: delete this branch. Predicted outcome — every unit suite stays green, and so does the required e2e_buildkitd_driver_forwarding job, because that guard is designed to fail inside bootstrap and never reaches a successful build's deferred cleanup (build.go:120-128). "The pod is deleted when the build ends" is the headline safety property of this change and the one thing no required check exercises.

One Builder-level test against the existing fakeAPIServer — construct the Builder, call Remove, assert the DELETE was issued — closes the hole without making the cluster job required.

(generated by pi-pi)

[alexey-igrychev]

Note, not a change request. With buildkitd_driver configured this branch is taken before buildxCreateArgs, so a process-wide TRDL_BUILDX_DRIVER / TRDL_BUILDX_DRIVER_OPTS_* is ignored without a word, while the mirror case a few lines up logs "the configured … settings are not used".

Configured-over-environment precedence makes the silence defensible; the asymmetry is worth a line of log. (The manual cluster job deliberately relies on the silence with TRDL_BUILDX_DRIVER: this-driver-does-not-exist; a log line would not break it.)

(generated by pi-pi)

[vmrm]

All five are addressed in e016a97. Two notes where my evidence differs from the report, and one where it does not reach as far as I would like.

1 — fix applied, premise not reproduced. The container now carries SecurityContext.AppArmorProfile: Unconfined with the annotation kept for older clusters, and a mutation removing the field fails TestBuildkitPodRootless alone. That change is right on its own terms: the annotation is deprecated since 1.30 and the field is the supported API.

I could not reproduce the stated reason, though, and would rather say so than let it stand as established. kind v0.32.0 does default to kindest/node:v1.36.1 — confirmed from its pkg/apis/config/defaults/image.go. But on release-1.36 the annotation-to-field sync is still there and still runs: pkg/registry/core/pod/strategy.go calls applyAppArmorVersionSkew on pod create, and that function copies container.apparmor.security.beta.kubernetes.io/<ctr> into SecurityContext.AppArmorProfile through podutil.ApparmorFieldForAnnotation, exactly as on release-1.35. CHANGELOG-1.35.md and CHANGELOG-1.36.md do not mention AppArmor at all.

What I did not do: run this against a live 1.36 cluster, or rule out a feature gate that disables the path. So this is "not reproduced from source", not "disproved" — and if you have the removal reference, I would like it, because it also decides whether the annotation is worth keeping at all.

2 — mine, and a regression I introduced. Fixing the comma-splitting made the harness read only TRDL_TEST_BUILDKITD_DRIVER_OPTS_<SUFFIX>, and I updated the guard job but not the dispatch one, which kept passing the unsuffixed name. It is now TRDL_TEST_BUILDKITD_DRIVER_OPTS_NAMESPACE.

3 — done. KIND_SHA256 pinned from the release's own kind-linux-amd64.sha256sum and checked with sha256sum --check --strict before sudo install, in both jobs. The workflow is permissions: contents: read with persist-credentials: false.

Minor docs — both corrected in the two locales and the regenerated partial. The exclusion is now stated as being about the configure fields, with configured buildkitd_* winning over the TRDL_BUILDX_* environment fallbacks because a process-wide variable cannot be refused when a project is configured; and the reference says the options require a driver and that a process-level TRDL_BUILDKITD_ADDRESS wins over a stored driver.

Unchanged from before: rootless has no live-cluster coverage in this PR — the kind job exercises the privileged path — so the AppArmor behaviour above is argued from source on both sides, not from a run.

[vmrm]

Review methodology

Self-review of PR head 997413cf against base main, base resolved from the PR's own baseRefName. Merge-base 443fdd3; origin/main has since moved to f0eb3b8 (#423), and git merge-tree --write-tree reports no conflict, so the two QUICKSTART files both sides touch merge cleanly. 21 changed files, +2215/-78 at the time the passes started.

1. Fetched refs/pull/422/head and read the complete diff and every changed file.
2. Split into three disjoint inspection passes, each given a different model so that no pass had already agreed with another: configure/release forwarding; the Kubernetes driver implementation; E2E/CI/docs. Findings from a pass were treated as leads and entered this report only after direct source inspection and, where possible, execution.
3. Traced both new values end to end — configure → stored configuration → pathRelease:176-177 → build.go:111-112 → builder.go:103 → newKubernetesBuilder — confirming no hop drops them and that neither has an environment fallback.
4. Every mutation was applied to a file copy, restored by absolute path, and confirmed restored by a positive marker before the next one.

Models and skills

• Configure/forwarding pass: OpenAI Codex CLI, via this repository's checkout.
• Kubernetes driver pass and E2E/CI/docs pass: Claude subagents, one each.
• Synthesis, tracing and all mutations: this session.
• Skills: the repository's own review, agent-code-review, test-the-tests and pull-request, plus a local adversarial-pr-review written for this round, whose distinguishing rule is to mutate what the change declares as its protection, not only its implementation.

Two passes converged independently on the same two defects — the blank image= and the silently ignored buildx environment — which is the reason for running them separately.

Verification

• task --yes server:test:unit paths='.' and paths='./pkg/docker' — passed.
• task --yes server:lint — 0 issues; task --yes e2e:lint — passed. Workflow YAML checked with prettier --check directly, because this repository runs prettier through a container and that host was unreachable for part of the session.
• Guard behaviour was established by a pair of CI runs rather than by argument: on unmutated code End-to-end tests (buildkitd driver forwarding) passes; on a throwaway branch with BuildkitdDriverOpts: cfg.BuildkitdDriverOpts deleted from path_release.go and nothing else, it fails on to contain substring: builder pod trdl-forwarding-guard/, the release having reported kubeconfig-namespace instead. That branch has been deleted.
• Not attempted locally: the flow_vault suite. BeforeSuite builds the plugin, which is Linux-only; on darwin/arm64 it dies in gcc_amd64.S under cgo cross-compilation.

Mutation evidence

Mutation	Expected evidence	Result
Remove ValidateBuildkitdDriverOpts from configure	options-without-driver and unsupported-option tests fail	Killed
firstSetConfigurationField always returns ""	cross-field conflict tests fail	Killed
Move driver validation below putConfiguration	five TestUpdate_RejectedBuildkitdDriverKeepsConfiguration subtests fail	Killed
dialerFor hands the stream gRPC's dialer context	TestKubernetesBuilderClientIgnoresDialerContext fails	Killed
Disable removeAfterFailure	never-ready, cancellation, failed-create and terminated-pod tests fail	Killed
isPodReady ignores the Ready condition	readiness test fails	Killed — after the fixture was reshaped; it survived the first attempt
Accept negative resource quantities	rejection table fails	Killed
Remove the AppArmorProfile field	TestBuildkitPodRootless fails	Killed
Drop the namespace from the create error	TestKubernetesBuilderBootstrapRemovesPodAfterFailedCreate fails	Killed
Replace instead of merging repeated map options	TestParseKubernetesDriverOptsMergesRepeatedMapOptions fails	Killed
Blank image= counts as set again	TestParseKubernetesDriverOptsBlankImageKeepsDefault fails	Killed
Rename the guard's ginkgo label	the job's post-check fails on Ran 0 of 2	Killed
Delete BuildkitdDriverOpts forwarding	the required guard job fails	Killed — separate CI run

Two mutations are worth naming rather than counting. isPodReady survived at first because the fake only ever served Pending for an unready pod, so no input made the correct and the broken implementation disagree; the fixture now serves a Running pod without a Ready condition. And the check that proves the guard job was not empty could not tell a guard that ran from one the label filter skipped — at --vv ginkgo prints a skipped spec's name too — so it now greps for output only the guard itself produces.

Findings, all fixed in this branch

Everything below was found by this review and is already fixed; none of it is outstanding.

1. Repeating a map-valued driver option replaced the earlier pairs instead of merging: nodeselector=a=1 then nodeselector=b=2 yielded only b=2, with no error at write or release time. Confirmed by execution before fixing.
2. A blank image= counted as an image being set, suppressing the rootless default and storing a spec the API server refuses only at release time. Confirmed by execution.
3. One failed read of the builder pod ended the entire readiness wait, so a single 500, a reset while an API server replica rolls, or a 429 failed a release the wait exists to ride out.
4. The buildkitd-driver branch ignored TRDL_BUILDX_DRIVER silently, while the address branch reports exactly that.
5. The guard-of-the-guard described above.
6. The comment on execConn justified CloseWrite/CloseRead with a claim about gRPC that is false: gRPC v1.82.1 contains no non-test call to either and closes the connection outright; buildkit's half-closes are in cmd/buildctl/dialstdio.go, inside the pod.
7. The namespace default was documented in the wrong order — an explicit kubeconfig context namespace wins over the ServiceAccount one, per clientcmd's DeferredLoadingClientConfig.Namespace.
8. The guard asserted builder pod <ns>/, which the readiness log line also carries; it now requires the create failure specifically.

Not run, and other limitations

• No required check builds through this driver. The guard fails inside createPod, so waitForPod on success, client/dialerFor, all of kubernetes_exec.go and remove are exercised only by the opt-in kind workflow. The guard proves the wiring, not the driver, and should not be read as covering it.
• Rootless has no live-cluster coverage here; the kind job exercises the privileged path. The AppArmor change is argued from source on both sides.
• deadline reaching the pod spec is covered; the kubelet enforcing it is not.
• execConn's deadlines are no-ops. gRPC does call SetWriteDeadline/SetReadDeadline in http2Client.Close, so this removes that guard; the cost is bounded by gRPC's own fallback timer, and implementing deadlines over io.Pipe incorrectly would be worse than not implementing them.
• A failed Builder.Remove at the end of a build is logged while the release still reports success. This is pre-existing — git show 443fdd3:server/pkg/docker/build.go carries the same defer — but a privileged pod gives it more weight than it had.
• security/snyk and license/snyk remain unreadable from outside: they post as commit statuses with no output payload. Independently: govulncheck and osv-scanner both give an empty delta against the base (15 and 28 findings on each side), the latter with a positive control — downgrading gogo/protobuf to v1.3.1 moves it to 30 and surfaces GHSA-c3h9-896r-86jm — and all 38 new modules are Apache-2.0, MIT or BSD.
• Notification and Upload coverage fail because a fork PR has no access to the repository secrets they need.

[vmrm]

Review round 4 — a full-PR pass, and what it caught

The self-review above covered 997413cf. One more pass was then run over the whole PR with a different model (OpenAI Codex CLI, --full-auto, reading the complete diff rather than a subsystem slice). It returned five findings and all five were real — no false positives. Head is now d7b21fa.

Three were settled by execution before anything was changed, using a throwaway server/pkg/docker probe that parsed the options and built the pod spec, deleted after the run:

deadline=0s                     -> accepted, ActiveDeadlineSeconds=<nil>
image=X then image=             -> image=""
requests.cpu=2 limits.cpu=1     -> accepted: requests=2, limits=1

#	Finding	Fix
1	image=X followed by image= left the flag set by the first element, so defaulting was skipped and configure stored a spec whose empty image the API server refuses only at release time	the flag now follows the current value
2	deadline=0s was accepted and meant nothing, while both QUICKSTART files state a one-second minimum	refused, tracked separately from "not given", as timeout already was
3	A request above its own limit was stored and refused only when the pod was created	compared per resource at write time
4	Neither the exec command nor the success-path Builder.Remove was covered by any required check	two tests added; the fake API server now records the request URI so the exec command is visible to a test at all
5	The documented cleanup promise was stronger than the code: a delete that itself fails leaves a privileged pod behind while the release reports success	documented in both locales, pointing at deadline

Finding 1 deserves naming rather than counting: it is a hole in finding 2 of the round above, fixed earlier the same day. That fix handled a single blank image= and left the "non-blank, then blank" case, which is the same defect one input away — evidence that fixing an instance is not fixing the class.

Finding 5 was not fixed in code on purpose. Making a failed cleanup fail the release changes behaviour shared by all three build backends and predates this branch (git show 443fdd3:server/pkg/docker/build.go carries the same defer), so the branch documents the exposure instead of quietly widening its scope.

Mutation evidence, added to the table above

Mutation	Expected evidence	Result
Exec command becomes anything other than buildctl dial-stdio	TestKubernetesBuilderDialsBuildctlDialStdio fails	Killed
The Kubernetes branch of Builder.Remove returns nil without deleting	TestBuilderRemoveDeletesTheProvisionedPod fails	Killed

Both are mutations the previous round listed under Not run: they were reachable only through the opt-in kind workflow. They are now killed by the required unit suite.

CI on d7b21fa

All code checks pass: Tests, lint, Client unit tests, Links checker (en)/(ru), DCO, and all four end-to-end jobs — End-to-end tests, (BuildKit client), (buildx driver from configure), (buildkitd driver forwarding).

The forwarding guard's post-check is what makes its green meaningful: the log carries Ran 1 of 2 Specs, and the release failed with

unable to create builder pod trdl-forwarding-guard/trdl-builder-b6b4e206-...:
Post "https://127.0.0.1:1/api/v1/namespaces/trdl-forwarding-guard/pods": connection refused

— the namespace the options carried, not the kubeconfig-namespace the kubeconfig names.

Still red, none of it from this branch and none of it fixable from a fork:

• Notification / notify on the tests workflow — Input required and not supplied: MATTERMOST_WEBHOOK_URL.
• Upload coverage — Either 'oidc' or 'token' must be provided.
• security/snyk and license/snyk — still unreadable from outside; the independent measurements in the comment above are unchanged.

Caveat on this round

Codex ran in a sandbox where the Go toolchain could not build the module, so its own findings were static reading. Every one of them was reproduced here by execution or by a named failing test before the code was touched; the three lines quoted at the top are that reproduction.

[alexey-igrychev]

Review: final pass (kubernetes buildkitd driver)

This report covers PR head d7b21fa5ce5b1ceaf2c4874c4d0f11d9dc8ad803 against base main 443fdd352d8a43c35ce0c1771fbe5610de6cbbdf: 21 changed files, +2342/-78.
This is the final pass of the review series: it re-adjudicates the earlier (unpublished) pass 1 and adds what that pass never audited in full: path_configure.go, the e2e harness, the buildkit.go extraction, the config plumbing, dependency hygiene.
Amended after two independent adversarial reviews (Anthropic Claude Fable, OpenAI GPT-5.6): one MAJOR (orphaned pod) and two MINORs (untrimmed values, probe defaults) added with fresh source verification, three checklist verdicts added to non-findings, five citations corrected, two proposed findings rejected with reasoning.
Verification: reused from this head — task server:test:unit paths="./pkg/docker/..." Test Suite Passed, task server:lint 0 issues. This pass ran no build/test command (the unit task writes tests_coverage/unit, which review-only scope forbids); all other evidence is read-only source inspection at d7b21fa and in $(go env GOMODCACHE).
Verdict: 0 BLOCKER, 3 MAJOR, 10 MINOR, 3 NIT — merge after the three MAJORs, with maintainer sign-off on dependencies and on the privileged-pod capability surface. The earlier pass's top MAJOR (exec-session leak) is retracted and downgraded on library evidence — see finding 4.

MAJOR

1. The builder pod mounts a ServiceAccount token by default

server/pkg/docker/kubernetes.go:480

holds. buildkitPod's corev1.PodSpec sets RestartPolicy, ServiceAccountName, NodeSelector and containers, and grep -rn "Automount" server/ returns nothing — so Kubernetes injects the default (or chosen) ServiceAccount's token into a container that runs untrusted project build instructions with Privileged: lo.ToPtr(true) (kubernetes.go:489). buildkitd needs no API access; only an operator-chosen serviceaccount (IRSA/workload identity) does.

if opts.serviceAccountName == "" {
    pod.Spec.AutomountServiceAccountToken = lo.ToPtr(false)
}

Same default as buildx's kubernetes driver, so this is hardening rather than a regression — but the docker-less deployment this PR unlocks was previously out of reach.

2. kubernetes names two different features

server/path_configure.go:136

holds. The buildkitd_driver field at path_configure.go:136 and the buildx_driver field at path_configure.go:126 both accept kubernetes in their descriptions, for different backends (plugin-provisioned pod vs docker-CLI-provisioned builder), with different option vocabularies and failure modes. It lands in stored configuration, so it is irreversible after release. buildkitd_driver=kubernetes-pod, or naming the field buildkitd_provisioner, costs nothing now. Maintainers' call — the buildkitd_* field family itself is right, and the PR body's rejection of a buildx_driver_mode field holds.

3. Nothing removes the builder pod if the plugin process dies mid-build

server/pkg/docker/kubernetes.go:508

New — raised by a round-1 reviewer, verified at head. Every deletion path is in-process: bootstrap's removeAfterFailure and the deferred builder.Remove in BuildReleaseArtifacts (build.go:119-127). The pod carries no ownerReferences (grep -n OwnerReferences server/pkg/docker/ is empty), and pod.Spec.ActiveDeadlineSeconds is set only inside if opts.deadline > 0 (kubernetes.go:507-508) — default: unset. So a Vault restart, a plugin crash or a SIGKILL during a build leaves a privileged, token-carrying (finding 1) pod running buildkitd indefinitely; RestartPolicy: Never does not help because the container never exits. The docker-CLI path leaks a docker-container builder the same way, so the class is pre-existing — the blast radius (shared cluster, privileged, credentialed) and the one-line fix are not.

Fix: always set ActiveDeadlineSeconds and let deadline= override it. The build is already bounded in-process by the task manager (30m default), so a default of that order kills nothing legitimate; the exact value is the maintainers' choice.

MINOR

4. RETRACTED from the earlier pass (was MAJOR): a gRPC redial does not leak an exec session

server/pkg/docker/kubernetes_exec.go:44

The earlier pass claimed the previous buildctl dial-stdio survives a redial because execConn.Close cannot end a stream scoped to the build context. That claim is wrong and is withdrawn. Read first-hand at this head:

• google.golang.org/grpc@v1.82.1/internal/transport/http2_client.go:1009 http2Client.Close calls t.cancel() then t.conn.Close() — the execConn is closed on transport teardown, before any redial.
• k8s.io/client-go@v0.34.1/tools/remotecommand/v2.go:90 copyStdin closes remoteStdin via once.Do when the local stdin copy ends (:101, :124), with an in-source comment warning that otherwise "the executed command will remain running". execConn.Close closes the stdin pipe, so that copy ends and the in-pod process gets EOF and exits.

What remains is defensive hardening, not a leak: nothing in the plugin asserts that ordering. Giving each dial a context.WithCancel child of the build context and cancelling it from Close (~5 lines) makes the teardown the plugin's own guarantee instead of two libraries' emergent behaviour.

5. "A pod may have been created" is reported when create was refused outright

server/pkg/docker/kubernetes.go:128

holds. The create-failure branch calls removeAfterFailure unconditionally, then reports "unable to create builder pod %s/%s: %w (a pod may have been created and was not removed: %w)". For a 403/401/400 or a refused connection nothing was persisted, so the second clause is a false alarm — and the new required CI guard job (kubeconfig pointing at a closed port) makes that the normal output there. Guard with apierrors.IsForbidden/IsUnauthorized/IsInvalid; keep the delete for genuinely ambiguous responses (lost response, 5xx, timeout), which is the case the comment at :129-133 actually describes.

6. A forbidden get pods burns the whole readiness timeout

server/pkg/docker/kubernetes.go:185

holds. Every read error becomes lastState = "unreadable (%v)" and the loop continues to the deadline. Correct for a 429 or a rolling API server, wrong for 401/403: a missing RBAC verb waits the full timeout (2m default) and then reports "is not ready" with the real cause buried in last state. Classify with apierrors.Is* and return terminal responses immediately, keeping the poll for transport/throttling failures.

7. newKubernetesBuilder itself has no unit coverage

server/pkg/docker/kubernetes.go:82

New in this pass. kubernetes_test.go:156 newTestBuilder assembles a kubernetesBuilder from a struct literal plus a hand-built rest.Config, and grep -rn "newKubernetesBuilder(" server/ shows the only call site is builder.go:112 — production code. So kubeconfig/in-cluster resolution, the parseKubernetesDriverOpts → namespace/readyTimeout/podName wiring, and the namespace fallback chain the docs describe are exercised only by the cluster-free CI guard, which asserts one namespace string. The 25 new unit tests are otherwise strong; this is the one hole.

8. Option values are not validated at write time

server/pkg/docker/kubernetes.go:354

holds. namespace, serviceaccount, image and label/annotation/nodeselector keys are accepted verbatim, so namespace=Trdl Build passes configure and fails at release — the exact failure mode the separate option vocabulary exists to prevent (ValidateBuildkitdDriverOpts' own doc comment at builder.go:205-208 states that intent). k8s.io/apimachinery/pkg/util/validation is already linked.

9. rootless, deadline and timeout values are parsed untrimmed

server/pkg/docker/kubernetes.go:365

New — raised by a round-1 reviewer, verified at head. parseKubernetesDriverOpts trims the option name (:350) and the namespace/image/serviceaccount values, but strconv.ParseBool(value) (:365) and time.ParseDuration(value) (:368, :371) get the raw value — so rootless= true is a configure-time rejection while namespace= foo is accepted, and the error does not mention whitespace. One strings.TrimSpace per arm.

10. The default builder image is a mutable tag

server/pkg/docker/kubernetes.go:27

holds. moby/buildkit:buildx-stable-1 receives the build context and every build secret, while server/pkg/config/trdl.go:35 refuses an unpinned docker_image in trdl.yaml via ValidateImageNameWithDigest. Not a regression (buildx defaults the same way) but trdl now chooses the image itself. One doc sentence: the tag is mutable, and image accepts a digest.

11. Env-provided buildx driver options are not reported as unused

server/pkg/docker/builder.go:106

holds. The buildkitd branch reports an ignored TRDL_BUILDX_DRIVER via resolveBuildxDriver("") but says nothing about TRDL_BUILDX_DRIVER_OPTS_*, while unusedBuilderSettings on the buildkitd_address path reports both. Asymmetric with the branch's own stated intent ("say what is being ignored", builder.go:103-105).

12. The readiness probe inherits kubelet's probe defaults

server/pkg/docker/kubernetes.go:491

New — raised by a round-1 reviewer; mechanism verified, impact not. The ReadinessProbe sets only Exec: buildctl debug workers (:491-497), so it inherits timeoutSeconds: 1, periodSeconds: 10, failureThreshold: 3. One second for an exec on a node that is, by construction, pulling the buildkit image and starting a privileged container is a plausible flake source, and 10s polling granularity is coarse against a 2m budget. TimeoutSeconds: 5, PeriodSeconds: 2 speeds up the common case and removes the flake source. No observed flake backs this — see Open note below.

13. The leftover-pod check races the termination grace period

.github/workflows/tests_buildx_kubernetes_driver.yaml:177

holds, quoted as written this time: the step (if: always(), :168) probes the API (kubectl get namespace default, :173), captures kubectl get pods -n default --no-headers > /tmp/pods-after.txt (:174) once, then fails on if grep '^trdl-builder-' /tmp/pods-after.txt (:177). Remove returns when the DELETE is accepted; the pod then terminates over its grace period, so a Terminating pod fails the job — and if: always() widens the window to failed-suite runs too. Wrap the capture+grep in a bounded wait (e.g. 30×2s, or kubectl wait --for=delete); keep the API probe and the diagnostics step as they are.

NIT

14. Docs claim the options are "passed through as is"

docs/pages_en/QUICKSTART.md:103, docs/pages_ru/QUICKSTART.md:102

holds. True of buildx_driver_opts, false here — values are split on the first =, parsed (bool/duration/quantity) and rejected on error, and nodeselector/labels/annotations values are comma-split by splitKeyValues (kubernetes.go:445) — the opposite of "as is". The intended meaning (configure does not split list elements) is true; the phrase is not. Drop the clause. (The generated reference docs/_includes/reference/vault_plugin/configure.md is correct: its rows are the field descriptions verbatim.)

15. Duplicated comment clause

server/pkg/docker/builder.go:149

holds. Ends with the same words as builder.go:105 ("…discovered from a builder that never appears").

16. The workflow file name still says buildx

.github/workflows/tests_buildx_kubernetes_driver.yaml:1

New in this pass. name: became "Tests. In-cluster Kubernetes builders" and the file now hosts a job that deliberately avoids buildx, but the filename still reads tests_buildx_kubernetes_driver.yaml. Cosmetic; renaming breaks any saved dispatch link, so "leave it" is a defensible answer — just a deliberate one.

Non-findings (checked, no action)

Carried forward from pass 1, each re-read at d7b21fa:

• bkclient.New(ctx, "", WithContextDialer) — re-derived, and pass 1's phrasing was imprecise. moby/buildkit@v0.31.2/client/client.go:120 guards if address == "" and :121 does substitute address = appdefaults.Address; that value is inert because :49 needDialer := true is set to false at :71 when a withDialer opt is present, so resolveDialer(address) at :112 never runs and the substituted address only feeds url.Parse for the :authority pseudo-header. Correct as written. holds.
• waitForPod treating 404 as transient — correct (create→read lag, watch-cache staleness). holds.
• execConn safe under double Close, concurrent Read/Write/Close, early StreamWithContext return, pod dying mid-build — io.Pipe tolerates all of it and both ends carry the stream error. holds.
• Build context is bounded by the task manager (30m default) and cleanup runs on a fresh context, so a wedged stream still deletes the pod. holds — for in-process failures only; out-of-process death is finding 3.
• SPDY-only exec: every supported API server still speaks it, buildx is SPDY-only; client-go 0.34 ships NewFallbackExecutor if a report ever arrives. holds.
• Pod name trdl-builder-<uuid> — valid DNS-1123, 49 chars. holds.
• RBAC in the docs (create/get/delete pods, create pods/exec) matches the code. holds.
• Comment density: high but mostly load-bearing rationale; two blocks restate the code (kubernetes.go:129-133, podState at :314). holds.
• 38 new modules / +12.3 MB vs a measured 88 / +36 MB for importing buildx's driver: proportionate, maintainer's call. holds.
• Cluster coverage limited to privileged/namespace=default (tests_buildx_kubernetes_driver.yaml:105); rootless=true, deadline, resources and serviceaccount are manifest-level only. holds — scope call, worth a follow-up dispatch.

New in this pass:

• server/path_configure.go audit. isConfigurationFieldSet's panic(fmt.Sprintf("field %q has no emptiness rule", name)) at :175 is unreachable today (all call sites are TypeString/TypeStringSlice) and fires only on a programmer error — which CLAUDE.md explicitly sanctions ("Panic on programmer errors"). A round-1 reviewer rates it MINOR (a panic on a Vault request path takes the plugin process down); rejected as a finding here because the house rule is explicit, the arm guards exactly the mistake it names (a new field type added without an emptiness rule), and silently returning false would turn that programmer error into a silently-skipped conflict check — worse. Validation order is ValidateBuildkitdAddress → ValidateBuildxDriver → ValidateBuildkitdDriver → conflict checks → ValidateBuildkitdDriverOpts → putConfiguration, so storage is untouched on every rejection — path_configure_test.go proves it for create (the assert.Nil(cfg, "a rejected configuration must not be stored") rejection tests) and update (TestUpdate_RejectedBuildkitdDriverKeepsConfiguration, :360), and the conflict error names the field the operator must remove ("%s cannot be combined with %s", :207/:213). The conflict checks read only the incoming *framework.FieldData, which matches the pre-existing whole-config-replacement semantics (:221-237 builds cfg entirely from fields) — not a new defect. firstSetConfigurationField collapses the pairwise checks cleanly.
• Driver-name dispatch: builder.go:112 and ValidateBuildkitdDriverOpts (:209) validate the driver string and then take the kubernetes path unconditionally — correct at one supported value, and the same shape the pre-existing ValidateBuildxDriver/resolveBuildxDriver pair uses. A round-1 reviewer rates this MINOR-advisory (a second driver would silently take the kubernetes path); kept as a non-finding because it is established house style, recorded here so the next driver author sees it.
• buildkit.go extraction: buildWithBuildkit (:103) now dials, defer bkClient.Close(), then delegates to buildWithBuildkitClient (:113) with identical arguments — the buildkitd_address backend is unchanged; the pod path holds the same discipline in Builder.Build. The two new buildkit_test.go cases (TestNewBuilder_ReportsUnusedBuildxSettingsInBuildkitMode at :229, TestNewBuilder_ReportsUnusedBuildkitdDriverInBuildkitMode at :250) assert the unused-settings log lines against the real messages built in builder.go — they do not exercise the extraction itself, and buildWithBuildkitClient has no direct test; the extraction's safety verdict above rests on inspection, which the mechanical delegation supports.
• Single-live-spec invariant in the e2e suite: suite_test.go:54 AfterEach runs server:dev:cleanup, whose first step deletes the plugin binary BeforeSuite (:35) builds once — so flow_vault tolerates exactly one live spec, and the guard is registered from init() gated on TRDL_TEST_BUILDKITD_DRIVER (buildkitd_driver_forwarding_test.go:32) rather than skipped at runtime. The invariant is documented in the spec comment (:20-25) and enforced only by the CI label filter staying exact — a real trap for whoever adds a second spec, recorded here; correctly handled today.
• serverConfigureProject appending empty positional args when driver envs are unset (e2e/tests/flow_vault/utils.go:272-275): pre-existing shape — buildkitdAddress and buildxDriver already did it; buildkitdDriver follows it. Not introduced here.
• Config plumbing traced end to end with no field dropped: path_configure.go fields → cfg.BuildkitdDriver/BuildkitdDriverOpts → path_release.go:176-177 → BuildReleaseArtifactsOpts (build.go:36-37) → BuilderOpts (build.go:111-112).
• Documented option list vs code: supportedKubernetesDriverOpts (kubernetes.go:41-57), the buildkitd_driver_opts field description (path_configure.go:143) and the generated configure.md row all list the same 15 options (annotations, deadline, image, labels, limits.cpu, limits.ephemeral-storage, limits.memory, namespace, nodeselector, requests.cpu, requests.ephemeral-storage, requests.memory, rootless, serviceaccount, timeout). No divergence.
• The required cluster-free guard is sound, and unusually well built: .github/workflows/tests.yml:241 asserts Ran [1-9][0-9]* of [0-9]+ Specs before the :244 grep for unable to create builder pod trdl-forwarding-guard/, closing the "ginkgo exits 0 when the label filter selects nothing" hole; the substring reaches the log through e2e/Taskfile.yaml:44 ginkgo --vv → server/pkg/testutil/command.go:51 GinkgoWriter.Write(res) of cmd.CombinedOutput(). The job also cannot trip the new configure-time conflict: it sets plugin-env TRDL_BUILDX_DRIVER=docker-container (reported as unused) while only TRDL_TEST_BUILDKITD_DRIVER sets a stored field — buildx_driver is written solely from TRDL_TEST_BUILDX_DRIVER (e2e/tests/flow_vault/utils.go:225), which is unset there.
• e2e/go.mod:180-188 k8s deps are // indirect only; grep -rn "k8s.io/" e2e --include=*.go is empty — pulled through the replaced server module, no direct e2e dependency.
• Drive-by changes, all in-scope hardening or harmless: KIND_SHA256 pin with sha256sum --check --strict before sudo install, two persist-credentials: false, permissions: contents: read, and the MVS-forced onsi/ginkgo/v2 v2.20.1 → v2.21.0 in server/go.mod.

Open note: finding 12 rests on kubelet defaults, not an observed flake; a loaded-node run of the kind job would settle it, and "1s is fine in practice" is an acceptable maintainer answer.

Dependency and security sign-off required

CLAUDE.md:108 ("NEVER add new external dependencies without flagging to the user first") and CLAUDE.md:110 ("NEVER introduce changes that may compromise security. Flag to the user first") both apply; neither is a defect, both need an explicit maintainer decision.

• New direct requires in server/go.mod: k8s.io/api, k8s.io/apimachinery, k8s.io/client-go, all v0.34.1, pulling moby/spdystream, gorilla/websocket, gogo/protobuf, json-iterator/go, fxamacker/cbor/v2, munnerz/goautoneg, mxk/go-flowrate, golang.org/x/oauth2, sigs.k8s.io/{json,randfill,structured-merge-diff/v6,yaml} and more — 38 modules total, plugin binary 48.8 → 61.1 MB. The author kept this as small as it can be: a hand-rolled core/v1-only REST client and scheme instead of the generated typed client — the right call.
• New capability surface: whoever can write <project>/configure chooses the namespace, ServiceAccount and image of a privileged pod created with the plugin's own Kubernetes credentials, and every build secret (project secrets, mac-signing certificate, password, notary key) streams into it. The precise off switch is the plugin's own Role — deny pods create and the feature is unavailable; an application-level namespace/image allowlist would duplicate RBAC with a weaker mechanism and is not recommended.

[alexey-igrychev]

Review methodology

This describes how the final-pass review above was produced. It covers PR head d7b21fa5ce5b1ceaf2c4874c4d0f11d9dc8ad803 against base main 443fdd352d8a43c35ce0c1771fbe5610de6cbbdf.

1. PR metadata, description, commits and the changed-file list were retrieved through the GitHub API; the base was resolved from the PR's actual baseRefName, not assumed. The head was checked out locally and git status --porcelain verified empty before and after every round.
2. Pass 1 read the complete diff and the Kubernetes driver core (kubernetes.go, kubernetes_exec.go, builder.go, both workflows, the e2e guard). The final pass re-adjudicated every pass-1 finding and audited what pass 1 never covered in full: path_configure.go, the e2e harness, the buildkit.go extraction, the config plumbing end to end, dependency hygiene.
3. The draft report was then subjected to two independent adversarial reviews by different model families. Their proposals were accepted only after fresh first-hand source verification: one MAJOR (the orphanable builder pod) and two MINORs (untrimmed option values, probe defaults) added; two proposed findings rejected with recorded reasoning; five citations corrected.
4. The amended report went through two further adversarial rounds against the report itself. In the last round both reviewers re-derived all 16 finding anchors at d7b21fa and approved; the two residual notes were audit-trail gaps in a non-finding bullet, closed with source citations.
5. One earlier MAJOR (a claimed exec-session leak across gRPC redials) was retracted after reading the involved libraries first-hand in the module cache; the retraction and its evidence are finding 4 of the report, kept visible rather than silently dropped.

Models and skills

Models

• Primary synthesis: active Pi coding-agent session (Anthropic Claude Fable).
• Independent adversarial passes, rounds 1–3: Anthropic Claude Fable 5 and OpenAI GPT-5.6 Sol, one of each per round.

The independent passes were used for adversarial inspection, not as proof by assertion: every accepted finding was re-verified against source before inclusion, and each round re-checked the previous round's edits.

Skills and project rules

• rigorous-review: finding classification, severity discipline, evidence-before-claim.
• agent-code-review: adversarial verification of the changed tests and CI guards (check-gaming inspection).
• pull-request: publication and PR-comment conventions.
• Project AGENTS.md: task-only commands, LSP-first navigation, review guidelines (dependencies, security, breaking changes).
• Active session modes: Ponytail (full) and Caveman (full). Neither changed severity or verification criteria.

Verification

• Review-only scope held throughout: no source file was modified at any point, and no build/test command was run by the review itself (the unit-test task writes tests_coverage/, which was verified untouched via find tests_coverage -newer <report>).
• Test and lint state was reused from runs at this same head: task server:test:unit paths="./pkg/docker/..." — Test Suite Passed; task server:lint — 0 issues.
• lsp diagnostics clean on server/pkg/docker/kubernetes.go.
• Library-level claims were verified in the module cache, not from documentation: google.golang.org/grpc@v1.82.1/internal/transport/http2_client.go:1009, k8s.io/client-go@v0.34.1/tools/remotecommand/v2.go:90, moby/buildkit@v0.31.2/client/client.go:112-121. These reads are the basis of the finding-4 retraction and of the bkclient.New non-finding.

Review limitations

• No mutation testing in this pass — that requires modifying the tree, which review-only scope forbids. Mutation evidence for this PR exists in the earlier verification report above (2026-08-31).
• No cluster was run in this pass; kind/e2e evidence is reused from the CI runs and the earlier verification report.
• Finding 12 (readiness-probe defaults) rests on kubelet defaults, not an observed flake, and is marked accordingly in the report.