From cef147a24dbd717b11d9141cafa01719be2d0ba0 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Wed, 28 Jul 2021 18:58:46 -0300 Subject: [PATCH] Fix IngressClass logic for newer releases (#7341) * Fix IngressClass logic for newer releases Signed-off-by: Ricardo Pchevuzinske Katz * Change e2e tests for the new IngressClass presence * Fix chart and admission tests Signed-off-by: Ricardo Pchevuzinske Katz * Fix helm chart test Signed-off-by: Ricardo Pchevuzinske Katz * Fix reviews * Remove ingressclass code from admission --- .github/workflows/ci.yaml | 19 +- .../ci/daemonset-customconfig-values.yaml | 4 + .../ci/daemonset-customnodeport-values.yaml | 4 + .../ci/daemonset-headers-values.yaml | 4 + .../ci/daemonset-internal-lb-values.yaml | 4 + .../ci/daemonset-nodeport-values.yaml | 4 + .../ci/daemonset-podannotations-values.yaml | 4 + ...set-tcp-udp-configMapNamespace-values.yaml | 4 + .../ci/daemonset-tcp-udp-values.yaml | 4 + .../ci/daemonset-tcp-values.yaml | 4 + .../ci/deamonset-default-values.yaml | 4 + .../ci/deamonset-metrics-values.yaml | 4 + .../ci/deamonset-psp-values.yaml | 4 + .../ci/deamonset-webhook-and-psp-values.yaml | 4 + .../ci/deamonset-webhook-values.yaml | 4 + .../ci/deployment-autoscaling-values.yaml | 4 + .../ci/deployment-customconfig-values.yaml | 4 + .../ci/deployment-customnodeport-values.yaml | 4 + .../ci/deployment-default-values.yaml | 4 + .../ci/deployment-headers-values.yaml | 4 + .../ci/deployment-internal-lb-values.yaml | 4 + .../ci/deployment-metrics-values.yaml | 4 + .../ci/deployment-nodeport-values.yaml | 4 + .../ci/deployment-podannotations-values.yaml | 4 + .../ci/deployment-psp-values.yaml | 4 + ...ent-tcp-udp-configMapNamespace-values.yaml | 4 + .../ci/deployment-tcp-udp-values.yaml | 4 + .../ci/deployment-tcp-values.yaml | 4 + .../ci/deployment-webhook-and-psp-values.yaml | 4 + .../ci/deployment-webhook-values.yaml | 4 + .../ingress-nginx/templates/clusterrole.yaml | 8 +- .../templates/controller-daemonset.yaml | 2 +- .../templates/controller-deployment.yaml | 2 +- .../templates/controller-ingressclass.yaml | 12 +- .../templates/controller-role.yaml | 10 +- charts/ingress-nginx/values.yaml | 10 +- cmd/nginx/flags.go | 35 +- cmd/nginx/main.go | 21 +- internal/ingress/annotations/class/main.go | 64 --- .../ingress/annotations/class/main_test.go | 103 ---- internal/ingress/controller/controller.go | 9 +- .../ingress/controller/controller_test.go | 26 +- .../controller/ingressclass/ingressclass.go | 45 ++ internal/ingress/controller/nginx.go | 12 +- .../ingress/controller/store/ingressclass.go | 39 ++ internal/ingress/controller/store/store.go | 133 ++++- .../ingress/controller/store/store_test.go | 530 ++++++++++++++---- internal/ingress/metric/main.go | 11 +- internal/ingress/status/status_test.go | 4 +- internal/k8s/main.go | 3 - .../forwarded-port-headers/values.yaml | 4 +- test/e2e/admission/admission.go | 55 +- test/e2e/annotations/affinity.go | 1 + test/e2e/annotations/canary.go | 56 +- test/e2e/defaultbackend/with_hosts.go | 1 + test/e2e/framework/framework.go | 18 +- test/e2e/framework/util.go | 91 +++ test/e2e/ingress/without_host.go | 1 + test/e2e/run-chart-test.sh | 24 +- test/e2e/run.sh | 4 +- test/e2e/servicebackend/service_backend.go | 2 + .../servicebackend/service_externalname.go | 15 +- test/e2e/settings/global_external_auth.go | 0 test/e2e/settings/ingress_class.go | 505 +++++++++++------ test/e2e/settings/no_auth_locations.go | 1 + test/e2e/settings/server_tokens.go | 1 + test/e2e/status/update.go | 2 +- test/e2e/wait-for-nginx.sh | 4 + 68 files changed, 1406 insertions(+), 593 deletions(-) delete mode 100644 internal/ingress/annotations/class/main.go delete mode 100644 internal/ingress/annotations/class/main_test.go create mode 100644 internal/ingress/controller/ingressclass/ingressclass.go create mode 100644 internal/ingress/controller/store/ingressclass.go mode change 100755 => 100644 test/e2e/settings/global_external_auth.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8904fe2023..924c74debf 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -57,8 +57,6 @@ jobs: name: Build runs-on: ubuntu-latest needs: changes - if: | - (needs.changes.outputs.go == 'true') steps: @@ -116,6 +114,7 @@ jobs: runs-on: ubuntu-latest needs: - changes + - build if: | (needs.changes.outputs.charts == 'true') @@ -123,6 +122,11 @@ jobs: - name: Checkout uses: actions/checkout@v2 + + - name: cache + uses: actions/download-artifact@v2 + with: + name: docker.tar.gz - name: Lint run: | @@ -139,11 +143,22 @@ jobs: with: version: v0.11.1 image: kindest/node:v1.21.1 + + - uses: geekyeggo/delete-artifact@v1 + with: + name: docker.tar.gz + failOnError: false + + - name: Load images from cache + run: | + echo "loading docker images..." + pigz -dc docker.tar.gz | docker load - name: Test env: KIND_CLUSTER_NAME: kind SKIP_CLUSTER_CREATION: true + SKIP_IMAGE_CREATION: true run: | kind get kubeconfig > $HOME/.kube/kind-config-kind make kind-e2e-chart-tests diff --git a/charts/ingress-nginx/ci/daemonset-customconfig-values.yaml b/charts/ingress-nginx/ci/daemonset-customconfig-values.yaml index e12b53421b..43dd2b2ac9 100644 --- a/charts/ingress-nginx/ci/daemonset-customconfig-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-customconfig-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null kind: DaemonSet admissionWebhooks: enabled: false diff --git a/charts/ingress-nginx/ci/daemonset-customnodeport-values.yaml b/charts/ingress-nginx/ci/daemonset-customnodeport-values.yaml index cfc545f69f..1d94be219b 100644 --- a/charts/ingress-nginx/ci/daemonset-customnodeport-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-customnodeport-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false diff --git a/charts/ingress-nginx/ci/daemonset-headers-values.yaml b/charts/ingress-nginx/ci/daemonset-headers-values.yaml index ff82cd9c70..ab7d47bd4d 100644 --- a/charts/ingress-nginx/ci/daemonset-headers-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-headers-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false addHeaders: diff --git a/charts/ingress-nginx/ci/daemonset-internal-lb-values.yaml b/charts/ingress-nginx/ci/daemonset-internal-lb-values.yaml index 443e39d8ba..abae84f6d7 100644 --- a/charts/ingress-nginx/ci/daemonset-internal-lb-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-internal-lb-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/daemonset-nodeport-values.yaml b/charts/ingress-nginx/ci/daemonset-nodeport-values.yaml index 6d6605f0e1..3b7aa2fcd2 100644 --- a/charts/ingress-nginx/ci/daemonset-nodeport-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-nodeport-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/daemonset-podannotations-values.yaml b/charts/ingress-nginx/ci/daemonset-podannotations-values.yaml index 04ac58dbd8..0b55306a10 100644 --- a/charts/ingress-nginx/ci/daemonset-podannotations-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-podannotations-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false metrics: diff --git a/charts/ingress-nginx/ci/daemonset-tcp-udp-configMapNamespace-values.yaml b/charts/ingress-nginx/ci/daemonset-tcp-udp-configMapNamespace-values.yaml index afb5487c57..acd86a77ab 100644 --- a/charts/ingress-nginx/ci/daemonset-tcp-udp-configMapNamespace-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-tcp-udp-configMapNamespace-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/daemonset-tcp-udp-values.yaml b/charts/ingress-nginx/ci/daemonset-tcp-udp-values.yaml index 7b4d7cbe7d..25ee64d856 100644 --- a/charts/ingress-nginx/ci/daemonset-tcp-udp-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-tcp-udp-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/daemonset-tcp-values.yaml b/charts/ingress-nginx/ci/daemonset-tcp-values.yaml index a359a6a401..380c8b4b13 100644 --- a/charts/ingress-nginx/ci/daemonset-tcp-values.yaml +++ b/charts/ingress-nginx/ci/daemonset-tcp-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deamonset-default-values.yaml b/charts/ingress-nginx/ci/deamonset-default-values.yaml index e63a7f5db3..82fa23e854 100644 --- a/charts/ingress-nginx/ci/deamonset-default-values.yaml +++ b/charts/ingress-nginx/ci/deamonset-default-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deamonset-metrics-values.yaml b/charts/ingress-nginx/ci/deamonset-metrics-values.yaml index 1e5190afc0..cb3cb54be2 100644 --- a/charts/ingress-nginx/ci/deamonset-metrics-values.yaml +++ b/charts/ingress-nginx/ci/deamonset-metrics-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false metrics: diff --git a/charts/ingress-nginx/ci/deamonset-psp-values.yaml b/charts/ingress-nginx/ci/deamonset-psp-values.yaml index 017b60a9c6..8026a6356f 100644 --- a/charts/ingress-nginx/ci/deamonset-psp-values.yaml +++ b/charts/ingress-nginx/ci/deamonset-psp-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deamonset-webhook-and-psp-values.yaml b/charts/ingress-nginx/ci/deamonset-webhook-and-psp-values.yaml index 88aafc66fd..fccdb134cf 100644 --- a/charts/ingress-nginx/ci/deamonset-webhook-and-psp-values.yaml +++ b/charts/ingress-nginx/ci/deamonset-webhook-and-psp-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: true service: diff --git a/charts/ingress-nginx/ci/deamonset-webhook-values.yaml b/charts/ingress-nginx/ci/deamonset-webhook-values.yaml index 6e3b371da6..54d364df11 100644 --- a/charts/ingress-nginx/ci/deamonset-webhook-values.yaml +++ b/charts/ingress-nginx/ci/deamonset-webhook-values.yaml @@ -1,5 +1,9 @@ controller: kind: DaemonSet + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: true service: diff --git a/charts/ingress-nginx/ci/deployment-autoscaling-values.yaml b/charts/ingress-nginx/ci/deployment-autoscaling-values.yaml index 5314cecb38..b8b3ac6862 100644 --- a/charts/ingress-nginx/ci/deployment-autoscaling-values.yaml +++ b/charts/ingress-nginx/ci/deployment-autoscaling-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null autoscaling: enabled: true admissionWebhooks: diff --git a/charts/ingress-nginx/ci/deployment-customconfig-values.yaml b/charts/ingress-nginx/ci/deployment-customconfig-values.yaml index f232531acb..85715ddb76 100644 --- a/charts/ingress-nginx/ci/deployment-customconfig-values.yaml +++ b/charts/ingress-nginx/ci/deployment-customconfig-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null config: use-proxy-protocol: "true" admissionWebhooks: diff --git a/charts/ingress-nginx/ci/deployment-customnodeport-values.yaml b/charts/ingress-nginx/ci/deployment-customnodeport-values.yaml index 9eda282b13..a564eaf931 100644 --- a/charts/ingress-nginx/ci/deployment-customnodeport-values.yaml +++ b/charts/ingress-nginx/ci/deployment-customnodeport-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deployment-default-values.yaml b/charts/ingress-nginx/ci/deployment-default-values.yaml index 93a393c975..9f46b4e7e9 100644 --- a/charts/ingress-nginx/ci/deployment-default-values.yaml +++ b/charts/ingress-nginx/ci/deployment-default-values.yaml @@ -1,4 +1,8 @@ # Left blank to test default values controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null service: type: ClusterIP diff --git a/charts/ingress-nginx/ci/deployment-headers-values.yaml b/charts/ingress-nginx/ci/deployment-headers-values.yaml index 665fd48d35..17a11ac370 100644 --- a/charts/ingress-nginx/ci/deployment-headers-values.yaml +++ b/charts/ingress-nginx/ci/deployment-headers-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false addHeaders: diff --git a/charts/ingress-nginx/ci/deployment-internal-lb-values.yaml b/charts/ingress-nginx/ci/deployment-internal-lb-values.yaml index 892f6de3f0..8b2eb78bf1 100644 --- a/charts/ingress-nginx/ci/deployment-internal-lb-values.yaml +++ b/charts/ingress-nginx/ci/deployment-internal-lb-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deployment-metrics-values.yaml b/charts/ingress-nginx/ci/deployment-metrics-values.yaml index 887ed0f620..9209ad5a6f 100644 --- a/charts/ingress-nginx/ci/deployment-metrics-values.yaml +++ b/charts/ingress-nginx/ci/deployment-metrics-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false metrics: diff --git a/charts/ingress-nginx/ci/deployment-nodeport-values.yaml b/charts/ingress-nginx/ci/deployment-nodeport-values.yaml index 84f1f7582e..cd9b323528 100644 --- a/charts/ingress-nginx/ci/deployment-nodeport-values.yaml +++ b/charts/ingress-nginx/ci/deployment-nodeport-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deployment-podannotations-values.yaml b/charts/ingress-nginx/ci/deployment-podannotations-values.yaml index b65a0910b3..b48d93c46a 100644 --- a/charts/ingress-nginx/ci/deployment-podannotations-values.yaml +++ b/charts/ingress-nginx/ci/deployment-podannotations-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false metrics: diff --git a/charts/ingress-nginx/ci/deployment-psp-values.yaml b/charts/ingress-nginx/ci/deployment-psp-values.yaml index e339c69c32..2f332a7b20 100644 --- a/charts/ingress-nginx/ci/deployment-psp-values.yaml +++ b/charts/ingress-nginx/ci/deployment-psp-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null service: type: ClusterIP diff --git a/charts/ingress-nginx/ci/deployment-tcp-udp-configMapNamespace-values.yaml b/charts/ingress-nginx/ci/deployment-tcp-udp-configMapNamespace-values.yaml index 141e06b687..c51a4e91fa 100644 --- a/charts/ingress-nginx/ci/deployment-tcp-udp-configMapNamespace-values.yaml +++ b/charts/ingress-nginx/ci/deployment-tcp-udp-configMapNamespace-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deployment-tcp-udp-values.yaml b/charts/ingress-nginx/ci/deployment-tcp-udp-values.yaml index bc29abeba7..5b45b69dcc 100644 --- a/charts/ingress-nginx/ci/deployment-tcp-udp-values.yaml +++ b/charts/ingress-nginx/ci/deployment-tcp-udp-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: false service: diff --git a/charts/ingress-nginx/ci/deployment-tcp-values.yaml b/charts/ingress-nginx/ci/deployment-tcp-values.yaml index b7f54c09fa..ac0b6e60eb 100644 --- a/charts/ingress-nginx/ci/deployment-tcp-values.yaml +++ b/charts/ingress-nginx/ci/deployment-tcp-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null service: type: ClusterIP diff --git a/charts/ingress-nginx/ci/deployment-webhook-and-psp-values.yaml b/charts/ingress-nginx/ci/deployment-webhook-and-psp-values.yaml index a829c36144..6195bb3391 100644 --- a/charts/ingress-nginx/ci/deployment-webhook-and-psp-values.yaml +++ b/charts/ingress-nginx/ci/deployment-webhook-and-psp-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: true service: diff --git a/charts/ingress-nginx/ci/deployment-webhook-values.yaml b/charts/ingress-nginx/ci/deployment-webhook-values.yaml index 4f18a70b9f..76669a5300 100644 --- a/charts/ingress-nginx/ci/deployment-webhook-values.yaml +++ b/charts/ingress-nginx/ci/deployment-webhook-values.yaml @@ -1,4 +1,8 @@ controller: + image: + repository: ingress-controller/controller + tag: 1.0.0-dev + digest: null admissionWebhooks: enabled: true service: diff --git a/charts/ingress-nginx/templates/clusterrole.yaml b/charts/ingress-nginx/templates/clusterrole.yaml index 8ec5f49fa4..95b7fec085 100644 --- a/charts/ingress-nginx/templates/clusterrole.yaml +++ b/charts/ingress-nginx/templates/clusterrole.yaml @@ -42,8 +42,7 @@ rules: - list - watch - apiGroups: - - extensions - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingresses verbs: @@ -58,14 +57,13 @@ rules: - create - patch - apiGroups: - - extensions - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingresses/status verbs: - update - apiGroups: - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingressclasses verbs: diff --git a/charts/ingress-nginx/templates/controller-daemonset.yaml b/charts/ingress-nginx/templates/controller-daemonset.yaml index d92fc31bd8..d32ed72e7c 100644 --- a/charts/ingress-nginx/templates/controller-daemonset.yaml +++ b/charts/ingress-nginx/templates/controller-daemonset.yaml @@ -80,7 +80,7 @@ spec: - --publish-service={{ template "ingress-nginx.controller.publishServicePath" . }} {{- end }} - --election-id={{ .Values.controller.electionID }} - - --ingress-class={{ .Values.controller.ingressClass }} + - --controller-class={{ .Values.controller.ingressClassResource.controllerValue }} - --configmap={{ default "$(POD_NAMESPACE)" .Values.controller.configMapNamespace }}/{{ include "ingress-nginx.controller.fullname" . }} {{- if .Values.tcp }} - --tcp-services-configmap={{ default "$(POD_NAMESPACE)" .Values.controller.tcp.configMapNamespace }}/{{ include "ingress-nginx.fullname" . }}-tcp diff --git a/charts/ingress-nginx/templates/controller-deployment.yaml b/charts/ingress-nginx/templates/controller-deployment.yaml index f30fea5ba5..c0c6f49332 100644 --- a/charts/ingress-nginx/templates/controller-deployment.yaml +++ b/charts/ingress-nginx/templates/controller-deployment.yaml @@ -84,7 +84,7 @@ spec: - --publish-service={{ template "ingress-nginx.controller.publishServicePath" . }} {{- end }} - --election-id={{ .Values.controller.electionID }} - - --ingress-class={{ .Values.controller.ingressClass }} + - --controller-class={{ .Values.controller.ingressClassResource.controllerValue }} - --configmap={{ default "$(POD_NAMESPACE)" .Values.controller.configMapNamespace }}/{{ include "ingress-nginx.controller.fullname" . }} {{- if .Values.tcp }} - --tcp-services-configmap={{ default "$(POD_NAMESPACE)" .Values.controller.tcp.configMapNamespace }}/{{ include "ingress-nginx.fullname" . }}-tcp diff --git a/charts/ingress-nginx/templates/controller-ingressclass.yaml b/charts/ingress-nginx/templates/controller-ingressclass.yaml index f94b9590de..9492784a28 100644 --- a/charts/ingress-nginx/templates/controller-ingressclass.yaml +++ b/charts/ingress-nginx/templates/controller-ingressclass.yaml @@ -1,9 +1,7 @@ -{{- if and (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion) (.Values.controller.ingressClassResource.enabled) -}} -{{- if and (semverCompare "=1.18-0" .Capabilities.KubeVersion.GitVersion) }} -apiVersion: networking.k8s.io/v1beta1 -{{- else }} +{{- if .Values.controller.ingressClassResource.enabled -}} +# We don't support namespaced ingressClass yet +# So a ClusterRole and a ClusterRoleBinding is required apiVersion: networking.k8s.io/v1 -{{- end }} kind: IngressClass metadata: labels: @@ -12,12 +10,12 @@ metadata: {{- with .Values.controller.labels }} {{- toYaml . | nindent 4 }} {{- end }} - name: {{ .Values.controller.ingressClass }} + name: {{ .Values.controller.ingressClassResource.name }} {{- if .Values.controller.ingressClassResource.default }} annotations: ingressclass.kubernetes.io/is-default-class: "true" {{- end }} spec: - controller: k8s.io/ingress-nginx + controller: {{ .Values.controller.ingressClassResource.controllerValue }} {{ template "ingressClass.parameters" . }} {{- end }} diff --git a/charts/ingress-nginx/templates/controller-role.yaml b/charts/ingress-nginx/templates/controller-role.yaml index 1a5ccd29bf..97c627dacb 100644 --- a/charts/ingress-nginx/templates/controller-role.yaml +++ b/charts/ingress-nginx/templates/controller-role.yaml @@ -34,8 +34,7 @@ rules: - list - watch - apiGroups: - - extensions - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingresses verbs: @@ -43,14 +42,13 @@ rules: - list - watch - apiGroups: - - extensions - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingresses/status verbs: - update - apiGroups: - - "networking.k8s.io" # k8s 1.14+ + - networking.k8s.io resources: - ingressclasses verbs: @@ -62,7 +60,7 @@ rules: resources: - configmaps resourceNames: - - {{ .Values.controller.electionID }}-{{ .Values.controller.ingressClass }} + - {{ .Values.controller.electionID }} verbs: - get - update diff --git a/charts/ingress-nginx/values.yaml b/charts/ingress-nginx/values.yaml index 59fb47a236..4c04842270 100644 --- a/charts/ingress-nginx/values.yaml +++ b/charts/ingress-nginx/values.yaml @@ -76,15 +76,13 @@ controller: ## electionID: ingress-controller-leader - ## Name of the ingress class to route through this controller - ## - ingressClass: nginx - # This section refers to the creation of the IngressClass resource - # IngressClass resources are supported since k8s >= 1.18 + # IngressClass resources are supported since k8s >= 1.18 and required since k8s >= 1.19 ingressClassResource: - enabled: false + name: nginx + enabled: true default: false + controllerValue: "k8s.io/ingress-nginx" # Parameters is a link to a custom resource containing additional # configuration for the controller. This is optional if the controller diff --git a/cmd/nginx/flags.go b/cmd/nginx/flags.go index b125719fdb..aabade07e9 100644 --- a/cmd/nginx/flags.go +++ b/cmd/nginx/flags.go @@ -24,10 +24,10 @@ import ( "github.com/spf13/pflag" apiv1 "k8s.io/api/core/v1" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/controller" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/internal/ingress/status" ing_net "k8s.io/ingress-nginx/internal/net" "k8s.io/ingress-nginx/internal/nginx" @@ -55,10 +55,18 @@ only when the flag --apiserver-host is specified.`) Takes the form "namespace/name". The controller configures NGINX to forward requests to the first port of this Service.`) - ingressClass = flags.String("ingress-class", "", - `Name of the ingress class this controller satisfies. -The class of an Ingress object is set using the field IngressClassName in Kubernetes clusters version v1.18.0 or higher or the annotation "kubernetes.io/ingress.class" (deprecated). -If this parameter is not set, or set to the default value of "nginx", it will handle ingresses with either an empty or "nginx" class name.`) + ingressClassAnnotation = flags.String("ingress-class", ingressclass.DefaultAnnotationValue, + `[IN DEPRECATION] Name of the ingress class this controller satisfies. +The class of an Ingress object is set using the annotation "kubernetes.io/ingress.class" (deprecated). +The parameter --controller-class has precedence over this.`) + + ingressClassController = flags.String("controller-class", ingressclass.DefaultControllerName, + `Ingress Class Controller value this Ingress satisfies. +The class of an Ingress object is set using the field IngressClassName in Kubernetes clusters version v1.19.0 or higher. The .spec.controller value of the IngressClass +referenced in an Ingress Object should be the same value specified here to make this object be watched.`) + + watchWithoutClass = flags.Bool("watch-ingress-without-class", false, + `Define if Ingress Controller should also watch for Ingresses without an IngressClass or the annotation specified`) configMap = flags.String("configmap", "", `Name of the ConfigMap containing custom global configurations for the controller.`) @@ -207,18 +215,6 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g status.UpdateInterval = *statusUpdateInterval } - if *ingressClass != "" { - klog.InfoS("Watching for Ingress", "class", *ingressClass) - - if *ingressClass != class.DefaultClass { - klog.Warningf("Only Ingresses with class %q will be processed by this Ingress controller", *ingressClass) - } else { - klog.Warning("Ingresses with an empty class will also be processed by this Ingress controller") - } - - class.IngressClass = *ingressClass - } - parser.AnnotationsPrefix = *annotationsPrefix // check port collisions @@ -297,6 +293,11 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g HTTPS: *httpsPort, SSLProxy: *sslProxyPort, }, + IngressClassConfiguration: &ingressclass.IngressClassConfiguration{ + Controller: *ingressClassController, + AnnotationValue: *ingressClassAnnotation, + WatchWithoutClass: *watchWithoutClass, + }, DisableCatchAll: *disableCatchAll, ValidationWebhook: *validationWebhook, ValidationWebhookCertPath: *validationWebhookCert, diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index b4d3932a1e..b21e1012ef 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -43,7 +43,6 @@ import ( "k8s.io/klog/v2" "k8s.io/ingress-nginx/internal/file" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/controller" "k8s.io/ingress-nginx/internal/ingress/metric" "k8s.io/ingress-nginx/internal/k8s" @@ -108,25 +107,13 @@ func main() { klog.Fatalf("ingress-nginx requires Kubernetes v1.19.0 or higher") } - k8s.IngressClass, err = kubeClient.NetworkingV1().IngressClasses(). - Get(context.TODO(), class.IngressClass, metav1.GetOptions{}) + _, err = kubeClient.NetworkingV1().IngressClasses().List(context.TODO(), metav1.ListOptions{}) if err != nil { if !errors.IsNotFound(err) { - if !errors.IsUnauthorized(err) && !errors.IsForbidden(err) { - klog.Fatalf("Error searching IngressClass: %v", err) + if errors.IsUnauthorized(err) || !errors.IsForbidden(err) { + klog.Fatalf("Error searching IngressClass: Please verify your RBAC and allow Ingress Controller to list and get Ingress Classes: %v", err) } - klog.ErrorS(err, "Searching IngressClass", "class", class.IngressClass) } - - klog.Warningf("No IngressClass resource with name %v found. Only annotation will be used.", class.IngressClass) - // TODO: remove once this is fixed in client-go - k8s.IngressClass = nil - - } - - if k8s.IngressClass != nil && k8s.IngressClass.Spec.Controller != k8s.IngressNGINXController { - klog.Errorf(`Invalid IngressClass (Spec.Controller) value "%v". Should be "%v"`, k8s.IngressClass.Spec.Controller, k8s.IngressNGINXController) - klog.Fatalf("IngressClass with name %v is not valid for ingress-nginx (invalid Spec.Controller)", class.IngressClass) } conf.Client = kubeClient @@ -146,7 +133,7 @@ func main() { mc := metric.NewDummyCollector() if conf.EnableMetrics { - mc, err = metric.NewCollector(conf.MetricsPerHost, reg) + mc, err = metric.NewCollector(conf.MetricsPerHost, reg, conf.IngressClassConfiguration.Controller) if err != nil { klog.Fatalf("Error creating prometheus collector: %v", err) } diff --git a/internal/ingress/annotations/class/main.go b/internal/ingress/annotations/class/main.go deleted file mode 100644 index 521ba9f0ff..0000000000 --- a/internal/ingress/annotations/class/main.go +++ /dev/null @@ -1,64 +0,0 @@ -/* -Copyright 2015 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package class - -import ( - networking "k8s.io/api/networking/v1" - "k8s.io/ingress-nginx/internal/k8s" -) - -const ( - // IngressKey picks a specific "class" for the Ingress. - // The controller only processes Ingresses with this annotation either - // unset, or set to either the configured value or the empty string. - IngressKey = "kubernetes.io/ingress.class" -) - -var ( - // DefaultClass defines the default class used in the nginx ingress controller - DefaultClass = "nginx" - - // IngressClass sets the runtime ingress class to use - // An empty string means accept all ingresses without - // annotation and the ones configured with class nginx - IngressClass = "nginx" -) - -// IsValid returns true if the given Ingress specify the ingress.class -// annotation or IngressClassName resource for Kubernetes >= v1.18 -func IsValid(ing *networking.Ingress) bool { - // 1. with annotation or IngressClass - ingress, ok := ing.GetAnnotations()[IngressKey] - if !ok && ing.Spec.IngressClassName != nil { - ingress = *ing.Spec.IngressClassName - } - - // empty ingress and IngressClass equal default - if len(ingress) == 0 && IngressClass == DefaultClass { - return true - } - - // k8s > v1.18. - // Processing may be redundant because k8s.IngressClass is obtained by IngressClass - // 3. without annotation and IngressClass. Check IngressClass - if k8s.IngressClass != nil { - return ingress == k8s.IngressClass.Name - } - - // 4. with IngressClass - return ingress == IngressClass -} diff --git a/internal/ingress/annotations/class/main_test.go b/internal/ingress/annotations/class/main_test.go deleted file mode 100644 index b2c863854e..0000000000 --- a/internal/ingress/annotations/class/main_test.go +++ /dev/null @@ -1,103 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package class - -import ( - "testing" - - api "k8s.io/api/core/v1" - networking "k8s.io/api/networking/v1" - meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/k8s" -) - -func TestIsValidClass(t *testing.T) { - dc := DefaultClass - ic := IngressClass - k8sic := k8s.IngressClass - v1Ready := k8s.IsIngressV1Ready - // restore original values after the tests - defer func() { - DefaultClass = dc - IngressClass = ic - k8s.IngressClass = k8sic - k8s.IsIngressV1Ready = v1Ready - }() - - tests := []struct { - ingress string - controller string - defClass string - annotation bool - ingressClassName bool - k8sClass *networking.IngressClass - v1Ready bool - isValid bool - }{ - {"", "", "nginx", true, false, nil, false, true}, - {"", "nginx", "nginx", true, false, nil, false, true}, - {"nginx", "nginx", "nginx", true, false, nil, false, true}, - {"custom", "custom", "nginx", true, false, nil, false, true}, - {"", "killer", "nginx", true, false, nil, false, false}, - {"custom", "nginx", "nginx", true, false, nil, false, false}, - {"nginx", "nginx", "nginx", false, true, nil, false, true}, - {"custom", "nginx", "nginx", false, true, nil, true, false}, - {"nginx", "nginx", "nginx", false, true, nil, true, true}, - {"", "custom", "nginx", false, false, - &networking.IngressClass{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "custom", - }, - }, - false, false}, - {"", "custom", "nginx", false, false, - &networking.IngressClass{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "custom", - }, - }, - true, false}, - } - - for _, test := range tests { - ing := &networking.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, - }, - } - - data := map[string]string{} - ing.SetAnnotations(data) - if test.annotation { - ing.Annotations[IngressKey] = test.ingress - } - if test.ingressClassName { - ing.Spec.IngressClassName = &[]string{test.ingress}[0] - } - - IngressClass = test.controller - DefaultClass = test.defClass - k8s.IngressClass = test.k8sClass - k8s.IsIngressV1Ready = test.v1Ready - - b := IsValid(ing) - if b != test.isValid { - t.Errorf("test %v - expected %v but %v was returned", test, test.isValid, b) - } - } -} diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 34457f640d..27cd2ffe43 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -33,11 +33,11 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/log" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/internal/ingress/annotations/proxy" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/internal/ingress/controller/store" "k8s.io/ingress-nginx/internal/ingress/errors" "k8s.io/ingress-nginx/internal/k8s" @@ -100,6 +100,8 @@ type Configuration struct { DisableCatchAll bool + IngressClassConfiguration *ingressclass.IngressClassConfiguration + ValidationWebhook string ValidationWebhookCertPath string ValidationWebhookKeyPath string @@ -221,11 +223,6 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error { return nil } - if !class.IsValid(ing) { - klog.Warningf("ignoring ingress %v in %v based on annotation %v", ing.Name, ing.ObjectMeta.Namespace, class.IngressKey) - return nil - } - if n.cfg.Namespace != "" && ing.ObjectMeta.Namespace != n.cfg.Namespace { klog.Warningf("ignoring ingress %v in namespace %v different from the namespace watched %s", ing.Name, ing.ObjectMeta.Namespace, n.cfg.Namespace) return nil diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 9ddf65dfaa..d33ac7d30f 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -45,6 +45,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/proxyssl" "k8s.io/ingress-nginx/internal/ingress/controller/config" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/internal/ingress/controller/store" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/metric" @@ -188,18 +189,6 @@ func TestCheckIngress(t *testing.T) { }, }, } - - t.Run("When the ingress class differs from nginx", func(t *testing.T) { - ing.ObjectMeta.Annotations["kubernetes.io/ingress.class"] = "different" - nginx.command = testNginxTestCommand{ - t: t, - err: fmt.Errorf("test error"), - } - if nginx.CheckIngress(ing) != nil { - t.Errorf("with a different ingress class, no error should be returned") - } - }) - t.Run("when the class is the nginx one", func(t *testing.T) { ing.ObjectMeta.Annotations["kubernetes.io/ingress.class"] = "nginx" nginx.command = testNginxTestCommand{ @@ -1899,7 +1888,12 @@ func newNGINXController(t *testing.T) *NGINXController { 10*time.Minute, clientSet, channels.NewRingChannel(10), - false) + false, + &ingressclass.IngressClassConfiguration{ + Controller: "k8s.io/ingress-nginx", + AnnotationValue: "nginx", + }, + ) sslCert := ssl.GetFakeSSLCert() config := &Configuration{ @@ -1957,7 +1951,11 @@ func newDynamicNginxController(t *testing.T, setConfigMap func(string) *v1.Confi 10*time.Minute, clientSet, channels.NewRingChannel(10), - false) + false, + &ingressclass.IngressClassConfiguration{ + Controller: "k8s.io/ingress-nginx", + AnnotationValue: "nginx", + }) sslCert := ssl.GetFakeSSLCert() config := &Configuration{ diff --git a/internal/ingress/controller/ingressclass/ingressclass.go b/internal/ingress/controller/ingressclass/ingressclass.go new file mode 100644 index 0000000000..025a4e2a5d --- /dev/null +++ b/internal/ingress/controller/ingressclass/ingressclass.go @@ -0,0 +1,45 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ingressclass + +const ( + // IngressKey picks a specific "class" for the Ingress. + // The controller only processes Ingresses with this annotation either + // unset, or set to either the configured value or the empty string. + IngressKey = "kubernetes.io/ingress.class" + + // DefaultControllerName defines the default controller name for Ingress NGINX + DefaultControllerName = "k8s.io/ingress-nginx" + + // DefaultAnnotationValue defines the default annotation value for the ingress-nginx controller + DefaultAnnotationValue = "nginx" +) + +// IngressClassConfiguration defines the various aspects of IngressClass parsing +// and how the controller should behave in each case +type IngressClassConfiguration struct { + // Controller defines the controller value this daemon watch to. + // Defaults to "k8s.io/ingress-nginx" defined in flags + Controller string + // AnnotationValue defines the annotation value this Controller watch to, in case of the + // ingressSpecName is not found but the annotation is. + // The Annotation is deprecated and should not be used in future releases + AnnotationValue string + // WatchWithoutClass defines if Controller should watch to Ingress Objects that does + // not contain an IngressClass configuration + WatchWithoutClass bool +} diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 9fa139cb7e..20f518f8e8 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -50,7 +50,6 @@ import ( adm_controller "k8s.io/ingress-nginx/internal/admission/controller" "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" "k8s.io/ingress-nginx/internal/ingress/controller/process" "k8s.io/ingress-nginx/internal/ingress/controller/store" @@ -131,7 +130,8 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro config.ResyncPeriod, config.Client, n.updateCh, - config.DisableCatchAll) + config.DisableCatchAll, + config.IngressClassConfiguration) n.syncQueue = task.NewTaskQueue(n.syncIngress) @@ -257,10 +257,10 @@ func (n *NGINXController) Start() { // we need to use the defined ingress class to allow multiple leaders // in order to update information about ingress status - electionID := fmt.Sprintf("%v-%v", n.cfg.ElectionID, class.DefaultClass) - if class.IngressClass != "" { - electionID = fmt.Sprintf("%v-%v", n.cfg.ElectionID, class.IngressClass) - } + // TODO: For now, as the the IngressClass logics has changed, is up to the + // cluster admin to create different Leader Election IDs. + // Should revisit this in a future + electionID := n.cfg.ElectionID setupLeaderElection(&leaderElectionConfig{ Client: n.cfg.Client, diff --git a/internal/ingress/controller/store/ingressclass.go b/internal/ingress/controller/store/ingressclass.go new file mode 100644 index 0000000000..da613d035d --- /dev/null +++ b/internal/ingress/controller/store/ingressclass.go @@ -0,0 +1,39 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package store + +import ( + networking "k8s.io/api/networking/v1" + "k8s.io/client-go/tools/cache" +) + +// IngressClassLister makes a Store that lists IngressClass. +type IngressClassLister struct { + cache.Store +} + +// ByKey returns the Ingress matching key in the local Ingress Store. +func (il IngressClassLister) ByKey(key string) (*networking.IngressClass, error) { + i, exists, err := il.GetByKey(key) + if err != nil { + return nil, err + } + if !exists { + return nil, NotExistsError(key) + } + return i.(*networking.IngressClass), nil +} diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index d4e8935154..39ba29abd9 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -41,14 +41,13 @@ import ( "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" - "k8s.io/utils/pointer" "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" ngx_template "k8s.io/ingress-nginx/internal/ingress/controller/template" "k8s.io/ingress-nginx/internal/ingress/defaults" "k8s.io/ingress-nginx/internal/ingress/errors" @@ -121,16 +120,18 @@ type Event struct { // Informer defines the required SharedIndexInformers that interact with the API server. type Informer struct { - Ingress cache.SharedIndexInformer - Endpoint cache.SharedIndexInformer - Service cache.SharedIndexInformer - Secret cache.SharedIndexInformer - ConfigMap cache.SharedIndexInformer + Ingress cache.SharedIndexInformer + IngressClass cache.SharedIndexInformer + Endpoint cache.SharedIndexInformer + Service cache.SharedIndexInformer + Secret cache.SharedIndexInformer + ConfigMap cache.SharedIndexInformer } // Lister contains object listers (stores). type Lister struct { Ingress IngressLister + IngressClass IngressClassLister Service ServiceLister Endpoint EndpointLister Secret SecretLister @@ -150,6 +151,7 @@ func (e NotExistsError) Error() string { func (i *Informer) Run(stopCh chan struct{}) { go i.Secret.Run(stopCh) go i.Endpoint.Run(stopCh) + go i.IngressClass.Run(stopCh) go i.Service.Run(stopCh) go i.ConfigMap.Run(stopCh) @@ -157,6 +159,7 @@ func (i *Informer) Run(stopCh chan struct{}) { // from the queue if !cache.WaitForCacheSync(stopCh, i.Endpoint.HasSynced, + i.IngressClass.HasSynced, i.Service.HasSynced, i.Secret.HasSynced, i.ConfigMap.HasSynced, @@ -221,7 +224,8 @@ func New( resyncPeriod time.Duration, client clientset.Interface, updateCh *channels.RingChannel, - disableCatchAll bool) Storer { + disableCatchAll bool, + icConfig *ingressclass.IngressClassConfiguration) Storer { store := &k8sStore{ informers: &Informer{}, @@ -296,6 +300,9 @@ func New( store.informers.Ingress = infFactory.Networking().V1().Ingresses().Informer() store.listers.Ingress.Store = store.informers.Ingress.GetStore() + store.informers.IngressClass = infFactory.Networking().V1().IngressClasses().Informer() + store.listers.IngressClass.Store = cache.NewStore(cache.MetaNamespaceKeyFunc) + store.informers.Endpoint = infFactory.Core().V1().Endpoints().Informer() store.listers.Endpoint.Store = store.informers.Endpoint.GetStore() @@ -324,7 +331,9 @@ func New( } } - if !class.IsValid(ing) { + _, err := store.GetIngressClass(ing, icConfig) + if err != nil { + klog.InfoS("Ignoring ingress because of error while validating ingress class", "ingress", klog.KObj(ing), "error", err) return } @@ -347,12 +356,14 @@ func New( ingEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { ing, _ := toIngress(obj) - if !class.IsValid(ing) { - ingressClass, _ := parser.GetStringAnnotation(class.IngressKey, ing) - klog.InfoS("Ignoring ingress", "ingress", klog.KObj(ing), "kubernetes.io/ingress.class", ingressClass, "ingressClassName", pointer.StringPtrDerefOr(ing.Spec.IngressClassName, "")) + ic, err := store.GetIngressClass(ing, icConfig) + if err != nil { + klog.InfoS("Ignoring ingress because of error while validating ingress class", "ingress", klog.KObj(ing), "error", err) return } + klog.InfoS("Found valid IngressClass", "ingress", klog.KObj(ing), "ingressclass", ic) + if hasCatchAllIngressRule(ing.Spec) && disableCatchAll { klog.InfoS("Ignoring add for catch-all ingress because of --disable-catch-all", "ingress", klog.KObj(ing)) return @@ -374,21 +385,21 @@ func New( oldIng, _ := toIngress(old) curIng, _ := toIngress(cur) - validOld := class.IsValid(oldIng) - validCur := class.IsValid(curIng) - if !validOld && validCur { + _, errOld := store.GetIngressClass(oldIng, icConfig) + classCur, errCur := store.GetIngressClass(curIng, icConfig) + if errOld != nil && errCur == nil { if hasCatchAllIngressRule(curIng.Spec) && disableCatchAll { klog.InfoS("ignoring update for catch-all ingress because of --disable-catch-all", "ingress", klog.KObj(curIng)) return } - klog.InfoS("creating ingress", "ingress", klog.KObj(curIng), "class", class.IngressKey) + klog.InfoS("creating ingress", "ingress", klog.KObj(curIng), "ingressclass", classCur) recorder.Eventf(curIng, corev1.EventTypeNormal, "Sync", "Scheduled for sync") - } else if validOld && !validCur { - klog.InfoS("removing ingress", "ingress", klog.KObj(curIng), "class", class.IngressKey) + } else if errOld == nil && errCur != nil { + klog.InfoS("removing ingress because of unknown ingressclass", "ingress", klog.KObj(curIng)) ingDeleteHandler(old) return - } else if validCur && !reflect.DeepEqual(old, cur) { + } else if errCur == nil && !reflect.DeepEqual(old, cur) { if hasCatchAllIngressRule(curIng.Spec) && disableCatchAll { klog.InfoS("ignoring update for catch-all ingress and delete old one because of --disable-catch-all", "ingress", klog.KObj(curIng)) ingDeleteHandler(old) @@ -412,6 +423,63 @@ func New( }, } + ingressClassEventHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + ingressclass := obj.(*networkingv1.IngressClass) + if ingressclass.Spec.Controller != icConfig.Controller { + klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(ingressclass)) + return + } + err := store.listers.IngressClass.Add(ingressclass) + if err != nil { + klog.InfoS("error adding ingressclass to store", "ingressclass", klog.KObj(ingressclass), "error", err) + return + } + + updateCh.In() <- Event{ + Type: CreateEvent, + Obj: obj, + } + }, + DeleteFunc: func(obj interface{}) { + ingressclass := obj.(*networkingv1.IngressClass) + if ingressclass.Spec.Controller != icConfig.Controller { + klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(ingressclass)) + return + } + err := store.listers.IngressClass.Delete(ingressclass) + if err != nil { + klog.InfoS("error removing ingressclass from store", "ingressclass", klog.KObj(ingressclass), "error", err) + return + } + updateCh.In() <- Event{ + Type: DeleteEvent, + Obj: obj, + } + }, + UpdateFunc: func(old, cur interface{}) { + oic := old.(*networkingv1.IngressClass) + cic := cur.(*networkingv1.IngressClass) + if cic.Spec.Controller != icConfig.Controller { + klog.InfoS("ignoring ingressclass as the spec.controller is not the same of this ingress", "ingressclass", klog.KObj(cic)) + return + } + // TODO: In a future we might be interested in parse parameters and use as + // current IngressClass for this case, crossing with configmap + if !reflect.DeepEqual(cic.Spec.Parameters, oic.Spec.Parameters) { + err := store.listers.IngressClass.Update(cic) + if err != nil { + klog.InfoS("error updating ingressclass in store", "ingressclass", klog.KObj(cic), "error", err) + return + } + updateCh.In() <- Event{ + Type: UpdateEvent, + Obj: cur, + } + } + }, + } + secrEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { sec := obj.(*corev1.Secret) @@ -608,6 +676,7 @@ func New( } store.informers.Ingress.AddEventHandler(ingEventHandler) + store.informers.IngressClass.AddEventHandler(ingressClassEventHandler) store.informers.Endpoint.AddEventHandler(epEventHandler) store.informers.Secret.AddEventHandler(secrEventHandler) store.informers.ConfigMap.AddEventHandler(cmEventHandler) @@ -758,6 +827,32 @@ func (s *k8sStore) GetService(key string) (*corev1.Service, error) { return s.listers.Service.ByKey(key) } +func (s *k8sStore) GetIngressClass(ing *networkingv1.Ingress, icConfig *ingressclass.IngressClassConfiguration) (string, error) { + // First we try ingressClassName + if ing.Spec.IngressClassName != nil { + iclass, err := s.listers.IngressClass.ByKey(*ing.Spec.IngressClassName) + if err != nil { + return "", err + } + return iclass.Name, nil + } + + // Then we try annotation + if ingressclass, ok := ing.GetAnnotations()[ingressclass.IngressKey]; ok { + if ingressclass != icConfig.AnnotationValue { + return "", fmt.Errorf("ingress class annotation is not equal to the expected by Ingress Controller") + } + return ingressclass, nil + } + + // Then we accept if the WithoutClass is enabled + if icConfig.WatchWithoutClass { + // Reserving "_" as a "wildcard" name + return "_", nil + } + return "", fmt.Errorf("ingress does not contain a valid IngressClass") +} + // getIngress returns the Ingress matching key. func (s *k8sStore) getIngress(key string) (*networkingv1.Ingress, error) { ing, err := s.listers.IngressWithAnnotation.ByKey(key) diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 1421f9111d..c84775ee58 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -37,13 +37,47 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/annotations/parser" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/test/e2e/framework" ) var pathPrefix networking.PathType = networking.PathTypePrefix +var DefaultClassConfig = &ingressclass.IngressClassConfiguration{ + Controller: ingressclass.DefaultControllerName, + AnnotationValue: ingressclass.DefaultAnnotationValue, + WatchWithoutClass: false, +} + +var ( + commonIngressSpec = networking.IngressSpec{ + Rules: []networking.IngressRule{ + { + Host: "dummy", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/", + PathType: &pathPrefix, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: "http-svc", + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +) + func TestStore(t *testing.T) { //TODO: move env definition to docker image? os.Setenv("KUBEBUILDER_ASSETS", "/usr/local/bin") @@ -86,7 +120,8 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) @@ -116,9 +151,10 @@ func TestStore(t *testing.T) { } }) - t.Run("should return one event for add, update and delete of ingress", func(t *testing.T) { + t.Run("should return no event for add, update and delete of ingress as the existing ingressclass is not the expected", func(t *testing.T) { ns := createNamespace(clientSet, t) defer deleteNamespace(ns, clientSet, t) + createConfigMap(clientSet, ns, t) stopCh := make(chan struct{}) @@ -163,40 +199,120 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) - + ic := createIngressClass(clientSet, t, "not-k8s.io/not-ingress-nginx") + defer deleteIngressClass(ic, clientSet, t) + validSpec := commonIngressSpec + validSpec.IngressClassName = &ic ing := ensureIngress(&networking.Ingress{ ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", + Name: "dummy-no-class", Namespace: ns, }, - Spec: networking.IngressSpec{ - Rules: []networking.IngressRule{ - { - Host: "dummy", - IngressRuleValue: networking.IngressRuleValue{ - HTTP: &networking.HTTPIngressRuleValue{ - Paths: []networking.HTTPIngressPath{ - { - Path: "/", - PathType: &pathPrefix, - Backend: networking.IngressBackend{ - Service: &networking.IngressServiceBackend{ - Name: "http-svc", - Port: networking.ServiceBackendPort{ - Number: 80, - }, - }, - }, - }, - }, - }, - }, - }, - }, + Spec: validSpec, + }, clientSet, t) + + err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name) + if err != nil { + t.Errorf("error waiting for secret: %v", err) + } + time.Sleep(1 * time.Second) + + ni := ing.DeepCopy() + ni.Spec.Rules[0].Host = "update-dummy" + _ = ensureIngress(ni, clientSet, t) + if err != nil { + t.Errorf("error creating ingress: %v", err) + } + // Secret takes a bit to update + time.Sleep(3 * time.Second) + + err = clientSet.NetworkingV1().Ingresses(ni.Namespace).Delete(context.TODO(), ni.Name, metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting ingress: %v", err) + } + + err = framework.WaitForNoIngressInNamespace(clientSet, ni.Namespace, ni.Name) + if err != nil { + t.Errorf("error waiting for secret: %v", err) + } + time.Sleep(1 * time.Second) + + if atomic.LoadUint64(&add) != 0 { + t.Errorf("expected 0 event of type Create but %v occurred", add) + } + if atomic.LoadUint64(&upd) != 0 { + t.Errorf("expected 0 event of type Update but %v occurred", upd) + } + if atomic.LoadUint64(&del) != 0 { + t.Errorf("expected 0 event of type Delete but %v occurred", del) + } + }) + + t.Run("should return one event for add, update and delete of ingress", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName) + defer deleteIngressClass(ic, clientSet, t) + createConfigMap(clientSet, ns, t) + + stopCh := make(chan struct{}) + updateCh := channels.NewRingChannel(1024) + + var add uint64 + var upd uint64 + var del uint64 + + go func(ch *channels.RingChannel) { + for { + evt, ok := <-ch.Out() + if !ok { + return + } + + e := evt.(Event) + if e.Obj == nil { + continue + } + if _, ok := e.Obj.(*networking.Ingress); !ok { + continue + } + + switch e.Type { + case CreateEvent: + atomic.AddUint64(&add, 1) + case UpdateEvent: + atomic.AddUint64(&upd, 1) + case DeleteEvent: + atomic.AddUint64(&del, 1) + } + } + }(updateCh) + + storer := New( + ns, + fmt.Sprintf("%v/config", ns), + fmt.Sprintf("%v/tcp", ns), + fmt.Sprintf("%v/udp", ns), + "", + 10*time.Minute, + clientSet, + updateCh, + false, + DefaultClassConfig) + + storer.Run(stopCh) + validSpec := commonIngressSpec + validSpec.IngressClassName = &ic + ing := ensureIngress(&networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy-class", + Namespace: ns, }, + Spec: validSpec, }, clientSet, t) err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name) @@ -205,40 +321,13 @@ func TestStore(t *testing.T) { } time.Sleep(1 * time.Second) - // create an invalid ingress (different class) + // create an invalid ingress (no ingress class and no watchWithoutClass config) invalidIngress := ensureIngress(&networking.Ingress{ ObjectMeta: metav1.ObjectMeta{ - Name: "custom-class", + Name: "no-class", Namespace: ns, - Annotations: map[string]string{ - class.IngressKey: "something", - }, - }, - Spec: networking.IngressSpec{ - Rules: []networking.IngressRule{ - { - Host: "dummy", - IngressRuleValue: networking.IngressRuleValue{ - HTTP: &networking.HTTPIngressRuleValue{ - Paths: []networking.HTTPIngressPath{ - { - Path: "/", - PathType: &pathPrefix, - Backend: networking.IngressBackend{ - Service: &networking.IngressServiceBackend{ - Name: "http-svc", - Port: networking.ServiceBackendPort{ - Number: 80, - }, - }, - }, - }, - }, - }, - }, - }, - }, }, + Spec: commonIngressSpec, }, clientSet, t) defer deleteIngress(invalidIngress, clientSet, t) @@ -253,7 +342,7 @@ func TestStore(t *testing.T) { err = clientSet.NetworkingV1().Ingresses(ni.Namespace).Delete(context.TODO(), ni.Name, metav1.DeleteOptions{}) if err != nil { - t.Errorf("error creating ingress: %v", err) + t.Errorf("error deleting ingress: %v", err) } err = framework.WaitForNoIngressInNamespace(clientSet, ni.Namespace, ni.Name) @@ -273,7 +362,132 @@ func TestStore(t *testing.T) { } }) - t.Run("should not receive updates for ingress with invalid class", func(t *testing.T) { + t.Run("should return two events for add and delete and one for update of ingress and watch-without-class", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + createConfigMap(clientSet, ns, t) + + stopCh := make(chan struct{}) + updateCh := channels.NewRingChannel(1024) + + var add uint64 + var upd uint64 + var del uint64 + + go func(ch *channels.RingChannel) { + for { + evt, ok := <-ch.Out() + if !ok { + return + } + + e := evt.(Event) + if e.Obj == nil { + continue + } + if _, ok := e.Obj.(*networking.Ingress); !ok { + continue + } + + switch e.Type { + case CreateEvent: + atomic.AddUint64(&add, 1) + case UpdateEvent: + atomic.AddUint64(&upd, 1) + case DeleteEvent: + atomic.AddUint64(&del, 1) + } + } + }(updateCh) + + ingressClassconfig := &ingressclass.IngressClassConfiguration{ + Controller: ingressclass.DefaultControllerName, + AnnotationValue: ingressclass.DefaultAnnotationValue, + WatchWithoutClass: true, + } + + storer := New( + ns, + fmt.Sprintf("%v/config", ns), + fmt.Sprintf("%v/tcp", ns), + fmt.Sprintf("%v/udp", ns), + "", + 10*time.Minute, + clientSet, + updateCh, + false, + ingressClassconfig) + + storer.Run(stopCh) + + validIngress1 := ensureIngress(&networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing1", + Namespace: ns, + }, + Spec: commonIngressSpec, + }, clientSet, t) + err := framework.WaitForIngressInNamespace(clientSet, ns, validIngress1.Name) + if err != nil { + t.Errorf("error waiting for ingress: %v", err) + } + + otherIngress := commonIngressSpec + otherIngress.Rules[0].Host = "other-ingress" + validIngress2 := ensureIngress(&networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ing2", + Namespace: ns, + }, + Spec: otherIngress, + }, clientSet, t) + err = framework.WaitForIngressInNamespace(clientSet, ns, validIngress2.Name) + if err != nil { + t.Errorf("error waiting for ingress: %v", err) + } + + time.Sleep(1 * time.Second) + + validIngressUpdated := validIngress1.DeepCopy() + validIngressUpdated.Spec.Rules[0].Host = "update-dummy" + _ = ensureIngress(validIngressUpdated, clientSet, t) + if err != nil { + t.Errorf("error updating ingress: %v", err) + } + // Secret takes a bit to update + time.Sleep(3 * time.Second) + + err = clientSet.NetworkingV1().Ingresses(validIngressUpdated.Namespace).Delete(context.TODO(), validIngressUpdated.Name, metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting ingress: %v", err) + } + err = clientSet.NetworkingV1().Ingresses(validIngress2.Namespace).Delete(context.TODO(), validIngress2.Name, metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting ingress: %v", err) + } + + err = framework.WaitForNoIngressInNamespace(clientSet, validIngressUpdated.Namespace, validIngressUpdated.Name) + if err != nil { + t.Errorf("error waiting for ingress deletion: %v", err) + } + err = framework.WaitForNoIngressInNamespace(clientSet, validIngress2.Namespace, validIngress2.Name) + if err != nil { + t.Errorf("error waiting for ingress deletion: %v", err) + } + time.Sleep(1 * time.Second) + + if atomic.LoadUint64(&add) != 2 { + t.Errorf("expected 0 event of type Create but %v occurred", add) + } + if atomic.LoadUint64(&upd) != 1 { + t.Errorf("expected 0 event of type Update but %v occurred", upd) + } + if atomic.LoadUint64(&del) != 2 { + t.Errorf("expected 0 event of type Delete but %v occurred", del) + } + }) + + t.Run("should not receive updates for ingress with invalid class annotation", func(t *testing.T) { ns := createNamespace(clientSet, t) defer deleteNamespace(ns, clientSet, t) createConfigMap(clientSet, ns, t) @@ -285,6 +499,7 @@ func TestStore(t *testing.T) { var upd uint64 var del uint64 + // TODO: This repeats a lot, transform in a local function go func(ch *channels.RingChannel) { for { evt, ok := <-ch.Out() @@ -320,7 +535,8 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) @@ -330,34 +546,101 @@ func TestStore(t *testing.T) { Name: "custom-class", Namespace: ns, Annotations: map[string]string{ - class.IngressKey: "something", + ingressclass.IngressKey: "something", }, }, - Spec: networking.IngressSpec{ - Rules: []networking.IngressRule{ - { - Host: "dummy", - IngressRuleValue: networking.IngressRuleValue{ - HTTP: &networking.HTTPIngressRuleValue{ - Paths: []networking.HTTPIngressPath{ - { - Path: "/", - PathType: &pathPrefix, - Backend: networking.IngressBackend{ - Service: &networking.IngressServiceBackend{ - Name: "http-svc", - Port: networking.ServiceBackendPort{ - Number: 80, - }, - }, - }, - }, - }, - }, - }, - }, - }, + Spec: commonIngressSpec, + }, clientSet, t) + err := framework.WaitForIngressInNamespace(clientSet, ns, invalidIngress.Name) + if err != nil { + t.Errorf("error waiting for ingress: %v", err) + } + time.Sleep(1 * time.Second) + + invalidIngressUpdated := invalidIngress.DeepCopy() + invalidIngressUpdated.Spec.Rules[0].Host = "update-dummy" + _ = ensureIngress(invalidIngressUpdated, clientSet, t) + if err != nil { + t.Errorf("error creating ingress: %v", err) + } + // Secret takes a bit to update + time.Sleep(3 * time.Second) + + if atomic.LoadUint64(&add) != 0 { + t.Errorf("expected 0 event of type Create but %v occurred", add) + } + if atomic.LoadUint64(&upd) != 0 { + t.Errorf("expected 0 event of type Update but %v occurred", upd) + } + if atomic.LoadUint64(&del) != 0 { + t.Errorf("expected 0 event of type Delete but %v occurred", del) + } + }) + + t.Run("should not receive updates for ingress with invalid class specification", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName) + defer deleteIngressClass(ic, clientSet, t) + + createConfigMap(clientSet, ns, t) + + stopCh := make(chan struct{}) + updateCh := channels.NewRingChannel(1024) + + var add uint64 + var upd uint64 + var del uint64 + + go func(ch *channels.RingChannel) { + for { + evt, ok := <-ch.Out() + if !ok { + return + } + + e := evt.(Event) + if e.Obj == nil { + continue + } + if _, ok := e.Obj.(*networking.Ingress); !ok { + continue + } + + switch e.Type { + case CreateEvent: + atomic.AddUint64(&add, 1) + case UpdateEvent: + atomic.AddUint64(&upd, 1) + case DeleteEvent: + atomic.AddUint64(&del, 1) + } + } + }(updateCh) + + storer := New( + ns, + fmt.Sprintf("%v/config", ns), + fmt.Sprintf("%v/tcp", ns), + fmt.Sprintf("%v/udp", ns), + "", + 10*time.Minute, + clientSet, + updateCh, + false, + DefaultClassConfig) + + storer.Run(stopCh) + invalidSpec := commonIngressSpec + invalidClassName := "blo123" + invalidSpec.IngressClassName = &invalidClassName + // create an invalid ingress (different class) + invalidIngress := ensureIngress(&networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom-class", + Namespace: ns, }, + Spec: invalidSpec, }, clientSet, t) err := framework.WaitForIngressInNamespace(clientSet, ns, invalidIngress.Name) if err != nil { @@ -428,7 +711,8 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) @@ -474,6 +758,8 @@ func TestStore(t *testing.T) { t.Run("should receive events from secret referenced from ingress", func(t *testing.T) { ns := createNamespace(clientSet, t) defer deleteNamespace(ns, clientSet, t) + ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName) + defer deleteIngressClass(ic, clientSet, t) createConfigMap(clientSet, ns, t) stopCh := make(chan struct{}) @@ -494,6 +780,11 @@ func TestStore(t *testing.T) { if e.Obj == nil { continue } + + // We should skip IngressClass events + if _, ok := e.Obj.(*networking.IngressClass); ok { + continue + } switch e.Type { case CreateEvent: atomic.AddUint64(&add, 1) @@ -514,7 +805,8 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) @@ -527,6 +819,7 @@ func TestStore(t *testing.T) { Namespace: ns, }, Spec: networking.IngressSpec{ + IngressClassName: &ic, TLS: []networking.IngressTLS{ { SecretName: secretName, @@ -586,6 +879,8 @@ func TestStore(t *testing.T) { t.Run("should create an ingress with a secret which does not exist", func(t *testing.T) { ns := createNamespace(clientSet, t) defer deleteNamespace(ns, clientSet, t) + ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName) + defer deleteIngressClass(ic, clientSet, t) createConfigMap(clientSet, ns, t) stopCh := make(chan struct{}) @@ -606,6 +901,12 @@ func TestStore(t *testing.T) { if e.Obj == nil { continue } + + // We should skip IngressClass objects here + if _, ok := e.Obj.(*networking.IngressClass); ok { + continue + } + switch e.Type { case CreateEvent: atomic.AddUint64(&add, 1) @@ -626,7 +927,8 @@ func TestStore(t *testing.T) { 10*time.Minute, clientSet, updateCh, - false) + false, + DefaultClassConfig) storer.Run(stopCh) @@ -639,6 +941,7 @@ func TestStore(t *testing.T) { Namespace: ns, }, Spec: networking.IngressSpec{ + IngressClassName: &ic, TLS: []networking.IngressTLS{ { Hosts: secretHosts, @@ -733,6 +1036,33 @@ func deleteNamespace(ns string, clientSet kubernetes.Interface, t *testing.T) { } } +func createIngressClass(clientSet kubernetes.Interface, t *testing.T, controller string) string { + t.Helper() + ingressclass := &networking.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("ingress-nginx-%v", time.Now().Unix()), + //Namespace: "xpto" // TODO: We don't support namespaced ingress-class yet + }, + Spec: networking.IngressClassSpec{ + Controller: controller, + }, + } + ic, err := clientSet.NetworkingV1().IngressClasses().Create(context.TODO(), ingressclass, metav1.CreateOptions{}) + if err != nil { + t.Errorf("error creating ingress class: %v", err) + } + return ic.Name +} + +func deleteIngressClass(ic string, clientSet kubernetes.Interface, t *testing.T) { + t.Helper() + + err := clientSet.NetworkingV1().IngressClasses().Delete(context.TODO(), ic, metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting the ingress class: %v", err) + } +} + func createConfigMap(clientSet kubernetes.Interface, ns string, t *testing.T) string { t.Helper() @@ -790,6 +1120,7 @@ func newStore(t *testing.T) *k8sStore { return &k8sStore{ listers: &Lister{ // add more listers if needed + IngressClass: IngressClassLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, IngressWithAnnotation: IngressWithAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)}, }, @@ -867,18 +1198,18 @@ func TestUpdateSecretIngressMap(t *testing.T) { func TestListIngresses(t *testing.T) { s := newStore(t) + invalidIngressClass := "something" + validIngressClass := ingressclass.DefaultControllerName ingressToIgnore := &ingress.Ingress{ Ingress: networking.Ingress{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-2", - Namespace: "testns", - Annotations: map[string]string{ - class.IngressKey: "something", - }, + Name: "test-2", + Namespace: "testns", CreationTimestamp: metav1.NewTime(time.Now()), }, Spec: networking.IngressSpec{ + IngressClassName: &invalidIngressClass, DefaultBackend: &networking.IngressBackend{ Service: &networking.IngressServiceBackend{ Name: "demo", @@ -900,6 +1231,7 @@ func TestListIngresses(t *testing.T) { CreationTimestamp: metav1.NewTime(time.Now()), }, Spec: networking.IngressSpec{ + IngressClassName: &validIngressClass, Rules: []networking.IngressRule{ { Host: "foo.bar", @@ -926,13 +1258,13 @@ func TestListIngresses(t *testing.T) { } s.listers.IngressWithAnnotation.Add(ingressWithoutPath) - ingressWithNginxClass := &ingress.Ingress{ + ingressWithNginxClassAnnotation := &ingress.Ingress{ Ingress: networking.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "test-4", Namespace: "testns", Annotations: map[string]string{ - class.IngressKey: "nginx", + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, }, CreationTimestamp: metav1.NewTime(time.Now()), }, @@ -963,7 +1295,7 @@ func TestListIngresses(t *testing.T) { }, }, } - s.listers.IngressWithAnnotation.Add(ingressWithNginxClass) + s.listers.IngressWithAnnotation.Add(ingressWithNginxClassAnnotation) ingresses := s.ListIngresses() diff --git a/internal/ingress/metric/main.go b/internal/ingress/metric/main.go index d85e1c9798..64810dd362 100644 --- a/internal/ingress/metric/main.go +++ b/internal/ingress/metric/main.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/metric/collectors" ) @@ -66,7 +65,7 @@ type collector struct { } // NewCollector creates a new metric collector the for ingress controller -func NewCollector(metricsPerHost bool, registry *prometheus.Registry) (Collector, error) { +func NewCollector(metricsPerHost bool, registry *prometheus.Registry, ingressclass string) (Collector, error) { podNamespace := os.Getenv("POD_NAMESPACE") if podNamespace == "" { podNamespace = "default" @@ -74,22 +73,22 @@ func NewCollector(metricsPerHost bool, registry *prometheus.Registry) (Collector podName := os.Getenv("POD_NAME") - nc, err := collectors.NewNGINXStatus(podName, podNamespace, class.IngressClass) + nc, err := collectors.NewNGINXStatus(podName, podNamespace, ingressclass) if err != nil { return nil, err } - pc, err := collectors.NewNGINXProcess(podName, podNamespace, class.IngressClass) + pc, err := collectors.NewNGINXProcess(podName, podNamespace, ingressclass) if err != nil { return nil, err } - s, err := collectors.NewSocketCollector(podName, podNamespace, class.IngressClass, metricsPerHost) + s, err := collectors.NewSocketCollector(podName, podNamespace, ingressclass, metricsPerHost) if err != nil { return nil, err } - ic := collectors.NewController(podName, podNamespace, class.IngressClass) + ic := collectors.NewController(podName, podNamespace, ingressclass) return Collector(&collector{ nginxStatus: nc, diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index 9578c8f731..79bb85891a 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -29,7 +29,7 @@ import ( testclient "k8s.io/client-go/kubernetes/fake" "k8s.io/ingress-nginx/internal/ingress" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/internal/k8s" "k8s.io/ingress-nginx/internal/task" ) @@ -214,7 +214,7 @@ func buildExtensionsIngresses() []networking.Ingress { Name: "foo_ingress_different_class", Namespace: metav1.NamespaceDefault, Annotations: map[string]string{ - class.IngressKey: "no-nginx", + ingressclass.IngressKey: "no-nginx", }, }, Status: networking.IngressStatus{ diff --git a/internal/k8s/main.go b/internal/k8s/main.go index 8539c7d2fd..1487e892ac 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -124,9 +124,6 @@ func MetaNamespaceKey(obj interface{}) string { // IsIngressV1Ready indicates if the running Kubernetes version is at least v1.19.0 var IsIngressV1Ready bool -// IngressClass indicates the class of the Ingress to use as filter -var IngressClass *networkingv1.IngressClass - // IngressNGINXController defines the valid value of IngressClass // Controller field for ingress-nginx const IngressNGINXController = "k8s.io/ingress-nginx" diff --git a/test/e2e-image/namespace-overlays/forwarded-port-headers/values.yaml b/test/e2e-image/namespace-overlays/forwarded-port-headers/values.yaml index c79154d289..4fef671a76 100644 --- a/test/e2e-image/namespace-overlays/forwarded-port-headers/values.yaml +++ b/test/e2e-image/namespace-overlays/forwarded-port-headers/values.yaml @@ -14,7 +14,9 @@ controller: https-port: "1443" # e2e tests do not require information about ingress status update-status: "false" - + ingressClassResource: + # We will create and remove each IC/ClusterRole/ClusterRoleBinding per test so there's no conflict + enabled: false scope: enabled: true diff --git a/test/e2e/admission/admission.go b/test/e2e/admission/admission.go index 71b86200d6..121d0d37aa 100644 --- a/test/e2e/admission/admission.go +++ b/test/e2e/admission/admission.go @@ -121,11 +121,7 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() { assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error") }) - ginkgo.It("should not return an error if the Ingress V1 definition is valid", func() { - if !f.IsIngressV1Ready { - ginkgo.Skip("Test requires Kubernetes v1.19 or higher") - } - + ginkgo.It("should not return an error if the Ingress V1 definition is valid with Ingress Class", func() { err := createIngress(f.Namespace, validV1Ingress) assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl") @@ -140,15 +136,26 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() { Status(http.StatusOK) }) - ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() { - if !f.IsIngressV1Ready { - ginkgo.Skip("Test requires Kubernetes v1.19 or higher") - } + ginkgo.It("should not return an error if the Ingress V1 definition is valid with IngressClass annotation", func() { + err := createIngress(f.Namespace, validV1IngressAnnotation) + assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl") + + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "extensions-class") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", "extensions-class"). + Expect(). + Status(http.StatusOK) + }) + ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() { err := createIngress(f.Namespace, invalidV1Ingress) assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl") - _, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), "extensions", metav1.GetOptions{}) + _, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), "extensions-invalid", metav1.GetOptions{}) if !apierrors.IsNotFound(err) { assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error") } @@ -172,6 +179,7 @@ kind: Ingress metadata: name: extensions spec: + ingressClassName: nginx rules: - host: extensions http: @@ -184,6 +192,28 @@ spec: port: number: 80 +--- +` + validV1IngressAnnotation = ` +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: extensions-class + annotations: + kubernetes.io/ingress.class: nginx +spec: + rules: + - host: extensions-class + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: echo + port: + number: 80 + --- ` @@ -191,13 +221,14 @@ spec: apiVersion: networking.k8s.io/v1 kind: Ingress metadata: - name: extensions + name: extensions-invalid annotations: nginx.ingress.kubernetes.io/configuration-snippet: | invalid directive spec: + ingressClassName: nginx rules: - - host: extensions + - host: extensions-invalid http: paths: - path: / diff --git a/test/e2e/annotations/affinity.go b/test/e2e/annotations/affinity.go index a4512ce151..998eca82f7 100644 --- a/test/e2e/annotations/affinity.go +++ b/test/e2e/annotations/affinity.go @@ -131,6 +131,7 @@ var _ = framework.DescribeAnnotation("affinity session-cookie-name", func() { Annotations: annotations, }, Spec: networking.IngressSpec{ + IngressClassName: &f.IngressClass, Rules: []networking.IngressRule{ { Host: host, diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index b452ef3b0e..fe3e1544f6 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -22,9 +22,7 @@ import ( "strings" "github.com/onsi/ginkgo" - "github.com/stretchr/testify/assert" - networking "k8s.io/api/networking/v1" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -32,7 +30,7 @@ const ( canaryService = "echo-canary" ) -var _ = framework.DescribeAnnotation("canary-*", func() { +var _ = framework.DescribeAnnotation("canary", func() { f := framework.NewDefaultFramework("canary") ginkgo.BeforeEach(func() { @@ -327,15 +325,25 @@ var _ = framework.DescribeAnnotation("canary-*", func() { f.Namespace, canaryService, 80, canaryAnnotations) f.EnsureIngress(canaryIng) - err := framework.UpdateIngress(f.KubeClientSet, f.Namespace, canaryIngName, - func(ingress *networking.Ingress) error { - ingress.ObjectMeta.Annotations = map[string]string{ - "nginx.ingress.kubernetes.io/canary": "true", - "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader2", - } - return nil + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + newAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader2", + } + + modIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, newAnnotations) + + f.UpdateIngress(modIng) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") }) - assert.Nil(ginkgo.GinkgoT(), err) ginkgo.By("routing requests destined for the mainline ingress to the mainline upstream") f.HTTPTestClient(). @@ -707,6 +715,11 @@ var _ = framework.DescribeAnnotation("canary-*", func() { f.Namespace, canaryService, 80, canaryAnnotations) f.EnsureIngress(canaryIng) + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + ginkgo.By("returning requests from the mainline only when weight is equal to 0") f.HTTPTestClient(). GET("/"). @@ -719,15 +732,20 @@ var _ = framework.DescribeAnnotation("canary-*", func() { ginkgo.By("returning requests from the canary only when weight is equal to 100") - err := framework.UpdateIngress(f.KubeClientSet, f.Namespace, canaryIngName, - func(ingress *networking.Ingress) error { - ingress.ObjectMeta.Annotations = map[string]string{ - "nginx.ingress.kubernetes.io/canary": "true", - "nginx.ingress.kubernetes.io/canary-weight": "100", - } - return nil + newAnnotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-weight": "100", + } + + modIng := framework.NewSingleIngress(canaryIngName, "/", host, + f.Namespace, canaryService, 80, newAnnotations) + + f.UpdateIngress(modIng) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") }) - assert.Nil(ginkgo.GinkgoT(), err) f.HTTPTestClient(). GET("/"). diff --git a/test/e2e/defaultbackend/with_hosts.go b/test/e2e/defaultbackend/with_hosts.go index 89c5f08a5c..c59b5807b9 100644 --- a/test/e2e/defaultbackend/with_hosts.go +++ b/test/e2e/defaultbackend/with_hosts.go @@ -47,6 +47,7 @@ var _ = framework.IngressNginxDescribe("[Default Backend] change default setting Annotations: annotations, }, Spec: networking.IngressSpec{ + IngressClassName: &f.IngressClass, DefaultBackend: &networking.IngressBackend{ Service: &networking.IngressServiceBackend{ Name: framework.EchoService, diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index f1baf80be8..e31fd1e4ee 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -68,7 +68,8 @@ type Framework struct { KubeConfig *restclient.Config APIExtensionsClientSet apiextcs.Interface - Namespace string + Namespace string + IngressClass string pod *corev1.Pod } @@ -108,6 +109,9 @@ func (f *Framework) BeforeEach() { f.Namespace, err = CreateKubeNamespace(f.BaseName, f.KubeClientSet) assert.Nil(ginkgo.GinkgoT(), err, "creating namespace") + f.IngressClass, err = CreateIngressClass(f.Namespace, f.KubeClientSet) + assert.Nil(ginkgo.GinkgoT(), err, "creating IngressClass") + err = f.newIngressController(f.Namespace, f.BaseName) assert.Nil(ginkgo.GinkgoT(), err, "deploying the ingress controller") @@ -127,6 +131,14 @@ func (f *Framework) AfterEach() { }() }(f.KubeClientSet, f.Namespace) + defer func(kubeClient kubernetes.Interface, ingressclass string) { + go func() { + defer ginkgo.GinkgoRecover() + err := deleteIngressClass(kubeClient, ingressclass) + assert.Nil(ginkgo.GinkgoT(), err, "deleting IngressClass") + }() + }(f.KubeClientSet, f.IngressClass) + if !ginkgo.CurrentGinkgoTestDescription().Failed { return } @@ -580,6 +592,7 @@ func NewSingleIngress(name, path, host, ns, service string, port int, annotation func NewSingleIngressWithMultiplePaths(name string, paths []string, host, ns, service string, port int, annotations map[string]string) *networking.Ingress { pathtype := networking.PathTypePrefix spec := networking.IngressSpec{ + IngressClassName: GetIngressClassName(ns), Rules: []networking.IngressRule{ { Host: host, @@ -611,6 +624,7 @@ func NewSingleIngressWithMultiplePaths(name string, paths []string, host, ns, se func newSingleIngressWithRules(name, path, host, ns, service string, port int, annotations map[string]string, tlsHosts []string) *networking.Ingress { pathtype := networking.PathTypePrefix spec := networking.IngressSpec{ + IngressClassName: GetIngressClassName(ns), Rules: []networking.IngressRule{ { IngressRuleValue: networking.IngressRuleValue{ @@ -656,6 +670,7 @@ func newSingleIngressWithRules(name, path, host, ns, service string, port int, a func NewSingleIngressWithBackendAndRules(name, path, host, ns, defaultService string, defaultPort int, service string, port int, annotations map[string]string) *networking.Ingress { pathtype := networking.PathTypePrefix spec := networking.IngressSpec{ + IngressClassName: GetIngressClassName(ns), DefaultBackend: &networking.IngressBackend{ Service: &networking.IngressServiceBackend{ Name: defaultService, @@ -695,6 +710,7 @@ func NewSingleIngressWithBackendAndRules(name, path, host, ns, defaultService st // NewSingleCatchAllIngress creates a simple ingress with a catch-all backend func NewSingleCatchAllIngress(name, ns, service string, port int, annotations map[string]string) *networking.Ingress { spec := networking.IngressSpec{ + IngressClassName: GetIngressClassName(ns), DefaultBackend: &networking.IngressBackend{ Service: &networking.IngressServiceBackend{ Name: service, diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index b0fa92b153..75fcb58ea4 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -24,6 +24,8 @@ import ( "github.com/onsi/ginkgo" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/uuid" @@ -31,6 +33,8 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd/api" + + "k8s.io/ingress-nginx/internal/k8s" ) const ( @@ -117,6 +121,93 @@ func deleteKubeNamespace(c kubernetes.Interface, namespace string) error { }) } +// CreateIngressClass creates a new IngressClass related to a test/namespace and also +// the required ClusterRole/ClusterRoleBinding +func CreateIngressClass(namespace string, c kubernetes.Interface) (string, error) { + icname := fmt.Sprintf("ic-%s", namespace) + var err error + + ic, err := c.NetworkingV1().IngressClasses(). + Create(context.TODO(), &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: icname, + }, + Spec: networkingv1.IngressClassSpec{ + Controller: k8s.IngressNGINXController, + }, + }, metav1.CreateOptions{}) + if err != nil { + return "", fmt.Errorf("Unexpected error creating IngressClass %s: %v", icname, err) + } + + _, err = c.RbacV1().ClusterRoles().Create(context.TODO(), &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{Name: icname}, + Rules: []rbacv1.PolicyRule{{ + APIGroups: []string{"networking.k8s.io"}, + Resources: []string{"ingressclasses"}, + Verbs: []string{"get", "list", "watch"}, + }}, + }, metav1.CreateOptions{}) + if err != nil { + return "", fmt.Errorf("Unexpected error creating IngressClass ClusterRole %s: %v", icname, err) + } + + _, err = c.RbacV1().ClusterRoleBindings().Create(context.TODO(), &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: icname, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: icname, + }, + Subjects: []rbacv1.Subject{ + { + APIGroup: "", + Kind: "ServiceAccount", + Namespace: namespace, + Name: "nginx-ingress", + }, + }, + }, metav1.CreateOptions{}) + if err != nil { + return "", fmt.Errorf("Unexpected error creating IngressClass ClusterRoleBinding %s: %v", icname, err) + } + return ic.Name, nil +} + +//deleteIngressClass deletes an IngressClass and its related ClusterRole* objects +func deleteIngressClass(c kubernetes.Interface, ingressclass string) error { + var err error + grace := int64(0) + pb := metav1.DeletePropagationBackground + deleteOptions := metav1.DeleteOptions{ + GracePeriodSeconds: &grace, + PropagationPolicy: &pb, + } + err = c.NetworkingV1().IngressClasses().Delete(context.TODO(), ingressclass, deleteOptions) + if err != nil { + return fmt.Errorf("Unexpected error deleting IngressClass %s: %v", ingressclass, err) + } + + err = c.RbacV1().ClusterRoleBindings().Delete(context.TODO(), ingressclass, deleteOptions) + if err != nil { + return fmt.Errorf("Unexpected error deleting IngressClass ClusterRoleBinding %s: %v", ingressclass, err) + } + err = c.RbacV1().ClusterRoles().Delete(context.TODO(), ingressclass, deleteOptions) + if err != nil { + return fmt.Errorf("Unexpected error deleting IngressClass ClusterRole %s: %v", ingressclass, err) + } + + return nil +} + +//GetIngressClassName returns the default IngressClassName given a namespace +func GetIngressClassName(namespace string) *string { + icname := fmt.Sprintf("ic-%s", namespace) + return &icname +} + // WaitForKubeNamespaceNotExist waits until a namespaces is not present in the cluster func WaitForKubeNamespaceNotExist(c kubernetes.Interface, namespace string) error { return wait.Poll(Poll, DefaultTimeout, namespaceNotExist(c, namespace)) diff --git a/test/e2e/ingress/without_host.go b/test/e2e/ingress/without_host.go index 88cf62f00c..c0c2d3b12e 100644 --- a/test/e2e/ingress/without_host.go +++ b/test/e2e/ingress/without_host.go @@ -61,6 +61,7 @@ var _ = framework.IngressNginxDescribe("[Ingress] definition without host", func Namespace: f.Namespace, }, Spec: networking.IngressSpec{ + IngressClassName: &f.IngressClass, DefaultBackend: &networking.IngressBackend{ Service: &networking.IngressServiceBackend{ Name: framework.EchoService, diff --git a/test/e2e/run-chart-test.sh b/test/e2e/run-chart-test.sh index 841c05e7df..0e618244ca 100755 --- a/test/e2e/run-chart-test.sh +++ b/test/e2e/run-chart-test.sh @@ -36,6 +36,8 @@ cleanup() { trap cleanup EXIT +export KIND_CLUSTER_NAME=${KIND_CLUSTER_NAME:-ingress-nginx-dev} + if ! command -v kind --version &> /dev/null; then echo "kind is not installed. Use the package manager or visit the official site https://kind.sigs.k8s.io/" exit 1 @@ -43,14 +45,17 @@ fi DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -export KIND_CLUSTER_NAME=${KIND_CLUSTER_NAME:-ingress-nginx-dev} +# Use 1.0.0-dev to make sure we use the latest configuration in the helm template +export TAG=1.0.0-dev +export ARCH=${ARCH:-amd64} +export REGISTRY=ingress-controller export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/kind-config-$KIND_CLUSTER_NAME}" if [ "${SKIP_CLUSTER_CREATION:-false}" = "false" ]; then echo "[dev-env] creating Kubernetes cluster with kind" - export K8S_VERSION=${K8S_VERSION:-v1.20.2@sha256:8f7ea6e7642c0da54f04a7ee10431549c0257315b3a634f6ef2fecaaedb19bab} + export K8S_VERSION=${K8S_VERSION:-v1.21.1@sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6} kind create cluster \ --verbosity=${KIND_LOG_LEVEL} \ @@ -61,8 +66,23 @@ if [ "${SKIP_CLUSTER_CREATION:-false}" = "false" ]; then echo "Kubernetes cluster:" kubectl get nodes -o wide + fi +if [ "${SKIP_IMAGE_CREATION:-false}" = "false" ]; then + if ! command -v ginkgo &> /dev/null; then + go get github.com/onsi/ginkgo/ginkgo@v1.16.4 + fi + echo "[dev-env] building image" + make -C ${DIR}/../../ clean-image build image +fi + + +KIND_WORKERS=$(kind get nodes --name="${KIND_CLUSTER_NAME}" | awk '{printf (NR>1?",":"") $1}') +echo "[dev-env] copying docker images to cluster..." + +kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes=${KIND_WORKERS} ${REGISTRY}/controller:${TAG} + echo "[dev-env] running helm chart e2e tests..." # Uses a custom chart-testing image to avoid timeouts waiting for namespace deletion. # The changes can be found here: https://github.com/aledbf/chart-testing/commit/41fe0ae0733d0c9a538099fb3cec522e888e3d82 diff --git a/test/e2e/run.sh b/test/e2e/run.sh index d3caf2537a..bfe13b428f 100755 --- a/test/e2e/run.sh +++ b/test/e2e/run.sh @@ -58,7 +58,7 @@ export KUBECONFIG="${KUBECONFIG:-$HOME/.kube/kind-config-$KIND_CLUSTER_NAME}" if [ "${SKIP_CLUSTER_CREATION:-false}" = "false" ]; then echo "[dev-env] creating Kubernetes cluster with kind" - export K8S_VERSION=${K8S_VERSION:-v1.20.2@sha256:8f7ea6e7642c0da54f04a7ee10431549c0257315b3a634f6ef2fecaaedb19bab} + export K8S_VERSION=${K8S_VERSION:-v1.21.1@sha256:69860bda5563ac81e3c0057d654b5253219618a22ec3a346306239bba8cfa1a6} kind create cluster \ --verbosity=${KIND_LOG_LEVEL} \ @@ -73,7 +73,7 @@ fi if [ "${SKIP_IMAGE_CREATION:-false}" = "false" ]; then if ! command -v ginkgo &> /dev/null; then - go get github.com/onsi/ginkgo/ginkgo + go get github.com/onsi/ginkgo/ginkgo@v1.16.4 fi echo "[dev-env] building image" diff --git a/test/e2e/servicebackend/service_backend.go b/test/e2e/servicebackend/service_backend.go index 9fe883b7ee..0467e434e3 100644 --- a/test/e2e/servicebackend/service_backend.go +++ b/test/e2e/servicebackend/service_backend.go @@ -80,6 +80,7 @@ func buildIngressWithNonexistentService(host, namespace, path string) *networkin Namespace: namespace, }, Spec: networking.IngressSpec{ + IngressClassName: framework.GetIngressClassName(namespace), Rules: []networking.IngressRule{ { Host: host, @@ -115,6 +116,7 @@ func buildIngressWithUnavailableServiceEndpoints(host, namespace, path string) ( Namespace: namespace, }, Spec: networking.IngressSpec{ + IngressClassName: framework.GetIngressClassName(namespace), Rules: []networking.IngressRule{ { Host: host, diff --git a/test/e2e/servicebackend/service_externalname.go b/test/e2e/servicebackend/service_externalname.go index 437b35144b..2c33c020f3 100644 --- a/test/e2e/servicebackend/service_externalname.go +++ b/test/e2e/servicebackend/service_externalname.go @@ -25,7 +25,6 @@ import ( "github.com/gavv/httpexpect/v2" "github.com/onsi/ginkgo" "github.com/stretchr/testify/assert" - core "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -43,7 +42,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -74,7 +73,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName without a port defined", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -108,7 +107,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName with a port defined", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -149,7 +148,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return status 502 for service type=ExternalName with an invalid host", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -180,7 +179,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName using a port name", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -230,7 +229,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should return 200 for service type=ExternalName using FQDN with trailing dot", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, @@ -261,7 +260,7 @@ var _ = framework.IngressNginxDescribe("[Service] Type ExternalName", func() { ginkgo.It("should update the external name after a service update", func() { host := "echo" - svc := &core.Service{ + svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: framework.HTTPBinService, Namespace: f.Namespace, diff --git a/test/e2e/settings/global_external_auth.go b/test/e2e/settings/global_external_auth.go old mode 100755 new mode 100644 diff --git a/test/e2e/settings/ingress_class.go b/test/e2e/settings/ingress_class.go index 646efc7fac..3ba42f3113 100644 --- a/test/e2e/settings/ingress_class.go +++ b/test/e2e/settings/ingress_class.go @@ -18,7 +18,6 @@ package settings import ( "context" - "fmt" "net/http" "strings" "sync" @@ -26,13 +25,11 @@ import ( "github.com/onsi/ginkgo" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" - networking "k8s.io/api/networking/v1" - rbacv1 "k8s.io/api/rbac/v1" + networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/ingress-nginx/internal/ingress/annotations/class" - "k8s.io/ingress-nginx/internal/k8s" + "k8s.io/ingress-nginx/internal/ingress/controller/ingressclass" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -41,39 +38,20 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() { var doOnce sync.Once - testIngressClassName := "test-new-ingress-class" + otherIngressClassName := "test-new-ingress-class" + otherController := "k8s.io/other-class" ginkgo.BeforeEach(func() { f.NewEchoDeploymentWithReplicas(1) doOnce.Do(func() { - f.KubeClientSet.RbacV1().ClusterRoles().Create(context.TODO(), &rbacv1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{Name: "ingress-nginx-class"}, - Rules: []rbacv1.PolicyRule{{ - APIGroups: []string{"networking.k8s.io"}, - Resources: []string{"ingressclasses"}, - Verbs: []string{"get", "list", "watch"}, - }}, - }, metav1.CreateOptions{}) - - f.KubeClientSet.RbacV1().ClusterRoleBindings().Create(context.TODO(), &rbacv1.ClusterRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ingress-nginx-class", - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "ingress-nginx-class", - }, - }, metav1.CreateOptions{}) - _, err := f.KubeClientSet.NetworkingV1().IngressClasses(). - Create(context.TODO(), &networking.IngressClass{ + Create(context.TODO(), &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ - Name: testIngressClassName, + Name: otherIngressClassName, }, - Spec: networking.IngressClassSpec{ - Controller: k8s.IngressNGINXController, + Spec: networkingv1.IngressClassSpec{ + Controller: otherController, }, }, metav1.CreateOptions{}) @@ -83,17 +61,23 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() { }) }) - ginkgo.Context("Without a specific ingress-class", func() { - ginkgo.It("should ignore Ingress with class", func() { + ginkgo.Context("With default ingress class config", func() { + ginkgo.It("should ignore Ingress with a different class annotation", func() { invalidHost := "foo" annotations := map[string]string{ - class.IngressKey: "testclass", + ingressclass.IngressKey: "testclass", } ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, annotations) + // We should drop the ingressClassName here as we just want to rely on the annotation in this test + ing.Spec.IngressClassName = nil f.EnsureIngress(ing) validHost := "bar" - ing = framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, nil) + annotationClass := map[string]string{ + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, + } + ing = framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, annotationClass) + ing.Spec.IngressClassName = nil f.EnsureIngress(ing) f.WaitForNginxConfiguration(func(cfg string) bool { @@ -113,222 +97,417 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() { Expect(). Status(http.StatusOK) }) - }) - ginkgo.Context("With a specific ingress-class", func() { - ginkgo.BeforeEach(func() { - err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { - args := []string{} - for _, v := range deployment.Spec.Template.Spec.Containers[0].Args { - if strings.Contains(v, "--ingress-class") { - continue - } - - args = append(args, v) - } + ginkgo.It("should ignore Ingress with different controller class", func() { + invalidHost := "foo-1" + ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = &otherIngressClassName + f.EnsureIngress(ing) - args = append(args, "--ingress-class=testclass") - deployment.Spec.Template.Spec.Containers[0].Args = args - _, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) + validHost := "bar-1" + ing = framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) - return err + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo-1") && + strings.Contains(cfg, "server_name bar-1") }) - assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags") - }) - ginkgo.It("should ignore Ingress with no class", func() { - invalidHost := "bar" + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", invalidHost). + Expect(). + Status(http.StatusNotFound) - ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, nil) - f.EnsureIngress(ing) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHost). + Expect(). + Status(http.StatusOK) + }) - validHost := "foo" - annotations := map[string]string{ - class.IngressKey: "testclass", + ginkgo.It("should accept both Ingresses with default IngressClassName and IngressClass annotation", func() { + validHostAnnotation := "foo-ok" + annotationClass := map[string]string{ + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, } - ing = framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, annotations) + ing := framework.NewSingleIngress(validHostAnnotation, "/", validHostAnnotation, f.Namespace, framework.EchoService, 80, annotationClass) + // We need to drop the Class here as we just want the annotation + ing.Spec.IngressClassName = nil f.EnsureIngress(ing) - f.WaitForNginxServer(validHost, func(cfg string) bool { - return strings.Contains(cfg, "server_name foo") - }) + validHostClass := "bar-ok" + ing = framework.NewSingleIngress(validHostClass, "/", validHostClass, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) f.WaitForNginxConfiguration(func(cfg string) bool { - return !strings.Contains(cfg, "server_name bar") + return strings.Contains(cfg, "server_name foo-ok") && + strings.Contains(cfg, "server_name bar-ok") }) f.HTTPTestClient(). GET("/"). - WithHeader("Host", validHost). + WithHeader("Host", validHostAnnotation). Expect(). Status(http.StatusOK) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHostClass). + Expect(). + Status(http.StatusOK) + }) + + ginkgo.It("should ignore Ingress without IngressClass configuration", func() { + invalidHost := "foo-invalid" + ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + validHostClass := "bar-valid" + ing = framework.NewSingleIngress(validHostClass, "/", validHostClass, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo-invalid") && + strings.Contains(cfg, "server_name bar-valid") + }) + f.HTTPTestClient(). GET("/"). WithHeader("Host", invalidHost). Expect(). Status(http.StatusNotFound) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHostClass). + Expect(). + Status(http.StatusOK) }) ginkgo.It("should delete Ingress when class is removed", func() { - host := "foo" + hostAnnotation := "foo-annotation" + annotations := map[string]string{ - class.IngressKey: "testclass", + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, } - ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations) + ing := framework.NewSingleIngress(hostAnnotation, "/", hostAnnotation, f.Namespace, framework.EchoService, 80, annotations) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + hostClass := "foo-class" + ing = framework.NewSingleIngress(hostClass, "/", hostClass, f.Namespace, framework.EchoService, 80, nil) f.EnsureIngress(ing) - f.WaitForNginxServer(host, func(cfg string) bool { - return strings.Contains(cfg, "server_name foo") + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "server_name foo-annotation") && + strings.Contains(cfg, "server_name foo-class") }) f.HTTPTestClient(). GET("/"). - WithHeader("Host", host). + WithHeader("Host", hostAnnotation). Expect(). Status(http.StatusOK) - ing, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), host, metav1.GetOptions{}) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostClass). + Expect(). + Status(http.StatusOK) + + ing, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostAnnotation, metav1.GetOptions{}) assert.Nil(ginkgo.GinkgoT(), err) - delete(ing.Annotations, class.IngressKey) + delete(ing.Annotations, ingressclass.IngressKey) _, err = f.KubeClientSet.NetworkingV1().Ingresses(ing.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{}) assert.Nil(ginkgo.GinkgoT(), err) + ingWithClass, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostClass, metav1.GetOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + ingWithClass.Spec.IngressClassName = nil + _, err = f.KubeClientSet.NetworkingV1().Ingresses(ingWithClass.Namespace).Update(context.TODO(), ingWithClass, metav1.UpdateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + framework.Sleep() f.WaitForNginxConfiguration(func(cfg string) bool { - return !strings.Contains(cfg, "server_name foo") + return !strings.Contains(cfg, "server_name foo-annotation") && + !strings.Contains(cfg, "server_name foo-class") }) f.HTTPTestClient(). GET("/"). - WithHeader("Host", host). + WithHeader("Host", hostAnnotation). + Expect(). + Status(http.StatusNotFound) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostClass). Expect(). Status(http.StatusNotFound) }) - }) - ginkgo.It("check scenarios for IngressClass and ingress.class annotation", func() { + ginkgo.It("should serve Ingress when class is added", func() { + hostNoAnnotation := "foo-no-annotation" - pod := f.GetIngressNGINXPod() + ing := framework.NewSingleIngress(hostNoAnnotation, "/", hostNoAnnotation, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) - crb, err := f.KubeClientSet.RbacV1().ClusterRoleBindings().Get(context.Background(), "ingress-nginx-class", metav1.GetOptions{}) - assert.Nil(ginkgo.GinkgoT(), err, "searching cluster role binding") + hostNoClass := "foo-no-class" + ing = framework.NewSingleIngress(hostNoClass, "/", hostNoClass, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) - // add service of current namespace - crb.Subjects = append(crb.Subjects, rbacv1.Subject{ - APIGroup: "", - Kind: "ServiceAccount", - Name: pod.Spec.ServiceAccountName, - Namespace: f.Namespace, - }) + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo-no-nnotation") && + !strings.Contains(cfg, "server_name foo-no-class") + }) - _, err = f.KubeClientSet.RbacV1().ClusterRoleBindings().Update(context.Background(), crb, metav1.UpdateOptions{}) - assert.Nil(ginkgo.GinkgoT(), err, "searching cluster role binding") + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostNoAnnotation). + Expect(). + Status(http.StatusNotFound) - err = f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { - args := []string{} - for _, v := range deployment.Spec.Template.Spec.Containers[0].Args { - if strings.Contains(v, "--ingress-class") { - continue - } + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostNoClass). + Expect(). + Status(http.StatusNotFound) - args = append(args, v) + ing, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostNoAnnotation, metav1.GetOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + annotation := map[string]string{ + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, } + ing.Annotations = annotation + _, err = f.KubeClientSet.NetworkingV1().Ingresses(ing.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + ingWithClass, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostNoClass, metav1.GetOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + ingWithClass.Spec.IngressClassName = framework.GetIngressClassName(f.Namespace) + _, err = f.KubeClientSet.NetworkingV1().Ingresses(ingWithClass.Namespace).Update(context.TODO(), ingWithClass, metav1.UpdateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) - args = append(args, fmt.Sprintf("--ingress-class=%v", testIngressClassName)) - deployment.Spec.Template.Spec.Containers[0].Args = args - _, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) - return err + framework.Sleep() + + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "server_name foo-no-annotation") && + strings.Contains(cfg, "server_name foo-no-class") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostNoAnnotation). + Expect(). + Status(http.StatusOK) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostNoClass). + Expect(). + Status(http.StatusOK) }) - assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags") - host := "ingress.class" + ginkgo.It("should serve Ingress when class is updated between annotation and ingressClassName", func() { + hostAnnotation2class := "foo-annotation2class" + annotationClass := map[string]string{ + ingressclass.IngressKey: ingressclass.DefaultAnnotationValue, + } + ing := framework.NewSingleIngress(hostAnnotation2class, "/", hostAnnotation2class, f.Namespace, framework.EchoService, 80, annotationClass) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + hostClass2Annotation := "foo-class2annotation" + ing = framework.NewSingleIngress(hostClass2Annotation, "/", hostClass2Annotation, f.Namespace, framework.EchoService, 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "server_name foo-annotation2class") && + strings.Contains(cfg, "server_name foo-class2annotation") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostAnnotation2class). + Expect(). + Status(http.StatusOK) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostClass2Annotation). + Expect(). + Status(http.StatusOK) + + ingAnnotation2Class, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostAnnotation2class, metav1.GetOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) - ginkgo.By("only having IngressClassName") - ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil) - ing.Spec.IngressClassName = &testIngressClassName - f.EnsureIngress(ing) + delete(ingAnnotation2Class.Annotations, ingressclass.IngressKey) + ingAnnotation2Class.Spec.IngressClassName = framework.GetIngressClassName(ingAnnotation2Class.Namespace) + _, err = f.KubeClientSet.NetworkingV1().Ingresses(ingAnnotation2Class.Namespace).Update(context.TODO(), ingAnnotation2Class, metav1.UpdateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + ingClass2Annotation, err := f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), hostClass2Annotation, metav1.GetOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) - f.WaitForNginxConfiguration(func(cfg string) bool { - return strings.Contains(cfg, fmt.Sprintf("server_name %v", host)) + ingClass2Annotation.Spec.IngressClassName = nil + ingClass2Annotation.Annotations = annotationClass + _, err = f.KubeClientSet.NetworkingV1().Ingresses(ingClass2Annotation.Namespace).Update(context.TODO(), ingClass2Annotation, metav1.UpdateOptions{}) + assert.Nil(ginkgo.GinkgoT(), err) + + framework.Sleep() + + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "server_name foo-annotation2class") && + strings.Contains(cfg, "server_name foo-class2annotation") + }) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostAnnotation2class). + Expect(). + Status(http.StatusOK) + + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", hostClass2Annotation). + Expect(). + Status(http.StatusOK) }) - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - Expect(). - Status(http.StatusOK) + }) - ginkgo.By("only having ingress.class annotation") - ing, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), host, metav1.GetOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) + ginkgo.Context("With specific ingress-class flags", func() { + ginkgo.BeforeEach(func() { + err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { + args := []string{} + for _, v := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.Contains(v, "--ingress-class") && strings.Contains(v, "--controller-class") { + continue + } - ing.Annotations = map[string]string{ - class.IngressKey: testIngressClassName, - } - ing.Spec.IngressClassName = nil + args = append(args, v) + } - _, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) + args = append(args, "--ingress-class=testclass") + args = append(args, "--controller-class=k8s.io/other-class") + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) - f.WaitForNginxConfiguration(func(cfg string) bool { - return strings.Contains(cfg, fmt.Sprintf("server_name %v", host)) + return err + }) + assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags") }) - framework.Sleep() + ginkgo.It("should ignore Ingress with no class and accept the correctly configured Ingresses", func() { + invalidHost := "bar" + + ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + validHost := "foo" + annotations := map[string]string{ + ingressclass.IngressKey: "testclass", + } + ing = framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, annotations) + // Delete the IngressClass as we want just the annotation here + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + validHostClass := "foobar123" + ing = framework.NewSingleIngress(validHostClass, "/", validHostClass, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = &otherIngressClassName + f.EnsureIngress(ing) - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - Expect(). - Status(http.StatusOK) + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name bar") && + strings.Contains(cfg, "server_name foo") && + strings.Contains(cfg, "server_name foobar123") + }) - ginkgo.By("having an invalid ingress.class annotation and no IngressClassName") - ing, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), host, metav1.GetOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHost). + Expect(). + Status(http.StatusOK) - ing.Annotations = map[string]string{ - class.IngressKey: "invalid", - } - ing.Spec.IngressClassName = nil + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHostClass). + Expect(). + Status(http.StatusOK) - _, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", invalidHost). + Expect(). + Status(http.StatusNotFound) + }) - framework.Sleep() + }) - f.WaitForNginxConfiguration(func(cfg string) bool { - return !strings.Contains(cfg, fmt.Sprintf("server_name %v", host)) + ginkgo.Context("With watch-ingress-without-class flag", func() { + ginkgo.BeforeEach(func() { + err := f.UpdateIngressControllerDeployment(func(deployment *appsv1.Deployment) error { + args := []string{} + for _, v := range deployment.Spec.Template.Spec.Containers[0].Args { + if strings.Contains(v, "--watch-ingress-without-class") && strings.Contains(v, "--controller-class") { + continue + } + + args = append(args, v) + } + + args = append(args, "--watch-ingress-without-class") + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{}) + + return err + }) + assert.Nil(ginkgo.GinkgoT(), err, "updating ingress controller deployment flags") }) - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - Expect(). - Status(http.StatusNotFound) + ginkgo.It("should watch Ingress with no class and ignore ingress with a different class", func() { + validHost := "bar" - ginkgo.By("not having ingress.class annotation and invalid IngressClassName") - ing, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Get(context.TODO(), host, metav1.GetOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) - ing.Annotations = map[string]string{} - invalidClassName := "invalidclass" - ing.Spec.IngressClassName = &invalidClassName + ing := framework.NewSingleIngress(validHost, "/", validHost, f.Namespace, framework.EchoService, 80, nil) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) - _, err = f.KubeClientSet.NetworkingV1().Ingresses(f.Namespace).Update(context.TODO(), ing, metav1.UpdateOptions{}) - assert.Nil(ginkgo.GinkgoT(), err) + invalidHost := "foo" + annotations := map[string]string{ + ingressclass.IngressKey: "testclass123", + } + ing = framework.NewSingleIngress(invalidHost, "/", invalidHost, f.Namespace, framework.EchoService, 80, annotations) + ing.Spec.IngressClassName = nil + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return strings.Contains(cfg, "server_name bar") && + !strings.Contains(cfg, "server_name foo") + }) - framework.Sleep() + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", validHost). + Expect(). + Status(http.StatusOK) - f.WaitForNginxConfiguration(func(cfg string) bool { - return !strings.Contains(cfg, fmt.Sprintf("server_name %v", host)) + f.HTTPTestClient(). + GET("/"). + WithHeader("Host", invalidHost). + Expect(). + Status(http.StatusNotFound) }) - f.HTTPTestClient(). - GET("/"). - WithHeader("Host", host). - Expect(). - Status(http.StatusNotFound) }) }) diff --git a/test/e2e/settings/no_auth_locations.go b/test/e2e/settings/no_auth_locations.go index 8475b02181..2d32b05d6b 100644 --- a/test/e2e/settings/no_auth_locations.go +++ b/test/e2e/settings/no_auth_locations.go @@ -106,6 +106,7 @@ func buildBasicAuthIngressWithSecondPath(host, namespace, secretName, pathName s }, }, Spec: networking.IngressSpec{ + IngressClassName: framework.GetIngressClassName(namespace), Rules: []networking.IngressRule{ { Host: host, diff --git a/test/e2e/settings/server_tokens.go b/test/e2e/settings/server_tokens.go index d65d7b0213..e84639b081 100644 --- a/test/e2e/settings/server_tokens.go +++ b/test/e2e/settings/server_tokens.go @@ -57,6 +57,7 @@ var _ = framework.DescribeSetting("server-tokens", func() { Annotations: map[string]string{}, }, Spec: networking.IngressSpec{ + IngressClassName: &f.IngressClass, Rules: []networking.IngressRule{ { Host: serverTokens, diff --git a/test/e2e/status/update.go b/test/e2e/status/update.go index 07d30243f8..23679afdc2 100644 --- a/test/e2e/status/update.go +++ b/test/e2e/status/update.go @@ -94,7 +94,7 @@ var _ = framework.IngressNginxDescribe("[Status] status update", func() { err = f.KubeClientSet.CoreV1(). ConfigMaps(f.Namespace). - Delete(context.TODO(), "ingress-controller-leader-nginx", metav1.DeleteOptions{}) + Delete(context.TODO(), "ingress-controller-leader", metav1.DeleteOptions{}) assert.Nil(ginkgo.GinkgoT(), err, "unexpected error deleting leader election configmap") _, cmd, err = f.KubectlProxy(port) diff --git a/test/e2e/wait-for-nginx.sh b/test/e2e/wait-for-nginx.sh index 1eb8b32ccd..9a37d1ffcf 100755 --- a/test/e2e/wait-for-nginx.sh +++ b/test/e2e/wait-for-nginx.sh @@ -73,6 +73,10 @@ controller: periodSeconds: 1 service: type: NodePort + electionID: ingress-controller-leader + ingressClassResource: + # We will create and remove each IC/ClusterRole/ClusterRoleBinding per test so there's no conflict + enabled: false extraArgs: tcp-services-configmap: $NAMESPACE/tcp-services # e2e tests do not require information about ingress status