envoyproxy / envoyproxy/gateway
Misleading docs for ClusterSettings CRD: `Retry` policy doesn't apply for all features
- Dominant language
- Go
- Stars
- 3k
- Forks
- 864
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 140
Description
## Background
[ClusterSettings](https://gateway.envoyproxy.io/docs/api/extension_types/#clustersettings) EG CRD is used by many other features, such as:
- [ExtProc](https://gateway.envoyproxy.io/docs/api/extension_types/#extproc)
- [GRPCExtAuthService](https://gateway.envoyproxy.io/docs/api/extension_types/#grpcextauthservice)
- [OIDCProvider](https://gateway.envoyproxy.io/docs/api/extension_types/#oidcprovider)
- etc.
## Problem
The `Retry` field in `ClusterSettings` only applies for some of the features. However, if the policy is specified for a feature that doesn't support `Retry` today in EG, the policy is silently accepted but the fields will be NOOP.
### Example
For example, when a user specifies the following `SecurityPolicy` for ext auth:
```
securityPolicies:
- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
namespace: default
name: policy-for-http-route-1
spec:
targetRef:
group: gateway.networking.k8s.io
kind: HTTPRoute
name: httproute-1
extAuth:
failOpen: true
headersToExtAuth:
- header1
- header2
grpc:
backendRefs:
- group: multicluster.x-k8s.io
kind: ServiceImport
name: grpc-backend
port: 9000
backendSettings:
#### Start of problematic ClusterSettings fields ####
retry:
numRetries: 2
retryOn:
triggers:
- 5xx
- unavailable
- internal
- deadline-exceeded
perRetry:
timeout: "3s"
backOff:
baseInterval: "0.2s"
maxInterval: "3s"
```
The entire `backendSettings::retry` field is accepted but is a NOOP in the outputted xDS.
## Where does `ClusterSettings::Retry` apply?
Each feature needs to implement the retry policy. It is currently only implemented for:
- [OIDC](https://github.com/envoyproxy/gateway/blob/66e63aad27f4409e15c41c587562d09d69a4ae8a/internal/xds/translator/oidc.go#L221)
- [JWT Authn](https://github.com/envoyproxy/gateway/blob/eb7266c042f4573d7e67c9ea02a19e0deff6e17f/internal/xds/translator/jwt.go#L169)
## Asks
1. Implement the retry feature for SecurityPolicy ext authz (specifically, `GRPCExtAuthService` for Confluent's use case). This looks fairly straightforward and I'm happy to put a PR together for it.
2. (more importantly) reject CRs with some sort of "unimplemented" status if the user accidently specifies a "valid" config that is a NOOP. e.g. they specify a `HTTPExtAuthService` with the `Retry` field set.
cc @zhaohuabing to keep me honest about retry policies
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.