mirror of
https://github.com/alibaba/higress.git
synced 2026-07-25 13:50:24 +08:00
Compare commits
1 Commits
fix/bump-e
...
add-deep-t
| Author | SHA1 | Date | |
|---|---|---|---|
|
8e426a5e99 |
61
.claude/skills/higress-deep-troubleshooting/SKILL.md
Normal file
61
.claude/skills/higress-deep-troubleshooting/SKILL.md
Normal file
@@ -0,0 +1,61 @@
|
||||
---
|
||||
name: higress-deep-troubleshooting
|
||||
description: Deep-dive troubleshooting methodology for Higress gateway bugs that require code-trace evidence across the higress-group/envoy and higress-group/proxy-wasm-cpp-host forks — CPU spins, infinite loops, Wasm VM reentrancy, filter-chain stage-mismatch, callback-ordering issues, SaveRestoreContext/current_context_ bugs. Use when investigating a complex Higress/Wasm/proxy-wasm incident that needs root-cause analysis with file:line citations, when ramping up on a reported CPU-spin or crash, or when verifying whether a proposed fix covers related bug classes. Accumulates case-study patterns under patterns/ — add a new pattern after each major investigation.
|
||||
---
|
||||
|
||||
# Higress Deep Troubleshooting
|
||||
|
||||
A methodology plus an accumulating pattern library for deep-diving into Higress gateway bugs that require code-level root cause analysis. The codebase spans two Higress-maintained forks — [`higress-group/envoy`](https://github.com/higress-group/envoy) (filter chain, Wasm `Context` integration) and [`higress-group/proxy-wasm-cpp-host`](https://github.com/higress-group/proxy-wasm-cpp-host) (Wasm VM host runtime, `WasmBase`, `ContextBase`) — and bugs often cross the boundary between them. This skill exists so that the next investigation doesn't start from scratch.
|
||||
|
||||
## When to use
|
||||
|
||||
Reach for this skill when ANY of these apply:
|
||||
|
||||
- A Higress issue reports a CPU spin, infinite loop, crash, or stage-mismatched behavior.
|
||||
- The bug involves the Wasm plugin lifecycle, `addAfterVmCallAction` / `doAfterVmCallActions`, `SaveRestoreContext`, `current_context_`, or the filter chain reentry paths.
|
||||
- You need to verify whether a one-repo fix also covers an upstream bug class (e.g., `proxy-wasm/proxy-wasm-cpp-host#326`).
|
||||
- You're ramping up on an incident and need a structured way to collect evidence before proposing a fix.
|
||||
- The maintainer is pushing back on a conclusion and you need to re-verify with code.
|
||||
|
||||
Do NOT reach for this skill for routine plugin development (`higress-wasm-go-plugin` covers that), config issues, or anything solvable by reading a single file without cross-repo context.
|
||||
|
||||
## Investigation methodology (brief)
|
||||
|
||||
1. **Symptom → code surface.** From perf data / repro steps / stack trace, identify which functions and files are involved. Note which fork each file lives in.
|
||||
2. **Parallel investigation.** Dispatch focused worker agents for independent aspects (envoy side, proxy-wasm-cpp-host side, upstream references). Each worker returns file:line evidence — never narration.
|
||||
3. **Code-trace verification.** Every claim must cite `file:line`. No "I think it does X" — only "see envoy context.cc:2085 where it calls ... synchronously".
|
||||
4. **Synthesize root cause.** Combine worker findings into a numbered call-chain trace, showing program state (queue contents, `current_context_` value, etc.) at each hop.
|
||||
5. **Fix design + side effects.** Propose a primitive, analyze what else it touches, check if it covers related bug classes upstream.
|
||||
6. **Verify when challenged.** If the maintainer pushes back, re-verify with code — don't restate the conclusion. The earlier analysis is often wrong in some detail that only a fresh code read catches.
|
||||
|
||||
Full process: [`references/investigation-methodology.md`](references/investigation-methodology.md). Verification patterns: [`references/code-trace-verification.md`](references/code-trace-verification.md).
|
||||
|
||||
## Accumulated patterns
|
||||
|
||||
Each major investigation that uses this skill should add a pattern under `patterns/`. Patterns are the "living" part of this skill — they capture both the technical root cause AND the meta-lessons (what tripped up the analysis, what the maintainer caught, what the fix primitive generalizes to).
|
||||
|
||||
Current patterns:
|
||||
|
||||
- [`patterns/2026-07-wasm-doaftervmcallactions-reentry.md`](patterns/2026-07-wasm-doaftervmcallactions-reentry.md) — #4034 CPU 100% from `onRedisCallFailure` + `injectEncodedDataToFilterChain` nested reentry. Root cause: unguarded `while(!empty())` drain in `WasmBase::doAfterVmCallActions`. Fix: drain-to-local via `std::deque::swap`. Meta-lesson: the same primitive covers the upstream #326 stage-mismatch class, which the first analysis round missed.
|
||||
|
||||
To add a new pattern after an investigation, see [`patterns/README.md`](patterns/README.md).
|
||||
|
||||
## Templates
|
||||
|
||||
- [`templates/investigation-report.md`](templates/investigation-report.md) — structure for a new investigation's findings.
|
||||
- [`templates/code-trace-table.md`](templates/code-trace-table.md) — step-by-step state-trace table format (very effective for communicating queue/state transitions to maintainers).
|
||||
|
||||
## Key code maps (quick orientation)
|
||||
|
||||
When investigating a Wasm-related Higress bug, the usual suspects:
|
||||
|
||||
| Concern | Repo | File |
|
||||
| --- | --- | --- |
|
||||
| Plugin callback entry points (`onRequestHeaders`, `onResponseBody`, `onHttpCallResponse`, etc.) | `higress-group/proxy-wasm-cpp-host` | `src/context.cc` |
|
||||
| Deferred-action queue (`addAfterVmCallAction`, `doAfterVmCallActions`, `DeferAfterCallActions`) | `higress-group/proxy-wasm-cpp-host` | `include/proxy-wasm/wasm.h`, `src/context.cc` |
|
||||
| `SaveRestoreContext` / `current_context_` | `higress-group/proxy-wasm-cpp-host` | `include/proxy-wasm/wasm_vm.h`, `src/exports.cc` |
|
||||
| Higress envoy `Context` (filter chain integration, foreign functions) | `higress-group/envoy` | `source/extensions/common/wasm/context.cc` |
|
||||
| Foreign-function registration (`proxy_inject_encoded_data_to_filter_chain`, etc.) | `higress-group/envoy` | `source/extensions/common/wasm/foreign.cc` |
|
||||
| envoy fork's pinned proxy-wasm-cpp-host commit | `higress-group/envoy` | `bazel/repository_locations.bzl` |
|
||||
|
||||
The Higress envoy fork pins a specific `proxy-wasm-cpp-host` commit via bazel — always check which fork commit is pinned before reasoning about `WasmBase` behavior, because the fork diverges from upstream (RedisCall, rebuild/recover, weak_handle additions).
|
||||
@@ -0,0 +1,116 @@
|
||||
# Wasm `doAfterVmCallActions` reentry — CPU-spin and stage-mismatch, fixed by drain-to-local
|
||||
|
||||
> Source: [#4034](https://github.com/higress-group/higress/issues/4034) — Wasm 插件死循环：onRedisCallFailure + injectEncodedDataToFilterChain 嵌套导致 Envoy CPU 100% 跑满
|
||||
> Upstream relative: [proxy-wasm/proxy-wasm-cpp-host#326](https://github.com/proxy-wasm/proxy-wasm-cpp-host/issues/326)
|
||||
> Date: 2026-07
|
||||
> Status: fix designed (SPEC-001), implementation pending
|
||||
|
||||
## Symptom
|
||||
|
||||
`release-2.2.2` Envoy pods CPU 100% on all 6 pods within ~3 minutes under concurrent load. Trigger: Redis unreachable + plugin calls `InjectEncodedDataToFilterChain` from an HTTP/Redis callback. perf:
|
||||
|
||||
| Overhead | Function |
|
||||
| --- | --- |
|
||||
| 39.21% | `Envoy::Extensions::Common::Wasm::Context::onRedisCallFailure` |
|
||||
| 25.41% | `proxy_wasm::WasmBase::doAfterVmCallActions` |
|
||||
| **合计** | **64.62%** |
|
||||
|
||||
## Root cause
|
||||
|
||||
`WasmBase::doAfterVmCallActions` (`higress-group/proxy-wasm-cpp-host` `include/proxy-wasm/wasm.h:158-169`) drains `after_vm_call_actions_` in an unguarded `while(!empty())` loop:
|
||||
|
||||
```cpp
|
||||
while (!self->after_vm_call_actions_.empty()) {
|
||||
auto f = std::move(self->after_vm_call_actions_.front());
|
||||
self->after_vm_call_actions_.pop_front();
|
||||
f(); // if f() re-adds an action, loop never terminates
|
||||
}
|
||||
```
|
||||
|
||||
Several Envoy-side callbacks re-defer themselves when `current_context_ != nullptr` (meaning: "VM is still on the stack, defer until it unwinds"):
|
||||
|
||||
- `Context::onRedisCallFailure` (envoy `context.cc:1273-1277`)
|
||||
- `Context::onRedisCallSuccess` (envoy `context.cc:1243-1249`)
|
||||
- `Context::onHttpCallSuccess` (envoy `context.cc:2352-2358`)
|
||||
- `Context::onHttpCallFailure` (envoy `context.cc:2382-2386`)
|
||||
|
||||
Under a synchronous reentry path — `injectEncodedDataToFilterChain` (envoy `context.cc:2085-2091`) is called from inside an HTTP/Redis callback — the outer `SaveRestoreContext` keeps `current_context_` non-null even after the inner `~SaveRestoreContext` unwinds. So the drain loop pops a self-reenqueueing callback, fires it, it sees `current_context_ != nullptr`, re-adds itself, loop iterates again → infinite → CPU 100%.
|
||||
|
||||
Full call chain (10 hops, each with file:line) is in [the #4034 proposal issue](https://github.com/higress-group/higress/issues/4064) body and SPEC-001.
|
||||
|
||||
## Fix
|
||||
|
||||
Swap the member queue to a local deque before iterating, so any action re-added during the drain lands in the (now-empty) member queue and is picked up by the next-outer `DeferAfterCallActions` frame — not re-fired within the same drain.
|
||||
|
||||
```cpp
|
||||
void doAfterVmCallActions() {
|
||||
if (!after_vm_call_actions_.empty()) {
|
||||
auto self = shared_from_this();
|
||||
std::deque<std::function<void()>> local;
|
||||
local.swap(self->after_vm_call_actions_);
|
||||
for (auto& f : local) f();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`std::deque::swap` is O(1) whole-content exchange. After swap: `local` has the pre-existing queue, `self->after_vm_call_actions_` is empty. The for-loop iterates `local`; any `addAfterVmCallAction` call during iteration appends to the empty member, invisible to this drain.
|
||||
|
||||
## Coverage
|
||||
|
||||
The same primitive covers BOTH:
|
||||
|
||||
- **#4034 (CPU-spin)**: re-added actions go to the empty member queue → picked up by next outer frame → loop terminates.
|
||||
- **#326 (stage-mismatch)**: pre-existing actions stay in `local` → inner drain (from a nested `~DeferAfterCallActions`) sees empty member → does nothing → pre-existing actions fire in the outer drain's correct phase.
|
||||
|
||||
The coverage of #326 is non-obvious — see Meta-lessons below.
|
||||
|
||||
## Evidence
|
||||
|
||||
| Claim | Citation |
|
||||
| --- | --- |
|
||||
| Unguarded drain loop | `higress-group/proxy-wasm-cpp-host` `include/proxy-wasm/wasm.h:158-169` |
|
||||
| Self-reenqueue guard `if (current_context_ != nullptr)` | `higress-group/envoy` `source/extensions/common/wasm/context.cc:1273-1277` (onRedisCallFailure); also lines 1243, 2352, 2382 |
|
||||
| Sync `injectEncodedDataToFilterChain` (the trigger) | `higress-group/envoy` `source/extensions/common/wasm/context.cc:2085-2091` |
|
||||
| Async sibling `injectEncodedDataToFilterChainOnHeader` (the contrast) | `higress-group/envoy` `source/extensions/common/wasm/context.cc:2093-2103` |
|
||||
| `sendLocalReply` is deferred via `addAfterVmCallAction` | `higress-group/envoy` `source/extensions/common/wasm/context.cc:1638-1657` |
|
||||
| `continueDecoding` is deferred via `addAfterVmCallAction` | `higress-group/envoy` `source/extensions/common/wasm/context.cc:1508-1514` |
|
||||
| `~DeferAfterCallActions` drains unconditionally | `higress-group/proxy-wasm-cpp-host` `src/context.cc` (top, `~DeferAfterCallActions` calls `wasm_->doAfterVmCallActions()`) |
|
||||
| `SaveRestoreContext` save/restore semantics | `higress-group/proxy-wasm-cpp-host` `include/proxy-wasm/wasm_vm.h:392-405` |
|
||||
| `current_context_` is thread_local | `higress-group/proxy-wasm-cpp-host` `src/exports.cc:26` |
|
||||
|
||||
## Meta-lessons
|
||||
|
||||
This investigation had three rounds of analysis. The meta-lessons are the most valuable part for future investigators.
|
||||
|
||||
### Lesson 1: a single primitive can cover two bug classes — check coverage explicitly
|
||||
|
||||
The first analysis round concluded "Layer A fixes #4034 but NOT #326" and proposed them as separate work items. That was wrong. The `swap-to-local` primitive covers BOTH because:
|
||||
|
||||
- For #4034: re-added actions go to the empty member.
|
||||
- For #326: pre-existing actions stay in `local`, invisible to the nested drain.
|
||||
|
||||
**Generalization**: whenever a fix changes how a queue/collection is iterated (swap-to-local, snapshot-then-iterate, copy-on-iterate), check whether it ALSO changes cross-frame or cross-stage visibility. These primitives often cover more than the primary bug.
|
||||
|
||||
### Lesson 2: model the PATCHED code, not the unpatched code
|
||||
|
||||
When analyzing whether a fix covers a related bug, it's easy to accidentally reason about the current (unpatched) behavior instead of the patched behavior. The first analysis round said "the inner drain sees `[continueDecoding]`" — which is true for the CURRENT code but FALSE for the patched code, because the patch's swap empties the member before the inner drain runs.
|
||||
|
||||
**Generalization**: when someone proposes a fix and asks "does this also cover X?", re-read the PATCHED code, not just the diff. The patch changes program state at moments you might not be tracking.
|
||||
|
||||
### Lesson 3: "X is deferred" vs "X is direct" is a load-bearing distinction
|
||||
|
||||
Several rounds of analysis hinged on whether `sendLocalReply` and `continueDecoding` are called directly or deferred via `addAfterVmCallAction`. Both are deferred — and that determines the queue order that produces the bug.
|
||||
|
||||
**Generalization**: when a function's name suggests "does X immediately" (like `sendLocalReply`), don't trust the name. Read the function body and check whether it actually calls X directly or wraps it in `addAfterVmCallAction` / `dispatcher().post(...)` / similar defer primitives. The defer-vs-direct distinction changes everything about reentrancy analysis.
|
||||
|
||||
### Lesson 4: maintainer pushback is a signal to re-verify, not restate
|
||||
|
||||
When the maintainer challenged the "Layer A doesn't fix #326" conclusion, the right move was to re-read the code with their specific objection in mind — not to restate the conclusion with different words. The re-read found that the swap-to-local empties the member queue, which the first analysis had missed.
|
||||
|
||||
**Generalization**: maintainer pushback is high-signal information. They know the codebase. If they say "I think X", dispatch a fresh worker to verify X with code evidence. Don't confirm your own previous answer; that's confirmation bias.
|
||||
|
||||
### Lesson 5: the #326 reproducer's plugin call order matters
|
||||
|
||||
The #326 reproducer calls `SendHttpResponse` THEN `ResumeHttpRequest`. If the order were reversed, the queue would be `[continueDecoding, sendLocalReply]`, FIFO drain would pop `continueDecoding` first (in the correct phase), then `sendLocalReply` (triggering encoder reentry with empty queue) — and there would be no stage mismatch even WITHOUT Layer A.
|
||||
|
||||
**Generalization**: when analyzing a reproducer, note the exact order of plugin calls. Stage-mismatch bugs often depend on call order, and a "harmless" reorder can mask the bug.
|
||||
@@ -0,0 +1,71 @@
|
||||
# Patterns — How to Add a New Investigation Case-Study
|
||||
|
||||
This directory accumulates case-studies from past deep investigations. Each pattern captures both the technical root cause AND the meta-lessons (what tripped up the analysis, what the maintainer caught, what the fix primitive generalizes to). The library grows by one entry per major investigation.
|
||||
|
||||
## When to add a pattern
|
||||
|
||||
Add a pattern after an investigation that:
|
||||
|
||||
- Required cross-repo code-trace evidence (envoy + proxy-wasm-cpp-host, or similar).
|
||||
- Resulted in a non-trivial fix (not just a one-line config change).
|
||||
- Has a meta-lesson worth carrying forward (a gotcha, a verification failure, a primitive that generalizes).
|
||||
|
||||
If the investigation was routine (single-file bug, obvious fix, no pushback), skip the pattern — the diff IS the documentation.
|
||||
|
||||
## File naming
|
||||
|
||||
`YYYY-MM-short-slug.md` where the date is when the investigation concluded. Examples:
|
||||
|
||||
- `2026-07-wasm-doaftervmcallactions-reentry.md`
|
||||
- `2026-09-filter-chain-headers-double-dispatch.md`
|
||||
- `2026-11-redis-pool-exhaustion-under-failover.md`
|
||||
|
||||
## Pattern structure
|
||||
|
||||
Use this template. Sections in `[brackets]` are required; sections in `<angle brackets>` are optional but encouraged.
|
||||
|
||||
```markdown
|
||||
# <Bug class one-line summary>
|
||||
|
||||
> Source: #ISSUE_NUMBER — <issue title>
|
||||
> Date: YYYY-MM
|
||||
> Status: <fixed / investigating / deferred>
|
||||
|
||||
## Symptom
|
||||
|
||||
<What observable behavior was wrong? Include perf data / repro steps if relevant.>
|
||||
|
||||
## Root cause
|
||||
|
||||
<One-paragraph explanation. Cite file:line for key claims.>
|
||||
|
||||
<Numbered call-chain trace, one hop per line, with file:line.>
|
||||
|
||||
## Fix
|
||||
|
||||
<The primitive. Code snippet if non-trivial.>
|
||||
|
||||
<Why it fixes the bug — the causal chain.>
|
||||
|
||||
## Coverage
|
||||
|
||||
<What bug classes does this fix cover? What does it NOT cover? Why?>
|
||||
|
||||
## Evidence
|
||||
|
||||
<File:line citations grouped by claim.>
|
||||
|
||||
## Meta-lessons
|
||||
|
||||
<What tripped up the first analysis round? What did the maintainer catch? What generalizes to other bugs in this area?>
|
||||
|
||||
<If the analysis was wrong initially and then corrected, document BOTH the wrong reasoning AND the correction — this is the most valuable part for future investigators.>
|
||||
```
|
||||
|
||||
## Worked example
|
||||
|
||||
See [`2026-07-wasm-doaftervmcallactions-reentry.md`](2026-07-wasm-doaftervmcallactions-reentry.md) for a complete pattern that follows this structure, including a meta-lesson section documenting an initial wrong conclusion that was corrected after maintainer pushback.
|
||||
|
||||
## Linking from new investigations
|
||||
|
||||
When a new investigation reuses a primitive or hits a similar gotcha to a past pattern, link to the pattern in the new investigation's report. Patterns are most valuable when they're cross-referenced — the second time you see a `std::deque::swap`-style fix, you should be able to find the #4034 pattern and reason by analogy.
|
||||
@@ -0,0 +1,73 @@
|
||||
# Code-Trace Verification
|
||||
|
||||
The format and rules for verifying claims with code evidence during a deep investigation. Maintainers trust analysis they can audit; file:line citations are the audit trail.
|
||||
|
||||
## Core rule
|
||||
|
||||
**Every claim must cite `file:line`.** If you can't cite it, you haven't read it; if you haven't read it, you don't know.
|
||||
|
||||
## Acceptable evidence
|
||||
|
||||
| Claim type | What to cite |
|
||||
| --- | --- |
|
||||
| "Function X does Y" | `repo/path/file.cc:NN` + ≤5-line excerpt |
|
||||
| "Function X is synchronous" | The call site that lacks `dispatcher().post(...)` (or equivalent) + the sibling function that DOES use it, as contrast |
|
||||
| "The queue state is `[a, b]` at this point" | The sequence of `push_back` / `pop_front` calls leading to that state, each with file:line |
|
||||
| "The maintainer's claim is wrong" | Re-state their claim, then cite the specific code that contradicts it |
|
||||
| "This fix covers bug class Y too" | The mechanism by which it covers Y, with the call chain showing why |
|
||||
|
||||
## Unacceptable evidence
|
||||
|
||||
- "I think" / "probably" / "should be" — read the code.
|
||||
- "Similar to upstream" — check the fork commit pin in `bazel/repository_locations.bzl`.
|
||||
- "The function name implies X" — names lie. `injectEncodedDataToFilterChain` sounds innocuous; it's the trigger for a CPU-spin bug.
|
||||
- Reasoning from the bug report without code verification — the report describes symptoms.
|
||||
|
||||
## State-trace table format
|
||||
|
||||
The most effective format for communicating queue/state transitions. Use it whenever the bug involves a sequence of state changes (queue operations, `current_context_` saves/restores, reference-counting transitions).
|
||||
|
||||
Template:
|
||||
|
||||
| Step | Action | `<state variable 1>` | `<state variable 2>` |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Enter context | `[]` | `nullptr` |
|
||||
| 2 | Operation X | `[x]` | `nullptr` |
|
||||
| 3 | Operation Y | `[x, y]` | `ctx` |
|
||||
| ... | ... | ... | ... |
|
||||
|
||||
Rules:
|
||||
|
||||
- One row per discrete state change.
|
||||
- State columns should be the variables that matter for the bug (queue contents, `current_context_`, depth counter, etc.).
|
||||
- Mark the critical transition with ★ and explain it below the table.
|
||||
- If a state is "unknown" or "depends on plugin behavior", say so — don't fake precision.
|
||||
|
||||
## Worked example (from #4034)
|
||||
|
||||
The #4034 investigation hinged on the queue state at the moment of a nested drain. The state-trace table that settled the disagreement:
|
||||
|
||||
| Step | Action | `self->after_vm_call_actions_` | outer drain's `local` |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Enter `onHttpCallResponse` | `[]` | — |
|
||||
| 2 | Plugin calls `SendHttpResponse` | `[sendLocalReply]` | — |
|
||||
| 3 | Plugin calls `ResumeHttpRequest` | `[sendLocalReply, continueDecoding]` | — |
|
||||
| 4 | Outer `~DeferAfterCallActions` fires | same | — |
|
||||
| 5 | **Layer A: `local.swap(after_vm_call_actions_)`** ★ | `[]` | `[sendLocalReply, continueDecoding]` |
|
||||
| 6 | iter 1: fire `sendLocalReply` → `encodeHeaders` → `onResponseHeaders` | `[]` | same |
|
||||
| 7 | Inner `~DeferAfterCallActions` fires | `[]` (empty → no-op) | same |
|
||||
| 8 | iter 2: fire `continueDecoding` in outer context | `[]` | same |
|
||||
|
||||
The ★ at step 5 is the key insight: `std::deque::swap` empties the member, so the inner drain at step 7 sees an empty queue and does nothing. The earlier (wrong) analysis confused "queue before swap" with "queue at inner-drain time".
|
||||
|
||||
## Re-verification protocol (when the maintainer pushes back)
|
||||
|
||||
When a maintainer disagrees with your conclusion:
|
||||
|
||||
1. **Don't restate the conclusion.** Restating without new evidence wastes their time.
|
||||
2. **Identify the specific point of disagreement.** What exactly do they claim?
|
||||
3. **Dispatch a fresh worker** with the specific objection. The worker brief should say: "Maintainer claims X. Verify whether X is true. Cite file:line."
|
||||
4. **If the worker confirms the maintainer**: acknowledge the error publicly, explain what you missed, update artifacts.
|
||||
5. **If the worker refutes the maintainer**: respond with the specific code evidence, walk through the state-trace table, be respectful.
|
||||
|
||||
The third verification (yours) doesn't count if it just restates the second (also yours). You need to actually re-read the code with the maintainer's objection in mind, not just re-derive your previous answer.
|
||||
@@ -0,0 +1,118 @@
|
||||
# Investigation Methodology
|
||||
|
||||
The step-by-step process for deep-diving a Higress bug. This is the canonical workflow; adapt it to the bug at hand but don't skip the evidence-collection steps.
|
||||
|
||||
## 0. Before you start: scope the investigation
|
||||
|
||||
Confirm the bug is real and worth a deep dive. Signs it's worth it:
|
||||
|
||||
- Reproducer exists (even if intermittent).
|
||||
- perf data or stack trace points at specific functions.
|
||||
- The fix likely requires code changes (not config/docs).
|
||||
- The blast radius is significant (production-affecting, multiple users).
|
||||
|
||||
If the bug is a one-off without a reproducer, or clearly a user-config issue, don't burn a deep investigation on it.
|
||||
|
||||
## 1. Symptom → code surface
|
||||
|
||||
Read the bug report and extract:
|
||||
|
||||
- **Symptom**: what observable behavior is wrong? (CPU spin, crash, wrong response, hang.)
|
||||
- **Trigger conditions**: what's the minimal repro path?
|
||||
- **perf / stack data**: which functions dominate?
|
||||
|
||||
Then map symptoms to code surfaces. For Wasm-related bugs, the surface is almost always split across:
|
||||
|
||||
- `higress-group/envoy` `source/extensions/common/wasm/context.cc` — Higress-specific `Context` subclass, foreign functions like `injectEncodedDataToFilterChain`.
|
||||
- `higress-group/proxy-wasm-cpp-host` `include/proxy-wasm/wasm.h` + `src/context.cc` — VM host runtime, `WasmBase`, `ContextBase`, deferred-action queue.
|
||||
|
||||
**Always check the envoy fork's `bazel/repository_locations.bzl` for the pinned `proxy-wasm-cpp-host` commit.** The fork diverges from upstream; reasoning about upstream code without checking the pin leads to wrong conclusions.
|
||||
|
||||
## 2. Parallel investigation (worker dispatch)
|
||||
|
||||
For non-trivial bugs, dispatch focused worker agents IN PARALLEL for independent aspects. Each worker gets a self-contained brief (it has no shared context with you). Example dispatch for a Wasm reentrancy bug:
|
||||
|
||||
- **Worker A (envoy side)**: verify the trigger path in `higress-group/envoy`. Find the synchronous call, the self-reenqueueing callbacks, the foreign-function registration. Return file:line evidence.
|
||||
- **Worker B (proxy-wasm-cpp-host side)**: verify the engine. Find the queue, the drain loop, the `SaveRestoreContext` semantics. Return file:line evidence.
|
||||
- **Worker C (upstream references)**: fetch any linked upstream issues/PRs, determine if they're related/resolved/portable.
|
||||
|
||||
Worker rules:
|
||||
|
||||
- Each worker is **research-only** unless you explicitly authorize code changes. Don't let workers edit code on their own initiative.
|
||||
- Each worker must **cite file:line for every claim**. Reject worker reports that say "the function does X" without a citation.
|
||||
- Cap report length (e.g., 600-800 words) to keep synthesis tractable.
|
||||
- Use `general-purpose` agent type for investigations that need to clone repos and reason across files; use `Explore` for narrow single-repo lookups.
|
||||
|
||||
## 3. Code-trace verification
|
||||
|
||||
Every claim in the final analysis must reduce to a file:line citation. The output format that works best for maintainer communication is the **step-by-step state-trace table**:
|
||||
|
||||
| Step | Action | Member queue state | Local queue state |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Enter callback | `[]` | — |
|
||||
| 2 | Plugin calls X | `[x]` | — |
|
||||
| ... | ... | ... | ... |
|
||||
|
||||
See [`code-trace-verification.md`](code-trace-verification.md) for the full format and a worked example.
|
||||
|
||||
Anti-patterns to avoid:
|
||||
|
||||
- **"I think the function does X"** — unacceptable. Read the code and cite the line.
|
||||
- **"This is probably similar to upstream"** — check the fork commit pin.
|
||||
- **"The drain loop terminates because of course it does"** — verify the actual loop condition.
|
||||
- **Reasoning from the bug report without reading the code** — the report describes symptoms, not causes.
|
||||
|
||||
## 4. Synthesize root cause
|
||||
|
||||
Combine worker findings into a numbered call-chain trace. For each hop:
|
||||
|
||||
- The function name.
|
||||
- The file:line.
|
||||
- What state it reads/writes (e.g., `current_context_` value, queue contents).
|
||||
|
||||
End with a one-sentence root cause: "X happens because Y, which is unguarded because Z."
|
||||
|
||||
## 5. Fix design + side effects
|
||||
|
||||
Propose a fix primitive. For each candidate:
|
||||
|
||||
- **What it changes**: the specific code transformation.
|
||||
- **Why it fixes the bug**: the causal chain.
|
||||
- **Side effects**: what else it touches. Be honest — even a 5-line fix can have ABI or lifecycle implications.
|
||||
- **Coverage**: does it also cover related bug classes? (E.g., drain-to-local covers both CPU-spin and stage-mismatch.)
|
||||
|
||||
If a fix has unacceptable side effects, document it as "deferred" and explain why. Don't sneak a behavior-change through as a "bug fix".
|
||||
|
||||
## 6. Cross-check related bug classes
|
||||
|
||||
Before declaring done, check: does the fix also cover bug classes that share the same machinery?
|
||||
|
||||
- Look at upstream issues in `proxy-wasm/proxy-wasm-cpp-host` that reference the same functions.
|
||||
- Look at the Higress fork's divergence from upstream — are there Higress-specific code paths that have the same bug?
|
||||
- Look at the `addAfterVmCallAction` callsites in envoy — are there other self-reenqueueing or cross-stage patterns?
|
||||
|
||||
Document the coverage explicitly: "Fixes X. Does NOT fix Y because Z. Y tracked separately as ..."
|
||||
|
||||
## 7. Verify when challenged
|
||||
|
||||
If the maintainer pushes back on a conclusion, **do not just restate the conclusion**. Re-read the code with their specific objection in mind.
|
||||
|
||||
Common failure modes in the first analysis round:
|
||||
|
||||
- **Confusing "state before operation" with "state after operation"**. (E.g., "the queue is `[X]`" — but is that before or after the swap?)
|
||||
- **Assuming a function is sync/async without checking**. (E.g., assuming `sendLocalReply` is direct when it's actually deferred via `addAfterVmCallAction`.)
|
||||
- **Reasoning about the patched code as if it were the unpatched code**. (E.g., analyzing Layer A's `doAfterVmCallActions` as if it still iterates the member queue in place, when actually it swaps to a local first.)
|
||||
- **Assuming plugin call order**. (E.g., assuming `ResumeHttpRequest` is called before `SendHttpResponse` — verify from the reproducer.)
|
||||
|
||||
When you re-verify, dispatch a fresh worker with the specific objection. The worker should be told: "the maintainer claims X; verify whether X is true with code evidence." This avoids confirmation bias.
|
||||
|
||||
## 8. Document the pattern
|
||||
|
||||
After the investigation concludes, add a pattern file to `patterns/` capturing:
|
||||
|
||||
- The bug class (one-line summary).
|
||||
- The root cause (with file:line evidence).
|
||||
- The fix primitive (with the code snippet).
|
||||
- The meta-lesson (what tripped up the analysis, what the maintainer caught, what generalizes).
|
||||
|
||||
This is what makes the skill accumulate value over time. See [`patterns/README.md`](../patterns/README.md) for the format.
|
||||
@@ -0,0 +1,64 @@
|
||||
# Code-Trace Table Template
|
||||
|
||||
Use this template for the state-trace table that communicates queue/state transitions to maintainers. This format was developed during the #4034 investigation and proved effective for settling disagreements about program state at specific moments.
|
||||
|
||||
## When to use
|
||||
|
||||
- The bug involves a sequence of state changes (queue operations, `current_context_` saves/restores, reference-count transitions).
|
||||
- You need to communicate "what the program state is at moment X" to someone who wasn't in your head.
|
||||
- A maintainer is challenging a conclusion about state at a specific point in the call chain.
|
||||
|
||||
## Template
|
||||
|
||||
```markdown
|
||||
| Step | Action | `<state variable 1>` | `<state variable 2>` | ... |
|
||||
| --- | --- | --- | --- | ... |
|
||||
| 1 | <entry point> | `<initial value>` | `<initial value>` | ... |
|
||||
| 2 | <operation> | `<value after>` | `<value after>` | ... |
|
||||
| 3 | <operation> | `<value after>` | `<value after>` | ... |
|
||||
| ... | ... | ... | ... | ... |
|
||||
| N ★ | <critical transition> | `<value>` | `<value>` | ... |
|
||||
```
|
||||
|
||||
## Rules
|
||||
|
||||
1. **One row per discrete state change.** Don't combine multiple operations into one row.
|
||||
2. **State columns are the variables that matter for the bug.** Common ones for Wasm bugs:
|
||||
- `self->after_vm_call_actions_` (the deferred-action queue)
|
||||
- `current_context_` (the thread_local ContextBase*)
|
||||
- `SaveRestoreContext` depth (how many nested VM frames)
|
||||
- `local` (the outer drain's swap target, post-Layer-A)
|
||||
3. **Mark the critical transition with ★** and explain it below the table. This is the moment where the bug happens (or doesn't).
|
||||
4. **Show the queue contents explicitly** — `[sendLocalReply, continueDecoding]`, not "non-empty". Order matters for FIFO drains.
|
||||
5. **If state is unknown / depends on plugin behavior, say so.** Don't fake precision. Better: "depends on whether the plugin called X before Y; both branches shown below".
|
||||
6. **Pair the table with a prose explanation of the ★ transition.** The table shows WHAT; the prose explains WHY.
|
||||
|
||||
## Worked example
|
||||
|
||||
From #4034 — the table that settled whether Layer A's swap-to-local covers #326:
|
||||
|
||||
```markdown
|
||||
| Step | Action | `self->after_vm_call_actions_` | outer drain's `local` |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Enter `onHttpCallResponse` | `[]` | — |
|
||||
| 2 | Plugin calls `SendHttpResponse` | `[sendLocalReply]` | — |
|
||||
| 3 | Plugin calls `ResumeHttpRequest` | `[sendLocalReply, continueDecoding]` | — |
|
||||
| 4 | Outer `~DeferAfterCallActions` fires | `[sendLocalReply, continueDecoding]` | — |
|
||||
| 5 ★ | **Layer A: `local.swap(after_vm_call_actions_)`** | `[]` | `[sendLocalReply, continueDecoding]` |
|
||||
| 6 | iter 1: fire `sendLocalReply` → `encodeHeaders` → `onResponseHeaders` | `[]` | `[sendLocalReply, continueDecoding]` |
|
||||
| 7 | Inner `~DeferAfterCallActions` fires | `[]` (empty → no-op) | `[sendLocalReply, continueDecoding]` |
|
||||
| 8 | iter 2: fire `continueDecoding` in outer context | `[]` | (iterating) |
|
||||
```
|
||||
|
||||
**Why the ★ matters**: `std::deque::swap` is O(1) whole-content exchange. After step 5, the member queue is empty — it's NOT `[continueDecoding]` anymore. So when step 7's inner drain checks `if (!after_vm_call_actions_.empty())`, it sees `false` and returns. `continueDecoding` stays in the outer drain's `local` and fires in step 8, in the outer drain's context (correct phase).
|
||||
|
||||
The earlier (wrong) analysis had assumed the member queue still contained `continueDecoding` at step 7 — confusing "queue before swap" with "queue after swap".
|
||||
|
||||
## Anti-patterns
|
||||
|
||||
Don't do these:
|
||||
|
||||
- **Combining steps**: "steps 5-8: drain happens, continueDecoding fires" — too coarse; the reader can't see the state at each moment.
|
||||
- **Hiding the critical transition**: burying the ★ in a paragraph of prose. The table IS the argument; make it sharp.
|
||||
- **Vague state**: "queue has some items" — which items? In what order? FIFO drains care about order.
|
||||
- **No prose explanation**: the table alone isn't enough; pair it with 2-3 sentences explaining the ★.
|
||||
@@ -0,0 +1,109 @@
|
||||
# Investigation Report Template
|
||||
|
||||
Use this structure when starting a new deep investigation. Fill in the brackets; remove sections that don't apply. The goal is a report that a maintainer can audit end-to-end: every claim has a citation, every fix has a side-effect analysis.
|
||||
|
||||
---
|
||||
|
||||
# Investigation: <one-line bug summary>
|
||||
|
||||
> Source: #ISSUE_NUMBER
|
||||
> Investigator: <name / agent session>
|
||||
> Date: YYYY-MM-DD
|
||||
> Status: <in-progress / root-caused / fix-designed / fixed>
|
||||
|
||||
## Symptom
|
||||
|
||||
<What observable behavior is wrong? Include:>
|
||||
|
||||
- **Observable**: <what the user sees — CPU spin, crash, wrong response, hang>
|
||||
- **Trigger conditions**: <minimal repro path>
|
||||
- **perf / stack data**: <paste perf sampling, stack trace, or relevant telemetry>
|
||||
- **Blast radius**: <which versions, which components, how many users>
|
||||
|
||||
## Code surfaces
|
||||
|
||||
<Which repos and files are involved? Use a table:>
|
||||
|
||||
| Concern | Repo | File | Why it matters |
|
||||
| --- | --- | --- | --- |
|
||||
| <concern> | <repo URL> | <path:line> | <one-line role> |
|
||||
|
||||
<For Wasm bugs, always check the envoy fork's `bazel/repository_locations.bzl` for the pinned proxy-wasm-cpp-host commit. Note it here.>
|
||||
|
||||
## Worker dispatch
|
||||
|
||||
<If you dispatched parallel workers, list them with their brief and key finding:>
|
||||
|
||||
- **Worker A (<repo>) — <one-line brief>**: <VERDICT — one-line finding>. Key evidence: <file:line>.
|
||||
- **Worker B (<repo>) — <one-line brief>**: ...
|
||||
|
||||
<If the investigation was single-threaded (no workers), say so and explain why.>
|
||||
|
||||
## Root cause
|
||||
|
||||
<One-paragraph explanation. Be specific about the mechanism.>
|
||||
|
||||
<Numbered call-chain trace, one hop per line:>
|
||||
|
||||
1. <Function> (`repo file.cc:NN`) — <what it does>
|
||||
2. <Function> (`repo file.cc:NN`) — <what it does>
|
||||
3. ...
|
||||
|
||||
<For queue/state bugs, include a state-trace table here. See `code-trace-verification.md` for the format.>
|
||||
|
||||
**Root cause (one sentence)**: <X happens because Y, which is unguarded because Z>.
|
||||
|
||||
## Fix
|
||||
|
||||
<The primitive. Code snippet if non-trivial.>
|
||||
|
||||
```cpp
|
||||
// Before
|
||||
<current code>
|
||||
|
||||
// After
|
||||
<fixed code>
|
||||
```
|
||||
|
||||
**Why it fixes the bug**: <causal chain, 2-3 sentences.>
|
||||
|
||||
**Side effects**: <what else the change touches. Be honest — even a 5-line fix can have ABI or lifecycle implications. If there are none, say "none identified" and explain the verification.>
|
||||
|
||||
## Coverage analysis
|
||||
|
||||
<Does the fix cover related bug classes? Check upstream issues, related callbacks, similar patterns.>
|
||||
|
||||
| Bug class | Covered? | Why |
|
||||
| --- | --- | --- |
|
||||
| <primary bug> | yes | <mechanism> |
|
||||
| <related class 1> | yes/no | <mechanism or reason> |
|
||||
| <related class 2> | yes/no | ... |
|
||||
|
||||
<For each "no", note where it's tracked (separate issue, deferred SPEC, etc.).>
|
||||
|
||||
## Evidence index
|
||||
|
||||
<All file:line citations grouped by claim. This is the audit trail.>
|
||||
|
||||
| Claim | Citation |
|
||||
| --- | --- |
|
||||
| <claim> | `repo/path/file.cc:NN` |
|
||||
| ... | ... |
|
||||
|
||||
## Open questions
|
||||
|
||||
<Any unresolved questions for the maintainer. Each should be specific and actionable.>
|
||||
|
||||
- **Q1**: <question>
|
||||
- **Q2**: <question>
|
||||
|
||||
## Next steps
|
||||
|
||||
<What happens next. Be concrete:>
|
||||
|
||||
- [ ] <action 1>
|
||||
- [ ] <action 2>
|
||||
|
||||
---
|
||||
|
||||
<After the investigation concludes, distill the meta-lessons and add a pattern file to `patterns/`. See `patterns/README.md` for the format.>
|
||||
156
AGENTS.md
156
AGENTS.md
@@ -1,156 +0,0 @@
|
||||
# AGENTS.md
|
||||
|
||||
Guidance for AI agents working in this repository.
|
||||
|
||||
Higress is a cloud-native API gateway built on Istio and Envoy. The control
|
||||
plane extends Istio/pilot (Go); the data plane is Envoy extended with WASM
|
||||
plugins (Go/Rust/C++/AssemblyScript) and a Go-based `golang-filter`. It supports
|
||||
Ingress/Gateway API and ships a rich plugin ecosystem (including AI gateway
|
||||
plugins).
|
||||
|
||||
## Repository layout
|
||||
|
||||
Top-level directories (all paths relative to repo root):
|
||||
|
||||
- `cmd/higress/` — main entrypoint (`main.go`) for the Higress controller binary.
|
||||
- `pkg/` — core Go control-plane packages: `bootstrap/`, `cert/`, `cmd/`,
|
||||
`common/`, `config/`, `ingress/` (Ingress/Gateway config translation),
|
||||
`kube/`.
|
||||
- `api/` — protobuf/CRD API definitions; Higress CRDs live in
|
||||
`api/extensions/v1alpha1` (e.g. the `WasmPlugin` type). Generated with
|
||||
`make gen-api` / `make gen-client` (see `api/gen.sh`, `buf.*`).
|
||||
- `client/` — generated Go clientset for Higress CRDs.
|
||||
- `istio/` — git submodules of higress-group forks of Istio (`api`, `istio`,
|
||||
`client-go`, `pkg`, `proxy`); see `.gitmodules`. Pulled via `make submodule`
|
||||
(part of `prebuild`).
|
||||
- `envoy/` — Envoy + `go-control-plane` submodules (higress-group forks).
|
||||
- `external/` — vendored/external mirror dirs used during build (istio, envoy,
|
||||
proxy, etc.).
|
||||
- `plugins/` — all data-plane plugins (see "Plugins" below).
|
||||
- `registry/` — service-discovery registry integrations (nacos, consul, eureka,
|
||||
zookeeper, direct, mcp, ...).
|
||||
- `hgctl/` — the `hgctl` CLI (separate Go module) for managing Higress.
|
||||
- `helm/` — Helm charts: `helm/core` (the dev/install chart) and `helm/higress`.
|
||||
- `test/` — `test/e2e/` (conformance/e2e, see "Build & test") and
|
||||
`test/gateway/`.
|
||||
- `tools/` — build/CI scripting: `tools/hack/` (build scripts), `tools/bin/`,
|
||||
`tools/linter/`, `*.mk`.
|
||||
- `samples/` — example manifests (gateway-api, hello-world, wasmplugin, ...).
|
||||
- `docker/`, `docs/`, `release-notes/` — packaging, docs, and release notes.
|
||||
- `Makefile` — istio common-files wrapper (supports `BUILD_WITH_CONTAINER`);
|
||||
real targets live in `Makefile.core.mk` (+ `Makefile.overrides.mk`).
|
||||
|
||||
## Plugins
|
||||
|
||||
All plugins live under `plugins/`. See `plugins/README.md` for the contributor
|
||||
overview. Prebuilt plugin images are published to
|
||||
`higress-registry.cn-hangzhou.cr.aliyuncs.com/plugins`.
|
||||
|
||||
### plugins/wasm-go/ (primary WASM plugin framework, Go)
|
||||
|
||||
- `extensions/<name>/` — one directory per plugin (~59 plugins, many `ai-*`).
|
||||
Each plugin is its own Go module: `main.go`, `go.mod`/`go.sum`, `VERSION`,
|
||||
`README.md`(+`README_EN.md`), often `config/`, `util/`, `main_test.go`.
|
||||
Optional `.buildrc` sets `EXTRA_TAGS`; optional `prepare.sh`/`prepare.sh`.
|
||||
`plugin.wasm` is a build artifact and is **not** committed.
|
||||
- Shared SDK: plugins depend on external modules
|
||||
`github.com/higress-group/wasm-go` and
|
||||
`github.com/higress-group/proxy-wasm-go-sdk` (NOT an in-repo SDK dir).
|
||||
In-repo, `plugins/wasm-go/pkg/mcp/` provides MCP helpers and
|
||||
`plugins/wasm-go/mcp-servers/` holds MCP server plugins.
|
||||
- `examples/` — minimal reference plugins (custom-log, custom-span-attribute,
|
||||
test-foreign-function).
|
||||
- Build: `plugins/wasm-go/Makefile`. `PLUGIN_NAME=<name> make build` builds a
|
||||
wasm file (output to `extensions/<name>/plugin.wasm`) + image via
|
||||
`Dockerfile`/`DockerfileBuilder` (uses a `wasm-go-builder` image, Go 1.24,
|
||||
TinyGo optional). `make build-push` pushes the image; `make local-build`
|
||||
builds locally with `GOOS=wasip1 GOARCH=wasm go build -buildmode=c-shared`.
|
||||
- Conventions: `VERSION` is the image tag; the CI/e2e batch builder
|
||||
(`tools/hack/build-wasm-plugins.sh`) only compiles a wasm-go plugin whose
|
||||
`VERSION` ends in `-alpha` (see the section at the bottom of this file).
|
||||
|
||||
### plugins/wasm-rust/ (Rust WASM plugins)
|
||||
|
||||
- Workspace-style: root `Cargo.toml`/`Cargo.lock`, shared `src/`,
|
||||
`extensions/<name>/` per plugin (e.g. `ai-data-masking`, `ai-intent`,
|
||||
`request-block`, `say-hello`, `demo-wasm`), `example/`.
|
||||
- Build via `plugins/wasm-rust/Makefile` (`PLUGIN_NAME=<name> make build`, plus
|
||||
`lint`/`test`); the batch builder runs it when `PLUGIN_TYPE=RUST`.
|
||||
|
||||
### plugins/wasm-cpp/ (C++ WASM plugins, Bazel)
|
||||
|
||||
- Bazel project: `WORKSPACE`, `BUILD`, `bazel/`, `common/`, `scripts/`,
|
||||
`extensions/<name>/` (e.g. `basic_auth`, `jwt_auth`, `key_rate_limit`,
|
||||
`model_router`, ...). Build via `plugins/wasm-cpp/Makefile`
|
||||
(`PLUGIN_NAME=<name> make build`), invoked with `PLUGIN_TYPE=CPP`.
|
||||
|
||||
### plugins/wasm-assemblyscript/ (AssemblyScript WASM plugins)
|
||||
|
||||
- Node/AssemblyScript project: `asconfig.json`, `package.json`, `assembly/`,
|
||||
`extensions/`.
|
||||
|
||||
### plugins/golang-filter/ (Envoy Go HTTP filter, NOT WASM)
|
||||
|
||||
- A native Envoy Golang HTTP filter (`main.go`, `mcp-server/`, `mcp-session/`);
|
||||
compiled as a shared object (`.so`) independent of Envoy — no Envoy rebuild
|
||||
needed. Requires Higress >= 2.1.0. Plugins register in `main.go`'s `init()`
|
||||
via `RegisterHttpFilterFactoryAndConfigParser`. See
|
||||
`plugins/golang-filter/README.md`.
|
||||
- Build: `plugins/golang-filter/Makefile` (docker build, outputs
|
||||
`golang-filter_<arch>.so`). Wired into the gateway image build via
|
||||
`Makefile.core.mk` targets `build-golang-filter[-amd64|-arm64]`.
|
||||
|
||||
### How plugins are loaded
|
||||
|
||||
`WasmPlugin` CRDs (`extensions.higress.io/v1alpha1`) reference a plugin by
|
||||
`url:` — either `oci://.../plugins/<name>:<version>` (image) or
|
||||
`file:///opt/plugins/.../plugin.wasm` (local mount used in e2e). The dev install
|
||||
`make install-dev-wasmplugin` sets Helm `global.volumeWasmPlugins=true` to mount
|
||||
locally built wasm files into the gateway.
|
||||
|
||||
## Build & test
|
||||
|
||||
Run targets from the repo root; `Makefile` delegates to `Makefile.core.mk`.
|
||||
Common ones:
|
||||
|
||||
- `make build` / `make build-linux` — build the Higress controller binary
|
||||
(`prebuild` first fetches submodules).
|
||||
- `make build-hgctl` — build the `hgctl` CLI.
|
||||
- `make build-gateway` / `make build-istio` / `make build-envoy` — data-plane
|
||||
and control-plane images (gateway pulls in the golang-filter).
|
||||
- `make build-wasmplugins` — runs `tools/hack/build-wasm-plugins.sh` to batch
|
||||
build WASM plugins (respects `PLUGIN_TYPE` / `PLUGIN_NAME`; Go plugins require
|
||||
a `-alpha` VERSION).
|
||||
- `make gen-api` / `make gen-client` — regenerate API/client code.
|
||||
|
||||
### Conformance / e2e tests (`test/e2e/`)
|
||||
|
||||
- Entrypoint `test/e2e/e2e_test.go`, run with build tag `conformance` and
|
||||
`--test-area` / `--execute-tests` flags.
|
||||
- Cases live in `test/e2e/conformance/tests/` as **paired `<name>.go` +
|
||||
`<name>.yaml`** files (~68 cases; WASM cases are prefixed by language, e.g.
|
||||
`go-wasm-*`, `cpp-wasm-*`). Support code: `conformance/base/`,
|
||||
`conformance/utils/`, `conformance/embed.go`.
|
||||
- Key Make targets (each spins up a kind cluster):
|
||||
- `make higress-conformance-test` — Ingress/Gateway conformance.
|
||||
- `make higress-wasmplugin-test` — WASM plugin e2e (uses
|
||||
`install-dev-wasmplugin`, which builds plugins and mounts them).
|
||||
- `*-prepare` / `*-skip-docker-build` / `*-clean` variants exist for
|
||||
iterating; `run-higress-e2e-test[-wasmplugin]` runs `go test` against an
|
||||
already-prepared cluster (filter with `TEST_SHORTNAME`).
|
||||
- For the specifics of authoring a wasm-go e2e test, see the section below.
|
||||
|
||||
## Writing e2e conformance tests with wasm-go plugins
|
||||
|
||||
When adding an e2e conformance test that ships its own wasm-go plugin under
|
||||
`plugins/wasm-go/extensions/<name>/`:
|
||||
|
||||
- The plugin's `VERSION` file **must end in `-alpha`** (e.g. `1.0.0-alpha`).
|
||||
CI's `tools/hack/build-wasm-plugins.sh` only compiles a wasm-go plugin when
|
||||
its version ends in `-alpha`; otherwise it silently skips it.
|
||||
- `plugin.wasm` is a build artifact and is **not** committed. If the plugin
|
||||
isn't built, the `file:///opt/plugins/.../plugin.wasm` URL in the test's
|
||||
`WasmPlugin` manifest resolves to a missing file, envoy rejects the wasm
|
||||
config and fails closed, and every request on that route returns HTTP 500.
|
||||
Locally this can be masked because a previously built `plugin.wasm` still
|
||||
exists on disk — so a test can pass locally yet 500 in CI.
|
||||
@@ -205,7 +205,7 @@ install: pre-install
|
||||
helm install higress helm/higress -n higress-system --create-namespace --set 'global.local=true'
|
||||
|
||||
HIGRESS_LATEST_IMAGE_TAG ?= latest
|
||||
ENVOY_LATEST_IMAGE_TAG ?= 91244c578aef498af93cacb2cf353f3878b92fc4
|
||||
ENVOY_LATEST_IMAGE_TAG ?= fcc50202f47e27f6b8391a4bd9bbc0a9127d89d7
|
||||
ISTIO_LATEST_IMAGE_TAG ?= de2c9628294f51b13c4a70b3a862b4372890797a
|
||||
|
||||
install-dev: pre-install
|
||||
|
||||
Submodule envoy/envoy updated: f468a1a3ac...f49d591e67
@@ -1 +0,0 @@
|
||||
1.0.0-alpha
|
||||
@@ -1,20 +0,0 @@
|
||||
module github.com/alibaba/higress/plugins/wasm-go/extensions/test-redis-inject-spin
|
||||
|
||||
go 1.24.1
|
||||
|
||||
toolchain go1.24.4
|
||||
|
||||
require (
|
||||
github.com/higress-group/proxy-wasm-go-sdk v0.0.0-20251103120604-77e9cce339d2
|
||||
github.com/higress-group/wasm-go v1.0.10-0.20260120033417-1c84f010156d
|
||||
github.com/tidwall/gjson v1.18.0
|
||||
github.com/tidwall/resp v0.1.1
|
||||
google.golang.org/protobuf v1.36.6
|
||||
)
|
||||
|
||||
require (
|
||||
github.com/google/uuid v1.6.0 // indirect
|
||||
github.com/tidwall/match v1.1.1 // indirect
|
||||
github.com/tidwall/pretty v1.2.1 // indirect
|
||||
github.com/tidwall/sjson v1.2.5 // indirect
|
||||
)
|
||||
@@ -1,32 +0,0 @@
|
||||
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
|
||||
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
|
||||
github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
|
||||
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
|
||||
github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
|
||||
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
|
||||
github.com/higress-group/proxy-wasm-go-sdk v0.0.0-20251103120604-77e9cce339d2 h1:NY33OrWCJJ+DFiLc+lsBY4Ywor2Ik61ssk6qkGF8Ypo=
|
||||
github.com/higress-group/proxy-wasm-go-sdk v0.0.0-20251103120604-77e9cce339d2/go.mod h1:tRI2LfMudSkKHhyv1uex3BWzcice2s/l8Ah8axporfA=
|
||||
github.com/higress-group/wasm-go v1.0.10-0.20260120033417-1c84f010156d h1:LgYbzEBtg0+LEqoebQeMVgAB6H5SgqG+KN+gBhNfKbM=
|
||||
github.com/higress-group/wasm-go v1.0.10-0.20260120033417-1c84f010156d/go.mod h1:uKVYICbRaxTlKqdm8E0dpjbysxM8uCPb9LV26hF3Km8=
|
||||
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
|
||||
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
|
||||
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
|
||||
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
|
||||
github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
|
||||
github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY=
|
||||
github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
|
||||
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
|
||||
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
|
||||
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
|
||||
github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
|
||||
github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
|
||||
github.com/tidwall/resp v0.1.1 h1:Ly20wkhqKTmDUPlyM1S7pWo5kk0tDu8OoC/vFArXmwE=
|
||||
github.com/tidwall/resp v0.1.1/go.mod h1:3/FrruOBAxPTPtundW0VXgmsQ4ZBA0Aw714lVYgwFa0=
|
||||
github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
|
||||
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
|
||||
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
|
||||
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
|
||||
google.golang.org/protobuf v1.36.6 h1:z1NpPI8ku2WgiWnf+t9wTPsn6eP1L7ksHUlkfLvd9xY=
|
||||
google.golang.org/protobuf v1.36.6/go.mod h1:jduwjTPXsFjZGTmRluh+L6NjiWu7pchiJ2/5YcXBHnY=
|
||||
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
|
||||
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
|
||||
@@ -1,119 +0,0 @@
|
||||
// Repro plugin for higress issue #4034: worker CPU spin in
|
||||
// WasmBase::doAfterVmCallActions when a deferred async callback synchronously
|
||||
// calls injectEncodedDataToFilterChain (nesting SaveRestoreContext) so that the
|
||||
// after-vm-call action re-queues itself forever.
|
||||
//
|
||||
// Repro shape (dead-Redis only, self-contained — no external HTTP dependency):
|
||||
// 1. In the response-header phase, pause the response.
|
||||
// 2. Fire N Redis commands against an UNREACHABLE redis cluster.
|
||||
// 3. Each failure callback (onRedisCallFailure) synchronously calls the
|
||||
// inject_encoded_data_to_filter_chain foreign function. Because sibling
|
||||
// failure callbacks are still queued as after-vm-call actions, the nested
|
||||
// SaveRestoreContext leaves current_context_ != nullptr and the deferred
|
||||
// action re-queues itself -> CPU spin on the pre-fix host.
|
||||
//
|
||||
// With the Layer A drain-to-local fix in place, the drain terminates over a
|
||||
// snapshot and worker CPU stays bounded.
|
||||
package main
|
||||
|
||||
import (
|
||||
"github.com/higress-group/proxy-wasm-go-sdk/proxywasm"
|
||||
"github.com/higress-group/proxy-wasm-go-sdk/proxywasm/types"
|
||||
"github.com/higress-group/wasm-go/pkg/log"
|
||||
pb "github.com/higress-group/wasm-go/pkg/protos"
|
||||
"github.com/higress-group/wasm-go/pkg/wrapper"
|
||||
"github.com/tidwall/gjson"
|
||||
"github.com/tidwall/resp"
|
||||
"google.golang.org/protobuf/proto"
|
||||
)
|
||||
|
||||
func main() {}
|
||||
|
||||
type Config struct {
|
||||
redisClient *wrapper.RedisClusterClient[wrapper.FQDNCluster]
|
||||
redisKey string
|
||||
injectBody string
|
||||
injectCount int
|
||||
}
|
||||
|
||||
func init() {
|
||||
wrapper.SetCtx(
|
||||
"test-redis-inject-spin",
|
||||
wrapper.ParseConfig(parseConfig),
|
||||
wrapper.ProcessResponseHeaders(onHttpResponseHeaders),
|
||||
)
|
||||
}
|
||||
|
||||
func parseConfig(json gjson.Result, config *Config) error {
|
||||
serviceName := json.Get("service_name").String()
|
||||
if serviceName == "" {
|
||||
serviceName = "dead-redis.dead-redis.svc.cluster.local"
|
||||
}
|
||||
servicePort := json.Get("service_port").Int()
|
||||
if servicePort == 0 {
|
||||
servicePort = 6379
|
||||
}
|
||||
config.redisKey = json.Get("redis_key").String()
|
||||
if config.redisKey == "" {
|
||||
config.redisKey = "higress-4034-key"
|
||||
}
|
||||
config.injectBody = json.Get("inject_body").String()
|
||||
if config.injectBody == "" {
|
||||
config.injectBody = "injected-by-4034-repro\n"
|
||||
}
|
||||
config.injectCount = int(json.Get("inject_count").Int())
|
||||
if config.injectCount == 0 {
|
||||
config.injectCount = 16
|
||||
}
|
||||
|
||||
config.redisClient = wrapper.NewRedisClusterClient(wrapper.FQDNCluster{
|
||||
FQDN: serviceName,
|
||||
Port: servicePort,
|
||||
})
|
||||
// Init never returns error for an unreachable host; commands' callbacks will
|
||||
// fire with an error value, which is exactly the #4034 trigger path.
|
||||
return config.redisClient.Init("", "", 1000)
|
||||
}
|
||||
|
||||
func onHttpResponseHeaders(ctx wrapper.HttpContext, config Config) types.Action {
|
||||
proxywasm.RemoveHttpResponseHeader("content-length")
|
||||
ctx.DontReadResponseBody()
|
||||
|
||||
inject := func() {
|
||||
d := &pb.InjectEncodedDataToFilterChainArguments{
|
||||
Body: config.injectBody,
|
||||
Endstream: true,
|
||||
}
|
||||
s, err := proto.Marshal(d)
|
||||
if err != nil {
|
||||
log.Errorf("marshal inject args failed: %+v", err)
|
||||
return
|
||||
}
|
||||
if _, err := proxywasm.CallForeignFunction("inject_encoded_data_to_filter_chain_on_header", s); err != nil {
|
||||
log.Errorf("call inject_encoded_data_to_filter_chain_on_header failed: %+v", err)
|
||||
}
|
||||
}
|
||||
|
||||
scheduled := 0
|
||||
for i := 0; i < config.injectCount; i++ {
|
||||
err := config.redisClient.Get(config.redisKey, func(response resp.Value) {
|
||||
// Fires on redis-unreachable failure; synchronously inject to nest
|
||||
// SaveRestoreContext while sibling callbacks are still queued.
|
||||
if response.Error() != nil {
|
||||
log.Debugf("redis get failed as expected: %v", response.Error())
|
||||
}
|
||||
inject()
|
||||
})
|
||||
if err != nil {
|
||||
log.Errorf("redis Get dispatch failed: %+v", err)
|
||||
continue
|
||||
}
|
||||
scheduled++
|
||||
}
|
||||
|
||||
if scheduled == 0 {
|
||||
// Nothing dispatched (e.g. cluster missing) — don't hang the response.
|
||||
return types.ActionContinue
|
||||
}
|
||||
return types.ActionPause
|
||||
}
|
||||
@@ -1,75 +0,0 @@
|
||||
// Copyright (c) 2026 Alibaba Group Holding Ltd.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package tests
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/alibaba/higress/v2/test/e2e/conformance/utils/http"
|
||||
"github.com/alibaba/higress/v2/test/e2e/conformance/utils/suite"
|
||||
)
|
||||
|
||||
func init() {
|
||||
Register(WasmPluginsRedisInjectSpin)
|
||||
}
|
||||
|
||||
// WasmPluginsRedisInjectSpin reproduces issue #4034: a deferred async Redis
|
||||
// failure callback synchronously calls injectEncodedDataToFilterChain, which
|
||||
// (pre-fix) makes WasmBase::doAfterVmCallActions re-queue an action forever and
|
||||
// spins a worker at 100% CPU. Redis is pointed at an unroutable endpoint so the
|
||||
// failure callbacks fire under concurrency.
|
||||
//
|
||||
// With the Layer A drain-to-local fix in place, the after-vm-call drain
|
||||
// terminates over a snapshot, deferred callbacks complete, and the gateway keeps
|
||||
// serving requests. The test asserts the plugin route stays responsive (200)
|
||||
// rather than hanging — a spinning worker would fail this via timeout.
|
||||
var WasmPluginsRedisInjectSpin = suite.ConformanceTest{
|
||||
ShortName: "WasmPluginsRedisInjectSpin",
|
||||
Description: "Reproduce #4034: redis-failure callback + inject must not spin the worker; gateway stays responsive.",
|
||||
Manifests: []string{"tests/go-wasm-test-redis-inject-spin.yaml"},
|
||||
Features: []suite.SupportedFeature{suite.WASMGoConformanceFeature},
|
||||
Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
|
||||
testcases := []http.Assertion{
|
||||
{
|
||||
Meta: http.AssertionMeta{
|
||||
TargetBackend: "infra-backend-v1",
|
||||
TargetNamespace: "higress-conformance-infra",
|
||||
},
|
||||
Request: http.AssertionRequest{
|
||||
ActualRequest: http.Request{
|
||||
Host: "redis-inject-spin.com",
|
||||
Path: "/",
|
||||
UnfollowRedirect: true,
|
||||
},
|
||||
},
|
||||
Response: http.AssertionResponse{
|
||||
ExpectedResponse: http.Response{
|
||||
StatusCode: 200,
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
t.Run("WasmPlugins redis-inject-spin (#4034 CPU spin repro)", func(t *testing.T) {
|
||||
// Drive repeated requests: on the pre-fix host the deferred callback
|
||||
// re-queues forever and a worker spins, so the route stops
|
||||
// responding; on the fixed host every request completes.
|
||||
for i := 0; i < 20; i++ {
|
||||
for _, testcase := range testcases {
|
||||
http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, suite.GatewayAddress, testcase)
|
||||
}
|
||||
}
|
||||
})
|
||||
},
|
||||
}
|
||||
@@ -1,75 +0,0 @@
|
||||
# Copyright (c) 2026 Alibaba Group Holding Ltd.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
# A "dead" Redis: the Service (with manual Endpoints) makes Higress create the
|
||||
# envoy cluster so RedisInit succeeds and commands dispatch, but the endpoint IP
|
||||
# is unroutable (240.0.0.1, class-E) so every connection fails -> the plugin's
|
||||
# redis-failure callbacks fire, which is the #4034 trigger.
|
||||
apiVersion: v1
|
||||
kind: Service
|
||||
metadata:
|
||||
name: dead-redis
|
||||
namespace: higress-conformance-infra
|
||||
spec:
|
||||
ports:
|
||||
- name: redis
|
||||
port: 6379
|
||||
protocol: TCP
|
||||
targetPort: 6379
|
||||
---
|
||||
apiVersion: v1
|
||||
kind: Endpoints
|
||||
metadata:
|
||||
name: dead-redis
|
||||
namespace: higress-conformance-infra
|
||||
subsets:
|
||||
- addresses:
|
||||
- ip: 240.0.0.1
|
||||
ports:
|
||||
- name: redis
|
||||
port: 6379
|
||||
protocol: TCP
|
||||
---
|
||||
apiVersion: networking.k8s.io/v1
|
||||
kind: Ingress
|
||||
metadata:
|
||||
name: redis-inject-spin
|
||||
namespace: higress-conformance-infra
|
||||
spec:
|
||||
ingressClassName: higress
|
||||
rules:
|
||||
- host: "redis-inject-spin.com"
|
||||
http:
|
||||
paths:
|
||||
- pathType: Prefix
|
||||
path: "/"
|
||||
backend:
|
||||
service:
|
||||
name: infra-backend-v1
|
||||
port:
|
||||
number: 8080
|
||||
---
|
||||
apiVersion: extensions.higress.io/v1alpha1
|
||||
kind: WasmPlugin
|
||||
metadata:
|
||||
name: test-redis-inject-spin
|
||||
namespace: higress-system
|
||||
spec:
|
||||
defaultConfig:
|
||||
service_name: "dead-redis.higress-conformance-infra.svc.cluster.local"
|
||||
service_port: 6379
|
||||
redis_key: "higress-4034-key"
|
||||
inject_body: "injected-by-4034-repro\n"
|
||||
inject_count: 16
|
||||
url: file:///opt/plugins/wasm-go/extensions/test-redis-inject-spin/plugin.wasm
|
||||
Reference in New Issue
Block a user