New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
internal/dag: configure 502 if include references another (#5016) #5157
internal/dag: configure 502 if include references another (#5016) #5157
Conversation
Hi @liangyuanpeng! Welcome to our community and thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Contour better. You can also join us on our mailing list and in our channel in the Kubernetes Slack Workspace |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5157 +/- ##
=======================================
Coverage 77.90% 77.91%
=======================================
Files 138 138
Lines 17856 17861 +5
=======================================
+ Hits 13911 13916 +5
Misses 3675 3675
Partials 270 270
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR, implementation looks great, just needs
- some test coverage (similar to this test case, but with an include that references a "root" httpproxy:
contour/internal/dag/builder_test.go
Line 10611 in aa750df
"insert httproxy with invalid include": { - and a release note (following guidelines here https://github.com/projectcontour/contour/blob/aa750df6bce7f6a46ff3739cc9f935f1134c0500/CONTRIBUTING.md#commit-message-and-pr-guidelines, in this case a
small
release note type)
The Contour project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to the #contour channel in the Kubernetes Slack |
4514034
to
ed7ac77
Compare
ed7ac77
to
0e3fd79
Compare
the test failure should be resolved by merging/rebasing main and swapping the Listener port in the test to 8080, that was a change that occurred on main more recently |
Set 502 response if include references another root Fixes projectcontour#5016 Signed-off-by: Lan Liang <gcslyp@gmail.com>
0e2f855
to
c383eb0
Compare
projectcontour#5157) Set 502 response if include references another root Fixes projectcontour#5016 Signed-off-by: Lan Liang <gcslyp@gmail.com>
Set 502 response if include references another root
Fixes #5016