-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
HTTPRoute not accepted if namespace selector is used #28186
Comments
im also coming across this issue trying to use a shared gateway. ---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: ingress
namespace: kube-system
annotations:
cert-manager.io/cluster-issuer: letsencrypt-prod-cluster
spec:
gatewayClassName: cilium
listeners:
- name: http-one
protocol: HTTP
port: 80
hostname: "one.example.com"
allowedRoutes:
namespaces:
from: Selector
selector:
matchLabels:
toolkit.fluxcd.io/tenant: one
- name: http-one-shorthand
protocol: HTTP
port: 80
hostname: "o.example.com"
allowedRoutes:
namespaces:
from: Selector
selector:
matchLabels:
toolkit.fluxcd.io/tenant: one
- name: http-two
protocol: HTTP
port: 80
hostname: "two.com"
allowedRoutes:
namespaces:
from: Selector
selector:
matchLabels:
kubernetes.io/metadata.name: kube-system
- name: https-two
protocol: HTTPS
port: 443
hostname: "two.com"
allowedRoutes:
namespaces:
from: Selector
selector:
matchLabels:
toolkit.fluxcd.io/tenant: two
tls:
certificateRefs:
- kind: Secret
name: two-cert
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: http-redirect
namespace: kube-system
spec:
parentRefs:
- name: ingress
namespace: kube-system
sectionName: http-two
hostnames:
- "two.com"
rules:
- filters:
- type: RequestRedirect
requestRedirect:
scheme: https
statusCode: 301 i see the following on the http route
|
one thing to note as well when i switch the selector to "All" The http redirect rule is also bound to the https listener and i get the https website redirecting in on itself constantly. @tobiaskohlbau i think this patch should be a good resolution I haven't gone to the effort yet to patch locally but id say this is most likely a valid bug based on expected usage patterns of the gateway / httproute object's documentation, might be a good idea to raise a PR. |
thanks both for reporting this issue, temporarily assigned to myself so that I can triage this later. |
Can confirm I have the same issue, adding my config as well if it is of any help.
|
👋 Seems like the issue is fixed recently in main with the PR #27309. I have tried to test with the below manifest, but unable to replicate the issue. apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: reproducer
spec:
gatewayClassName: cilium
listeners:
- name: one
protocol: HTTPS
port: 443
hostname: "one.example.org"
tls:
certificateRefs:
- kind: Secret
name: one-tls
# Un-comment and apply again later
# - name: two
# protocol: HTTPS
# port: 443
# hostname: "two.example.org"
# tls:
# certificateRefs:
# - kind: Secret
# name: two-tls
# allowedRoutes:
# namespaces:
# from: Selector
# selector:
# matchLabels:
# kubernetes.io/metadata.name: kube-system
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: one
spec:
parentRefs:
- name: reproducer
sectionName: one
rules:
- matches:
- path:
type: PathPrefix
value: /
backendRefs:
- name: details
port: 9080 $ kg httproute one -o json | jqs
{
"parents": [
{
"conditions": [
{
"lastTransitionTime": "2023-10-10T12:30:37Z",
"message": "Accepted HTTPRoute",
"observedGeneration": 2,
"reason": "Accepted",
"status": "True",
"type": "Accepted"
},
{
"lastTransitionTime": "2023-10-10T12:30:37Z",
"message": "Service reference is valid",
"observedGeneration": 2,
"reason": "ResolvedRefs",
"status": "True",
"type": "ResolvedRefs"
}
],
"controllerName": "io.cilium/gateway-controller",
"parentRef": {
"group": "gateway.networking.k8s.io",
"kind": "Gateway",
"name": "reproducer",
"sectionName": "one"
}
}
]
} |
im using 1.14.2 and see this issue which i believe that PR was introduced? @sayboras heres another example slimmed down to reproduce. with the below configuration you always hit the redirect route regardless what listener you land on as the section filter isn't working when you start using multiple listeners on the same hostname ---
apiversion: gateway.networking.k8s.io/v1beta1
kind: gateway
metadata:
name: ingress
namespace: kube-system
annotations:
cert-manager.io/cluster-issuer: letsencrypt-prod-cluster
spec:
gatewayclassname: cilium
listeners:
- name: http-example
protocol: http
port: 80
hostname: "example.com"
allowedroutes:
namespaces:
from: all
- name: https-example
protocol: https
port: 443
hostname: "example.com"
allowedroutes:
namespaces:
from: all
tls:
certificaterefs:
- kind: secret
name: example-cert
---
apiversion: gateway.networking.k8s.io/v1beta1
kind: httproute
metadata:
name: http-redirect
namespace: kube-system
spec:
parentrefs:
- name: ingress
namespace: kube-system
sectionname: http-example
hostnames:
- "example.com"
rules:
- filters:
- type: requestredirect
requestredirect:
scheme: https
statuscode: 301
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: example-route
namespace: example-app
spec:
parentRefs:
- name: ingress
namespace: kube-system
sectionName: https-example
hostnames:
- "example.com"
rules:
- backendRefs:
- name: example
port: 80
---
apiVersion: v1
kind: Service
metadata:
name: example
labels:
app: example
spec:
ports:
- port: 80
targetPort: 3000
name: web
protocol: TCP
selector:
app: example |
The redirect loop issue might not be related to the issue reported in the first post. I am watching the upstream issue, so I would have expected there would be some change in the upstream API. |
Thanks for your above manifest, let me give it a try. |
im not sure if what i reported is related to that issue as the Origional post mentioned this as a workaround
i believe my manifests above meet that condition of a HTTP route per listener to avoid a loop scenario so it wouldn't be related to that upstream issue which is more a feature request to modify behaviour when you intentionally loop back into the same listener. |
Thanks for the multiple manifests @AnthonyPoschen. It does look like we have a problem somewhere with the @sayboras, this seems like it could probably use a conformance test upstream once we figure it out as well. |
I can reproduce this issue locally, working on how we can fix it now. Thanks a lot for your details. |
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: #28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras I am still getting this on 1.14.4 with this gateway config: spec:
gatewayClassName: cilium
listeners:
- name: web-http
protocol: HTTP
port: 80
hostname: '*.example.com'
allowedRoutes:
namespaces:
from: Same
- name: web-https
protocol: HTTPS
port: 443
hostname: '*.example.com'
allowedRoutes:
namespaces:
from: All
tls:
mode: Terminate
certificateRefs:
- group: ''
kind: Secret
name: star-example-com-tls
- name: ldaps
protocol: TLS
port: 636
hostname: 'ldap.example.com'
allowedRoutes:
namespaces:
from: Selector
selector:
matchLabels:
use-private-gateway-ldaps: 'true'
tls:
mode: Terminate
certificateRefs:
- group: ''
kind: Secret
name: ldap-example-com-tls
---
apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
name: tls-upgrade
spec:
parentRefs:
- name: private-gateway
sectionName: web-http
hostnames:
- '*.example.com'
rules:
- filters:
- type: RequestRedirect
requestRedirect:
scheme: https
port: 443 Once I delete the listeners Status of status:
parents:
- conditions:
- lastTransitionTime: '2023-11-26T13:22:30Z'
message: >-
HTTPRoute is not allowed to attach to this Gateway due to namespace
selector restrictions
observedGeneration: 1
reason: NotAllowedByListeners
status: 'False'
type: Accepted
- lastTransitionTime: '2023-11-26T13:22:30Z'
message: Service reference is valid
observedGeneration: 1
reason: ResolvedRefs
status: 'True'
type: ResolvedRefs
controllerName: io.cilium/gateway-controller
parentRef:
group: gateway.networking.k8s.io
kind: Gateway
name: private-gateway
sectionName: web-http |
Unfortunately I'm unable to validate the fix myself as this was only for a homelab and I moved to a simpler setup. Maybe I will find time to recreate the setup. |
@rouke-broersma I don't think this fix is included in 1.14.4, it has not been backported yet. So it will be included in |
@youngnick there are two issues and two mentioned PRs for this issue. #27309 is the issue I'm having, and I think that was included in 1.14.2 so should be fixed, but I'm still hitting it in 1.14.4. The other issue #29115 is not yet backported but I'm not hitting that issue. |
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: #28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: #28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
Thanks for that explanation @rouke-broersma. @sayboras, did you want to take another look here? |
Somehow this didn't come up in my GitHub notification, re-open the issue now. |
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: #28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
This is to fix the issue in which the wrong list of HTTP routes is used to construct virtual host for insecure and secure routes. Ideally, only related HTTP routes for the same listener ports should be considered. Normally, this should not have any side effect, except using weightage clusters for the multiple, but the same, backends. However, in case of redirect filter, this will cause redirect loop as mentioned in the below issue. Fixes: 299648f Fixes: cilium#28186 Signed-off-by: Tam Mach <tam.mach@cilium.io>
Face same problem on 1.15.0-rc.0
And this not:
|
This should be fixed as part of #30100, which will be backported to 1.15 and 1.14 later. |
Is there an existing issue for this?
What happened?
Having the following scenario within a Gateway API enabled cluster:
A Gateway with multiple listeners for different hostnames. One of the listeners needs to allow a HTTPRoute from another namespace. Enabling a namespace selector for one listener breaks every HTTPRoute attached to the gateway.
Gateway definition:
HTTPRoute:
The HTTP route gets rejected as soon as the second listener is added. The Gateway API specification states the following about sectionName:
I read this as such setup of multiple restrictions within a gateway should be explicitly supported. Makes sense as otherwise bringing allowedRoutes into each listener is meaningless.
I would interpret this as an oversight during implementation. Especially cause the CheckGatewayMatchingSection already checks against sectionName. I would suggest CheckGatewayAllowedForNamespace should get the same check. I've changed a local copy of cilium with the required check to exclude listeners not matching the sectionName and everything is working as expected.
If the current behaviour gets confirmed as not intended I would like to submit a PR for this. The code is already done, but I want to confirm that this changes is welcomed.
Cilium Version
1.14.2
Kernel Version
Linux node 5.15.0-1037-raspi #40-Ubuntu SMP PREEMPT Fri Aug 25 16:42:00 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux
Kubernetes Version
Client Version: v1.28.1
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.5+k3s1
Sysdump
No response
Relevant log output
Anything else?
The current changes can be found here.
Code of Conduct
The text was updated successfully, but these errors were encountered: