Bug: setWeight incorrectly modifies header-based routing rules, breaking canary traffic routing

[guoliangmiao]

Description

I've discovered a bug in the rollouts-plugin-trafficrouter-gatewayapi plugin where the setWeight operation incorrectly modifies ALL routing rules, including header-based routes, causing canary traffic routing to fail.

Problem

The plugin's setWeight logic is too aggressive - it doesn't distinguish between regular routing rules and header-based routing rules. When setting weights, it modifies the weight of all backend references pointing to the canary service, including those in header-match rules that should remain unaffected.

Root Cause

In the plugin source code at pkg/plugin/httproute.go, the setHTTPRouteWeight function implements the following logic:

// 1. Get all rules from the HTTPRoute
routeRuleList := HTTPRouteRuleList(httpRoute.Spec.Rules)

// 2. Find all BackendRefs pointing to the Canary service across ALL rules
canaryBackendRefs, err := getBackendRefs(canaryServiceName, routeRuleList)

// 3. Set the weight for ALL found references to desiredWeight
for _, ref := range canaryBackendRefs {
    ref.Weight = &desiredWeight
}

Reproduction Steps

When a Rollout progresses through its first step with both header routing and weight configuration:

setHeaderRoute executes first: The plugin adds a new rule (e.g., Rule #2) to the HTTPRoute that matches the X-Canary-start header and routes to the Canary service with default weight (effectively 100%).

setWeight: 0 executes: The plugin runs the logic described above:

• It iterates through ALL rules in the HTTPRoute
• It finds the Canary service in Rule [BE]:Add Gateway API plugin #1 (main route) and sets weight to 0 ✓
• It also finds the Canary service in Rule Update readme #2 (header route) and sets its weight to 0 ✗

Result

The generated HTTPRoute YAML looks like this:

spec:
  rules:
    - matches:
        - path:
            type: PathPrefix
            value: /
      backendRefs:
        - name: my-app-stable
          port: 80
          weight: 100
        - name: my-app-canary
          port: 80
          weight: 0 
    - matches:
        - headers:
            - name: X-Canary-start
              value: "true"
          path:
            type: PathPrefix
            value: /
      backendRefs:
        - name: my-app-canary
          port: 80
          weight: 100 

Impact:

The header-based routing rule exists but doesn't receive any traffic (or produces undefined behavior) because weight: 0 effectively disables it.

Expected Behavior

setWeight should only control the traffic distribution in the main routing rule and should NOT modify weights in header-based routing rules created by setHeaderRoute.Proposed FixWhen updating weights, the plugin should skip rules that were created by setHeaderRoute.

This could be achieved by:

1. Adding metadata/annotations to identify header-based rules
2. Checking for the presence of header matches before modifying weights
3. Only applying weight changes to the primary/default route rule

Would appreciate any guidance on this issue or if there's a workaround available. Happy to provide more details or help with a PR if needed.

[kostis-codefresh]

Hello

The behavior of the plugin is modelled to mimic what Istio does in core Argo Rollouts https://argo-rollouts.readthedocs.io/en/stable/features/traffic-management/#traffic-routing-based-on-a-header-values-for-canary

Can you check the behavior there first please? I understand the reasoning behind this issue, but we also need to honor existing user expectations that migrate from built-in istio to gateway API.

If the current behavior is indeed different that what Istio is doing, then I agree that this is a bug.

(Can you also fix the yaml formatting above please?)

[guoliangmiao]

spec:
  rules:
    - matches:
        - path:
            type: PathPrefix
            value: /
      backendRefs:
        - name: my-app-stable
          port: 80
          weight: 100
        - name: my-app-canary
          port: 80
          weight: 0 
    - matches:
        - headers:
            - name: X-Canary-start
              value: "true"
          path:
            type: PathPrefix
            value: /
      backendRefs:
        - name: my-app-canary
          port: 80
          weight: 100 

[Robyt96]

Hello!

I agree with @guoliangmiao on the expected result, i think the weight of an header-based managed route should never change. With Istio + core Argo Rollout it is possible to configure header based routing without using any setWeight step.

Reproduction steps

# Rollout
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: rollouts-demo
  namespace: rollouts
spec:
  replicas: 2
  strategy:
    canary:
      canaryService: rollouts-demo-canary
      stableService: rollouts-demo
      trafficRouting:
        managedRoutes:
        - name: canary-route1
        istio:
          virtualService:
            name: rollouts-demo
            routes:
            - primary
      steps:
      - setCanaryScale:
          replicas: 1
      - setHeaderRoute:
          name: canary-route1
          match:
            - headerName: X-Version
              headerValue:
                exact: canary
      - pause: {}
      - setWeight: 50
      - pause: {}
  revisionHistoryLimit: 2
  selector:
    matchLabels:
      app: rollouts-demo
  template:
    ...
---

# Virtual Service
apiVersion: networking.istio.io/v1
kind: VirtualService
metadata:
  name: rollouts-demo
  namespace: rollouts
spec:
  hosts:
  - rollouts-demo.rollouts.svc.cluster.local
  http:
  - name: primary
    route:
    - destination:
        host: rollouts-demo
      weight: 100
    - destination:
        host: rollouts-demo-canary
      weight: 0

After the first rollout step, the Virtual Service is modified to:

apiVersion: networking.istio.io/v1
kind: VirtualService
metadata:
  generation: 2
  name: rollouts-demo
  namespace: rollouts
spec:
  hosts:
  - rollouts-demo.rollouts.svc.cluster.local
  http:
  - match:
    - headers:
        X-Version:
          exact: canary
    name: canary-route1
    route:
    - destination:
        host: rollouts-demo-canary
      weight: 100 # The weight here is set to 100 by default
  - name: primary
    route:
    - destination:
        host: rollouts-demo
      weight: 100
    - destination:
        host: rollouts-demo-canary
      weight: 0

So I'd expected the same behavior with the plugin. Instead, the weight of the backendRef in the HTTPRoute header based rule is set to 0:

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  generation: 10
  name: rollouts-http-route
  namespace: rollouts
spec:
  parentRefs:
  - group: ""
    kind: Service
    name: rollouts-demo
    port: 80
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: rollouts-demo
      port: 80
      weight: 100
    - group: ""
      kind: Service
      name: rollouts-demo-canary
      port: 80
      weight: 0
    matches:
    - path:
        type: PathPrefix
        value: /
    name: primary
  - backendRefs:
    - group: ""
      kind: Service
      name: rollouts-demo-canary
      port: 80
      weight: 0 # this is wrong
    matches:
    - headers:
      - name: X-Version
        type: Exact
        value: canary
      path:
        type: PathPrefix
        value: /

Reproduction steps

# Rollout
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
  name: rollouts-demo
  namespace: rollouts
spec:
  replicas: 2
  strategy:
    canary:
      canaryService: rollouts-demo-canary
      stableService: rollouts-demo
      trafficRouting:
        managedRoutes:
        - name: canary-route1
        plugins:
          argoproj-labs/gatewayAPI:
            namespace: rollouts
            httpRoutes:
            - name: rollouts-http-route
              useHeaderRoutes: true
      steps:
      - setCanaryScale:
          replicas: 1
      - setHeaderRoute:
          name: canary-route1
          match:
            - headerName: X-Version
              headerValue:
                exact: canary
      - pause: {}
      - setWeight: 50
      - pause: {}
  revisionHistoryLimit: 2
  selector:
    matchLabels:
      app: rollouts-demo
  template:
    ...
---

# HTTPRoute
kind: HTTPRoute
apiVersion: gateway.networking.k8s.io/v1
metadata:
  name: rollouts-http-route
  namespace: rollouts
spec:
  parentRefs:
    - group: ""
      kind: Service
      name: rollouts-demo
      port: 80
  rules:
  - name: primary
    matches:
    - path:
        type: PathPrefix
        value: /
    backendRefs:
    - name: rollouts-demo
      kind: Service
      port: 80
    - name: rollouts-demo-canary
      kind: Service
      port: 80

[kostis-codefresh]

Ok, so the plugin should be modified to match existing Istio behavior.

That being said, with the new introduction of step canary plugins I believe it is better to implement header based routing in its own plugin. This will make the present project focused only the Gateway API implementation rather than trying to cover all different user scenarios for header based routing as well.

[ngc4579]

Header-based routing also doesn't seem to set weights correctly when no setWeight step is specified at all - it simply sets the canary route to weight: 0.

Rollout spec:

kind: Rollout
metadata:
  name: http-echo
  namespace: http-echo
spec:
  replicas: 2
  selector:
    matchLabels:
      app.kubernetes.io/instance: http-echo
      app.kubernetes.io/name: http-echo
  strategy:
    canary:
      canaryService: http-echo-canary
      stableService: http-echo-stable
      steps:
      - setCanaryScale:
          replicas: 1
      - setHeaderRoute:
          match:
          - headerName: X-Routing-Hint
            headerValue:
              exact: canary
          name: canary-route
      - pause: {}
      - setHeaderRoute:
          name: canary-route
      trafficRouting:
        managedRoutes:
        - name: canary-route
        plugins:
          argoproj-labs/gatewayAPI:
            httproutes:
            - name: http-echo
              useHeaderRoutes: true
            namespace: http-echo
  workloadRef:
    apiVersion: apps/v1
    kind: Deployment
    name: http-echo

HTTPRoute spec while rolling out:

kind: HTTPRoute
metadata:
  name: http-echo
  namespace: http-echo
spec:
  hostnames:
  - <redacted>
  parentRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: traefik-gateway
    namespace: traefik
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: http-echo-stable
      port: 8080
      weight: 100
    - group: ""
      kind: Service
      name: http-echo-canary
      port: 8080
      weight: 0
    matches:
    - path:
        type: PathPrefix
        value: /<redacted>/http-echo/
  - backendRefs:
    - group: ""
      kind: Service
      name: http-echo-canary
      port: 8080
      weight: 0         # <---------- This doesn't seem right as it effectively deactivates the rule
    matches:
    - headers:
      - name: X-Routing-Hint
        type: Exact
        value: canary
      path:
        type: PathPrefix
        value: /<redacted>/http-echo/

(Note added hint above)

[ngc4579]

The issue might be mitigated by setting an additional step setWeight: 1 in order to have the canary route weight set to 1, effectively enabling it to receive traffic - but that leaves a small portion of traffic out of control by the desired header... So if you're relying on the header as the only means of traffic routing, this mitigation is out of scope.

[ngc4579]

Maybe getBackendRefs (plugin.go) could feature an additional param (generic filter function) to be defined by the calling facilities...

[duychu]

I'm facing the same issue. While waiting the solution, I think I will look into guide how to implement custom step to adjust weight of managed routes

[m-castaldo]

Also facing this issue, trying to look at a work around untill this fix is in

[Va1]

facing the same issue, curious for possible workarounds

[m-castaldo]

facing the same issue, curious for possible workarounds

I just created a separate httproute for header routing and removed header routing from the rollout, so then we can check using the separate httproute but still use rollouts to manage the deployment... so:

Rollout - httproute (main and canary)
Non Rollout - httproute (header based route to canary deployment)

for example:

That works for us

[jbilliau-rcd]

This issue is happening to us too, using Istio + Argo Rollouts + GatewayAPI with plugin 0.9.0. We do a rollout with the following steps:

steps:
  - setCanaryScale:
      replicas: 1
  - setHeaderRoute:
      name: "integration-test-rule"
      match:
      - headerName: testing
        headerValue:
          exact: "yes"
  - pause: {}

and the plugin adds this bottom section:

rules:
    - backendRefs:
        - group: ''
          kind: Service
          name: test-whale
          port: 80
          weight: 99
        - group: ''
          kind: Service
          name: test-whale-canary
          port: 80
          weight: 1
      matches:
        - path:
            type: PathPrefix
            value: /
      name: default
    - backendRefs:      #### this gets added
        - group: ''
          kind: Service
          name: test-whale-canary
          port: 80
          weight: 0
      matches:
        - headers:
            - name: testing
              type: Exact
              value: 'yes'
          path:
            type: PathPrefix
            value: /

Because the weight is 0, Istio creates that as "no upstream" and we get a 500.