Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Implement disableMergeSlashes and escapedSlashesAction #2413

Merged
merged 2 commits into from
Jan 10, 2024
Merged

feat: Implement disableMergeSlashes and escapedSlashesAction #2413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1251937
Implemented the Path Settings API
liorokman Jan 9, 2024
f105701
Make all the rebased tests pass
liorokman Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions internal/gatewayapi/clienttrafficpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ func (t *Translator) translateClientTrafficPolicyForListener(policySpec *egv1a1.
// Translate Suppress Envoy Headers
translateListenerSuppressEnvoyHeaders(policySpec.SuppressEnvoyHeaders, httpIR)

// Translate Path Settings
translatePathSettings(policySpec.Path, httpIR)

// enable http3 if set and TLS is enabled
if httpIR.TLS != nil && policySpec.HTTP3 != nil {
httpIR.HTTP3 = &ir.HTTP3Settings{}
Expand Down Expand Up @@ -346,6 +349,18 @@ func translateListenerTCPKeepalive(tcpKeepAlive *egv1a1.TCPKeepalive, httpIR *ir
httpIR.TCPKeepalive = irTCPKeepalive
}

func translatePathSettings(pathSettings *egv1a1.PathSettings, httpIR *ir.HTTPListener) {
if pathSettings == nil {
return
}
if pathSettings.DisableMergeSlashes != nil {
httpIR.Path.MergeSlashes = !*pathSettings.DisableMergeSlashes
}
if pathSettings.EscapedSlashesAction != nil {
httpIR.Path.EscapedSlashesAction = ir.PathEscapedSlashAction(*pathSettings.EscapedSlashesAction)
}
}

func translateListenerProxyProtocol(enableProxyProtocol *bool, httpIR *ir.HTTPListener) {
// Return early if not set
if enableProxyProtocol == nil {
Expand Down
4 changes: 4 additions & 0 deletions internal/gatewayapi/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

MergeSlashes: true,
EscapedSlashesAction: ir.UnescapeAndRedirect,
},
}
if listener.Hostname != nil {
irListener.Hostnames = append(irListener.Hostnames, string(*listener.Hostname))
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand All @@ -372,6 +375,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-fault-injection.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -341,6 +344,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-circuitbreakers-error.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-circuitbreakers.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -271,6 +274,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-healthcheck.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -442,6 +445,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-loadbalancer.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -351,6 +354,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions ...api/testdata/backendtrafficpolicy-with-local-ratelimit-default-route-level-limit.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions ...gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-limit-unit.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions ...gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit-invalid-match-type.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions ...ta/backendtrafficpolicy-with-local-ratelimit-invalid-multiple-route-level-limits.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-local-ratelimit.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-proxyprotocol.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -265,6 +268,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit-invalid-regex.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
6 changes: 6 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-ratelimit.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,9 @@ xdsIR:
- '*'
isHTTP2: true
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down Expand Up @@ -295,6 +298,9 @@ xdsIR:
- '*'
isHTTP2: false
name: envoy-gateway/gateway-2/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-with-same-prefix-httproutes.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ xdsIR:
- '*'
isHTTP2: false
name: default/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- backendWeights:
Expand Down
3 changes: 3 additions & 0 deletions internal/gatewayapi/testdata/clienttrafficpolicy-http3.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@ xdsIR:
http3: {}
isHTTP2: false
name: envoy-gateway/gateway-1/tls
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10443
routes:
- backendWeights:
Expand Down
36 changes: 36 additions & 0 deletions internal/gatewayapi/testdata/clienttrafficpolicy-path-settings.in.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
clientTrafficPolicies:
- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: ClientTrafficPolicy
metadata:
namespace: envoy-gateway
name: target-gateway-1
spec:
path:
disableMergeSlashes: true
escapedSlashesAction: KeepUnchanged
targetRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
namespace: envoy-gateway
name: gateway-1
spec:
gatewayClassName: envoy-gateway-class
listeners:
- name: http-1
protocol: HTTP
port: 80
allowedRoutes:
namespaces:
from: Same
- name: http-2
protocol: HTTP
port: 8080
allowedRoutes:
namespaces:
from: Same
Loading