Skip to content

[Platform-API] Store the Gateway Version #1283

Closed
VirajSalaka wants to merge 4 commits intowso2:mainfrom
VirajSalaka:gateway-version
Closed

[Platform-API] Store the Gateway Version #1283
VirajSalaka wants to merge 4 commits intowso2:mainfrom
VirajSalaka:gateway-version

Conversation

@VirajSalaka
Copy link
Copy Markdown
Contributor

@VirajSalaka VirajSalaka commented Feb 24, 2026

This pull request introduces support for a version field in the gateway model, API, and database schema. The changes ensure that gateway runtime version information is stored, retrieved, and updated throughout the platform, including API requests, responses, database operations, and test cases.

Gateway Version Support

  • Added a version field to the gateway database schema in both Postgres and SQLite, making it a required column for all gateway records. [1] [2]
  • Updated gateway-related API request and response structs (CreateGatewayRequest, UpdateGatewayRequest, GatewayResponse, RESTAPIGatewayResponse) to include the version field, allowing clients to specify and receive gateway version information. [1] [2] [3] [4]
  • Modified the gateway model (Gateway, APIGatewayWithDetails) and repository methods to handle the version field, including all relevant SQL queries and struct mappings. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]
  • Updated gateway service methods (RegisterGateway, UpdateGateway) and handler logic to support creating and updating gateways with a specified version, including validation and defaulting logic. [1] [2] [3] [4] [5] [6]
  • Set a default gateway version (0.9.0) for newly created gateways when not specified, and updated test cases to include the version field. [1] [2]

API and Handler Enhancements

  • Improved request binding logic in gateway handlers to better support partial updates and version field handling. [1] [2] [3]
  • Updated conversion utilities to ensure the version field is properly mapped in API responses.## Purpose

Explain why this feature or fix is required. Describe the underlying problems, issues, or needs driving this feature/fix and include links to related issues in the following format: Resolves issue1, issue2, etc.

Goals

Describe what solutions this feature or fix introduces to address the problems outlined above.

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI. Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

User stories

Summary of user stories addressed by this change>

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Summary by CodeRabbit

  • New Features

    • Gateway versioning: set a version when creating or updating gateways.
    • Version is returned in gateway responses and gateway detail views.
    • New default version applied for existing gateways where none provided.
  • Bug Fixes

    • Creation/update now validate version input and return proper validation errors when missing/invalid.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 24, 2026

Walkthrough

Adds gateway versioning: new version field across API schemas, models, DB (default '0.9.0'), handlers, service and repository logic; input validation updated; tests and OpenAPI updated.

Changes

Cohort / File(s) Summary
API Contracts & Generated Types
platform-api/src/api/generated.go, platform-api/src/resources/openapi.yaml
Added version/Version to CreateGatewayRequest, GatewayResponse, RESTAPIGatewayResponse, and UpdateGatewayRequest in API types and OpenAPI spec.
Internal Models
platform-api/src/internal/model/gateway.go
Added Version string to Gateway and APIGatewayWithDetails model structs.
Database Schema
platform-api/src/internal/database/schema.postgres.sql, platform-api/src/internal/database/schema.sqlite.sql
Added version TEXT NOT NULL DEFAULT '0.9.0' column to gateways table.
Repository & Queries
platform-api/src/internal/repository/gateway.go, platform-api/src/internal/repository/api.go, platform-api/src/internal/repository/api_deployments_test.go
INSERT/SELECT/UPDATE queries updated to include version; row scans populate model.Version; test fixtures updated to include version value.
Service Layer
platform-api/src/internal/service/gateway.go, platform-api/src/internal/service/api.go, platform-api/src/internal/service/gateway_properties_test.go, platform-api/src/internal/service/gateway_test.go
RegisterGateway/UpdateGateway signatures extended to accept version; validateGatewayInput adds version validation; mapping populates Version in API responses; tests updated for new param and behaviors.
Handler Layer
platform-api/src/internal/handler/gateway.go
CreateGateway and UpdateGateway handlers now forward req.Version to service methods and mirror validation error handling.
Misc / Module
go.mod
Module metadata touched (no functional change described).

Sequence Diagram

sequenceDiagram
    actor Client
    participant Handler as CreateGateway<br/>Handler
    participant Service as GatewayService
    participant Repo as GatewayRepository
    participant DB as Database

    Client->>Handler: POST /gateways (body includes version)
    Handler->>Service: RegisterGateway(..., version)
    Service->>Service: validateGatewayInput(version)
    alt invalid version
        Service-->>Handler: validation error
        Handler-->>Client: 400 Bad Request
    else valid version
        Service->>Repo: Create(gateway with version)
        Repo->>DB: INSERT INTO gateways (version, ...)
        DB-->>Repo: OK
        Repo-->>Service: gateway model (with version)
        Service->>Handler: GatewayResponse (includes version)
        Handler-->>Client: 201 Created (response)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through schemas, code, and test,
A tiny field tucked in the rest.
From POST to rows the version flows,
"0.9.0" now proudly shows.
Hooray—each gateway knows! 🎉

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete against the template; most required sections like Purpose, Goals, User stories, Documentation, Automation tests, and Security checks are either missing or unfilled. Fill in all required template sections: Purpose/Goals (why/what), User stories, Documentation impact, test coverage details, and security checklist with yes/no answers.
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: adding gateway version support to storage and retrieval.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@VirajSalaka VirajSalaka marked this pull request as ready for review February 25, 2026 07:55
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
platform-api/src/internal/database/schema.sqlite.sql (1)

81-81: Constrain version to non-empty at the database layer.

NOT NULL still allows empty strings. Adding a CHECK avoids persisting invalid empty versions through non-service paths.

