Merge pull request #6242 from backstage/freben/kafka-cluster-errors

Properly return a 404 when an unknown cluster is given

.changeset/warm-mangos-share.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-kafka-backend': patch
+---
+
+Properly return a 404 when an unknown cluster is given

plugins/kafka-backend/package.json

@@ -34,6 +34,7 @@
     "@backstage/backend-common": "^0.8.2",
     "@backstage/catalog-model": "^0.8.2",
     "@backstage/config": "^0.1.5",
+    "@backstage/errors": "^0.1.1",
     "@types/express": "^4.17.6",
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",

plugins/kafka-backend/src/service/router.test.ts

@@ -17,7 +17,7 @@
 import request from 'supertest';
 import express from 'express';
 import { makeRouter, ClusterApi } from './router';
-import { getVoidLogger } from '@backstage/backend-common';
+import { errorHandler, getVoidLogger } from '@backstage/backend-common';
 import { KafkaApi } from './KafkaApi';
 import { when } from 'jest-when';
 
@@ -111,6 +111,19 @@ describe('router', () => {
       );
     });
 
+    it('handles unknown cluster errors correctly', async () => {
+      const response = await request(app.use(errorHandler())).get(
+        '/consumers/unknown/hey/offsets',
+      );
+      expect(response.status).toEqual(404);
+      expect(response.body).toMatchObject({
+        error: {
+          message:
+            'Found no configured cluster "unknown", candidates are "dev", "prod"',
+        },
+      });
+    });
+
     it('handles internal error correctly', async () => {
       prodKafkaApi.fetchGroupOffsets.mockRejectedValue(Error('oh no'));
 

plugins/kafka-backend/src/service/router.ts

@@ -18,6 +18,7 @@ import express from 'express';
 import Router from 'express-promise-router';
 import { Logger } from 'winston';
 import { Config } from '@backstage/config';
+import { NotFoundError } from '@backstage/errors';
 import { KafkaApi, KafkaJsApiImpl } from './KafkaApi';
 import _ from 'lodash';
 import { getClusterDetails } from '../config/ClusterReader';
@@ -45,12 +46,20 @@ export const makeRouter = (
     const clusterId = req.params.clusterId;
     const consumerId = req.params.consumerId;
 
+    const kafkaApi = kafkaApiByClusterName[clusterId];
+    if (!kafkaApi) {
+      const candidates = Object.keys(kafkaApiByClusterName)
+        .map(n => `"${n}"`)
+        .join(', ');
+      throw new NotFoundError(
+        `Found no configured cluster "${clusterId}", candidates are ${candidates}`,
+      );
+    }
+
     logger.info(
       `Fetch consumer group ${consumerId} offsets from cluster ${clusterId}`,
     );
 
-    const kafkaApi = kafkaApiByClusterName[clusterId];
-
     const groupOffsets = await kafkaApi.api.fetchGroupOffsets(consumerId);
 
     const groupWithTopicOffsets = await Promise.all(
@@ -68,6 +77,7 @@ export const makeRouter = (
         }));
       }),
     );
+
     res.json({ consumerId, offsets: groupWithTopicOffsets.flat() });
   });