Skip to content

fix: isolate maintenance handler test filesystem state #1703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
toddhainsworth wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: isolate maintenance handler test filesystem state #1703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions .changeset/isolate-maintenance-test-filesystem-state.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@aligent/cdk-graphql-mesh-server": patch
---

Resolve MAINTENANCE_FILE_PATH lazily so the maintenance handler honours the runtime-configured path; fixes flaky maintenance handler tests via per-suite filesystem isolation.
27 changes: 27 additions & 0 deletions packages/constructs/graphql-mesh-server/assets/handlers/maintenance/lib/file.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { mkdtempSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { getFilePath, getMaintenanceFile, setWhitelist } from "./file";

describe("maintenance file storage", () => {
it("resolves the file path under the current MAINTENANCE_FILE_PATH", () => {
const dir = mkdtempSync(join(tmpdir(), "maint-"));
process.env.MAINTENANCE_FILE_PATH = dir;

expect(getFilePath().startsWith(dir)).toBe(true);
});

it("keeps state in separate directories isolated from each other", () => {
const dirA = mkdtempSync(join(tmpdir(), "maint-a-"));
const dirB = mkdtempSync(join(tmpdir(), "maint-b-"));

process.env.MAINTENANCE_FILE_PATH = dirA;
setWhitelist(["10.0.0.1"]);

process.env.MAINTENANCE_FILE_PATH = dirB;
expect(getMaintenanceFile().whitelist).toEqual([]);

process.env.MAINTENANCE_FILE_PATH = dirA;
expect(getMaintenanceFile().whitelist).toEqual(["10.0.0.1"]);
});
});
33 changes: 19 additions & 14 deletions packages/constructs/graphql-mesh-server/assets/handlers/maintenance/lib/file.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,36 @@ const IP_REGEX =
const IP_V6_REGEX =
/(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))/;

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const MAINTENANCE_FILE_PATH = process.env.MAINTENANCE_FILE_PATH!;
const FILE_NAME = "maintenance";
const PATHS = [
`${MAINTENANCE_FILE_PATH}/${FILE_NAME}.disabled`,
`${MAINTENANCE_FILE_PATH}/${FILE_NAME}.enabled`,
];

// Resolved lazily so each caller honours the current MAINTENANCE_FILE_PATH
// rather than a value captured at module load.
const getPaths = (): [string, string] => {
const basePath = process.env.MAINTENANCE_FILE_PATH;
if (!basePath) throw new Error("Maintenance File path is missing.");

return [
`${basePath}/${FILE_NAME}.disabled`,
`${basePath}/${FILE_NAME}.enabled`,
];
};

export interface MaintenanceFile {
whitelist: Array<string>;
sites: Record<string, boolean>;
}

