Skip to content

fix: Added a flag to reduce deduplication while handling route server path#294

Merged
shivaygupta-dotcom merged 4 commits into
mainfrom
fix/path-server-deduplication
May 25, 2026
Merged

fix: Added a flag to reduce deduplication while handling route server path#294
shivaygupta-dotcom merged 4 commits into
mainfrom
fix/path-server-deduplication

Conversation

@shivaygupta-dotcom
Copy link
Copy Markdown
Contributor

@shivaygupta-dotcom shivaygupta-dotcom commented May 13, 2026

FTI: https://konghq.atlassian.net/browse/FTI-7457

Summary:
When multiple OAS paths or operations defined identical servers blocks, we were creating separate Kong services for each path and operation even though they pointed to the same upstream, resulting in unnecessary duplicate services in Kong.

We added a --reuse-services flag to cache and reuse services based on server URL and defaults.

  • Path-level services are reused only when no x-kong-plugin-* extensions are present, with plugins attached to routes to avoid conflicts.
  • Operation-level services are reused without the plugin-extension check, since operation-level plugins are attached directly to routes instead of services.

https://kongstrong.slack.com/archives/C07AQH7SAF8/p1778764775374189

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

❌ Patch coverage is 68.96552% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.07%. Comparing base (d157bf3) to head (351ecc3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
openapitools/service.go 0.00% 20 Missing ⚠️
openapi2kong/openapi2kong.go 89.55% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
+ Coverage   64.98%   65.07%   +0.08%     
==========================================
  Files          25       25              
  Lines        3119     3204      +85     
==========================================
+ Hits         2027     2085      +58     
- Misses        881      905      +24     
- Partials      211      214       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shivaygupta-dotcom shivaygupta-dotcom marked this pull request as ready for review May 18, 2026 06:45
@shivaygupta-dotcom shivaygupta-dotcom changed the title fix: Added a flag and check to reduce deduplication while generating … fix: Added a flag to reduce deduplication while handling route server path May 18, 2026
Comment thread openapi2kong/openapi2kong.go
@shivaygupta-dotcom shivaygupta-dotcom force-pushed the fix/path-server-deduplication branch from 08395a7 to 3b0fc7e Compare May 19, 2026 07:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an opt-in --reuse-services conversion mode to reduce duplicate Kong services when multiple OpenAPI path/operation server blocks resolve to the same upstream, while attempting to avoid plugin conflicts by keeping plugin attachment behavior consistent with existing service-vs-route rules.

Changes:

  • Introduces O2kOptions.ReuseServices and a --reuse-services CLI flag to enable caching/reuse of generated services.
  • Implements path-level and operation-level service caches keyed by server config + service/upstream defaults, with special handling for path-level plugin extensions.
  • Adds/updates tests and fixtures to validate service reuse behavior (including plugin-related cases).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
openapi2kong/openapi2kong.go Adds the reuse-services option, cache/keying logic, and plugin-flow adjustments when reusing services.
openapi2kong/openapi2kong_test.go Plumbs reuseServices through fixture-driven tests and adds a focused unit test for reuse behavior.
openapi2kong/oas3_testfiles/30-reuse-identical-services.yaml New fixture covering path-level reuse with identical servers.
openapi2kong/oas3_testfiles/30-reuse-identical-services.expected.json Expected output for path-level reuse fixture.
openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.yaml New fixture ensuring path-level plugin extensions prevent reuse.
openapi2kong/oas3_testfiles/31-reuse-with-plugins-on-routes.expected.json Expected output for “no reuse when path plugins exist” fixture.
openapi2kong/oas3_testfiles/32-reuse-operation-level-services.yaml New fixture covering operation-level reuse with identical servers.
openapi2kong/oas3_testfiles/32-reuse-operation-level-services.expected.json Expected output for operation-level reuse fixture.
openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.yaml New fixture covering operation-level reuse while keeping plugins on routes.
openapi2kong/oas3_testfiles/33-reuse-operation-services-with-plugins.expected.json Expected output for operation-level reuse with plugins fixture.
cmd/openapi2kong.go Adds the --reuse-services CLI flag and wires it into O2kOptions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread openapi2kong/openapi2kong.go Outdated
Comment thread openapi2kong/openapi2kong.go Outdated
Comment thread openapi2kong/openapi2kong_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Comment thread openapi2kong/openapi2kong.go Outdated
Comment thread openapi2kong/openapi2kong.go Outdated
Comment thread openapi2kong/oas3_testfiles/37-reuse-server-variables-same-resolved.yaml Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

openapi2kong/openapi2kong.go:1185

  • These calls to getPluginsList discard the returned error. If doc/path-level plugin parsing fails, the conversion can proceed with an incomplete/incorrect plugin list instead of returning a failure. Capture and propagate errors for all three getPluginsList calls (doc, path, operation).
			} 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,
					operationBaseName, kongComponents, kongTags, opts.SkipID)
				operationPluginList, _ = getPluginsList(pathitem.Extensions, nil, operationPluginList, opts.UUIDNamespace,
					operationBaseName, kongComponents, kongTags, opts.SkipID)
				operationPluginList, err = getPluginsList(operation.Extensions, nil, operationPluginList, opts.UUIDNamespace,
					operationBaseName, kongComponents, kongTags, opts.SkipID)

openapi2kong/openapi2kong_test.go:110

  • This failure path builds an error string using fmt.Sprintf with a literal "%%w" and then passes err as a separate argument to t.Error, so the formatted message never includes the error in a useful way. Prefer t.Errorf("%s didn't expect error: %v", ..., err) (or t.Fatalf) so the actual error is included.
			})
			if err != nil {
				t.Error(fmt.Sprintf("'%s' didn't expect error: %%w", fixturePath+fileNameIn), err)
			} else {

Comment thread openapi2kong/openapi2kong.go
@shivaygupta-dotcom shivaygupta-dotcom merged commit 04e8549 into main May 25, 2026
9 checks passed
@shivaygupta-dotcom shivaygupta-dotcom deleted the fix/path-server-deduplication branch May 25, 2026 07:16
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.

4 participants