Skip to content

Commit f151d2e

Browse files
joehanfredzqmhlshenbkendall
authored andcommitted
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 <bkend@google.com> Co-authored-by: joehan <joehanley@google.com> * 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 <fredzqm@google.com> Co-authored-by: Harold Shen <hlshen@google.com> Co-authored-by: Bryan Kendall <bkend@google.com>
1 parent 3597533 commit f151d2e

File tree

14 files changed

+176
-98
lines changed

14 files changed

+176
-98
lines changed

firebase-vscode/package.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"Other"
1515
],
1616
"extensionDependencies": [
17-
"graphql.vscode-graphql-syntax"
17+
"graphql.vscode-graphql-syntax",
18+
"redhat.vscode-yaml"
1819
],
1920
"activationEvents": [
2021
"onStartupFinished",
@@ -110,7 +111,7 @@
110111
"mono-firebase": {
111112
"description": "Firebase icon",
112113
"default": {
113-
"fontPath": "./resources/Monicons.woff",
114+
"fontPath": "./resources/monicons.woff",
114115
"fontCharacter": "\\F101"
115116
}
116117
},
@@ -180,11 +181,11 @@
180181
},
181182
{
182183
"fileMatch": "dataconnect.yaml",
183-
"url": "./schema/dataconnect-yaml.json"
184+
"url": "./dist/schema/dataconnect-yaml.json"
184185
},
185186
{
186187
"fileMatch": "connector.yaml",
187-
"url": "./schema/connector-yaml.json"
188+
"url": "./dist/schema/connector-yaml.json"
188189
}
189190
]
190191
},
8.34 KB
Binary file not shown.
28.5 KB
Loading

