envoyproxy / envoyproxy/envoy

dependency reachability aspect only covers the default config

Open
#46,917 0 comments 0 reactions 0 assignees View on GitHub
dependencies
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
437

Description

*Title*: dependency reachability aspect only covers the default config

*Description*:

`//tools/dependency:dep-reachability` and `:dep-reachability-core` are built in a single configuration, so the reachability JSON only ever reflects whichever branch each `select()` resolves to by default. Any dep that is only reachable under a non-default config is invisible to every check in `tools/dependency/validate_reachability_test.py`.

This was known in general terms but surfaced concretely while working on #46875.

Reproduction — the wasm runtime selects are mutually exclusive, so only one runtime's deps are ever observed:

```
jq -r '.dependencies | keys[]' bazel-bin/tools/dependency/dep-reachability.json \
| grep -E 'v8|highway|fast_float|wasm|simdutf'
proxy-wasm-cpp-host
proxy-wasm-cpp-sdk
```

Building with `--define wasm=wasmtime` produces different output, confirming the data is configuration-scoped.

Consequences:

- `use_category` / `extensions:` allowlists are unenforced for any `select`-gated dep.
- `validate_dep_names_resolved` cannot detect a removed or misnamed metadata entry for those deps.
- The same applies beyond wasm — fips/openssl, http3 on/off, and other config-gated deps have the same gap.

The module docstring in `validate_reachability_test.py` states that checks are "fully retained here" relative to the old `validate.py`; that is only true for deps reachable in the default configuration and should be qualified.

Proposed approach: use transitions so a single `dependency_reachability` target can emit reachability data for several configurations, and union the results before validation. The set of configurations covered should be explicit and reviewable rather than implicitly "whatever the default happens to be".

Note that `//source/extensions/wasm_runtime/v8:config` only depends on `@proxy-wasm-cpp-host//:base_lib`, so it is worth confirming separately whether `@v8` enters the closure at all even in the v8 configuration — config multiplexing may not be sufficient on its own here.

[optional *Relevant Links*:]

- https://github.com/envoyproxy/envoy/pull/46875
- https://github.com/envoyproxy/envoy/pull/42890
- `tools/dependency/BUILD` — `dependency_reachability` root declarations
- `tools/dependency/validate_reachability_test.py`
- `bazel/envoy_select.bzl` — `envoy_select_wasm_v8` and friends
- `bazel/dependency/reachability.bzl` in envoyproxy/toolshed

Contributor guide

Open the contributing guide

Research direction

Start with tools/dependency/BUILD and tools/dependency/validate_reachability_test.py, then read bazel/envoy_select.bzl and bazel/dependency/reachability.bzl. Reproduce the default and --define wasm=wasmtime outputs, and verify the relevant wasm runtime closure. Done means reachability data covers an explicit, reviewable set of configurations and validation checks the unioned results.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
build-system, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.