💡 Suggested schema hardening
-    version TEXT NOT NULL default '0.9.0',
+    version TEXT NOT NULL DEFAULT '0.9.0' CHECK (length(trim(version)) > 0),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/database/schema.sqlite.sql` at line 81, The schema
allows an empty string in the version column despite NOT NULL; update the column
definition for the version field to enforce non-empty values at the DB level by
adding a CHECK constraint (e.g., CHECK(trim(version) <> '') or
CHECK(length(version) > 0) ) so the version TEXT NOT NULL default '0.9.0'
definition prevents empty strings; update any migration or create-table
statement that defines the version column to include this CHECK constraint.
platform-api/src/internal/service/gateway.go (1)

61-64: Apply create-time defaulting in the service before validation.

Line 63 validates version before Line 90 normalizes it. This makes the create flow depend on caller-side default injection. Moving defaulting into the service keeps business behavior consistent across call paths.

🔧 Suggested service hardening
 func (s *GatewayService) RegisterGateway(orgID, name, displayName, description, vhost string, isCritical bool,
 	functionalityType string, properties map[string]interface{}, version string) (*api.GatewayResponse, error) {
+	gatewayVersion := strings.TrimSpace(version)
+	if gatewayVersion == "" {
+		gatewayVersion = "0.9.0"
+	}
+
 	// 1. Validate inputs
-	if err := s.validateGatewayInput(orgID, name, displayName, vhost, functionalityType, version); err != nil {
+	if err := s.validateGatewayInput(orgID, name, displayName, vhost, functionalityType, gatewayVersion); err != nil {
 		return nil, err
 	}
@@
-	gatewayVersion := strings.TrimSpace(version)
-
 	// 5. Create Gateway model

Also applies to: 90-90

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/service/gateway.go` around lines 61 - 64, The
service validates the incoming version before applying create-time
defaulting/normalization, so move the defaulting logic into the service: in the
gateway creation method (the function with signature returning
(*api.GatewayResponse, error)), apply the version default/normalization (the
existing normalization logic currently located after validation at line ~90) to
the version parameter before calling s.validateGatewayInput(orgID, name,
displayName, vhost, functionalityType, version); then pass the normalized
version into validateGatewayInput and the rest of the flow (ensure you update
any local variable that holds version accordingly and keep the existing
normalize/default behavior intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@platform-api/src/internal/database/schema.postgres.sql`:
- Line 81: Add a migration that alters the existing gateways table to include
the new non-nullable version column with the same default used in the CREATE
TABLE (i.e., add a migration that runs ALTER TABLE gateways ADD COLUMN version
TEXT NOT NULL DEFAULT '0.9.0';), ensure the migration is idempotent or checks
for the column before adding, and wire it into the project's migration sequence
so existing deployments receive the column prior to any code that reads/writes
gateways.version.

In `@platform-api/src/internal/handler/gateway.go`:
- Line 240: The current handler treats all errors from
h.gatewayService.UpdateGateway as 500; change the error handling in the gateway
update handler (the call to h.gatewayService.UpdateGateway(gatewayId, orgId,
req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version))
to detect validation/client errors returned by the service and respond with HTTP
400 (Bad Request) instead of 500, while other errors remain 500. Also ensure the
error is logged at Error level via the request handler's slog logger (e.g.,
h.logger.Error or equivalent) including the error value and context (gatewayId,
orgId, version) for both 4xx and 5xx cases.

In `@platform-api/src/internal/service/gateway.go`:
- Around line 203-208: The current update silently ignores an explicitly
provided whitespace-only version; instead validate and reject it: in the update
handler where you check version (the block that trims and sets gateway.Version),
after trimming (strings.TrimSpace(*version)) if the trimmed result is empty
return a validation error indicating "version cannot be blank" (do not set
gateway.Version), otherwise set gateway.Version = trimmedVersion; ensure the
handler returns the appropriate HTTP 4xx validation response so callers see the
failure.

In `@platform-api/src/resources/openapi.yaml`:
- Around line 3532-3535: The version property currently allows any string
(including whitespace); update the OpenAPI schemas for CreateGatewayRequest and
UpdateGatewayRequest to enforce stricter validation on the version field by
adding a minLength: 1 and a pattern that matches the server's expected version
format (e.g., a semantic version regex such as one that requires
major.minor.patch like ^\d+\.\d+\.\d+(-[A-Za-z0-9.+-]+)?$). Apply these
constraints to the "version" property in both CreateGatewayRequest and
UpdateGatewayRequest so clients cannot submit empty/whitespace-only or
non-semver values.

---

Nitpick comments:
In `@platform-api/src/internal/database/schema.sqlite.sql`:
- Line 81: The schema allows an empty string in the version column despite NOT
NULL; update the column definition for the version field to enforce non-empty
values at the DB level by adding a CHECK constraint (e.g., CHECK(trim(version)
<> '') or CHECK(length(version) > 0) ) so the version TEXT NOT NULL default
'0.9.0' definition prevents empty strings; update any migration or create-table
statement that defines the version column to include this CHECK constraint.

In `@platform-api/src/internal/service/gateway.go`:
- Around line 61-64: The service validates the incoming version before applying
create-time defaulting/normalization, so move the defaulting logic into the
service: in the gateway creation method (the function with signature returning
(*api.GatewayResponse, error)), apply the version default/normalization (the
existing normalization logic currently located after validation at line ~90) to
the version parameter before calling s.validateGatewayInput(orgID, name,
displayName, vhost, functionalityType, version); then pass the normalized
version into validateGatewayInput and the rest of the flow (ensure you update
any local variable that holds version accordingly and keep the existing
normalize/default behavior intact).

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0eef58a and 7591954.

📒 Files selected for processing (12)
  • platform-api/src/api/generated.go
  • platform-api/src/internal/database/schema.postgres.sql
  • platform-api/src/internal/database/schema.sqlite.sql
  • platform-api/src/internal/handler/gateway.go
  • platform-api/src/internal/model/gateway.go
  • platform-api/src/internal/repository/api.go
  • platform-api/src/internal/repository/api_deployments_test.go
  • platform-api/src/internal/repository/gateway.go
  • platform-api/src/internal/service/api.go
  • platform-api/src/internal/service/gateway.go
  • platform-api/src/internal/service/gateway_properties_test.go
  • platform-api/src/resources/openapi.yaml

is_critical BOOLEAN DEFAULT FALSE,
gateway_functionality_type VARCHAR(20) DEFAULT 'regular' NOT NULL,
is_active BOOLEAN DEFAULT FALSE,
version TEXT NOT NULL default '0.9.0',
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Feb 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) List SQL files under database area.
echo "=== SQL files in database area ==="
fd -t f '\.sql$' platform-api/src/internal/database 2>/dev/null || echo "Directory or files not found"

# 2) Find DDL touching gateways.version (creation + alteration).
echo -e "\n=== DDL statements touching gateways.version ==="
rg -n --type-add 'sql:*.sql' --type sql \
  'CREATE TABLE IF NOT EXISTS gateways|ALTER TABLE\s+gateways|ADD COLUMN\s+version' \
  platform-api/src/internal/database 2>/dev/null || echo "No matches found"

# 3) Surface possible migration directories across repo.
echo -e "\n=== SQL files with migration/migrate/schema patterns ==="
fd -t f '\.sql$' 2>/dev/null | rg -n 'migration|migrate|schema' || echo "No migration patterns found"

# 4) Additional search: look for any migration-related structures
echo -e "\n=== Migration directories structure ==="
fd -t d 'migration|migrate' 2>/dev/null || echo "No migration directories found"

Repository: wso2/api-platform

Length of output: 895


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check init-platform-api-db.sql
echo "=== init-platform-api-db.sql content ==="
cat -n platform-api/src/internal/database/init-platform-api-db.sql

echo -e "\n=== schema.postgres.sql - gateways table section (lines 70-85) ==="
sed -n '70,85p' platform-api/src/internal/database/schema.postgres.sql | cat -n

# Search for any ALTER TABLE in all SQL files
echo -e "\n=== Check for ALTER TABLE across all SQL files ==="
rg -n 'ALTER TABLE' platform-api/src/internal/database/ || echo "No ALTER TABLE found"

# Check if there's any conditional table creation/upgrade logic
echo -e "\n=== Search for upgrade/migration logic ==="
rg -n -i 'upgrade|migration|version.*column|alter.*gateways' platform-api/src/internal/database/ || echo "No upgrade logic found"

Repository: wso2/api-platform

Length of output: 2013


Add an ALTER TABLE migration for the version column in gateways table.

The schema file adds version TEXT NOT NULL default '0.9.0' to the gateways table definition using CREATE TABLE IF NOT EXISTS. This only affects fresh database installations; existing deployments retain their gateways table without this column. No migration script exists in the repository to add this column to pre-existing tables.

When the application attempts to INSERT, UPDATE, or SELECT the version column, existing deployments will encounter runtime SQL errors. An explicit ALTER TABLE gateways ADD COLUMN version TEXT NOT NULL DEFAULT '0.9.0'; migration is required for seamless upgrades.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/database/schema.postgres.sql` at line 81, Add a
migration that alters the existing gateways table to include the new
non-nullable version column with the same default used in the CREATE TABLE
(i.e., add a migration that runs ALTER TABLE gateways ADD COLUMN version TEXT
NOT NULL DEFAULT '0.9.0';), ensure the migration is idempotent or checks for the
column before adding, and wire it into the project's migration sequence so
existing deployments receive the column prior to any code that reads/writes
gateways.version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will handle when migrating the actual database explicitly.

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.

@VirajSalaka, understood—I've noted that you'll handle the migration explicitly during database deployment.

For reference, if you need idempotent migration logic in the future, PostgreSQL's ALTER TABLE ... SET NOT NULL "silently does nothing if the column already has a not-null constraint", and you can check information_schema.columns before adding columns to make migrations safe to run multiple times. But for this PR, manual handling during deployment works as well.


🧠 Learnings used
Learnt from: Arshardh
Repo: wso2/api-platform PR: 841
File: platform-api/src/internal/database/schema.postgres.sql:345-423
Timestamp: 2026-02-01T19:10:45.800Z
Learning: In SQL schema files under the platform-api module, avoid relying on database-level foreign key cascades to handle orphaned LLM policies. Ensure llm_policies references only organization_uuid and do not configure ON DELETE CASCADE for related tables (e.g., llm_providers, llm_proxies). Orphan handling should be implemented at the application layer for this domain to preserve schema flexibility.

Learnt from: RakhithaRR
Repo: wso2/api-platform PR: 989
File: platform-api/src/internal/database/schema.postgres.sql:40-53
Timestamp: 2026-02-09T05:23:35.668Z
Learning: In platform-api/src/internal/database/schema.postgres.sql, ensure the artifacts table enforces a global unique constraint on (name, version, organization_uuid) across all artifact kinds (e.g., RestApi, LLMProvider, LLMProxy). This prevents different artifact kinds within the same organization from sharing the same name+version. If this constraint is not present, add a unique constraint like UNIQUE(name, version, organization_uuid) or a composite unique index, and ensure existing data respects it before applying to maintain data integrity.

}

