Skip to content

Conversation

M00nF1sh
Copy link
Collaborator

@M00nF1sh M00nF1sh commented Nov 19, 2020

Add ingress class support

This change will add Ingress class support, the only supported field in IngressClass is the controller field. In the future, we'll leverage the parameters field for more customizations.

Behavior

  • If only kubernete.io/ingress.class annotations presents, we consider IngressClass matches this controller if

    • when --ingress-class is specified with non-empty value, and it matches annotation value (when install via YAML/Helm, by default it's specified as alb):
    • when --ingress-class is specified with empty value, and annotation value is empty or alb.
  • If only spec.ingressClassName non-nil, we consider IngressClass matches this controller if

    • the corresponding IngressClass exists and the spec.controller of IngressClass have value ingress.k8s.aws/alb
  • If both kubernete.io/ingress.class annotation presents and spec.ingressClassName non-nil, we will favor kubernete.io/ingress.class and issue a warning event to Ingress if match result conflicts.
    we do this to align with IngressSpec's recommendation:For backwards compatibility, when that annotation is set, it must be given precedence over this field. The controller may emit a warning if the field and annotation have different values.

  • If neither kubernete.io/ingress.class annotation presents and spec.ingressClassName non-nil, we consider IngressClass matches this controller if --ingress-class is specified with empty value

Future plans

  1. we'll offer an option to disable kubernete.io/ingress.class annotation support in future versions with default value false in future versions. (this will help us enforce permission control via IngressClass)
  2. we'll change the option above to have default value true post X versions.
  3. we'll remove the code that support kubernete.io/ingress.class post Y versions.
  4. IngressClass's parameters will only be respected if IngressClass is decided from spec.ingressClassName. (i.e. kubernete.io/ingress.class annotation absents)

Note to reviewer:

  1. I'll have follow up PR to add RBAC permissions
  2. I'll have follow up PR to adjust the behavior of decide whether Ingress should be groupMember if there are errors like ingressClassName not found.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 19, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kishorj, M00nF1sh
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@M00nF1sh M00nF1sh merged commit 15066ef into kubernetes-sigs:main Nov 19, 2020
Timothy-Dougherty pushed a commit to adammw/aws-load-balancer-controller that referenced this pull request Nov 9, 2023
* add ingress class support

* address comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants