Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/api/src/modules/deployments/deployment.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,11 +353,15 @@ export async function prepare(c: Context) {
branch?: string;
path?: string;
composePath?: string;
env?: Record<string, string>;
}>();

// Determine source - callers may send { owner, repo } without an explicit source
const source = body.source ?? (body.owner && body.repo ? "github" : undefined);
const composePath = body.composePath?.trim() || undefined;
// Interpolation-only: never persisted here, and the response masks every
// service env below, so supplying a value cannot echo it back unmasked.
const composeEnv = body.env && Object.keys(body.env).length > 0 ? body.env : undefined;

try {
let input: prepareService.Source;
Expand All @@ -373,6 +377,7 @@ export async function prepare(c: Context) {
branch: body.branch,
ctx,
composePath,
env: composeEnv,
};
} else if (source === "local") {
if (env.CLOUD_MODE) {
Expand All @@ -381,7 +386,7 @@ export async function prepare(c: Context) {
if (!body.path) {
return c.json({ error: "path is required" }, 400);
}
input = { source: "local", path: body.path, composePath };
input = { source: "local", path: body.path, composePath, env: composeEnv };
} else {
return c.json({ error: "source must be 'github' or 'local'" }, 400);
}
Expand Down
6 changes: 6 additions & 0 deletions apps/api/src/modules/deployments/deployment.schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,12 @@ export const PrepareDeployBody = Type.Object({
"Where the compose file lives when it is not at the auto-detected root — the file itself (\"deploy/stack.yml\", which also covers non-standard filenames) or the directory holding it (\"deploy/docker-compose\"). Detects the project as a compose/services deploy; errors when no compose file is there.",
}),
),
env: Type.Optional(
Type.Record(Type.String(), Type.String(), {
description:
"Env already configured for this deploy. Compose interpolation resolves against these on top of the repo .env, so a file declaring ${VAR:?...} scans once the user has supplied VAR.",
}),
),
});

// POST /:id/build/respond — answer a build gate/prompt.
Expand Down
30 changes: 26 additions & 4 deletions apps/api/src/modules/deployments/prepare.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,16 @@ export type Source =
ctx?: RequestContext;
/** See {@link ResolveOptions.composePath}. */
composePath?: string;
/** See {@link ResolveOptions.env}. */
env?: Record<string, string>;
}
| { source: "local"; path: string; composePath?: string };
| {
source: "local";
path: string;
composePath?: string;
/** See {@link ResolveOptions.env}. */
env?: Record<string, string>;
};

export interface ResolveOptions {
/**
Expand All @@ -84,6 +92,14 @@ export interface ResolveOptions {
* buildpack build (the confusing behaviour this option exists to replace).
*/
composePath?: string;
/**
* Env the caller already holds for this deploy (the values configured on the
* project / entered in the wizard). Compose interpolation resolves against
* these on top of the repo `.env`, so a file declaring `${VAR:?...}` scans
* cleanly once the user has supplied VAR — without it the scan reports the
* file as unparseable even though the deploy would have succeeded (#383).
*/
env?: Record<string, string>;
}

/** Thrown when a declared `composePath` has no compose file behind it. */
Expand Down Expand Up @@ -602,6 +618,7 @@ export async function resolveProjectInfo(input: Source): Promise<ProjectInfo> {
}
return resolveFromGitHub(input.ctx, input.owner, input.repo, input.branch, {
composePath: input.composePath,
env: input.env,
});
}

Expand All @@ -611,7 +628,7 @@ export async function resolveProjectInfo(input: Source): Promise<ProjectInfo> {

// Dynamic import keeps local-source (node:fs) out of the cloud module graph.
const { resolveFromLocal } = await import("./local-source");
return resolveFromLocal(input.path, { composePath: input.composePath });
return resolveFromLocal(input.path, { composePath: input.composePath, env: input.env });
}

type RepoMeta = Parameters<typeof toProjectInfo>[0];
Expand Down Expand Up @@ -704,7 +721,7 @@ export async function resolveFromReader(
composeEnvContent,
root.monorepo,
routing,
{ declaredCompose: !!root.declaredComposePath },
{ declaredCompose: !!root.declaredComposePath, env: opts.env },
);
const overlaid = applyOpenshipOverlay(info, openshipConfig);

