Skip to content

Commit ae1d192

Browse files
SarumyanSarumyan
committed
Merge branch 'fix/188-cli-bugs' into 'main'
fix(cli): IPv6 default network, instances.yml round-trip, sslmode in test Closes #189 See merge request postgres-ai/postgresai!260
2 parents 6202803 + 61d310b commit ae1d192

4 files changed

Lines changed: 624 additions & 124 deletions

File tree

‎cli/bin/postgres-ai.ts‎

Lines changed: 88 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ import { REPORT_GENERATORS, CHECK_INFO, generateAllReports } from "../lib/checku
2626
import { getCheckupEntry } from "../lib/checkup-dictionary";
2727
import { createCheckupReport, uploadCheckupReportJson, convertCheckupReportJsonToMarkdown, RpcError, formatRpcErrorForDisplay, withRetry } from "../lib/checkup-api";
2828
import { generateCheckSummary } from "../lib/checkup-summary";
29+
import {
30+
type Instance,
31+
InstancesParseError,
32+
loadInstances,
33+
buildInstance,
34+
addInstanceToFile,
35+
removeInstanceFromFile,
36+
buildClientConfig,
37+
sslOptionFromConnString,
38+
warnIfLaxSslmode,
39+
} from "../lib/instances";
2940

3041
// Node.js version check - require Node 18+
3142
// Node 14 reached EOL in April 2023, Node 16 in September 2023.
@@ -419,19 +430,6 @@ interface ConfigResult {
419430
apiKey: string;
420431
}
421432

422-
/**
423-
* Instance configuration
424-
*/
425-
interface Instance {
426-
name: string;
427-
conn_str?: string;
428-
preset_metrics?: string;
429-
custom_metrics?: any;
430-
is_enabled?: boolean;
431-
group?: string;
432-
custom_tags?: Record<string, any>;
433-
}
434-
435433
/**
436434
* Path resolution result
437435
*/
@@ -2302,6 +2300,18 @@ const mon = program.command("mon").description("monitoring services management")
23022300
mon
23032301
.command("local-install")
23042302
.description("install local monitoring stack (generate config, start services)")
2303+
.addHelpText(
2304+
"after",
2305+
[
2306+
"",
2307+
"Networking:",
2308+
" Compose enables IPv6 on the project's default network so containers can",
2309+
" reach IPv6-only databases (e.g. Supabase free-tier db.<ref>.supabase.co).",
2310+
" Override on hosts whose Docker daemon cannot create an IPv6 network:",
2311+
" PGAI_ENABLE_IPV6=false (accepted: true|false|yes|no, lowercase)",
2312+
"",
2313+
].join("\n"),
2314+
)
23052315
.option("--demo", "demo mode with sample database", false)
23062316
.option("--api-key <key>", "Postgres AI API key for automated report uploads")
23072317
.option("--db-url <url>", "PostgreSQL connection URL to monitor")
@@ -2487,16 +2497,16 @@ mon
24872497
const db = m[5];
24882498
const instanceName = `${host}-${db}`.replace(/[^a-zA-Z0-9-]/g, "-");
24892499

2490-
const body = `- name: ${instanceName}\n conn_str: ${connStr}\n preset_metrics: full\n custom_metrics:\n is_enabled: true\n group: default\n custom_tags:\n env: production\n cluster: default\n node_name: ${instanceName}\n sink_type: ~sink_type~\n`;
2491-
fs.appendFileSync(instancesPath, body, "utf8");
2500+
addInstanceToFile(instancesPath, buildInstance(instanceName, connStr));
24922501
console.log(`✓ Monitoring target '${instanceName}' added\n`);
24932502

