fix: validate unique names for monitors

src/pepr/operator/crd/validators/package-validator.spec.ts

@@ -1,13 +1,24 @@
 import { afterEach, describe, expect, it, jest } from "@jest/globals";
 import { PeprValidateRequest } from "pepr";
-import { Gateway, Expose, UDSPackage, Allow, Sso, Direction, RemoteGenerated, Protocol } from "..";
+import {
+  Gateway,
+  Expose,
+  UDSPackage,
+  Allow,
+  Sso,
+  Direction,
+  RemoteGenerated,
+  Protocol,
+  Monitor,
+} from "..";
 import { validator } from "./package-validator";
 
 const makeMockReq = (
   pkg: Partial<UDSPackage>,
   exposeList: Partial<Expose>[],
   allowList: Partial<Allow>[],
   ssoClients: Partial<Sso>[],
+  monitorList: Partial<Monitor>[],
 ) => {
   const defaultPkg: UDSPackage = {
     metadata: {
@@ -20,6 +31,7 @@ const makeMockReq = (
         allow: [],
       },
       sso: [],
+      monitor: [],
     },
   };
 
@@ -46,6 +58,16 @@ const makeMockReq = (
     defaultPkg.spec!.sso?.push({ ...defaultClient, ...client });
   }
 
+  for (const monitor of monitorList) {
+    const defaultMonitor: Monitor = {
+      description: "Default Monitor",
+      portName: "http-metrics",
+      selector: {},
+      targetPort: 8080,
+    };
+    defaultPkg.spec!.monitor?.push({ ...defaultMonitor, ...monitor });
+  }
+
   return {
     Raw: { ...defaultPkg, ...pkg },
     Approve: jest.fn(),
@@ -59,13 +81,13 @@ describe("Test validation of Exemption CRs", () => {
   });
 
   it("allows packages that have no issues", async () => {
-    const mockReq = makeMockReq({}, [{}], [{}], [{}]);
+    const mockReq = makeMockReq({}, [{}], [{}], [{}], [{}]);
     await validator(mockReq);
     expect(mockReq.Approve).toHaveBeenCalledTimes(1);
   });
 
   it("denies system namespaces", async () => {
-    const mockReq = makeMockReq({ metadata: { namespace: "kube-system" } }, [], [], []);
+    const mockReq = makeMockReq({ metadata: { namespace: "kube-system" } }, [], [], [], []);
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
@@ -83,6 +105,7 @@ describe("Test validation of Exemption CRs", () => {
       ],
       [],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -101,6 +124,7 @@ describe("Test validation of Exemption CRs", () => {
       ],
       [],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -119,6 +143,7 @@ describe("Test validation of Exemption CRs", () => {
       ],
       [],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -137,6 +162,7 @@ describe("Test validation of Exemption CRs", () => {
       ],
       [],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -155,13 +181,14 @@ describe("Test validation of Exemption CRs", () => {
       ],
       [],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
 
   it("denies virtual services that are the same name", async () => {
-    const mockReq = makeMockReq({}, [{}, {}], [], []);
+    const mockReq = makeMockReq({}, [{}, {}], [], [], []);
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
@@ -177,6 +204,7 @@ describe("Test validation of Exemption CRs", () => {
         },
       ],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -193,19 +221,20 @@ describe("Test validation of Exemption CRs", () => {
         },
       ],
       [],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
 
   it("denies network policies that are the same name", async () => {
-    const mockReq = makeMockReq({}, [], [{}, {}], []);
+    const mockReq = makeMockReq({}, [], [{}, {}], [], []);
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
 
   it("denies clients with clientIDs that are not unique", async () => {
-    const mockReq = makeMockReq({}, [], [], [{}, {}]);
+    const mockReq = makeMockReq({}, [], [], [{}, {}], []);
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
   });
@@ -220,6 +249,7 @@ describe("Test validation of Exemption CRs", () => {
           secretName: "HELLO_KITTEH",
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -235,6 +265,7 @@ describe("Test validation of Exemption CRs", () => {
           redirectUris: undefined,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -251,6 +282,7 @@ describe("Test validation of Exemption CRs", () => {
           redirectUris: undefined,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Approve).toHaveBeenCalledTimes(1);
@@ -268,6 +300,7 @@ describe("Test validation of Exemption CRs", () => {
           standardFlowEnabled: true,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -286,6 +319,7 @@ describe("Test validation of Exemption CRs", () => {
           secret: "app-client-secret",
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -304,6 +338,7 @@ describe("Test validation of Exemption CRs", () => {
           secretName: "app-k8s-secret",
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -322,6 +357,7 @@ describe("Test validation of Exemption CRs", () => {
           secretTemplate: {},
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -340,6 +376,7 @@ describe("Test validation of Exemption CRs", () => {
           enableAuthserviceSelector: {},
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -358,6 +395,7 @@ describe("Test validation of Exemption CRs", () => {
           protocol: Protocol.Saml,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -374,6 +412,7 @@ describe("Test validation of Exemption CRs", () => {
           standardFlowEnabled: false,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Deny).toHaveBeenCalledTimes(1);
@@ -391,8 +430,27 @@ describe("Test validation of Exemption CRs", () => {
           standardFlowEnabled: false,
         },
       ],
+      [],
     );
     await validator(mockReq);
     expect(mockReq.Approve).toHaveBeenCalledTimes(1);
   });
+
+  it("denies monitors with undefined descriptions", async () => {
+    const mockReq = makeMockReq({}, [], [], [], [{}, {}]);
+    await validator(mockReq);
+    expect(mockReq.Deny).toHaveBeenCalledTimes(1);
+  });
+
+  it("denies monitors that do not have unique descriptions", async () => {
+    const mockReq = makeMockReq(
+      {},
+      [],
+      [],
+      [],
+      [{ description: "Metrics" }, { description: "Metrics" }],
+    );
+    await validator(mockReq);
+    expect(mockReq.Deny).toHaveBeenCalledTimes(1);
+  });
 });

src/pepr/operator/crd/validators/package-validator.ts

@@ -115,5 +115,20 @@ export async function validator(req: PeprValidateRequest<UDSPackage>) {
     }
   }
 
+  const monitors = pkg.spec?.monitor ?? [];
+
+  // Ensure the descriptions are generating unique names
+  const monitorDescriptions = new Set<string>();
+
+  for (const monitor of monitors) {
+    const description = sanitizeResourceName(monitor.description || "null");
+
+    if (monitorDescriptions.has(description)) {
+      return req.Deny(`The sanitized description "${description}" is not unique`);
+    }
+
+    monitorDescriptions.add(description);
+  }
+
   return req.Approve();
 }