response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties)
response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version)
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.

⚠️ Potential issue | 🟠 Major

Map update validation errors to 400 instead of 500.

UpdateGateway now accepts version; validation failures from the service should return Bad Request, not Internal Server Error.

✅ Suggested fix
 response, err := h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description, req.DisplayName, req.IsCritical, req.Properties, req.Version)
 if err != nil {
 	if errors.Is(err, constants.ErrGatewayNotFound) {
 		h.slogger.Error("Gateway not found during update", "error", err)
 		c.JSON(http.StatusNotFound, utils.NewErrorResponse(404, "Not Found",
 			"Gateway not found"))
 		return
 	}
+	errMsg := err.Error()
+	if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") ||
+		strings.Contains(errMsg, "must") || strings.Contains(errMsg, "cannot") {
+		h.slogger.Error("Invalid gateway update request", "error", err)
+		c.JSON(http.StatusBadRequest, utils.NewErrorResponse(400, "Bad Request", errMsg))
+		return
+	}
 	h.slogger.Error("Failed to update gateway", "error", err)
 	c.JSON(http.StatusInternalServerError, utils.NewErrorResponse(500, "Internal Server Error",
 		"Failed to update gateway"))
 	return
 }
Based on learnings: "In the platform-api Go codebase, ensure that all errors—including expected client errors like 4xx responses—are logged at the Error level using slog.Logger."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/handler/gateway.go` at line 240, The current
handler treats all errors from h.gatewayService.UpdateGateway as 500; change the
error handling in the gateway update handler (the call to
h.gatewayService.UpdateGateway(gatewayId, orgId, req.Description,
req.DisplayName, req.IsCritical, req.Properties, req.Version)) to detect
validation/client errors returned by the service and respond with HTTP 400 (Bad
Request) instead of 500, while other errors remain 500. Also ensure the error is
logged at Error level via the request handler's slog logger (e.g.,
h.logger.Error or equivalent) including the error value and context (gatewayId,
orgId, version) for both 4xx and 5xx cases.

Comment on lines +203 to +208
if version != nil {
trimmedVersion := strings.TrimSpace(*version)
if trimmedVersion != "" {
gateway.Version = trimmedVersion
}
}
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.

⚠️ Potential issue | 🟡 Minor

Reject blank version updates instead of silently no-oping.

If clients send version explicitly but as whitespace, the API returns success without applying that field. Returning a validation error is clearer and avoids misleading successful updates.

✅ Suggested validation behavior
 	if version != nil {
 		trimmedVersion := strings.TrimSpace(*version)
-		if trimmedVersion != "" {
-			gateway.Version = trimmedVersion
-		}
+		if trimmedVersion == "" {
+			return nil, errors.New("version cannot be empty")
+		}
+		gateway.Version = trimmedVersion
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if version != nil {
trimmedVersion := strings.TrimSpace(*version)
if trimmedVersion != "" {
gateway.Version = trimmedVersion
}
}
if version != nil {
trimmedVersion := strings.TrimSpace(*version)
if trimmedVersion == "" {
return nil, errors.New("version cannot be empty")
}
gateway.Version = trimmedVersion
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/service/gateway.go` around lines 203 - 208, The
current update silently ignores an explicitly provided whitespace-only version;
instead validate and reject it: in the update handler where you check version
(the block that trims and sets gateway.Version), after trimming
(strings.TrimSpace(*version)) if the trimmed result is empty return a validation
error indicating "version cannot be blank" (do not set gateway.Version),
otherwise set gateway.Version = trimmedVersion; ensure the handler returns the
appropriate HTTP 4xx validation response so callers see the failure.