24942503
// Test connection
24952504
console.log("Testing connection to the added instance...");
24962505
{
24972506
let testClient: InstanceType<typeof Client> | null = null;
24982507
try {
2499-
testClient = new Client({ connectionString: connStr, connectionTimeoutMillis: 10000 });
2508+
warnIfLaxSslmode(connStr);
2509+
testClient = new Client(buildClientConfig(connStr, { connectionTimeoutMillis: 10000 }));
25002510
await testClient.connect();
25012511
const result = await testClient.query("select version();");
25022512
console.log("✓ Connection successful");
@@ -2535,16 +2545,16 @@ mon
25352545
const db = m[5];
25362546
const instanceName = `${host}-${db}`.replace(/[^a-zA-Z0-9-]/g, "-");
25372547

2538-
const body = `- name: ${instanceName}\n conn_str: ${connStr}\n preset_metrics: full\n custom_metrics:\n is_enabled: true\n group: default\n custom_tags:\n env: production\n cluster: default\n node_name: ${instanceName}\n sink_type: ~sink_type~\n`;
2539-
fs.appendFileSync(instancesPath, body, "utf8");
2548+
addInstanceToFile(instancesPath, buildInstance(instanceName, connStr));
25402549
console.log(`✓ Monitoring target '${instanceName}' added\n`);
25412550

25422551
// Test connection
25432552
console.log("Testing connection to the added instance...");
25442553
{
25452554
let testClient: InstanceType<typeof Client> | null = null;
25462555
try {
2547-
testClient = new Client({ connectionString: connStr, connectionTimeoutMillis: 10000 });
2556+
warnIfLaxSslmode(connStr);
2557+
testClient = new Client(buildClientConfig(connStr, { connectionTimeoutMillis: 10000 }));
25482558
await testClient.connect();
25492559
const result = await testClient.query("select version();");
25502560
console.log("✓ Connection successful");
@@ -3173,42 +3183,32 @@ targets
31733183
return;
31743184
}
31753185

3186+
let instances: Instance[];
31763187
try {
3177-
const content = fs.readFileSync(instancesPath, "utf8");
3178-
const instances = yaml.load(content) as Instance[] | null;
3179-
3180-
if (!instances || !Array.isArray(instances) || instances.length === 0) {
3181-
console.log("No monitoring targets configured");
3182-
console.log("");
3183-
console.log("To add a monitoring target:");
3184-
console.log(" postgres-ai mon targets add <connection-string> <name>");
3185-
console.log("");
3186-
console.log("Example:");
3187-
console.log(" postgres-ai mon targets add 'postgresql://user:pass@host:5432/db' my-db");
3188-
return;
3189-
}
3190-
3191-
// Filter out disabled instances (e.g., demo placeholders)
3192-
const filtered = instances.filter((inst) => inst.name && inst.is_enabled !== false);
3193-
3194-
if (filtered.length === 0) {
3195-
console.log("No monitoring targets configured");
3196-
console.log("");
3197-
console.log("To add a monitoring target:");
3198-
console.log(" postgres-ai mon targets add <connection-string> <name>");
3199-
console.log("");
3200-
console.log("Example:");
3201-
console.log(" postgres-ai mon targets add 'postgresql://user:pass@host:5432/db' my-db");
3202-
return;
3203-
}
3204-
3205-
for (const inst of filtered) {
3206-
console.log(`Target: ${inst.name}`);
3207-
}
3188+
instances = loadInstances(instancesPath);
32083189
} catch (err) {
32093190
const message = err instanceof Error ? err.message : String(err);
32103191
console.error(`Error parsing instances.yml: ${message}`);
32113192
process.exitCode = 1;
3193+
return;
3194+
}
3195+
3196+
// Filter out disabled instances (e.g., demo placeholders)
3197+
const filtered = instances.filter((inst) => inst.name && inst.is_enabled !== false);
3198+
3199+
if (filtered.length === 0) {
3200+
console.log("No monitoring targets configured");
3201+
console.log("");
3202+
console.log("To add a monitoring target:");
3203+
console.log(" postgres-ai mon targets add <connection-string> <name>");
3204+
console.log("");
3205+
console.log("Example:");
3206+
console.log(" postgres-ai mon targets add 'postgresql://user:pass@host:5432/db' my-db");
3207+
return;
3208+
}
3209+
3210+
for (const inst of filtered) {
3211+
console.log(`Target: ${inst.name}`);
32123212
}
32133213
});
32143214
targets
@@ -3231,40 +3231,17 @@ targets
32313231
const db = m[5];
32323232
const instanceName = name && name.trim() ? name.trim() : `${host}-${db}`.replace(/[^a-zA-Z0-9-]/g, "-");
32333233

3234-
// Check if instance already exists
32353234
try {
3236-
if (fs.existsSync(file) && !fs.lstatSync(file).isDirectory()) {
3237-
const content = fs.readFileSync(file, "utf8");
3238-
const instances = yaml.load(content) as Instance[] | null || [];
3239-
if (Array.isArray(instances)) {
3240-
const exists = instances.some((inst) => inst.name === instanceName);
3241-
if (exists) {
3242-
console.error(`Monitoring target '${instanceName}' already exists`);
3243-
process.exitCode = 1;
3244-
return;
3245-
}
3246-
}
3247-
}
3235+
addInstanceToFile(file, buildInstance(instanceName, connStr));
3236+
console.log(`Monitoring target '${instanceName}' added`);
32483237
} catch (err) {
3249-
// If YAML parsing fails, fall back to simple check
3250-
const isFile = fs.existsSync(file) && !fs.lstatSync(file).isDirectory();
3251-
const content = isFile ? fs.readFileSync(file, "utf8") : "";
3252-
const escapedName = instanceName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
3253-
if (new RegExp(`^- name: ${escapedName}$`, "m").test(content)) {
3254-
console.error(`Monitoring target '${instanceName}' already exists`);
3255-
process.exitCode = 1;
3256-
return;
3257-
}
3258-
}
3259-
3260-
// Add new instance — if instances.yml is a directory (Docker artifact), replace it with a file
3261-
if (fs.existsSync(file) && fs.lstatSync(file).isDirectory()) {
3262-
fs.rmSync(file, { recursive: true, force: true });
3238+
// Surface InstancesParseError as-is so we don't silently overwrite a
3239+
// corrupted file (which could discard several targets, including the
3240+
// credentials in their conn_str values).
3241+
const message = err instanceof Error ? err.message : String(err);
3242+
console.error(message);
3243+
process.exitCode = 1;
32633244
}
3264-
const body = `- name: ${instanceName}\n conn_str: ${connStr}\n preset_metrics: full\n custom_metrics:\n is_enabled: true\n group: default\n custom_tags:\n env: production\n cluster: default\n node_name: ${instanceName}\n sink_type: ~sink_type~\n`;
3265-
const content = fs.existsSync(file) ? fs.readFileSync(file, "utf8") : "";
3266-
fs.appendFileSync(file, (content && !/\n$/.test(content) ? "\n" : "") + body, "utf8");
3267-
console.log(`Monitoring target '${instanceName}' added`);
32683245
});
32693246
targets
32703247
.command("remove <name>")
@@ -3278,24 +3255,12 @@ targets
32783255
}
32793256