export const getFilePath = (): string => {
for (const path of PATHS) {
const paths = getPaths();
for (const path of paths) {
if (existsSync(path)) {
return path;
}
}

// If the maintenance file wasn't found, create one
writeFileSync(PATHS[0], JSON.stringify({ whitelist: [], sites: {} }));
return PATHS[0];
writeFileSync(paths[0], JSON.stringify({ whitelist: [], sites: {} }));
return paths[0];
};

export const getMaintenanceFile = (): MaintenanceFile => {
Expand Down Expand Up @@ -63,12 +70,10 @@ export const updateMaintenanceStatus = (sites: Record<string, boolean>) => {
};

export const toggleMaintenanceStatus = (status: boolean) => {
const desiredStatus = status ? "enabled" : "disabled";
const [disabledPath, enabledPath] = getPaths();
const target = status ? enabledPath : disabledPath;

renameSync(
getFilePath(),
`${MAINTENANCE_FILE_PATH}/${FILE_NAME}.${desiredStatus}`
);
renameSync(getFilePath(), target);
};

const validateIps = (ipList: Array<string>) => {
Expand Down
32 changes: 16 additions & 16 deletions packages/constructs/graphql-mesh-server/assets/handlers/maintenance/maintenance.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
// handler.test.ts
import { cwd } from "process";
import { handler } from "./maintenance";
import { APIGatewayProxyEvent } from "aws-lambda";
import { updateMaintenanceStatus } from "./lib/file";
import { existsSync, rmSync } from "fs";
import { existsSync, mkdtempSync, rmSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";

// Each suite owns an isolated directory so parallel suites can't race on a
// shared maintenance file (see lib/file.ts MAINTENANCE_FILE_PATH).
const maintenanceDir = mkdtempSync(join(tmpdir(), "maint-maintenance-"));
process.env.MAINTENANCE_FILE_PATH = maintenanceDir;

const createMockEvent = (
method: string,
Expand All @@ -28,11 +34,11 @@ const mockSites = { "example.com": false, "example.com.au": false };
describe("Lambda handler", () => {
beforeEach(() => {
// Clean up any existing maintenance files before each test
if (existsSync(`${cwd()}/maintenance.enabled`)) {
rmSync(`${cwd()}/maintenance.enabled`);
if (existsSync(`${maintenanceDir}/maintenance.enabled`)) {
rmSync(`${maintenanceDir}/maintenance.enabled`);
}
if (existsSync(`${cwd()}/maintenance.disabled`)) {
rmSync(`${cwd()}/maintenance.disabled`);
if (existsSync(`${maintenanceDir}/maintenance.disabled`)) {
rmSync(`${maintenanceDir}/maintenance.disabled`);
}
// Reset the maintenance status before each test to ensure test isolation
updateMaintenanceStatus(mockSites);
Expand All @@ -55,7 +61,7 @@ describe("Lambda handler", () => {

expect(result.statusCode).toBe(200);
expect(JSON.parse(result.body)).toEqual(mockUpdate);
expect(existsSync(`${cwd()}/maintenance.enabled`)).toBe(true);
expect(existsSync(`${maintenanceDir}/maintenance.enabled`)).toBe(true);
});

it("should handle POST'ing a disable all sites", async () => {
Expand All @@ -67,7 +73,7 @@ describe("Lambda handler", () => {
await handler(enableEvent);

// Verify the file is in enabled state as expected
expect(existsSync(`${cwd()}/maintenance.enabled`)).toBe(true);
expect(existsSync(`${maintenanceDir}/maintenance.enabled`)).toBe(true);

// Now test disabling all sites
const mockUpdate = {
Expand All @@ -78,16 +84,10 @@ describe("Lambda handler", () => {

expect(result.statusCode).toBe(200);
expect(JSON.parse(result.body)).toEqual(mockUpdate);
expect(existsSync(`${cwd()}/maintenance.disabled`)).toBe(true);
expect(existsSync(`${maintenanceDir}/maintenance.disabled`)).toBe(true);
});

afterAll(() => {
if (existsSync(`${cwd()}/maintenance.enabled`)) {
rmSync(`${cwd()}/maintenance.enabled`);
}

if (existsSync(`${cwd()}/maintenance.disabled`)) {
rmSync(`${cwd()}/maintenance.disabled`);
}
rmSync(maintenanceDir, { recursive: true, force: true });
});
});
26 changes: 13 additions & 13 deletions packages/constructs/graphql-mesh-server/assets/handlers/maintenance/whitelist.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
// handler.test.ts
import { cwd } from "process";
import { handler } from "./whitelist";
import { APIGatewayProxyEvent } from "aws-lambda";
import { setWhitelist } from "./lib/file";
import { existsSync, rmSync } from "fs";
import { existsSync, mkdtempSync, rmSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";

// Each suite owns an isolated directory so parallel suites can't race on a
// shared maintenance file (see lib/file.ts MAINTENANCE_FILE_PATH).
const maintenanceDir = mkdtempSync(join(tmpdir(), "maint-whitelist-"));
process.env.MAINTENANCE_FILE_PATH = maintenanceDir;

const createMockEvent = (
method: string,
Expand Down Expand Up @@ -43,11 +49,11 @@ const mockAllowlist = [
describe("Lambda handler", () => {
beforeEach(() => {
// Clean up any existing maintenance files before each test
if (existsSync(`${cwd()}/maintenance.enabled`)) {
rmSync(`${cwd()}/maintenance.enabled`);
if (existsSync(`${maintenanceDir}/maintenance.enabled`)) {
rmSync(`${maintenanceDir}/maintenance.enabled`);
}
if (existsSync(`${cwd()}/maintenance.disabled`)) {
rmSync(`${cwd()}/maintenance.disabled`);
if (existsSync(`${maintenanceDir}/maintenance.disabled`)) {
rmSync(`${maintenanceDir}/maintenance.disabled`);
}
// Reset the whitelist before each test to ensure test isolation
setWhitelist(mockAllowlist);
Expand Down Expand Up @@ -101,12 +107,6 @@ describe("Lambda handler", () => {
});

afterAll(() => {
if (existsSync(`${cwd()}/maintenance.enabled`)) {
rmSync(`${cwd()}/maintenance.enabled`);
}

if (existsSync(`${cwd()}/maintenance.disabled`)) {
rmSync(`${cwd()}/maintenance.disabled`);
}
rmSync(maintenanceDir, { recursive: true, force: true });
});
});