mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-09-10 01:15:55 +00:00
feat: preserve exact guardrail message for Save & Deploy and Start
When 'Block deploy on missing required env vars' is enabled, the exact guardrail message now reaches Save & Deploy and Start (not just Update). Previously the external-networks preflight replaced the message with a generic networking error. The fix selects the guardrail-aware message inside the existing render pipeline without duplicating the render. Changes: - Backend: isGuardrailEnabled() plus guardrail-aware message selection in renderModel(), with exact singular/plural grammar matching the ComposeService guard method. - Frontend: parseMissingExternalNetworksPayload validates non-empty renderError; missingExternalBlocksDeploy prefers it with generic fallback; MissingExternalNetworksPayload extended with optional field. - Tests: backend regression for plural grammar; frontend regression for guardrail-on/off, older-node fallback, empty-string fallback, and no deploy-POST assertions. - Documentation: environment-guardrails tutorial updated.
This commit is contained in:
@@ -25,6 +25,14 @@ export interface MissingExternalNetworksEnvelope {
|
||||
networks: MissingExternalNetwork[];
|
||||
/** Count of external network declarations when the model rendered; 0 otherwise. */
|
||||
declaredExternalCount: number;
|
||||
/**
|
||||
* Present when the model could not be rendered due to a missing required variable or
|
||||
* another compose-config failure. When `env_block_deploy_on_missing_required` is enabled,
|
||||
* contains the exact guardrail message naming the missing variable(s). Otherwise
|
||||
* contains a neutral diagnostic. Undefined when the model rendered successfully or
|
||||
* when the render error was a Docker spawn/timeout failure.
|
||||
*/
|
||||
renderError?: string;
|
||||
}
|
||||
|
||||
const MAX_RENDER_ERROR = 600;
|
||||
@@ -42,9 +50,20 @@ function isAutoCreateEnabled(nodeId: number): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
function isGuardrailEnabled(nodeId: number): boolean {
|
||||
try {
|
||||
return (
|
||||
DatabaseService.getInstance().getGlobalSettings()['env_block_deploy_on_missing_required'] === '1'
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function renderModel(
|
||||
nodeId: number,
|
||||
stackName: string,
|
||||
guardrailEnabled: boolean,
|
||||
): Promise<{ model: EffectiveModel | null; renderError: string | null }> {
|
||||
try {
|
||||
const result = await ComposeService.getInstance(nodeId).renderConfig(stackName);
|
||||
@@ -61,11 +80,23 @@ async function renderModel(
|
||||
}
|
||||
}
|
||||
const missing = parseMissingRequiredVars(result.stderr);
|
||||
if (missing.length > 0) {
|
||||
if (guardrailEnabled) {
|
||||
const plural = missing.length > 1;
|
||||
return {
|
||||
model: null,
|
||||
renderError: `Deploy blocked: required environment variable${plural ? 's' : ''} ${missing.join(', ')} ` +
|
||||
`${plural ? 'are' : 'is'} missing. Define ${plural ? 'them' : 'it'} in a .env or env_file, then deploy again.`,
|
||||
};
|
||||
}
|
||||
return {
|
||||
model: null,
|
||||
renderError: `Required variable${missing.length > 1 ? 's' : ''} ${missing.join(', ')} ${missing.length > 1 ? 'have' : 'has'} no value, so the effective model cannot be rendered.`,
|
||||
};
|
||||
}
|
||||
return {
|
||||
model: null,
|
||||
renderError: missing.length
|
||||
? `Required variable${missing.length > 1 ? 's' : ''} ${missing.join(', ')} ${missing.length > 1 ? 'have' : 'has'} no value, so the effective model cannot be rendered.`
|
||||
: 'Sencho could not render the effective Compose model. Check the compose and env files for a YAML syntax error, an unresolved include or merge, or a required variable with no value.',
|
||||
renderError: 'Sencho could not render the effective Compose model. Check the compose and env files for a YAML syntax error, an unresolved include or merge, or a required variable with no value.',
|
||||
};
|
||||
} catch (err) {
|
||||
const msg = redactSensitiveText(getErrorMessage(err, 'docker compose could not be started.'))
|
||||
@@ -81,7 +112,8 @@ export async function resolveMissingExternalNetworks(
|
||||
stackName: string,
|
||||
): Promise<MissingExternalNetworksEnvelope> {
|
||||
const autoCreateEnabled = isAutoCreateEnabled(nodeId);
|
||||
const { model } = await renderModel(nodeId, stackName);
|
||||
const guardrailEnabled = isGuardrailEnabled(nodeId);
|
||||
const { model, renderError } = await renderModel(nodeId, stackName, guardrailEnabled);
|
||||
if (!model) {
|
||||
return {
|
||||
status: 'render_unavailable',
|
||||
@@ -89,6 +121,7 @@ export async function resolveMissingExternalNetworks(
|
||||
stackName,
|
||||
networks: [],
|
||||
declaredExternalCount: 0,
|
||||
renderError: renderError ?? undefined,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -59,6 +59,14 @@ This tutorial covers one guardrail: **Block deploy on missing required env vars*
|
||||
<img src="/images/tutorials/configure-environment-guardrails/env-file-password-cleared.png" alt=".env editor for inventory-db showing a single line, DB_PASSWORD with no value, with the stack still running above." />
|
||||
</Frame>
|
||||
</Step>
|
||||
<Step title="Watch Save & Deploy block with the exact guardrail message">
|
||||
Select **Save & Deploy** and this time keep **Save & Deploy** in the dropdown (not **Save Only**). The operation fails immediately, before any container changes, with **Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.** The full message appears in the notification bell in the top bar.
|
||||
|
||||
<Frame>
|
||||
<img src="/images/tutorials/configure-environment-guardrails/save-deploy-blocked-notification.png" alt="Notifications panel showing 'Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.' from a Save & Deploy operation." />
|
||||
</Frame>
|
||||
This demonstrates the fix: the guardrail message now properly appears when using **Save & Deploy**, matching the exact message shown when using **Update**.
|
||||
</Step>
|
||||
<Step title="Update the stack and watch the guardrail fire">
|
||||
Select **Update**. A readiness dialog summarizes preflight, drift, current containers, and a few other checks; note that none of them mention environment variables; the dialog can say **ready** even though this update is about to be refused. Select **Update now** anyway.
|
||||
|
||||
|
||||
@@ -2411,6 +2411,127 @@ describe('useStackActions reactive external-network retry ownership', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('useStackActions missing required variable guardrail message propagation', () => {
|
||||
function okJson(payload: unknown): Response {
|
||||
return new Response(JSON.stringify(payload), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
it('prefers guardrail-on renderError in missingExternalBlocksDeploy', async () => {
|
||||
const { result } = setup();
|
||||
vi.mocked(apiFetch).mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/missing-external-networks')) {
|
||||
return okJson({
|
||||
status: 'render_unavailable',
|
||||
stackName: 'test-stack',
|
||||
networks: [],
|
||||
autoCreateEnabled: false,
|
||||
declaredExternalCount: 0,
|
||||
renderError: 'Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.',
|
||||
});
|
||||
}
|
||||
return okJson({});
|
||||
});
|
||||
|
||||
await result.current.deployStack({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent);
|
||||
expect(toast.error).toHaveBeenCalledWith('Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.');
|
||||
// Should NOT have called /deploy (blocked by missingExternalBlocksDeploy)
|
||||
expect(apiFetch).not.toHaveBeenCalledWith(expect.stringContaining('/deploy'), expect.any(Object));
|
||||
});
|
||||
|
||||
it('prefers guardrail-off neutral diagnostic in missingExternalBlocksDeploy', async () => {
|
||||
const { result } = setup();
|
||||
vi.mocked(apiFetch).mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/missing-external-networks')) {
|
||||
return okJson({
|
||||
status: 'render_unavailable',
|
||||
stackName: 'test-stack',
|
||||
networks: [],
|
||||
autoCreateEnabled: false,
|
||||
declaredExternalCount: 0,
|
||||
renderError: 'Required variable DB_PASSWORD has no value, so the effective model cannot be rendered.',
|
||||
});
|
||||
}
|
||||
return okJson({});
|
||||
});
|
||||
|
||||
await result.current.deployStack({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent);
|
||||
expect(toast.error).toHaveBeenCalledWith('Required variable DB_PASSWORD has no value, so the effective model cannot be rendered.');
|
||||
// Should NOT have called /deploy (blocked by missingExternalBlocksDeploy)
|
||||
expect(apiFetch).not.toHaveBeenCalledWith(expect.stringContaining('/deploy'), expect.any(Object));
|
||||
});
|
||||
|
||||
it('falls back to generic message when renderError missing (older-node compatibility)', async () => {
|
||||
const { result } = setup();
|
||||
vi.mocked(apiFetch).mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/missing-external-networks')) {
|
||||
return okJson({
|
||||
status: 'render_unavailable',
|
||||
stackName: 'test-stack',
|
||||
networks: [],
|
||||
autoCreateEnabled: false,
|
||||
declaredExternalCount: 0,
|
||||
// No renderError field - older node
|
||||
});
|
||||
}
|
||||
return okJson({});
|
||||
});
|
||||
|
||||
await result.current.deployStack({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent);
|
||||
expect(toast.error).toHaveBeenCalledWith('Sencho could not render this stack\'s Compose model to check external networks.');
|
||||
// Should NOT have called /deploy (blocked by missingExternalBlocksDeploy)
|
||||
expect(apiFetch).not.toHaveBeenCalledWith(expect.stringContaining('/deploy'), expect.any(Object));
|
||||
});
|
||||
|
||||
it('falls back to generic message when renderError is empty string', async () => {
|
||||
const { result } = setup();
|
||||
vi.mocked(apiFetch).mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/missing-external-networks')) {
|
||||
return okJson({
|
||||
status: 'render_unavailable',
|
||||
stackName: 'test-stack',
|
||||
networks: [],
|
||||
autoCreateEnabled: false,
|
||||
declaredExternalCount: 0,
|
||||
renderError: '', // empty string - should fallback
|
||||
});
|
||||
}
|
||||
return okJson({});
|
||||
});
|
||||
|
||||
await result.current.deployStack({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent);
|
||||
expect(toast.error).toHaveBeenCalledWith('Sencho could not render this stack\'s Compose model to check external networks.');
|
||||
// Should NOT have called /deploy (blocked by missingExternalBlocksDeploy)
|
||||
expect(apiFetch).not.toHaveBeenCalledWith(expect.stringContaining('/deploy'), expect.any(Object));
|
||||
});
|
||||
|
||||
it('does not call /deploy when guardrail blocks with exact message', async () => {
|
||||
const { result } = setup();
|
||||
vi.mocked(apiFetch).mockImplementation(async (url: string) => {
|
||||
if (url.endsWith('/missing-external-networks')) {
|
||||
return okJson({
|
||||
status: 'render_unavailable',
|
||||
stackName: 'test-stack',
|
||||
networks: [],
|
||||
autoCreateEnabled: false,
|
||||
declaredExternalCount: 0,
|
||||
renderError: 'Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.',
|
||||
});
|
||||
}
|
||||
if (url.endsWith('/deploy')) {
|
||||
return okJson({}); // This should NOT be called
|
||||
}
|
||||
return okJson({});
|
||||
});
|
||||
|
||||
await result.current.deployStack({ preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent);
|
||||
expect(toast.error).toHaveBeenCalledWith('Deploy blocked: required environment variable DB_PASSWORD is missing. Define it in a .env or env_file, then deploy again.');
|
||||
expect(apiFetch).not.toHaveBeenCalledWith(expect.stringContaining('/deploy'), expect.any(Object));
|
||||
});
|
||||
});
|
||||
|
||||
describe('useStackActions.refreshGitSourcePending', () => {
|
||||
beforeEach(() => {
|
||||
vi.mocked(apiFetch).mockReset();
|
||||
|
||||
@@ -272,6 +272,10 @@ function parseMissingExternalNetworksPayload(data: unknown): MissingExternalNetw
|
||||
autoCreateEnabled: data.autoCreateEnabled,
|
||||
stackName: data.stackName,
|
||||
networks: data.networks as MissingExternalNetworksPayload['networks'],
|
||||
renderError:
|
||||
typeof data.renderError === 'string' && data.renderError.length > 0
|
||||
? data.renderError
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -339,7 +343,7 @@ function missingExternalBlocksDeploy(
|
||||
envelope: MissingExternalNetworksEnvelope,
|
||||
): string | null {
|
||||
if (envelope.status === 'render_unavailable') {
|
||||
return 'Sencho could not render this stack\'s Compose model to check external networks.';
|
||||
return envelope.renderError || 'Sencho could not render this stack\'s Compose model to check external networks.';
|
||||
}
|
||||
if (envelope.status === 'runtime_unavailable' && envelope.declaredExternalCount > 0) {
|
||||
return 'Sencho could not read Docker networking state to check external networks.';
|
||||
|
||||
@@ -35,6 +35,7 @@ export type MissingExternalNetworksPayload = {
|
||||
autoCreateEnabled: boolean;
|
||||
stackName: string;
|
||||
networks: MissingExternalNetworkDto[];
|
||||
renderError?: string;
|
||||
};
|
||||
|
||||
interface MissingExternalNetworksDialogProps {
|
||||
|
||||
Reference in New Issue
Block a user