From f72618eec83af185f6b388ecbb92f1dbf5fb1e2a Mon Sep 17 00:00:00 2001 From: Brandon Squizzato Date: Fri, 14 Aug 2026 16:52:27 -0400 Subject: [PATCH 1/2] fix(gateway): pass NamespaceConfig by pointer to reconcileKeycloakClient reconcileKeycloakClient received nsConfig by value, so its OIDC mutation (nsConfig.Gateway.OIDC = oidcConfig) was invisible to the caller. deployGateway then rendered gateway.toml with an empty OIDC config, producing gateways without an [openshell.gateway.oidc] section. The UpdateOIDC callback persisted the OIDC data to the API server, but the phase gate prevented re-reconciliation, so the gateway never self-healed. Pass *NamespaceConfig so the OIDC config is visible when deployGateway renders the ConfigMap. Co-Authored-By: Claude Opus 4.6 --- components/control-plane/internal/gateway/reconciler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/control-plane/internal/gateway/reconciler.go b/components/control-plane/internal/gateway/reconciler.go index 88914af..f5578a0 100644 --- a/components/control-plane/internal/gateway/reconciler.go +++ b/components/control-plane/internal/gateway/reconciler.go @@ -88,7 +88,7 @@ func ReconcileGateway( } if opts.Keycloak != nil { - if err := reconcileKeycloakClient(ctx, opts, nsConfig); err != nil { + if err := reconcileKeycloakClient(ctx, opts, &nsConfig); err != nil { return fmt.Errorf("reconcile keycloak client in %s: %w", nsConfig.Name, err) } } @@ -885,7 +885,7 @@ func reconcileDatabaseCredentials(ctx context.Context, clientset *kubernetes.Cli return nil } -func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig NamespaceConfig) error { +func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig *NamespaceConfig) error { kc := keycloak.NewClient( opts.Keycloak.ServerURL, opts.Keycloak.Realm, From f729bdd1eb9460699d240e5142d0b4856fec0264 Mon Sep 17 00:00:00 2001 From: Brandon Squizzato Date: Fri, 14 Aug 2026 16:55:10 -0400 Subject: [PATCH 2/2] fix(keycloak): use {name}-{id} format for Keycloak client IDs The spec requires clientId = "{name}-{id}" to prevent name clashes when gateways share a name across fleets or are deleted and recreated. The code was using just the gateway name, so two gateways named "my-gateway" would collide in Keycloak. Apply the {name}-{id} format in all three paths: - reconcileKeycloakClient (provisioning) - DeleteGatewayResources (cleanup) - RoleBindingReconciler.resolveKeycloakClientID (role assignment) Co-Authored-By: Claude Opus 4.6 --- .../internal/gateway/reconciler.go | 32 +++++++++++-------- .../reconciler/role_binding_reconciler.go | 6 ++-- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/components/control-plane/internal/gateway/reconciler.go b/components/control-plane/internal/gateway/reconciler.go index f5578a0..84699a3 100644 --- a/components/control-plane/internal/gateway/reconciler.go +++ b/components/control-plane/internal/gateway/reconciler.go @@ -192,11 +192,12 @@ func DeleteGatewayResources( log.Printf("INFO deleted ClusterRoleBinding %s", crbName) } - if opts.KeycloakClient != nil && opts.GatewayName != "" { - if err := opts.KeycloakClient.DeleteGatewayClient(ctx, opts.GatewayName); err != nil { - log.Printf("WARN failed to delete keycloak client %s (orphaned): %v", opts.GatewayName, err) + if opts.KeycloakClient != nil && opts.GatewayName != "" && opts.GatewayID != "" { + kcClientID := fmt.Sprintf("%s-%s", opts.GatewayName, opts.GatewayID) + if err := opts.KeycloakClient.DeleteGatewayClient(ctx, kcClientID); err != nil { + log.Printf("WARN failed to delete keycloak client %s (orphaned): %v", kcClientID, err) } else { - log.Printf("INFO deleted keycloak client %s", opts.GatewayName) + log.Printf("INFO deleted keycloak client %s", kcClientID) } } @@ -893,30 +894,33 @@ func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig * opts.Keycloak.ClientSecret, ) - gatewayName := opts.GatewayName - if gatewayName == "" { + if opts.GatewayName == "" { return fmt.Errorf("gateway name is required for keycloak provisioning") } + if opts.GatewayID == "" { + return fmt.Errorf("gateway ID is required for keycloak provisioning") + } + kcClientID := fmt.Sprintf("%s-%s", opts.GatewayName, opts.GatewayID) - existingUUID, err := kc.GetClientUUID(ctx, gatewayName) + existingUUID, err := kc.GetClientUUID(ctx, kcClientID) if err != nil { return fmt.Errorf("check existing keycloak client: %w", err) } if existingUUID != "" { - log.Printf("INFO keycloak client %s already exists (uuid=%s), skipping provisioning", gatewayName, existingUUID) + log.Printf("INFO keycloak client %s already exists (uuid=%s), skipping provisioning", kcClientID, existingUUID) } else { - clientUUID, err := kc.ProvisionGatewayClient(ctx, gatewayName) + clientUUID, err := kc.ProvisionGatewayClient(ctx, kcClientID) if err != nil { - return fmt.Errorf("provision keycloak client %s: %w", gatewayName, err) + return fmt.Errorf("provision keycloak client %s: %w", kcClientID, err) } - log.Printf("INFO provisioned keycloak client %s (uuid=%s)", gatewayName, clientUUID) + log.Printf("INFO provisioned keycloak client %s (uuid=%s)", kcClientID, clientUUID) } oidcConfig := OIDCConfig{ Issuer: kc.Issuer(), - ClientID: gatewayName, - Audience: gatewayName, + ClientID: kcClientID, + Audience: kcClientID, JwksTTL: 3600, RolesClaim: "hypershell.roles", AdminRole: "openshell-admin", @@ -938,7 +942,7 @@ func reconcileKeycloakClient(ctx context.Context, opts ReconcileOpts, nsConfig * return fmt.Errorf("marshal oidc config: %w", err) } if err := opts.UpdateOIDC(ctx, string(oidcJSON)); err != nil { - log.Printf("WARN failed to persist oidc config for %s: %v", gatewayName, err) + log.Printf("WARN failed to persist oidc config for %s: %v", kcClientID, err) } } diff --git a/components/control-plane/internal/reconciler/role_binding_reconciler.go b/components/control-plane/internal/reconciler/role_binding_reconciler.go index 33d09d9..3f26743 100644 --- a/components/control-plane/internal/reconciler/role_binding_reconciler.go +++ b/components/control-plane/internal/reconciler/role_binding_reconciler.go @@ -98,15 +98,15 @@ func (r *RoleBindingReconciler) Handle(ctx context.Context, event watcher.Event[ return nil } -// resolveKeycloakClientID looks up the gateway by ID and returns the gateway -// name, which is used directly as the Keycloak client ID. +// resolveKeycloakClientID looks up the gateway by ID and returns the Keycloak +// client ID in the {name}-{id} format specified by the Keycloak provisioning spec. func (r *RoleBindingReconciler) resolveKeycloakClientID(ctx context.Context, gatewayID string) (string, error) { client := pb.NewGatewayServiceClient(r.grpcConn) resp, err := client.GetGateway(ctx, &pb.GetGatewayRequest{Id: gatewayID}) if err != nil { return "", fmt.Errorf("get gateway %s: %w", gatewayID, err) } - return resp.GetGateway().GetName(), nil + return fmt.Sprintf("%s-%s", resp.GetGateway().GetName(), gatewayID), nil } // assignClientRoleWithRetry retries AssignClientRole to handle the race where