feat: getting OIDC client ID from secret

api/v1alpha1/oidc_types.go

@@ -10,28 +10,33 @@ import (
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 )
 
-const OIDCClientSecretKey = "client-secret"
+const (
+	OIDCClientSecretKey = "client-secret"
+	OIDCClientIDKey     = "client-id"
+)
 
 // OIDC defines the configuration for the OpenID Connect (OIDC) authentication.
+// +kubebuilder:validation:XValidation:rule="(has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID) && has(self.clientIDRef))", message="only one of clientID or clientIDRef must be set"
 type OIDC struct {
 	// The OIDC Provider configuration.
 	Provider OIDCProvider `json:"provider"`
 
 	// The client ID to be used in the OIDC
 	// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
 	//
+	// Only one of clientID or clientIDRef must be set.
+	// +optional
 	// +kubebuilder:validation:MinLength=1
-	ClientID string `json:"clientID"`
-
-	// TODO zhaohuabing make ClientID optional in the implementation PR
+	ClientID *string `json:"clientID,omitempty"`
 
 	// The Kubernetes secret which contains the client ID to be used in the
 	// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
 	// Exactly one of clientID or clientIDRef must be set.
 	// This is an Opaque secret. The client ID should be stored in the key "client-id".
 	//
+	// Only one of clientID or clientIDRef must be set.
+	//
 	// +optional
-	// +notImplementedHide
 	ClientIDRef *gwapiv1.SecretObjectReference `json:"clientIDRef,omitempty"`
 
 	// The Kubernetes secret which contains the OIDC client secret to be used in the

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml

@@ -3711,6 +3711,8 @@ spec:
                     description: |-
                       The client ID to be used in the OIDC
                       [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
+
+                      Only one of clientID or clientIDRef must be set.
                     minLength: 1
                     type: string
                   clientIDRef:
@@ -3719,6 +3721,8 @@ spec:
                       [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
                       Exactly one of clientID or clientIDRef must be set.
                       This is an Opaque secret. The client ID should be stored in the key "client-id".
+
+                      Only one of clientID or clientIDRef must be set.
                     properties:
                       group:
                         default: ""
@@ -4943,10 +4947,13 @@ spec:
                       type: string
                     type: array
                 required:
-                - clientID
                 - clientSecret
                 - provider
                 type: object
+                x-kubernetes-validations:
+                - message: only one of clientID or clientIDRef must be set
+                  rule: (has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID)
+                    && has(self.clientIDRef))
               targetRef:
                 description: |-
                   TargetRef is the name of the resource this policy is being attached to.

charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml

@@ -3710,6 +3710,8 @@ spec:
                     description: |-
                       The client ID to be used in the OIDC
                       [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
+
+                      Only one of clientID or clientIDRef must be set.
                     minLength: 1
                     type: string
                   clientIDRef:
@@ -3718,6 +3720,8 @@ spec:
                       [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
                       Exactly one of clientID or clientIDRef must be set.
                       This is an Opaque secret. The client ID should be stored in the key "client-id".
+
+                      Only one of clientID or clientIDRef must be set.
                     properties:
                       group:
                         default: ""
@@ -4942,10 +4946,13 @@ spec:
                       type: string
                     type: array
                 required:
-                - clientID
                 - clientSecret
                 - provider
                 type: object
+                x-kubernetes-validations:
+                - message: only one of clientID or clientIDRef must be set
+                  rule: (has(self.clientID) && !has(self.clientIDRef)) || (!has(self.clientID)
+                    && has(self.clientIDRef))
               targetRef:
                 description: |-
                   TargetRef is the name of the resource this policy is being attached to.

internal/gatewayapi/securitypolicy.go

@@ -1120,6 +1120,7 @@ func (t *Translator) buildOIDC(
 	var (
 		oidc                  = policy.Spec.OIDC
 		provider              *ir.OIDCProvider
+		clientID              string
 		clientSecret          *corev1.Secret
 		redirectURL           = defaultRedirectURL
 		redirectPath          = defaultRedirectPath
@@ -1140,6 +1141,25 @@ func (t *Translator) buildOIDC(
 		namespace: policy.Namespace,
 	}
 
+	// Client ID can be specified either as a string or as a reference to a secret.
+	switch {
+	case oidc.ClientID != nil:
+		clientID = *oidc.ClientID
+	case oidc.ClientIDRef != nil:
+		var clientIDSecret *corev1.Secret
+		if clientIDSecret, err = t.validateSecretRef(false, from, *oidc.ClientIDRef, resources); err != nil {
+			return nil, err
+		}
+		clientIDBytes, ok := clientIDSecret.Data[egv1a1.OIDCClientIDKey]
+		if !ok || len(clientIDBytes) == 0 {
+			return nil, fmt.Errorf("client ID not found in secret %s/%s", clientIDSecret.Namespace, clientIDSecret.Name)
+		}
+		clientID = string(clientIDBytes)
+	default:
+		// This is just a sanity check - the CRD validation should have caught this.
+		return nil, fmt.Errorf("client ID must be specified in OIDC policy %s/%s", policy.Namespace, policy.Name)
+	}
+
 	if clientSecret, err = t.validateSecretRef(
 		false, from, oidc.ClientSecret, resources); err != nil {
 		return nil, err
@@ -1198,7 +1218,7 @@ func (t *Translator) buildOIDC(
 	return &ir.OIDC{
 		Name:                   irConfigName(policy),
 		Provider:               *provider,
-		ClientID:               oidc.ClientID,
+		ClientID:               clientID,
 		ClientSecret:           clientSecretBytes,
 		Scopes:                 scopes,
 		Resources:              oidc.Resources,

internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml

@@ -13,13 +13,7 @@ secrets:
     name: client2-secret
   data:
     client-secret: Y2xpZW50MTpzZWNyZXQK
-- apiVersion: v1
-  kind: Secret
-  metadata:
-    namespace: default
-    name: client3-secret
-  data:
-    client-secret: Y2xpZW50MTpzZWNyZXQK
+    client-id: Y2xpZW50Mi5vYXV0aC5mb28uY29t
 - apiVersion: v1
   kind: Secret
   metadata:
@@ -121,7 +115,8 @@ securityPolicies:
         issuer: "https://oauth.foo.com"
         authorizationEndpoint: "https://oauth.foo.com/oauth2/v2/auth"
         tokenEndpoint: "https://oauth.foo.com/token"
-      clientID: "client2.oauth.foo.com"
+      clientIDRef:
+        name: "client2-secret"
       clientSecret:
         name: "client2-secret"
       scopes: ["openid", "email", "profile"]

internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml

@@ -146,7 +146,10 @@ securityPolicies:
     uid: 08335a80-83ba-4592-888f-6ac0bba44ce4
   spec:
     oidc:
-      clientID: client2.oauth.foo.com
+      clientIDRef:
+        group: null
+        kind: null
+        name: client2-secret
       clientSecret:
         group: null
         kind: null

internal/provider/kubernetes/controller.go

@@ -789,6 +789,25 @@ func (r *gatewayAPIReconciler) processSecurityPolicyObjectRefs(
 					"policy", policy, "secretRef", oidc.ClientSecret)
 			}
 
+			if oidc.ClientIDRef != nil {
+				if err := r.processSecretRef(
+					ctx,
+					resourceMap,
+					resourceTree,
+					resource.KindSecurityPolicy,
+					policy.Namespace,
+					policy.Name,
+					*oidc.ClientIDRef); err != nil {
+					// If the error is transient, we return it to allow Reconcile to retry.
+					if isTransientError(err) {
+						return err
+					}
+					r.log.Error(err,
+						"failed to process OIDC ClientIDRef for SecurityPolicy",
+						"policy", policy, "secretRef", oidc.ClientIDRef)
+				}
+			}
+
 			var backendRefs []gwapiv1.BackendObjectReference
 			if oidc.Provider.BackendRef != nil {
 				backendRefs = append(backendRefs, *oidc.Provider.BackendRef)

internal/provider/kubernetes/indexers.go

@@ -589,6 +589,9 @@ func secretSecurityPolicyIndexFunc(rawObj client.Object) []string {
 
 	if securityPolicy.Spec.OIDC != nil {
 		secretReferences = append(secretReferences, securityPolicy.Spec.OIDC.ClientSecret)
+		if securityPolicy.Spec.OIDC.ClientIDRef != nil {
+			secretReferences = append(secretReferences, *securityPolicy.Spec.OIDC.ClientIDRef)
+		}
 	}
 	if securityPolicy.Spec.APIKeyAuth != nil {
 		secretReferences = append(secretReferences, securityPolicy.Spec.APIKeyAuth.CredentialRefs...)

internal/provider/kubernetes/predicates_test.go

@@ -371,7 +371,7 @@ func TestValidateSecretForReconcile(t *testing.T) {
 								AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
 								TokenEndpoint:         ptr.To("https://oauth2.googleapis.com/token"),
 							},
-							ClientID: "client-id",
+							ClientID: ptr.To("client-id"),
 							ClientSecret: gwapiv1.SecretObjectReference{
 								Name: "secret",
 							},

release-notes/current.yaml

@@ -39,6 +39,7 @@ new features: |
   Added metric `watchable_publish_total` counting store events in watchable message queues.
   Added support for forwarding client ID header and sanitizing API keys for API Key authentication in SecurityPolicy.
   Accessloggers of type ALS now have http2 enabled on the cluster by default.
+  Added support for using Secret as a source of the OIDC client ID.
   Added support for listeners and routes in PostTranslateModifyHook extension hook.
   Added admin console support with web UI for the Envoy Gateway admin server.
 

site/content/en/latest/api/extension_types.md

@@ -3262,7 +3262,8 @@ _Appears in:_
 | Field | Type | Required | Default | Description |
 | ---   | ---  | ---      | ---     | ---         |
 | `provider` | _[OIDCProvider](#oidcprovider)_ |  true  |  | The OIDC Provider configuration. |
-| `clientID` | _string_ |  true  |  | The client ID to be used in the OIDC<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest). |
+| `clientID` | _string_ |  false  |  | The client ID to be used in the OIDC<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />Only one of clientID or clientIDRef must be set. |
+| `clientIDRef` | _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ |  false  |  | The Kubernetes secret which contains the client ID to be used in the<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />Exactly one of clientID or clientIDRef must be set.<br />This is an Opaque secret. The client ID should be stored in the key "client-id".<br />Only one of clientID or clientIDRef must be set. |
 | `clientSecret` | _[SecretObjectReference](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1.SecretObjectReference)_ |  true  |  | The Kubernetes secret which contains the OIDC client secret to be used in the<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />This is an Opaque secret. The client secret should be stored in the key<br />"client-secret". |
 | `cookieNames` | _[OIDCCookieNames](#oidccookienames)_ |  false  |  | The optional cookie name overrides to be used for Bearer and IdToken cookies in the<br />[Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).<br />If not specified, uses a randomly generated suffix |
 | `cookieConfig` | _[OIDCCookieConfig](#oidccookieconfig)_ |  false  |  | CookieConfigs allows setting the SameSite attribute for OIDC cookies.<br />By default, its unset. |

test/cel-validation/securitypolicy_test.go

@@ -1335,7 +1335,7 @@ func TestSecurityPolicyTarget(t *testing.T) {
 							AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
 							TokenEndpoint:         ptr.To("https://oauth2.googleapis.com/token"),
 						},
-						ClientID: "client-id",
+						ClientID: ptr.To("client-id"),
 						ClientSecret: gwapiv1b1.SecretObjectReference{
 							Name: "secret",
 						},
@@ -1380,7 +1380,7 @@ func TestSecurityPolicyTarget(t *testing.T) {
 							AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
 							TokenEndpoint:         ptr.To("https://oauth2.googleapis.com/token"),
 						},
-						ClientID: "client-id",
+						ClientID: ptr.To("client-id"),
 						ClientSecret: gwapiv1b1.SecretObjectReference{
 							Name: "secret",
 						},
@@ -1389,6 +1389,68 @@ func TestSecurityPolicyTarget(t *testing.T) {
 			},
 			wantErrors: []string{"Retry timeout is not supported", "HTTPStatusCodes is not supported"},
 		},
+		{
+			desc: "oidc-without-clientid",
+			mutate: func(sp *egv1a1.SecurityPolicy) {
+				sp.Spec = egv1a1.SecurityPolicySpec{
+					PolicyTargetReferences: egv1a1.PolicyTargetReferences{
+						TargetSelectors: []egv1a1.TargetSelector{
+							{
+								Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")),
+								Kind:  "HTTPRoute",
+								MatchLabels: map[string]string{
+									"eg/namespace": "reference-apps",
+								},
+							},
+						},
+					},
+					OIDC: &egv1a1.OIDC{
+						Provider: egv1a1.OIDCProvider{
+							Issuer:                "https://accounts.google.com",
+							AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
+							TokenEndpoint:         ptr.To("https://oauth2.googleapis.com/token"),
+						},
+						ClientSecret: gwapiv1b1.SecretObjectReference{
+							Name: "secret",
+						},
+					},
+				}
+			},
+			wantErrors: []string{"only one of clientID or clientIDRef must be set"},
+		},
+		{
+			desc: "oidc-two-clientids",
+			mutate: func(sp *egv1a1.SecurityPolicy) {
+				sp.Spec = egv1a1.SecurityPolicySpec{
+					PolicyTargetReferences: egv1a1.PolicyTargetReferences{
+						TargetSelectors: []egv1a1.TargetSelector{
+							{
+								Group: ptr.To(gwapiv1a2.Group("gateway.networking.k8s.io")),
+								Kind:  "HTTPRoute",
+								MatchLabels: map[string]string{
+									"eg/namespace": "reference-apps",
+								},
+							},
+						},
+					},
+					OIDC: &egv1a1.OIDC{
+						Provider: egv1a1.OIDCProvider{
+							Issuer:                "https://accounts.google.com",
+							AuthorizationEndpoint: ptr.To("https://accounts.google.com/o/oauth2/v2/auth"),
+							TokenEndpoint:         ptr.To("https://oauth2.googleapis.com/token"),
+						},
+						ClientID: ptr.To("client-id"),
+						ClientIDRef: &gwapiv1b1.SecretObjectReference{
+							Name: "secret",
+						},
+						ClientSecret: gwapiv1b1.SecretObjectReference{
+							Name: "secret",
+						},
+					},
+				}
+			},
+			wantErrors: []string{"only one of clientID or clientIDRef must be set"},
+		},
 	}
 
 	for _, tc := range cases {

test/e2e/testdata/oidc-securitypolicy-backendcluster.yaml

@@ -34,6 +34,14 @@ spec:
 ---
 apiVersion: v1
 kind: Secret
+metadata:
+  namespace: gateway-conformance-infra
+  name: oidctest-client-id
+data:
+  client-id: b2lkY3Rlc3Q=     # base64 encoding of "oidctest"
+---
+apiVersion: v1
+kind: Secret
 metadata:
   namespace: gateway-conformance-infra
   name: oidctest-secret
@@ -67,7 +75,8 @@ spec:
           retryOn:
             triggers: ["5xx", "gateway-error", "reset"]
       issuer: "https://keycloak.gateway-conformance-infra/realms/master"   # Test fetching auth endpoint from the issuer url
-    clientID: "oidctest"
+    clientIDRef:
+      name: "oidctest-client-id"
     clientSecret:
       name: "oidctest-secret"
     redirectURL: "http://www.example.com/myapp/oauth2/callback"