envoyproxy/envoy

Route table check tool coverage calculation incorrect when using cluster_header

Open

#25,182 创建于 2023年1月26日

在 GitHub 查看
 (3 评论) (1 反应) (0 负责人)C++ (5,373 fork)batch import
area/routerbughelp wanted

仓库指标

Star
 (27,997 star)
PR 合并指标
 (平均合并 8天) (30 天内合并 378 个 PR)

描述

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.

贡献者指南