From a766a0e3a261b5934000714fd06e5cc659d0468c Mon Sep 17 00:00:00 2001 From: shivay Date: Wed, 13 May 2026 15:41:24 +0530 Subject: [PATCH 1/4] fix: Added a flag and check to reduce deduplication while generating services in paths. --- cmd/openapi2kong.go | 11 ++ .../30-reuse-identical-services.expected.json | 73 +++++++++ .../30-reuse-identical-services.yaml | 53 +++++++ ...reuse-with-plugins-on-routes.expected.json | 83 ++++++++++ .../31-reuse-with-plugins-on-routes.yaml | 43 +++++ openapi2kong/openapi2kong.go | 117 +++++++++++++- openapi2kong/openapi2kong_test.go | 148 ++++++++++++++++++ 7 files changed, 527 insertions(+), 1 deletion(-) create mode 100644 openapi2kong/oas3_testfiles/30-reuse-identical-services.expected.json create mode 100644 openapi2kong/oas3_testfiles/30-reuse-identical-services.yaml create mode 100644 openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.expected.json create mode 100644 openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.yaml diff --git a/cmd/openapi2kong.go b/cmd/openapi2kong.go index c66f842..2c29267 100644 --- a/cmd/openapi2kong.go +++ b/cmd/openapi2kong.go @@ -85,6 +85,14 @@ func executeOpenapi2Kong(cmd *cobra.Command, _ []string) error { } } + var reuseServices bool + { + reuseServices, err = cmd.Flags().GetBool("reuse-services") + if err != nil { + return fmt.Errorf("failed getting cli argument 'reuse-services'; %w", err) + } + } + options := openapi2kong.O2kOptions{ Tags: entityTags, DocName: docName, @@ -92,6 +100,7 @@ func executeOpenapi2Kong(cmd *cobra.Command, _ []string) error { IgnoreSecurityErrors: ignoreSecurityErrors, InsoCompat: insoCompatibility, IgnoreCircularRefs: ignoreCircularRefs, + ReuseServices: reuseServices, } trackInfo := deckformat.HistoryNewEntry("openapi2kong") @@ -147,4 +156,6 @@ directive from the file)`) openapi2kongCmd.Flags().BoolP("ignore-security-errors", "", false, "ignore errors for unsupported security schemes") openapi2kongCmd.Flags().BoolP("inso-compatible", "", false, "generate the config in an Inso compatible way") openapi2kongCmd.Flags().BoolP("ignore-circular-refs", "", false, "ignore circular references in the spec") + openapi2kongCmd.Flags().BoolP("reuse-services", "", false, "reuse services when multiple paths have identical "+ + "server configurations and no path-level plugins") } diff --git a/openapi2kong/oas3_testfiles/30-reuse-identical-services.expected.json b/openapi2kong/oas3_testfiles/30-reuse-identical-services.expected.json new file mode 100644 index 0000000..4f0c3da --- /dev/null +++ b/openapi2kong/oas3_testfiles/30-reuse-identical-services.expected.json @@ -0,0 +1,73 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "e2a5410a-10a5-5983-943e-53d9907339ec", + "name": "test-api_user", + "path": "/userservice/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "6f644a83-6865-5618-91a5-469612a425d3", + "methods": [ + "POST" + ], + "name": "test-api_user_post", + "paths": [ + "~/user$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_30-reuse-identical-services.yaml" + ] + }, + { + "id": "13a57c04-8579-569a-a0ba-25219440b03d", + "methods": [ + "GET" + ], + "name": "test-api_users_get", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_30-reuse-identical-services.yaml" + ] + }, + { + "id": "5f4a62d1-a71f-54e8-8cf5-ff8a3fab4d32", + "methods": [ + "GET" + ], + "name": "test-api_users-id_get", + "paths": [ + "~/users/(?[^#?/]+)$" + ], + "plugins": [], + "regex_priority": 100, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_30-reuse-identical-services.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_30-reuse-identical-services.yaml" + ] + } + ], + "upstreams": [] +} + diff --git a/openapi2kong/oas3_testfiles/30-reuse-identical-services.yaml b/openapi2kong/oas3_testfiles/30-reuse-identical-services.yaml new file mode 100644 index 0000000..1c13fab --- /dev/null +++ b/openapi2kong/oas3_testfiles/30-reuse-identical-services.yaml @@ -0,0 +1,53 @@ +# Test: Service reuse when identical path-level servers exist +# +# When multiple paths define the same path-level servers, only ONE service +# should be created with all routes attached to it. +# +# Expected behavior: +# - /users, /users/{id}, and /user all have identical path-level servers +# - Only 1 service should be created (not 3) +# - All 3 routes should be attached to the single service + +openapi: 3.0.3 +info: + title: Test API + version: 1.0.0 + description: This is a test API for service reuse demonstration. + +x-test-config: + reuseServices: true + +paths: + /users: + get: + summary: Get a list of users + responses: + "200": + description: A list of users + servers: + - url: https://api.example.com/userservice/v1 + + /users/{id}: + get: + summary: Get a user by ID + parameters: + - name: id + in: path + required: true + schema: + type: integer + responses: + "200": + description: A single user + servers: + - url: https://api.example.com/userservice/v1 + + /user: + post: + summary: Create a new user + responses: + "201": + description: User created successfully + servers: + - url: https://api.example.com/userservice/v1 + diff --git a/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.expected.json b/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.expected.json new file mode 100644 index 0000000..be91dbe --- /dev/null +++ b/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.expected.json @@ -0,0 +1,83 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "3e49c123-65d4-5b46-b6a1-bf0eb2c46077", + "name": "multi-service-api-with-plugins_orders", + "path": "/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "ca924a13-bcab-5886-a4f8-2387eb92a747", + "methods": [ + "GET" + ], + "name": "multi-service-api-with-plugins_orders_get", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_31-reuse-with-plugins-on-routes.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_31-reuse-with-plugins-on-routes.yaml" + ] + }, + { + "host": "api.example.com", + "id": "0c8d6ae0-1b14-5264-8135-fc544d8b89a9", + "name": "multi-service-api-with-plugins_users", + "path": "/v1", + "plugins": [ + { + "config": { + "minute": 100, + "policy": "local" + }, + "id": "1bcd1368-3e5b-5d79-8526-5caba1cd1eb0", + "name": "rate-limiting", + "tags": [ + "OAS3_import", + "OAS3file_31-reuse-with-plugins-on-routes.yaml" + ] + } + ], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "af8b1575-6102-56e5-9ad9-9a2220ba8c29", + "methods": [ + "GET" + ], + "name": "multi-service-api-with-plugins_users_get", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_31-reuse-with-plugins-on-routes.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_31-reuse-with-plugins-on-routes.yaml" + ] + } + ], + "upstreams": [] +} diff --git a/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.yaml b/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.yaml new file mode 100644 index 0000000..d918cb3 --- /dev/null +++ b/openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.yaml @@ -0,0 +1,43 @@ +# Test: NO service reuse when path-level plugins exist +# +# When a path has x-kong-plugin-* extensions, we should NOT reuse +# services even if server configurations are identical. This prevents +# unintended plugin propagation to other routes. +# +# Expected behavior: +# - /users has a rate-limiting plugin +# - /orders has NO plugin but same server +# - 2 separate services should be created (not 1) +# - This ensures rate-limiting doesn't accidentally apply to /orders + +openapi: 3.0.3 +info: + title: Multi-Service API with Plugins + version: 1.0.0 + +x-test-config: + reuseServices: true + +paths: + /users: + get: + summary: Get users (rate limited) + responses: + "200": + description: A list of users + servers: + - url: https://api.example.com/v1 + x-kong-plugin-rate-limiting: + config: + minute: 100 + policy: local + + /orders: + get: + summary: Get orders (no rate limiting) + responses: + "200": + description: A list of orders + servers: + - url: https://api.example.com/v1 + diff --git a/openapi2kong/openapi2kong.go b/openapi2kong/openapi2kong.go index 1c8bba0..42e2b20 100644 --- a/openapi2kong/openapi2kong.go +++ b/openapi2kong/openapi2kong.go @@ -53,6 +53,10 @@ type O2kOptions struct { TreatAllHeadersAsRequired bool // Skip generation of separate routes for header parameter enums SkipRouteByHeader bool + // Enable service reuse: when multiple paths have identical server configurations + // and no path-level plugins, they will share a single Kong service instead of + // creating duplicate services. This reduces resource bloat in Kong. + ReuseServices bool } // setDefaults sets the defaults for the OpenAPI2Kong operation. @@ -547,6 +551,75 @@ func constructHeaderCombinationsForRouting(headers []*v3.Parameter) []map[string return result } +// generateServiceKey creates a unique key for a service configuration based on +// servers, service defaults, and upstream defaults. This is used to identify +// identical service configurations for potential reuse. +func generateServiceKey(servers []*v3.Server, serviceDefaults []byte, upstreamDefaults []byte) string { + var sb strings.Builder + + // Add sorted server URLs to the key + serverURLs := make([]string, 0, len(servers)) + for _, server := range servers { + serverURLs = append(serverURLs, server.URL) + } + sort.Strings(serverURLs) + for _, url := range serverURLs { + sb.WriteString(url) + sb.WriteString("|") + } + + sb.WriteString("||") + + // Add service defaults + if serviceDefaults != nil { + sb.Write(serviceDefaults) + } + sb.WriteString("||") + + // Add upstream defaults + if upstreamDefaults != nil { + sb.Write(upstreamDefaults) + } + + return sb.String() +} + +// hasPathLevelPlugins checks if a path item has any x-kong-plugin-* extensions +func hasPathLevelPlugins(extensions *orderedmap.Map[string, *yaml.Node]) bool { + if extensions == nil { + return false + } + + pair := extensions.First() + for pair != nil { + if strings.HasPrefix(pair.Key(), "x-kong-plugin-") { + return true + } + pair = pair.Next() + } + return false +} + +// serviceCache holds cached services for reuse when identical configurations are found +type serviceCache struct { + services map[string]map[string]interface{} +} + +func newServiceCache() *serviceCache { + return &serviceCache{ + services: make(map[string]map[string]interface{}), + } +} + +func (c *serviceCache) get(key string) (map[string]interface{}, bool) { + service, found := c.services[key] + return service, found +} + +func (c *serviceCache) set(key string, service map[string]interface{}) { + c.services[key] = service +} + // MustConvert is the same as Convert, but will panic if an error is returned. func MustConvert(content []byte, opts O2kOptions) map[string]interface{} { result, err := Convert(content, opts) @@ -615,6 +688,9 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { nameConcatChar = "_" } + // Initialize service cache for reuse of identical service configurations + svcCache := newServiceCache() + // Load and parse the OAS file openapiDoc, err := libopenapi.NewDocument(content) if err != nil { @@ -852,6 +928,23 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { newPathService = true } + // Check if we can reuse an existing service. + // We only reuse services if: + // 1. ReuseServices option is enabled + // 2. A new path service would be created (newPathService is true) + // 3. The path has no path-level plugins (to avoid plugin conflicts) + // 4. An identical service configuration already exists in the cache + var reusedPathService bool + if opts.ReuseServices && newPathService && !hasPathLevelPlugins(pathitem.Extensions) { + serviceKey := generateServiceKey(pathServers, pathServiceDefaults, pathUpstreamDefaults) + if cachedService, found := svcCache.get(serviceKey); found { + logbasics.Debug("reusing existing service for path", "path", pathKey, "service", cachedService["name"]) + pathService = cachedService + reusedPathService = true + newPathService = false // Mark as not new since we're reusing + } + } + // create a new service if we need to do so if newPathService { // create the path-level service and (optional) upstream @@ -895,6 +988,23 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { pathService["host"] = docService["host"] } } + + // Cache the new service for potential reuse by subsequent paths (only if enabled and no plugins) + if opts.ReuseServices && !hasPathLevelPlugins(pathitem.Extensions) { + serviceKey := generateServiceKey(pathServers, pathServiceDefaults, pathUpstreamDefaults) + svcCache.set(serviceKey, pathService) + } + } else if reusedPathService { + // We're reusing an existing service, so collect path plugins separately. + // Path plugins will flow to routes via operationPluginList. + pathPluginList, err = getPluginsList(pathitem.Extensions, componentExtensions, nil, + opts.UUIDNamespace, pathBaseName, kongComponents, kongTags, opts.SkipID) + if err != nil { + return nil, fmt.Errorf("failed to create plugins list from path item: %w", err) + } + + // Extract the request-validator config from the plugin list + pathValidatorConfig, pathPluginList = getValidatorPlugin(pathPluginList, docValidatorConfig) } else { // no new path-level service entity required, so stick to the doc-level one pathService = docService @@ -1043,7 +1153,7 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { } // collect operation plugins - if !newOperationService && !newPathService { + if !newOperationService && !newPathService && !reusedPathService { // we're operating on the doc-level service entity, so we need the plugins // from the path and operation operationPluginList, err = getPluginsList(operation.Extensions, nil, pathPluginList, @@ -1062,6 +1172,11 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { // from the operation. operationPluginList, err = getPluginsList(operation.Extensions, nil, nil, opts.UUIDNamespace, operationBaseName, kongComponents, kongTags, opts.SkipID) + } else if reusedPathService { + // we're reusing an existing service, so path plugins need to flow to routes. + // Include path-level plugins in the operation plugin list (attached to route). + operationPluginList, err = getPluginsList(operation.Extensions, nil, pathPluginList, + opts.UUIDNamespace, operationBaseName, kongComponents, kongTags, opts.SkipID) } if err != nil { return nil, fmt.Errorf("failed to create plugins list from operation item: %w", err) diff --git a/openapi2kong/openapi2kong_test.go b/openapi2kong/openapi2kong_test.go index f465031..6983af8 100644 --- a/openapi2kong/openapi2kong_test.go +++ b/openapi2kong/openapi2kong_test.go @@ -86,6 +86,7 @@ func Test_Openapi2kong(t *testing.T) { // Test option configuration allHeadersRequired := false + reuseServices := false var config map[string]any yaml.Unmarshal(dataIn, &config) @@ -93,12 +94,16 @@ func Test_Openapi2kong(t *testing.T) { if val, ok := testConfig["treatAllHeadersAsRequired"]; ok { allHeadersRequired = val.(bool) } + if val, ok := testConfig["reuseServices"]; ok { + reuseServices = val.(bool) + } } dataOut, err := Convert(dataIn, O2kOptions{ Tags: []string{"OAS3_import", "OAS3file_" + fileNameIn}, OIDC: true, TreatAllHeadersAsRequired: allHeadersRequired, + ReuseServices: reuseServices, }) if err != nil { t.Error(fmt.Sprintf("'%s' didn't expect error: %%w", fixturePath+fileNameIn), err) @@ -232,3 +237,146 @@ func Test_Openapi2kong_SkipRouteByHeader(t *testing.T) { } } } + +func Test_Openapi2kong_ReuseServices(t *testing.T) { + // OAS with 3 paths having identical path-level servers and no plugins + testData := ` +openapi: 3.0.3 +info: + title: Reuse Test + version: v1 + +paths: + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + /products: + get: + operationId: getProducts + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 +` + + t.Run("flag disabled creates separate services", func(t *testing.T) { + result, err := Convert([]byte(testData), O2kOptions{ + ReuseServices: false, + SkipID: true, + }) + assert.NoError(t, err) + + services := result["services"].([]interface{}) + assert.Equal(t, 3, len(services), "should create 3 separate services without reuse") + + // Each service should have exactly 1 route + for _, svc := range services { + s := svc.(map[string]interface{}) + routes := s["routes"].([]interface{}) + assert.Equal(t, 1, len(routes)) + } + }) + + t.Run("flag enabled reuses identical services", func(t *testing.T) { + result, err := Convert([]byte(testData), O2kOptions{ + ReuseServices: true, + SkipID: true, + }) + assert.NoError(t, err) + + services := result["services"].([]interface{}) + assert.Equal(t, 1, len(services), "should create 1 shared service with reuse") + + // The single service should have all 3 routes + svc := services[0].(map[string]interface{}) + routes := svc["routes"].([]interface{}) + assert.Equal(t, 3, len(routes), "shared service should have 3 routes") + }) + + t.Run("flag enabled but plugins prevent reuse", func(t *testing.T) { + testDataWithPlugin := ` +openapi: 3.0.3 +info: + title: Reuse Plugin Test + version: v1 + +paths: + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + x-kong-plugin-rate-limiting: + config: + minute: 100 +` + result, err := Convert([]byte(testDataWithPlugin), O2kOptions{ + ReuseServices: true, + SkipID: true, + }) + assert.NoError(t, err) + + services := result["services"].([]interface{}) + assert.Equal(t, 2, len(services), "should create 2 services when path has plugins") + }) + + t.Run("flag enabled with different servers no reuse", func(t *testing.T) { + testDataDiffServers := ` +openapi: 3.0.3 +info: + title: Different Servers Test + version: v1 + +paths: + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://orders.example.com/v1 + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://users.example.com/v1 +` + result, err := Convert([]byte(testDataDiffServers), O2kOptions{ + ReuseServices: true, + SkipID: true, + }) + assert.NoError(t, err) + + services := result["services"].([]interface{}) + assert.Equal(t, 2, len(services), "should create 2 services for different server URLs") + }) +} From 2ffd94a18adddb9186fb0e4559c19f8c2bb42b9f Mon Sep 17 00:00:00 2001 From: shivay Date: Tue, 19 May 2026 12:50:57 +0530 Subject: [PATCH 2/4] fix: Added validation for operation level service deduplication. --- ...use-operation-level-services.expected.json | 72 +++++++++++++++ .../32-reuse-operation-level-services.yaml | 48 ++++++++++ ...ration-services-with-plugins.expected.json | 92 +++++++++++++++++++ ...reuse-operation-services-with-plugins.yaml | 45 +++++++++ openapi2kong/openapi2kong.go | 37 ++++++-- 5 files changed, 288 insertions(+), 6 deletions(-) create mode 100644 openapi2kong/oas3_testfiles/32-reuse-operation-level-services.expected.json create mode 100644 openapi2kong/oas3_testfiles/32-reuse-operation-level-services.yaml create mode 100644 openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.expected.json create mode 100644 openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.yaml diff --git a/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.expected.json b/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.expected.json new file mode 100644 index 0000000..4ae0612 --- /dev/null +++ b/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.expected.json @@ -0,0 +1,72 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "eaebb3a0-32b6-5782-bcfa-b3113c77974a", + "name": "operation-level-service-reuse_listinvoices", + "path": "/orderservice/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "54d88be6-da43-59f5-b70a-377effe7049b", + "methods": [ + "GET" + ], + "name": "operation-level-service-reuse_listinvoices", + "paths": [ + "~/invoices$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_32-reuse-operation-level-services.yaml" + ] + }, + { + "id": "a3302a8b-5663-541b-8e0e-ec8fa0d07f0e", + "methods": [ + "GET" + ], + "name": "operation-level-service-reuse_listorders", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_32-reuse-operation-level-services.yaml" + ] + }, + { + "id": "79b2ab21-4d0d-50e7-a3ac-8a7645acb0be", + "methods": [ + "POST" + ], + "name": "operation-level-service-reuse_createorder", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_32-reuse-operation-level-services.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_32-reuse-operation-level-services.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.yaml b/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.yaml new file mode 100644 index 0000000..9c8395f --- /dev/null +++ b/openapi2kong/oas3_testfiles/32-reuse-operation-level-services.yaml @@ -0,0 +1,48 @@ +# Test: Service reuse when identical operation-level servers exist +# +# When multiple operations define the same operation-level servers, only ONE +# service should be created with all routes attached to it. +# +# Expected behavior: +# - GET /orders and POST /orders both specify the same operation-level servers +# - GET /invoices also specifies the same operation-level servers +# - Only 1 operation-level service should be created (not 3) +# - All 3 routes should be attached to the single service + +openapi: 3.0.3 +info: + title: Operation Level Service Reuse + version: 1.0.0 + +x-test-config: + reuseServices: true + +paths: + /orders: + get: + operationId: listOrders + summary: List all orders + servers: + - url: https://api.example.com/orderservice/v1 + responses: + "200": + description: A list of orders + post: + operationId: createOrder + summary: Create an order + servers: + - url: https://api.example.com/orderservice/v1 + responses: + "201": + description: Order created + + /invoices: + get: + operationId: listInvoices + summary: List all invoices + servers: + - url: https://api.example.com/orderservice/v1 + responses: + "200": + description: A list of invoices + diff --git a/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.expected.json b/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.expected.json new file mode 100644 index 0000000..21e315c --- /dev/null +++ b/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.expected.json @@ -0,0 +1,92 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "6d69319a-2f10-5a56-8faa-fc556d510f86", + "name": "operation-reuse-with-plugins_listorders", + "path": "/orderservice/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "58976488-7669-5bdd-b769-d98eab427a72", + "methods": [ + "GET" + ], + "name": "operation-reuse-with-plugins_listorders", + "paths": [ + "~/orders$" + ], + "plugins": [ + { + "config": { + "minute": 100 + }, + "id": "d1315668-41c8-56ab-8b19-885316e53679", + "name": "rate-limiting", + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + } + ], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + }, + { + "id": "bf0d0fa7-3b8e-556d-ba4d-430320db47aa", + "methods": [ + "POST" + ], + "name": "operation-reuse-with-plugins_createorder", + "paths": [ + "~/orders$" + ], + "plugins": [ + { + "config": { + "key_names": [ + "apikey" + ] + }, + "id": "73b4b44d-fc02-5871-a818-add11a7c5535", + "name": "key-auth", + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + }, + { + "config": { + "minute": 100 + }, + "id": "b95d1e10-008e-5171-af0d-367f8ecc4f61", + "name": "rate-limiting", + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + } + ], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_33-reuse-operation-services-with-plugins.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.yaml b/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.yaml new file mode 100644 index 0000000..ebed502 --- /dev/null +++ b/openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.yaml @@ -0,0 +1,45 @@ +# Test: Operation-level service reuse with plugins on routes +# +# When operations reuse the same service, plugins defined at doc/path/operation +# level should flow to routes (not the shared service). +# +# Expected behavior: +# - GET /orders and POST /orders share the same operation-level service +# - Doc-level plugin (rate-limiting) should appear on each route +# - Operation-level plugin (key-auth on POST) should only appear on that route + +openapi: 3.0.3 +info: + title: Operation Reuse With Plugins + version: 1.0.0 + +x-test-config: + reuseServices: true + +x-kong-plugin-rate-limiting: + config: + minute: 100 + +paths: + /orders: + get: + operationId: listOrders + summary: List all orders + servers: + - url: https://api.example.com/orderservice/v1 + responses: + "200": + description: A list of orders + post: + operationId: createOrder + summary: Create an order + servers: + - url: https://api.example.com/orderservice/v1 + x-kong-plugin-key-auth: + config: + key_names: + - apikey + responses: + "201": + description: Order created + diff --git a/openapi2kong/openapi2kong.go b/openapi2kong/openapi2kong.go index 42e2b20..ad03dfd 100644 --- a/openapi2kong/openapi2kong.go +++ b/openapi2kong/openapi2kong.go @@ -688,8 +688,12 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { nameConcatChar = "_" } - // Initialize service cache for reuse of identical service configurations - svcCache := newServiceCache() + // Initialize separate service caches for path-level and operation-level services. + // We use separate caches because path-level services have plugins attached to them + // (doc + path plugins), while operation-level services do NOT have plugins on them + // (all plugins go to routes). Mixing them would cause plugin duplication. + pathSvcCache := newServiceCache() + opSvcCache := newServiceCache() // Load and parse the OAS file openapiDoc, err := libopenapi.NewDocument(content) @@ -937,7 +941,7 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { var reusedPathService bool if opts.ReuseServices && newPathService && !hasPathLevelPlugins(pathitem.Extensions) { serviceKey := generateServiceKey(pathServers, pathServiceDefaults, pathUpstreamDefaults) - if cachedService, found := svcCache.get(serviceKey); found { + if cachedService, found := pathSvcCache.get(serviceKey); found { logbasics.Debug("reusing existing service for path", "path", pathKey, "service", cachedService["name"]) pathService = cachedService reusedPathService = true @@ -992,7 +996,7 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { // Cache the new service for potential reuse by subsequent paths (only if enabled and no plugins) if opts.ReuseServices && !hasPathLevelPlugins(pathitem.Extensions) { serviceKey := generateServiceKey(pathServers, pathServiceDefaults, pathUpstreamDefaults) - svcCache.set(serviceKey, pathService) + pathSvcCache.set(serviceKey, pathService) } } else if reusedPathService { // We're reusing an existing service, so collect path plugins separately. @@ -1120,6 +1124,20 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { newOperationService = true } + // Check if we can reuse an existing service at the operation level. + var reusedOperationService bool + if opts.ReuseServices && newOperationService { + serviceKey := generateServiceKey(operationServers, operationServiceDefaults, operationUpstreamDefaults) + if cachedService, found := opSvcCache.get(serviceKey); found { + logbasics.Debug("reusing existing op-level service", + "path", pathKey, "method", methodKey, + "service", cachedService["name"]) + operationService = cachedService + reusedOperationService = true + newOperationService = false + } + } + // create a new service if we need to do so if newOperationService { // create the operation-level service and (optional) upstream @@ -1146,6 +1164,13 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { operationService["host"] = pathService["host"] } } + // Cache the new operation service for potential reuse + if opts.ReuseServices { + serviceKey := generateServiceKey(operationServers, operationServiceDefaults, operationUpstreamDefaults) + opSvcCache.set(serviceKey, operationService) + } + operationRoutes = operationService["routes"].([]interface{}) + } else if reusedOperationService { operationRoutes = operationService["routes"].([]interface{}) } else { operationService = pathService @@ -1153,12 +1178,12 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { } // collect operation plugins - if !newOperationService && !newPathService && !reusedPathService { + if !newOperationService && !newPathService && !reusedPathService && !reusedOperationService { // we're operating on the doc-level service entity, so we need the plugins // from the path and operation operationPluginList, err = getPluginsList(operation.Extensions, nil, pathPluginList, opts.UUIDNamespace, operationBaseName, kongComponents, kongTags, opts.SkipID) - } else if newOperationService { + } else if newOperationService || reusedOperationService { // we're operating on an operation-level service entity, so we need the plugins // from the document, path, and operation. operationPluginList, _ = getPluginsList(doc.Extensions, nil, nil, opts.UUIDNamespace, From 4a55e45f8e935421b8c61353b75b4d5ac1859bb0 Mon Sep 17 00:00:00 2001 From: shivay Date: Sat, 23 May 2026 15:19:19 +0530 Subject: [PATCH 3/4] chore: resolved review comments for test fix, sorting order and template rendering. --- ...es-with-headers.generated_skip_header.json | 55 +++++++ ...e-different-servers-no-reuse.expected.json | 70 +++++++++ .../34-reuse-different-servers-no-reuse.yaml | 37 +++++ .../35-reuse-disabled-no-dedup.expected.json | 70 +++++++++ .../35-reuse-disabled-no-dedup.yaml | 34 +++++ ...se-server-variables-no-reuse.expected.json | 70 +++++++++ .../36-reuse-server-variables-no-reuse.yaml | 44 ++++++ ...rver-variables-same-resolved.expected.json | 55 +++++++ ...-reuse-server-variables-same-resolved.yaml | 42 +++++ openapi2kong/openapi2kong.go | 28 +++- openapi2kong/openapi2kong_test.go | 143 ------------------ 11 files changed, 498 insertions(+), 150 deletions(-) create mode 100644 openapi2kong/oas3_testfiles/25-routes-with-headers.generated_skip_header.json create mode 100644 openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.expected.json create mode 100644 openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.yaml create mode 100644 openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.expected.json create mode 100644 openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.yaml create mode 100644 openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.expected.json create mode 100644 openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.yaml create mode 100644 openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.expected.json create mode 100644 openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml diff --git a/openapi2kong/oas3_testfiles/25-routes-with-headers.generated_skip_header.json b/openapi2kong/oas3_testfiles/25-routes-with-headers.generated_skip_header.json new file mode 100644 index 0000000..342ae19 --- /dev/null +++ b/openapi2kong/oas3_testfiles/25-routes-with-headers.generated_skip_header.json @@ -0,0 +1,55 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "server1.com", + "id": "0907c4ab-d9e4-5d21-813b-c57a97eeaad9", + "name": "simple-api-overview", + "path": "/", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "4d47cf34-1c69-5228-acf1-b2c05994bd02", + "methods": [ + "GET" + ], + "name": "simple-api-overview_get-doc-service", + "paths": [ + "~/path1/(?\u003cpathparam\u003e[^#?/]+)$" + ], + "plugins": [], + "regex_priority": 100, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_25-routes-with-headers.yaml" + ] + }, + { + "id": "27ea9a0a-216e-5c92-ae0b-8c9aa8493750", + "methods": [ + "GET" + ], + "name": "simple-api-overview_get-path2-service", + "paths": [ + "~/path2$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_25-routes-with-headers.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_25-routes-with-headers.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.expected.json b/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.expected.json new file mode 100644 index 0000000..41338b0 --- /dev/null +++ b/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.expected.json @@ -0,0 +1,70 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "orders.example.com", + "id": "b7a32115-a920-516d-81d5-bf1768b323c8", + "name": "different-servers-test_orders", + "path": "/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "5be34c6c-7890-558e-bb77-68e6648a7cfd", + "methods": [ + "GET" + ], + "name": "different-servers-test_getorders", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_34-reuse-different-servers-no-reuse.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_34-reuse-different-servers-no-reuse.yaml" + ] + }, + { + "host": "users.example.com", + "id": "3659ace2-c2e6-58b0-b2ba-d153bf057c7c", + "name": "different-servers-test_users", + "path": "/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "78eb5a0b-e667-54b3-b1d2-9fbd5b147868", + "methods": [ + "GET" + ], + "name": "different-servers-test_getusers", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_34-reuse-different-servers-no-reuse.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_34-reuse-different-servers-no-reuse.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.yaml b/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.yaml new file mode 100644 index 0000000..350156f --- /dev/null +++ b/openapi2kong/oas3_testfiles/34-reuse-different-servers-no-reuse.yaml @@ -0,0 +1,37 @@ +# Test: Different server URLs prevent service reuse +# +# When paths define different path-level servers, they should NOT be reused +# even when ReuseServices is enabled. +# +# Expected behavior: +# - /orders has server https://orders.example.com/v1 +# - /users has server https://users.example.com/v1 +# - 2 separate services should be created (no reuse due to different URLs) + +openapi: 3.0.3 +info: + title: Different Servers Test + version: v1 + +x-test-config: + reuseServices: true + +paths: + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://orders.example.com/v1 + + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://users.example.com/v1 + diff --git a/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.expected.json b/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.expected.json new file mode 100644 index 0000000..1e4ad53 --- /dev/null +++ b/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.expected.json @@ -0,0 +1,70 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "0fbc88cb-6386-574f-ac02-22f55392a905", + "name": "no-reuse-test_orders", + "path": "/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "0c91c325-fc4d-511e-a878-f43931fcafba", + "methods": [ + "GET" + ], + "name": "no-reuse-test_getorders", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_35-reuse-disabled-no-dedup.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_35-reuse-disabled-no-dedup.yaml" + ] + }, + { + "host": "api.example.com", + "id": "09ee22d6-4d42-5819-849f-2ff0f549fac9", + "name": "no-reuse-test_users", + "path": "/v1", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "3a3ddd34-0383-5889-93e0-281d0932b7f4", + "methods": [ + "GET" + ], + "name": "no-reuse-test_getusers", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_35-reuse-disabled-no-dedup.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_35-reuse-disabled-no-dedup.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.yaml b/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.yaml new file mode 100644 index 0000000..305986e --- /dev/null +++ b/openapi2kong/oas3_testfiles/35-reuse-disabled-no-dedup.yaml @@ -0,0 +1,34 @@ +# Test: No service deduplication when ReuseServices flag is disabled +# +# When the ReuseServices option is NOT enabled, identical path-level servers +# should still produce separate services (default behavior). +# +# Expected behavior: +# - /users and /orders have identical path-level servers +# - ReuseServices is NOT set (defaults to false) +# - 2 separate services should be created (no deduplication) + +openapi: 3.0.3 +info: + title: No Reuse Test + version: v1 + +paths: + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://api.example.com/v1 + diff --git a/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.expected.json b/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.expected.json new file mode 100644 index 0000000..24a57fe --- /dev/null +++ b/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.expected.json @@ -0,0 +1,70 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "c4810952-f606-5cb8-a131-fd7144b5b908", + "name": "server-variables-test_orders", + "path": "/v2/userservice", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "1ad5d76f-05ab-556d-b25c-294291bc42a0", + "methods": [ + "GET" + ], + "name": "server-variables-test_getorders", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_36-reuse-server-variables-no-reuse.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_36-reuse-server-variables-no-reuse.yaml" + ] + }, + { + "host": "api.example.com", + "id": "be27ff64-af54-50b7-b44c-53065c319df2", + "name": "server-variables-test_users", + "path": "/v1/userservice", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "38c89a6e-ec33-5812-ae5a-6df5dfa954bb", + "methods": [ + "GET" + ], + "name": "server-variables-test_getusers", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_36-reuse-server-variables-no-reuse.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_36-reuse-server-variables-no-reuse.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.yaml b/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.yaml new file mode 100644 index 0000000..31bca5c --- /dev/null +++ b/openapi2kong/oas3_testfiles/36-reuse-server-variables-no-reuse.yaml @@ -0,0 +1,44 @@ +# Test: Server variable substitution produces distinct services +# +# Two paths use the same URL template but with different variable defaults. +# Since CreateKongService resolves variables using their defaults, the resulting +# services would point to different backends and must NOT be reused. +# +# Expected behavior: +# - /users has server template with version default "v1" +# - /orders has server template with version default "v2" +# - 2 separate services should be created (different resolved URLs) + +openapi: 3.0.3 +info: + title: Server Variables Test + version: 1.0.0 + +x-test-config: + reuseServices: true + +paths: + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://api.example.com/{version}/userservice + variables: + version: + default: v1 + + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://api.example.com/{version}/userservice + variables: + version: + default: v2 + diff --git a/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.expected.json b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.expected.json new file mode 100644 index 0000000..b9cf863 --- /dev/null +++ b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.expected.json @@ -0,0 +1,55 @@ +{ + "_format_version": "3.0", + "services": [ + { + "host": "api.example.com", + "id": "505884ef-98c9-52b8-b113-9070a1570181", + "name": "server-variables-same-resolved-url_orders", + "path": "/v1/service", + "plugins": [], + "port": 443, + "protocol": "https", + "routes": [ + { + "id": "944bea4a-636b-51d8-8180-e3e73b5ef842", + "methods": [ + "GET" + ], + "name": "server-variables-same-resolved-url_getorders", + "paths": [ + "~/orders$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_37-reuse-server-variables-same-resolved.yaml" + ] + }, + { + "id": "66012c4f-f8d5-568d-a1f7-1a31bf14ebc6", + "methods": [ + "GET" + ], + "name": "server-variables-same-resolved-url_getusers", + "paths": [ + "~/users$" + ], + "plugins": [], + "regex_priority": 200, + "strip_path": false, + "tags": [ + "OAS3_import", + "OAS3file_37-reuse-server-variables-same-resolved.yaml" + ] + } + ], + "tags": [ + "OAS3_import", + "OAS3file_37-reuse-server-variables-same-resolved.yaml" + ] + } + ], + "upstreams": [] +} \ No newline at end of file diff --git a/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml new file mode 100644 index 0000000..a87998f --- /dev/null +++ b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml @@ -0,0 +1,42 @@ +# Test: Server variable substitution allows reuse when resolved URLs match +# +# Two paths use different URL templates but with variables that resolve to the +# same final URL. Since the resolved URLs are identical, services CAN be reused. +# +# Expected behavior: +# - /users and /orders both resolve to https://api.example.com/v1/service +# - Only 1 service should be created (reuse since resolved URLs match) + +openapi: 3.0.3 +info: + title: Server Variables Same Resolved URL + version: 1.0.0 + +x-test-config: + reuseServices: true + +paths: + /users: + get: + operationId: getUsers + responses: + "200": + description: OK + servers: + - url: https://api.example.com/{version}/service + variables: + version: + default: v1 + + /orders: + get: + operationId: getOrders + responses: + "200": + description: OK + servers: + - url: https://api.example.com/{version}/service + variables: + version: + default: v1 + diff --git a/openapi2kong/openapi2kong.go b/openapi2kong/openapi2kong.go index ad03dfd..dc7ef8b 100644 --- a/openapi2kong/openapi2kong.go +++ b/openapi2kong/openapi2kong.go @@ -554,17 +554,31 @@ func constructHeaderCombinationsForRouting(headers []*v3.Parameter) []map[string // generateServiceKey creates a unique key for a service configuration based on // servers, service defaults, and upstream defaults. This is used to identify // identical service configurations for potential reuse. +// Server order is preserved (not sorted) because CreateKongService uses the +// first server entry to derive protocol, path, and port for the Kong service. +// Server URLs are rendered after variable substitution (matching parseServerUris +// behavior) so that templates with different variable defaults produce distinct keys. func generateServiceKey(servers []*v3.Server, serviceDefaults []byte, upstreamDefaults []byte) string { var sb strings.Builder - // Add sorted server URLs to the key - serverURLs := make([]string, 0, len(servers)) + // Add rendered server URLs in their original order to the key. + // We substitute template variables using their defaults, matching + // how parseServerUris resolves URLs before service creation. + // Order matters because CreateKongService uses targets[0] to set + // protocol/path/port on the service entity. for _, server := range servers { - serverURLs = append(serverURLs, server.URL) - } - sort.Strings(serverURLs) - for _, url := range serverURLs { - sb.WriteString(url) + rendered := server.URL + if server.Variables != nil { + pair := server.Variables.First() + for pair != nil { + name := pair.Key() + svar := pair.Value() + rendered = strings.ReplaceAll( + rendered, "{"+name+"}", svar.Default) + pair = pair.Next() + } + } + sb.WriteString(rendered) sb.WriteString("|") } diff --git a/openapi2kong/openapi2kong_test.go b/openapi2kong/openapi2kong_test.go index 6983af8..3175a30 100644 --- a/openapi2kong/openapi2kong_test.go +++ b/openapi2kong/openapi2kong_test.go @@ -237,146 +237,3 @@ func Test_Openapi2kong_SkipRouteByHeader(t *testing.T) { } } } - -func Test_Openapi2kong_ReuseServices(t *testing.T) { - // OAS with 3 paths having identical path-level servers and no plugins - testData := ` -openapi: 3.0.3 -info: - title: Reuse Test - version: v1 - -paths: - /orders: - get: - operationId: getOrders - responses: - "200": - description: OK - servers: - - url: https://api.example.com/v1 - /products: - get: - operationId: getProducts - responses: - "200": - description: OK - servers: - - url: https://api.example.com/v1 - /users: - get: - operationId: getUsers - responses: - "200": - description: OK - servers: - - url: https://api.example.com/v1 -` - - t.Run("flag disabled creates separate services", func(t *testing.T) { - result, err := Convert([]byte(testData), O2kOptions{ - ReuseServices: false, - SkipID: true, - }) - assert.NoError(t, err) - - services := result["services"].([]interface{}) - assert.Equal(t, 3, len(services), "should create 3 separate services without reuse") - - // Each service should have exactly 1 route - for _, svc := range services { - s := svc.(map[string]interface{}) - routes := s["routes"].([]interface{}) - assert.Equal(t, 1, len(routes)) - } - }) - - t.Run("flag enabled reuses identical services", func(t *testing.T) { - result, err := Convert([]byte(testData), O2kOptions{ - ReuseServices: true, - SkipID: true, - }) - assert.NoError(t, err) - - services := result["services"].([]interface{}) - assert.Equal(t, 1, len(services), "should create 1 shared service with reuse") - - // The single service should have all 3 routes - svc := services[0].(map[string]interface{}) - routes := svc["routes"].([]interface{}) - assert.Equal(t, 3, len(routes), "shared service should have 3 routes") - }) - - t.Run("flag enabled but plugins prevent reuse", func(t *testing.T) { - testDataWithPlugin := ` -openapi: 3.0.3 -info: - title: Reuse Plugin Test - version: v1 - -paths: - /orders: - get: - operationId: getOrders - responses: - "200": - description: OK - servers: - - url: https://api.example.com/v1 - /users: - get: - operationId: getUsers - responses: - "200": - description: OK - servers: - - url: https://api.example.com/v1 - x-kong-plugin-rate-limiting: - config: - minute: 100 -` - result, err := Convert([]byte(testDataWithPlugin), O2kOptions{ - ReuseServices: true, - SkipID: true, - }) - assert.NoError(t, err) - - services := result["services"].([]interface{}) - assert.Equal(t, 2, len(services), "should create 2 services when path has plugins") - }) - - t.Run("flag enabled with different servers no reuse", func(t *testing.T) { - testDataDiffServers := ` -openapi: 3.0.3 -info: - title: Different Servers Test - version: v1 - -paths: - /orders: - get: - operationId: getOrders - responses: - "200": - description: OK - servers: - - url: https://orders.example.com/v1 - /users: - get: - operationId: getUsers - responses: - "200": - description: OK - servers: - - url: https://users.example.com/v1 -` - result, err := Convert([]byte(testDataDiffServers), O2kOptions{ - ReuseServices: true, - SkipID: true, - }) - assert.NoError(t, err) - - services := result["services"].([]interface{}) - assert.Equal(t, 2, len(services), "should create 2 services for different server URLs") - }) -} From 351ecc326138c44bdd308fb1d70995839ff4b126 Mon Sep 17 00:00:00 2001 From: shivay Date: Sat, 23 May 2026 15:41:03 +0530 Subject: [PATCH 4/4] chore: resolved review comments --- ...-reuse-server-variables-same-resolved.yaml | 12 +++-- openapi2kong/openapi2kong.go | 45 +++++-------------- openapitools/service.go | 35 +++++++++++++++ 3 files changed, 54 insertions(+), 38 deletions(-) diff --git a/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml index a87998f..7200a19 100644 --- a/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml +++ b/openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml @@ -1,10 +1,12 @@ # Test: Server variable substitution allows reuse when resolved URLs match # -# Two paths use different URL templates but with variables that resolve to the -# same final URL. Since the resolved URLs are identical, services CAN be reused. +# Two paths use different URL templates that resolve to the same final URL. +# Since the resolved URLs are identical, services CAN be reused. # # Expected behavior: -# - /users and /orders both resolve to https://api.example.com/v1/service +# - /users resolves https://api.example.com/{version}/service with version=v1 +# - /orders resolves https://{host}/{version}/service with host=api.example.com, version=v1 +# - Both resolve to https://api.example.com/v1/service # - Only 1 service should be created (reuse since resolved URLs match) openapi: 3.0.3 @@ -35,8 +37,10 @@ paths: "200": description: OK servers: - - url: https://api.example.com/{version}/service + - url: https://{host}/{version}/service variables: + host: + default: api.example.com version: default: v1 diff --git a/openapi2kong/openapi2kong.go b/openapi2kong/openapi2kong.go index dc7ef8b..798fd5f 100644 --- a/openapi2kong/openapi2kong.go +++ b/openapi2kong/openapi2kong.go @@ -552,33 +552,15 @@ func constructHeaderCombinationsForRouting(headers []*v3.Parameter) []map[string } // generateServiceKey creates a unique key for a service configuration based on -// servers, service defaults, and upstream defaults. This is used to identify -// identical service configurations for potential reuse. -// Server order is preserved (not sorted) because CreateKongService uses the -// first server entry to derive protocol, path, and port for the Kong service. -// Server URLs are rendered after variable substitution (matching parseServerUris -// behavior) so that templates with different variable defaults produce distinct keys. +// servers, service defaults, and upstream defaults. +// (normalized the same way as CreateKongService) plus service/upstream defaults. func generateServiceKey(servers []*v3.Server, serviceDefaults []byte, upstreamDefaults []byte) string { var sb strings.Builder - // Add rendered server URLs in their original order to the key. - // We substitute template variables using their defaults, matching - // how parseServerUris resolves URLs before service creation. - // Order matters because CreateKongService uses targets[0] to set - // protocol/path/port on the service entity. - for _, server := range servers { - rendered := server.URL - if server.Variables != nil { - pair := server.Variables.First() - for pair != nil { - name := pair.Key() - svar := pair.Value() - rendered = strings.ReplaceAll( - rendered, "{"+name+"}", svar.Default) - pair = pair.Next() - } - } - sb.WriteString(rendered) + // Normalized URLs (variable substitution + default ports/paths), order-preserving. + renderedURLs := openapitools.RenderServerURLs(servers) + for _, u := range renderedURLs { + sb.WriteString(u) sb.WriteString("|") } @@ -1013,16 +995,11 @@ func Convert(content []byte, opts O2kOptions) (map[string]interface{}, error) { pathSvcCache.set(serviceKey, pathService) } } else if reusedPathService { - // We're reusing an existing service, so collect path plugins separately. - // Path plugins will flow to routes via operationPluginList. - pathPluginList, err = getPluginsList(pathitem.Extensions, componentExtensions, nil, - opts.UUIDNamespace, pathBaseName, kongComponents, kongTags, opts.SkipID) - if err != nil { - return nil, fmt.Errorf("failed to create plugins list from path item: %w", err) - } - - // Extract the request-validator config from the plugin list - pathValidatorConfig, pathPluginList = getValidatorPlugin(pathPluginList, docValidatorConfig) + // Service reuse is gated on !hasPathLevelPlugins, so there are no + // path-level plugins to collect. Just reset to empty/inherited values. + emptyList := make([]*map[string]interface{}, 0) + pathPluginList = &emptyList + pathValidatorConfig = docValidatorConfig } else { // no new path-level service entity required, so stick to the doc-level one pathService = docService diff --git a/openapitools/service.go b/openapitools/service.go index 8b90793..4a26909 100644 --- a/openapitools/service.go +++ b/openapitools/service.go @@ -16,6 +16,41 @@ const ( httpsScheme = "https" ) +// RenderServerURLs renders server URLs after variable substitution and URL +// normalization (same as parseServerUris + setServerDefaults). This produces +// canonical URL strings suitable for use as cache keys, ensuring that +// equivalent server definitions (e.g., "https://api.example.com" vs +// "https://api.example.com:443") produce the same output. +func RenderServerURLs(servers []*v3.Server) []string { + targets, err := parseServerUris(servers) + if err != nil { + // On parse error, fall back to raw variable-substituted URLs + result := make([]string, len(servers)) + for i, server := range servers { + rendered := server.URL + if server.Variables != nil { + pair := server.Variables.First() + for pair != nil { + name := pair.Key() + svar := pair.Value() + rendered = strings.ReplaceAll(rendered, "{"+name+"}", svar.Default) + pair = pair.Next() + } + } + result[i] = rendered + } + return result + } + + setServerDefaults(targets, httpsScheme) + + result := make([]string, len(targets)) + for i, t := range targets { + result[i] = t.String() + } + return result +} + // parseServerUris parses the server uri's after rendering the template variables. // result will always have at least 1 entry, but not necessarily a hostname/port/scheme func parseServerUris(servers []*v3.Server) ([]*url.URL, error) {