Fb/redis client options fixes (#93)

* propagate tls options to client for non-clusters
ignore uri from cf credentials and rely on hostname port password instead

* add 5min ping interval by default

* add clarifying note

* update snaps

Author: rlindner81

src/redis-adapter.js

@@ -90,20 +90,30 @@ const _createClientBase = (clientName) => {
     try {
       // NOTE: settings the user explicitly to empty resolves auth problems, see
       // https://github.com/go-redis/redis/issues/1343
-      const redisCredentials = cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
-      const redisIsCluster = redisCredentials.cluster_mode;
-      const url = redisCredentials.uri.replace(/(?<=rediss:\/\/)[\w-]+?(?=:)/, "");
-      if (redisIsCluster) {
+      const {
+        cluster_mode: isCluster,
+        hostname: host,
+        port,
+        password,
+        tls,
+      } = cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
+      const redisClientOptions = {
+        password,
+        pingInterval: 5 * 60000,
+        socket: {
+          host,
+          port,
+          tls,
+        },
+      };
+      if (isCluster) {
         return redis.createCluster({
-          rootNodes: [{ url }],
+          rootNodes: [redisClientOptions], // NOTE: assume this ignores everything but socket/url
           // https://github.com/redis/node-redis/issues/1782
-          defaults: {
-            password: redisCredentials.password,
-            socket: { tls: redisCredentials.tls },
-          },
+          defaults: redisClientOptions, // NOTE: assume this ignores socket/url
         });
       }
-      return redis.createClient({ url });
+      return redis.createClient(redisClientOptions);
     } catch (err) {
       throw new VError(
         { name: VERROR_CLUSTER_NAME, cause: err, info: { clientName } },

test/redis-adapter.test.js

@@ -87,17 +87,37 @@ describe("redis-adapter test", () => {
 
   test("_createClientBase on CF", async () => {
     const mockUrl = "rediss://BAD_USERNAME:pwd@mockUrl";
-    const mockUrlUsable = mockUrl.replace("BAD_USERNAME", "");
 
     envMock.isOnCf = true;
-    envMock.cfServiceCredentialsForLabel.mockReturnValueOnce({ uri: mockUrl });
+    envMock.cfServiceCredentialsForLabel.mockReturnValueOnce({
+      cluster_mode: false,
+      uri: mockUrl,
+      hostname: "my-domain.com",
+      port: "1234",
+      password: "mock-password",
+      tls: { tlsOption: "tlsOption" },
+    });
 
     const client = redisAdapter._._createClientBase();
 
     expect(envMock.cfServiceCredentialsForLabel).toHaveBeenCalledTimes(1);
     expect(envMock.cfServiceCredentialsForLabel).toHaveBeenCalledWith("redis-cache");
     expect(redis.createClient).toHaveBeenCalledTimes(1);
-    expect(redis.createClient).toHaveBeenCalledWith({ url: mockUrlUsable });
+    expect(redis.createClient.mock.calls[0]).toMatchInlineSnapshot(`
+      [
+        {
+          "password": "mock-password",
+          "pingInterval": 300000,
+          "socket": {
+            "host": "my-domain.com",
+            "port": "1234",
+            "tls": {
+              "tlsOption": "tlsOption",
+            },
+          },
+        },
+      ]
+    `);
     expect(client).toBe(mockClient);
     expect(loggerSpy.error).not.toHaveBeenCalled();
   });