Merge branch 'feature/require-project-name' into 'main'

fix(monitoring): remove postgres-ai-monitoring default; require project name

Closes #249

See merge request postgres-ai/postgresai!333

Author: Sarumyan

cli/bin/postgres-ai.ts

@@ -2443,9 +2443,44 @@ interface MonitoringRegistration {
  *
  * Never throws — registration is best-effort; returns null on failure.
  */
+
+/**
+ * Classify how `mon local-install` should register the monitoring instance,
+ * given the raw `--project` value and the resolved instance id.
+ *
+ * - With an instance id: ADOPT the provisioned instance. No project name is
+ *   required (the platform returns the real project); a provided name is
+ *   normalized and carried through for messaging/fallback.
+ * - No instance id and no project name: ERROR. The hardcoded
+ *   "postgres-ai-monitoring" default was removed, and a nameless legacy
+ *   self-registration is rejected by v1.monitoring_instance_register (PT400).
+ * - No instance id but a project name: legacy SELF-REGISTER.
+ *
+ * Pure (no I/O) so the decision is unit-testable independently of the large
+ * install command. `projectName` is the trimmed value, or undefined when empty.
+ */
+type MonRegistrationPlan =
+  | { kind: "adopt"; projectName: string | undefined }
+  | { kind: "self-register"; projectName: string }
+  | { kind: "error-missing-project"; projectName: undefined };
+
+function planMonitoringRegistration(args: {
+  project?: string;
+  instanceId?: string;
+}): MonRegistrationPlan {
+  const projectName = args.project?.trim() || undefined;
+  if (args.instanceId) {
+    return { kind: "adopt", projectName };
+  }
+  if (!projectName) {
+    return { kind: "error-missing-project", projectName: undefined };
+  }
+  return { kind: "self-register", projectName };
+}
+
 async function registerMonitoringInstance(
   apiKey: string,
-  projectName: string,
+  projectName: string | undefined,
   opts?: { apiBaseUrl?: string; debug?: boolean; instanceId?: string; retries?: number; retryDelayMs?: number }
 ): Promise<MonitoringRegistration | null> {
   const { apiBaseUrl } = resolveBaseUrls(opts);
@@ -2457,16 +2492,25 @@ async function registerMonitoringInstance(
   // moment to recover; skipped before the first attempt. Tests pass 0.
   const retryDelayMs = opts?.retryDelayMs ?? 400;
 
+  // Omit project_name entirely when empty: the adopt path (instance_id present)
+  // relies on the platform returning the real project, and a nameless legacy
+  // self-registration is rejected by v1.monitoring_instance_register (PT400).
+  const hasProjectName = !!(projectName && projectName.trim());
+
   if (debug) {
     console.error(`\nDebug: Registering monitoring instance...`);
     console.error(`Debug: POST ${url}`);
-    console.error(`Debug: project_name=${projectName}${instanceId ? ` instance_id=${instanceId}` : ""}`);
+    console.error(
+      `Debug: ${hasProjectName ? `project_name=${projectName}` : "project_name=(omitted)"}${instanceId ? ` instance_id=${instanceId}` : ""}`
+    );
   }
 
   const requestBody: Record<string, string> = {
     api_token: apiKey,
-    project_name: projectName,
   };
+  if (hasProjectName) {
+    requestBody.project_name = projectName as string;
+  }
   if (instanceId) {
     requestBody.instance_id = instanceId;
   }
@@ -3137,8 +3181,11 @@ mon
     // and persisted; the legacy self-registration stays fire-and-forget
     // (issue platform-all#311).
     if (apiKey && !opts.demo) {
-      const projectName = opts.project || "postgres-ai-monitoring";
       const instanceId = opts.instanceId || process.env.PGAI_INSTANCE_ID;
+      const plan = planMonitoringRegistration({ project: opts.project, instanceId });
+      const projectName = plan.projectName;
+      // `instanceId` truthy ⟺ plan.kind === "adopt"; branch on it directly so
+      // TypeScript narrows instanceId to a defined string in the adopt path.
       if (instanceId) {
         const reg = await registerMonitoringInstance(apiKey, projectName, {
           apiBaseUrl: globalOpts.apiBaseUrl,
@@ -3160,11 +3207,17 @@ mon
           // Request succeeded but carried no usable project field — don't claim
           // adoption, but don't report a hard failure either (no re-run needed).
           console.error(
-            `⚠ Adopted provisioned instance ${instanceId} but the platform returned no project — reports will use project '${projectName}'`
+            `⚠ Adopted provisioned instance ${instanceId} but the platform returned no project` +
+              (projectName
+                ? ` — reports will use project '${projectName}'`
+                : ` — reports will have no project until 'postgresai mon local-install' is re-run with --project <name>`)
           );
         } else {
           console.error(
-            `⚠ Could not adopt provisioned instance ${instanceId} — reports will use project '${projectName}' until 'postgresai mon local-install' is re-run`
+            `⚠ Could not adopt provisioned instance ${instanceId}` +
+              (projectName
+                ? ` — reports will use project '${projectName}' until 'postgresai mon local-install' is re-run`
+                : ` — reports will have no project until 'postgresai mon local-install' is re-run with --project <name>`)
           );
         }
 
@@ -3188,6 +3241,17 @@ mon
             `⚠ AAS auto-collection not registered (${aas.reason}); it can be enabled later by re-running 'postgresai mon local-install'\n`
           );
         }
+      } else if (plan.kind === "error-missing-project") {
+        // Legacy self-registration (no --instance-id) now requires a project
+        // name: the hardcoded "postgres-ai-monitoring" default was removed, and
+        // v1.monitoring_instance_register raises PT400 for a nameless legacy
+        // registration. Console-provisioned installs should adopt with
+        // --instance-id instead.
+        console.error(
+          "✗ A project name is required for self-registration (the 'postgres-ai-monitoring' default was removed). " +
+            "Re-run with --project <name>, or adopt a console-provisioned instance with --instance-id <uuid>."
+        );
+        process.exitCode = 1;
       } else {
         void registerMonitoringInstance(apiKey, projectName, {
           apiBaseUrl: globalOpts.apiBaseUrl,
@@ -5488,3 +5552,4 @@ if (import.meta.main) {
 // same functions used by the `mon` commands).
 export { refreshBundledComposeIfStale, readDeployedTag, isValidComposeYaml };
 export { registerMonitoringInstance, resolveAdoptedProject, type MonitoringRegistration };
+export { planMonitoringRegistration, type MonRegistrationPlan };

cli/test/monitoring.test.ts

@@ -19,6 +19,7 @@ import {
 import {
   registerMonitoringInstance,
   resolveAdoptedProject,
+  planMonitoringRegistration,
 } from "../bin/postgres-ai";
 
 /**
@@ -241,12 +242,13 @@ describe("registerMonitoringInstance", () => {
   });
 
   // Issue platform-all#311: console-provisioned installs pass instance_id so
-  // the platform adopts the provisioned instance instead of self-registering
-  // a duplicate under an auto-created "postgres-ai-monitoring" project.
+  // the platform adopts the provisioned instance and returns its real project
+  // — the CLI sends NO project_name on the adopt path (the hardcoded
+  // "postgres-ai-monitoring" default was removed).
   test("includes instance_id in body when adopting a provisioned instance", async () => {
     const instanceId = "019eb300-3f2a-7a75-b54d-4f10572b25b8";
 
-    await registerMonitoringInstance("key", "postgres-ai-monitoring", opts({ instanceId }));
+    await registerMonitoringInstance("key", undefined, opts({ instanceId }));
 
     const body = JSON.parse(fetchCalls[0].options.body as string);
     expect(body.instance_id).toBe(instanceId);
@@ -255,13 +257,31 @@ describe("registerMonitoringInstance", () => {
     expect(headers["instance-id"]).toBeUndefined();
   });
 
+  test("omits project_name from the body when undefined (adopt path)", async () => {
+    await registerMonitoringInstance("key", undefined, opts({ instanceId: "i" }));
+
+    const body = JSON.parse(fetchCalls[0].options.body as string);
+    // The adopt path sends no name; the platform returns the real project.
+    expect("project_name" in body).toBe(false);
+    expect(body.api_token).toBe("key");
+  });
+
+  test("omits project_name from the body when empty/whitespace", async () => {
+    await registerMonitoringInstance("key", "   ", opts({ instanceId: "i" }));
+
+    const body = JSON.parse(fetchCalls[0].options.body as string);
+    expect("project_name" in body).toBe(false);
+  });
+
   test("omits instance_id from the body for legacy self-registration", async () => {
     await registerMonitoringInstance("key", "my-project", opts());
 
     const body = JSON.parse(fetchCalls[0].options.body as string);
     // PostgREST matches the 3-arg function via its default — the key must be
     // ABSENT (not null) so legacy CLIs and the new one hit the same overload.
     expect("instance_id" in body).toBe(false);
+    // A real project name still rides in the body for legacy registration.
+    expect(body.project_name).toBe("my-project");
   });
 
   test("a 200 with {project_id, project_name} returns a populated result", async () => {
@@ -323,6 +343,37 @@ describe("registerMonitoringInstance", () => {
   });
 });
 
+describe("planMonitoringRegistration — mon local-install registration decision", () => {
+  test("instance id present → adopt, no project name required", () => {
+    const plan = planMonitoringRegistration({ instanceId: "i-123" });
+    expect(plan.kind).toBe("adopt");
+    expect(plan.projectName).toBeUndefined();
+  });
+
+  test("instance id present + project → adopt, carries the trimmed name", () => {
+    const plan = planMonitoringRegistration({ instanceId: "i-123", project: "  prod-db  " });
+    expect(plan.kind).toBe("adopt");
+    expect(plan.projectName).toBe("prod-db");
+  });
+
+  test("no instance id + no project → error-missing-project (exit 1 path)", () => {
+    const plan = planMonitoringRegistration({});
+    expect(plan.kind).toBe("error-missing-project");
+    expect(plan.projectName).toBeUndefined();
+  });
+
+  test("no instance id + empty/whitespace project → error-missing-project", () => {
+    expect(planMonitoringRegistration({ project: "" }).kind).toBe("error-missing-project");
+    expect(planMonitoringRegistration({ project: "   " }).kind).toBe("error-missing-project");
+  });
+
+  test("no instance id + real project → legacy self-register with the trimmed name", () => {
+    const plan = planMonitoringRegistration({ project: "  my-project  " });
+    expect(plan.kind).toBe("self-register");
+    expect(plan.projectName).toBe("my-project");
+  });
+});
+
 describe("resolveAdoptedProject — what gets persisted to .pgwatch-config", () => {
   test("prefers the numeric project_id over the name (survives renames)", () => {
     expect(resolveAdoptedProject({ projectId: 42, projectName: "prod-db" })).toBe("42");

config/scripts/postgres-reports.sh

@@ -11,7 +11,8 @@ use_current_time="${USE_CURRENT_TIME:-false}"
 
 pgwatch_config_path="${REPORTER_PGWATCH_CONFIG_PATH:-/app/.pgwatch-config}"
 api_url="${REPORTER_API_URL:-https://postgres.ai/api/general}"
-# Project name: env var takes priority, then config file, then default
+# Project name: env var takes priority, then config file. No default — uploads
+# require a project name (the hardcoded "postgres-ai-monitoring" default was removed).
 project_name="${REPORTER_PROJECT_NAME:-}"
 
 sleep_seconds() {
@@ -60,12 +61,11 @@ read_project_name() {
   return 1
 }
 
-# Resolve project name: env var > config file > default
+# Resolve project name: env var > config file. No default — uploads require a
+# project name (the hardcoded "postgres-ai-monitoring" default was removed).
 if [[ -z "${project_name}" ]]; then
   if config_project_name="$(read_project_name)"; then
     project_name="${config_project_name}"
-  else
-    project_name="postgres-ai-monitoring"
   fi
 fi
 
@@ -82,14 +82,20 @@ while true; do
   fi
 
   if api_key="$(read_api_key)"; then
-    echo "postgres-reports: generating reports (upload enabled) -> ${output_path}"
-    python -m reporter.postgres_reports \
-      --prometheus-url "${prometheus_url}" \
-      --output "${output_path}" \
-      --api-url "${api_url}" \
-      --project-name "${project_name}" \
-      --token "${api_key}" \
-      ${use_current_time_arg}
+    if [[ -z "${project_name}" ]]; then
+      # Upload requires a project name; there is no default. Skip this cycle's
+      # upload rather than send an empty --project-name.
+      echo "postgres-reports: ERROR project name is required for upload — set REPORTER_PROJECT_NAME or project_name in .pgwatch-config; skipping upload this cycle" >&2
+    else
+      echo "postgres-reports: generating reports (upload enabled) -> ${output_path}"
+      python -m reporter.postgres_reports \
+        --prometheus-url "${prometheus_url}" \
+        --output "${output_path}" \
+        --api-url "${api_url}" \
+        --project-name "${project_name}" \
+        --token "${api_key}" \
+        ${use_current_time_arg}
+    fi
   else
     echo "postgres-reports: generating reports (no upload) -> ${output_path}"
     python -m reporter.postgres_reports \

postgres_ai_helm/templates/reporter-cronjob.yaml

@@ -109,14 +109,28 @@ spec:
                 - |
                   set -eu
                   if [ -n "${API_KEY:-}" ]; then
+                    # Uploads require a project name; there is no default (the
+                    # hardcoded "postgres-ai-monitoring" default was removed).
+                    # Enforce it at runtime, not via Helm `required`: whether
+                    # uploads happen is decided here at runtime by the presence
+                    # of API_KEY (an optional secret key), so Helm cannot know it
+                    # at render time. A render-time `required` would also break
+                    # `helm template`/`lint` and the no-upload path. Fail loudly.
+                    # REPORTER_PROJECT comes from the env (see below), never
+                    # inlined into this script, so the value can't break out of
+                    # the shell command.
+                    if [ -z "${REPORTER_PROJECT:-}" ]; then
+                      echo "reporter.project is required for uploads (the 'postgres-ai-monitoring' default was removed); set reporter.project" >&2
+                      exit 1
+                    fi
                     exec python postgres_reports.py \
                       --prometheus-url "$PROMETHEUS_URL" \
                       --postgres-sink-url "postgresql://{{ $root.Values.sinkPostgres.user }}:${POSTGRES_PASSWORD}@{{ include "postgres-ai-monitoring.fullname" $root }}-sink-postgres:5432/{{ $root.Values.sinkPostgres.database }}" \
                       --cluster "$CLUSTER" \
                       --node-name "$NODE_NAME" \
                       --output /app/reports/report.json \
                       --api-url "{{ $root.Values.reporter.apiUrl | default "https://postgres.ai/api/general" }}" \
-                      --project "{{ $root.Values.reporter.project | default "postgres-ai-monitoring" }}" \
+                      --project "$REPORTER_PROJECT" \
                       --token "$API_KEY"
                   else
                     exec python postgres_reports.py \
@@ -137,6 +151,8 @@ spec:
                       key: postgres-password
                 - name: CLUSTER
                   value: {{ $clusterName | default "k8s-cluster" | quote }}
+                - name: REPORTER_PROJECT
+                  value: {{ $root.Values.reporter.project | quote }}
                 - name: NODE_NAME
 {{- if $nodeName }}
                   value: {{ $nodeName | quote }}
@@ -153,7 +169,7 @@ spec:
                       key: pgai-api-key
                       optional: true
                 {{- end }}
-                
+
                 {{- if $root.Values.victoriaMetrics.auth.enabled }}
                 - name: VM_AUTH_USERNAME
                   value: {{ $root.Values.victoriaMetrics.auth.username | quote }}

postgres_ai_helm/values.yaml

@@ -117,6 +117,12 @@ reporter:
   # clusterName/nodeName per database in monitoredDatabases.
   clusterName: ""
   nodeName: ""
+  # REQUIRED for uploads: the console project name reports are uploaded to.
+  # There is no default (the hardcoded "postgres-ai-monitoring" was removed).
+  # If uploads are enabled (an API key is present) but this is empty, the
+  # reporter cronjob exits with an error at runtime. Chart rendering and the
+  # no-upload path are unaffected by an empty value.
+  project: ""
   apiUrl: https://postgres.ai/api/general
   successfulJobsHistoryLimit: 3
   failedJobsHistoryLimit: 3

reporter/postgres_reports.py

@@ -5129,8 +5129,8 @@ def main():
                         help='Output file (default: stdout)')
     parser.add_argument('--api-url', default='https://postgres.ai/api/general')
     parser.add_argument('--token', default='')
-    parser.add_argument('--project-name', default='project-name',
-                        help='Project name for API upload (default: project-name)')
+    parser.add_argument('--project-name', default=None,
+                        help='Project name for API upload (required when uploading)')
     parser.add_argument('--epoch', default='1')
     parser.add_argument('--no-upload', action='store_true', default=False,
                         help='Do not upload reports to the API')
@@ -5186,12 +5186,18 @@ def main():
                 # Generate all reports for this cluster
                 report_id = None
                 if not args.no_upload:
-                    # Use cluster name as project name if not specified
-                    project_name = args.project_name if args.project_name != 'project-name' else cluster
-                    report_id = generator.create_report(args.api_url, args.token, project_name, args.epoch)
-                    # If report creation failed, disable uploads for this cluster
-                    if report_id is None:
-                        logger.info(f"Skipping API uploads for cluster {cluster}")
+                    project_name = args.project_name
+                    if not project_name:
+                        # No default (cluster-name fallback removed): a project
+                        # name is required to upload. Skip the upload for this
+                        # cluster; report_id stays None so all downstream
+                        # uploads are disabled too.
+                        logger.error("project name is required for upload (--project-name); skipping upload")
+                    else:
+                        report_id = generator.create_report(args.api_url, args.token, project_name, args.epoch)
+                        # If report creation failed, disable uploads for this cluster
+                        if report_id is None:
+                            logger.info(f"Skipping API uploads for cluster {cluster}")
 
                 reports = generator.generate_all_reports(cluster, args.node_name, combine_nodes)
 
@@ -5310,10 +5316,16 @@ def main():
                         json.dump(report, f, indent=2)
                     logger.info(f"Report written to {output_filename}")
                     if not args.no_upload:
-                        project_name = args.project_name if args.project_name != 'project-name' else cluster
-                        report_id = generator.create_report(args.api_url, args.token, project_name, args.epoch)
-                        if report_id:
-                            generator.upload_report_file(args.api_url, args.token, report_id, output_filename)
+                        project_name = args.project_name
+                        if not project_name:
+                            # No default (cluster-name fallback removed): a
+                            # project name is required to upload. Skip the
+                            # upload for this report.
+                            logger.error("project name is required for upload (--project-name); skipping upload")
+                        else:
+                            report_id = generator.create_report(args.api_url, args.token, project_name, args.epoch)
+                            if report_id:
+                                generator.upload_report_file(args.api_url, args.token, report_id, output_filename)
 
             # Free memory after processing each cluster
             logger.info(f"Freeing memory after processing cluster {cluster}...")