fix: resolve environment variables on application rollback - #4923
Open
dmtrTm wants to merge 1 commit into
Open
Conversation
rollbackApplication() called prepareEnvironmentVariables() without the
environment env, so every ${{environment.*}} reference threw "Invalid
environment variable: environment.X" and the rollback failed with an
opaque 400 before the swarm service was updated.
fb749cd added the third argument to every other call site but left
services/rollbacks.ts, which had been calling the helper since 24bff96.
The value is already captured in the snapshot by createRollback(); both
fullContext declarations simply typed environment as { project: Project },
so the missing argument was invisible to the compiler.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3 tasks
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.
What is this PR about?
Rolling back an application whose env references environment-scoped variables (e.g.
${{environment.FOO}}) always failed withInvalid environment variable: environment.FOO, surfaced to the client as an opaqueHTTP 400 "Error input: Rolling back". Regular deployments resolve those variables correctly.rollbackApplication()was missing the third parameter when callingprepareEnvironmentVariables():This completes the rollout started in fb749cd, which added the third argument to every other call site (all six databases, the builders, compose) but left
services/rollbacks.ts, which had been calling the helper since 24bff96. It was the last remaining two-argument caller in the repo.The value is already captured in the snapshot -
createRollback()stores the result offindApplicationById(), which loadsenvironment: { with: { project: true } }- it was simply never passed to the resolver. BothfullContextdeclarations (the local one inrollbackApplication()and thejsonb().$type<>()annotation on therollbacktable) typedenvironmentas{ project: Project }withoutenv, which is why the missing argument was invisible to the compiler; both are widened toEnvironment & { project: Project }.No schema change, no migration, no backfill: existing snapshots already contain the value, so this also repairs rollbacks stored by earlier versions. Snapshots that predate the environment tables keep today's exact behaviour -
parse(undefined ?? "")yields the same empty map, and the same error. Both scope arguments come from the snapshot rather than the live rows, consistent with the rest ofrollbackApplication()(service env, project env, mounts, ports and resources all come from the snapshot).How this was verified
Local Dokploy dev instance (v0.29.13, single-node swarm), application on the Docker provider with
NODE_ENV=${{environment.NODE_ENV}}plus a marker variable, an environment-levelNODE_ENV=production, and rollbacks enabled against a local registry. Two deployments produced:v1(nginx:alpine,APP_MARKER=v1) and:v2(nginx:1.27-alpine,APP_MARKER=v2), then Rollback to:v1:Before -
HTTP 400 {"message":"Error input: Rolling back","code":"BAD_REQUEST","zodError":null}, with the real cause only in the backend log:The swarm service was left untouched (still
nginx:1.27-alpine) - the throw happens beforeservice.update().After -
HTTP 200, and the swarm service was updated:NODE_ENVresolves from the environment scope, andAPP_MARKER=v1comes from the v1 snapshot rather than the live value, confirming the snapshot semantics.Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
Same class of bug as #3452 / #3457, which fixed the Stack compose path.
Screenshots (if applicable)
N/A - Backend logic change, verified end to end as described above.