Skip to content

feat: move Backends back to RouteRule#633

Merged
mathetake merged 2 commits intoenvoyproxy:mainfrom
kerthcet:fix/map-backends-with-headers
May 22, 2025
Merged

feat: move Backends back to RouteRule#633
mathetake merged 2 commits intoenvoyproxy:mainfrom
kerthcet:fix/map-backends-with-headers

Conversation

@kerthcet
Copy link
Copy Markdown
Contributor

@kerthcet kerthcet commented May 22, 2025

Commit Message

The backends and headers in filter config are M:N, with #599, we swapped out the backend from the config.rules, leading to the lost mapping relationship between them. With this PR, we'll move the backends back to the config.rules which is more straightforward.

Related Issues/PRs (if applicable)

Related PR: #620, #599

Special notes for reviewers (if applicable)

None

@kerthcet kerthcet force-pushed the fix/map-backends-with-headers branch from 51fa137 to efe7d34 Compare May 22, 2025 06:24
@kerthcet kerthcet changed the title Move Backends back to RouteRule feat: Move Backends back to RouteRule May 22, 2025
@kerthcet kerthcet changed the title feat: Move Backends back to RouteRule feat: move Backends back to RouteRule May 22, 2025
Signed-off-by: kerthcet <kerthcet@gmail.com>
@kerthcet kerthcet force-pushed the fix/map-backends-with-headers branch from fd1e824 to 1b730b2 Compare May 22, 2025 09:49
@kerthcet kerthcet marked this pull request as ready for review May 22, 2025 09:53
@kerthcet kerthcet requested a review from a team as a code owner May 22, 2025 09:53
@kerthcet
Copy link
Copy Markdown
Contributor Author

/assign @mathetake as discussed offline.

Comment on lines +88 to +91
for _, r := range config.Rules {
for _, backend := range r.Backends {
b := backend
var h backendauth.Handler
Copy link
Copy Markdown
Member

@mathetake mathetake May 22, 2025

Choose a reason for hiding this comment

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

can you merge this for-loop over r.Backends to the for-loop above at l75? we do looping over config.Rules twice here

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.

good catch. Addressed. PTAL.

Copy link
Copy Markdown
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

LGMT modulo the comment

@mathetake mathetake self-assigned this May 22, 2025
Signed-off-by: kerthcet <kerthcet@gmail.com>
@mathetake mathetake merged commit 762be89 into envoyproxy:main May 22, 2025
17 checks passed
@mathetake
Copy link
Copy Markdown
Member

Thank you! @kerthcet

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