Add tests to cover override behaviour for IngressRoute and HTTPProxy #1981
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youngnick
force-pushed
the
1949-IRandHPpriority
branch
from
4db3c02
to
675d381
Compare
|
After some discussion with Dave, this is missing some tests, and a feature: The ingressroute that is superseded by the HTTPProxy should have an invalid status. I'll work more on this tomorrow. |
davecheney
approved these changes
Dec 6, 2019
internal/dag/builder_test.go
Outdated
| @@ -5479,6 +5550,57 @@ func TestDAGInsert(t *testing.T) { | |||
| }, | |||
| ), | |||
| }, | |||
| // This tests what happens when you build with an ingressroute and a HTTPProxy | |||
There was a problem hiding this comment.
suggestion: try the imperative mood
// Assert that a route in IngressRoute takes precedence over Ingress.
There was a problem hiding this comment.
Hmm, good idea, that's much more concise. I'll change this, and then merge.
|
pls ignore my previous reply, wrong window |
Fixes projectcontour#1949 Issue projectcontour#1949 is about ensuring that HTTPProxy objects will override IngressRoute ones when they match on {FQDN, path} tuple. Turns out this is already the case, this PR adds tests to assert this behaviour. Signed-off-by: Nick Young <ynick@vmware.com>
youngnick
force-pushed
the
1949-IRandHPpriority
branch
from
675d381
to
4129db5
Compare
|
@youngnick ping, this Is approved, are you ok to land it? |
|
Sorry, yes, will land it now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1949
Issue #1949 is about ensuring that HTTPProxy objects will override IngressRoute
ones when they match on {FQDN, path} tuple. Turns out this is already the case,
this PR adds tests to assert this behaviour.
The tests have an Ingress, IngressRoute, and HTTPProxy that are identical aside from the service where the traffic is routed to, and assert that the override behaviour holds based on which service ends up chosen.
Note that routes that are not exactly the same (on FQDN, path tuple), will just be additive, no matter where they come from. This is why the cert-manager ingress-shim thing works.
Signed-off-by: Nick Young ynick@vmware.com