envoyproxy/envoy

Route table check tool coverage calculation incorrect when using cluster_header

Open

#25,182 opened on Jan 26, 2023

View on GitHub
 (3 comments) (1 reaction) (0 assignees)C++ (5,373 forks)batch import
area/routerbughelp wanted

Repository metrics

Stars
 (27,997 stars)
PR merge metrics
 (Avg merge 8d) (378 merged PRs in 30d)

Description

Title: Route table check tool coverage calculation incorrect when using cluster_header in a route config

Description: When a configuration contains multiple routes which utilise cluster_header for selecting a cluster the route tester does not calculate coverage properly. In the example below you see 50% coverage reported when both routes exist yet isolating each route and test reports 100%. This is particularly an issue when using --fail-under as part of a pipeline. If you switch to using cluster rather than cluster_header the problem goes away.

In case anyone experiences this a workaround is to replace the cluster_header node with cluster and a fixed value when running the tests. A tool such as yq can do this.

Repro steps: Take the following Envoy config and tests as an example.

Envoy route table check tool version 1.21.4

virtual_hosts:
  - name: api_domain
    domains: ["test.com"]
    routes:
      - match:
          prefix: /test1
        route:
          cluster_header: "x-cluster"

      - match:
          prefix: /test2
        route:
          cluster_header: "x-cluster"
tests:
  - test_name: test1
    input:
      authority: test.com
      path: /test1
      method: GET
      additional_request_headers:
      - key: x-cluster
        value: "cluster_a"
    validate:
      cluster_name: cluster_a
    
  - test_name: test1
    input:
      authority: test.com
      path: /test2
      method: GET
      additional_request_headers:
      - key: x-cluster
        value: "cluster_a"
    validate:
      cluster_name: cluster_a

Running the route test will return 50% coverage when the expected is 100%. Now, remove the first or second route and matching test and see 100% coverage is returned. Doing the same with the other route/test will also return100%. Replacing cluster_header ... with cluster: cluster_a in either route will also return 100%.

Luckily this is deterministic so repro should not be an issue.

Contributor guide