Skip to content

[PlatformAPI] Add global policies support for LLM providers and proxies#2245

Open
ashera96 wants to merge 3 commits into
wso2:mainfrom
ashera96:pr/global-policy-llm-platform-api
Open

[PlatformAPI] Add global policies support for LLM providers and proxies#2245
ashera96 wants to merge 3 commits into
wso2:mainfrom
ashera96:pr/global-policy-llm-platform-api

Conversation

@ashera96

Copy link
Copy Markdown

Purpose

Introduces globalPolicies and operationPolicies as first-class fields on LLM provider and proxy deployment artifacts, replacing the flat policies list for new gateways.

Schema change. Artifacts now carry two distinct policy lists:

  • globalPolicies — applied uniformly across every operation (provider-wide quota, auth, CORS). For advanced-ratelimit, the platform-api automatically injects keyExtraction: [{type: "apiname"}] so the counter is shared across all operations rather than siloed per route.
  • operationPolicies — scoped to specific paths and HTTP methods, preserving per-operation behaviour.

deploymenttransform package (new). A version-aware artifact adaptation layer that sits between the generator and the wire format. Generators always produce the canonical latest shape; deploymenttransform downgrades the artifact for gateways below v1.2.0 (flattens both new lists into the legacy policies field and stamps the correct apiVersion). Adding a new version boundary in future is a one-file registration, not a service-layer change.

Version centralisation. constants.APIBasePath is now the single source of truth for the platform-api route prefix (/api/v1alpha2). All 24 handler files build their r.Group(...) call from this constant. A sync test locks the cross-language coupling between the OpenAPI servers: URL and the handler prefix so scope enforcement cannot silently mismatch on a future version bump.

Bug fix. Gateway version was not being resolved from the database when generating a deployment YAML, causing the version check to always fall back to the old path regardless of the actual target gateway version.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 831fc691-72a5-4de1-8f9d-bd74277af583

📥 Commits

Reviewing files that changed from the base of the PR and between 8581011 and d23bc9b.

📒 Files selected for processing (7)
  • platform-api/src/internal/constants/constants.go
  • platform-api/src/internal/deploymenttransform/policies.go
  • platform-api/src/internal/handler/api.go
  • platform-api/src/internal/handler/deployment.go
  • platform-api/src/internal/service/llm.go
  • platform-api/src/internal/service/llm_deployment.go
  • platform-api/src/resources/openapi.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
  • platform-api/src/internal/constants/constants.go
  • platform-api/src/internal/deploymenttransform/policies.go
  • platform-api/src/internal/handler/deployment.go
  • platform-api/src/internal/handler/api.go
  • platform-api/src/internal/service/llm.go
  • platform-api/src/resources/openapi.yaml
  • platform-api/src/internal/service/llm_deployment.go

📝 Walkthrough

Schema Extension: Global and Operation-Scoped Policies

The PR introduces globalPolicies and operationPolicies as first-class fields on LLM provider and proxy deployment artifacts, replacing the flat policies list for gateways at v1.2.0 and above. Global policies apply uniformly across all operations for provider-wide configuration (quota, authentication, CORS), while operation policies remain scoped to specific paths and HTTP methods. When a provider uses advanced-ratelimit at the global level, the platform-api automatically injects keyExtraction: [{type: "apiname"}] to maintain rate-limit counters shared across all operations rather than isolated per route.

Artifact Version Adaptation

A new deploymenttransform package handles version-aware artifact adaptation between the generator and wire formats. Generators produce the canonical latest schema shape. For gateways older than v1.2.0, the package automatically downconverts artifacts by flattening both policy lists into the legacy policies field, applying the correct apiVersion, and reordering policies when needed (e.g., ensuring llm-cost-based-ratelimit precedes llm-cost). This design allows future version boundary additions through simple one-file registration without modifying service layer code.

Centralized API Versioning

The PR establishes constants.APIBasePath as the single source of truth for the platform-api route prefix (/api/v1alpha2). All 24 handler files now build their route groups from this constant, separating the handler URL versioning axis from the gateway artifact apiVersion axis. A new synchronization test prevents cross-language coupling mismatches between the OpenAPI servers: URL and handler prefix on future version updates.

Database-Driven Gateway Version Resolution

A bug fix ensures gateway version is resolved from the database during deployment YAML generation, correcting version checks that previously always fell back to the legacy code path regardless of the target gateway's actual version. The ReceiveGatewayManifest service now persists controller-reported gateway versions via the new UpdateGatewayVersion repository method.

Policy Migration for Backward Compatibility