Expand Down Expand Up @@ -769,6 +786,8 @@ function toProjectInfo(
* services project even when stack detection wouldn't say so on its own
* (a non-standard filename like `stack.yml` matches no root marker). */
declaredCompose?: boolean;
/** See {@link ResolveOptions.env}. */
env?: Record<string, string>;
},
): ProjectInfo {
const stack = projectRoot.stack;
Expand All @@ -777,7 +796,10 @@ function toProjectInfo(
let services: ComposeService[] | undefined;
if (composeContent && (opts?.declaredCompose || stack.projectType === "services")) {
try {
const parsed = parseComposeFile(composeContent, { envFileContent: composeEnvContent });
const parsed = parseComposeFile(composeContent, {
envFileContent: composeEnvContent,
env: opts?.env,
});
services = parsed.services;
} catch (err) {
// Surface the broken file. Swallowing it returns a services project with
Expand Down
31 changes: 31 additions & 0 deletions apps/api/test/modules/deployments/prepare.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,37 @@ describe("resolveProjectInfo", () => {
);
});

it("interpolates required Compose variables from the configured deploy env", async () => {
const tempDir = await mkdtemp(join(tmpdir(), "openship-prepare-"));
tempDirs.push(tempDir);

// #383: compose lives outside the repo root (#330) and declares a required
// variable. The user supplied it in the Openship deploy configuration, so
// the scan must interpolate it instead of reporting the file as unparseable.
await mkdir(join(tempDir, "deploy", "docker-compose"), { recursive: true });
await writeFile(
join(tempDir, "deploy", "docker-compose", "docker-compose.yaml"),
[
"services:",
" db:",
" image: postgres:16-alpine",
" environment:",
" POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?Set POSTGRES_PASSWORD in .env}",
].join("\n"),
);

const info = await resolveProjectInfo({
source: "local",
path: tempDir,
composePath: "deploy/docker-compose",
env: { POSTGRES_PASSWORD: "s3cret" },
});

expect(info.services?.find((s) => s.name === "db")?.environment).toMatchObject({
POSTGRES_PASSWORD: "s3cret",
});
});

it("reports invalid Compose YAML instead of returning no services", async () => {
const tempDir = await mkdtemp(join(tmpdir(), "openship-prepare-"));
tempDirs.push(tempDir);
Expand Down
37 changes: 34 additions & 3 deletions apps/dashboard/src/context/deployment/useDeploymentConfig.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { useState, useCallback, useEffect, useRef } from "react";
import type { FrameworkId } from "@/components/import-project/types";
import type { EnvironmentVariable, FrameworkId } from "@/components/import-project/types";
import { deployApi, projectsApi, servicesApi, serviceKind } from "@/lib/api";
import { folderApi } from "@/lib/api/folder";
import type { PrepareProjectResponse, PrepareComposeService, PrepareMonorepoApp } from "@/lib/api/deploy";
Expand Down Expand Up @@ -140,6 +140,24 @@ function scanComposePath(
return trimmed ? { composePath: trimmed } : {};
}

/**
* The env the scan should interpolate the compose file against (#383). A compose
* file declaring `${VAR:?...}` is unparseable until VAR has a value, so a re-scan
* has to carry what the user already entered — otherwise the wizard reports the
* file as broken even though the deploy itself would resolve the same variable.
*
* Returns an `{ env }` fragment to spread into the prepare body, empty when
* nothing is set so a blank map never reaches the API.
*/
function scanEnv(envVars: EnvironmentVariable[]): { env?: Record<string, string> } {
const env: Record<string, string> = {};
for (const { key, value } of envVars) {
const name = key.trim();
if (name) env[name] = value;
}
return Object.keys(env).length > 0 ? { env } : {};
}

function hasSavedProjectPort(project: PersistedProject) {
if (!project) return false;

Expand Down Expand Up @@ -757,7 +775,12 @@ export function useDeploymentConfig() {
owner: string,
repo: string,
force?: string,
context?: { branch?: string; projectId?: string; composePath?: string },
context?: {
branch?: string;
projectId?: string;
composePath?: string;
env?: Record<string, string>;
},
): Promise<{ success: boolean; error?: string; errorType?: string; buildInProgress?: boolean }> => {
try {
let project: PersistedProject = null;
Expand Down Expand Up @@ -786,6 +809,7 @@ export function useDeploymentConfig() {
branch: requestedBranch,
force,
...scanComposePath(context?.composePath, project),
...(context?.env ? { env: context.env } : {}),
});

if (response?.error) {
Expand Down Expand Up @@ -835,7 +859,7 @@ export function useDeploymentConfig() {
const initializeFromLocal = useCallback(
async (
path: string,
context?: { projectId?: string; composePath?: string },
context?: { projectId?: string; composePath?: string; env?: Record<string, string> },
): Promise<{ success: boolean; error?: string; errorType?: string }> => {
try {
let project: PersistedProject = null;
Expand All @@ -849,6 +873,7 @@ export function useDeploymentConfig() {
source: "local",
path,
...scanComposePath(context?.composePath, project),
...(context?.env ? { env: context.env } : {}),
});

if (response?.error) {
Expand Down Expand Up @@ -891,11 +916,15 @@ export function useDeploymentConfig() {
// "" clears the pin: the initialize* paths drop a blank value, so the scan
// falls back to ordinary root detection.
const trimmed = composePath.trim();
// Carry the env the user has already entered so a compose file with
// required variables re-scans instead of erroring as unparseable (#383).
const env = scanEnv(config.envVars);

if (config.localPath) {
return initializeFromLocal(config.localPath, {
projectId: config.projectId,
composePath: trimmed,
...env,
});
}
if (!config.owner || !config.repo) {
Expand All @@ -909,6 +938,7 @@ export function useDeploymentConfig() {
branch: config.branch,
projectId: config.projectId,
composePath: trimmed,
...env,
});
return { success: result.success, error: result.error, errorType: result.errorType };
},
Expand All @@ -918,6 +948,7 @@ export function useDeploymentConfig() {
config.repo,
config.branch,
config.projectId,
config.envVars,
initializeFromLocal,
initializeFromRepo,
],
Expand Down
10 changes: 9 additions & 1 deletion apps/dashboard/src/lib/api/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,16 @@ export type PrepareProjectSource =
force?: string | boolean;
/** Pin the compose file location (file or directory) instead of detecting the root. */
composePath?: string;
/** Env already configured for this deploy, for compose interpolation. */
env?: Record<string, string>;
}
| { source: "local"; path: string; composePath?: string };
| {
source: "local";
path: string;
composePath?: string;
/** Env already configured for this deploy, for compose interpolation. */
env?: Record<string, string>;
};

export interface PrepareComposeService {
name: string;
Expand Down