envoyproxy / envoyproxy/envoy

deps: Enforce direct visibility of external deps by centralizing under bazel/deps

Open
#46,786 1 comment 0 reactions 0 assignees View on GitHub
deps tech debt
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
428

Description

*Title*: Enforce direct visibility of external deps by centralizing under `bazel/deps`

*Description*:

## Proposal

Declare every external dependency once as an alias target under a central package (e.g. `//bazel/deps:cel_cpp`, `//bazel/deps:abseil_cpp`), attach Bazel `visibility` to each, and ban direct `@repo//...` deps everywhere else in the repo.

Today the equivalent constraint is expressed as metadata in `bazel/deps.yaml`:

```yaml
cel_cpp:
use_category:
- dataplane_ext
extensions:
- envoy.filters.http.rbac
- envoy.filters.network.rbac
...
```

...and enforced *after the fact* by a checker that reads a dump of the build graph and compares it against that metadata. The `extensions` allowlist is a visibility rule, expressed in YAML and validated out-of-band. Expressing it as actual Bazel visibility would have Bazel enforce it at build time.

## What this would give us

**Build-time enforcement of the extensions allowlist.** An extension depending on a dep that does not list it currently fails in a separate CI check, long after the fact — if it is caught at all. As visibility, it fails at build time with an error naming the offending target.

This is not hypothetical. While fixing the dependency validation checker (#46761) we found `envoy.filters.network.ext_proc` depends on `cel_cpp`, but `cel_cpp` does not list it in its `extensions` allowlist. `flatbuffers` — a CEL companion dep with an otherwise near-identical allowlist — *does* list it, so the omission is clearly an oversight rather than intent. It went unnoticed because the old checker only ever traversed core-reachable deps, and `cel_cpp` arrives via non-core extensions. Under a visibility scheme this would have been a build failure the moment the dep was written.

**Elimination of the repo-naming problem.** The reachability aspect reports canonical Bazel repo names, while `deps.yaml` keys are WORKSPACE spec names (`abseil-cpp` vs `abseil_cpp`). This currently requires an `apparent_name` field per affected entry, and a name-resolution step in the checker that must be kept total or it silently mis-attributes deps. Nine such declarations had to be added in #46761 to make resolution complete.

If all deps are referenced through in-repo alias labels, consumers never see the external repo name at all. The naming mismatch becomes an implementation detail of a single package, and is likely to change again with the bzlmod migration.

**A single place to see and change the dependency surface.** Currently a dep's constraints live in `deps.yaml` while its usage is scattered across BUILD files, with no direct link between the two.

## What this does *not* give us

Worth being explicit, since it is tempting to assume this replaces dependency validation entirely. It does not.

**Visibility only constrains direct dependency edges.** Bazel does not visibility-check transitive deps. If `//bazel/deps:cel_cpp` is visible to an extension, everything cel-cpp pulls in comes along unchecked. Two of the three real gaps found in #46761 were exactly this class:

- `antlr4_runtimes` — defined internally by cel-cpp, reached transitively, needs declaring as an `implied_untracked_deps` entry
- `zlib-ng` — reached transitively via protobuf

Neither is a direct dependency edge anywhere in the repo, so neither would be visible to a visibility scheme. Answering "what did we actually end up linking, and is it categorised?" requires graph reachability regardless.

**The `dataplane_core` / `controlplane` checks are not visibility rules.** These assert what a given code path actually *reaches*, not who is permitted to depend on what. You could approximate them by restricting visibility to `//source/common/http` and friends, but that inverts the check — it asserts intent rather than observing outcome, and the entire point is to catch deps arriving somewhere unexpected.

**The ban itself needs enforcing.** "No direct `@repo//` deps outside `bazel/deps`" is a BUILD-file lint. Cheaper and more direct than graph reachability, but still a checker that has to exist and be maintained.

So the end state is complementary, not a replacement:

| Concern | Mechanism |
|---|---|
| Extension allowlist | Bazel visibility (build-time) |
| External repo naming | In-repo alias labels |
| Direct dep hygiene | BUILD-file lint |
| Transitive attribution / untracked deps | Reachability validation |
| Dataplane / controlplane categorisation | Reachability validation |
| `test_only` deps not reachable from production | Reachability validation |

## Challenges

- **Scale.** Every external dep referenced anywhere in the repo needs rewriting to the alias label. Large, mechanical, and touches effectively every BUILD file.
- **Migration ordering.** The initial visibility lists should be *generated* from observed reachability data rather than hand-written, otherwise they will be wrong from day one. This argues for landing the reachability validation work first (#46761) and using its output as the source for generation.
- **Select / config-dependent deps.** Deps behind `select()` (FIPS, openssl, platform-specific) need care so visibility does not become configuration-dependent in surprising ways.
- **Interaction with the bzlmod migration.** This may *help* — insulating consumers from canonical repo names removes a whole category of migration churn — but doing both simultaneously risks conflating two large changes. Worth deciding explicitly whether this lands before, during, or after.
- **Granularity.** Some deps expose multiple targets (`@foo//:a`, `@foo//:b`) with different appropriate visibility. Whether to alias per-target or per-repo needs deciding.

## Relevant Links

- #46761 — dependency validation rewrite (aspect-based reachability); source of the concrete examples above
- `bazel/deps.yaml`, `api/bazel/deps.yaml` — current dependency metadata
- `tools/dependency/validate_reachability_test.py` — current validation

Contributor guide

Open the contributing guide

Research direction

Start with bazel/deps.yaml and api/bazel/deps.yaml, then read tools/dependency/validate_reachability_test.py and issue #46761. Map the proposed //bazel/deps aliases, visibility lists, BUILD-file lint, and migration ordering against the existing reachability validation. Done means external dependencies use central aliases with Bazel-enforced extension visibility while the complementary transitive and categorisation checks remain covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, yaml
Domain
build-system, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.