validator: make sure Ingress/RouteGroup doesn't have duplicate hosts #2753
Conversation
| @@ -0,0 +1,28 @@ | |||
| apiVersion: zalando.org/v1 | |||
There was a problem hiding this comment.
I think the test fixture for Ingress is missing
There was a problem hiding this comment.
Right now we don't validate ingress in skipper we only do in ValidationWebhook, I think we will need another PR to migrate current ingress adhock validation to validator object.
There was a problem hiding this comment.
What I mean by adhock validation https://github.com/zalando/skipper/blob/master/dataclients/kubernetes/ingress.go#L257C1-L269C2
| @@ -0,0 +1,41 @@ | |||
| { | |||
| "request": { | |||
There was a problem hiding this comment.
I still think we do not need to add tests for admission webhook when we change validator, its just increases size of PR but adds no value as webhook simply calls validator
MustafaSaber
force-pushed
the
validate-duplicate-routes-per-resource
branch
from
8828e25 to
6df46c7
Compare
MustafaSaber
changed the title
|
@MustafaSaber What's up with this one? Looks like it needs rebase |
MustafaSaber
added
the
major
|
This was based on |
|
Revert PR |
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
MustafaSaber
force-pushed
the
validate-duplicate-routes-per-resource
branch
from
dbbe504 to
2b64634
Compare
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
|
👍 |
1 similar comment
|
👍 |
| for _, rule := range item.Spec.Rules { | ||
| if _, ok := uniqueHosts[rule.Host]; ok { | ||
| errs = append(errs, fmt.Errorf("duplicate host %q", rule.Host)) | ||
| } | ||
| uniqueHosts[rule.Host] = struct{}{} | ||
| } |
There was a problem hiding this comment.
I think ingress rules may have the same host repeated with different rules, lets check
There was a problem hiding this comment.
We agreed to disable duplicate hostname validation for ingress.
Relates to #1618