Dataconnect (#7193)

* Cloud SQL create and update polish (#7159) * improve logged message execute SQL commands (#7156) * Error handling and logging when linking CSQL instance (#7158) * Display accurate message for INACCESSIBLE_SCHEMA error (#7157) * handle INACCESSIBLE_SCHEMA case separately * error msg * m * m * m * merge * feedbacks * Jh idx no metadata calls (#7179) * Avoid triggering metadata server calls in idx * Remove outdated service account check * Update Firebase Logo (#7180) * unleash turtles (#7174) * unleash turtles * Update CHANGELOG.md * Flag flip for gen kit (#7175) * Flag flip for gen kit * Update CHANGELOG.md * update icons --------- Co-authored-by: Bryan Kendall <[email protected]> Co-authored-by: joehan <[email protected]> * Use client instead of size 1 pool (#7182) * Fix schema validation (#7185) * Fix schema validation * Also depend on redhat.yaml * fix typo --------- Co-authored-by: Fred Zhang <[email protected]> Co-authored-by: Harold Shen <[email protected]> Co-authored-by: Bryan Kendall <[email protected]>
firebase · May 20, 2024 · d6d8028 · d6d8028
1 parent a5c14cb
commit d6d8028
Show file tree

Hide file tree

Showing 14 changed files with 176 additions and 98 deletions.
diff --git a/firebase-vscode/package.json b/firebase-vscode/package.json
@@ -14,7 +14,8 @@
     "Other"
   ],
   "extensionDependencies": [
-    "graphql.vscode-graphql-syntax"
+    "graphql.vscode-graphql-syntax",
+    "redhat.vscode-yaml"
   ],
   "activationEvents": [
     "onStartupFinished",
@@ -110,7 +111,7 @@
       "mono-firebase": {
         "description": "Firebase icon",
         "default": {
-          "fontPath": "./resources/Monicons.woff",
+          "fontPath": "./resources/monicons.woff",
           "fontCharacter": "\\F101"
         }
       },
@@ -180,11 +181,11 @@
       },
       {
         "fileMatch": "dataconnect.yaml",
-        "url": "./schema/dataconnect-yaml.json"
+        "url": "./dist/schema/dataconnect-yaml.json"
       },
       {
         "fileMatch": "connector.yaml",
-        "url": "./schema/connector-yaml.json"
+        "url": "./dist/schema/connector-yaml.json"
       }
     ]
   },

diff --git a/firebase-vscode/resources/Monicons.woff b/firebase-vscode/resources/Monicons.woff
diff --git a/firebase-vscode/resources/firebase_logo.png b/firebase-vscode/resources/firebase_logo.png
diff --git a/firebase-vscode/src/cli.ts b/firebase-vscode/src/cli.ts
@@ -60,19 +60,10 @@ async function getServiceAccount() {
     if (e.original?.message) {
       errorMessage += ` (original: ${e.original.message})`;
     }
-    if (process.env.MONOSPACE_ENV) {
-      // If it can't find a service account in Monospace, that's a blocking
-      // error and we should throw.
-      throw new Error(
-        `Unable to find service account. ` + `requireAuthError: ${errorMessage}`
-      );
-    } else {
-      // In other environments, it is common to not find a service account.
-      pluginLogger.debug(
-        `No service account found (this may be normal), ` +
-          `requireAuth error output: ${errorMessage}`
-      );
-    }
+    pluginLogger.debug(
+      `No service account found (this may be normal), ` +
+        `requireAuth error output: ${errorMessage}`
+    );
     return null;
   }
   if (process.env.WORKSPACE_SERVICE_ACCOUNT_EMAIL) {

diff --git a/firebase-vscode/src/core/project.ts b/firebase-vscode/src/core/project.ts
@@ -1,12 +1,11 @@
-import vscode, { Disposable, ExtensionContext, QuickPickItem } from "vscode";
+import vscode, { Disposable } from "vscode";
 import { ExtensionBrokerImpl } from "../extension-broker";
 import { computed, effect } from "@preact/signals-react";
 import { firebaseRC, updateFirebaseRCProject } from "./config";
 import { FirebaseProjectMetadata } from "../types/project";
 import { currentUser, isServiceAccount } from "./user";
 import { listProjects } from "../cli";
 import { pluginLogger } from "../logger-wrapper";
-import { currentOptions } from "../options";
 import { globalSignal } from "../utils/globals";
 import { firstWhereDefined } from "../utils/signal";
 

diff --git a/src/dataconnect/client.ts b/src/dataconnect/client.ts
@@ -80,8 +80,9 @@ export async function deleteService(
   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(),

diff --git a/src/dataconnect/errors.ts b/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)) || [];
+}
diff --git a/src/dataconnect/provisionCloudSql.ts b/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;
 }