Skip to content

Fix: header behaviour in openapi2kong#289

Open
harshadixit12 wants to merge 5 commits intomainfrom
fix/header-behaviour-in-openapi2kong
Open

Fix: header behaviour in openapi2kong#289
harshadixit12 wants to merge 5 commits intomainfrom
fix/header-behaviour-in-openapi2kong

Conversation

@harshadixit12
Copy link
Contributor

Second attempt at #285, which was reverted due to failing tests.

Fixes:
Kong/deck#1856
Kong/deck#1838

mheap and others added 3 commits March 7, 2026 21:58
There is a `TreatAllHeadersAsRequired` option to keep the old behaviour
if needed. I do not intend to add a flag to deck yet unless people ask for
this functionality
@harshadixit12 harshadixit12 force-pushed the fix/header-behaviour-in-openapi2kong branch from c47496a to 6463172 Compare March 7, 2026 16:39
@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.82%. Comparing base (a6a5619) to head (a15fc97).

Files with missing lines Patch % Lines
openapi2kong/openapi2kong.go 90.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #289      +/-   ##
==========================================
+ Coverage   69.75%   69.82%   +0.07%     
==========================================
  Files          24       24              
  Lines        2738     2751      +13     
==========================================
+ Hits         1910     1921      +11     
- Misses        652      653       +1     
- Partials      176      177       +1     

☔ 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.

@harshadixit12
Copy link
Contributor Author

I've added a commit to fix duplicate plugin IDs in older tests.
Also - in 6463172#diff-c655e44d9e55d9766b44a0cff7734f9e880cc5185762cc3a51f462deea661492 we are fixing a test - because based on implementation when there are >1 headers - say one required (m variants in enum), and one optional, we are creating m routes - no fallback.

@harshadixit12 harshadixit12 marked this pull request as ready for review March 10, 2026 03:17
@harshadixit12 harshadixit12 requested a review from mheap March 10, 2026 03:18
@mheap
Copy link
Member

mheap commented Mar 10, 2026

@harshadixit12 Do you think the implemented behaviour is correct here? Should we have a fallback?

@harshadixit12
Copy link
Contributor Author

@harshadixit12 Do you think the implemented behaviour is correct here? Should we have a fallback?

Since this header behaviour is limited to required headers - technically this may be ok. However, I imagine APIs don't start out with required headers - for example, a version header would only become required after there is > 1 version of API live.
Let me add a fallback.

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.

3 participants