LLM provider and proxy services now support the new policy structure while maintaining backward compatibility. Legacy policies from requests are migrated on write into globalPolicies and operationPolicies based on their scope (wildcard paths become global, specific paths become operation-level). Read responses normalize stored policies into the canonical structure for API consumers, with legacy policies forced to nil in responses.

Code Quality Improvements

The constants file consolidates gateway artifact versioning: GatewayApiVersionV1Alpha1 for legacy downgrades and updated GatewayApiVersion for v1alpha2. Version-related constants are now logically grouped for improved readability.

Walkthrough

This PR upgrades the platform-api from v1/v1alpha1 to v1alpha2: all HTTP handler RegisterRoutes implementations replace hardcoded /api/v1 prefixes with constants.APIBasePath, gateway manifest apiVersion constants are updated, and default LLM provider template YAMLs are bumped to v1alpha2.

A new split policy model is introduced: globalPolicies (API-level) and operationPolicies (path-level) replace the deprecated unified policies field across OpenAPI schema, generated Go types, internal models, and deployment DTOs. LLM provider and proxy create/update operations map the new policy fields and call migration helpers that fold legacy Policies entries (rules: /* with method * becomes GlobalPolicy; specific paths become OperationPolicy) and clear the deprecated field before persistence. Read paths in mapProviderModelToAPI and mapProxyModelToAPI perform split-on-read from legacy Policies when canonical fields are empty, then force Policies to nil in API responses. A new deploymenttransform package provides a registry-based transformation system that conditionally down-converts the split policy shape back to the legacy unified format for gateway versions below 1.2.0. The gateway service now persists the controller-reported gateway version on manifest receipt.

Suggested Reviewers

  • tharindu1st
  • RakhithaRR
  • VirajSalaka
  • renuka-fernando
  • malinthaprasan
  • AnuGayan
  • Krishanx92
  • HeshanSudarshana
  • PasanT9
  • HiranyaKavishani
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers Purpose, Goals, and Approach with technical depth. However, required sections including User stories, Documentation, Automation tests, Security checks, Samples, Test environment, and Related PRs are missing or incomplete. Add missing sections: User stories, Documentation, Automation tests (unit/integration with coverage details), Security checks (secure coding/FindSecurityBugs/secrets verification), Samples, Related PRs, and Test environment details.
Docstring Coverage ⚠️ Warning Docstring coverage is 46.39% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main feature: adding global policies support for LLM providers and proxies, which aligns with the primary schema changes and deployment artifact structure throughout the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/internal/handler/api.go`:
- Line 570: The comment on the ImportAPIProject function documents an incorrect
API route path. Update the comment from `/api/v1alpha2/import/api-project` to
match the actual registered route path `/api/v1alpha2/api-projects/import`
(which is registered at lines 994-997). This ensures the function documentation
accurately reflects the actual HTTP endpoint being used.

In `@platform-api/src/internal/handler/deployment.go`:
- Line 47: Update the handler comments in the deployment.go file to align with
the actual registered routes. The comments for DeployAPI and other handlers (at
lines 47, 259, 308, and 353) currently reference the path
`/api/v1alpha2/apis/:apiId` but should be updated to
`/api/v1alpha2/rest-apis/:apiId` to match what is actually registered in
RegisterRoutes. Replace the old API path with the correct rest-apis path in all
handler comment strings while keeping the handler function names and HTTP
methods unchanged.

In `@platform-api/src/internal/service/llm.go`:
- Around line 1171-1194: The appendLegacyOperationPath function's matching logic
is incomplete. The function comment states it should merge paths into an
OperationPolicy with the same name AND version, but the condition at line
1183-1184 only checks if the Name matches. This causes incorrect behavior when
multiple policies with the same name but different versions exist. Update the
matching condition in the loop to check both (*policies)[i].Name equals the name
parameter AND (*policies)[i].Version equals the version parameter before
appending the path to that policy entry.

In `@platform-api/src/resources/openapi.yaml`:
- Around line 9870-9874: The OperationPolicyPath.methods field in the
openapi.yaml file has a description documenting the accepted HTTP methods but
lacks an enum constraint to enforce them at the API contract boundary. Add an
enum constraint to the OperationPolicyPath.methods field that includes the valid
values (GET, POST, PUT, DELETE, PATCH, OPTIONS, HEAD, and *) by referencing the
same enum pattern already used in LLMPolicyPath.methods. This will ensure
invalid method values are rejected during API validation rather than only
documented in the description.
🪄 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: 3fb34bd5-fca1-4e7c-8551-c4c02ccc6662

📥 Commits

Reviewing files that changed from the base of the PR and between 58e813e and 8581011.

📒 Files selected for processing (52)
  • platform-api/src/api/generated.go
  • platform-api/src/internal/constants/constants.go
  • platform-api/src/internal/deploymenttransform/policies.go
  • platform-api/src/internal/deploymenttransform/registry.go
  • platform-api/src/internal/deploymenttransform/transform_test.go
  • platform-api/src/internal/deploymenttransform/version.go
  • platform-api/src/internal/dto/llm_deployment.go
  • platform-api/src/internal/handler/api.go
  • platform-api/src/internal/handler/api_key.go
  • platform-api/src/internal/handler/apikey_user.go
  • platform-api/src/internal/handler/application.go
  • platform-api/src/internal/handler/deployment.go
  • platform-api/src/internal/handler/devportal.go
  • platform-api/src/internal/handler/gateway.go
  • platform-api/src/internal/handler/git.go
  • platform-api/src/internal/handler/llm.go
  • platform-api/src/internal/handler/llm_apikey.go
  • platform-api/src/internal/handler/llm_deployment.go
  • platform-api/src/internal/handler/llm_proxy_apikey.go
  • platform-api/src/internal/handler/mcp.go
  • platform-api/src/internal/handler/mcp_deployment.go
  • platform-api/src/internal/handler/organization.go
  • platform-api/src/internal/handler/project.go
  • platform-api/src/internal/handler/subscription_handler.go
  • platform-api/src/internal/handler/subscription_plan_handler.go
  • platform-api/src/internal/handler/webbroker_api.go
  • platform-api/src/internal/handler/webbroker_api_deployment.go
  • platform-api/src/internal/handler/webbroker_apikey.go
  • platform-api/src/internal/handler/websub_api.go
  • platform-api/src/internal/handler/websub_api_deployment.go
  • platform-api/src/internal/handler/websub_apikey.go
  • platform-api/src/internal/middleware/openapi_scope_registry_test.go
  • platform-api/src/internal/model/llm.go
  • platform-api/src/internal/repository/gateway.go
  • platform-api/src/internal/repository/interfaces.go
  • platform-api/src/internal/service/deployment_test.go
  • platform-api/src/internal/service/gateway.go
  • platform-api/src/internal/service/llm.go
  • platform-api/src/internal/service/llm_deployment.go
  • platform-api/src/internal/service/llm_deployment_test.go
  • platform-api/src/internal/service/llm_policy_migration_test.go
  • platform-api/src/internal/service/llm_test.go
  • platform-api/src/internal/utils/api.go
  • platform-api/src/internal/utils/api_test.go
  • platform-api/src/resources/default-llm-provider-templates/anthropic-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/awsbedrock-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/azureaifoundry-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/azureopenai-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/gemini-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/mistral-template.yaml
  • platform-api/src/resources/default-llm-provider-templates/openai-template.yaml
  • platform-api/src/resources/openapi.yaml

Comment thread platform-api/src/internal/handler/api.go Outdated
Comment thread platform-api/src/internal/handler/deployment.go Outdated
Comment thread platform-api/src/internal/service/llm.go
Comment thread platform-api/src/resources/openapi.yaml
@ashera96

Copy link
Copy Markdown
Author

@coderabbitai please review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat platform-api/src/internal/deploymenttransform/version.go
echo "---"
cat platform-api/src/internal/deploymenttransform/registry.go
echo "---"
cat platform-api/src/internal/deploymenttransform/policies.go
echo "---"
cat platform-api/src/internal/constants/constants.go
echo "---"
cat platform-api/src/internal/dto/llm_deployment.go
echo "---"
cat platform-api/src/internal/repository/gateway.go | tail -30

Length of output: 21308


🏁 Script executed:

#!/bin/bash
cat platform-api/src/internal/service/llm_deployment.go

Length of output: 63569


🏁 Script executed:

#!/bin/bash
cat platform-api/src/internal/service/llm.go | grep -n "migrateLegacy\|splitLegacy\|GlobalPolicies\|OperationPolicies" | head -60
echo "---"
cat platform-api/src/internal/service/gateway.go | grep -n -A5 -B5 "UpdateGatewayVersion"
echo "---"
# Check if there's a sync test for APIBasePath
fd -e go "sync\|base_path\|openapi" platform-api/src
echo "---"
cat platform-api/src/internal/middleware/openapi_scope_registry_test.go

Length of output: 6051



@ashera96 Here is the review for PR #2245.


Review Summary

The overall design is solid — the deploymenttransform registry pattern is clean, version-aware downconversion is well-tested, and centralizing APIBasePath eliminates a whole class of drift bugs. The points below are ordered by severity.


1. DeployLLMProxy silently swallows a SetCurrentWithDetails error

File: platform-api/src/internal/service/llm_deployment.go

In DeployLLMProvider, a failure from SetCurrentWithDetails is returned to the caller:

if _, err := s.deploymentRepo.SetCurrentWithDetails(...); err != nil {
    return nil, fmt.Errorf("failed to set deployment status for LLM provider: %w", err)
}

In DeployLLMProxy, the same call only logs a warning:

if _, err := s.deploymentRepo.SetCurrentWithDetails(...); err != nil {
    s.slogger.Warn("Failed to set deployment status for LLM proxy", "error", err)
}

If the status write fails, the response returns nil error, the broadcast fires, and the caller has no way to know the deployment record is inconsistent. This should match the provider behavior and return the error.


2. Missing deduplication check in generateLLMProxyDeploymentYAML

File: platform-api/src/internal/service/llm_deployment.go

generateLLMProviderDeploymentYAML guards against duplicate global policies when carrying through user-set entries:

for _, p := range provider.Configuration.GlobalPolicies {
    if hasGlobalPolicy(globalPolicies, p.Name) {
        continue  // dedup guard
    }
    ...
}

The corresponding section in generateLLMProxyDeploymentYAML has no such guard:

for _, p := range proxy.Configuration.GlobalPolicies {
    // no hasGlobalPolicy check
    entry := api.Policy{...}
    proxyGlobalPolicies = append(proxyGlobalPolicies, entry)
}

If a proxy has API-key security enabled and the same policy name in its explicit GlobalPolicies, the artifact will contain two api-key-auth entries. The fix is to add the same check:

for _, p := range proxy.Configuration.GlobalPolicies {
    if hasGlobalPolicy(proxyGlobalPolicies, p.Name) {
        continue
    }
    ...
}

3. Ambiguous error messages from shared transformation Name

File: platform-api/src/internal/deploymenttransform/policies.go

Both the LLMProvider and LLMProxy transformations are registered with the same Name:

defaultRegistry.Register(Transformation{
    Name: "policy-lists/downconvert-pre-1.2.0",
    Kind: constants.LLMProvider,
    ...
})
defaultRegistry.Register(Transformation{
    Name: "policy-lists/downconvert-pre-1.2.0",
    Kind: constants.LLMProxy,
    ...
})

When registry.go wraps errors, the output is deploymenttransform "policy-lists/downconvert-pre-1.2.0": <err> — the kind is not included. Distinguishing the affected artifact type from logs requires inspecting surrounding context. Consider using names like "policy-lists/downconvert-pre-1.2.0/llm-provider" and "policy-lists/downconvert-pre-1.2.0/llm-proxy".


4. withGlobalAdvancedRatelimitKeyExtraction does not normalize existing keyExtraction

File: platform-api/src/internal/service/llm_deployment.go

func withGlobalAdvancedRatelimitKeyExtraction(params map[string]interface{}) map[string]interface{} {
    if _, ok := params["keyExtraction"]; ok {
        return params  // passes through whatever the caller set
    }
    ...
}

If a user explicitly sets keyExtraction in their advanced-ratelimit global policy params (e.g., {type: routename}), the function silently retains it. The PR objective states that keyExtraction: [{type: "apiname"}] must be injected for global advanced-ratelimit to maintain a shared API-level counter. Consider replacing (or at minimum logging a warning) when the existing value differs from [{type: "apiname"}].


5. Minor: stale TODO: Temporary comment

File: platform-api/src/internal/service/llm_deployment.go

// TODO: Temporary
const (
    tokenBasedRateLimitPolicyName   = "token-based-ratelimit"
    advancedRateLimitPolicyName     = "advanced-ratelimit"
    ...
)

These constants are now used broadly across policy generation, migration, and the deploymenttransform package. If there is no active plan to relocate them, consider removing the TODO or converting them to a package-level constant block to avoid misleading future contributors.


6. Minor: ungrouped const declarations in constants.go

File: platform-api/src/internal/constants/constants.go

GatewayApiVersionV1Alpha1, GatewayApiVersion, APIVersion, and APIBasePath are declared as four separate const statements rather than being grouped together. Grouping them improves readability and makes the versioning axis self-documenting:

const (
    GatewayApiVersionV1Alpha1 = "gateway.api-platform.wso2.com/v1alpha1"
    GatewayApiVersion         = "gateway.api-platform.wso2.com/v1alpha2"
)

const (
    APIVersion  = "v1alpha2"
    APIBasePath = "/api/" + APIVersion
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant