[wip] SpiceDB reconnect behavior

components/server/package.json

@@ -34,7 +34,7 @@
     "/src"
   ],
   "dependencies": {
-    "@authzed/authzed-node": "^0.10.0",
+    "@authzed/authzed-node": "^0.12.1",
     "@bufbuild/connect": "^0.8.1",
     "@bufbuild/connect-express": "^0.8.1",
     "@gitbeaker/node": "^35.7.0",

components/server/src/authorization/authorizer.ts

@@ -24,13 +24,14 @@ import {
 } from "./definitions";
 import { SpiceDBAuthorizer } from "./spicedb-authorizer";
 import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
+import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
 
 export function createInitializingAuthorizer(spiceDbAuthorizer: SpiceDBAuthorizer): Authorizer {
     const target = new Authorizer(spiceDbAuthorizer);
     const initialized = (async () => {
         await target.addInstallationAdminRole(BUILTIN_INSTLLATION_ADMIN_USER_ID);
         await target.addUser(BUILTIN_INSTLLATION_ADMIN_USER_ID);
-    })();
+    })().catch((err) => log.error("Failed to initialize authorizer", err));
     return new Proxy(target, {
         get(target, propKey, receiver) {
             const originalMethod = target[propKey as keyof typeof target];
@@ -481,6 +482,7 @@ export class Authorizer {
                     optionalSubjectId: relation.subject.object.objectId,
                 },
             },
+            optionalLimit: 0,
         });
         if (relationships.length === 0) {
             return undefined;
@@ -505,6 +507,7 @@ export class Authorizer {
                     optionalSubjectId: relation.subject.object.objectId,
                 },
             },
+            optionalLimit: 0,
         });
         return relationships.map((r) => r.relationship!);
     }

components/server/src/authorization/spicedb-authorizer.ts

@@ -11,13 +11,14 @@ import { TrustedValue } from "@gitpod/gitpod-protocol/lib/util/scrubbing";
 import { getExperimentsClientForBackend } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server";
 import { inject, injectable } from "inversify";
 import { observeSpicedbClientLatency, spicedbClientLatency } from "../prometheus-metrics";
