envoyproxy / envoyproxy/gateway

Misleading docs for ClusterSettings CRD: `Retry` policy doesn't apply for all features

Open
#6,914 3 comments 1 reaction 0 assignees View on GitHub
stale triage
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.