Use BackendCluster to represent OIDCProvider (envoyproxy#4227)

* gateway api translation Signed-off-by: Huabing Zhao <[email protected]> * gateway api translation Signed-off-by: Huabing Zhao <[email protected]> * xds translation Signed-off-by: Huabing Zhao <[email protected]> * add cel validation Signed-off-by: Huabing Zhao <[email protected]> * add cel validation Signed-off-by: Huabing Zhao <[email protected]> * CEL validation Signed-off-by: Huabing Zhao <[email protected]> * add CEL validation Signed-off-by: Huabing Zhao <[email protected]> * add retry trigger Signed-off-by: Huabing Zhao <[email protected]> * refactory Signed-off-by: Huabing Zhao <[email protected]> * fix gen Signed-off-by: Huabing Zhao <[email protected]> * add e2e test for OIDC provider with BackendCluster Signed-off-by: Huabing Zhao <[email protected]> * add dump Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * Update internal/gatewayapi/ext_service.go Co-authored-by: Arko Dasgupta <[email protected]> Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]>
zhaohuabing · Oct 18, 2024 · a351c4b · a351c4b
1 parent 5375cf0
commit a351c4b
Show file tree

Hide file tree

Showing 29 changed files with 1,464 additions and 332 deletions.
diff --git a/api/v1alpha1/oidc_types.go b/api/v1alpha1/oidc_types.go
@@ -107,7 +107,8 @@ type OIDC struct {
 
 // OIDCProvider defines the OIDC Provider configuration.
 // +kubebuilder:validation:XValidation:rule="!has(self.backendRef)",message="BackendRefs must be used, backendRef is not supported."
-// +kubebuilder:validation:XValidation:rule="has(self.backendRefs)? self.backendRefs.size() > 1 : true",message="Only one backendRefs is allowed."
+// +kubebuilder:validation:XValidation:rule="has(self.backendSettings)? (has(self.backendSettings.retry)?(has(self.backendSettings.retry.perRetry)? !has(self.backendSettings.retry.perRetry.timeout):true):true):true",message="Retry timeout is not supported."
+// +kubebuilder:validation:XValidation:rule="has(self.backendSettings)? (has(self.backendSettings.retry)?(has(self.backendSettings.retry.retryOn)? !has(self.backendSettings.retry.retryOn.httpStatusCodes):true):true):true",message="HTTPStatusCodes is not supported."
 type OIDCProvider struct {
 	// BackendRefs is used to specify the address of the OIDC Provider.
 	// If the BackendRefs is not specified, The host and port of the OIDC Provider's token endpoint

diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml
@@ -3261,9 +3261,12 @@ spec:
                     x-kubernetes-validations:
                     - message: BackendRefs must be used, backendRef is not supported.
                       rule: '!has(self.backendRef)'
-                    - message: Only one backendRefs is allowed.
-                      rule: 'has(self.backendRefs)? self.backendRefs.size() > 1 :
-                        true'
+                    - message: Retry timeout is not supported.
+                      rule: has(self.backendSettings)? (has(self.backendSettings.retry)?(has(self.backendSettings.retry.perRetry)?
+                        !has(self.backendSettings.retry.perRetry.timeout):true):true):true
+                    - message: HTTPStatusCodes is not supported.
+                      rule: has(self.backendSettings)? (has(self.backendSettings.retry)?(has(self.backendSettings.retry.retryOn)?
+                        !has(self.backendSettings.retry.retryOn.httpStatusCodes):true):true):true
                   redirectURL:
                     description: |-
                       The redirect URL to be used in the OIDC

diff --git a/go.mod b/go.mod
@@ -159,7 +159,7 @@ require (
 	github.com/moby/patternmatcher v0.6.0 // indirect
 	github.com/moby/sys/mountinfo v0.7.2 // indirect
 	github.com/moby/sys/sequential v0.5.0 // indirect
-	github.com/moby/sys/user v0.2.0 // indirect
+	github.com/moby/sys/user v0.3.0 // indirect
 	github.com/moby/sys/userns v0.1.0 // indirect
 	github.com/onsi/ginkgo v1.16.5 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect

diff --git a/go.sum b/go.sum
@@ -601,8 +601,8 @@ github.com/moby/sys/mountinfo v0.7.2 h1:1shs6aH5s4o5H2zQLn796ADW1wMrIwHsyJ2v9Kou
 github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4=
 github.com/moby/sys/sequential v0.5.0 h1:OPvI35Lzn9K04PBbCLW0g4LcFAJgHsvXsRyewg5lXtc=
 github.com/moby/sys/sequential v0.5.0/go.mod h1:tH2cOOs5V9MlPiXcQzRC+eEyab644PWKGRYaaV5ZZlo=
-github.com/moby/sys/user v0.2.0 h1:OnpapJsRp25vkhw8TFG6OLJODNh/3rEwRWtJ3kakwRM=
-github.com/moby/sys/user v0.2.0/go.mod h1:RYstrcWOJpVh+6qzUqp2bU3eaRpdiQeKGlKitaH0PM8=
+github.com/moby/sys/user v0.3.0 h1:9ni5DlcW5an3SvRSx4MouotOygvzaXbaSrc/wGDFWPo=
+github.com/moby/sys/user v0.3.0/go.mod h1:bG+tYYYJgaMtRKgEmuueC0hJEAZWwtIbZTB+85uoHjs=
 github.com/moby/sys/userns v0.1.0 h1:tVLXkFOxVu9A64/yh59slHVv9ahO9UIev4JZusOLG/g=
 github.com/moby/sys/userns v0.1.0/go.mod h1:IHUYgu/kao6N8YZlp9Cf444ySSvCmDlmzUcYfDHOl28=
 github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0=

diff --git a/internal/gatewayapi/backendtrafficpolicy.go b/internal/gatewayapi/backendtrafficpolicy.go
@@ -16,7 +16,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/utils/ptr"
 	gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
@@ -324,7 +323,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 		errs = errors.Join(errs, err)
 	}
 	if policy.Spec.Retry != nil {
-		rt = t.buildRetry(policy)
+		rt = buildRetry(policy.Spec.Retry)
 	}
 	if to, err = buildClusterSettingsTimeout(policy.Spec.ClusterSettings, nil); err != nil {
 		err = perr.WithMessage(err, "Timeout")
@@ -460,7 +459,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 		errs = errors.Join(errs, err)
 	}
 	if policy.Spec.Retry != nil {
-		rt = t.buildRetry(policy)
+		rt = buildRetry(policy.Spec.Retry)
 	}
 	if ct, err = buildClusterSettingsTimeout(policy.Spec.ClusterSettings, nil); err != nil {
 		err = perr.WithMessage(err, "Timeout")
@@ -812,67 +811,6 @@ func (t *Translator) buildFaultInjection(policy *egv1a1.BackendTrafficPolicy) *i
 	return fi
 }
 
-func (t *Translator) buildRetry(policy *egv1a1.BackendTrafficPolicy) *ir.Retry {
-	var rt *ir.Retry
-	if policy.Spec.Retry != nil {
-		prt := policy.Spec.Retry
-		rt = &ir.Retry{}
-
-		if prt.NumRetries != nil {
-			rt.NumRetries = ptr.To(uint32(*prt.NumRetries))
-		}
-
-		if prt.RetryOn != nil {
-			ro := &ir.RetryOn{}
-			bro := false
-			if prt.RetryOn.HTTPStatusCodes != nil {
-				ro.HTTPStatusCodes = makeIrStatusSet(prt.RetryOn.HTTPStatusCodes)
-				bro = true
-			}
-
-			if prt.RetryOn.Triggers != nil {
-				ro.Triggers = makeIrTriggerSet(prt.RetryOn.Triggers)
-				bro = true
-			}
-
-			if bro {
-				rt.RetryOn = ro
-			}
-		}
-
-		if prt.PerRetry != nil {
-			pr := &ir.PerRetryPolicy{}
-			bpr := false
-
-			if prt.PerRetry.Timeout != nil {
-				pr.Timeout = prt.PerRetry.Timeout
-				bpr = true
-			}
-
-			if prt.PerRetry.BackOff != nil {
-				if prt.PerRetry.BackOff.MaxInterval != nil || prt.PerRetry.BackOff.BaseInterval != nil {
-					bop := &ir.BackOffPolicy{}
-					if prt.PerRetry.BackOff.MaxInterval != nil {
-						bop.MaxInterval = prt.PerRetry.BackOff.MaxInterval
-					}
-
-					if prt.PerRetry.BackOff.BaseInterval != nil {
-						bop.BaseInterval = prt.PerRetry.BackOff.BaseInterval
-					}
-					pr.BackOff = bop
-					bpr = true
-				}
-			}
-
-			if bpr {
-				rt.PerRetry = pr
-			}
-		}
-	}
-
-	return rt
-}
-
 func makeIrStatusSet(in []egv1a1.HTTPStatus) []ir.HTTPStatus {
 	statusSet := sets.NewInt()
 	for _, r := range in {

diff --git a/internal/gatewayapi/clustersettings.go b/internal/gatewayapi/clustersettings.go
@@ -71,6 +71,8 @@ func translateTrafficFeatures(policy *egv1a1.ClusterSettings) (*ir.TrafficFeatur
 		ret.HTTP2 = h2
 	}
 
+	ret.Retry = buildRetry(policy.Retry)
+
 	// If nothing was set in any of the above calls, return nil instead of an empty
 	// container
 	var empty ir.TrafficFeatures
@@ -507,3 +509,64 @@ func translateDNS(policy egv1a1.ClusterSettings) *ir.DNS {
 		DNSRefreshRate: policy.DNS.DNSRefreshRate,
 	}
 }
+
+func buildRetry(r *egv1a1.Retry) *ir.Retry {
+	if r == nil {
+		return nil
+	}
+
+	rt := &ir.Retry{}
+
+	if r.NumRetries != nil {
+		rt.NumRetries = ptr.To(uint32(*r.NumRetries))
+	}
+
+	if r.RetryOn != nil {
+		ro := &ir.RetryOn{}
+		bro := false
+		if r.RetryOn.HTTPStatusCodes != nil {
+			ro.HTTPStatusCodes = makeIrStatusSet(r.RetryOn.HTTPStatusCodes)
+			bro = true
+		}
+
+		if r.RetryOn.Triggers != nil {
+			ro.Triggers = makeIrTriggerSet(r.RetryOn.Triggers)
+			bro = true
+		}
+
+		if bro {
+			rt.RetryOn = ro
+		}
+	}
+
+	if r.PerRetry != nil {
+		pr := &ir.PerRetryPolicy{}
+		bpr := false
+
+		if r.PerRetry.Timeout != nil {
+			pr.Timeout = r.PerRetry.Timeout
+			bpr = true
+		}
+
+		if r.PerRetry.BackOff != nil {
+			if r.PerRetry.BackOff.MaxInterval != nil || r.PerRetry.BackOff.BaseInterval != nil {
+				bop := &ir.BackOffPolicy{}
+				if r.PerRetry.BackOff.MaxInterval != nil {
+					bop.MaxInterval = r.PerRetry.BackOff.MaxInterval
+				}
+
+				if r.PerRetry.BackOff.BaseInterval != nil {
+					bop.BaseInterval = r.PerRetry.BackOff.BaseInterval
+				}
+				pr.BackOff = bop
+				bpr = true
+			}
+		}
+
+		if bpr {
+			rt.PerRetry = pr
+		}
+	}
+
+	return rt
+}
diff --git a/internal/gatewayapi/envoyextensionpolicy.go b/internal/gatewayapi/envoyextensionpolicy.go
@@ -411,7 +411,7 @@ func (t *Translator) buildExtProcs(policy *egv1a1.EnvoyExtensionPolicy, resource
 
 	for idx, ep := range policy.Spec.ExtProc {
 		name := irConfigNameForExtProc(policy, idx)
-		extProcIR, err := t.buildExtProc(name, utils.NamespacedName(policy), ep, idx, resources, envoyProxy)
+		extProcIR, err := t.buildExtProc(name, policy, ep, idx, resources, envoyProxy)
 		if err != nil {
 			return nil, err
 		}
@@ -422,59 +422,33 @@ func (t *Translator) buildExtProcs(policy *egv1a1.EnvoyExtensionPolicy, resource
 
 func (t *Translator) buildExtProc(
 	name string,
-	policyNamespacedName types.NamespacedName,
+	policy *egv1a1.EnvoyExtensionPolicy,
 	extProc egv1a1.ExtProc,
 	extProcIdx int,
 	resources *resource.Resources,
 	envoyProxy *egv1a1.EnvoyProxy,
 ) (*ir.ExtProc, error) {
 	var (
-		ds        *ir.DestinationSetting
+		rd        *ir.RouteDestination
 		authority string
 		err       error
 	)
 
-	var dsl []*ir.DestinationSetting
-	for i := range extProc.BackendRefs {
-		if err = t.validateExtServiceBackendReference(
-			&extProc.BackendRefs[i].BackendObjectReference,
-			policyNamespacedName.Namespace,
-			egv1a1.KindEnvoyExtensionPolicy,
-			resources); err != nil {
-			return nil, err
-		}
-
-		ds, err = t.processExtServiceDestination(
-			&extProc.BackendRefs[i],
-			policyNamespacedName,
-			egv1a1.KindEnvoyExtensionPolicy,
-			ir.GRPC,
-			resources,
-			envoyProxy,
-		)
-		if err != nil {
-			return nil, err
-		}
-
-		dsl = append(dsl, ds)
-	}
-
-	rd := ir.RouteDestination{
-		Name:     irIndexedExtServiceDestinationName(policyNamespacedName, egv1a1.KindEnvoyExtensionPolicy, extProcIdx),
-		Settings: dsl,
+	if rd, err = t.translateExtServiceBackendRefs(policy, extProc.BackendRefs, ir.GRPC, resources, envoyProxy, extProcIdx); err != nil {
+		return nil, err
 	}
 
 	if extProc.BackendRefs[0].Port != nil {
 		authority = fmt.Sprintf(
 			"%s.%s:%d",
 			extProc.BackendRefs[0].Name,
-			NamespaceDerefOr(extProc.BackendRefs[0].Namespace, policyNamespacedName.Namespace),
+			NamespaceDerefOr(extProc.BackendRefs[0].Namespace, policy.Namespace),
 			*extProc.BackendRefs[0].Port)
 	} else {
 		authority = fmt.Sprintf(
 			"%s.%s",
 			extProc.BackendRefs[0].Name,
-			NamespaceDerefOr(extProc.BackendRefs[0].Namespace, policyNamespacedName.Namespace))
+			NamespaceDerefOr(extProc.BackendRefs[0].Namespace, policy.Namespace))
 	}
 
 	traffic, err := translateTrafficFeatures(extProc.BackendCluster.BackendSettings)
@@ -484,7 +458,7 @@ func (t *Translator) buildExtProc(
 
 	extProcIR := &ir.ExtProc{
 		Name:        name,
-		Destination: rd,
+		Destination: *rd,
 		Traffic:     traffic,
 		Authority:   authority,
 	}

diff --git a/internal/gatewayapi/ext_service.go b/internal/gatewayapi/ext_service.go
@@ -12,15 +12,66 @@ import (
 
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 	"github.com/envoyproxy/gateway/internal/gatewayapi/resource"
 	"github.com/envoyproxy/gateway/internal/ir"
+	"github.com/envoyproxy/gateway/internal/utils"
 )
 
-// TODO: zhaohuabing combine this function with the one in the route translator
+// translateExtServiceBackendRefs translates external service backend references to route destinations.
+func (t *Translator) translateExtServiceBackendRefs(
+	policy client.Object,
+	backendRefs []egv1a1.BackendRef,
+	protocol ir.AppProtocol,
+	resources *resource.Resources,
+	envoyProxy *egv1a1.EnvoyProxy,
+	index int, // index is used to differentiate between multiple external services in the same policy
+) (*ir.RouteDestination, error) {
+	var (
+		rs  *ir.RouteDestination
+		ds  []*ir.DestinationSetting
+		err error
+	)
+
+	if len(backendRefs) == 0 {
+		return nil, errors.New("no backendRefs found for external service")
+	}
+
+	pnn := utils.NamespacedName(policy)
+	for _, backendRef := range backendRefs {
+		if err = t.validateExtServiceBackendReference(
+			&backendRef.BackendObjectReference,
+			policy.GetNamespace(),
+			policy.GetObjectKind().GroupVersionKind().Kind,
+			resources); err != nil {
+			return nil, err
+		}
+
+		var extServiceDest *ir.DestinationSetting
+		if extServiceDest, err = t.processExtServiceDestination(
+			&backendRef,
+			pnn,
+			policy.GetObjectKind().GroupVersionKind().Kind,
+			protocol,
+			resources,
+			envoyProxy,
+		); err != nil {
+			return nil, err
+		}
+		ds = append(ds, extServiceDest)
+	}
+
+	rs = &ir.RouteDestination{
+		Name:     irIndexedExtServiceDestinationName(pnn, policy.GetObjectKind().GroupVersionKind().Kind, index),
+		Settings: ds,
+	}
+	return rs, nil
+}
+
 func (t *Translator) processExtServiceDestination(
 	backendRef *egv1a1.BackendRef,
 	policyNamespacedName types.NamespacedName,