fix: annotation konghq.com/rewrite always works #6626
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Fixes bug reported by user description of the problem and steps to reproduce it robustly are here #6542 (comment)
Which issue this PR fixes:
Fixes #6542
Special notes for your reviewer:
This feature was covered with a quite complicated integration test
TestIngressRewriteURI
designed to catch exactly such a bug, but it failed to do so. Why?8e5f883
Changes in the test have no effect (it's just an opportunistic cleanup not to have redundant stuff). The bug was hard to catch due to the order of items in the Go map during iteration - it's random and it even may be randomized for each iteration. Hence this commit always enforces the same order for defined routes by sorting them (IMHO KIC should always operate in a consistent and reproducible manner so paying the cost of sorting is worth it). This change leads to the consistent failure of the test
TestIngressRewriteURI
as it is supposed to be.6898b9c
Actual one-line fix
return
->continue
to not ignore a rewrite annotation that is not attached to the first item in a list. It resolves the problem (as I said even without changes from the first commit).PR Readiness Checklist:
Complete these before marking the PR as
ready to review
:CHANGELOG.md
release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR