Webhook: Validate unique port, protocol, and hostname per listener

[robscott]

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.

[bishtsaurabh5]

/assign

[bishtsaurabh5]

@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 ?

[k8s-triage-robot]

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

[hbagdi]

/lifecycle frozen

[gyohuangxin]

@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.

[robscott]

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.

[akankshakumari393]

Hey i would like to contribute if no one is working on it.

[gyohuangxin]

@robscott Thanks! I'd like to contribute on this.
@akankshakumari393 Sorry! I'm working on this, but any reviews and comments are welcome :)

[gyohuangxin]

/assign

[gyohuangxin]

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?

[robscott]

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

[gyohuangxin]

@robscott Understand, thanks for clarifying!

[dprotaso]

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 (#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.

[dprotaso]

Actually I realize the listeners would have unique hostname's when setting tls - so my comment isn't applicable.