abstracting event-gateway from gateway-controller#2623
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (64)
📝 WalkthroughWalkthroughThis PR adds a standalone Event Gateway Controller with WebSub and WebBroker management APIs, deployment and policy integrations, WebSubHub xDS support, webhook-secret handling, Docker builds, and extension hooks that move Event Gateway behavior out of the core gateway controller. ChangesEvent Gateway Controller
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
ed5d020 to
0266a36
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
event-gateway/gateway-controller/pkg/handler/webbroker_api_handler.go (1)
96-118: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the documented WebBroker list filters.
The generated API accepts
displayName,version, andstatus, but this handler always returns every configuration.Proposed fix
items := make([]any, 0, len(configs)) for _, cfg := range configs { + if params.DisplayName != nil && cfg.DisplayName != *params.DisplayName { + continue + } + if params.Version != nil && cfg.Version != *params.Version { + continue + } + if params.Status != nil && cfg.DesiredState != string(*params.Status) { + continue + } items = append(items, buildResourceResponseFromStored(cfg.SourceConfiguration, cfg)) }🤖 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/gateway-controller/pkg/handler/webbroker_api_handler.go` around lines 96 - 118, Update ListWebBrokerApis to apply the documented params.DisplayName, params.Version, and params.Status filters before building the items response. Match each configuration against only the provided query parameters, preserve all configurations when filters are absent, and ensure count reflects the filtered results.
🟠 Major comments (21)
event-gateway/gateway-controller/pkg/kindsupport/kindsupport.go-116-154 (1)
116-154: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate topic-name derivation logic between
topicsForWebSubUpdateandUpdateTopicManager.Both functions independently re-implement the exact same "context+version+channel → topic name" mangling. Extracting a shared helper avoids the two falling out of sync if the naming rule ever changes.
♻️ Proposed refactor
+func webSubTopicSet(asyncData eventgateway.WebSubAPISpec) map[string]bool { + topics := make(map[string]bool) + var channels map[string]eventgateway.WebSubChannel + if asyncData.Channels != nil { + channels = *asyncData.Channels + } + for chName := range channels { + contextWithVersion := strings.ReplaceAll(asyncData.Context, "$version", asyncData.Version) + contextWithVersion = strings.TrimPrefix(contextWithVersion, "/") + contextWithVersion = strings.ReplaceAll(contextWithVersion, "/", "_") + name := strings.TrimPrefix(chName, "/") + topics[fmt.Sprintf("%s_%s", contextWithVersion, name)] = true + } + return topics +}Then both
topicsForWebSubUpdateandUpdateTopicManagercan callwebSubTopicSet(asyncData)instead of duplicating the loop.Also applies to: 159-185
🤖 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/gateway-controller/pkg/kindsupport/kindsupport.go` around lines 116 - 154, Extract the shared WebSub topic-name derivation from topicsForWebSubUpdate and UpdateTopicManager into a helper such as webSubTopicSet(asyncData). Have both functions use this helper for the context/version/channel mangling and topic-set construction, while preserving their existing registration and unregistration behavior.event-gateway/gateway-controller/pkg/translator/hooks.go-337-343 (1)
337-343: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd
dns_cache_configto the dynamic forward proxy cluster
The DFP cluster and HTTP filter must use the same DNS cache settings;createDynamicForwardProxyClustercurrently builds an emptydfpcluster.ClusterConfig{}, so the WebSubHub dynamic forward proxy path won’t pair correctly with the filter’s cache config. Consider sharing onednsCacheConfighelper between both call sites to keep them in sync.🤖 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/gateway-controller/pkg/translator/hooks.go` around lines 337 - 343, Update createDynamicForwardProxyCluster to populate dfpcluster.ClusterConfig with the same dnsCacheConfig used by the dynamic forward proxy HTTP filter. Extract or reuse a shared dnsCacheConfig helper so both call sites consistently use identical DNS cache settings.event-gateway/gateway-controller/pkg/kindsupport/kindsupport.go-198-260 (1)
198-260: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClone
Spec.Vhostsbefore resolving sentinels.
unmarshalWebSubAPI/unmarshalWebBrokerApiassign the same decoded value to bothSourceConfigurationandConfiguration, soSpec.Vhostsaliases the stored source config. Mutatingc.Spec.VhostsinresolveWebSubVhostSentinels/resolveWebBrokerVhostSentinelsoverwrites the pristine input with resolved defaults. CopyVhostsfirst, then mutate the copy.🤖 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/gateway-controller/pkg/kindsupport/kindsupport.go` around lines 198 - 260, In resolveWebSubVhostSentinels and resolveWebBrokerVhostSentinels, clone c.Spec.Vhosts before applying any default or sentinel resolution, including the nil-initialization path, so mutations affect only the returned configuration. Preserve the existing source Vhosts value and assign the cloned structure back to c.Spec.Vhosts before modifying its Main or Sandbox fields.gateway/gateway-controller/pkg/xds/translator.go-838-849 (1)
838-849: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the event-gateway feature gate
event_gateway.enabledno longer suppresses hub-resource translation:BuildHubResourcesis now driven only byt.eventGatewayHooks != nil, and neither hook checksEnabled. A config withenabled = falsewill still generate WebSubHub cluster/listener resources when event-gateway support is compiled in.🤖 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 `@gateway/gateway-controller/pkg/xds/translator.go` around lines 838 - 849, Restore the event_gateway.enabled gate around the hub-resource translation in the enclosing translator flow, so BuildHubResources is invoked only when event-gateway hooks are present and the feature is enabled. Use the existing event-gateway configuration’s Enabled value, while preserving the current error handling and resource appends.event-gateway/gateway-controller/pkg/hubtopic/deregistrar.go-68-87 (1)
68-87: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound concurrent hub deregistration calls.
A deletion starts one goroutine and HTTP request per topic. Large configurations can exhaust sockets/goroutines or overload WebSubHub despite the per-request timeout. Use a configured worker limit or semaphore.
🤖 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/gateway-controller/pkg/hubtopic/deregistrar.go` around lines 68 - 87, Bound concurrent topic deregistration in the loop surrounding UnregisterTopicWithHub by introducing a configured worker limit or semaphore. Ensure each goroutine acquires a slot before issuing the request and releases it on completion, while preserving the existing timeout, logging, error counting, and childWg synchronization behavior.event-gateway/gateway-controller/pkg/handler/helpers.go-44-63 (1)
44-63: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReturn sterile rendering errors to clients.
SecretError.Error(),TemplateParseError.Error(), andRenderError.Causecan expose secret identifiers or internal template details. Keep the complete error in server logs and return fixed messages.Proposed fix
- var secretErr *funcs.SecretError - if errors.As(renderErr, &secretErr) { - httputil.WriteJSON(w, http.StatusBadRequest, eventgateway.ErrorResponse{ - Status: "error", - Message: secretErr.Error(), - }) - return true - } - var tmplParseErr *templateengine.TemplateParseError - if errors.As(renderErr, &tmplParseErr) { - httputil.WriteJSON(w, http.StatusBadRequest, eventgateway.ErrorResponse{ - Status: "error", - Message: tmplParseErr.Error(), - }) - return true - } httputil.WriteJSON(w, http.StatusBadRequest, eventgateway.ErrorResponse{ Status: "error", - Message: fmt.Sprintf("Failed to render configuration: %v", renderErr.Cause), + Message: "Failed to render configuration", })As per coding guidelines, internal errors must be sanitized before marshaling into client-facing responses.
🤖 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/gateway-controller/pkg/handler/helpers.go` around lines 44 - 63, Sanitize client-facing responses in the render-error handling logic by replacing secretErr.Error(), tmplParseErr.Error(), and renderErr.Cause with fixed, non-sensitive messages. Preserve logging of the complete render error separately, and keep the existing status and response structure in the surrounding handler unchanged.Source: Coding guidelines
event-gateway/gateway-controller/cmd/controller/runtime_bootstrap.go-132-154 (1)
132-154: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClone configs before startup rendering
ConfigStore.GetAll()returns the stored*models.StoredConfigpointers, andtemplateengine.RenderSpecmutatescfg.Configurationin place. RenderingapiConfighere will overwrite the shared in-memory copy with resolved secrets; render a copy instead.🤖 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/gateway-controller/cmd/controller/runtime_bootstrap.go` around lines 132 - 154, Clone each config returned by ConfigStore.GetAll() before passing it to templateengine.RenderSpec, and render the cloned StoredConfig rather than the shared apiConfig pointer. Keep the existing error handling and subsequent transformation flow intact, while ensuring resolved secrets never mutate the stored in-memory configuration.Source: Learnings
event-gateway/gateway-controller/api/eventgateway-openapi.yaml-10-16 (1)
10-16: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not advertise Basic authentication over cleartext HTTP.
Both server URLs use
http://while the global security scheme is Basic authentication, exposing reusable credentials to interception. Publish HTTPS endpoints and ensure the controller serves TLS or is reachable only through enforced TLS termination.Also applies to: 1149-1153
🤖 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/gateway-controller/api/eventgateway-openapi.yaml` around lines 10 - 16, Update the OpenAPI server URLs in the event-gateway specification to use HTTPS instead of HTTP for both local development and Docker/Kubernetes deployment, while ensuring the controller or its ingress enforces TLS termination so Basic authentication is never sent over cleartext.Source: Linters/SAST tools
event-gateway/gateway-controller/cmd/controller/main.go-627-631 (1)
627-631: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound full request, response, and idle lifetimes.
ReadHeaderTimeoutdoes not constrain slow request bodies or indefinitely blocked writes. ConfigureReadTimeout,WriteTimeout, andIdleTimeoutto prevent connection-exhaustion attacks.🤖 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/gateway-controller/cmd/controller/main.go` around lines 627 - 631, Update the http.Server initialization in the controller startup flow to configure ReadTimeout, WriteTimeout, and IdleTimeout alongside the existing ReadHeaderTimeout. Use appropriate bounded durations for each so request bodies, responses, and keep-alive connections cannot remain indefinitely active.Source: Linters/SAST tools
event-gateway/gateway-controller/cmd/controller/main.go-276-326 (1)
276-326: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not register secret endpoints with a nil webhook-secret service.
With no encryption providers,
webhookSecretServiceremains nil, but the WebSub handler and secret routes are still installed. Those handlers dereference the service, causing every secret operation to fail or panic. Require encryption at startup when these APIs are enabled, or explicitly return a sterile service-unavailable response.🤖 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/gateway-controller/cmd/controller/main.go` around lines 276 - 326, Ensure the WebSub handler and webhook-secret routes are not registered when webhookSecretService is nil; either require at least one encryption provider during startup or make those endpoints return a service-unavailable response without dereferencing the nil service. Update the route/handler registration flow alongside the existing webhookSecretService initialization, preserving normal behavior when encryption is configured.event-gateway/gateway-controller/cmd/controller/main.go-583-591 (1)
583-591: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftInstall a configurable request-body size limit.
The generated handlers receive the network request body without a global ceiling. Wrap requests with
http.MaxBytesReaderor an equivalent middleware using a configuration-backed safe default, and return HTTP 413 when exceeded.As per coding guidelines, every network-originated reader must be bounded using a configurable limit and oversized requests must receive a generic HTTP 413 response.
🤖 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/gateway-controller/cmd/controller/main.go` around lines 583 - 591, Update the outer middleware chain assembled in main around outerMiddlewares and httpHandler to apply a configurable request-body size limit with a safe default to every incoming request, using http.MaxBytesReader or equivalent. Ensure requests exceeding the limit return a generic HTTP 413 response, while preserving the existing correlation, error-handling, logging, metrics, and mux routing behavior.Source: Coding guidelines
gateway/gateway-controller/pkg/storage/memory.go-99-103 (1)
99-103: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
AddandUpdateatomic when the topic hook fails.Both methods mutate indexes before calling the fallible hook. An error therefore leaves
Addwith an inserted configuration andUpdatewith modified indexes pointing at the old configuration. Validate and invoke the hook before committing mutations, or roll back every changed map and topic entry.Example ordering for Add
+ if cfg.Kind == "WebSubApi" && cs.webSubTopicUpdater != nil { + if err := cs.webSubTopicUpdater(cfg, cs.TopicManager); err != nil { + return err + } + } + cs.configs[cfg.UUID] = cfg cs.handle[handleKey] = cfg.UUID cs.nameVersion[key] = cfg.UUID - - if cfg.Kind == "WebSubApi" && cs.webSubTopicUpdater != nil { - err := cs.webSubTopicUpdater(cfg, cs.TopicManager) - if err != nil { - return err - } - }Apply the same validate-then-commit ordering to
Update.Also applies to: 148-152
🤖 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 `@gateway/gateway-controller/pkg/storage/memory.go` around lines 99 - 103, Make Add and Update atomic by validating and invoking cs.webSubTopicUpdater before committing any configuration, index, or topic mutations. If the hook returns an error, leave all existing maps and topic entries unchanged; otherwise apply the complete mutation. Update both Add and Update flows consistently, or roll back every mutation on failure.gateway/gateway-controller/pkg/service/restapi/service.go-423-427 (1)
423-427: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not abort replica convergence after the database deletion has committed.
A deregistration error returns before
publishEvent, although the configuration is already deleted. The caller receives an error while local and replica caches retain a configuration that can no longer be retrieved or deleted again. Either deregister before the database mutation or log the cleanup failure and continue publishing the deletion event.Proposed post-delete handling
if cfg.Kind == "WebSubApi" && s.webSubTopicDeregistrar != nil { if err := s.webSubTopicDeregistrar(cfg, log); err != nil { - return nil, err + log.Warn("Failed to deregister WebSub topics after deleting configuration", + slog.String("id", cfg.UUID), + slog.Any("error", err)) } }🤖 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 `@gateway/gateway-controller/pkg/service/restapi/service.go` around lines 423 - 427, Update the WebSub deregistration handling in the service method containing publishEvent so a failure from webSubTopicDeregistrar does not return after the database deletion has committed. Either move deregistration before the deletion mutation, or retain the post-delete ordering while logging the error and continuing to publish the deletion event; ensure successful deletion always reaches publishEvent.event-gateway/gateway-controller/cmd/controller/main.go-729-732 (1)
729-732: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAlign webhook-secret authorization roles with the OpenAPI contract.
The OpenAPI operations authorize
adminanddeveloper, while runtime authorization grantsadminandconsumer. This denies documented developer access and grants consumers undocumented access to HMAC-secret creation and rotation.Proposed role correction
- "POST /websub-apis/{id}/secrets": {"admin", "consumer"}, - "GET /websub-apis/{id}/secrets": {"admin", "consumer"}, - "DELETE /websub-apis/{id}/secrets/{secretName}": {"admin", "consumer"}, - "POST /websub-apis/{id}/secrets/{secretName}/regenerate": {"admin", "consumer"}, + "POST /websub-apis/{id}/secrets": {"admin", "developer"}, + "GET /websub-apis/{id}/secrets": {"admin", "developer"}, + "DELETE /websub-apis/{id}/secrets/{secretName}": {"admin", "developer"}, + "POST /websub-apis/{id}/secrets/{secretName}/regenerate": {"admin", "developer"},🤖 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/gateway-controller/cmd/controller/main.go` around lines 729 - 732, Update the webhook-secret authorization entries in the route permissions map to grant “admin” and “developer” instead of “admin” and “consumer” for creation, retrieval, deletion, and regeneration endpoints. Keep the existing route paths and permission structure unchanged.event-gateway/gateway-controller/cmd/controller/main.go-157-162 (1)
157-162: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHonor
event_gateway.enabledthroughout bootstrapevent-gateway/gateway-controller/cmd/controller/main.go:157-162
Enabled=falseonly skipsValidate(): startup still registers WebSub/WebBroker kinds, wires the Event Gateway XDS hooks, installs the deregistrar, and mounts the Event Gateway HTTP handlers. Gate all of that on the flag, or remove the option if this binary always exposes Event Gateway.🤖 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/gateway-controller/cmd/controller/main.go` around lines 157 - 162, Gate all Event Gateway bootstrap setup on eventGatewayCfg.Enabled, not only eventGatewayCfg.Validate() in main: skip WebSub/WebBroker registration, Event Gateway XDS hook wiring, deregistrar installation, and HTTP handler mounting when disabled. If the binary must always expose Event Gateway, remove the enabled option and its conditional validation instead.gateway/gateway-controller/pkg/utils/api_deployment.go-332-341 (1)
332-341: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRegister a configuration validator for
WebBrokerApi.The event-gateway registration supplies a WebBroker deploy parser but no
KindConfigValidator. Consequently, every parsedeventgateway.WebBrokerApireaches this default branch and fails withunexpected configuration type, making WebBroker create/update operations unusable.Implement and register the WebBroker validator before enabling these endpoints.
🤖 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 `@gateway/gateway-controller/pkg/utils/api_deployment.go` around lines 332 - 341, The WebBrokerApi configuration lacks a registered KindConfigValidator, causing rendered WebBroker configurations to hit the unexpected-type error. Implement a validator for eventgateway.WebBrokerApi that returns its API name, version, and validation errors, then register it in kindConfigValidators alongside the existing validators before enabling WebBroker create/update endpoints.event-gateway/gateway-controller/pkg/handler/websub_api_handler.go-78-80 (1)
78-80: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not return deployment errors verbatim to clients.
DeployAPIConfigurationcan wrap storage, parsing, and other internal failures. Returningerr.Error()may expose database details or internal paths. Log the original error and map validation versus internal failures to sterile response objects.As per coding guidelines, raw database errors, internal service details, and filesystem paths must be sanitized before marshaling HTTP responses.
Also applies to: 220-222
🤖 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/gateway-controller/pkg/handler/websub_api_handler.go` around lines 78 - 80, Update the error handling around DeployAPIConfiguration in the websub API handler: log the original error for diagnostics, but never expose err.Error() in the HTTP response. Map validation failures to the established sterile validation response and all other failures to a generic internal-error response, including the corresponding error path noted in the comment.Source: Coding guidelines
event-gateway/gateway-controller/pkg/handler/websub_api_handler.go-45-45 (1)
45-45: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound request-body reads and return HTTP 413 when exceeded.
Both endpoints read attacker-controlled bodies without a size ceiling, allowing a request to consume unbounded memory. Use a configured limit and distinguish oversized payloads from malformed requests.
As per coding guidelines, wrap every network-originated
io.Readerinio.LimitReader, source the limit from configuration, and return a generic HTTP 413 response when exceeded.Also applies to: 176-176
🤖 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/gateway-controller/pkg/handler/websub_api_handler.go` at line 45, Update the request-body reads in both endpoint handlers around the visible io.ReadAll calls to wrap r.Body with io.LimitReader using the configured request-size limit. Detect when the limit is exceeded, return a generic HTTP 413 response for oversized payloads, and preserve the existing malformed-request handling for other read or parsing errors.Source: Coding guidelines
event-gateway/gateway-controller/pkg/config/validator.go-106-113 (1)
106-113: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftValidate the policies inside
allChannelsand each channel value.
validateChannelPoliciesonly examines map keys, so empty/invalid policy names, versions, parameters, and everyspec.AllChannelspolicy are accepted. This can persist configurations that fail later during policy translation.Add policy-reference validation for every event policy group and cover missing/invalid versions with tests.
Based on learnings,
Policy.Versionis required and must remain a major-only value such asv1.🤖 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/gateway-controller/pkg/config/validator.go` around lines 106 - 113, The channel validation flow around validateChannelPolicies must validate policy references in every channel value and in spec.AllChannels, including policy names, required major-only Policy.Version values such as v1, and parameters. Extend or reuse the existing policy-reference validator for all event policy groups, and add tests covering missing and invalid versions plus allChannels validation.Source: Learnings
gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go-83-101 (1)
83-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAuth-failure payload doesn't match the mandated shape.
On missing auth context, this returns
{"status":"error","message":"Authentication context not available"}with401. The required contract is a fixed payload for all auth failures.As per coding guidelines, auth failures must "return the exact same client-facing payload and HTTP status for invalid, expired, missing, or revoked credentials:
401 Unauthorizedwith{"error":"unauthorized","message":"Invalid or expired credentials."}."🔒 Proposed fix to align the response payload
- httputil.WriteJSON(w, http.StatusUnauthorized, errorResponse{ - Status: "error", - Message: "Authentication context not available", - }) + httputil.WriteJSON(w, http.StatusUnauthorized, struct { + Error string `json:"error"` + Message string `json:"message"` + }{ + Error: "unauthorized", + Message: "Invalid or expired credentials.", + })🤖 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 `@gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go` around lines 83 - 101, Update the missing-auth-context branch in ExtractAuthenticatedUser to return HTTP 401 with the mandated fixed payload: error "unauthorized" and message "Invalid or expired credentials." Replace the current errorResponse fields while preserving the existing logging and false return behavior.Source: Coding guidelines
gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go-61-78 (1)
61-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnbounded request body read — add a size limit.
io.ReadAll(r.Body)has no size cap, so a large/slow request body can exhaust server memory. This helper is shared across every handler kind (REST, LLM, MCP, WebSub/WebBroker), so the risk is broad.As per coding guidelines, "Wrap every user- or network-originated
io.Readerinio.LimitReaderbefore reading it into memory," with the limit "sourced from configuration... provide a safe default," and on exceeding it "return HTTP 413 with a generic message."🛡️ Proposed fix to bound the request body size
+const maxRequestBodyBytes = 10 << 20 // TODO: source from config with a safe default + func BindRequestBody(r *http.Request, request interface{}) error { contentType := r.Header.Get("Content-Type") contentType = strings.TrimSpace(contentType) if idx := strings.Index(contentType, ";"); idx != -1 { contentType = contentType[:idx] } contentType = strings.TrimSpace(strings.ToLower(contentType)) - body, err := io.ReadAll(r.Body) + body, err := io.ReadAll(io.LimitReader(r.Body, maxRequestBodyBytes)) if err != nil { return err }🤖 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 `@gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go` around lines 61 - 78, Update BindRequestBody to read r.Body through an io.LimitReader with a configurable maximum request-body size and a safe default when configuration is absent. Detect bodies exceeding the limit and return an HTTP 413 error with a generic message, while preserving the existing YAML/JSON unmarshalling behavior for allowed payloads.Source: Coding guidelines
🟡 Minor comments (2)
event-gateway/gateway-controller/pkg/translator/hooks.go-132-138 (1)
132-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the unused
parsedMainURLpath handlingparsedMainURL.Pathis set here but never used;CreateRoutestill takesconstants.WEBSUB_PATHdirectly. Either wire the parsed path into the route or drop this block.🤖 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/gateway-controller/pkg/translator/hooks.go` around lines 132 - 138, Remove the unused parsedMainURL.Path handling in the hook URL parsing flow: delete the conditional that assigns constants.WEBSUB_PATH, while retaining URL parsing and invalid-URL error handling because CreateRoute continues to receive constants.WEBSUB_PATH directly.event-gateway/gateway-controller/pkg/config/validator.go-77-90 (1)
77-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReport the actual
displayNamefield in validation errors.These errors currently point clients to nonexistent
spec.name, preventing accurate field-level feedback.Proposed fix
- Field: "spec.name", + Field: "spec.displayName",Apply this replacement to all three display-name errors.
🤖 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/gateway-controller/pkg/config/validator.go` around lines 77 - 90, Update all three validation errors in the display-name checks within the validator to report the actual field path "spec.displayName" instead of the nonexistent "spec.name"; leave their validation conditions and messages unchanged.
🧹 Nitpick comments (1)
event-gateway/gateway-controller/Makefile (1)
48-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate repeated
--build-contextflags across the four docker targets.
build,build-and-push-multiarch,build-coverage-image, andbuild-debugall repeat the identical 7-line--build-contextblock, which is also what triggers checkmake'smaxbodylengthwarnings. Extracting a shared variable reduces duplication and keeps the contexts in sync if a path ever changes.♻️ Proposed refactor
+DOCKER_BUILD_CONTEXTS = \ + --build-context sdk=../../sdk \ + --build-context sdk-core=../../sdk/core \ + --build-context common=../../common \ + --build-context httpkit=../../httpkit \ + --build-context gateway-controller=../../gateway/gateway-controller \ + --build-context policies=$(POLICIES_BUILD_CONTEXT) \ + --build-context target=target + build: generate-server-code test ## Build Docker image using buildx `@echo` "Building Docker image ($(IMAGE_NAME):$(VERSION))..." `@mkdir` -p target `@cp` ../../LICENSE target/ docker buildx build -f Dockerfile \ - --build-context sdk=../../sdk \ - --build-context sdk-core=../../sdk/core \ - --build-context common=../../common \ - --build-context httpkit=../../httpkit \ - --build-context gateway-controller=../../gateway/gateway-controller \ - --build-context policies=$(POLICIES_BUILD_CONTEXT) \ - --build-context target=target \ + $(DOCKER_BUILD_CONTEXTS) \ --build-arg VERSION=$(VERSION) \ --build-arg GIT_COMMIT=$(GIT_COMMIT) \ --target production \ -t $(IMAGE_NAME):$(VERSION) \ -t $(IMAGE_NAME):latest \ --load \ .Apply the same substitution to the other three targets.
🤖 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/gateway-controller/Makefile` around lines 48 - 134, Consolidate the repeated seven --build-context arguments from build, build-and-push-multiarch, build-coverage-image, and build-debug into one shared Make variable. Replace each duplicated block with that variable while preserving all existing context paths and build behavior, so future context changes remain synchronized and maxbodylength warnings are avoided.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 `@event-gateway/gateway-controller/pkg/translator/hooks.go`:
- Around line 337-373: Update the first HttpFilter constructed after
dynamicFwdAny so its Name uses the dynamic-forward-proxy filter identifier, such
as the corresponding wellknown constant, while keeping the router name on the
second HttpFilter that carries routerAny.
---
Outside diff comments:
In `@event-gateway/gateway-controller/pkg/handler/webbroker_api_handler.go`:
- Around line 96-118: Update ListWebBrokerApis to apply the documented
params.DisplayName, params.Version, and params.Status filters before building
the items response. Match each configuration against only the provided query
parameters, preserve all configurations when filters are absent, and ensure
count reflects the filtered results.
---
Major comments:
In `@event-gateway/gateway-controller/api/eventgateway-openapi.yaml`:
- Around line 10-16: Update the OpenAPI server URLs in the event-gateway
specification to use HTTPS instead of HTTP for both local development and
Docker/Kubernetes deployment, while ensuring the controller or its ingress
enforces TLS termination so Basic authentication is never sent over cleartext.
In `@event-gateway/gateway-controller/cmd/controller/main.go`:
- Around line 627-631: Update the http.Server initialization in the controller
startup flow to configure ReadTimeout, WriteTimeout, and IdleTimeout alongside
the existing ReadHeaderTimeout. Use appropriate bounded durations for each so
request bodies, responses, and keep-alive connections cannot remain indefinitely
active.
- Around line 276-326: Ensure the WebSub handler and webhook-secret routes are
not registered when webhookSecretService is nil; either require at least one
encryption provider during startup or make those endpoints return a
service-unavailable response without dereferencing the nil service. Update the
route/handler registration flow alongside the existing webhookSecretService
initialization, preserving normal behavior when encryption is configured.
- Around line 583-591: Update the outer middleware chain assembled in main
around outerMiddlewares and httpHandler to apply a configurable request-body
size limit with a safe default to every incoming request, using
http.MaxBytesReader or equivalent. Ensure requests exceeding the limit return a
generic HTTP 413 response, while preserving the existing correlation,
error-handling, logging, metrics, and mux routing behavior.
- Around line 729-732: Update the webhook-secret authorization entries in the
route permissions map to grant “admin” and “developer” instead of “admin” and
“consumer” for creation, retrieval, deletion, and regeneration endpoints. Keep
the existing route paths and permission structure unchanged.
- Around line 157-162: Gate all Event Gateway bootstrap setup on
eventGatewayCfg.Enabled, not only eventGatewayCfg.Validate() in main: skip
WebSub/WebBroker registration, Event Gateway XDS hook wiring, deregistrar
installation, and HTTP handler mounting when disabled. If the binary must always
expose Event Gateway, remove the enabled option and its conditional validation
instead.
In `@event-gateway/gateway-controller/cmd/controller/runtime_bootstrap.go`:
- Around line 132-154: Clone each config returned by ConfigStore.GetAll() before
passing it to templateengine.RenderSpec, and render the cloned StoredConfig
rather than the shared apiConfig pointer. Keep the existing error handling and
subsequent transformation flow intact, while ensuring resolved secrets never
mutate the stored in-memory configuration.
In `@event-gateway/gateway-controller/pkg/config/validator.go`:
- Around line 106-113: The channel validation flow around
validateChannelPolicies must validate policy references in every channel value
and in spec.AllChannels, including policy names, required major-only
Policy.Version values such as v1, and parameters. Extend or reuse the existing
policy-reference validator for all event policy groups, and add tests covering
missing and invalid versions plus allChannels validation.
In `@event-gateway/gateway-controller/pkg/handler/helpers.go`:
- Around line 44-63: Sanitize client-facing responses in the render-error
handling logic by replacing secretErr.Error(), tmplParseErr.Error(), and
renderErr.Cause with fixed, non-sensitive messages. Preserve logging of the
complete render error separately, and keep the existing status and response
structure in the surrounding handler unchanged.
In `@event-gateway/gateway-controller/pkg/handler/websub_api_handler.go`:
- Around line 78-80: Update the error handling around DeployAPIConfiguration in
the websub API handler: log the original error for diagnostics, but never expose
err.Error() in the HTTP response. Map validation failures to the established
sterile validation response and all other failures to a generic internal-error
response, including the corresponding error path noted in the comment.
- Line 45: Update the request-body reads in both endpoint handlers around the
visible io.ReadAll calls to wrap r.Body with io.LimitReader using the configured
request-size limit. Detect when the limit is exceeded, return a generic HTTP 413
response for oversized payloads, and preserve the existing malformed-request
handling for other read or parsing errors.
In `@event-gateway/gateway-controller/pkg/hubtopic/deregistrar.go`:
- Around line 68-87: Bound concurrent topic deregistration in the loop
surrounding UnregisterTopicWithHub by introducing a configured worker limit or
semaphore. Ensure each goroutine acquires a slot before issuing the request and
releases it on completion, while preserving the existing timeout, logging, error
counting, and childWg synchronization behavior.
In `@event-gateway/gateway-controller/pkg/kindsupport/kindsupport.go`:
- Around line 116-154: Extract the shared WebSub topic-name derivation from
topicsForWebSubUpdate and UpdateTopicManager into a helper such as
webSubTopicSet(asyncData). Have both functions use this helper for the
context/version/channel mangling and topic-set construction, while preserving
their existing registration and unregistration behavior.
- Around line 198-260: In resolveWebSubVhostSentinels and
resolveWebBrokerVhostSentinels, clone c.Spec.Vhosts before applying any default
or sentinel resolution, including the nil-initialization path, so mutations
affect only the returned configuration. Preserve the existing source Vhosts
value and assign the cloned structure back to c.Spec.Vhosts before modifying its
Main or Sandbox fields.
In `@event-gateway/gateway-controller/pkg/translator/hooks.go`:
- Around line 337-343: Update createDynamicForwardProxyCluster to populate
dfpcluster.ClusterConfig with the same dnsCacheConfig used by the dynamic
forward proxy HTTP filter. Extract or reuse a shared dnsCacheConfig helper so
both call sites consistently use identical DNS cache settings.
In `@gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go`:
- Around line 83-101: Update the missing-auth-context branch in
ExtractAuthenticatedUser to return HTTP 401 with the mandated fixed payload:
error "unauthorized" and message "Invalid or expired credentials." Replace the
current errorResponse fields while preserving the existing logging and false
return behavior.
- Around line 61-78: Update BindRequestBody to read r.Body through an
io.LimitReader with a configurable maximum request-body size and a safe default
when configuration is absent. Detect bodies exceeding the limit and return an
HTTP 413 error with a generic message, while preserving the existing YAML/JSON
unmarshalling behavior for allowed payloads.
In `@gateway/gateway-controller/pkg/service/restapi/service.go`:
- Around line 423-427: Update the WebSub deregistration handling in the service
method containing publishEvent so a failure from webSubTopicDeregistrar does not
return after the database deletion has committed. Either move deregistration
before the deletion mutation, or retain the post-delete ordering while logging
the error and continuing to publish the deletion event; ensure successful
deletion always reaches publishEvent.
In `@gateway/gateway-controller/pkg/storage/memory.go`:
- Around line 99-103: Make Add and Update atomic by validating and invoking
cs.webSubTopicUpdater before committing any configuration, index, or topic
mutations. If the hook returns an error, leave all existing maps and topic
entries unchanged; otherwise apply the complete mutation. Update both Add and
Update flows consistently, or roll back every mutation on failure.
In `@gateway/gateway-controller/pkg/utils/api_deployment.go`:
- Around line 332-341: The WebBrokerApi configuration lacks a registered
KindConfigValidator, causing rendered WebBroker configurations to hit the
unexpected-type error. Implement a validator for eventgateway.WebBrokerApi that
returns its API name, version, and validation errors, then register it in
kindConfigValidators alongside the existing validators before enabling WebBroker
create/update endpoints.
In `@gateway/gateway-controller/pkg/xds/translator.go`:
- Around line 838-849: Restore the event_gateway.enabled gate around the
hub-resource translation in the enclosing translator flow, so BuildHubResources
is invoked only when event-gateway hooks are present and the feature is enabled.
Use the existing event-gateway configuration’s Enabled value, while preserving
the current error handling and resource appends.
---
Minor comments:
In `@event-gateway/gateway-controller/pkg/config/validator.go`:
- Around line 77-90: Update all three validation errors in the display-name
checks within the validator to report the actual field path "spec.displayName"
instead of the nonexistent "spec.name"; leave their validation conditions and
messages unchanged.
In `@event-gateway/gateway-controller/pkg/translator/hooks.go`:
- Around line 132-138: Remove the unused parsedMainURL.Path handling in the hook
URL parsing flow: delete the conditional that assigns constants.WEBSUB_PATH,
while retaining URL parsing and invalid-URL error handling because CreateRoute
continues to receive constants.WEBSUB_PATH directly.
---
Nitpick comments:
In `@event-gateway/gateway-controller/Makefile`:
- Around line 48-134: Consolidate the repeated seven --build-context arguments
from build, build-and-push-multiarch, build-coverage-image, and build-debug into
one shared Make variable. Replace each duplicated block with that variable while
preserving all existing context paths and build behavior, so future context
changes remain synchronized and maxbodylength warnings are avoided.
🪄 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: 2903102c-48b0-4930-b894-2126c23c0b19
⛔ Files ignored due to path filters (3)
event-gateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/gateway-controller/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.work
📒 Files selected for processing (61)
event-gateway/Makefileevent-gateway/gateway-controller/Dockerfileevent-gateway/gateway-controller/Makefileevent-gateway/gateway-controller/api/eventgateway-openapi.yamlevent-gateway/gateway-controller/cmd/controller/main.goevent-gateway/gateway-controller/cmd/controller/runtime_bootstrap.goevent-gateway/gateway-controller/go.modevent-gateway/gateway-controller/oapi-codegen.yamlevent-gateway/gateway-controller/pkg/api/eventgateway/generated.goevent-gateway/gateway-controller/pkg/config/eventgateway_config.goevent-gateway/gateway-controller/pkg/config/validator.goevent-gateway/gateway-controller/pkg/config/websub_topic_registration_test_staged.go.txtevent-gateway/gateway-controller/pkg/eventlistener/webhook_secret_handler.goevent-gateway/gateway-controller/pkg/handler/helpers.goevent-gateway/gateway-controller/pkg/handler/resource_response.goevent-gateway/gateway-controller/pkg/handler/server.goevent-gateway/gateway-controller/pkg/handler/webbroker_api_handler.goevent-gateway/gateway-controller/pkg/handler/websub_api_handler.goevent-gateway/gateway-controller/pkg/hubtopic/deregistrar.goevent-gateway/gateway-controller/pkg/kindsupport/kindsupport.goevent-gateway/gateway-controller/pkg/policyhooks/event_channel_translator.goevent-gateway/gateway-controller/pkg/translator/hooks.goevent-gateway/gateway-controller/pkg/webhooksecretservice/webhook_secret.goevent-gateway/gateway-controller/pkg/webhooksecretxds/snapshot.gogateway/gateway-controller/Dockerfilegateway/gateway-controller/Makefilegateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/go.modgateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.gogateway/gateway-controller/pkg/api/handlers/handlers.gogateway/gateway-controller/pkg/api/handlers/handlers_test.gogateway/gateway-controller/pkg/api/handlers/resource_response.gogateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/api_validator_test.gogateway/gateway-controller/pkg/config/config.gogateway/gateway-controller/pkg/config/config_test.gogateway/gateway-controller/pkg/config/parser.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/eventlistener/listener.gogateway/gateway-controller/pkg/eventlistener/listener_test.gogateway/gateway-controller/pkg/models/stored_config.gogateway/gateway-controller/pkg/models/stored_config_test.gogateway/gateway-controller/pkg/policy/builder.gogateway/gateway-controller/pkg/policyxds/combined_cache.gogateway/gateway-controller/pkg/policyxds/server.gogateway/gateway-controller/pkg/policyxds/snapshot.gogateway/gateway-controller/pkg/service/restapi/service.gogateway/gateway-controller/pkg/storage/memory.gogateway/gateway-controller/pkg/storage/sql_store.gogateway/gateway-controller/pkg/utils/api_deployment.gogateway/gateway-controller/pkg/utils/api_deployment_test.gogateway/gateway-controller/pkg/utils/api_key.gogateway/gateway-controller/pkg/utils/helpers.gogateway/gateway-controller/pkg/utils/llm_provider_transformer_test.gogateway/gateway-controller/pkg/xds/eventgateway_hooks.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/gateway-controller/tests/integration/storage_test.go
💤 Files with no reviewable changes (13)
- gateway/gateway-controller/tests/integration/storage_test.go
- gateway/gateway-controller/pkg/config/parser.go
- gateway/gateway-controller/pkg/utils/llm_provider_transformer_test.go
- gateway/gateway-controller/pkg/models/stored_config_test.go
- gateway/gateway-controller/pkg/utils/helpers.go
- gateway/gateway-controller/pkg/api/handlers/resource_response.go
- gateway/gateway-controller/pkg/api/handlers/handlers_test.go
- gateway/gateway-controller/pkg/models/stored_config.go
- gateway/gateway-controller/pkg/config/config_test.go
- gateway/gateway-controller/pkg/config/api_validator_test.go
- gateway/gateway-controller/pkg/config/config.go
- gateway/gateway-controller/pkg/eventlistener/listener_test.go
- gateway/gateway-controller/pkg/xds/translator_test.go
Dependency Validation ResultsDependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Next Steps
|
2 similar comments
Dependency Validation ResultsDependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Next Steps
|
Dependency Validation ResultsDependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Next Steps
|
0266a36 to
ed3a9ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/gateway-controller/api/eventgateway-openapi.yaml`:
- Around line 10-16: Update both server URLs in the OpenAPI servers section to
use https:// instead of http://, preserving their existing hosts, paths, and
descriptions so the basicAuth security scheme is not documented over cleartext.
- Around line 384-393: The WebSub secret endpoint roles are inconsistent between
the OpenAPI operations and generateAuthConfig. Reconcile all
`/websub-apis/{id}/secrets` operations and their corresponding entries in
generateAuthConfig so both documentation and enforcement use the same intended
role set, removing consumer access if the documented admin/developer contract is
authoritative.
In `@event-gateway/gateway-controller/cmd/controller/main.go`:
- Around line 729-732: The WebSub secret routes currently grant consumer access
instead of matching the documented roles. In the route permission entries for
POST/GET `/websub-apis/{id}/secrets`, DELETE
`/websub-apis/{id}/secrets/{secretName}`, and POST regenerate, replace
`consumer` with `developer` while retaining `admin`.
In `@event-gateway/gateway-controller/go.mod`:
- Line 73: Update the dependency pins in go.mod: raise golang.org/x/crypto from
v0.51.0 to the patched v0.52.0 or later, and update go.opentelemetry.io/otel/sdk
from v1.41.0 to a version outside the GHSA-hfvc-g4fc-pqhx vulnerable range.
Regenerate module checksums as needed.
In `@event-gateway/gateway-controller/pkg/translator/hooks.go`:
- Around line 305-311: Update createDynamicFwdListenerForWebSubHub to populate
VirtualHost.Domains with the parsed host/authority from WebSubHubURL rather than
the raw URL, while preserving the existing URL validation flow. Correct the
VirtualHost.Name typo from DYNAMIXC_FORWARD_PROXY_VHOST_WEBSUBHUB to
DYNAMIC_FORWARD_PROXY_VHOST_WEBSUBHUB.
🪄 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: da0dad45-5531-4f05-b95c-4cc319ffca17
⛔ Files ignored due to path filters (7)
cli/src/go.sumis excluded by!**/*.sumevent-gateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/it/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (64)
cli/src/go.modevent-gateway/Makefileevent-gateway/gateway-controller/Dockerfileevent-gateway/gateway-controller/Makefileevent-gateway/gateway-controller/api/eventgateway-openapi.yamlevent-gateway/gateway-controller/cmd/controller/main.goevent-gateway/gateway-controller/cmd/controller/runtime_bootstrap.goevent-gateway/gateway-controller/go.modevent-gateway/gateway-controller/oapi-codegen.yamlevent-gateway/gateway-controller/pkg/api/eventgateway/generated.goevent-gateway/gateway-controller/pkg/config/eventgateway_config.goevent-gateway/gateway-controller/pkg/config/validator.goevent-gateway/gateway-controller/pkg/config/websub_topic_registration_test_staged.go.txtevent-gateway/gateway-controller/pkg/eventlistener/webhook_secret_handler.goevent-gateway/gateway-controller/pkg/handler/helpers.goevent-gateway/gateway-controller/pkg/handler/resource_response.goevent-gateway/gateway-controller/pkg/handler/server.goevent-gateway/gateway-controller/pkg/handler/webbroker_api_handler.goevent-gateway/gateway-controller/pkg/handler/websub_api_handler.goevent-gateway/gateway-controller/pkg/hubtopic/deregistrar.goevent-gateway/gateway-controller/pkg/kindsupport/kindsupport.goevent-gateway/gateway-controller/pkg/policyhooks/event_channel_translator.goevent-gateway/gateway-controller/pkg/translator/hooks.goevent-gateway/gateway-controller/pkg/webhooksecretservice/webhook_secret.goevent-gateway/gateway-controller/pkg/webhooksecretxds/snapshot.gogateway/gateway-controller/Dockerfilegateway/gateway-controller/Makefilegateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/go.modgateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.gogateway/gateway-controller/pkg/api/handlers/handlers.gogateway/gateway-controller/pkg/api/handlers/handlers_test.gogateway/gateway-controller/pkg/api/handlers/resource_response.gogateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/api_validator_test.gogateway/gateway-controller/pkg/config/config.gogateway/gateway-controller/pkg/config/config_test.gogateway/gateway-controller/pkg/config/parser.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/eventlistener/listener.gogateway/gateway-controller/pkg/eventlistener/listener_test.gogateway/gateway-controller/pkg/models/stored_config.gogateway/gateway-controller/pkg/models/stored_config_test.gogateway/gateway-controller/pkg/policy/builder.gogateway/gateway-controller/pkg/policyxds/combined_cache.gogateway/gateway-controller/pkg/policyxds/server.gogateway/gateway-controller/pkg/policyxds/snapshot.gogateway/gateway-controller/pkg/service/restapi/service.gogateway/gateway-controller/pkg/storage/memory.gogateway/gateway-controller/pkg/storage/sql_store.gogateway/gateway-controller/pkg/utils/api_deployment.gogateway/gateway-controller/pkg/utils/api_deployment_test.gogateway/gateway-controller/pkg/utils/api_key.gogateway/gateway-controller/pkg/utils/helpers.gogateway/gateway-controller/pkg/utils/llm_provider_transformer_test.gogateway/gateway-controller/pkg/xds/eventgateway_hooks.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/gateway-controller/tests/integration/storage_test.gogateway/it/go.modplatform-api/go.mod
💤 Files with no reviewable changes (13)
- gateway/gateway-controller/pkg/models/stored_config_test.go
- gateway/gateway-controller/pkg/config/parser.go
- gateway/gateway-controller/pkg/utils/helpers.go
- gateway/gateway-controller/tests/integration/storage_test.go
- gateway/gateway-controller/pkg/eventlistener/listener_test.go
- gateway/gateway-controller/pkg/utils/llm_provider_transformer_test.go
- gateway/gateway-controller/pkg/config/config.go
- gateway/gateway-controller/pkg/api/handlers/resource_response.go
- gateway/gateway-controller/pkg/api/handlers/handlers_test.go
- gateway/gateway-controller/pkg/config/config_test.go
- gateway/gateway-controller/pkg/models/stored_config.go
- gateway/gateway-controller/pkg/xds/translator_test.go
- gateway/gateway-controller/pkg/config/api_validator_test.go
🚧 Files skipped from review as they are similar to previous changes (39)
- event-gateway/gateway-controller/oapi-codegen.yaml
- event-gateway/gateway-controller/pkg/handler/helpers.go
- event-gateway/gateway-controller/pkg/config/eventgateway_config.go
- event-gateway/gateway-controller/pkg/webhooksecretxds/snapshot.go
- event-gateway/gateway-controller/pkg/hubtopic/deregistrar.go
- event-gateway/gateway-controller/pkg/handler/resource_response.go
- gateway/gateway-controller/Dockerfile
- gateway/gateway-controller/pkg/policyxds/server.go
- event-gateway/gateway-controller/pkg/config/validator.go
- event-gateway/gateway-controller/Dockerfile
- event-gateway/gateway-controller/pkg/handler/server.go
- event-gateway/gateway-controller/pkg/webhooksecretservice/webhook_secret.go
- event-gateway/Makefile
- gateway/gateway-controller/pkg/xds/eventgateway_hooks.go
- event-gateway/gateway-controller/pkg/eventlistener/webhook_secret_handler.go
- gateway/gateway-controller/pkg/storage/memory.go
- gateway/gateway-controller/go.mod
- gateway/gateway-controller/pkg/storage/sql_store.go
- gateway/gateway-controller/Makefile
- gateway/gateway-controller/pkg/policyxds/combined_cache.go
- gateway/gateway-controller/pkg/config/api_validator.go
- event-gateway/gateway-controller/pkg/config/websub_topic_registration_test_staged.go.txt
- gateway/gateway-controller/pkg/service/restapi/service.go
- gateway/gateway-controller/pkg/controlplane/client.go
- gateway/gateway-controller/pkg/policy/builder.go
- event-gateway/gateway-controller/pkg/kindsupport/kindsupport.go
- gateway/gateway-controller/pkg/utils/api_key.go
- gateway/gateway-controller/pkg/utils/api_deployment_test.go
- event-gateway/gateway-controller/pkg/handler/webbroker_api_handler.go
- gateway/gateway-controller/pkg/config/validator_test.go
- gateway/gateway-controller/api/management-openapi.yaml
- gateway/gateway-controller/pkg/eventlistener/listener.go
- event-gateway/gateway-controller/pkg/policyhooks/event_channel_translator.go
- gateway/gateway-controller/pkg/utils/api_deployment.go
- gateway/gateway-controller/pkg/xds/translator.go
- gateway/gateway-controller/pkg/api/handlers/handlers.go
- gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go
- event-gateway/gateway-controller/pkg/handler/websub_api_handler.go
- gateway/gateway-controller/pkg/api/management/generated.go
ed3a9ab to
9738089
Compare
Dependency Validation ResultsDependency name: golang.org/x/term Dependency name: golang.org/x/text Dependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Dependency name: github.com/oapi-codegen/runtime Dependency name: github.com/oapi-codegen/runtime Next Steps
|
9738089 to
4c26c24
Compare
Dependency Validation ResultsDependency name: golang.org/x/term Dependency name: golang.org/x/text Dependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Dependency name: github.com/oapi-codegen/runtime Dependency name: github.com/oapi-codegen/runtime Next Steps
|
|
@coderabbitai review approve |
|
✅ Action performedComments resolved and changes approved. Review finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/gateway-controller/pkg/config/validator.go`:
- Around line 182-197: The DisplayName validation errors in validateAsyncData
use the incorrect spec.name field path and inconsistent message. Update all
three errors in this validation block to use spec.displayName and match the
corresponding validateWebBrokerData wording, preserving their existing
validation conditions.
🪄 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: 13a15f22-a866-49a3-9a16-cbf5678a7f5b
⛔ Files ignored due to path filters (7)
cli/src/go.sumis excluded by!**/*.sumevent-gateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/gateway-controller/go.sumis excluded by!**/*.sumgateway/it/go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumplatform-api/go.sumis excluded by!**/*.sum
📒 Files selected for processing (64)
cli/src/go.modevent-gateway/Makefileevent-gateway/gateway-controller/Dockerfileevent-gateway/gateway-controller/Makefileevent-gateway/gateway-controller/api/eventgateway-openapi.yamlevent-gateway/gateway-controller/cmd/controller/main.goevent-gateway/gateway-controller/cmd/controller/runtime_bootstrap.goevent-gateway/gateway-controller/go.modevent-gateway/gateway-controller/oapi-codegen.yamlevent-gateway/gateway-controller/pkg/api/eventgateway/generated.goevent-gateway/gateway-controller/pkg/config/eventgateway_config.goevent-gateway/gateway-controller/pkg/config/validator.goevent-gateway/gateway-controller/pkg/config/websub_topic_registration_test_staged.go.txtevent-gateway/gateway-controller/pkg/eventlistener/webhook_secret_handler.goevent-gateway/gateway-controller/pkg/handler/helpers.goevent-gateway/gateway-controller/pkg/handler/resource_response.goevent-gateway/gateway-controller/pkg/handler/server.goevent-gateway/gateway-controller/pkg/handler/webbroker_api_handler.goevent-gateway/gateway-controller/pkg/handler/websub_api_handler.goevent-gateway/gateway-controller/pkg/hubtopic/deregistrar.goevent-gateway/gateway-controller/pkg/kindsupport/kindsupport.goevent-gateway/gateway-controller/pkg/policyhooks/event_channel_translator.goevent-gateway/gateway-controller/pkg/translator/hooks.goevent-gateway/gateway-controller/pkg/webhooksecretservice/webhook_secret.goevent-gateway/gateway-controller/pkg/webhooksecretxds/snapshot.gogateway/gateway-controller/Dockerfilegateway/gateway-controller/Makefilegateway/gateway-controller/api/management-openapi.yamlgateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/go.modgateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.gogateway/gateway-controller/pkg/api/handlers/handlers.gogateway/gateway-controller/pkg/api/handlers/handlers_test.gogateway/gateway-controller/pkg/api/handlers/resource_response.gogateway/gateway-controller/pkg/api/management/generated.gogateway/gateway-controller/pkg/config/api_validator.gogateway/gateway-controller/pkg/config/api_validator_test.gogateway/gateway-controller/pkg/config/config.gogateway/gateway-controller/pkg/config/config_test.gogateway/gateway-controller/pkg/config/parser.gogateway/gateway-controller/pkg/config/validator_test.gogateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/eventlistener/listener.gogateway/gateway-controller/pkg/eventlistener/listener_test.gogateway/gateway-controller/pkg/models/stored_config.gogateway/gateway-controller/pkg/models/stored_config_test.gogateway/gateway-controller/pkg/policy/builder.gogateway/gateway-controller/pkg/policyxds/combined_cache.gogateway/gateway-controller/pkg/policyxds/server.gogateway/gateway-controller/pkg/policyxds/snapshot.gogateway/gateway-controller/pkg/service/restapi/service.gogateway/gateway-controller/pkg/storage/memory.gogateway/gateway-controller/pkg/storage/sql_store.gogateway/gateway-controller/pkg/utils/api_deployment.gogateway/gateway-controller/pkg/utils/api_deployment_test.gogateway/gateway-controller/pkg/utils/api_key.gogateway/gateway-controller/pkg/utils/helpers.gogateway/gateway-controller/pkg/utils/llm_provider_transformer_test.gogateway/gateway-controller/pkg/xds/eventgateway_hooks.gogateway/gateway-controller/pkg/xds/translator.gogateway/gateway-controller/pkg/xds/translator_test.gogateway/gateway-controller/tests/integration/storage_test.gogateway/it/go.modplatform-api/go.mod
💤 Files with no reviewable changes (13)
- gateway/gateway-controller/pkg/utils/llm_provider_transformer_test.go
- gateway/gateway-controller/tests/integration/storage_test.go
- gateway/gateway-controller/pkg/config/config.go
- gateway/gateway-controller/pkg/config/config_test.go
- gateway/gateway-controller/pkg/api/handlers/resource_response.go
- gateway/gateway-controller/pkg/models/stored_config_test.go
- gateway/gateway-controller/pkg/config/parser.go
- gateway/gateway-controller/pkg/utils/helpers.go
- gateway/gateway-controller/pkg/api/handlers/handlers_test.go
- gateway/gateway-controller/pkg/models/stored_config.go
- gateway/gateway-controller/pkg/eventlistener/listener_test.go
- gateway/gateway-controller/pkg/config/api_validator_test.go
- gateway/gateway-controller/pkg/xds/translator_test.go
🚧 Files skipped from review as they are similar to previous changes (39)
- event-gateway/gateway-controller/oapi-codegen.yaml
- event-gateway/gateway-controller/go.mod
- gateway/gateway-controller/Dockerfile
- platform-api/go.mod
- event-gateway/gateway-controller/pkg/webhooksecretxds/snapshot.go
- event-gateway/gateway-controller/pkg/hubtopic/deregistrar.go
- event-gateway/gateway-controller/pkg/handler/resource_response.go
- event-gateway/gateway-controller/pkg/handler/server.go
- event-gateway/gateway-controller/pkg/config/websub_topic_registration_test_staged.go.txt
- event-gateway/gateway-controller/pkg/eventlistener/webhook_secret_handler.go
- gateway/it/go.mod
- gateway/gateway-controller/pkg/controlplane/client.go
- event-gateway/Makefile
- event-gateway/gateway-controller/Dockerfile
- gateway/gateway-controller/pkg/utils/api_key.go
- gateway/gateway-controller/pkg/policy/builder.go
- gateway/gateway-controller/pkg/policyxds/combined_cache.go
- event-gateway/gateway-controller/pkg/config/eventgateway_config.go
- gateway/gateway-controller/pkg/api/handlers/handlerkit/handlerkit.go
- event-gateway/gateway-controller/pkg/webhooksecretservice/webhook_secret.go
- gateway/gateway-controller/go.mod
- gateway/gateway-controller/pkg/config/validator_test.go
- gateway/gateway-controller/pkg/xds/translator.go
- gateway/gateway-controller/pkg/storage/memory.go
- event-gateway/gateway-controller/pkg/kindsupport/kindsupport.go
- event-gateway/gateway-controller/cmd/controller/runtime_bootstrap.go
- gateway/gateway-controller/pkg/service/restapi/service.go
- gateway/gateway-controller/pkg/policyxds/server.go
- gateway/gateway-controller/Makefile
- gateway/gateway-controller/pkg/utils/api_deployment.go
- gateway/gateway-controller/api/management-openapi.yaml
- gateway/gateway-controller/pkg/utils/api_deployment_test.go
- event-gateway/gateway-controller/pkg/handler/webbroker_api_handler.go
- gateway/gateway-controller/cmd/controller/main.go
- gateway/gateway-controller/pkg/eventlistener/listener.go
- event-gateway/gateway-controller/pkg/handler/websub_api_handler.go
- event-gateway/gateway-controller/pkg/policyhooks/event_channel_translator.go
- gateway/gateway-controller/pkg/storage/sql_store.go
- gateway/gateway-controller/pkg/api/handlers/handlers.go
4c26c24 to
45ed1d7
Compare
Dependency Validation ResultsDependency name: golang.org/x/term Dependency name: golang.org/x/text Dependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Dependency name: github.com/oapi-codegen/runtime Dependency name: github.com/oapi-codegen/runtime Next Steps
|
1 similar comment
Dependency Validation ResultsDependency name: golang.org/x/term Dependency name: golang.org/x/text Dependency name: github.com/envoyproxy/go-control-plane Dependency name: github.com/envoyproxy/go-control-plane/envoy Dependency name: github.com/getkin/kin-openapi Dependency name: github.com/google/uuid Dependency name: github.com/knadh/koanf/parsers/toml/v2 Dependency name: github.com/knadh/koanf/providers/file Dependency name: github.com/knadh/koanf/v2 Dependency name: github.com/oapi-codegen/runtime Dependency name: google.golang.org/protobuf Dependency name: github.com/gorilla/websocket Dependency name: github.com/oapi-codegen/runtime Dependency name: github.com/oapi-codegen/runtime Next Steps
|
2 participants
abstracting event-gateway from gateway-controller