fix: handle client id names with special characters (#659)

## Description

This PR ensures that the using certain client id names will get handled
properly when creating child resources.

Authservice is a special case due to an upstream issue:
https://www.github.com/istio-ecosystem/authservice/issues/263

## Related Issue

Fixes #451

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

src/pepr/operator/controllers/keycloak/authservice/authorization-policy.ts

@@ -6,7 +6,7 @@ import {
   IstioRequestAuthentication,
   UDSPackage,
 } from "../../../crd";
-import { getOwnerRef, purgeOrphans } from "../../utils";
+import { getOwnerRef, purgeOrphans, sanitizeResourceName } from "../../utils";
 import { log } from "./authservice";
 import { Action as AuthServiceAction, AuthServiceEvent } from "./types";
 
@@ -26,7 +26,7 @@ function authserviceAuthorizationPolicy(
   return {
     kind: "AuthorizationPolicy",
     metadata: {
-      name: `${name}-authservice`,
+      name: sanitizeResourceName(`${name}-authservice`),
       namespace,
     },
     spec: {
@@ -59,7 +59,7 @@ function jwtAuthZAuthorizationPolicy(
   return {
     kind: "AuthorizationPolicy",
     metadata: {
-      name: `${name}-jwt-authz`,
+      name: sanitizeResourceName(`${name}-jwt-authz`),
       namespace,
     },
     spec: {
@@ -89,7 +89,7 @@ function authNRequestAuthentication(
   return {
     kind: "RequestAuthentication",
     metadata: {
-      name: `${name}-jwt-authn`,
+      name: sanitizeResourceName(`${name}-jwt-authn`),
       namespace,
     },
     spec: {

src/pepr/operator/controllers/keycloak/client-sync.ts

@@ -4,7 +4,7 @@ import { UDSConfig } from "../../../config";
 import { Component, setupLogger } from "../../../logger";
 import { Store } from "../../common";
 import { Sso, UDSPackage } from "../../crd";
-import { getOwnerRef, purgeOrphans } from "../utils";
+import { getOwnerRef, purgeOrphans, sanitizeResourceName } from "../utils";
 import { Client, clientKeys } from "./types";
 
 let apiURL =
@@ -187,11 +187,12 @@ async function syncClient(
   // Create or update the client secret
   if (!client.publicClient) {
     const generation = (pkg.metadata?.generation ?? 0).toString();
+    const sanitizedSecretName = sanitizeResourceName(secretName || name);
     await K8s(kind.Secret).Apply({
       metadata: {
         namespace: pkg.metadata!.namespace,
         // Use the CR secret name if provided, otherwise use the client name
-        name: secretName || name,
+        name: sanitizedSecretName,
         labels: {
           "uds/package": pkg.metadata!.name,
           "uds/generation": generation,

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

@@ -395,6 +395,40 @@ describe("Test validation of Exemption CRs", () => {
     await validator(mockReq);
     expect(mockReq.Approve).toHaveBeenCalledTimes(1);
   });
+
+  it("denies authservice clients with : in client ID", async () => {
+    const mockReq = makeMockReq(
+      {},
+      [],
+      [],
+      [
+        {
+          clientId: "http://example.com",
+          enableAuthserviceSelector: {
+            app: "foobar",
+          },
+        },
+      ],
+    );
+    await validator(mockReq);
+    expect(mockReq.Deny).toHaveBeenCalledTimes(1);
+  });
+
+  it("allows non-authservice clients with : in client ID", async () => {
+    const mockReq = makeMockReq(
+      {},
+      [],
+      [],
+      [
+        {
+          clientId: "http://example.com",
+          enableAuthserviceSelector: undefined, // explicitly undefined
+        },
+      ],
+    );
+    await validator(mockReq);
+    expect(mockReq.Approve).toHaveBeenCalledTimes(1);
+  });
 });
 
 describe("Test Allowed SSO Client Attributes", () => {

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

@@ -133,6 +133,12 @@ export async function validator(req: PeprValidateRequest<UDSPackage>) {
         }
       }
     }
+    // If this is an authservice client ensure it does not contain a `:`, see https://github.com/istio-ecosystem/authservice/issues/263
+    if (client.enableAuthserviceSelector && client.clientId.includes(":")) {
+      return req.Deny(
+        `The client ID "${client.clientId}" is invalid as an Authservice client - Authservice does not support client IDs with the ":" character`,
+      );
+    }
   }
 
   return req.Approve();

src/test/tasks.yaml

@@ -15,13 +15,29 @@ tasks:
             name: httpbin
             namespace: test-admin-app
 
+      - description: Verify admin package CR is ready
+        wait:
+          cluster:
+            kind: Package
+            name: httpbin
+            namespace: test-admin-app
+            condition: "'{.status.phase}'=Ready"
+
       - description: Wait for the tenant app to be ready
         wait:
           cluster:
             kind: Deployment
             name: http-echo-multi-port
             namespace: test-tenant-app
 
+      - description: Verify tenant package CR is ready
+        wait:
+          cluster:
+            kind: Package
+            name: test-tenant-app
+            namespace: test-tenant-app
+            condition: "'{.status.phase}'=Ready"
+
       - description: Verify the admin app is accessible
         wait:
           network:
@@ -57,6 +73,14 @@ tasks:
             address: demo-8081.uds.dev
             code: 200
 
+      - description: Verify authservice app package CR is ready
+        wait:
+          cluster:
+            kind: Package
+            name: httpbin-other
+            namespace: authservice-test-app
+            condition: "'{.status.phase}'=Ready"
+
       - description: Verify the authservice tenant app is accessible
         wait:
           network: