Skip to content

e2e: cover egress on grpc requests and bidi streaming - #1119

Open
yufan-su wants to merge 1 commit into
agent-substrate:mainfrom
yufan-su:e2e-egress-grpc-test
Open

e2e: cover egress on grpc requests and bidi streaming#1119
yufan-su wants to merge 1 commit into
agent-substrate:mainfrom
yufan-su:e2e-egress-grpc-test

Conversation

@yufan-su

Copy link
Copy Markdown
Collaborator

Add an e2e test to cover egress on sending grpc requests and bidi streaming in it.

What's here

  • internal/proto/grpcechopb — a small Echo service with one method per streaming shape: unary Echo, server-streaming EchoStream, bidirectional EchoBidi. Generated files checked in, per the convention in the neighbouring proto packages.

  • internal/e2e/fixtures/grpcecho — the origin: a cleartext-HTTP/2 server, no TLS anywhere, with the standard health service so the pod can use a grpc readinessProbe and nothing between the actor and the origin parses HTTP. Pod + Service template, deployed per test into the suite's namespace.

  • demos/egress — the actor gains POST /grpc, which dials the target and runs whichever RPCs the request asks for. It dials per request and closes with it: the actor is checkpointed and restored, and an HTTP/2 connection opened before a snapshot does not survive one. The gRPC status comes back as a string rather than being flattened into the HTTP status, since that status is the trailer assertion.

  • internal/e2e/suites/networking/grpcegress_test.go — TestActorEgressGRPC drives all three shapes through nftables REDIRECT → atunnel → atenet-egress → origin, then asserts the gateway's access log recorded the CONNECT for that actor's certificate. Without that last check everything above would also pass on masqueraded traffic that never reached the gateway.

  • Tests pass

  • Appropriate changes to documentation are included in the PR

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grpcegress_test.go:161-239 duplicates sdsmint_test.go:352-437:

  • startGRPCEcho ≈ startProbe
  • waitForGRPCEchoReady ≈ waitForProbeReady
  • describeGRPCEchoState:226 is identical to describeProbeState apart from the loop variable name

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems mostly duplicate with internal/e2e/fixtures/egressprobe/egressprobe.yaml.tmpl. Can we reuse it somehow to reduce duplication?

@haiyanmeng

Copy link
Copy Markdown
Collaborator

yufan-su thanks for the work.

Can you take a look across this PR, #1054, and the existing test code, to figure out a solution minimizing the duplication in our test code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants