gateway-api
gateway-api copied to clipboard
Webhook: Validate unique port, protocol, and hostname per listener
What would you like to be added: The validating webhook should ensure that port, protocol, and hostname are unique among Gateway listeners. For example, 2 listeners for foo.com would be valid if they were on different ports or protocols, but not if port and protocol were also identical.
Why this is needed: To provide a better user experience.
/assign
@robscott I am thinking of generating a hash based on the (Hostname + Port +Protocol) and then store them in a map where key is hash of the listener array and value is the index and if the key is already present then log all the indexes in the errors . Do you have views ?
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/stale
is applied - After 30d of inactivity since
lifecycle/stale
was applied,lifecycle/rotten
is applied - After 30d of inactivity since
lifecycle/rotten
was applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale
- Mark this issue or PR as rotten with
/lifecycle rotten
- Close this issue or PR with
/close
- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/lifecycle frozen
@robscott @bishtsaurabh5 May I know if this feature is still required based on the current design? If so, I'd like to continue to implement this.
Hey @gyohuangxin, thanks for checking! This is still unimplemented and would be a nice addition to https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1beta1/validation/gateway.go. Let me know if you're still interested in implementing this.
Hey i would like to contribute if no one is working on it.
@robscott Thanks! I'd like to contribute on this. @akankshakumari393 Sorry! I'm working on this, but any reviews and comments are welcome :)
/assign
I meet an issue when implementing this feature. My method is to add a function in https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1beta1/validation/gateway.go, which will create a Kubernetes client via "sigs.k8s.io/controller-runtime/pkg/client"
to get gateways
resource list at cluster scope, here is the example code:
func listGateway() (*v1beta1.GatewayList, error) {
cfg, err := config.GetConfig()
if err != nil {
klog.Errorf("Error loading Kubernetes config: %v", err)
}
cl, err := client.New(cfg, client.Options{})
if err != nil {
klog.Errorf("Failed to create new Kubernetes client: %v", err)
}
v1beta1.AddToScheme(cl.Scheme())
gatewayList := &v1beta1.GatewayList{}
err = cl.List(context.Background(), gatewayList, &client.ListOptions{})
if err != nil {
return nil, err
}
return gatewayList, nil
}
func validateUniquePort(listeners []gatewayv1b1.Listener, path *field.Path) field.ErrorList {
var errs field.ErrorList
gatewayList, err := listGateway()
if err != nil {
klog.Errorf("Failed to list gateways: %v", err)
}
klog.Infof("Gateways resources: %v", gatewayList)
return errs
}
However, it doesn't work because the user "system:serviceaccount:gateway-system:default" used by webhook server is forbidden to list "gateway" resource at cluster scope.
I1012 14:51:51.693874 1 request.go:1073] Response Body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"gateways.gateway.networking.k8s.io is forbidden: User \"system:serviceaccount:gateway-system:default\" cannot list resource \"gateways\" in API group \"gateway.networking.k8s.io\" at the cluster scope","reason":"Forbidden","details":{"group":"gateway.networking.k8s.io","kind":"gateways"},"code":403}
E1012 15:44:42.933257 1 gateway.go:132] Failed to list gateways: gateways.gateway.networking.k8s.io is forbidden: User "system:serviceaccount:gateway-system:default" cannot list resource "gateways" in API group "gateway.networking.k8s.io" at the cluster scope
I1012 15:44:42.933278 1 gateway.go:134] Listing gateways resources: <nil>
@robscott Do you have any ideas?
Hey @gyohuangxin, sorry for the confusion here! This was actually intended to be smaller in scope than that. Instead of ensuring that a Listener's port, protocol, and hostname are unique across all Gateways, we want to make sure that they are unique within each individual Gateway. A quick example:
What we're trying to prevent with this validation:
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: example-gateway
spec:
gatewayClassName: example-gateway-class
listeners:
- name: http
protocol: HTTP
port: 80
- name: other-http
protocol: HTTP
port: 80
What we don't care about:
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: example-gateway
spec:
gatewayClassName: example-gateway-class
listeners:
- name: http
protocol: HTTP
port: 80
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: example-gateway2
spec:
gatewayClassName: example-gateway-class
listeners:
- name: http
protocol: HTTP
port: 80
@robscott Understand, thanks for clarifying!
we want to make sure that they are unique within each individual Gateway. A quick example:
This seems fairly limited in my opinion. What about other properties in the listeners?
Looking at the discussion (https://github.com/kubernetes-sigs/gateway-api/discussions/1248) we could have many certificates (HTTP01) for services in a cluster. If there are multiple controllers updating the gateway listener resource that'll lead to conflicts and churn.
In theory a way to coordinate this could be by adding multiple listeners but with the same protocol/port and hostname but with a different name
and tls
configuration
ie.
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: example-gateway
spec:
gatewayClassName: example-gateway-class
listeners:
- name: https
protocol: HTTPS
port: 443
tls:
certificateRefs: [some-cert]
- name: https-2
protocol: HTTPS
port: 443
tls:
certificateRefs: [another-cert]
Alternatively maybe we should ensure merging of distinct Gateways resources is part of the core
functionality.
Actually I realize the listeners would have unique hostname
's when setting tls
- so my comment isn't applicable.