Comment on lines +3532 to +3535
version:
type: string
description: Gateway runtime version
example: "0.9.0"
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.

⚠️ Potential issue | 🟡 Minor

Constrain version in request schemas to match server-side expectations.

Line 3532 and Line 3626 currently allow any string, including whitespace-only values. Tightening this in OpenAPI reduces client/server contract drift and prevents avoidable 400s.

📌 Suggested OpenAPI adjustment
         version:
           type: string
+          minLength: 1
+          pattern: '.*\S.*'
           description: Gateway runtime version

Apply the same constraints in both CreateGatewayRequest and UpdateGatewayRequest.

Also applies to: 3626-3629

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/resources/openapi.yaml` around lines 3532 - 3535, The
version property currently allows any string (including whitespace); update the
OpenAPI schemas for CreateGatewayRequest and UpdateGatewayRequest to enforce
stricter validation on the version field by adding a minLength: 1 and a pattern
that matches the server's expected version format (e.g., a semantic version
regex such as one that requires major.minor.patch like
^\d+\.\d+\.\d+(-[A-Za-z0-9.+-]+)?$). Apply these constraints to the "version"
property in both CreateGatewayRequest and UpdateGatewayRequest so clients cannot
submit empty/whitespace-only or non-semver values.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
platform-api/src/internal/service/gateway_test.go (1)

182-192: Add a whitespace-only version test case.

You validate empty version, but not " ". Adding this case will prevent trim-related regressions in input handling.

Proposed test case addition
 		{
 			name:              "empty version",
 			orgID:             "123e4567-e89b-12d3-a456-426614174000",
 			gatewayName:       "prod-gateway-01",
 			displayName:       "Production Gateway 01",
 			vhost:             "api.example.com",
 			functionalityType: constants.GatewayFunctionalityTypeRegular,
 			version:           "",
 			wantErr:           true,
 			errContains:       "version is required",
 		},
+		{
+			name:              "whitespace-only version",
+			orgID:             "123e4567-e89b-12d3-a456-426614174000",
+			gatewayName:       "prod-gateway-01",
+			displayName:       "Production Gateway 01",
+			vhost:             "api.example.com",
+			functionalityType: constants.GatewayFunctionalityTypeRegular,
+			version:           "   ",
+			wantErr:           true,
+			errContains:       "version is required",
+		},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/service/gateway_test.go` around lines 182 - 192,
Add a test case that covers a whitespace-only version to prevent trim-related
regressions: in the table-driven tests (the struct with fields like name, orgID,
gatewayName, displayName, vhost, functionalityType, version, wantErr,
errContains) add a new case (e.g., name: "whitespace-only version") where
version is set to "   " (spaces) and wantErr is true with errContains "version
is required" so the same validation that rejects empty string also rejects
whitespace-only input.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@platform-api/src/internal/service/gateway_test.go`:
- Around line 182-192: Add a test case that covers a whitespace-only version to
prevent trim-related regressions: in the table-driven tests (the struct with
fields like name, orgID, gatewayName, displayName, vhost, functionalityType,
version, wantErr, errContains) add a new case (e.g., name: "whitespace-only
version") where version is set to "   " (spaces) and wantErr is true with
errContains "version is required" so the same validation that rejects empty
string also rejects whitespace-only input.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7591954 and f6c8d06.

📒 Files selected for processing (1)
  • platform-api/src/internal/service/gateway_test.go

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
platform-api/src/internal/handler/gateway.go (1)

248-254: ⚠️ Potential issue | 🟡 Minor

Inconsistent validation-error keywords between CreateGateway and UpdateGateway.

CreateGateway (line 101-102) also matches "cannot" in its string-based validation check, but this block omits it. A service-layer error containing only "cannot" would return 400 on create but 500 on update.

Proposed fix
-		if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must")  {
+		if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must") || strings.Contains(errMsg, "cannot") {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@platform-api/src/internal/handler/gateway.go` around lines 248 - 254, The
UpdateGateway validation check is missing the same "cannot" keyword used in
CreateGateway, causing inconsistent 400 vs 500 responses; update the
error-string inspection in the UpdateGateway handler (the block that inspects
err.Error() and calls h.slogger.Error / c.JSON) to include "cannot" alongside
"required", "invalid", and "must" so validation errors containing "cannot"
return HTTP 400 just like in CreateGateway.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@platform-api/src/internal/handler/gateway.go`:
- Around line 248-254: The UpdateGateway validation check is missing the same
"cannot" keyword used in CreateGateway, causing inconsistent 400 vs 500
responses; update the error-string inspection in the UpdateGateway handler (the
block that inspects err.Error() and calls h.slogger.Error / c.JSON) to include
"cannot" alongside "required", "invalid", and "must" so validation errors
containing "cannot" return HTTP 400 just like in CreateGateway.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6c8d06 and a30b539.

📒 Files selected for processing (1)
  • platform-api/src/internal/handler/gateway.go

}
// Similar to how it is handled in CreateGateway, check for validation errors and return 400 Bad Request
errMsg := err.Error()
if strings.Contains(errMsg, "required") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "must") {
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.

String match is fragile, what if we check error is

var ErrValidation = errors.New("validation error")

// In validation logic:
return fmt.Errorf("%w: name is required", ErrValidation)

// In handler:
if errors.Is(err, ErrValidation) {
    c.JSON(http.StatusBadRequest, ...)
    return
}

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.

2 participants