Event Gateway Integration test module#2019
Conversation
|
Warning Review limit reached
More reviews will be available in 50 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis pull request adds a BDD-driven integration test suite for Event Gateway: feature specs (health, WebSub API management, WebSub E2E), Godog step definitions, test state and orchestration helpers (ComposeManager), a webhook-listener admin endpoint to observe deliveries, Makefile targets and go.mod for the test module, image tagging updates to :latest, and a GitHub Actions workflow to build images and run the tests. Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant TestRunner as Godog Test Suite
participant ComposeMgr as ComposeManager
participant GatewayCtrl as Gateway Controller
participant EventGateway as Event Gateway Runtime
participant Webhook as Webhook Listener
CI->>TestRunner: invoke make -C event-gateway/it test
TestRunner->>ComposeMgr: start docker-compose stack
ComposeMgr->>GatewayCtrl: bring up controller
ComposeMgr->>EventGateway: bring up runtime
TestRunner->>GatewayCtrl: create/update/delete WebSub APIs
TestRunner->>GatewayCtrl: publish/subscribe actions
GatewayCtrl->>EventGateway: enforce/control runtime delivery
EventGateway->>Webhook: POST delivered events
TestRunner->>Webhook: GET /received-events to verify delivery
TestRunner->>ComposeMgr: dump logs and teardown
CI->>CI: upload logs artifact
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Dependency Validation ResultsDependency name: github.com/cucumber/godog Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Next Steps
|
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
event-gateway/it/state.go (1)
97-99: ⚡ Quick winAdd comment explaining TLS certificate verification bypass.
The TLS configuration disables certificate verification, which is flagged by static analysis. While this is acceptable for integration tests and marked with
nolint:gosec, adding a comment explaining why this is necessary would improve code clarity.📝 Suggested improvement
func NewTestState(cfg *Config) *TestState { + // Skip TLS verification in test environment to avoid certificate issues + // when connecting to locally-hosted services via docker-compose. transport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec }🤖 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 `@event-gateway/it/state.go` around lines 97 - 99, Add a brief explanatory comment above the transport/TLS config (the http.Transport with TLSClientConfig: &tls.Config{InsecureSkipVerify: true}) stating that certificate verification is intentionally disabled only for integration tests (e.g., self-signed or test certificates), that this is why the gosec lint is suppressed (//nolint:gosec), and that this must not be used in production; keep the comment short and next to the TLSClientConfig initialization for clarity.
🤖 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 @.github/workflows/event-gateway-integration-test.yml:
- Line 42: Replace version-tagged GitHub Actions references with immutable
commit SHAs to improve supply chain security; specifically update each "uses:
actions/checkout@v4" (and the other uses lines flagged at 45, 51, 70) to
reference the corresponding action repository commit SHA (e.g.,
actions/checkout@<commit-sha>) by looking up the desired release commit in the
action's GitHub repo and substituting the tag with that commit SHA.
- Around line 41-42: The checkout step using actions/checkout@v4 currently
leaves credentials persisted; update the "Checkout code" step (uses:
actions/checkout@v4) to add persist-credentials: false under its step inputs so
that GitHub token credentials are not written to the workspace or artifacts,
ensuring the checkout action is invoked with the persist-credentials option set
to false.
- Around line 53-61: The workflow reads the VERSION into the step with id
"version" and expands it directly into make commands via
steps.version.outputs.version; add a validation step after "Read version" that
checks the VERSION string against the expected semver pattern (e.g.,
major.minor.patch with optional prerelease/build) and fails the job if it
doesn't match, and ensure the value is safely quoted/escaped when passed into
the make commands; reference the "version" step id and the two run lines that
invoke make -C event-gateway build-event-gateway-controller VERSION=${{
steps.version.outputs.version }} and make -C event-gateway build-gateway-runtime
VERSION=${{ steps.version.outputs.version }} to locate where to wire the
validation and quoting.
In `@event-gateway/it/features/websub-e2e.feature`:
- Around line 127-149: The scenario title "Scenario: Publish an event to a known
topic returns 200" is inconsistent with the step asserting "Then the response
status code should be 202"; update the scenario title to "returns 202" to match
the assertion (or alternatively change the assertion to 200 if the intended
behavior is HTTP 200), by editing the scenario header text in the feature file
so the title and the step "Then the response status code should be 202" are
aligned.
In `@event-gateway/it/go.mod`:
- Line 210: Update the pinned dependency google.golang.org/grpc from v1.74.2 to
at least v1.79.3 in the repository's go.mod (replace the version string for
google.golang.org/grpc with v1.79.3 or a newer stable release) and then refresh
module metadata (i.e., update go.sum / run module tidy) so the patched version
is used.
In `@event-gateway/it/setup.go`:
- Around line 137-142: The test startup is polling the gate's /health endpoint
only; update the check to wait on readiness instead of liveness by changing the
endpoint string(s) in the services list from
fmt.Sprintf("http://localhost:%s/health", EventGatewayAdminPort) to
fmt.Sprintf("http://localhost:%s/ready", EventGatewayAdminPort) (and the other
occurrences referenced in the comment), or modify the WaitForHealthy helper to
poll both /health and /ready (or prefer /ready) so the test only proceeds when
the runtime is fully ready; locate the services variable and calls to
WaitForHealthy in setup.go and update their endpoints or logic accordingly.
- Around line 68-70: The current os.Stat call only checks for IsNotExist and
returns a "compose file not found" error, letting other filesystem errors (e.g.,
permission denied) fall through; update the check around os.Stat(absPath) so
that if err != nil you inspect errors.Is(err, fs.ErrNotExist) or
os.IsNotExist(err) to return the "compose file not found: %s" message, but for
any other non-nil err return a wrapped/error-with-context describing the failure
to stat absPath (e.g., "failed to stat compose file: <absPath>: <err>");
reference the os.Stat call, absPath variable, and the existing fmt.Errorf usage
when making the change.
In `@event-gateway/it/state.go`:
- Around line 66-68: Update the top-of-function comment for DefaultConfig
(returning *Config) to reference the correct compose file name: replace
"docker-compose.test.yaml" with "docker-compose.dev.yaml" so the comment
accurately reflects the expected docker-compose mapping used by the test config.
In `@event-gateway/it/steps.go`:
- Around line 254-257: The code reads HTTP response bodies with io.ReadAll but
discards the error; change the reads in the step helper(s) (the blocks that set
state.lastResponse and state.lastBody) to capture the returned error (e.g.,
bodyBytes, err := io.ReadAll(resp.Body)), check if err != nil and return or wrap
that error instead of ignoring it; apply the same fix to the other occurrences
referenced (the similar read at the other two blocks) so protocol step failures
surface the io.ReadAll error via the step function's returned error.
- Around line 325-334: The step registered with ctx.Step(`^the webhook listener
should have received the event "([^"]*)"$`) only checks publish acceptance via
state.lastResponse.StatusCode and does not verify actual delivery; update this
step to either (preferred) poll the webhook listener's observable (e.g., call
the wh-listener admin/logs endpoint or a test-only /received-events API) and
assert the payload appears within a timeout, or (if no listener API exists)
rename the step to reflect that it only verifies publish acceptance. Implement
the observable check by adding a helper (e.g., checkListenerReceivedEvent or
pollWebhookListenerLogs) that queries the listener, retries until found or
timeout, and use it inside this ctx.Step, falling back to the existing 2xx check
only when the listener API is unavailable.
---
Nitpick comments:
In `@event-gateway/it/state.go`:
- Around line 97-99: Add a brief explanatory comment above the transport/TLS
config (the http.Transport with TLSClientConfig: &tls.Config{InsecureSkipVerify:
true}) stating that certificate verification is intentionally disabled only for
integration tests (e.g., self-signed or test certificates), that this is why the
gosec lint is suppressed (//nolint:gosec), and that this must not be used in
production; keep the comment short and next to the TLSClientConfig
initialization for clarity.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4562d073-a2ba-4225-8317-f27489a5c051
⛔ Files ignored due to path filters (2)
event-gateway/it/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.work
📒 Files selected for processing (14)
.github/workflows/event-gateway-integration-test.ymlevent-gateway/Makefileevent-gateway/docker-compose.dev.yamlevent-gateway/gateway-runtime/Makefileevent-gateway/it/Makefileevent-gateway/it/features/health.featureevent-gateway/it/features/websub-api-management.featureevent-gateway/it/features/websub-e2e.featureevent-gateway/it/go.modevent-gateway/it/logs/compose-logs.txtevent-gateway/it/setup.goevent-gateway/it/state.goevent-gateway/it/steps.goevent-gateway/it/suite_test.go
f3155e3 to
ad5da7d
Compare
Dependency Validation ResultsDependency name: github.com/cucumber/godog Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Next Steps
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
event-gateway/it/steps.go (1)
336-347:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid silently downgrading delivery verification to publish acceptance.
The current fallback allows this step to pass without confirming listener-side delivery when the first listener check is unavailable. That can hide delivery regressions and make E2E results less reliable.
Suggested change
ctx.Step(`^the webhook listener should have received the event "([^"]*)"$`, func(payload string) error { err := checkListenerReceivedEvent(state, payload, 10*time.Second) if errors.Is(err, errListenerUnavailable) { - // /received-events endpoint not present; fall back to verifying publish acceptance. - if state.lastResponse == nil { - return fmt.Errorf("no response from publish step") - } - if state.lastResponse.StatusCode < 200 || state.lastResponse.StatusCode >= 300 { - return fmt.Errorf("event publish was not accepted: HTTP %d", state.lastResponse.StatusCode) - } - return nil + return fmt.Errorf("cannot verify listener delivery: %w", err) } return err })Also applies to: 365-375
🤖 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 `@event-gateway/it/steps.go` around lines 336 - 347, The test currently treats errListenerUnavailable from checkListenerReceivedEvent as a silent fallback to only validating publish HTTP acceptance (using state.lastResponse), which hides delivery failures; change the handler for errListenerUnavailable in the block handling checkListenerReceivedEvent so that it returns a descriptive error instead of treating HTTP 2xx as success (or at minimum fails the test when state.lastResponse is nil or non-2xx), referencing checkListenerReceivedEvent, errListenerUnavailable and state.lastResponse to locate the code; apply the same change to the duplicate logic around the other occurrence (the 365-375 block) so listener-side delivery unavailability does not downgrade verification to publish acceptance..github/workflows/event-gateway-integration-test.yml (1)
54-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate version format before template expansion.
The VERSION file content is expanded directly into shell commands without validation. Validate that the version follows the expected format to prevent unintended interpretation.
🛡️ Proposed fix to validate version format
- name: Read version id: version - run: echo "version=$(cat event-gateway/VERSION)" >> $GITHUB_OUTPUT + run: | + VERSION=$(cat event-gateway/VERSION) + if [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9]+)?$ ]]; then + echo "Invalid version format: $VERSION" + exit 1 + fi + echo "version=$VERSION" >> $GITHUB_OUTPUT🤖 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 @.github/workflows/event-gateway-integration-test.yml around lines 54 - 62, The workflow reads VERSION and injects it into shell commands without validation, so add a validation step right after the "Read version" step to assert the format (e.g. a semantic version pattern) and fail early if it doesn't match; reference the "Read version" step that sets steps.version.outputs.version and ensure the subsequent "Build event-gateway-controller image" and "Build event-gateway-runtime image" steps only use the validated value (and use a quoted/sanitized expansion) so malformed or malicious VERSION contents cannot be expanded into the shell.
🤖 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 `@event-gateway/it/steps.go`:
- Around line 379-380: The JSON decoder error from
json.NewDecoder(resp.Body).Decode(&bodies) is being ignored; change the listener
polling logic to capture and return that error instead of discarding it.
Specifically, replace the blank assignment for Decode on resp.Body with error
handling that closes resp.Body and returns the decode error (propagating it up
from the polling function), so callers see JSON parse failures for bodies rather
than silent timeouts; ensure resp.Body.Close() still runs in all paths.
---
Duplicate comments:
In @.github/workflows/event-gateway-integration-test.yml:
- Around line 54-62: The workflow reads VERSION and injects it into shell
commands without validation, so add a validation step right after the "Read
version" step to assert the format (e.g. a semantic version pattern) and fail
early if it doesn't match; reference the "Read version" step that sets
steps.version.outputs.version and ensure the subsequent "Build
event-gateway-controller image" and "Build event-gateway-runtime image" steps
only use the validated value (and use a quoted/sanitized expansion) so malformed
or malicious VERSION contents cannot be expanded into the shell.
In `@event-gateway/it/steps.go`:
- Around line 336-347: The test currently treats errListenerUnavailable from
checkListenerReceivedEvent as a silent fallback to only validating publish HTTP
acceptance (using state.lastResponse), which hides delivery failures; change the
handler for errListenerUnavailable in the block handling
checkListenerReceivedEvent so that it returns a descriptive error instead of
treating HTTP 2xx as success (or at minimum fails the test when
state.lastResponse is nil or non-2xx), referencing checkListenerReceivedEvent,
errListenerUnavailable and state.lastResponse to locate the code; apply the same
change to the duplicate logic around the other occurrence (the 365-375 block) so
listener-side delivery unavailability does not downgrade verification to publish
acceptance.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1244d3e0-ea0e-4816-bed2-d89b4f5880ee
⛔ Files ignored due to path filters (1)
event-gateway/it/go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
.github/workflows/event-gateway-integration-test.ymlevent-gateway/Makefileevent-gateway/docker-compose.dev.yamlevent-gateway/gateway-runtime/Makefileevent-gateway/it/features/websub-e2e.featureevent-gateway/it/go.modevent-gateway/it/setup.goevent-gateway/it/state.goevent-gateway/it/steps.goevent-gateway/webhook-listener/main.go
ad5da7d to
a598122
Compare
Dependency Validation ResultsDependency name: github.com/cucumber/godog Dependency name: github.com/testcontainers/testcontainers-go/modules/compose Next Steps
|
Event Gateway Integration test module