firebase-vscode/src/cli.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,10 @@ async function getServiceAccount() {
6060
if (e.original?.message) {
6161
errorMessage += ` (original: ${e.original.message})`;
6262
}
63-
if (process.env.MONOSPACE_ENV) {
64-
// If it can't find a service account in Monospace, that's a blocking
65-
// error and we should throw.
66-
throw new Error(
67-
`Unable to find service account. ` + `requireAuthError: ${errorMessage}`
68-
);
69-
} else {
70-
// In other environments, it is common to not find a service account.
71-
pluginLogger.debug(
72-
`No service account found (this may be normal), ` +
73-
`requireAuth error output: ${errorMessage}`
74-
);
75-
}
63+
pluginLogger.debug(
64+
`No service account found (this may be normal), ` +
65+
`requireAuth error output: ${errorMessage}`
66+
);
7667
return null;
7768
}
7869
if (process.env.WORKSPACE_SERVICE_ACCOUNT_EMAIL) {

firebase-vscode/src/core/project.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import vscode, { Disposable, ExtensionContext, QuickPickItem } from "vscode";
1+
import vscode, { Disposable } from "vscode";
22
import { ExtensionBrokerImpl } from "../extension-broker";
33
import { computed, effect } from "@preact/signals-react";
44
import { firebaseRC, updateFirebaseRCProject } from "./config";
55
import { FirebaseProjectMetadata } from "../types/project";
66
import { currentUser, isServiceAccount } from "./user";
77
import { listProjects } from "../cli";
88
import { pluginLogger } from "../logger-wrapper";
9-
import { currentOptions } from "../options";
109
import { globalSignal } from "../utils/globals";
1110
import { firstWhereDefined } from "../utils/signal";
1211

src/dataconnect/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,9 @@ export async function deleteService(
8080
locationId: string,
8181
serviceId: string,
8282
): Promise<types.Service> {
83+
// NOTE(fredzqm): Don't force delete yet. Backend would leave orphaned resources.
8384
const op = await dataconnectClient().delete<types.Service>(
84-
`projects/${projectId}/locations/${locationId}/services/${serviceId}?force=true`,
85+
`projects/${projectId}/locations/${locationId}/services/${serviceId}`,
8586
);
8687
const pollRes = await operationPoller.pollOperation<types.Service>({
8788
apiOrigin: dataconnectOrigin(),

src/dataconnect/errors.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,28 +5,27 @@ const PRECONDITION_ERROR_TYPESTRING = "type.googleapis.com/google.rpc.Preconditi
55
const INCOMPATIBLE_CONNECTOR_TYPE = "INCOMPATIBLE_CONNECTOR";
66

77
export function getIncompatibleSchemaError(err: any): IncompatibleSqlSchemaError | undefined {
8-
const original = err.context?.body?.error || err.orignal;
9-
if (!original) {
10-
// If we can't get the original, rethrow so we don't cover up the original error.
11-
throw err;
8+
const incompatibles = errorDetails(err, INCOMPATIBLE_SCHEMA_ERROR_TYPESTRING);
9+
if (incompatibles.length === 0) {
10+
return undefined;
1211
}
13-
const details: any[] = original.details;
14-
const incompatibles = details.filter((d) =>
15-
d["@type"]?.includes(INCOMPATIBLE_SCHEMA_ERROR_TYPESTRING),
16-
);
1712
// Should never get multiple incompatible schema errors
18-
return incompatibles[0];
13+
const incompatible = incompatibles[0];
14+
// Extract the violation type from the precondition error detail.
15+
const preconditionErrs = errorDetails(err, PRECONDITION_ERROR_TYPESTRING);
16+
const violationTypes = (incompatible.violationType = preconditionErrs
17+
.flatMap((preCondErr) => preCondErr.violations)
18+
.flatMap((viol) => viol.type)
19+
.filter((type) => type === "INACCESSIBLE_SCHEMA" || type === "INCOMPATIBLE_SCHEMA"));
20+
incompatible.violationType = violationTypes[0];
21+
return incompatible;
1922
}
2023

2124
// Note - the backend just includes file name, not the name of the connector resource in the GQLerror extensions.
2225
// so we don't use this yet. Ideally, we'd just include connector name in the extensions.
2326
export function getInvalidConnectors(err: any): string[] {
27+
const preconditionErrs = errorDetails(err, PRECONDITION_ERROR_TYPESTRING);
2428
const invalidConns: string[] = [];
25-
const original = err.context?.body?.error || err?.orignal;
26-
const details: any[] = original?.details;
27-
const preconditionErrs = details?.filter((d) =>
28-
d["@type"]?.includes(PRECONDITION_ERROR_TYPESTRING),
29-
);
3029
for (const preconditionErr of preconditionErrs) {
3130
const incompatibleConnViolation = preconditionErr?.violations?.filter(
3231
(v: { type: string }) => v.type === INCOMPATIBLE_CONNECTOR_TYPE,
@@ -36,3 +35,9 @@ export function getInvalidConnectors(err: any): string[] {
3635
}
3736
return invalidConns;
3837
}
38+
39+
function errorDetails(err: any, ofType: string): any[] {
40+
const original = err.context?.body?.error || err?.original;
41+
const details: any[] = original?.details;
42+
return details?.filter((d) => d["@type"]?.includes(ofType)) || [];
43+
}

src/dataconnect/provisionCloudSql.ts

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ export async function provisionCloudSql(args: {
2727
const existingInstance = await cloudSqlAdminClient.getInstance(projectId, instanceId);
2828
silent || utils.logLabeledBullet("dataconnect", `Found existing instance ${instanceId}.`);
2929
connectionName = existingInstance?.connectionName || "";
30-
if (!checkInstanceConfig(existingInstance, enableGoogleMlIntegration)) {
31-
// TODO: Return message from checkInstanceConfig to explain exactly what changes are made
30+
const why = getUpdateReason(existingInstance, enableGoogleMlIntegration);
31+
if (why) {
3232
silent ||
3333
utils.logLabeledBullet(
3434
"dataconnect",
3535
`Instance ${instanceId} settings not compatible with Firebase Data Connect. ` +
36-
`Updating instance to enable Cloud IAM authentication and public IP. This may take a few minutes...`,
36+
`Updating instance. This may take a few minutes...` +
37+
why,
3738
);
3839
await promiseWithSpinner(
3940
() =>
@@ -77,11 +78,21 @@ export async function provisionCloudSql(args: {
7778
try {
7879
await cloudSqlAdminClient.getDatabase(projectId, instanceId, databaseId);
7980
silent || utils.logLabeledBullet("dataconnect", `Found existing database ${databaseId}.`);
80-
} catch (err) {
81-
silent ||
82-
utils.logLabeledBullet("dataconnect", `Database ${databaseId} not found, creating it now...`);
83-
await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
84-
silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
81+
} catch (err: any) {
82+
if (err.status === 404) {
83+
// Create the database if not found.
84+
silent ||
85+
utils.logLabeledBullet(
86+
"dataconnect",
87+
`Database ${databaseId} not found, creating it now...`,
88+
);
89+
await cloudSqlAdminClient.createDatabase(projectId, instanceId, databaseId);
90+
silent || utils.logLabeledBullet("dataconnect", `Database ${databaseId} created.`);
91+
} else {
92+
// Skip it if the database is not accessible.
93+
// Possible that the CSQL instance is in the middle of something.
94+
silent || utils.logLabeledWarning("dataconnect", `Database ${databaseId} is not accessible.`);
95+
}
8596
}
8697
if (enableGoogleMlIntegration) {
8798
await grantRolesToCloudSqlServiceAccount(projectId, instanceId, [GOOGLE_ML_INTEGRATION_ROLE]);
@@ -92,26 +103,24 @@ export async function provisionCloudSql(args: {
92103
/**
93104
* Validate that existing CloudSQL instances have the necessary settings.
94105
*/
95-
export function checkInstanceConfig(
96-
instance: Instance,
97-
requireGoogleMlIntegration: boolean,
98-
): boolean {
106+
export function getUpdateReason(instance: Instance, requireGoogleMlIntegration: boolean): string {
107+
let reason = "";
99108
const settings = instance.settings;
100109
// CloudSQL instances must have public IP enabled to be used with Firebase Data Connect.
101110
if (!settings.ipConfiguration?.ipv4Enabled) {
102-
return false;
111+
reason += "\n - to enable public IP.";
103112
}
104113

105114
if (requireGoogleMlIntegration) {
106115
if (!settings.enableGoogleMlIntegration) {
107-
return false;
116+
reason += "\n - to enable Google ML integration.";
108117
}
109118
if (
110119
!settings.databaseFlags?.some(
111120
(f) => f.name === "cloudsql.enable_google_ml_integration" && f.value === "on",
112121
)
113122
) {
114-
return false;
123+
reason += "\n - to enable Google ML integration database flag.";
115124
}
116125
}
117126

@@ -120,6 +129,9 @@ export function checkInstanceConfig(
120129
settings.databaseFlags?.some(
121130
(f) => f.name === "cloudsql.iam_authentication" && f.value === "on",
122131
) ?? false;
132+
if (!isIamEnabled) {
133+
reason += "\n - to enable IAM authentication database flag.";
134+
}
123135

124-
return isIamEnabled;
136+
return reason;
125137
}

0 commit comments

Comments
 (0)