For API key event processing in the gateway, use the artifact UUID instead of the handle in the event payload#1382
Merged
ShalkiWenushika merged 6 commits intowso2:mainfrom Mar 18, 2026
Conversation
…stead of the handle in the event payload.
Contributor
WalkthroughThreads an artifact Kind into API key create/update/revoke flows, adds Kind fields to API key param structs, switches external API key handlers to accept artifact UUIDs and resolve artifacts, and changes gateway/LLM deployment flows to use proxy UUIDs for identification. Changes
Sequence DiagramsequenceDiagram
participant Evt as External Event
participant Svc as APIKeyService
participant Store as ArtifactStore
participant Core as APIKeyCore
Evt->>Svc: CreateExternalAPIKeyFromEvent(artifactUUID)
Svc->>Store: Get(artifactUUID)
Store-->>Svc: artifact { uuid, kind, handle }
Svc->>Svc: Build APIKeyCreationParams(kind, handle, ...)
Svc->>Core: CreateAPIKey(params)
Core-->>Svc: result
Svc-->>Evt: publish success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gateway/gateway-controller/pkg/api/handlers/handlers.go (1)
1832-1838: Consider adding Kind field for consistency with CreateAPIKey and UpdateAPIKey.This PR adds explicit
Kind: models.KindRestApitoCreateAPIKey(line 1772) andUpdateAPIKey(line 1912), butRevokeAPIKeyrelies on the service layer default. While functionally correct, addingKindhere would maintain consistency across all handlers.♻️ Suggested change for consistency
// Prepare parameters params := utils.APIKeyRevocationParams{ + Kind: models.KindRestApi, Handle: handle, APIKeyName: apiKeyName, User: user, CorrelationID: correlationID, Logger: log, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/api/handlers/handlers.go` around lines 1832 - 1838, The API key revocation params construction (utils.APIKeyRevocationParams assigned to params) should include the Kind field for consistency with CreateAPIKey and UpdateAPIKey; add Kind: models.KindRestApi to the params literal in the RevokeAPIKey handler so the struct explicitly matches the other handlers (refer to utils.APIKeyRevocationParams and the params variable in handlers.go).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@gateway/gateway-controller/pkg/api/handlers/handlers.go`:
- Around line 1832-1838: The API key revocation params construction
(utils.APIKeyRevocationParams assigned to params) should include the Kind field
for consistency with CreateAPIKey and UpdateAPIKey; add Kind: models.KindRestApi
to the params literal in the RevokeAPIKey handler so the struct explicitly
matches the other handlers (refer to utils.APIKeyRevocationParams and the params
variable in handlers.go).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cac914ea-58d0-483a-971a-2011a3c7aba2
📒 Files selected for processing (2)
gateway/gateway-controller/pkg/api/handlers/handlers.gogateway/gateway-controller/pkg/utils/api_key.go
renuka-fernando
previously approved these changes
Mar 17, 2026
Krishanx92
previously approved these changes
Mar 17, 2026
renuka-fernando
previously approved these changes
Mar 17, 2026
VirajSalaka
reviewed
Mar 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gateway/gateway-controller/pkg/utils/api_key.go (1)
402-419:⚠️ Potential issue | 🟠 MajorDon't keep revoke/update REST-only after a kind-aware lookup.
Both paths now resolve configs by
kind, but they still immediately requireconfig.Configurationto beapi.RestAPI. For LLM provider/proxy artifacts that will fail before any cleanup or update is applied, so non-REST revoke/update events never take effect.CreateAPIKeyalready usesconfig.DisplayNameandconfig.Version; these methods should do the same instead of downcasting to a REST-specific payload.Suggested fix
- restCfg, ok := config.Configuration.(api.RestAPI) - if !ok { - logger.Error("Configuration is not a RestAPI") - return nil, fmt.Errorf("configuration is not a RestAPI") - } - apiConfig := restCfg.Spec @@ - apiName := apiConfig.DisplayName - apiVersion := apiConfig.Version + apiName := config.DisplayName + apiVersion := config.VersionApply the same change in both
RevokeAPIKeyandUpdateAPIKey.Also applies to: 554-576
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 402 - 419, In RevokeAPIKey and UpdateAPIKey, remove the unconditional downcast of config.Configuration to api.RestAPI (the block using restCfg := config.Configuration.(api.RestAPI)) so non-REST kinds can proceed; instead use the generic config fields like config.DisplayName and config.Version when building the revoke/update payload and only perform a type assertion when you actually need REST-specific fields. Update both code paths (the shown block and the similar block around the 554–576 region) to stop failing early on non-RestAPI types and rely on config's generic accessors until REST-specific behavior is required.platform-api/src/internal/service/llm_deployment.go (1)
991-996:⚠️ Potential issue | 🟠 MajorPropagate the UUID switch to restore/undeploy LLM proxy events too.
This only fixes the initial deploy path.
RestoreLLMProxyDeploymentstill sendsproxy.IDat Line 1064, so the gateway will callplatform-api/src/internal/service/gateway_internal.gowith a handle while Line 157 now looks up the active deployment by artifact UUID. Restoring an archived proxy will miss the deployment row keyed byproxy.UUID;UndeployLLMProxyDeploymentat Line 1131 should be updated to the same contract.Suggested fix
@@ - ProxyId: proxy.ID, + ProxyId: proxy.UUID, @@ - ProxyId: proxy.ID, + ProxyId: proxy.UUID,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-api/src/internal/service/llm_deployment.go` around lines 991 - 996, The deploy path was changed to use proxy.UUID in the LLMProxy deployment event but RestoreLLMProxyDeployment and UndeployLLMProxyDeployment still send proxy.ID; update both functions to send proxy.UUID for the ProxyId field in the model.LLMProxyDeploymentEvent (and any other payload fields where proxy.ID was used) so the gateway and gateway_internal lookup (which uses artifact UUID) can find the active deployment; ensure DeploymentID and Vhost usage remains unchanged and that any callers/handlers expecting the old ID are adjusted to accept the UUID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 238-243: getAPIConfigByHandle currently ignores the incoming
params.Kind and hardcodes models.KindRestApi, so lookups for
LlmProvider/LlmProxy fail; update getAPIConfigByHandle to accept and use the
resolved kind (from params.Kind with fallback logic already in the caller)
instead of always using models.KindRestApi, and ensure all call sites (e.g., the
caller that sets kind from params.Kind and the other occurrences around lines
554-558) pass that kind through; change the function signature or its internal
use to reference the provided kind parameter and remove the hardcoded
models.KindRestApi so the lookup honors LlmProvider/LlmProxy kinds.
---
Outside diff comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 402-419: In RevokeAPIKey and UpdateAPIKey, remove the
unconditional downcast of config.Configuration to api.RestAPI (the block using
restCfg := config.Configuration.(api.RestAPI)) so non-REST kinds can proceed;
instead use the generic config fields like config.DisplayName and config.Version
when building the revoke/update payload and only perform a type assertion when
you actually need REST-specific fields. Update both code paths (the shown block
and the similar block around the 554–576 region) to stop failing early on
non-RestAPI types and rely on config's generic accessors until REST-specific
behavior is required.
In `@platform-api/src/internal/service/llm_deployment.go`:
- Around line 991-996: The deploy path was changed to use proxy.UUID in the
LLMProxy deployment event but RestoreLLMProxyDeployment and
UndeployLLMProxyDeployment still send proxy.ID; update both functions to send
proxy.UUID for the ProxyId field in the model.LLMProxyDeploymentEvent (and any
other payload fields where proxy.ID was used) so the gateway and
gateway_internal lookup (which uses artifact UUID) can find the active
deployment; ensure DeploymentID and Vhost usage remains unchanged and that any
callers/handlers expecting the old ID are adjusted to accept the UUID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0328f49-6ee8-4456-af2d-fa9af13665bb
📒 Files selected for processing (4)
gateway/gateway-controller/pkg/api/handlers/handlers.gogateway/gateway-controller/pkg/utils/api_key.goplatform-api/src/internal/service/gateway_internal.goplatform-api/src/internal/service/llm_deployment.go
🚧 Files skipped from review as they are similar to previous changes (1)
- gateway/gateway-controller/pkg/api/handlers/handlers.go
ShalkiWenushika
dismissed stale reviews from renuka-fernando and Krishanx92
via
9158c66
ShalkiWenushika
requested review from
Krishanx92,
VirajSalaka and
renuka-fernando
ShalkiWenushika
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
gateway/gateway-controller/pkg/utils/api_key.go (2)
1337-1349:⚠️ Potential issue | 🟠 MajorPreserve immutable
IssuerandExternalRefIdduring update.
updateAPIKeyFromRequestrebuildsupdatedKeywithout copying these fields, which clears them after update.✅ Proposed fix
updatedKey := &models.APIKey{ UUID: existingKey.UUID, Name: existingKey.Name, APIKey: hashedAPIKeyValue, // Store hashed key MaskedAPIKey: maskedAPIKeyValue, // Store masked key for display ArtifactUUID: existingKey.ArtifactUUID, Status: models.APIKeyStatusActive, CreatedAt: existingKey.CreatedAt, CreatedBy: existingKey.CreatedBy, UpdatedAt: now, ExpiresAt: expiresAt, Source: existingKey.Source, // Preserve source from original key. + Issuer: existingKey.Issuer, + ExternalRefId: existingKey.ExternalRefId, }Based on learnings: In
gateway/gateway-controller/pkg/utils/api_key.go, ensureAPIKey.IssuerandAPIKey.ExternalRefIdremain immutable after creation and are copied fromexistingKeyinupdateAPIKeyFromRequest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 1337 - 1349, updateAPIKeyFromRequest currently reconstructs updatedKey without copying immutable fields, causing APIKey.Issuer and APIKey.ExternalRefId to be cleared; modify the updatedKey construction in updateAPIKeyFromRequest to copy Issuer and ExternalRefId from existingKey (e.g., set Issuer: existingKey.Issuer and ExternalRefId: existingKey.ExternalRefId) so those values remain unchanged after update while leaving other fields as already handled.
600-607:⚠️ Potential issue | 🟠 MajorPropagate resolved
kindwhen update falls back to create.When key-not-found triggers create,
APIKeyCreationParamsomitsKind.CreateAPIKeythen defaults to REST and can resolve the wrong artifact for non-REST kinds.✅ Proposed fix
creationParams := APIKeyCreationParams{ + Kind: kind, Handle: params.Handle, Request: params.Request, User: user, CorrelationID: params.CorrelationID, Logger: logger, ApiKeyHashes: params.ApiKeyHashes, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 600 - 607, When UpdateAPIKey falls back to creating a new key it constructs APIKeyCreationParams without propagating the resolved Kind, causing CreateAPIKey to default to REST and choose the wrong artifact for non-REST kinds; update the construction of APIKeyCreationParams in the fallback path to include the resolved Kind (params.Kind or the local resolved variable) so CreateAPIKey receives the correct Kind and resolves the proper artifact. Ensure the same Kind value used to decide fallback is passed into APIKeyCreationParams before calling CreateAPIKey.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 407-416: The RevokeAPIKey function references an undefined
variable operationType in the slog.String calls, causing a compile error; fix by
introducing a local operationType variable (e.g., operationType := "revoke") at
the top of RevokeAPIKey or replace the references with the correct existing
identifier or literal string ("revoke") so the two logger calls in RevokeAPIKey
(the IsNotFoundError branch and the subsequent error log) reference a defined
symbol; ensure any other logging in RevokeAPIKey uses the same defined
operationType for consistency.
---
Outside diff comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 1337-1349: updateAPIKeyFromRequest currently reconstructs
updatedKey without copying immutable fields, causing APIKey.Issuer and
APIKey.ExternalRefId to be cleared; modify the updatedKey construction in
updateAPIKeyFromRequest to copy Issuer and ExternalRefId from existingKey (e.g.,
set Issuer: existingKey.Issuer and ExternalRefId: existingKey.ExternalRefId) so
those values remain unchanged after update while leaving other fields as already
handled.
- Around line 600-607: When UpdateAPIKey falls back to creating a new key it
constructs APIKeyCreationParams without propagating the resolved Kind, causing
CreateAPIKey to default to REST and choose the wrong artifact for non-REST
kinds; update the construction of APIKeyCreationParams in the fallback path to
include the resolved Kind (params.Kind or the local resolved variable) so
CreateAPIKey receives the correct Kind and resolves the proper artifact. Ensure
the same Kind value used to decide fallback is passed into APIKeyCreationParams
before calling CreateAPIKey.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: edcc0399-bf90-4da1-a36c-25c385004bba
📒 Files selected for processing (1)
gateway/gateway-controller/pkg/utils/api_key.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
gateway/gateway-controller/pkg/utils/api_key.go (3)
1335-1347:⚠️ Potential issue | 🟠 Major
updateAPIKeyFromRequestdrops immutable metadata fields.
updatedKeyis rebuilt without carryingexistingKey.IssuerandexistingKey.ExternalRefId, so updates can clear immutable metadata.Suggested fix
updatedKey := &models.APIKey{ UUID: existingKey.UUID, Name: existingKey.Name, APIKey: hashedAPIKeyValue, // Store hashed key MaskedAPIKey: maskedAPIKeyValue, // Store masked key for display ArtifactUUID: existingKey.ArtifactUUID, Status: models.APIKeyStatusActive, CreatedAt: existingKey.CreatedAt, CreatedBy: existingKey.CreatedBy, UpdatedAt: now, ExpiresAt: expiresAt, Source: existingKey.Source, // Preserve source from original key. + Issuer: existingKey.Issuer, + ExternalRefId: existingKey.ExternalRefId, }Based on learnings In gateway/gateway-controller/pkg/utils/api_key.go, ensure APIKey.Issuer and APIKey.ExternalRefId remain immutable after creation. During updateAPIKeyFromRequest, copy these fields from existingKey and do not overwrite or clear them during the update.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 1335 - 1347, The updateAPIKeyFromRequest rebuilds an APIKey but omits immutable metadata fields, which allows Issuer and ExternalRefId to be cleared; update the construction of updatedKey in updateAPIKeyFromRequest to copy existingKey.Issuer and existingKey.ExternalRefId into the new models.APIKey so those fields are preserved (do not overwrite or set them to zero values) when updating APIKey.
419-423:⚠️ Potential issue | 🔴 Critical
RestAPItype assertion blocks non-REST artifact key operations.After introducing
Kind, both revoke and update still hard-fail onconfig.Configuration.(api.RestAPI). For non-REST artifacts this returns"configuration is not a RestAPI"and aborts the flow.Suggested fix
- // Validate config type before any storage mutations to fail fast - restCfg, ok := config.Configuration.(api.RestAPI) - if !ok { - logger.Error("Configuration is not a RestAPI") - return nil, fmt.Errorf("configuration is not a RestAPI") - } - apiConfig := restCfg.Spec + // Use common stored config metadata for downstream operations + apiName := config.DisplayName + apiVersion := config.Version- apiId := config.UUID - apiName := apiConfig.DisplayName - apiVersion := apiConfig.Version + apiId := config.UUIDApply the same replacement in both
RevokeAPIKeyandUpdateAPIKey.Also applies to: 576-580
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 419 - 423, Replace the hard type assertion config.Configuration.(api.RestAPI) in both RevokeAPIKey and UpdateAPIKey with a conditional check that only attempts to cast to api.RestAPI when the artifact Kind indicates a REST artifact (e.g., artifact.Kind == api.KindREST); for non-REST kinds skip REST-specific handling and continue the flow instead of returning an error. Update both occurrences (the block around config.Configuration.(api.RestAPI) and the similar code at the other location referenced in the comment) so REST-specific logic is gated by the Kind check and does not abort non-REST revoke/update paths.
598-605:⚠️ Potential issue | 🟠 MajorPass
Kindinto the create-on-miss path inUpdateAPIKey.When the key is missing and update falls back to create,
creationParamsomitsKind.CreateAPIKeythen defaults toRestApi, which can resolve the wrong artifact for non-REST kinds.Suggested fix
creationParams := APIKeyCreationParams{ + Kind: kind, Handle: params.Handle, Request: params.Request, User: user, CorrelationID: params.CorrelationID, Logger: logger, ApiKeyHashes: params.ApiKeyHashes, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway/gateway-controller/pkg/utils/api_key.go` around lines 598 - 605, In UpdateAPIKey, the creationParams used when falling back to create an API key omits the Kind field which causes CreateAPIKey to default to RestApi; update the creationParams assignment (APIKeyCreationParams) to include Kind: params.Kind (or the appropriate source of Kind) so CreateAPIKey receives the correct kind for non-REST keys; ensure the APIKeyCreationParams struct passed into CreateAPIKey contains the Kind value used elsewhere in UpdateAPIKey.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 1859-1867: When s.store.Get(artifactUUID) returns nil or error
inside the three external event handlers (CreateExternalAPIKeyFromEvent,
RevokeExternalAPIKeyFromEvent, UpdateExternalAPIKeyFromEvent), add a fallback to
query persistent storage using s.db.GetConfigByKindAndHandle(...) before
returning "artifact not found". Specifically, replace the current immediate
return in the storedConfig == nil || err branch with a call to
s.db.GetConfigByKindAndHandle passing the artifact kind and handle (matching the
pattern used elsewhere), set storedConfig from that result if found, and only
log and return the error after the DB fallback also fails; keep the existing
slog fields (artifact_uuid/correlation_id/error) when logging the final failure.
---
Outside diff comments:
In `@gateway/gateway-controller/pkg/utils/api_key.go`:
- Around line 1335-1347: The updateAPIKeyFromRequest rebuilds an APIKey but
omits immutable metadata fields, which allows Issuer and ExternalRefId to be
cleared; update the construction of updatedKey in updateAPIKeyFromRequest to
copy existingKey.Issuer and existingKey.ExternalRefId into the new models.APIKey
so those fields are preserved (do not overwrite or set them to zero values) when
updating APIKey.
- Around line 419-423: Replace the hard type assertion
config.Configuration.(api.RestAPI) in both RevokeAPIKey and UpdateAPIKey with a
conditional check that only attempts to cast to api.RestAPI when the artifact
Kind indicates a REST artifact (e.g., artifact.Kind == api.KindREST); for
non-REST kinds skip REST-specific handling and continue the flow instead of
returning an error. Update both occurrences (the block around
config.Configuration.(api.RestAPI) and the similar code at the other location
referenced in the comment) so REST-specific logic is gated by the Kind check and
does not abort non-REST revoke/update paths.
- Around line 598-605: In UpdateAPIKey, the creationParams used when falling
back to create an API key omits the Kind field which causes CreateAPIKey to
default to RestApi; update the creationParams assignment (APIKeyCreationParams)
to include Kind: params.Kind (or the appropriate source of Kind) so CreateAPIKey
receives the correct kind for non-REST keys; ensure the APIKeyCreationParams
struct passed into CreateAPIKey contains the Kind value used elsewhere in
UpdateAPIKey.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a24402c0-dfe6-454f-bcab-884763966297
📒 Files selected for processing (1)
gateway/gateway-controller/pkg/utils/api_key.go
slahirucd7
approved these changes
Mar 18, 2026
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
5 participants
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.
Purpose
Goals
Approach
User stories
Documentation
Automation tests
Security checks
Samples
Related PRs
Test environment
Summary by CodeRabbit
New Features
Bug Fixes / Improvements