Why not admit ingress failed when the same path and host is defined? #5651
Labels
kind/support
Categorizes issue or PR as a support question.
lifecycle/stale
Denotes an issue or PR has remained open with no activity and has become stale.
Comments
Because there was no admission-webhook concept in the inception of the project and also the admission-webhook can be disabled/removed.
Yes, but the logic of using the oldest definition must be preserved. Pull requests are welcome :) |
8 tasks
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
k8s-ci-robot
added
the
lifecycle/stale
Fsero
added a commit
to Fsero/ingress-nginx
that referenced
this issue
Jan 30, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones kubernetes#10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) kubernetes#10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes fixes kubernetes#10820 fixes kubernetes#10090 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to adevinta/ingress-nginx
that referenced
this issue
Feb 1, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to adevinta/ingress-nginx
that referenced
this issue
Feb 5, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to Fsero/ingress-nginx
that referenced
this issue
Sep 5, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones kubernetes#10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) kubernetes#10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes It might help with kubernetes#10820 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to Fsero/ingress-nginx
that referenced
this issue
Oct 4, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones kubernetes#10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) kubernetes#10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes It might help with kubernetes#10820 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to Fsero/ingress-nginx
that referenced
this issue
Oct 7, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones kubernetes#10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) kubernetes#10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes It might help with kubernetes#10820 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Fsero
added a commit
to Fsero/ingress-nginx
that referenced
this issue
Nov 26, 2024
## What this PR does / why we need it: In kubernetes#5651 there was a request to throw an error when there are two ingresses defining the same host and path, which was implemented as part of the validation webhook. Despite of this there are clear rules on the ingress controller that describes what the controller would do in [such situation (the oldest rule wins)](https://github.com/kubernetes/ingress-nginx/blob/main/docs/how-it-works.md?plain=1#L27) Some users are relying on this validation behaviour to prevent misconfigurations, but there are use cases where allowing it, maybe temporarily, is helpful. Those use cases includes: - Splitting large ingresses objects in smaller ones kubernetes#10820 - Moving ingress objects between namespaces without downtime (like when you transfer an ingress from team A that owns namespace A to team B that owns namespace B) kubernetes#10090 <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [X] New feature (non-breaking change which adds functionality) - [ ] CVE Report (Scanner found CVE and adding report) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation only ## Which issue/s this PR fixes It might help with kubernetes#10820 <!--- Please describe in detail how you tested your changes. --> <!--- Include details of your testing environment, and the tests you ran to --> <!--- see how your change affects other areas of the code, etc. --> ## How Has This Been Tested? building an image and testing it in a local cluster, will update later with some real life scenarios <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> ## Checklist: - [X] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [X] I've read the [CONTRIBUTION](https://github.com/kubernetes/ingress-nginx/blob/main/CONTRIBUTING.md) guide - [X] I have added unit and/or e2e tests to cover my changes. - [X] All new and existing tests passed. Change-Id: I9d4124d1c36876b06d63100cd10988eaf2f41db9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
/triage support
As it says in how-it-worksl:
Why not we return a error in this case, it's maybe a misconfiguration， can we deal it in admission-webhook?
The text was updated successfully, but these errors were encountered: