feat(components/router): add kubernetes native common ingress component #52
Conversation
Pangjiping
force-pushed
the
feat/components/proxy
branch
from
5201f13 to
b1e1026
Compare
Pangjiping
requested review from
hittyt,
jwx0925 and
ninan-nn
as code owners
Pangjiping
force-pushed
the
feat/components/proxy
branch
from
b1e1026 to
05bdeeb
Compare
Pangjiping
added
documentation
Pangjiping
force-pushed
the
feat/components/proxy
branch
from
05bdeeb to
4ed09a0
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if code == http.StatusNotFound { | ||
| Logger.Warnw("Proxy: no pod found for ingress rule", "ingress", host.ingressKey) | ||
| } | ||
|
|
||
| http.Error(w, fmt.Sprintf("Proxy: %v", err), code) |
There was a problem hiding this comment.
If fetchRealHost returns an error (no matching pod or multiple pods), the handler writes an error response but immediately continues to set the request host and call serve with the unresolved targetHost. The second proxy attempt can emit a new 502/500 and overwrite the earlier 404/409, making error responses flaky whenever the ingress lookup fails. Return after the http.Error so failed lookups do not continue into the proxy path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
nice suggestion, fixed.
components/router/pkg/proxy/proxy.go
Outdated
| defer func() { | ||
| if err := recover(); err != nil { | ||
| Logger.Errorw("Proxy: proxy causes panic", "error", err) | ||
| http.Error(w, err.(error).Error(), http.StatusBadGateway) |
There was a problem hiding this comment.
The recovery block assumes the panic value implements error and type-asserts err.(error) before responding. Panics triggered with plain strings (e.g., panic("boom") from helpers or libraries) will fail that assertion and crash the process instead of returning the intended 502 response, defeating the panic guard in those scenarios. Safeguard the type assertion or format the recovered value to avoid a secondary panic.
Useful? React with 👍 / 👎.
2 participants
Summary
routerfor kubernetes ingress proxy.Testing
Breaking Changes
Checklist