Unclear specification for a single host with weight=0 in HTTPRoute

[nak3]

What happened:

As per spec comment:

gateway-api/apis/v1alpha1/httproute_types.go

Lines 624 to 627 in 1caea70

	// If only one backend is specified and it has a weight greater than 0, 100%
	// of the traffic is forwarded to that backend. If weight is set to 0, no
	// traffic should be forwarded for this entry. If unspecified, weight
	// defaults to 1.

it seems that weight = 0 for only one backend is not invalid but what error code is expected - 404, 503 or something else?
Also, a single host with weight = 0 should be invalid configuration?

How to reproduce it (as minimally and precisely as possible):

1. deploy Gateway,GatewayClass,HTTPRoute (with weight=0) ,Deployments

cat <<EOF | kubectl apply -f -
apiVersion: v1
kind: Namespace
metadata:
  name: gateway-demo
---
apiVersion: networking.x-k8s.io/v1alpha1
kind: GatewayClass
metadata:
  name: istio
spec:
  controller: istio.io/gateway-controller
---
kind: Gateway
apiVersion: networking.x-k8s.io/v1alpha1
metadata:
  name: simple-gateway
  namespace: gateway-demo
spec:
  gatewayClassName: istio
  listeners:  # Use GatewayClass defaults for listener definition.
  - protocol: HTTP
    port: 80
    routes:
      kind: HTTPRoute
      namespaces:
        from: "All"
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: foo
  namespace: gateway-demo
spec:
  replicas: 1
  selector:
    matchLabels:
      app: foo
  template:
    metadata:
      labels:
        app: foo
    spec:
      containers:
      - name: whereami
        image: docker.io/kennethreitz/httpbin
        ports:
          - containerPort: 80
        env:
        - name: METADATA
          value: "foo"
---
apiVersion: v1
kind: Service
metadata:
  name: foo-svc
  namespace: gateway-demo
spec:
  selector:
    app: foo
  ports:
  - port: 8000
    targetPort: 80
---
apiVersion: networking.x-k8s.io/v1alpha1
kind: HTTPRoute
metadata:
  name: test
  namespace: gateway-demo
spec:
  gateways:
    allow: SameNamespace
  hostnames:
  - "*"
  rules:
  - forwardTo:
    - port: 8000
      serviceName: foo-svc
      weight: 0
    matches:
    - path:
        type: Prefix
        value: /
EOF

2. Access to the service.

$ curl $IP:$PORT/headers -s -o /dev/null -w "%{http_code}" 

It is not clear what error code (404 503 or something else) the client gets.

[robscott]

Agree that we should clarify this a bit more. My interpretation of weight==0 is that it should generally be equivalent to a backend not existing. The slight exception to that rule may be that some providers may choose to leave some provisioned infra in place for if/when that weight transitions to a non-zero value. I think the question becomes a broader one of how implementations should handle a Gateway with no backends. It may be acceptable to leave that as an implementation-specific decision unless there's a need for conformance around this behavior.

[howardjohn]

There is also a potential distinction between "process the request and send to no backend" and "realize there is no backend and drop the entire HTTPRoute". for example, if we had a filter with a side effect, etc

[robscott]

Are there any use cases where we'd want to run a filter + have no valid backends? That seems like it could create some really confusing config.

[howardjohn]

I hope not! I agree it's confusing. I think we should just treat the whole route as discarded if there is no backend with a non zero weight? It could be rejected by the webhook even potentially?

[robscott]

Yeah good call, this probably could be handled by the webhook.

[bowei]

Single backend, weight = 0 should probably be a valid config:

Scenario: admin is adding a filter chain to protect their backend. They want to test it but not allow any requests to go through. (Think DoS attack on a production system).

[hbagdi]

In my experience, processing filters and not forwarding requests to upstream service is a common use-case.
It is used for debugging (in production), temporarily disable an endpoint, or simply as a placeholder for variety of business reasons.
I'm not sure if it is feasible to have conformance for behavior at this granularity for all implementations. It could be possible but requires more experience and discussions.

503 feels like the right one to me. How about advising that but not conforming it?

[howardjohn]

So what I am hearing is the spec should be:

• Implementors MUST not send traffic to the backend
• Implementors MUST (or SHOULD?) process all filters
• Implementors SHOULD response with a 503 for HTTP requests

Is that accurate?

[hbagdi]

Implementors MUST (or SHOULD?) process all filters

I feel we should use MUST here but I'd love to hear thoughts from others.

Is that accurate?

I think so.

[bowei]

I think the last bullet (503) should be qualified with the fact that in most cases, we would expect some filter in the chain to generate the actual response (e.g. 404 filter)

[howardjohn]

I feel we should use MUST here but I'd love to hear thoughts from others.

That WFM, but would be good to hear from non-envoy implementations as well

[jpeach]

If weight is set to 0, no traffic should be forwarded for this entry.

This reads pretty clearly to me. There's no traffic, therefore there can never be a 503 or 404 response (since a response would be generated by traffic).

That seems to also be the consensus of the discussion above, so what still needs to happen? Change the should to a MUST?

[youngnick]

So, in the case that you have a weight of 0 on a single host, and no filters, then the config should be admitted but route no traffic? How do we surface that to the user? Feels like a Condition that says "This doesn't do anything, are you sure?" might be useful.

[jpeach]

It's not broken and it does what it says on the box, my preference would be to silently accept it.

[youngnick]

If that's the case, then we should change the spec like this, agreed:

// If only one backend is specified and it has a weight greater than 0, 100%
- // of the traffic is forwarded to that backend. If weight is set to 0, no
- // traffic should be forwarded for this entry. If unspecified, weight
+ // of the traffic is forwarded to that backend. If weight is set to 0,
+ // traffic must not be be forwarded for this entry. If unspecified, weight
// defaults to 1. 

[bowei]

Seems like an easy PR if this language is clearer -- send away?

(Github should do word-wise diff for this one)

[youngnick]

Done.