Skip to content

Commit 89f0627

Browse files
committed
fix(cli): round-2 review refinements for local-bundle
- sync env vars BEFORE initializing the deployment: initialization enqueues the remote build synchronously, so a post-init sync raced a fast build, a run triggered right after promotion could execute without the synced vars - keep the bundle dir on --dry-run so the printed path is inspectable - always append the build-args exclusions as the LAST .dockerignore lines so a pre-existing negation cannot re-include them - warn when --from-bundle initializes a fresh deployment (attach mode is the supported flow)
1 parent 26353da commit 89f0627

1 file changed

Lines changed: 56 additions & 54 deletions

File tree

packages/cli-v3/src/commands/deploy.ts

Lines changed: 56 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,7 +1139,8 @@ async function handleNativeBuildServerDeploy({
11391139
const serverEnvVars = await apiClient.getEnvironmentVariables(config.project);
11401140
loadDotEnvVars(config.workingDir, options.envFile);
11411141

1142-
const destination = getTmpDir(config.workingDir, "build", false);
1142+
// Keep the bundle dir around on dry runs so the printed path is inspectable
1143+
const destination = getTmpDir(config.workingDir, "build", options.dryRun);
11431144
const forcedExternals = await resolveAlwaysExternal(apiClient);
11441145

11451146
const $buildSpinner = spinner({ plain: options.plain });
@@ -1181,23 +1182,58 @@ async function handleNativeBuildServerDeploy({
11811182
env: buildManifest.build.env ?? {},
11821183
});
11831184

1184-
// Append to a .dockerignore a build extension may have produced, never clobber it
1185+
// Append to a .dockerignore a build extension may have produced, never clobber it.
1186+
// Our exclusions always go LAST so a pre-existing negation (!file) can't re-include
1187+
// the build-args file into the image context.
11851188
const dockerignorePath = join(destination.path, ".dockerignore");
11861189
const [, existingDockerignore] = await tryCatch(readFile(dockerignorePath, "utf-8"));
1187-
const dockerignoreEntries = [BUNDLE_BUILD_ARGS_FILE, ".dockerignore"].filter(
1188-
(entry) => !existingDockerignore?.split("\n").includes(entry)
1190+
await writeFile(
1191+
dockerignorePath,
1192+
`${
1193+
existingDockerignore ? existingDockerignore.trimEnd() + "\n" : ""
1194+
}${BUNDLE_BUILD_ARGS_FILE}\n.dockerignore\n`
11891195
);
1190-
if (dockerignoreEntries.length > 0) {
1191-
await writeFile(
1192-
dockerignorePath,
1193-
`${existingDockerignore ? existingDockerignore.trimEnd() + "\n" : ""}${dockerignoreEntries.join("\n")}\n`
1194-
);
1195-
}
11961196

11971197
if (options.dryRun) {
11981198
logger.info(`Dry run complete. View the built bundle at ${destination.path}`);
11991199
return;
12001200
}
1201+
1202+
// Sync env vars BEFORE initializing the deployment: initialization enqueues the
1203+
// remote build synchronously, so syncing afterwards would race a fast build —
1204+
// a run triggered right after promotion could execute without the synced vars.
1205+
// Syncing is environment-scoped and needs no deployment, so pre-init is safe.
1206+
if (!options.skipSyncEnvVars) {
1207+
const childVars = buildManifest.deploy.sync?.env ?? {};
1208+
const parentVars = buildManifest.deploy.sync?.parentEnv ?? {};
1209+
const secretChildVars = buildManifest.deploy.sync?.secretEnv ?? {};
1210+
const secretParentVars = buildManifest.deploy.sync?.secretParentEnv ?? {};
1211+
1212+
const hasVarsToSync =
1213+
Object.keys(childVars).length > 0 ||
1214+
Object.keys(secretChildVars).length > 0 ||
1215+
// Only sync parent variables if this is a branch environment
1216+
(branch &&
1217+
(Object.keys(parentVars).length > 0 || Object.keys(secretParentVars).length > 0));
1218+
1219+
if (hasVarsToSync) {
1220+
const uploadResult = await syncEnvVarsWithServer(
1221+
apiClient,
1222+
config.project,
1223+
options.env,
1224+
childVars,
1225+
parentVars,
1226+
secretChildVars,
1227+
secretParentVars
1228+
);
1229+
1230+
if (!uploadResult.success) {
1231+
throw new Error(`Failed to sync env vars with the server: ${uploadResult.error}`);
1232+
}
1233+
1234+
logger.debug("Synced env vars with the server");
1235+
}
1236+
}
12011237
}
12021238

12031239
const $deploymentSpinner = spinner();
@@ -1303,50 +1339,6 @@ async function handleNativeBuildServerDeploy({
13031339

13041340
const deployment = initializeDeploymentResult.data;
13051341

1306-
// In --local-bundle mode the build server never runs install/bundle, so the env-var
1307-
// sync that extensions rely on (syncEnvVars) must happen here on the client, using
1308-
// the unscrubbed in-memory manifest — same semantics as the classic local path.
1309-
if (bundleManifest && !options.skipSyncEnvVars) {
1310-
const childVars = bundleManifest.deploy.sync?.env ?? {};
1311-
const parentVars = bundleManifest.deploy.sync?.parentEnv ?? {};
1312-
const secretChildVars = bundleManifest.deploy.sync?.secretEnv ?? {};
1313-
const secretParentVars = bundleManifest.deploy.sync?.secretParentEnv ?? {};
1314-
1315-
const hasVarsToSync =
1316-
Object.keys(childVars).length > 0 ||
1317-
Object.keys(secretChildVars).length > 0 ||
1318-
// Only sync parent variables if this is a branch environment
1319-
(branch && (Object.keys(parentVars).length > 0 || Object.keys(secretParentVars).length > 0));
1320-
1321-
if (hasVarsToSync) {
1322-
const uploadResult = await syncEnvVarsWithServer(
1323-
apiClient,
1324-
config.project,
1325-
options.env,
1326-
childVars,
1327-
parentVars,
1328-
secretChildVars,
1329-
secretParentVars
1330-
);
1331-
1332-
if (!uploadResult.success) {
1333-
$deploymentSpinner.stop("Failed to sync env vars");
1334-
log.error(chalk.bold(chalkError(`Failed to sync env vars: ${uploadResult.error}`)));
1335-
1336-
await apiClient.failDeployment(deployment.id, {
1337-
error: {
1338-
name: "SyncEnvVarsError",
1339-
message: `Failed to sync env vars with the server: ${uploadResult.error}`,
1340-
},
1341-
});
1342-
1343-
throw new OutroCommandError(`Deployment failed`);
1344-
}
1345-
1346-
logger.debug("Synced env vars with the server");
1347-
}
1348-
}
1349-
13501342
const rawDeploymentLink = `${dashboardUrl}/projects/v3/${config.project}/deployments/${deployment.shortCode}`;
13511343
const rawTestLink = `${dashboardUrl}/projects/v3/${config.project}/test?environment=${
13521344
options.env === "prod" ? "prod" : "stg"
@@ -1745,6 +1737,16 @@ async function handleFromBundleDeploy({
17451737
throw new Error("Failed to get project client");
17461738
}
17471739

1740+
if (!existingDeploymentId) {
1741+
// The supported flow is attach mode (the build server sets
1742+
// TRIGGER_EXISTING_DEPLOYMENT_ID). Fresh-init from a bundle is equivalent to a
1743+
// plain local build and mainly useful for local testing — warn so nobody relies
1744+
// on it against cloud by accident.
1745+
logger.warn(
1746+
"No existing deployment to attach to — initializing a fresh local-build deployment from the bundle. This path is intended for testing."
1747+
);
1748+
}
1749+
17481750
const deployment = await initializeOrAttachDeployment(
17491751
projectClient.client,
17501752
{

0 commit comments

Comments
 (0)