Skip to content

load_balancers: fix crash on invalid endpoint update with health checks#41114

Merged
ggreenway merged 7 commits intoenvoyproxy:mainfrom
ggreenway:eds-lb-hc-crash
Sep 24, 2025
Merged

load_balancers: fix crash on invalid endpoint update with health checks#41114
ggreenway merged 7 commits intoenvoyproxy:mainfrom
ggreenway:eds-lb-hc-crash

Conversation

@ggreenway
Copy link
Copy Markdown
Member

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes #41102

@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #41114 was opened by ggreenway.

see: more, trace.

Fixes envoyproxy#41102

Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
Signed-off-by: Greg Greenway <ggreenway@apple.com>
@wbpcode
Copy link
Copy Markdown
Member

wbpcode commented Sep 19, 2025

Is these necessary to make the validator part of load balancer config? Seems it's a common validation that should always be applied in all cases and has non-business with LB directly?

/wait-any

@ggreenway
Copy link
Copy Markdown
Member Author

This validation has always been part of these particular load balancers. If we made it common, we may end up rejecting a config that was previously valid, and that still works.

But also, I discovered this crash while working on a load balancer internally that requires specific metadata on each endpoint and rejects the endpoints if it isn't present. I think it's reasonable for load balancers to have the opportunity to reject sets of endpoints.

Signed-off-by: Greg Greenway <ggreenway@apple.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/coverage-shephards: FYI only for changes made to (test/coverage.yaml).
envoyproxy/coverage-shephards assignee is @RyanTheOptimist

🐱

Caused by: #41114 was synchronize by ggreenway.

see: more, trace.

@ggreenway ggreenway merged commit f1004fa into envoyproxy:main Sep 24, 2025
25 checks passed
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.

Crash when load balancer rejects host_set change due to health checks

4 participants