Add StatefulFormatter to ProxyConfig and mesh.ProxyConfig #3350
Conversation
|
😊 Welcome @whitneygriffith! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
istio-testing
added
the
size/M
whitneygriffith
force-pushed
the
stateful-formatters
branch
from
1a29603
to
f1c0513
Compare
Signed-off-by: whitneygriffith <[email protected]>
whitneygriffith
force-pushed
the
stateful-formatters
branch
from
f1c0513
to
7100825
Compare
Signed-off-by: whitneygriffith <[email protected]>
whitneygriffith
changed the title
Signed-off-by: whitneygriffith <[email protected]>
| // To preserve the header case for HTTP 1.x requests, set the `statefulFormatter.preserveCase` field on the `ProxyConfig` resource: | ||
| // ```yaml | ||
| // apiVersion: networking.istio.io/v1beta1 | ||
| // kind: ProxyConfig |
There was a problem hiding this comment.
I don't think we want to add this to the ProxyConfig resource; only the proxyConfig annotation
There was a problem hiding this comment.
Can you point me to the proxyConfig annotation you are referring to? From what I understand to enable preserveCase at the namespace or workload level this feature should be apart of the proxyConfig resource. Where the annotation is used to apply a specific proxyConfig resource to a workload.
There was a problem hiding this comment.
If you check the recording of the community meeting, the consensus was that we didn't want to make this too flexible/easy to use since we want to encourage users to adopt http2. Adding the field to the annotation and not the resource allows us to do so. Notice that the networking/v1beta1/proxy_config.proto file and the mesh/v1alpha1/proxy.proto files have different fields. The former is the CRD and the latter is the annotation struct; please add the new fields there
mesh/v1alpha1/proxy.proto
Outdated
| @@ -603,6 +603,9 @@ message ProxyConfig { | |||
| // Specifies the details of the proxy image. | |||
| istio.networking.v1beta1.ProxyImage image = 35; | |||
|
|
|||
| // Defines options for stateful header formatters that maintain context or state across the processing of a request. | |||
| istio.networking.v1beta1.StatefulFormatter stateful_formatter = 40; | |||
There was a problem hiding this comment.
I'm not sure I like the idea of passing the complexity of "stateful formatters" to the users who just want to preserve their header casing. Can we utilize the proxyHeaders struct in some way to make this more intuitive?
There was a problem hiding this comment.
I agree that we should avoid exposing the complexity of stateful formatters directly to users who are using the higher level meshConfig to enable header case preservation. I will look into adding preserveCase or preserveHeaderCase to the proxyHeaders struct.
Signed-off-by: whitneygriffith <[email protected]>
whitneygriffith
force-pushed
the
stateful-formatters
branch
from
c112077
to
2ddb0fa
Compare
Part of istio/istio#53680