32803257
try {
3281-
const content = fs.readFileSync(file, "utf8");
3282-
const instances = yaml.load(content) as Instance[] | null;
3283-
3284-
if (!instances || !Array.isArray(instances)) {
3285-
console.error("Invalid instances.yml format");
3286-
process.exitCode = 1;
3287-
return;
3288-
}
3289-
3290-
const filtered = instances.filter((inst) => inst.name !== name);
3291-
3292-
if (filtered.length === instances.length) {
3258+
const removed = removeInstanceFromFile(file, name);
3259+
if (!removed) {
32933260
console.error(`Monitoring target '${name}' not found`);
32943261
process.exitCode = 1;
32953262
return;
32963263
}
3297-
3298-
fs.writeFileSync(file, yaml.dump(filtered), "utf8");
32993264
console.log(`Monitoring target '${name}' removed`);
33003265
} catch (err) {
33013266
const message = err instanceof Error ? err.message : String(err);
@@ -3314,35 +3279,35 @@ targets
33143279
return;
33153280
}
33163281

3282+
let instances: Instance[];
33173283
try {
3318-
const content = fs.readFileSync(instancesPath, "utf8");
3319-
const instances = yaml.load(content) as Instance[] | null;
3320-
3321-
if (!instances || !Array.isArray(instances)) {
3322-
console.error("Invalid instances.yml format");
3323-
process.exitCode = 1;
3324-
return;
3325-
}
3326-
3327-
const instance = instances.find((inst) => inst.name === name);
3284+
instances = loadInstances(instancesPath);
3285+
} catch (err) {
3286+
const message = err instanceof Error ? err.message : String(err);
3287+
console.error(`Error parsing instances.yml: ${message}`);
3288+
process.exitCode = 1;
3289+
return;
3290+
}
3291+
const instance = instances.find((inst) => inst.name === name);
33283292

3329-
if (!instance) {
3330-
console.error(`Monitoring target '${name}' not found`);
3331-
process.exitCode = 1;
3332-
return;
3333-
}
3293+
if (!instance) {
3294+
console.error(`Monitoring target '${name}' not found`);
3295+
process.exitCode = 1;
3296+
return;
3297+
}
33343298

3335-
if (!instance.conn_str) {
3336-
console.error(`Connection string not found for monitoring target '${name}'`);
3337-
process.exitCode = 1;
3338-
return;
3339-
}
3299+
if (!instance.conn_str) {
3300+
console.error(`Connection string not found for monitoring target '${name}'`);
3301+
process.exitCode = 1;
3302+
return;
3303+
}
33403304

3341-
console.log(`Testing connection to monitoring target '${name}'...`);
3305+
console.log(`Testing connection to monitoring target '${name}'...`);
33423306

3343-
// Use native pg client instead of requiring psql to be installed
3344-
const client = new Client({ connectionString: instance.conn_str, connectionTimeoutMillis: 10000 });
3307+
warnIfLaxSslmode(instance.conn_str);
3308+
const client = new Client(buildClientConfig(instance.conn_str, { connectionTimeoutMillis: 10000 }));
33453309

3310+
try {
33463311
try {
33473312
await client.connect();
33483313
const result = await client.query('select version();');
@@ -3376,17 +3341,17 @@ auth
33763341
process.exitCode = 1;
33773342
return;
33783343
}
3379-
3344+
33803345
// Read existing config to check for defaultProject before updating
33813346
const existingConfig = config.readConfig();
33823347
const existingProject = existingConfig.defaultProject;
3383-
3348+
33843349
config.writeConfig({ apiKey: trimmedKey });
33853350
// When API key is set directly, only clear orgId (org selection may differ).
33863351
// Preserve defaultProject to avoid orphaning historical reports.
33873352
// If the new key lacks access to the project, upload will fail with a clear error.
33883353
config.deleteConfigKeys(["orgId"]);
3389-
3354+
33903355
console.log(`API key saved to ${config.getConfigPath()}`);
33913356
if (existingProject) {
33923357
console.log(`Note: Your default project "${existingProject}" has been preserved.`);
@@ -3570,13 +3535,13 @@ auth
35703535
const existingOrgId = existingConfig.orgId;
35713536
const existingProject = existingConfig.defaultProject;
35723537
const orgChanged = existingOrgId && existingOrgId !== orgId;
3573-
3538+
35743539
config.writeConfig({
35753540
apiKey: apiToken,
35763541
baseUrl: apiBaseUrl,
35773542
orgId: orgId,
35783543
});
3579-
3544+
35803545
// Only clear defaultProject if org actually changed
35813546
if (orgChanged && existingProject) {
35823547
config.deleteConfigKeys(["defaultProject"]);
@@ -4971,4 +4936,3 @@ mcp
49714936
program.parseAsync(process.argv).finally(() => {
49724937
closeReadline();
49734938
});
4974-

0 commit comments

Comments
 (0)