-import { SpiceDBClient } from "./spicedb";
+import { SpiceDBClientProvider } from "./spicedb";
+import * as grpc from "@grpc/grpc-js";
 
 @injectable()
 export class SpiceDBAuthorizer {
     constructor(
-        @inject(SpiceDBClient)
-        private client: SpiceDBClient,
+        @inject(SpiceDBClientProvider)
+        private readonly clientProvider: SpiceDBClientProvider,
     ) {}
 
     async check(
@@ -26,10 +27,6 @@ export class SpiceDBAuthorizer {
             userId: string;
         },
     ): Promise<boolean> {
-        if (!this.client) {
-            return true;
-        }
-
         const featureEnabled = await getExperimentsClientForBackend().getValueAsync("centralizedPermissions", false, {
             user: {
                 id: experimentsFields.userId,
@@ -42,7 +39,8 @@ export class SpiceDBAuthorizer {
         const timer = spicedbClientLatency.startTimer();
         let error: Error | undefined;
         try {
-            const response = await this.client.checkPermission(req);
+            const client = this.clientProvider.getClient();
+            const response = await client.checkPermission(req, this.callOptions);
             const permitted = response.permissionship === v1.CheckPermissionResponse_Permissionship.HAS_PERMISSION;
 
             return permitted;
@@ -58,17 +56,15 @@ export class SpiceDBAuthorizer {
     }
 
     async writeRelationships(...updates: v1.RelationshipUpdate[]): Promise<v1.WriteRelationshipsResponse | undefined> {
-        if (!this.client) {
-            return undefined;
-        }
-
         const timer = spicedbClientLatency.startTimer();
         let error: Error | undefined;
         try {
-            const response = await this.client.writeRelationships(
+            const client = this.clientProvider.getClient();
+            const response = await client.writeRelationships(
                 v1.WriteRelationshipsRequest.create({
                     updates,
                 }),
+                this.callOptions,
             );
             log.info("[spicedb] Successfully wrote relationships.", { response, updates });
 
@@ -82,17 +78,14 @@ export class SpiceDBAuthorizer {
     }
 
     async deleteRelationships(req: v1.DeleteRelationshipsRequest): Promise<v1.ReadRelationshipsResponse[]> {
-        if (!this.client) {
-            return [];
-        }
-
         const timer = spicedbClientLatency.startTimer();
         let error: Error | undefined;
         try {
-            const existing = await this.client.readRelationships(v1.ReadRelationshipsRequest.create(req));
+            const client = this.clientProvider.getClient();
+            const existing = await client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions);
             if (existing.length > 0) {
-                const response = await this.client.deleteRelationships(req);
-                const after = await this.client.readRelationships(v1.ReadRelationshipsRequest.create(req));
+                const response = await client.deleteRelationships(req, this.callOptions);
+                const after = await client.readRelationships(v1.ReadRelationshipsRequest.create(req), this.callOptions);
                 if (after.length > 0) {
                     log.error("[spicedb] Failed to delete relationships.", { existing, after, request: req });
                 }
@@ -115,9 +108,21 @@ export class SpiceDBAuthorizer {
     }
 
     async readRelationships(req: v1.ReadRelationshipsRequest): Promise<v1.ReadRelationshipsResponse[]> {
-        if (!this.client) {
+        const client = this.clientProvider.getClient();
+        if (!client) {
             return [];
         }
-        return this.client.readRelationships(req);
+        return client.readRelationships(req, this.callOptions);
+    }
+
+    /**
+     * permission_service.grpc-client.d.ts has all methods overloaded with this pattern:
+     *  - xyzRelationships(input: Request, metadata?: grpc.Metadata | grpc.CallOptions, options?: grpc.CallOptions): grpc.ClientReadableStream<ReadRelationshipsResponse>;
+     * But the promisified client somehow does not have the same overloads. Thus we convince it here that options may be passed as 2nd argument.
+     */
+    private get callOptions(): grpc.Metadata {
+        return (<grpc.CallOptions>{
+            deadline: Date.now() + 8000,
+        }) as any as grpc.Metadata;
     }
 }

components/server/src/authorization/spicedb.ts

@@ -5,12 +5,24 @@
  */
 
 import { v1 } from "@authzed/authzed-node";
+import { IClientCallMetrics, createClientCallMetricsInterceptor } from "@gitpod/gitpod-protocol/lib/util/grpc";
 import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
+import * as grpc from "@grpc/grpc-js";
 
-export const SpiceDBClient = Symbol("SpiceDBClient");
-export type SpiceDBClient = v1.ZedPromiseClientInterface | undefined;
+export const SpiceDBClientProvider = Symbol("SpiceDBClientProvider");
 
-export function spicedbClientFromEnv(): SpiceDBClient {
+export interface SpiceDBClientConfig {
+    address: string;
+    token: string;
+}
+
+export type SpiceDBClient = v1.ZedPromiseClientInterface;
+type Client = v1.ZedClientInterface & grpc.Client;
+export interface SpiceDBClientProvider {
+    getClient(): SpiceDBClient;
+}
+
+export function spiceDBConfigFromEnv(): SpiceDBClientConfig | undefined {
     const token = process.env["SPICEDB_PRESHARED_KEY"];
     if (!token) {
         log.error("[spicedb] No preshared key configured.");
@@ -22,6 +34,69 @@ export function spicedbClientFromEnv(): SpiceDBClient {
         log.error("[spicedb] No service address configured.");
         return undefined;
     }
+    return {
+        address,
+        token,
+    };
+}
+
+function spicedbClientFromConfig(config: SpiceDBClientConfig): Client {
+    const clientOptions: grpc.ClientOptions = {
+        "grpc.client_idle_timeout_ms": 10000, // this ensures a connection is not stuck in the "READY" state for too long. Required for the reconnect logic below.
+        "grpc.max_reconnect_backoff_ms": 5000, // default: 12000
+    };
+
+    return v1.NewClient(
+        config.token,
+        config.address,
+        v1.ClientSecurity.INSECURE_PLAINTEXT_CREDENTIALS,
+        undefined,
+        clientOptions,
+    ) as Client;
+}
+
+export class CachingSpiceDBClientProvider implements SpiceDBClientProvider {
+    private client: Client | undefined;
+
+    private readonly interceptors: grpc.Interceptor[] = [];
+
+    constructor(
+        private readonly _clientConfig: SpiceDBClientConfig,
+        private readonly clientCallMetrics?: IClientCallMetrics | undefined,
+    ) {
+        if (this.clientCallMetrics) {
+            this.interceptors.push(createClientCallMetricsInterceptor(this.clientCallMetrics));
+        }
+    }
+
+    getClient(): SpiceDBClient {
+        let client = this.client;
+        if (!client) {
+            client = spicedbClientFromConfig(this.clientConfig);
+        } else if (client.getChannel().getConnectivityState(true) !== grpc.connectivityState.READY) {
+            // (gpl): We need this check and explicit re-connect because we observe a ~120s connection timeout without it.
+            // It's not entirely clear where that timeout comes from, but likely from the underlying transport, that is not exposed by grpc/grpc-js
+            client.close();
+
+            log.warn("[spicedb] Lost connection to SpiceDB - reconnecting...");
 
-    return v1.NewClient(token, address, v1.ClientSecurity.INSECURE_PLAINTEXT_CREDENTIALS).promises;
+            client = spicedbClientFromConfig(this.clientConfig);
+        }
+        this.client = client;
+
+        return client.promises;
+    }
+
+    protected get clientConfig() {
+        const config = this._clientConfig;
+        if (this.interceptors) {
+            return {
+                ...config,
+                options: {
+                    interceptors: [...this.interceptors],
+                },
+            };
+        }
+        return config;
+    }
 }

components/server/src/container-module.ts

@@ -46,7 +46,7 @@ import { AuthJWT, SignInJWT } from "./auth/jwt";
 import { LoginCompletionHandler } from "./auth/login-completion-handler";
 import { VerificationService } from "./auth/verification-service";
 import { Authorizer, createInitializingAuthorizer } from "./authorization/authorizer";
-import { SpiceDBClient, spicedbClientFromEnv } from "./authorization/spicedb";
+import { CachingSpiceDBClientProvider, SpiceDBClientProvider, spiceDBConfigFromEnv } from "./authorization/spicedb";
 import { BillingModes } from "./billing/billing-mode";
 import { EntitlementService, EntitlementServiceImpl } from "./billing/entitlement-service";
 import { EntitlementServiceUBP } from "./billing/entitlement-service-ubp";
@@ -302,8 +302,15 @@ export const productionContainerModule = new ContainerModule(
         bind(IamSessionApp).toSelf().inSingletonScope();
 
         // Authorization & Perms
-        bind(SpiceDBClient)
-            .toDynamicValue(() => spicedbClientFromEnv())
+        bind(SpiceDBClientProvider)
+            .toDynamicValue((ctx) => {
+                const config = spiceDBConfigFromEnv();
+                if (!config) {
+                    throw new Error("[spicedb] Missing configuration expected in env vars!");
+                }
+                const clientCallMetrics = ctx.container.get<IClientCallMetrics>(IClientCallMetrics);
+                return new CachingSpiceDBClientProvider(config, clientCallMetrics);
+            })
             .inSingletonScope();
         bind(SpiceDBAuthorizer).toSelf().inSingletonScope();
         bind(Authorizer)

components/server/src/test/service-testing-container-module.ts

@@ -5,7 +5,6 @@
  */
 
 import * as grpc from "@grpc/grpc-js";
-import { v1 } from "@authzed/authzed-node";
 import { IAnalyticsWriter, NullAnalyticsWriter } from "@gitpod/gitpod-protocol/lib/analytics";
 import { IDEServiceClient, IDEServiceDefinition } from "@gitpod/ide-service-api/lib/ide.pb";
 import { UsageServiceDefinition } from "@gitpod/usage-api/lib/usage/v1/usage.pb";
@@ -14,7 +13,7 @@ import { v4 } from "uuid";
 import { AuthProviderParams } from "../auth/auth-provider";
 import { HostContextProvider, HostContextProviderFactory } from "../auth/host-context-provider";
 import { HostContextProviderImpl } from "../auth/host-context-provider-impl";
-import { SpiceDBClient } from "../authorization/spicedb";
+import { CachingSpiceDBClientProvider, SpiceDBClientProvider } from "../authorization/spicedb";
 import { Config } from "../config";
 import { StorageClient } from "../storage/storage-client";
 import { testContainer } from "@gitpod/gitpod-db/lib";
@@ -189,10 +188,13 @@ const mockApplyingContainerModule = new ContainerModule((bind, unbound, isbound,
         }))
         .inSingletonScope();
 
-    rebind(SpiceDBClient)
+    rebind(SpiceDBClientProvider)
         .toDynamicValue(() => {
-            const token = v4();
-            return v1.NewClient(token, "localhost:50051", v1.ClientSecurity.INSECURE_PLAINTEXT_CREDENTIALS).promises;
+            const config = {
+                token: v4(),
+                address: "localhost:50051",
+            };
+            return new CachingSpiceDBClientProvider(config);
         })
         .inSingletonScope();
 });

install/installer/pkg/components/spicedb/deployment.go

@@ -155,10 +155,12 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
 										},
 									},
 									InitialDelaySeconds: 5,
-									PeriodSeconds:       30,
-									FailureThreshold:    5,
-									SuccessThreshold:    1,
-									TimeoutSeconds:      3,
+									// try again every 10 seconds
+									PeriodSeconds: 10,
+									// fail after 6 * 10 + 5 = 65 seconds
+									FailureThreshold: 6,
+									SuccessThreshold: 1,
+									TimeoutSeconds:   3,
 								},
 								VolumeMounts: []v1.VolumeMount{
 									bootstrapVolumeMount,

yarn.lock

@@ -29,12 +29,12 @@
     jsonpointer "^5.0.0"
     leven "^3.1.0"
 
-"@authzed/authzed-node@^0.10.0":
-  version "0.10.0"
-  resolved "https://registry.yarnpkg.com/@authzed/authzed-node/-/authzed-node-0.10.0.tgz#623e4911fde221bb526e7f2e9ca335d9f3b9072d"
-  integrity sha512-TnAnatcU5dHvyGqrWoZzPNaO1opPpVU1y7P5LrJsV2j54y0xvx/OFhYtfeguMxHSz2kpbdCuIvIKJuB8WFbRRA==
+"@authzed/authzed-node@^0.12.1":
+  version "0.12.1"
+  resolved "https://registry.yarnpkg.com/@authzed/authzed-node/-/authzed-node-0.12.1.tgz#0c28395a64f9b1ecf33faf67259e32a9a3bce300"
+  integrity sha512-BVHLaPfiHQw1Vz+199m9i4xltT3YyFhqVHtkYPIQ28q8a7iJpnXmFRZIWuTMJcxJI01wtAxJYFuRJq3ktFe6qw==
   dependencies:
-    "@grpc/grpc-js" "^1.2.8"
+    "@grpc/grpc-js" "^1.8.3"
     "@protobuf-ts/runtime" "^2.8.1"
     "@protobuf-ts/runtime-rpc" "^2.8.1"
     google-protobuf "^3.15.3"
@@ -2222,22 +2222,14 @@
     qs "^6.10.1"
     xcase "^2.0.1"
 
-"@grpc/[email protected]":
+"@grpc/[email protected]", "@grpc/grpc-js@^1.8.3":
   version "1.8.8"
   resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.8.8.tgz#a7c6765d0302f47ba67c0ce3cb79718d6b028248"
   integrity sha512-4gfDqMLXTrorvYTKA1jL22zLvVwiHJ73t6Re1OHwdCFRjdGTDOVtSJuaWhtHaivyeDGg0LeCkmU77MTKoV3wPA==
   dependencies:
     "@grpc/proto-loader" "^0.7.0"
     "@types/node" ">=12.12.47"
 
-"@grpc/grpc-js@^1.2.8":
-  version "1.8.7"
-  resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.8.7.tgz#2154fc0134462ad45f4134e8b54682a25ed05956"
-  integrity sha512-dRAWjRFN1Zy9mzPNLkFFIWT8T6C9euwluzCHZUKuhC+Bk3MayNPcpgDRyG+sg+n2sitEUySKxUynirVpu9ItKw==
-  dependencies:
-    "@grpc/proto-loader" "^0.7.0"
-    "@types/node" ">=12.12.47"
-
 "@grpc/grpc-js@^1.6.1":
   version "1.7.0"
   resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.7.0.tgz#5a96bdbe51cce23faa38a4db6e43595a5c584849"