e2e: add A2A test server#1200
Draft
Aman-Cool wants to merge 10 commits into
Draft
Conversation
A2A v0.3.0 test agent serving an agent card and handling message/send, message/stream, tasks/get and tasks/cancel with SSE streaming. Part of Kuadrant#766 Signed-off-by: Aman-Cool <aman017102007@gmail.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new Go-based A2A test server, its container and Kubernetes manifests, and the local/CI image build wiring needed for the new test image. ChangesA2A test server rollout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…GENT_PREFIX) - drop the message/send + Accept streaming branch; streaming is message/stream only (§7.2) - remove now-dead wantsSSE - rename SKILL_PREFIX env -> AGENT_PREFIX (matches the agentPrefix rename in Kuadrant#1114) Signed-off-by: Aman-Cool <aman017102007@gmail.com>
- 'large'/'image' trigger returns a FilePart with a deterministic, size-configurable base64 payload (ARTIFACT_BYTES, default 1 MiB) - message/send: single large FilePart artifact (buffered rewrite path) - message/stream: chunked artifact-update events (append/lastChunk), split mid-base64 on purpose so a decoder chokes but a passthrough doesn't (proves envelope-only, no decode) - deterministic content so e2e can regenerate and assert byte-for-byte forwarding - adds fileContent/artifactUpdateEvent types; exercises the Task 12 SSE design (Kuadrant#1114) Signed-off-by: Aman-Cool <aman017102007@gmail.com>
raw ': ping' comment after the initial task event, so the gateway's data:-only SSE rewriter must pass non-data: lines through untouched without JSON-parsing them (regression guard for the Task 12 passthrough, Kuadrant#1114) Signed-off-by: Aman-Cool <aman017102007@gmail.com>
streaming method: reconnect replays the current task state over SSE and, for a working task, streams working updates then a terminal final event (bounded so it can't hang); an already-terminal task still gets an SSE final event, not a buffered response; unknown id -> -32001. completes the v0.3.0 method matrix (Kuadrant#1114) Signed-off-by: Aman-Cool <aman017102007@gmail.com>
- agentCard gains security/securitySchemes; AUTH_MODE declares them - enforcement, not just declaration: apikey requires X-API-Key (API_KEY) on the card fetch (tests credentialRef discovery) and /a2a; bearer requires Authorization: Bearer on /a2a (tests forward/token-exchange invocation); none (default) stays open - 401 + WWW-Authenticate when the credential is missing - lets e2e validate the gateway's credentialRef + token-exchange auth brokering (Kuadrant#1114) Signed-off-by: Aman-Cool <aman017102007@gmail.com>
CLAUDE.md entry had drifted: rename SKILL_PREFIX -> AGENT_PREFIX (the dead name), add tasks/resubscribe, the large/image FilePart trigger, and the ARTIFACT_BYTES/ AUTH_MODE/API_KEY env vars Signed-off-by: Aman-Cool <aman017102007@gmail.com>
the in-memory tasks map was append-only; under sustained load (stress-testing the gateway with the heavy-payload fixture) it grew without limit. a background sweep drops terminal tasks older than TASK_TTL_MS (default 10m); in-flight tasks are never removed, and the TTL is generous so e2e tasks aren't reaped mid-assertion Signed-off-by: Aman-Cool <aman017102007@gmail.com>
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
added
high-risk
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tests/servers/a2a-server/main.go (1)
786-796: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
ReadTimeoutis intentionally omittable here, but worth a one-line note. Static analysis flags the missingReadTimeout(Slowloris).ReadHeaderTimeoutalready bounds header reads, and addingWriteTimeoutwould break the long-lived SSE responses — so leaving those off is reasonable for a test server. A brief comment recording that rationale would stop the next reader (or a future linter fix) from naively addingWriteTimeoutand killing streaming.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/servers/a2a-server/main.go` around lines 786 - 796, Add a brief inline comment near the http.Server setup in main() explaining that ReadTimeout is intentionally omitted for this test server because ReadHeaderTimeout already limits header reads and adding WriteTimeout would break long-lived SSE streaming responses. Keep the rationale close to the httpServer initialization so future changes to ListenAndServe behavior are less likely to reintroduce the linter-driven fix.Source: Linters/SAST tools
tests/servers/a2a-server/Dockerfile (1)
2-16: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a runtime stage instead of shipping the Go toolchain.
This image currently runs from
golang:1.25-alpine, so every push/pull/bake includes the full toolchain. With.github/workflows/test-images.yamlpublishing it andbuild/ci-node.mkbaking it into the CI node image, that cost lands on every e2e path. Copy the binary into a small runtime image and drop root there.Proposed change
# Build stage FROM golang:1.25-alpine AS builder WORKDIR /app @@ RUN CGO_ENABLED=0 GOOS=linux go build -o /a2a-test-server -CMD ["/a2a-test-server"] +FROM alpine:3.22 + +RUN addgroup -S app && adduser -S -G app app + +COPY --from=builder /a2a-test-server /a2a-test-server + +USER app + +CMD ["/a2a-test-server"]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/servers/a2a-server/Dockerfile` around lines 2 - 16, The Dockerfile for the A2A test server is using the Go builder image as the final runtime image, so it ships the full toolchain instead of just the binary. Split the build into a builder stage and a separate runtime stage in the same Dockerfile, then copy the built /a2a-test-server binary into a small non-Go base image. Keep the build steps in the existing builder section and move the final CMD to the runtime stage, ensuring the container runs as a non-root user.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/test-servers/a2a-server-deployment.yaml`:
- Around line 29-30: The AGENT_URL value is exposing an internal cluster-local
address in the Agent Card, which breaks clients outside the cluster and bypasses
the gateway route. Update the A2A server deployment so the Agent Card url comes
from a routed hostname or the incoming Host header instead of the hardcoded
a2a-test-server.mcp-test.svc.cluster.local address, and keep the URL aligned
with the gateway path used by the A2A server setup.
- Around line 16-37: The a2a-test-server deployment is still using the default
root-capable security settings, so harden the pod and container security context
directly in this manifest. Update the a2a-test-server container spec to run as
non-root and add restrictive container settings such as disallowing privilege
escalation and dropping unnecessary capabilities, and add a pod-level security
context if needed for filesystem/runAs behavior. Use the existing spec for the
a2a-test-server container as the target location so the deployment no longer
depends on cluster defaults.
In `@config/test-servers/CLAUDE.md`:
- Line 16: The auth description for the A2A Server is too broad and implies both
auth modes protect the agent card fetch. Update the wording in CLAUDE.md to
reflect the actual behavior in main.go: card-fetch auth is enforced only in
apikey mode, while bearer mode applies to /a2a only. Keep the references to
/.well-known/agent-card.json and /a2a clear so tests do not assume a Bearer
token is required for the card endpoint.
In `@tests/servers/a2a-server/main.go`:
- Around line 445-454: The task insertion order in the request handling path is
wrong: `completeLater` is started before `s.tasks` contains the new task, so
`completeLater` can look up the task too early and exit without completing it.
In the branch that calls `s.completeLater` for slow tasks, move the
`s.tasks[t.ID] = t` insertion (under `s.mu`) to happen before spawning the
goroutine, and keep the map update and lookup flow consistent with
`completeLater` and `sweep`.
- Around line 725-732: The task result is being marshaled after releasing s.mu,
so writeRPC can race with background mutations of the same Task fields. In the
affected handlers (including the task lookup path here, plus handleSend,
handleCancel, and the initial send in handleStream), take a snapshot of the task
while holding s.mu and marshal/write that snapshot after unlocking. Use the
existing task access points in s.tasks, t.Status, and t.Artifacts to locate the
spots and apply the same fix consistently.
---
Nitpick comments:
In `@tests/servers/a2a-server/Dockerfile`:
- Around line 2-16: The Dockerfile for the A2A test server is using the Go
builder image as the final runtime image, so it ships the full toolchain instead
of just the binary. Split the build into a builder stage and a separate runtime
stage in the same Dockerfile, then copy the built /a2a-test-server binary into a
small non-Go base image. Keep the build steps in the existing builder section
and move the final CMD to the runtime stage, ensuring the container runs as a
non-root user.
In `@tests/servers/a2a-server/main.go`:
- Around line 786-796: Add a brief inline comment near the http.Server setup in
main() explaining that ReadTimeout is intentionally omitted for this test server
because ReadHeaderTimeout already limits header reads and adding WriteTimeout
would break long-lived SSE streaming responses. Keep the rationale close to the
httpServer initialization so future changes to ListenAndServe behavior are less
likely to reintroduce the linter-driven fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6023c3d4-3568-4701-a14d-7335601391e4
📒 Files selected for processing (12)
.github/workflows/test-images.yamlMakefilebuild/ci-node.mkconfig/test-servers/CLAUDE.mdconfig/test-servers/a2a-server-deployment.yamlconfig/test-servers/a2a-server-httproute-ext.yamlconfig/test-servers/a2a-server-httproute.yamlconfig/test-servers/a2a-server-service.yamlconfig/test-servers/kustomization.yamltests/servers/a2a-server/Dockerfiletests/servers/a2a-server/go.modtests/servers/a2a-server/main.go
- snapshot the task under the lock before encoding it; handlers were marshalling the *task after unlocking, racing completeLater's mutation of Status/Artifacts (go build -race + concurrent tasks/get now clean, was a confirmed DATA RACE) - tasks/resubscribe now observes the task to its terminal state instead of force-completing it, so it no longer suppresses the echo/request-info artifacts completeLater attaches Signed-off-by: Aman-Cool <aman017102007@gmail.com>
- harden the deployment security context (runAsNonRoot, drop ALL caps, read-only rootfs, no privilege escalation), mirroring tls-server-deployment - insert the task into the map before spawning completeLater, so the goroutine's lookup can't race ahead of the insert and pin the task in working forever - correct the CLAUDE.md auth note: bearer enforces on /a2a only, not the card fetch Signed-off-by: Aman-Cool <aman017102007@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of: #766
Adds the A2A test server the e2e suite routes through.., the upstream the gateway talks to once A2A support lands (the design in #1114). No gateway code here, just the test fixture and its manifests.
It's a small hand-rolled Go server that implements enough of A2A v0.3.0 to exercise the real path; it serves an Agent Card at the v0.3.0
/.well-known/agent-card.json(with the legacyagent.jsonalias), and handlesmessage/send,message/stream(SSE),tasks/get,tasks/cancelandtasks/resubscribeas JSON-RPC 2.0 over HTTP, including multi-turn continuation viamessage.taskIdand the-32601/-32602/-32001error codes from the spec.tasks/resubscribereconnects to a task over SSE.., a working task streams updates through to a terminal event, a finished one still gets a final SSE event rather than a buffered response (it's a streaming method, so the response has to stay an SSE one).The behavior is deterministic so a test can drive task state straight from the message text;
"slow"starts inworkingand completes afterTASK_DURATION_MS(gives streaming and polling something to watch),"fail"returns a failed task,"large"(or"image") attaches a heavy multi-modal artifact, anything else completes immediately. Every task also carries anechoartifact with the message text and arequest-infoartifact with the headers the server actually received; that second one is the useful bit, it's what lets the gateway-side tests assert the task ID got rewritten, thex-a2a-*headers were stripped, and the right auth was forwarded.., real assertions instead of mocks.The
"large"/"image"trigger is there for the multi-modal passthrough path; it returns aFilePartwith a deterministic, size-configurable base64 payload (ARTIFACT_BYTES, default 1 MiB); onmessage/sendas one big artifact, onmessage/streamas chunked artifact-update events split mid-base64 on purpose, so a gateway that tries to decode it chokes but one that forwards the raw bytes doesn't. The stream also emits a raw: pingSSE keepalive comment, so the gateway'sdata:-only rewriter has to pass non-data:lines through untouched without parsing them.It's auth-aware too, via
AUTH_MODE;"apikey"makes the card declare anapiKeyscheme and requiresX-API-Keyon both the card fetch (socredentialRefdiscovery is actually testable) and/a2a,"bearer"declares anoauth2scheme and requiresAuthorization: Beareron/a2a(for the forward / token-exchange path),"none"(default) leaves it open. The card'ssecuritySchemes/securityreflect whichever mode is set.., it enforces, rather than just declaring, so the gateway's auth brokering can't pass untested.Configurable via env (
AGENT_NAME,SKILLS,AGENT_PREFIX,AGENT_URL,TASK_DURATION_MS,ARTIFACT_BYTES,AUTH_MODE…), so two instances with different skills and prefixes give the "no cross-routing between agents" case for free. Ships with the deployment/service/HTTPRoute manifests underconfig/test-servers/and the CI image wiring (test-a2a-server). Hand-rolled rather than pullinga2aproject/a2a-gosince that SDK only speaks v1.0, and the tests need controlled edge cases (SSE split mid-JSON, deterministic timing, header echo) the SDK would abstract away; follows the existingcustom-response-serverpattern.No e2e specs wired against it yet.., those land with the router/broker work and basically write themselves against this. Happy to tweak the canned behavior, artifact shape or auth knobs if it'd make the assertions cleaner… a quick sanity check on the agent-card env knobs before I build tests around them would be helpful.
Summary by CodeRabbit