Fix issues around pushing existing API keys to a gateway on artifact deployment#2588
Merged
Conversation
DinithHerath
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Krishanx92,
PasanT9,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub,
tharindu1st and
thivindu
as code owners
July 10, 2026 13:20
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughChangesAPI key creation now permits artifacts without gateway associations, LLM key events target associated gateways, and deployment or restore flows backfill active keys across REST, LLM, MCP, WebSub, and WebBroker services. API key targeting and persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant APIKeyService
participant APIRepository
participant GatewayEventsService
participant DeploymentService
Client->>APIKeyService: create API key
APIKeyService->>APIRepository: find associated gateways
APIRepository-->>APIKeyService: associated gateways or none
APIKeyService->>GatewayEventsService: publish key event for associated gateways
DeploymentService->>GatewayEventsService: publish deployment event
DeploymentService->>APIKeyService: backfill existing active keys
APIKeyService->>GatewayEventsService: publish backfill key events
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
platform-api/internal/service/llm_apikey.go (1)
252-255: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winStale early-return breaks the new "empty associations are valid" behavior.
The comment at Lines 243-245 and the empty-case handling at Lines 335-338 state that zero associated gateways is valid: the key is persisted and delivered later via deploy-time backfill. But this block still returns
GatewayConnectionUnavailablewhenlen(gateways) == 0, before the key is persisted (Line 295). Consequences:
- The key is never persisted for an unassociated provider — the opposite of the intended behavior.
- Lines 335-340 become unreachable dead code.
TestCreateLLMProviderAPIKey_AssociationScoped(usesdpKeyNoAssocAPIReporeturning nil, expects success + 0 broadcasts) will fail here.This block should be removed so the flow proceeds to persist and skip broadcasting.
Proposed fix
- if len(gateways) == 0 { - s.slogger.Warn("No gateways found for organization", "organizationId", orgID) - return nil, apperror.GatewayConnectionUnavailable.Wrap(constants.ErrGatewayUnavailable) - } -🤖 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/internal/service/llm_apikey.go` around lines 252 - 255, Remove the len(gateways) == 0 early-return block in the relevant API-key creation flow so zero associated gateways remains valid. Allow execution to continue through persistence and the existing empty-association handling that skips broadcasts, preserving success for unassociated providers; update any now-unused warning or imports if necessary.platform-api/internal/service/llm_proxy_apikey.go (1)
215-218: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winStale early-return breaks the new "empty associations are valid" behavior.
Same defect as in
llm_apikey.go. The comment at Lines 206-208 and the empty handling at Lines 304-307 declare zero associated gateways valid, yet this block returnsGatewayConnectionUnavailablebefore the key is persisted (Line 258). This prevents key persistence for an unassociated proxy, makes Lines 304-309 unreachable, and failsTestCreateLLMProxyAPIKey_AssociationScoped(dpKeyNoAssocAPIRepo→ nil, expects success + 0 broadcasts). Remove this block.Proposed fix
- if len(gateways) == 0 { - s.slogger.Warn("No gateways found for organization", "organizationId", orgID) - return nil, apperror.GatewayConnectionUnavailable.Wrap(constants.ErrGatewayUnavailable) - } -🤖 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/internal/service/llm_proxy_apikey.go` around lines 215 - 218, Remove the len(gateways) == 0 early-return block in the LLM proxy API key creation flow, using the surrounding key persistence logic and the empty-association handling near the later gateway-processing section as references. Allow unassociated proxies to continue through persistence and complete successfully without gateway broadcasts, matching the behavior in llm_apikey.go.platform-api/internal/service/apikey.go (1)
479-484: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEmpty gateway set now logs a spurious error.
With the empty-gateways error return removed (Lines 393-399), zero associated gateways is a valid state handled by deploy-time backfill. But when
gatewaysis empty,successCount == 0still triggerss.slogger.Error("API key created event was not broadcast to any gateway", ...), producing false alarms/monitoring noise. Distinguish the empty case (info) from a genuine all-failed case, mirroring the LLM paths.Proposed fix
- if successCount == 0 { + if len(gateways) == 0 { + s.slogger.Info("API not associated with any gateway; API key saved and will be delivered at deploy time", "apiHandle", apiHandle, "keyName", keyName) + } else if successCount == 0 { s.slogger.Error("API key created event was not broadcast to any gateway", "apiHandle", apiHandle, "keyName", keyName, "lastError", lastError) } else if failureCount > 0 {🤖 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/internal/service/apikey.go` around lines 479 - 484, Update the API key creation broadcast summary logic to handle an empty gateways set separately: when len(gateways) == 0, log an informational message and do not emit the all-failed error; retain the existing error for non-empty gateways where successCount == 0 and preserve the partial-failure warning. Mirror the equivalent handling in the LLM broadcast paths.
🤖 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/internal/service/apikey.go`:
- Around line 335-375: BackfillAPIKeysToGateway must enforce each key’s
AllowedTargets before broadcasting. Use the target gateway name and the existing
filterAPIGatewaysByAllowedTargets logic (or equivalent) within
BackfillAPIKeysToGateway, and skip keys not permitted for that gateway while
preserving active and expiration checks.
---
Outside diff comments:
In `@platform-api/internal/service/apikey.go`:
- Around line 479-484: Update the API key creation broadcast summary logic to
handle an empty gateways set separately: when len(gateways) == 0, log an
informational message and do not emit the all-failed error; retain the existing
error for non-empty gateways where successCount == 0 and preserve the
partial-failure warning. Mirror the equivalent handling in the LLM broadcast
paths.
In `@platform-api/internal/service/llm_apikey.go`:
- Around line 252-255: Remove the len(gateways) == 0 early-return block in the
relevant API-key creation flow so zero associated gateways remains valid. Allow
execution to continue through persistence and the existing empty-association
handling that skips broadcasts, preserving success for unassociated providers;
update any now-unused warning or imports if necessary.
In `@platform-api/internal/service/llm_proxy_apikey.go`:
- Around line 215-218: Remove the len(gateways) == 0 early-return block in the
LLM proxy API key creation flow, using the surrounding key persistence logic and
the empty-association handling near the later gateway-processing section as
references. Allow unassociated proxies to continue through persistence and
complete successfully without gateway broadcasts, matching the behavior in
llm_apikey.go.
🪄 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: 3145d356-6d86-47d0-a9a9-88862ccc4723
📒 Files selected for processing (12)
platform-api/internal/server/server.goplatform-api/internal/service/apikey.goplatform-api/internal/service/artifact_dp_apikey_test.goplatform-api/internal/service/deployment.goplatform-api/internal/service/deployment_apikey_backfill_test.goplatform-api/internal/service/llm_apikey.goplatform-api/internal/service/llm_deployment.goplatform-api/internal/service/llm_proxy_apikey.goplatform-api/internal/service/mcp_deployment.goplatform-api/plugins/eventgateway/plugin.goplatform-api/plugins/eventgateway/service/webbroker_api_deployment.goplatform-api/plugins/eventgateway/service/websub_api_deployment.go
Comment on lines
+335
to
+375
| func BackfillAPIKeysToGateway(apiKeyRepo repository.APIKeyRepository, events *GatewayEventsService, slogger *slog.Logger, artifactUUID, gatewayID, actor string) { | ||
| if events == nil || apiKeyRepo == nil { | ||
| return | ||
| } | ||
|
|
||
| keys, err := apiKeyRepo.ListByArtifact(artifactUUID) | ||
| if err != nil { | ||
| if slogger != nil { | ||
| slogger.Warn("Failed to load API keys for deploy-time backfill", | ||
| "artifactId", artifactUUID, "gatewayId", gatewayID, "error", err) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| now := time.Now() | ||
| backfilled := 0 | ||
| for _, k := range keys { | ||
| if k == nil || k.Status != constants.APIKeyStatusActive { | ||
| continue | ||
| } | ||
| // Skip expired keys — never push a dead key to a gateway. | ||
| if k.ExpiresAt != nil && !k.ExpiresAt.After(now) { | ||
| continue | ||
| } | ||
|
|
||
| event := APIKeyCreatedEventFromModel(k) | ||
| if err := events.BroadcastAPIKeyCreatedEvent(gatewayID, actor, event); err != nil { | ||
| if slogger != nil { | ||
| slogger.Warn("Failed to backfill API key to gateway", | ||
| "artifactId", artifactUUID, "gatewayId", gatewayID, "keyName", k.Name, "error", err) | ||
| } | ||
| continue | ||
| } | ||
| backfilled++ | ||
| } | ||
|
|
||
| if backfilled > 0 && slogger != nil { | ||
| slogger.Info("Backfilled existing API keys to gateway at deploy time", | ||
| "artifactId", artifactUUID, "gatewayId", gatewayID, "count", backfilled) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm AllowedTargets semantics and that model.APIKey carries it.
rg -nP 'AllowedTargets' -g '*.go' platform-api/internal/model platform-api/internal/service | rg -n 'APIKey|filterAPIGateways'
ast-grep run --pattern 'func filterAPIGatewaysByAllowedTargets($$$) { $$$ }' --lang go platform-api/internal/service/apikey.goRepository: wso2/api-platform
Length of output: 1734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target-filter helper and the backfill function in context.
sed -n '150,210p' platform-api/internal/service/apikey.go
printf '\n---\n'
sed -n '335,375p' platform-api/internal/service/apikey.go
printf '\n---\n'
# Inspect the API key model for AllowedTargets and the gateway model fields used for matching.
rg -n "type APIKey|AllowedTargets|type APIGatewayWithDetails|Name string|GatewayID" platform-api/internal/model platform-api/internal/serviceRepository: wso2/api-platform
Length of output: 35752
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the API key created event payload and broadcast path.
sed -n '1,140p' platform-api/internal/model/apikey_event.go
printf '\n---\n'
sed -n '280,360p' platform-api/internal/service/gateway_events.go
printf '\n---\n'
# Inspect backfill tests for allowed-target behavior.
sed -n '1,260p' platform-api/internal/service/deployment_apikey_backfill_test.goRepository: wso2/api-platform
Length of output: 13589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the deployment call sites around the backfill invocation.
sed -n '380,450p' platform-api/internal/service/deployment.go
printf '\n---\n'
sed -n '520,575p' platform-api/internal/service/llm_deployment.go
printf '\n---\n'
sed -n '540,585p' platform-api/internal/service/mcp_deployment.go
printf '\n---\n'
sed -n '370,400p' platform-api/internal/service/llm_proxy_apikey.goRepository: wso2/api-platform
Length of output: 6082
Backfill must respect AllowedTargets. BackfillAPIKeysToGateway currently broadcasts every active, non-expired key to gatewayID, but the create paths filter by gateway Name via filterAPIGatewaysByAllowedTargets. Pass the target gateway name here, or filter before broadcasting, so restricted keys don’t reach other gateways.
🤖 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/internal/service/apikey.go` around lines 335 - 375,
BackfillAPIKeysToGateway must enforce each key’s AllowedTargets before
broadcasting. Use the target gateway name and the existing
filterAPIGatewaysByAllowedTargets logic (or equivalent) within
BackfillAPIKeysToGateway, and skip keys not permitted for that gateway while
preserving active and expiration checks.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git fetch origin pull/2588/head:pr2588 2>/dev/null || true
git log --oneline -5 pr2588 2>/dev/null || true
git show 5951ae0821d64393481168c51611ef52cc91e7d7 -- platform-api/internal/service/apikey.go 2>/dev/null | head -150Length of output: 8032
Confirmed — the fix in commit 5951ae0 adds gatewayRepo to BackfillAPIKeysToGateway, resolves the target gateway's name, and applies the shared gatewayAllowedByTargets check before broadcasting each key, exactly as requested.
@DinithHerath, thanks for the fix — verified it correctly enforces AllowedTargets during backfill now.
ᕦ(ò_óˇ)ᕤ
🧬 Code Graph Analysis Results
gateway/gateway-controller/pkg/api/management/generated.go (lines 539-569)
APIKey struct {
// ApiId Unique public identifier of the API that the key is associated with
ApiId string `json:"apiId" yaml:"apiId"`
// ApiKey Generated API key with apip_ prefix
ApiKey *string `json:"apiKey,omitempty" yaml:"apiKey,omitempty"`
// CreatedAt Timestamp when the API key was generated
CreatedAt time.Time `json:"createdAt" yaml:"createdAt"`
// CreatedBy Identifier of the user who generated the API key
CreatedBy string `json:"createdBy" yaml:"createdBy"`
// DisplayName Human-readable name for the API key (user-provided, mutable)
DisplayName *string `json:"displayName,omitempty" yaml:"displayName,omitempty"`
// ExpiresAt Expiration timestamp (null if no expiration)
ExpiresAt *time.Time `json:"expiresAt" yaml:"expiresAt"`
// ExternalRefId External reference ID for the API key
ExternalRefId *string `json:"externalRefId,omitempty" yaml:"externalRefId,omitempty"`
// Name URL-safe identifier for the API key (auto-generated from displayName, immutable, used as path parameter)
Name string `json:"name" yaml:"name"`
// Source Source of the API key (local or external)
Source APIKeySource `json:"source" yaml:"source"`
// Status Status of the API key
Status APIKeyStatus `json:"status" yaml:"status"`
}gateway/gateway-controller/pkg/api/management/generated.go (line 43)
APIKeyStatusActive APIKeyStatus = "active"platform-api/internal/service/artifact_dp_apikey_test.go (lines 59-61)
func (dpKeyAPIRepo) GetAPIGatewaysWithDetails(string, string) ([]*model.APIGatewayWithDetails, error) {
return []*model.APIGatewayWithDetails{{ID: "gw-1", Name: "gw-1"}}, nil
}platform-api/internal/service/artifact_dp_apikey_test.go (lines 68-70)
func (dpKeyNoAssocAPIRepo) GetAPIGatewaysWithDetails(string, string) ([]*model.APIGatewayWithDetails, error) {
return nil, nil
}platform-api/internal/service/artifact_dp_apikey_test.go (lines 79-82)
func (c *dpCapturingAPIKeyRepo) Create(k *model.APIKey) error {
c.created = k
return nil
}platform-api/internal/service/deployment_apikey_backfill_test.go (lines 57-59)
func (r *stubBackfillAPIKeyRepo) ListByArtifact(string) ([]*model.APIKey, error) {
return r.keys, r.err
}platform-api/internal/service/deployment_apikey_backfill_test.go (lines 68-70)
func (r *stubBackfillGatewayRepo) GetByUUID(id string) (*model.Gateway, error) {
return &model.Gateway{ID: id, Name: r.name}, nil
}
Thushani-Jayasekera
approved these changes
Jul 14, 2026
renuka-fernando
approved these changes
Jul 15, 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
3 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.
Description
This pull request introduces several improvements to how API keys are handled and broadcasted during artifact deployment and association, especially for LLM providers and proxies. The main focus is to ensure that API keys are only broadcast to gateways actually associated with an artifact, preventing unnecessary or incorrect broadcasts, and to immediately sync existing keys to gateways upon new associations. The changes also add comprehensive tests for these behaviors.
API Key Broadcast and Association Improvements:
BackfillAPIKeysToGatewayhelper, which (re)broadcasts all existing active API keys for an artifact to a gateway whenever the artifact is deployed or associated, closing the gap where a gateway might not recognize pre-existing keys until a later sync. This is now called in bothDeployAPIandRestoreDeploymentin theDeploymentService. [1] [2] [3] [4]APIKeyCreatedEventFromModelhelper to standardize event payload construction. [1] [2]filterAPIGatewaysByAllowedTargetshelper for filtering association-scoped gateway lists.Dependency Injection and Service Construction:
apiKeyRepointoDeploymentServiceand related API key services, enabling deployment-time key backfill. [1] [2] [3] [4] [5] [6]Testing Enhancements:
These changes significantly improve the correctness and reliability of API key distribution during artifact deployment and association.