add platform api changes for event gateway hmac secret generation#2153
Open
tharindu1st wants to merge 1 commit into
Open
add platform api changes for event gateway hmac secret generation#2153tharindu1st wants to merge 1 commit into
tharindu1st wants to merge 1 commit into
Conversation
tharindu1st
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika and
tharikaGitHub
as code owners
Contributor
📝 WalkthroughOverviewThis pull request adds support for platform-managed WebSub API HMAC secret generation and lifecycle management. The changes enable the event gateway to retrieve, cache, and sync platform-generated HMAC secrets for webhook signature verification, while providing management endpoints in the platform API. Key ChangesGateway Controller
Platform API Database Schema
Platform API Services
Platform API Handlers
Configuration
API Specifications
ImpactThese changes enable WebSub API consumers to manage webhook verification secrets through the platform, with automatic synchronization to connected gateways, eliminating the need for manual secret distribution and improving webhook security posture. WalkthroughThis PR implements platform-managed WebSub HMAC secrets with end-to-end lifecycle support across both platform-api and gateway-controller. The platform-api introduces encrypted-at-rest database persistence, AES-256-GCM encryption with fallback key derivation, CRUD service operations, and REST endpoints for secret management. A new internal gateway API endpoint delivers decrypted secrets securely to controllers. The gateway-controller extends its WebSocket client to synchronize secrets via platform events, stores plaintext secrets locally in the webhook-secret xDS store for validator access, and provides APIs for retrieving encrypted secrets. The implementation includes unique-constraint error handling, feature gating for disabled services, and event broadcasting to keep all gateways in sync with platform secret changes. Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" 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 |
tharindu1st
force-pushed
the
event-gateway-test
branch
3 times, most recently
from
da41672 to
def7de7
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@gateway/gateway-controller/pkg/controlplane/client.go`:
- Around line 2483-2525: Add a delete-side cleanup helper that removes platform
HMAC secrets from the in-memory store and refreshes the xDS snapshot, and call
it from both the normal WebSub delete path and the not-found branch;
specifically, implement a method (e.g., cleanupHmacSecretsForArtifact or reuse
existing logic around webhookSecretStore.RemoveAllByAPI and
webhookSecretSnapshotManager.RefreshSnapshot) that takes artifactID, calls
webhookSecretStore.RemoveAllByAPI(artifactID) and logs failures, then calls
webhookSecretSnapshotManager.RefreshSnapshot() if not nil and logs failures, and
invoke this helper from handleWebSubAPIDeletedEvent and the "not found" branch
so secrets are removed whenever a WebSub API is deleted.
In `@platform-api/src/internal/handler/gateway_internal.go`:
- Around line 826-829: In the hmacSecretService nil check block in
gateway_internal.go, the code currently returns HTTP 200 with an empty
GatewayHmacSecretsResponse when the service is not configured. Change the
http.StatusOK response code to http.StatusServiceUnavailable (503) to properly
signal that this is a temporary platform-side configuration issue rather than an
authoritative empty secrets list. This allows the gateway controller to retry
the request instead of reconciling with an empty set when the HMAC secret
service fails to initialize.
In `@platform-api/src/internal/handler/websub_api_hmac_secret.go`:
- Around line 94-99: The handler currently converts req.Secret to a plain string
before calling h.secretService.Generate, losing the difference between omitted
and empty; instead, detect and reject an explicit empty secret (if req.Secret !=
nil && *req.Secret == "" return 400) before calling h.secretService.Generate,
and apply the same check in the regenerate flow (the code around the second
occurrence at lines 194-199). Reference req.Secret, h.secretService.Generate
(and the regenerate handler) and ensure you return a proper 400 error for
explicit empty secrets rather than treating them as “auto-generate.”
- Around line 190-192: RegenerateHmacSecret currently swallows all JSON bind
errors by doing `_ = c.ShouldBindJSON(&req)`; change this to capture the error
(`err := c.ShouldBindJSON(&req)`) and if err != nil and err is not io.EOF,
respond with HTTP 400 (e.g., c.AbortWithStatusJSON(400, ...)) so malformed JSON
is rejected while still allowing an empty body; import io and reference the req
variable and c.ShouldBindJSON in your fix.
In `@platform-api/src/internal/repository/websub_api_hmac_secret.go`:
- Around line 57-70: The repository reads nullable database column display_name
into a plain string causing Scan to fail on NULL; update GetByArtifactAndName to
scan display_name into a nullable type (e.g., sql.NullString or *string) and
then assign s.DisplayName appropriately (convert NullString.Valid ?
NullString.String : "" or set pointer), and apply the same change to the other
read/list method in this file that also scans display_name so all read paths
match the table schema.
In `@platform-api/src/internal/service/websub_api_hmac_secret.go`:
- Around line 128-132: The Create error handling currently only checks
isSQLiteUniqueConstraint after calling s.repo.Create, so PostgreSQL
unique-constraint failures slip through; add a backend-agnostic uniqueness check
(e.g., isUniqueConstraint(err)) or extend the existing check to also detect
Postgres unique-constraint errors before returning a generic error. Update the
branch that handles s.repo.Create's error to call this normalized helper (or add
isPostgresUniqueConstraint and combine with isSQLiteUniqueConstraint) and return
constants.ErrHmacSecretAlreadyExists when the helper indicates a duplicate (so
the handler correctly maps to 409).
- Around line 91-94: The derived secret name from slugifyHmacSecret(displayName)
can exceed the DB column limit (name VARCHAR(63)); before calling repo.Create
use slugifyHmacSecret(displayName) then enforce a 63-character limit
(trim/truncate) and fall back to "secret-"+apiHandle if empty, and repeat the
same truncation/validation in the other places noted (the blocks around where
name is set at lines ~128-132 and ~273-285) so that repo.Create always receives
a name <=63 chars and never relies on DB to reject long values.
🪄 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: e041e676-6a4b-4ac9-8cf5-4280e33f3311
📒 Files selected for processing (20)
gateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/controlplane/client_integration_test.gogateway/gateway-controller/pkg/controlplane/controlplane_test.gogateway/gateway-controller/pkg/utils/api_utils.goplatform-api/src/api/websub_hmac_secret.goplatform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/dto/gateway_internal.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/handler/websub_api_hmac_secret.goplatform-api/src/internal/model/websub_api_hmac_secret.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/repository/websub_api_hmac_secret.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/websub_api_hmac_secret.go
Comment on lines
+57
to
+70
| func (r *WebSubAPIHmacSecretRepo) GetByArtifactAndName(artifactUUID, name string) (*model.WebSubAPIHmacSecret, error) { | ||
| query := ` | ||
| SELECT uuid, artifact_uuid, name, display_name, encrypted_secret, status, created_at, updated_at | ||
| FROM websub_api_hmac_secrets | ||
| WHERE artifact_uuid = ? AND name = ?` | ||
| row := r.db.QueryRow(r.db.Rebind(query), artifactUUID, name) | ||
| s := &model.WebSubAPIHmacSecret{} | ||
| if err := row.Scan(&s.UUID, &s.ArtifactUUID, &s.Name, &s.DisplayName, &s.EncryptedSecret, &s.Status, &s.CreatedAt, &s.UpdatedAt); err != nil { | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return nil, nil | ||
| } | ||
| return nil, err | ||
| } | ||
| return s, nil |
Contributor
There was a problem hiding this comment.
Handle nullable display_name consistently with the schema.
display_name is nullable in platform-api/src/internal/database/schema.postgres.sql:371-384, but both read paths scan it into a plain string. A NULL row will make these queries fail during Scan, which then bubbles up as read/list failures. Use sql.NullString/*string or COALESCE(display_name, '') so the repository matches the table contract.
Also applies to: 74-94
🤖 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 `@platform-api/src/internal/repository/websub_api_hmac_secret.go` around lines
57 - 70, The repository reads nullable database column display_name into a plain
string causing Scan to fail on NULL; update GetByArtifactAndName to scan
display_name into a nullable type (e.g., sql.NullString or *string) and then
assign s.DisplayName appropriately (convert NullString.Valid ? NullString.String
: "" or set pointer), and apply the same change to the other read/list method in
this file that also scans display_name so all read paths match the table schema.
Comment on lines
+91
to
+94
| name := slugifyHmacSecret(displayName) | ||
| if name == "" { | ||
| name = "secret-" + apiHandle | ||
| } |
Contributor
There was a problem hiding this comment.
Bound generated secret names to the persisted column limit.
slugifyHmacSecret(displayName) can produce values longer than the name VARCHAR(63) column defined for websub_api_hmac_secrets. On PostgreSQL, a long display name will fail at insert time and surface as a generic create error instead of a clean client-side validation failure. Enforce or trim the derived name before calling repo.Create.
Also applies to: 128-132, 273-285
🤖 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 `@platform-api/src/internal/service/websub_api_hmac_secret.go` around lines 91
- 94, The derived secret name from slugifyHmacSecret(displayName) can exceed the
DB column limit (name VARCHAR(63)); before calling repo.Create use
slugifyHmacSecret(displayName) then enforce a 63-character limit (trim/truncate)
and fall back to "secret-"+apiHandle if empty, and repeat the same
truncation/validation in the other places noted (the blocks around where name is
set at lines ~128-132 and ~273-285) so that repo.Create always receives a name
<=63 chars and never relies on DB to reject long values.
Comment on lines
+128
to
+132
| if err := s.repo.Create(secret); err != nil { | ||
| if isSQLiteUniqueConstraint(err) { | ||
| return nil, "", constants.ErrHmacSecretAlreadyExists | ||
| } | ||
| return nil, "", fmt.Errorf("failed to persist HMAC secret: %w", err) |
Contributor
There was a problem hiding this comment.
Normalize duplicate-name errors across supported databases.
This branch only recognizes SQLite uniqueness failures. The PR adds PostgreSQL/SQLite schema support, so duplicate (artifact_uuid, name) inserts on PostgreSQL will currently fall through as generic errors instead of ErrHmacSecretAlreadyExists, which breaks the handler’s 409 mapping. Normalize unique-constraint errors in a backend-agnostic way at the repository boundary or here before returning.
🤖 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 `@platform-api/src/internal/service/websub_api_hmac_secret.go` around lines 128
- 132, The Create error handling currently only checks isSQLiteUniqueConstraint
after calling s.repo.Create, so PostgreSQL unique-constraint failures slip
through; add a backend-agnostic uniqueness check (e.g., isUniqueConstraint(err))
or extend the existing check to also detect Postgres unique-constraint errors
before returning a generic error. Update the branch that handles s.repo.Create's
error to call this normalized helper (or add isPostgresUniqueConstraint and
combine with isSQLiteUniqueConstraint) and return
constants.ErrHmacSecretAlreadyExists when the helper indicates a duplicate (so
the handler correctly maps to 409).
def7de7 to
98ebb36
Compare
tharindu1st
force-pushed
the
event-gateway-test
branch
from
98ebb36 to
4f693ea
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/controlplane/client.go (1)
2486-2497: ⚡ Quick winAdd nil guard for
apiUtilsServicefor consistency with similar methods.Other sync methods in this file (e.g.,
syncSubscriptionPlans,syncSubscriptionsForExistingAPIs) checkapiUtilsServicefor nil before invoking it. This method should follow the same pattern.Suggested fix
func (c *Client) syncHmacSecretsForArtifact(artifactID string) { if c.webhookSecretStore == nil { return } + if c.apiUtilsService == nil { + return + } secrets, err := c.apiUtilsService.FetchWebSubAPIHmacSecrets(artifactID)🤖 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/controlplane/client.go` around lines 2486 - 2497, Add a nil guard for c.apiUtilsService in the syncHmacSecretsForArtifact method for consistency with other sync methods in the file. Right after the existing nil check for c.webhookSecretStore, add a nil check for c.apiUtilsService that returns early if it is nil, before calling c.apiUtilsService.FetchWebSubAPIHmacSecrets.
🤖 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 `@platform-api/src/config/config.go`:
- Around line 170-173: The HMACSecretEncryptionKey field is using an envconfig
tag which is incompatible with the koanf-based configuration system used in this
file. Remove or update the envconfig tag to align with the koanf configuration
approach, and add a case in the envToKoanfKey function to properly map the
DATABASE_HMAC_SECRET_ENCRYPTION_KEY environment variable (which is documented in
the field's comment) so it gets loaded correctly as the koanf configuration key
for hmac_secret_encryption_key.
---
Nitpick comments:
In `@gateway/gateway-controller/pkg/controlplane/client.go`:
- Around line 2486-2497: Add a nil guard for c.apiUtilsService in the
syncHmacSecretsForArtifact method for consistency with other sync methods in the
file. Right after the existing nil check for c.webhookSecretStore, add a nil
check for c.apiUtilsService that returns early if it is nil, before calling
c.apiUtilsService.FetchWebSubAPIHmacSecrets.
🪄 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: a84496d1-f8fd-4433-87a1-751441adc318
📒 Files selected for processing (24)
gateway/gateway-controller/cmd/controller/main.gogateway/gateway-controller/pkg/controlplane/client.gogateway/gateway-controller/pkg/controlplane/client_integration_test.gogateway/gateway-controller/pkg/controlplane/controlplane_test.gogateway/gateway-controller/pkg/utils/api_utils.goplatform-api/src/api/generated.goplatform-api/src/api/manual_types.goplatform-api/src/api/websub_hmac_secret.goplatform-api/src/config/config.goplatform-api/src/internal/constants/error.goplatform-api/src/internal/database/schema.postgres.sqlplatform-api/src/internal/database/schema.sqlplatform-api/src/internal/database/schema.sqlite.sqlplatform-api/src/internal/dto/gateway_internal.goplatform-api/src/internal/handler/gateway_internal.goplatform-api/src/internal/handler/websub_api_hmac_secret.goplatform-api/src/internal/model/websub_api_hmac_secret.goplatform-api/src/internal/repository/interfaces.goplatform-api/src/internal/repository/websub_api_hmac_secret.goplatform-api/src/internal/server/server.goplatform-api/src/internal/service/gateway_events.goplatform-api/src/internal/service/websub_api_hmac_secret.goplatform-api/src/resources/gateway-internal-api.yamlplatform-api/src/resources/openapi.yaml
💤 Files with no reviewable changes (8)
- platform-api/src/internal/service/gateway_events.go
- platform-api/src/resources/gateway-internal-api.yaml
- platform-api/src/internal/model/websub_api_hmac_secret.go
- platform-api/src/resources/openapi.yaml
- platform-api/src/internal/repository/interfaces.go
- platform-api/src/internal/server/server.go
- platform-api/src/internal/service/websub_api_hmac_secret.go
- platform-api/src/internal/repository/websub_api_hmac_secret.go
✅ Files skipped from review due to trivial changes (2)
- platform-api/src/api/websub_hmac_secret.go
- platform-api/src/api/generated.go
🚧 Files skipped from review as they are similar to previous changes (8)
- gateway/gateway-controller/pkg/controlplane/client_integration_test.go
- platform-api/src/internal/dto/gateway_internal.go
- platform-api/src/internal/database/schema.postgres.sql
- gateway/gateway-controller/pkg/utils/api_utils.go
- platform-api/src/internal/database/schema.sqlite.sql
- platform-api/src/internal/database/schema.sql
- gateway/gateway-controller/cmd/controller/main.go
- platform-api/src/internal/handler/websub_api_hmac_secret.go
Comment on lines
+170
to
+173
| // HMACSecretEncryptionKey is the 32-byte key for AES-256-GCM encryption of WebSub API HMAC secrets. | ||
| // Provide as 64 hex chars or 44 base64 chars. | ||
| // Env: DATABASE_HMAC_SECRET_ENCRYPTION_KEY. If empty, falls back to SubscriptionTokenEncryptionKey then JWT_SECRET_KEY. | ||
| HMACSecretEncryptionKey string `envconfig:"HMAC_SECRET_ENCRYPTION_KEY" default:""` |
Contributor
There was a problem hiding this comment.
Environment variable mapping missing for HMACSecretEncryptionKey.
The field uses envconfig tag, but this file uses koanf-based configuration. Additionally, envToKoanfKey has no case for database_hmac_secret_encryption_key, so the documented DATABASE_HMAC_SECRET_ENCRYPTION_KEY environment variable won't be loaded.
Suggested fix
Update the struct field tag:
- HMACSecretEncryptionKey string `envconfig:"HMAC_SECRET_ENCRYPTION_KEY" default:""`
+ HMACSecretEncryptionKey string `koanf:"hmac_secret_encryption_key"`Add case in envToKoanfKey (around line 355):
case "database_subscription_token_encryption_key":
return "database.subscription_token_encryption_key"
+ case "database_hmac_secret_encryption_key":
+ return "database.hmac_secret_encryption_key"🤖 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 `@platform-api/src/config/config.go` around lines 170 - 173, The
HMACSecretEncryptionKey field is using an envconfig tag which is incompatible
with the koanf-based configuration system used in this file. Remove or update
the envconfig tag to align with the koanf configuration approach, and add a case
in the envToKoanfKey function to properly map the
DATABASE_HMAC_SECRET_ENCRYPTION_KEY environment variable (which is documented in
the field's comment) so it gets loaded correctly as the koanf configuration key
for hmac_secret_encryption_key.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add platform api changes for event gateway hmac secret generation