chore(events): apply post-merge comments

Introduces a new interface `RequestDetails` to abstract `Request`
providing access to request body and headers.

**BREAKING:** Replace `request: Request` with `request: RequestDetails` at `RequestValidator`.

**BREAKING:** Remove required field `router` at `HttpPostIngressEventPublisher.fromConfig`
  and replace it with `bind(router: Router)`.
  Additionally, the path prefix `/http` will be added inside `HttpPostIngressEventPublisher`.

Relates-to: PR #13931
Signed-off-by: Patrick Jungermann <Patrick.Jungermann@gmail.com>

plugins/events-backend/src/service/EventsPlugin.ts

@@ -90,14 +90,11 @@ export const eventsPlugin = createBackendPlugin({
     env.registerInit({
       deps: {
         config: configServiceRef,
-        httpRouter: httpRouterServiceRef,
         logger: loggerServiceRef,
+        router: httpRouterServiceRef,
       },
-      async init({ config, httpRouter, logger }) {
+      async init({ config, logger, router }) {
         const winstonLogger = loggerToWinstonLogger(logger);
-        const eventsRouter = Router();
-        const router = Router();
-        eventsRouter.use('/http', router);
 
         const ingresses = Object.fromEntries(
           extensionPoint.httpPostIngresses.map(ingress => [
@@ -108,23 +105,20 @@ export const eventsPlugin = createBackendPlugin({
 
         const http = HttpPostIngressEventPublisher.fromConfig({
           config,
-          logger: winstonLogger,
-          router,
           ingresses,
+          logger: winstonLogger,
         });
+        const eventsRouter = Router();
+        http.bind(eventsRouter);
+        router.use(eventsRouter);
 
-        if (!extensionPoint.eventBroker) {
-          extensionPoint.setEventBroker(new InMemoryEventBroker(winstonLogger));
-        }
+        const eventBroker =
+          extensionPoint.eventBroker ?? new InMemoryEventBroker(winstonLogger);
 
-        extensionPoint.eventBroker!.subscribe(extensionPoint.subscribers);
+        eventBroker.subscribe(extensionPoint.subscribers);
         [extensionPoint.publishers, http]
           .flat()
-          .forEach(publisher =>
-            publisher.setEventBroker(extensionPoint.eventBroker!),
-          );
-
-        httpRouter.use(eventsRouter);
+          .forEach(publisher => publisher.setEventBroker(eventBroker));
       },
     });
   },

plugins/events-backend/src/service/http/HttpPostIngressEventPublisher.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { errorHandler, getVoidLogger } from '@backstage/backend-common';
+import { getVoidLogger } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
 import { TestEventBroker } from '@backstage/plugin-events-backend-test-utils';
 import express from 'express';
@@ -35,37 +35,35 @@ describe('HttpPostIngressEventPublisher', () => {
     });
 
     const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
     const app = express().use(router);
 
     const publisher = HttpPostIngressEventPublisher.fromConfig({
       config,
-      logger,
-      router,
       ingresses: {
         testB: {},
       },
+      logger,
     });
+    publisher.bind(router);
 
     const eventBroker = new TestEventBroker();
     await publisher.setEventBroker(eventBroker);
 
     const notFoundResponse = await request(app)
-      .post('/unknown')
+      .post('/http/unknown')
       .timeout(100)
       .send({ test: 'data' });
     expect(notFoundResponse.status).toBe(404);
 
     const response1 = await request(app)
-      .post('/testA')
+      .post('/http/testA')
       .set('X-Custom-Header', 'test-value')
       .timeout(100)
       .send({ testA: 'data' });
     expect(response1.status).toBe(202);
 
     const response2 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Custom-Header', 'test-value')
       .timeout(100)
       .send({ testB: 'data' });
@@ -100,14 +98,10 @@ describe('HttpPostIngressEventPublisher', () => {
     });
 
     const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
     const app = express().use(router);
 
     const publisher = HttpPostIngressEventPublisher.fromConfig({
       config,
-      logger,
-      router,
       ingresses: {
         testB: {
           validator: async (req, context) => {
@@ -148,26 +142,28 @@ describe('HttpPostIngressEventPublisher', () => {
           },
         },
       },
+      logger,
     });
+    publisher.bind(router);
 
     const eventBroker = new TestEventBroker();
     await publisher.setEventBroker(eventBroker);
 
     const response1 = await request(app)
-      .post('/testA')
+      .post('/http/testA')
       .timeout(100)
       .send({ test: 'data' });
     expect(response1.status).toBe(202);
 
     const response2 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .timeout(100)
       .send({ test: 'data' });
     expect(response2.status).toBe(400);
     expect(response2.body).toEqual({ message: 'wrong signature' });
 
     const response3 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Test-Signature', 'wrong')
       .timeout(100)
       .send({ test: 'data' });
@@ -175,21 +171,21 @@ describe('HttpPostIngressEventPublisher', () => {
     expect(response3.body).toEqual({ message: 'wrong signature' });
 
     const response4 = await request(app)
-      .post('/testB')
+      .post('/http/testB')
       .set('X-Test-Signature', 'testB-signature')
       .timeout(100)
       .send({ test: 'data' });
     expect(response4.status).toBe(202);
 
     const response5 = await request(app)
-      .post('/testC')
+      .post('/http/testC')
       .timeout(100)
       .send({ test: 'data' });
     expect(response5.status).toBe(404);
     expect(response5.body).toEqual({});
 
     const response6 = await request(app)
-      .post('/testD')
+      .post('/http/testD')
       .timeout(100)
       .send({ test: 'data' });
     expect(response6.status).toBe(403);
@@ -210,15 +206,10 @@ describe('HttpPostIngressEventPublisher', () => {
   it('without configuration', async () => {
     const config = new ConfigReader({});
 
-    const router = Router();
-    router.use(express.json());
-    router.use(errorHandler());
-
     expect(() =>
       HttpPostIngressEventPublisher.fromConfig({
         config,
         logger,
-        router,
       }),
     ).not.toThrow();
   });

plugins/events-backend/src/service/http/HttpPostIngressEventPublisher.ts

@@ -41,7 +41,6 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
     config: Config;
     ingresses?: { [topic: string]: Omit<HttpPostIngressOptions, 'topic'> };
     logger: Logger;
-    router: express.Router;
   }): HttpPostIngressEventPublisher {
     const topics =
       env.config.getOptionalStringArray('events.http.topics') ?? [];
@@ -55,15 +54,18 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
       }
     });
 
-    return new HttpPostIngressEventPublisher(env.logger, env.router, ingresses);
+    return new HttpPostIngressEventPublisher(env.logger, ingresses);
   }
 
   private constructor(
-    private logger: Logger,
-    router: express.Router,
-    ingresses: { [topic: string]: Omit<HttpPostIngressOptions, 'topic'> },
-  ) {
-    router.use(this.createRouter(ingresses));
+    private readonly logger: Logger,
+    private readonly ingresses: {
+      [topic: string]: Omit<HttpPostIngressOptions, 'topic'>;
+    },
+  ) {}
+
+  bind(router: express.Router): void {
+    router.use('/http', this.createRouter(this.ingresses));
   }
 
   async setEventBroker(eventBroker: EventBroker): Promise<void> {
@@ -92,8 +94,12 @@ export class HttpPostIngressEventPublisher implements EventPublisher {
     const path = `/${topic}`;
 
     router.post(path, async (request, response) => {
+      const requestDetails = {
+        body: request.body,
+        headers: request.headers,
+      };
       const context = new RequestValidationContextImpl();
-      await validator?.(request, context);
+      await validator?.(requestDetails, context);
       if (context.wasRejected()) {
         response
           .status(context.rejectionDetails!.status)