feat: Implement disableMergeSlashes and escapedSlashesAction #2413
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2413 +/- ##
==========================================
- Coverage 64.67% 64.66% -0.01%
==========================================
Files 115 115
Lines 17326 17364 +38
==========================================
+ Hits 11206 11229 +23
- Misses 5409 5423 +14
- Partials 711 712 +1 ☔ View full report in Codecov by Sentry. |
| @@ -207,6 +207,8 @@ type HTTPListener struct { | |||
| // HTTP3 provides HTTP/3 configuration on the listener. | |||
| // +optional | |||
| HTTP3 *HTTP3Settings `json:"http3,omitempty"` | |||
| // Path contains settings for path URI manipulations | |||
| Path PathSettings `json:"path,omitempty"` | |||
There was a problem hiding this comment.
It doesn't make sense to have a pointer here. This is not an optional structure.
| @@ -262,6 +264,22 @@ func (t TLSListenerConfig) Validate() error { | |||
| return errs | |||
| } | |||
|
|
|||
| type PathEscapedSlashAction egv1a1.PathEscapedSlashAction | |||
There was a problem hiding this comment.
why not using egv1a1.PathEscapedSlashAction directly?
There was a problem hiding this comment.
Because if I use it directly, I would need to import egv1a1 into the XDS layer in order to translate the API constants into XDS constants.
| @@ -104,6 +104,10 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap | |||
| Address: "0.0.0.0", | |||
| Port: uint32(containerPort), | |||
| TLS: irTLSConfigs(listener.tlsSecrets), | |||
| Path: ir.PathSettings{ | |||
There was a problem hiding this comment.
maintaining logic in two places in the gatewayapi layer may be harder to reason with for the next dev trying to figure out translation, thoughts ?
instead if ir.path is nil, the xds translator layer could set defaults
There was a problem hiding this comment.
maintaining logic in two places in the gatewayapi layer may be harder to reason with for the next dev trying to figure out translation, thoughts ?
I think the IR should encapsulate what is expected of the XDS layer, and all defaults should be set in the IR layer. There should be no magic translations happening in the XDS layer - XDS translation should be a technical translation without filling in any controlled attribute.
instead if ir.path is nil, the xds translator layer could set defaults
By that logic, should I also remove the Address default of 0.0.0.0 in this structure (multiple places in the gatewayapi folder) and the default path being set in processAccessLog?
Signed-off-by: Lior Okman <[email protected]>
Signed-off-by: Lior Okman <[email protected]>
liorokman
force-pushed
the
ctp-path-impl
branch
from
df5326a to
f105701
Compare
What this PR does / why we need it:
This PR implements the APIs added in #2384