Display accurate message for INACCESSIBLE_SCHEMA error

src/dataconnect/client.ts

@@ -5,14 +5,14 @@
 import { logger } from "../logger";
 
 const DATACONNECT_API_VERSION = "v1alpha";
 const dataconnectClient = () =>
   new Client({
     urlPrefix: dataconnectOrigin(),
     apiVersion: DATACONNECT_API_VERSION,
     auth: true,
   });
 
 export async function listLocations(projectId: string): Promise<string[]> {
   const res = await dataconnectClient().get<{
     locations: {
       name: string;
@@ -33,7 +33,7 @@
         const locationServices = await listServices(projectId, l);
         services = services.concat(locationServices);
       } catch (err) {
         logger.debug(`Unable to listServices in ${l}: ${err}`);
       }
     }),
   );
@@ -41,7 +41,7 @@
   return services;
 }
 
 export async function listServices(
   projectId: string,
   locationId: string,
 ): Promise<types.Service[]> {
@@ -51,7 +51,7 @@
   return res.body.services ?? [];
 }
 
 export async function createService(
   projectId: string,
   locationId: string,
   serviceId: string,
@@ -75,13 +75,14 @@
   return pollRes;
 }
 
 export async function deleteService(
   projectId: string,
   locationId: string,
   serviceId: string,
 ): Promise<types.Service> {
+  // NOTE(fredzqm): Don't force delete yet. Backend would leave orphaned resources.
   const op = await dataconnectClient().delete<types.Service>(
-    `projects/${projectId}/locations/${locationId}/services/${serviceId}?force=true`,
+    `projects/${projectId}/locations/${locationId}/services/${serviceId}`,
   );
   const pollRes = await operationPoller.pollOperation<types.Service>({
     apiOrigin: dataconnectOrigin(),
@@ -93,16 +94,16 @@
 
 /** Schema methods */
 
 export async function getSchema(serviceName: string): Promise<types.Schema> {
   const res = await dataconnectClient().get<types.Schema>(
     `${serviceName}/schemas/${types.SCHEMA_ID}`,
   );
   return res.body;
 }
 
 export async function upsertSchema(
   schema: types.Schema,
   validateOnly: boolean = false,
 ): Promise<types.Schema | undefined> {
   const op = await dataconnectClient().patch<types.Schema, types.Schema>(`${schema.name}`, schema, {
     queryParams: {
@@ -122,7 +123,7 @@
 
 /** Connector methods */
 
 export async function getConnector(name: string): Promise<types.Connector> {
   const res = await dataconnectClient().get<types.Connector>(name);
   return res.body;
 }

src/dataconnect/errors.ts

@@ -5,28 +5,27 @@ const PRECONDITION_ERROR_TYPESTRING = "type.googleapis.com/google.rpc.Preconditi
 const INCOMPATIBLE_CONNECTOR_TYPE = "INCOMPATIBLE_CONNECTOR";
 
 export function getIncompatibleSchemaError(err: any): IncompatibleSqlSchemaError | undefined {
-  const original = err.context?.body?.error || err.orignal;
-  if (!original) {
-    // If we can't get the original, rethrow so we don't cover up the original error.
-    throw err;
+  const incompatibles = errorDetails(err, INCOMPATIBLE_SCHEMA_ERROR_TYPESTRING);
+  if (incompatibles.length === 0) {
+    return undefined;
   }
-  const details: any[] = original.details;
-  const incompatibles = details.filter((d) =>
-    d["@type"]?.includes(INCOMPATIBLE_SCHEMA_ERROR_TYPESTRING),
-  );
   // Should never get multiple incompatible schema errors
-  return incompatibles[0];
+  const incompatible = incompatibles[0];
+  // Extract the violation type from the precondition error detail.
+  const preconditionErrs = errorDetails(err, PRECONDITION_ERROR_TYPESTRING);
+  const violationTypes = (incompatible.violationType = preconditionErrs
+    .flatMap((preCondErr) => preCondErr.violations)
+    .flatMap((viol) => viol.type)
+    .filter((type) => type === "INACCESSIBLE_SCHEMA" || type === "INCOMPATIBLE_SCHEMA"));
+  incompatible.violationType = violationTypes[0];
+  return incompatible;
 }
 
 // Note - the backend just includes file name, not the name of the connector resource in the GQLerror extensions.
 // so we don't use this yet. Ideally, we'd just include connector name in the extensions.
 export function getInvalidConnectors(err: any): string[] {
+  const preconditionErrs = errorDetails(err, PRECONDITION_ERROR_TYPESTRING);
   const invalidConns: string[] = [];
-  const original = err.context?.body?.error || err?.orignal;
-  const details: any[] = original?.details;
-  const preconditionErrs = details?.filter((d) =>
-    d["@type"]?.includes(PRECONDITION_ERROR_TYPESTRING),
-  );
   for (const preconditionErr of preconditionErrs) {
     const incompatibleConnViolation = preconditionErr?.violations?.filter(
       (v: { type: string }) => v.type === INCOMPATIBLE_CONNECTOR_TYPE,
@@ -36,3 +35,9 @@ export function getInvalidConnectors(err: any): string[] {
   }
   return invalidConns;
 }
+
+function errorDetails(err: any, ofType: string): any[] {
+  const original = err.context?.body?.error || err?.original;
+  const details: any[] = original?.details;
+  return details?.filter((d) => d["@type"]?.includes(ofType)) || [];
+}

src/dataconnect/provisionCloudSql.ts

@@ -27,13 +27,14 @@ export async function provisionCloudSql(args: {
     const existingInstance = await cloudSqlAdminClient.getInstance(projectId, instanceId);
     silent || utils.logLabeledBullet("dataconnect", `Found existing instance ${instanceId}.`);
     connectionName = existingInstance?.connectionName || "";
-    if (!checkInstanceConfig(existingInstance, enableGoogleMlIntegration)) {
-      // TODO: Return message from checkInstanceConfig to explain exactly what changes are made
+    const why = getUpdateReason(existingInstance, enableGoogleMlIntegration);
+    if (why) {
       silent ||
         utils.logLabeledBullet(
           "dataconnect",
           `Instance ${instanceId} settings not compatible with Firebase Data Connect. ` +
-            `Updating instance to enable Cloud IAM authentication and public IP. This may take a few minutes...`,
+            `Updating instance. This may take a few minutes...` +
+            why,
         );
       await promiseWithSpinner(
         () =>
@@ -77,11 +78,21 @@ export async function provisionCloudSql(args: {
   try {
     await cloudSqlAdminClient.getDatabase(projectId, instanceId, databaseId);
     silent || utils.logLabeledBullet("dataconnect", `Found existing database ${databaseId}.`);
-  } catch (err) {
-    silent ||
-      utils.logLabeledBullet("dataconnect", `Database ${databaseId} not found, creating it now...`);
-    await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
-    silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
+  } catch (err: any) {
+    if (err.status === 404) {
+      // Create the database if not found.
+      silent ||
+        utils.logLabeledBullet(
+          "dataconnect",
+          `Database ${databaseId} not found, creating it now...`,
+        );
+      await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
+      silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
+    } else {
+      // Skip it if the database is not accessible.
+      // Possible that the CSQL instance is in the middle of something.
+      silent || utils.logLabeledWarning("dataconnect", `Database ${databaseId} is not accessible.`);
+    }
   }
   if (enableGoogleMlIntegration) {
     await grantRolesToCloudSqlServiceAccount(projectId, instanceId, [GOOGLE_ML_INTEGRATION_ROLE]);
@@ -92,26 +103,24 @@ export async function provisionCloudSql(args: {
 /**
  * Validate that existing CloudSQL instances have the necessary settings.
  */
-export function checkInstanceConfig(
-  instance: Instance,
-  requireGoogleMlIntegration: boolean,
-): boolean {
+export function getUpdateReason(instance: Instance, requireGoogleMlIntegration: boolean): string {
+  let reason = "";
   const settings = instance.settings;
   // CloudSQL instances must have public IP enabled to be used with Firebase Data Connect.
   if (!settings.ipConfiguration?.ipv4Enabled) {
-    return false;
+    reason += "\n - to enable public IP.";
   }
 
   if (requireGoogleMlIntegration) {
     if (!settings.enableGoogleMlIntegration) {
-      return false;
+      reason += "\n - to enable Google ML integration.";
     }
     if (
       !settings.databaseFlags?.some(
         (f) => f.name === "cloudsql.enable_google_ml_integration" && f.value === "on",
       )
     ) {
-      return false;
+      reason += "\n - to enable Google ML integration database flag.";
     }
   }
 
@@ -120,6 +129,9 @@ export function checkInstanceConfig(
     settings.databaseFlags?.some(
       (f) => f.name === "cloudsql.iam_authentication" && f.value === "on",
     ) ?? false;
+  if (!isIamEnabled) {
+    reason += "\n - to enable IAM authentication database flag.";
+  }
 
-  return isIamEnabled;
+  return reason;
 }

src/dataconnect/schemaMigration.ts

@@ -10,26 +10,40 @@ import { Schema } from "./types";
 import { Options } from "../options";
 import { FirebaseError } from "../error";
 import { needProjectId } from "../projectUtils";
-import { logLabeledWarning, logLabeledSuccess } from "../utils";
+import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils";
 import * as errors from "./errors";
 
 export async function diffSchema(schema: Schema): Promise<Diff[]> {
   const { serviceName, instanceName, databaseId } = getIdentifiers(schema);
-  await ensureServiceIsConnectedToCloudSql(serviceName, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ false,
+  );
   try {
     await upsertSchema(schema, /** validateOnly=*/ true);
+    logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   } catch (err: any) {
+    if (err.status !== 400) {
+      throw err;
+    }
     const invalidConnectors = errors.getInvalidConnectors(err);
+    const incompatible = errors.getIncompatibleSchemaError(err);
+    if (!incompatible && !invalidConnectors.length) {
+      // If we got a different type of error, throw it
+      throw err;
+    }
+
+    // Display failed precondition errors nicely.
     if (invalidConnectors.length) {
       displayInvalidConnectors(invalidConnectors);
     }
-    const incompatible = errors.getIncompatibleSchemaError(err);
     if (incompatible) {
       displaySchemaChanges(incompatible);
       return incompatible.diffs;
     }
   }
-  logLabeledSuccess("dataconnect", `Database schema is up to date.`);
   return [];
 }
 
@@ -42,17 +56,27 @@ export async function migrateSchema(args: {
   const { options, schema, allowNonInteractiveMigration, validateOnly } = args;
 
   const { serviceName, instanceId, instanceName, databaseId } = getIdentifiers(schema);
-  await ensureServiceIsConnectedToCloudSql(serviceName, instanceName, databaseId);
+  await ensureServiceIsConnectedToCloudSql(
+    serviceName,
+    instanceName,
+    databaseId,
+    /* linkIfNotConnected=*/ true,
+  );
   try {
     await upsertSchema(schema, validateOnly);
     logger.debug(`Database schema was up to date for ${instanceId}:${databaseId}`);
   } catch (err: any) {
+    if (err.status !== 400) {
+      throw err;
+    }
+    // Parse and handle failed precondition errors, then retry.
     const incompatible = errors.getIncompatibleSchemaError(err);
     const invalidConnectors = errors.getInvalidConnectors(err);
     if (!incompatible && !invalidConnectors.length) {
       // If we got a different type of error, throw it
       throw err;
     }
+
     const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError(
       options,
       invalidConnectors,
@@ -61,7 +85,7 @@ export async function migrateSchema(args: {
     if (!shouldDeleteInvalidConnectors && invalidConnectors.length) {
       const cmd = suggestedCommand(serviceName, invalidConnectors);
       throw new FirebaseError(
-        `Command aborted. Try deploying compatible connectors first with ${clc.bold(cmd)}`,
+        `Command aborted. Try deploying those connectors first with ${clc.bold(cmd)}`,
       );
     }
     const migrationMode = incompatible
@@ -266,44 +290,61 @@ function displayInvalidConnectors(invalidConnectors: string[]) {
 
 // If a service has never had a schema with schemaValidation=strict
 // (ie when users create a service in console),
-// the backend will not have the necesary permissions to check cSQL for differences.
+// the backend will not have the necessary permissions to check cSQL for differences.
 // We fix this by upserting the currently deployed schema with schemaValidation=strict,
 async function ensureServiceIsConnectedToCloudSql(
   serviceName: string,
   instanceId: string,
   databaseId: string,
+  linkIfNotConnected: boolean,
 ) {
   let currentSchema: Schema;
   try {
     currentSchema = await getSchema(serviceName);
   } catch (err: any) {
-    if (err.status === 404) {
-      // If no schema has been deployed yet, deploy an empty one to get connectivity.
-      currentSchema = {
-        name: `${serviceName}/schemas/${SCHEMA_ID}`,
-        source: {
-          files: [],
-        },
-        primaryDatasource: {
-          postgresql: {
-            database: databaseId,
-            cloudSql: {
-              instance: instanceId,
-            },
-          },
-        },
-      };
-    } else {
+    if (err.status !== 404) {
       throw err;
     }
+    if (!linkIfNotConnected) {
+      logLabeledWarning("dataconnect", `Not yet linked to the Cloud SQL instance.`);
+      return;
+    }
+    // TODO: make this prompt
+    // Should we upsert service here as well? so `database:sql:migrate` work for new service as well.
+    logLabeledBullet("dataconnect", `Linking the Cloud SQL instance...`);
+    // If no schema has been deployed yet, deploy an empty one to get connectivity.
+    currentSchema = {
+      name: `${serviceName}/schemas/${SCHEMA_ID}`,
+      source: {
+        files: [],
+      },
+      primaryDatasource: {
+        postgresql: {
+          database: databaseId,
+          cloudSql: {
+            instance: instanceId,
+          },
+        },
+      },
+    };
   }
-  if (
-    !currentSchema.primaryDatasource.postgresql ||
-    currentSchema.primaryDatasource.postgresql.schemaValidation === "STRICT"
-  ) {
+  const postgresql = currentSchema.primaryDatasource.postgresql;
+  if (postgresql?.cloudSql.instance !== instanceId) {
+    logLabeledWarning(
+      "dataconnect",
+      `Switching connected Cloud SQL instance\nFrom ${postgresql?.cloudSql.instance}\nTo ${instanceId}`,
+    );
+  }
+  if (postgresql?.database !== databaseId) {
+    logLabeledWarning(
+      "dataconnect",
+      `Switching connected Postgres database from ${postgresql?.database} to ${databaseId}`,
+    );
+  }
+  if (!postgresql || postgresql.schemaValidation === "STRICT") {
     return;
   }
-  currentSchema.primaryDatasource.postgresql.schemaValidation = "STRICT";
+  postgresql.schemaValidation = "STRICT";
   try {
     await upsertSchema(currentSchema, /** validateOnly=*/ false);
   } catch (err: any) {
@@ -315,11 +356,31 @@ async function ensureServiceIsConnectedToCloudSql(
 }
 
 function displaySchemaChanges(error: IncompatibleSqlSchemaError) {
-  const message =
-    "Your new schema is incompatible with the schema of your CloudSQL database. " +
-    "The following SQL statements will migrate your database schema to match your new Data Connect schema.\n" +
-    error.diffs.map(toString).join("\n");
-  logLabeledWarning("dataconnect", message);
+  switch (error.violationType) {
+    case "INCOMPATIBLE_SCHEMA":
+      {
+        const message =
+          "Your new schema is incompatible with the schema of your CloudSQL database. " +
+          "The following SQL statements will migrate your database schema to match your new Data Connect schema.\n" +
+          error.diffs.map(toString).join("\n");
+        logLabeledWarning("dataconnect", message);
+      }
+      break;
+    case "INACCESSIBLE_SCHEMA":
+      {
+        const message =
+          "Cannot access your CloudSQL database to validate schema. " +
+          "The following SQL statements can setup a new database schema.\n" +
+          error.diffs.map(toString).join("\n");
+        logLabeledWarning("dataconnect", message);
+        logLabeledWarning("dataconnect", "Some SQL resources may already exist.");
+      }
+      break;
+    default:
+      throw new FirebaseError(
+        `Unknown schema violation type: ${error.violationType}, IncompatibleSqlSchemaError: ${error}`,
+      );
+  }
 }
 
 function toString(diff: Diff) {

src/dataconnect/types.ts

@@ -48,12 +48,15 @@ export interface File {
   content: string;
 }
 
-// An error indicating that the SQL database schema is incomptible with a data connect schema.
+// An error indicating that the SQL database schema is incompatible with a data connect schema.
 export interface IncompatibleSqlSchemaError {
-  // A list of differences between the two schema with instrucitons how to resolve them.
+  // A list of differences between the two schema with instructions how to resolve them.
   diffs: Diff[];
   // Whether any of the changes included are destructive.
   destructive: boolean;
+
+  // The failed precondition validation type.
+  violationType: "INCOMPATIBLE_SCHEMA" | "INACCESSIBLE_SCHEMA" | string;
 }
 
 export interface Diff {

src/gcp/cloudsql/cloudsqladmin.ts

@@ -51,6 +51,8 @@ export async function createInstance(
       userLabels: { "firebase-data-connect": "ft" },
       insightsConfig: {
         queryInsightsEnabled: true,
+        queryPlansPerMinute: 5, // Match the default settings
+        queryStringLength: 1024, // Match the default settings
       },
     },
   });