fix(diff): ignore preview env rows so they cannot shadow production (#85)
`GET /applications/{uuid}/envs` does not return one row per key: it merges
the production vars with the PREVIEW ones into one flat array
(`environment_variables->merge(environment_variables_preview)`,
ApplicationsController@envs v4.1.2). The two relations are complements split
on `is_preview`, with a unique index per (key, resource, is_preview) — so the
same key legitimately arrives twice. `fetchEnv` keyed by `key` alone, and
`Object.fromEntries` keeps the LAST, so cast diffed the manifest against
whichever row Coolify happened to serialize last.
Confirmed on prod: REPORTING_ENABLED came back as {value:"true",
is_preview:false} AND {value:"false", is_preview:true}; cast read the "false"
twin and re-proposed a `change` that could never clear.
That is also why #78 looked like a stale read. Both rows are born equal
(Coolify seeds a preview twin), and syncEnv only ever PATCHes the PRODUCTION
row — so the two diverge for exactly the vars updated in place. Five prod
flags flipped false->true re-proposed on every diff forever, while
created-once vars stayed clean because their twins still agreed. Nothing was
stale: cast was reading the other deployment's value. `real_value` tracked
`value` on every row, exactly as the accessor predicts.
cast declares PRODUCTION env and already says so on every write — syncEnv
sends `is_preview: false` on each bulk upsert. This is the read finally
saying the same thing; the asymmetry was the whole bug. Services and
databases map a single set, so this is a no-op for them.
Tests pin the exact prod shape, both serialization orders (the fix is "drop
preview", not "take the first"), a preview-only key, and rows with no
is_preview field at all.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
61fd72cea9
commit
d083f74bec
2 changed files with 110 additions and 3 deletions
38
src/cli.ts
38
src/cli.ts
|
|
@ -875,13 +875,38 @@ export function aliasLive<T extends { name: string }>(
|
||||||
// The diff picks between them per var (see LiveEnvVar, diffEnv); callers that
|
// The diff picks between them per var (see LiveEnvVar, diffEnv); callers that
|
||||||
// only need one flattened string use flattenEnv below.
|
// only need one flattened string use flattenEnv below.
|
||||||
//
|
//
|
||||||
|
// PREVIEW ROWS ARE DROPPED, and that is load-bearing rather than tidy.
|
||||||
|
//
|
||||||
|
// GET /applications/{uuid}/envs does NOT return one row per key. It MERGES two
|
||||||
|
// parallel sets into one flat array — the production vars and the PREVIEW ones
|
||||||
|
// (`environment_variables->merge(environment_variables_preview)`,
|
||||||
|
// ApplicationsController@envs v4.1.2) — and the two relations are complements
|
||||||
|
// split on `is_preview`, with a unique index per (key, resource, is_preview). So
|
||||||
|
// the SAME KEY legitimately arrives TWICE, and keying by `key` alone kept
|
||||||
|
// whichever row Coolify happened to serialize last.
|
||||||
|
//
|
||||||
|
// That is #85, and it is why #78 looked like a "stale read": both rows are born
|
||||||
|
// equal (Coolify seeds a preview twin), and syncEnv below only ever PATCHes the
|
||||||
|
// PRODUCTION row — so the two diverge for exactly the vars that were updated in
|
||||||
|
// place. Five prod flags flipped false->true re-proposed as `change` on every
|
||||||
|
// diff forever, while created-once vars (NODE_ENV, …) stayed clean because their
|
||||||
|
// twins still agreed. Nothing was stale; cast was reading the other deployment's
|
||||||
|
// value.
|
||||||
|
//
|
||||||
|
// cast declares PRODUCTION env, and already says so on every WRITE — syncEnv
|
||||||
|
// sends `is_preview: false` on each bulk upsert. This is the read finally saying
|
||||||
|
// the same thing. A preview var is another deployment's value for the same name:
|
||||||
|
// not cast's to compare, and not cast's to write. Services and databases have no
|
||||||
|
// preview relation at all (their controllers map a single set), so this is a
|
||||||
|
// no-op for them.
|
||||||
|
//
|
||||||
// A 404 (a resource we just listed no longer having an envs endpoint — not
|
// A 404 (a resource we just listed no longer having an envs endpoint — not
|
||||||
// expected in practice, but consistent with treating "gone" as "no env vars")
|
// expected in practice, but consistent with treating "gone" as "no env vars")
|
||||||
// collapses to {}; anything else (401, 5xx, network) must surface. Swallowing
|
// collapses to {}; anything else (401, 5xx, network) must surface. Swallowing
|
||||||
// it would make a live resource's env look EMPTY, which turns every one of its
|
// it would make a live resource's env look EMPTY, which turns every one of its
|
||||||
// vars into a spurious create in a diff, and into a spurious "missing" in a
|
// vars into a spurious create in a diff, and into a spurious "missing" in a
|
||||||
// capture.
|
// capture.
|
||||||
async function fetchEnv(
|
export async function fetchEnv(
|
||||||
client: CoolifyClient,
|
client: CoolifyClient,
|
||||||
l: Pick<Live, "kind" | "uuid">,
|
l: Pick<Live, "kind" | "uuid">,
|
||||||
): Promise<Record<string, LiveEnvVar>> {
|
): Promise<Record<string, LiveEnvVar>> {
|
||||||
|
|
@ -889,9 +914,16 @@ async function fetchEnv(
|
||||||
const envs = (await client.get(`/${base}/${l.uuid}/envs`).catch((err) => {
|
const envs = (await client.get(`/${base}/${l.uuid}/envs`).catch((err) => {
|
||||||
if (err instanceof HttpError && err.status === 404) return [];
|
if (err instanceof HttpError && err.status === 404) return [];
|
||||||
throw err;
|
throw err;
|
||||||
})) as Array<{ key: string; real_value?: string; value: string }>;
|
})) as Array<{
|
||||||
|
key: string;
|
||||||
|
real_value?: string;
|
||||||
|
value: string;
|
||||||
|
is_preview?: boolean;
|
||||||
|
}>;
|
||||||
return Object.fromEntries(
|
return Object.fromEntries(
|
||||||
envs.map((e) => [e.key, { value: e.value, realValue: e.real_value }]),
|
envs
|
||||||
|
.filter((e) => e.is_preview !== true)
|
||||||
|
.map((e) => [e.key, { value: e.value, realValue: e.real_value }]),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest";
|
||||||
import {
|
import {
|
||||||
attachBackup,
|
attachBackup,
|
||||||
attachServiceDomains,
|
attachServiceDomains,
|
||||||
|
fetchEnv,
|
||||||
fetchLive,
|
fetchLive,
|
||||||
renderAbsentTarget,
|
renderAbsentTarget,
|
||||||
} from "../src/cli.js";
|
} from "../src/cli.js";
|
||||||
|
|
@ -276,6 +277,80 @@ describe("attachBackup", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("fetchEnv — preview rows never shadow production (cast#85)", () => {
|
||||||
|
const app = { kind: "application" as const, uuid: "a1" };
|
||||||
|
const client = (body: unknown) =>
|
||||||
|
new CoolifyClient(
|
||||||
|
"https://coolify.test",
|
||||||
|
"tok",
|
||||||
|
vi.fn(
|
||||||
|
async () => new Response(JSON.stringify(body), { status: 200 }),
|
||||||
|
) as unknown as typeof fetch,
|
||||||
|
);
|
||||||
|
|
||||||
|
// The exact shape prod returned (#85): GET /applications/{uuid}/envs merges
|
||||||
|
// environment_variables with environment_variables_preview, so a key arrives
|
||||||
|
// TWICE — production first, its preview twin second. Keying by `key` alone
|
||||||
|
// kept the LAST, which is how five flipped prod flags re-proposed forever.
|
||||||
|
it("takes the production row even though the preview twin is serialized last", async () => {
|
||||||
|
const env = await fetchEnv(
|
||||||
|
client([
|
||||||
|
{
|
||||||
|
key: "REPORTING_ENABLED",
|
||||||
|
value: "true",
|
||||||
|
real_value: "true",
|
||||||
|
is_preview: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
key: "REPORTING_ENABLED",
|
||||||
|
value: "false",
|
||||||
|
real_value: "false",
|
||||||
|
is_preview: true,
|
||||||
|
},
|
||||||
|
]),
|
||||||
|
app,
|
||||||
|
);
|
||||||
|
expect(env.REPORTING_ENABLED).toEqual({ value: "true", realValue: "true" });
|
||||||
|
});
|
||||||
|
|
||||||
|
// Order must not decide the answer: the fix is "drop preview", not "take the
|
||||||
|
// first" — a serialization order that put the twin first would otherwise just
|
||||||
|
// move the bug rather than remove it.
|
||||||
|
it("takes the production row even when the preview twin is serialized FIRST", async () => {
|
||||||
|
const env = await fetchEnv(
|
||||||
|
client([
|
||||||
|
{ key: "BRAIN_ENABLED", value: "false", is_preview: true },
|
||||||
|
{ key: "BRAIN_ENABLED", value: "true", is_preview: false },
|
||||||
|
]),
|
||||||
|
app,
|
||||||
|
);
|
||||||
|
expect(env.BRAIN_ENABLED).toEqual({ value: "true", realValue: undefined });
|
||||||
|
});
|
||||||
|
|
||||||
|
// A preview-only var is not production state at all. It must not surface as an
|
||||||
|
// orphan/remove-candidate either — cast neither compares nor writes it.
|
||||||
|
it("drops a key that exists ONLY as a preview var", async () => {
|
||||||
|
const env = await fetchEnv(
|
||||||
|
client([
|
||||||
|
{ key: "PORT", value: "3000", is_preview: false },
|
||||||
|
{ key: "PREVIEW_ONLY", value: "x", is_preview: true },
|
||||||
|
]),
|
||||||
|
app,
|
||||||
|
);
|
||||||
|
expect(Object.keys(env)).toEqual(["PORT"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Services and databases map a single set and their rows may carry no
|
||||||
|
// is_preview at all — absent must mean "keep", never "drop".
|
||||||
|
it("keeps rows with no is_preview field (services/databases)", async () => {
|
||||||
|
const env = await fetchEnv(client([{ key: "APP_SECRET", value: "s" }]), {
|
||||||
|
kind: "service",
|
||||||
|
uuid: "s1",
|
||||||
|
});
|
||||||
|
expect(env.APP_SECRET).toEqual({ value: "s", realValue: undefined });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("attachServiceDomains (cast#72)", () => {
|
describe("attachServiceDomains (cast#72)", () => {
|
||||||
const svc = () => ({
|
const svc = () => ({
|
||||||
kind: "service" as const,
|
kind: "service" as const,
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue