Skip to content
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

ingress v1: parse ingress class field in Ingress v1 resource #790

Closed
hbagdi opened this issue Aug 5, 2020 · 1 comment
Closed

ingress v1: parse ingress class field in Ingress v1 resource #790

hbagdi opened this issue Aug 5, 2020 · 1 comment
Assignees
Milestone

Comments

@hbagdi
Copy link
Member

hbagdi commented Aug 5, 2020

Ingress v1beta1 resource are fitlered based on the kubernetes.io/ingress.class annotations. Class field is introduced in ingress v1 which should replace the annotation.
The scope here is to parse this field.
The priority here is not intuitive so please take that into account:
https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/20190125-ingress-api-group.md#interoperability-with-previous-annotation

For #590

@hbagdi hbagdi added the trello label Aug 5, 2020
@hbagdi hbagdi added this to the 0.10.0 milestone Aug 5, 2020
@hbagdi hbagdi assigned rainest and mflendrich and unassigned rainest Aug 28, 2020
@mflendrich
Copy link
Contributor

@rainest rainest self-assigned this Sep 11, 2020
mflendrich pushed a commit that referenced this issue Sep 15, 2020
**What this PR does / why we need it**:
This handles Ingress v1 class, as we need to handle Ingress v1 class.

**Which issue this PR fixes**: fixes #790

**Notes/questions for reviewers**:
This is in very rough draft state, so it's bad, untested, and full of misery, but it should be 😄 

- AFAIK we do indeed intend to have separate flags for ignoring class on v1 Ingress and v1beta1 Ingress--I think I remember that from previous discussions, and it seems a bit weird (SO MANY FLAGS), but eh, whatever.
- "annotations" is in a weird spot: the minimally invasive version of this adds a new function generator there, but it's for something that's no longer an annotation. Other option was to export `validIngress` and stick the Ingress V1 validator function in store, but eh, that's no good either.
- That the validation functions are part of the store itself seems kinda weird, but maybe that does make sense.
- Checking between the original KEP, which [just called this ingress class](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/20190125-ingress-api-group.md#ingress-class-field), and the [actual 1.19 API](https://github.com/kubernetes/kubernetes/blob/release-1.19/staging/src/k8s.io/api/networking/v1/types.go#L244-L257) apparently calls it `IngressClassName`. Pretty sure those are equivalent, but wanted to double-check, and want to make sure I understand the relationship between that and the actual [IngressClass object](https://github.com/kubernetes/kubernetes/blob/release-1.19/staging/src/k8s.io/api/networking/v1/types.go#L468-L484) correctly.

* feat(store) handle Ingress v1 class

Update the Ingress v1 lister to handle both the legacy annotation used
in Ingress v1beta1 and the native Ingress class field introduced in
Ingress v1.

If the legacy annotation is present, the controller uses its value to
decide whether or not to process the Ingress, regardless of whether the
new native field is present. If the legacy annoation is absent, the
controller will use the native field. The decision logic itself is
unchanged: it continues to require an exact class match by default, and
can optionally also match Ingresses with no annotation or native class
if users override this default.

* test(annotations+store) add Ingress v1 class tests

Test Ingress v1 class matching. In annotations, test that the match
behaviors are the same for native class matching as they are for ingress
class annotations. In store, test that class matching successfully
filters Ingresses with the correct class into the store and that
Ingresses with the correct class, but with an incorrect class
annotation, are not filtered in, because the class annotation takes
precedence.

From #843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants