diff --git a/src/pepr/operator/crd/validators/package-validator.spec.ts b/src/pepr/operator/crd/validators/package-validator.spec.ts index fd1e0e5c5..476bcea38 100644 --- a/src/pepr/operator/crd/validators/package-validator.spec.ts +++ b/src/pepr/operator/crd/validators/package-validator.spec.ts @@ -1,6 +1,16 @@ import { afterEach, describe, expect, it, jest } from "@jest/globals"; import { PeprValidateRequest } from "pepr"; -import { Allow, Direction, Expose, Gateway, Protocol, RemoteGenerated, Sso, UDSPackage } from ".."; +import { + Allow, + Direction, + Expose, + Gateway, + Monitor, + Protocol, + RemoteGenerated, + Sso, + UDSPackage, +} from ".."; import { validator } from "./package-validator"; const makeMockReq = ( @@ -8,6 +18,7 @@ const makeMockReq = ( exposeList: Partial[], allowList: Partial[], ssoClients: Partial[], + monitorList: Partial[], ) => { 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,6 +430,7 @@ describe("Test validation of Exemption CRs", () => { standardFlowEnabled: false, }, ], + [], ); await validator(mockReq); expect(mockReq.Approve).toHaveBeenCalledTimes(1); @@ -409,6 +449,7 @@ describe("Test validation of Exemption CRs", () => { }, }, ], + [], ); await validator(mockReq); expect(mockReq.Deny).toHaveBeenCalledTimes(1); @@ -425,6 +466,7 @@ describe("Test validation of Exemption CRs", () => { enableAuthserviceSelector: undefined, // explicitly undefined }, ], + [], ); await validator(mockReq); expect(mockReq.Approve).toHaveBeenCalledTimes(1); @@ -448,6 +490,7 @@ describe("Test Allowed SSO Client Attributes", () => { }, }, ], + [], ); await validator(mockReq); expect(mockReq.Deny).toHaveBeenCalledTimes(1); @@ -474,6 +517,7 @@ describe("Test Allowed SSO Client Attributes", () => { }, }, ], + [], ); await validator(mockReq); expect(mockReq.Approve).toHaveBeenCalledTimes(1); @@ -492,6 +536,7 @@ describe("Test Allowed SSO Client Attributes", () => { }, }, ], + [], ); await validator(mockReq); expect(mockReq.Deny).toHaveBeenCalledTimes(1); @@ -510,14 +555,48 @@ describe("Test Allowed SSO Client Attributes", () => { attributes: {}, }, ], + [], ); await validator(mockReq); expect(mockReq.Approve).toHaveBeenCalledTimes(1); }); it("allows clients with no attributes defined", async () => { - const mockReq = makeMockReq({}, [], [], [{}]); + const mockReq = makeMockReq({}, [], [], [{}], []); await validator(mockReq); expect(mockReq.Approve).toHaveBeenCalledTimes(1); }); }); + +describe("Test proper generation of a unique name for service monitors", () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it("given an undefined description, a unique serviceMonitor name should be generated using the selector and portName fields", async () => { + const mockReq = makeMockReq( + {}, + [], + [], + [], + [ + { description: undefined, portName: "http-foo", selector: { key: "foo" } }, + { description: undefined, portName: "http-bar", selector: { key: "bar" } }, + ], + ); + await validator(mockReq); + expect(mockReq.Deny).toHaveBeenCalledTimes(0); + }); + + 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); + }); +}); diff --git a/src/pepr/operator/crd/validators/package-validator.ts b/src/pepr/operator/crd/validators/package-validator.ts index 9283f508c..3ff046d10 100644 --- a/src/pepr/operator/crd/validators/package-validator.ts +++ b/src/pepr/operator/crd/validators/package-validator.ts @@ -141,5 +141,24 @@ export async function validator(req: PeprValidateRequest) { } } + const monitors = pkg.spec?.monitor ?? []; + + // Ensure serviceMonitors use a unique description or selector/portName used for generating the resource name + const monitorNames = new Set(); + + for (const monitor of monitors) { + const monitorName = sanitizeResourceName( + monitor.description || `${Object.values(monitor.selector)}-${monitor.portName}`, + ); + + if (monitorNames.has(monitorName)) { + return req.Deny( + `A serviceMonitor resource generated from the Package, ${pkg.metadata?.name} contains a duplicate name, please provide a unique description for each item in the monitor array`, + ); + } + + monitorNames.add(monitorName); + } + return req.Approve(); }