Handle Ingress v1 class #843
Conversation
rainest
force-pushed
the
feat/ingressv1-class-field
branch
from
ae2335d to
8d9bc75
Compare
mflendrich
force-pushed
the
feat/ingressv1-parser
branch
from
acaf329 to
2ccafa8
Compare
rainest
force-pushed
the
feat/ingressv1-class-field
branch
from
e1e095c to
1904a91
Compare
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 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.
rainest
force-pushed
the
feat/ingressv1-class-field
branch
from
1904a91 to
b4370e2
Compare
|
As Harry and I have noted during informal review, the separation between annotations and store is all sorts of connections and interdependencies that shouldn't really exist. This PR seeks to disregard that debt for now and adds new logic that matches the form of the existing logic, even if that form is horrid. Task for tomorrow is to write up an issue that describes this in more detail, so that we can work that out either in 1.0 or mark it down as something we need to address after, before proceeding with further v1 Ingress work. That's probably a good project for me to work with @mflendrich on, as we're now both familiar with the current state of the code, and I'd benefit a lot from watching him go through the process of refactoring it into something that's saner in light of our new need to support multiple Ingress object versions. |
There was a problem hiding this comment.
LGTM.
@mflendrich can do another pass and then we can merge.
There was a problem hiding this comment.
LGTM, with the following notes:
- Like @rainest said: let's separate the storing concern and the ingress class filtering concern before we touch this code again.
--process-classless-ingress-v1beta1vs--process-classless-ingress-v1is a scary user trap 👻, because v1beta1 and v1 don't refer to ingress apiversions uploaded by the user. The effective flag (out of these two) changes after an operation as simple as upgrading from k8s <1.19 to >=1.19.
Цо ПР ділать бы, и пошімў:
This handles Ingress v1 class, as we need to handle Ingress v1 class.
Пробрэма, хторюю хорректировяло ПР: fixes #790
Нёты:
This is in very rough draft state, so it's bad, untested, and full of misery, but it should be 😄
validIngressand stick the Ingress V1 validator function in store, but eh, that's no good either.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 correctly.