New ingress controller chart

Makefile

@@ -7,7 +7,7 @@ DOCKER_TAG            ?= $(USER)
 # default helm chart version must be 0.0.42 for local development (because 42 is the answer to the universe and everything)
 HELM_SEMVER           ?= 0.0.42
 # The list of helm charts needed on internal kubernetes testing environments
-CHARTS_INTEGRATION    := wire-server databases-ephemeral redis-cluster fake-aws nginx-ingress-controller nginx-ingress-services fluent-bit kibana sftd restund coturn
+CHARTS_INTEGRATION    := wire-server databases-ephemeral redis-cluster fake-aws ingress-nginx-controller nginx-ingress-controller nginx-ingress-services fluent-bit kibana sftd restund coturn
 # The list of helm charts to publish on S3
 # FUTUREWORK: after we "inline local subcharts",
 # (e.g. move charts/brig to charts/wire-server/brig)
@@ -17,7 +17,7 @@ CHARTS_RELEASE := wire-server redis-ephemeral redis-cluster databases-ephemeral
 fake-aws fake-aws-s3 fake-aws-sqs aws-ingress fluent-bit kibana backoffice		\
 calling-test demo-smtp elasticsearch-curator elasticsearch-external				\
 elasticsearch-ephemeral minio-external cassandra-external						\
-nginx-ingress-controller nginx-ingress-services reaper sftd restund coturn		\
+nginx-ingress-controller ingress-nginx-controller nginx-ingress-services reaper sftd restund coturn		\
 inbucket k8ssandra-test-cluster postgresql
 KIND_CLUSTER_NAME     := wire-server
 HELM_PARALLELISM      ?= 1 # 1 for sequential tests; 6 for all-parallel tests

changelog.d/0-release-notes/ingress-controller

@@ -0,0 +1,35 @@
+New 'ingress-nginx-controller' wrapper chart compatible with kubernetes versions [1.23 - 1.26]. The old one 'nginx-ingress-controller' (compatible only up to k8s 1.19) is now DEPRECATED.
+We advise to upgrade your version of kubernetes in use to 1.23 or higher (we tested on kubernetes version 1.26), and to make use of the new ingress controller chart. Main features:
+- up-to-date nginx version ('1.21.6')
+- TLS 1.3 support (including allowing specifying which cipher suites to use)
+- security fixes
+- no more accidental logging of Wire access tokens under specific circumstances
+
+The 'kind: Ingress' resources installed via 'nginx-ingress-services' chart remain compatible with both the old and the new ingress controller, and k8s versions [1.18 - 1.26]. In case you upgrade an existing kubernetes cluster (not recommended), you may need to first uninstall the old controller before installing the new controller chart.
+
+In case you have custom overrides, you need to modify the directory name and top-level configuration key:
+
+```diff
+# If you have overrides for the controller chart (such as cipher suites), ensure to rename file and top-level key:
+-# nginx-ingress-controller/values.yaml
++# ingress-nginx-controller/values.yaml
+-nginx-ingress:
++ingress-nginx:
+   controller:
+     # ...
+```
+
+and double-check if all overrides you use are indeed provided under the same name by the upstream chart. See also the default overrides in [the default values.yaml](https://github.com/wireapp/wire-server/blob/develop/charts/ingress-nginx-controller/values.yaml).
+
+In case you use helmfile change your ingress controller like this:
+
+```diff
+# helmfile.yaml
+releases:
+-  - name: 'nginx-ingress-controller'
++  - name: 'ingress-nginx-controller'
+     namespace: 'wire'
+-    chart: 'wire/nginx-ingress-controller'
++    chart: 'wire/ingress-nginx-controller'
+     version: 'CHANGE_ME'
+```

changelog.d/5-internal/ingress-controller

@@ -0,0 +1,5 @@
+- integration tests on CI will use either the old or the new ingress controller; depending on which kubernetes version they run on.
+- upgrade `kubectl` to default from the nixpkgs channel (currently `1.26`) by removing the manual version pin on 1.19
+- upgrade `helmfile` to default from the nixpkgs channel by removing the manual version pin
+- upgrade `helm` to default from the nixpkgs channel by removing the manual version pin
+- add `kubelogin-oidc` so the kubectl in this environment can also talk to kubernetes clusters using OIDC

charts/ingress-nginx-controller/Chart.yaml

@@ -0,0 +1,8 @@
+apiVersion: v2
+description: A Helm chart for an ingress controller (using nginx) on Kubernetes
+name: ingress-nginx-controller
+version: 0.0.42
+dependencies:
+- name: ingress-nginx
+  version: 4.5.2 # k8s compatibility [1.23 - 1.26]
+  repository: https://kubernetes.github.io/ingress-nginx

charts/ingress-nginx-controller/values.yaml

@@ -0,0 +1,44 @@
+# the following defaults apply to an on-prem bare-metal setup in the same spirit as the
+# older similarly named wrapper chart 'nginx-ingress-controller' (note the swapped words
+# 'nginx' and 'ingress') We assume no load balancer support and instead expose NodePorts
+# on ports 31773 and 31772, assuming traffic gets to these ports in another way.
+#
+# See https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/values.yaml for all possible values to override.
+ingress-nginx:
+  controller:
+    enableTopologyAwareRouting: true
+    # -- Use a `DaemonSet` or `Deployment`
+    kind: DaemonSet
+    service:
+      type: NodePort  # or LoadBalancer
+      externalTrafficPolicy: Local
+      nodePorts:
+        # The nginx instance is exposed on ports 31773 (https) and 31772 (http)
+        # on the node on which it runs. You should add a port-forwarding rule
+        # on the node or on the loadbalancer that forwards ports 443 and 80 to
+        # these respective ports.
+        https: 31773
+        http: 31772
+    config:
+      # NOTE: These are some sane defaults (compliant to TR-02102-2), you may want to overrride them on your own installation
+      # For TR-02102-2 see https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TG02102/BSI-TR-02102-2.html
+      # As a Wire employee, for Wire-internal discussions and context see
+      # * https://wearezeta.atlassian.net/browse/FS-33
+      # * https://wearezeta.atlassian.net/browse/FS-444
+      ssl-protocols: "TLSv1.2 TLSv1.3"
+      # override cipher suites used in TLS 1.2 (only, if TLS 1.2 is used)
+      ssl-ciphers: "ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384"
+      # override cipher suites used in TLS 1.3 (only, if TLS 1.3 is used)
+      server-snippet: "ssl_conf_command Ciphersuites TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384;"
+      # used to be called http2-max-(header|field)-size, removed in controller v1.3
+      large-client-header-buffers: "16 32k"
+      proxy-buffer-size: "16k"
+      proxy-body-size: "1024m"
+      hsts-max-age: "31536000"
+      # Override log format to remove logging access tokens:
+      # removes 'request_query: "$args"', since it can include '?access_token=...'
+      # (sometimes sent for assets and websocket establishments)
+      # We do not wish to log these (SEC-47)
+      # Also add ssl/tls protocol/cipher to gain some observability here (can we turn off TLS 1.2?)
+      log-format-escape-json: true
+      log-format-upstream: '{"bytes_sent": "$bytes_sent", "duration": "$request_time", "http_referrer": "$http_referer", "http_user_agent": "$http_user_agent", "method": "$request_method", "path": "$uri", "remote_addr": "$proxy_protocol_addr", "remote_user": "$remote_user", "request_id": "$req_id", "request_length": "$request_length", "request_proto": "$server_protocol", "request_time": "$request_time", "status": "$status", "time": "$time_iso8601", "tls_cipher": "$ssl_cipher", "tls_protocol": "$ssl_protocol", "vhost": "$host", "x_forwarded_for": "$proxy_add_x_forwarded_for"}'

charts/nginx-ingress-controller/Chart.yaml

@@ -1,4 +1,5 @@
 apiVersion: v1
-description: A Helm chart for an ingress controller (using nginx) on Kubernetes
+description: ingress-controller. DEPRECATED. Use ingress-nginx-controller chart instead.
 name: nginx-ingress-controller
 version: 0.0.42
+deprecated: true

charts/nginx-ingress-controller/README.md

@@ -1,3 +1,5 @@
+WARNING: deprecated. Use ingress-nginx-controller instead, if possible.
+
 This deploys a single ingress controller - ideally, you want this on a separate, shared namespace since controllers listen on all namespaces by default (you can also modify that but it's generally discouraged).
 
 It is mostly a wrapper of the [nginx-ingress](https://github.com/helm/charts/blob/master/stable/nginx-ingress/README.md) with some other defaults that make sense for our use case(s).

charts/nginx-ingress-controller/values.yaml

@@ -43,6 +43,7 @@ nginx-ingress:
 # downsides of this setting
 #   https://www.asykim.com/blog/deep-dive-into-kubernetes-external-traffic-policies
     kind: DaemonSet
+    ingressClass: nginx
     # By default, each node will now be configured to accept ingress traffic. You should add
     # all the nodes to your external load balancer, or add them to DNS records.
     #

charts/nginx-ingress-services/README.md

@@ -1,5 +1,5 @@
 This helm chart is a helper to set up needed services, ingresses and (likely) secrets to access your cluster.
-It will _NOT_ deploy an ingress controller! Ensure you already have one on your cluster - or have a look at our [nginx-ingress-controller](../nginx-ingress-controller/README.md)
+It will _NOT_ deploy an ingress controller! Ensure you already have one on your cluster - or have a look at our [ingress-nginx-controller](../ingress-nginx-controller/README.md)
 
 If tls.enabled == true, then you need to supply 2 variables, `tlsWildcardCert` and `tlsWildcardKey` that could either be supplied as plain text in the form of a `-f path/to/secrets.yaml`, like this:
 
@@ -28,27 +28,19 @@ Q: My ingress keeps serving "Kubernetes Ingress Controller Fake Certificate"!!
 
 A: Ensure that your certificate is _valid_ and has _not expired_; trying to serve expired certificates will silently fail and the nginx ingress will simply fallback to the default certificate.
 
-
 ## About cert-manager
 
 ### Prerequisites
 
-* `cert-manager` and its CRDs have to be installed upfront, 
+* `cert-manager` and its CRDs have to be installed upfront,
    e.g. `helm upgrade --install -n cert-manager-ns --set 'installCRDs=true' cert-manager jetstack/cert-manager`,
    because upstream decided that this is the way (https://github.com/jetstack/cert-manager/pull/2964)
 
-
 ### What does this chart do?
 
 * define `Ingress` for various services and their corresponding FQDNS
 * do TLS termination either by explicitly providing a wildcard certificate or letting
   *cert-manager* take care of this
 * [optional] configure an *Issuer* to issue ACME HTTP01 certificates provided by Letsencrypt
 * [optional] define a *Certificate* representation that causes *cert-manager* to issue a
-  certificate that is then used by `Ingress` 
-
-
-### Todo when introducing support for K8s >= 1.15 
-
-* the `apiVersion` of all resources based on cert-manager's CRDs, namely `./templates/issuer.yaml` and 
-  `./templates/certificate.yaml`, has to be changed to `cert-manager.io/v1alpha3`
+  certificate that is then used by `Ingress`

charts/nginx-ingress-services/templates/_helpers.tpl

@@ -86,3 +86,7 @@ Returns the Letsencrypt API server URL based on whether testMode is enabled or d
 {{- define "ingress.supportsPathType" -}}
   {{- or (eq (include "ingress.isStable" .) "true") (and (eq (include "ingress.apiVersion" .) "networking.k8s.io/v1beta1") (semverCompare ">= 1.18-0" (include "kubeVersion" .))) -}}
 {{- end -}}
+
+{{- define "integrationTestHelperNewLabels" -}}
+  {{- (semverCompare ">= 1.23-0" (include "kubeVersion" .)) -}}
+{{- end -}}

charts/nginx-ingress-services/templates/federation-test-helper.yaml

@@ -1,3 +1,4 @@
+{{- $newLabels := eq (include "integrationTestHelperNewLabels" .) "true" -}}
 # Assumes that the controller is deployed in the same namespace. Only used for
 # enabling discovery by creating SRV records while running integration tests.
 {{- if (and .Values.federator.enabled .Values.federator.integrationTestHelper) }}
@@ -13,7 +14,12 @@ spec:
     protocol: TCP
     targetPort: https
   selector:
+    {{- if $newLabels }}
+    app.kubernetes.io/component: controller
+    app.kubernetes.io/name: ingress-nginx
+    {{- else }}
     app: nginx-ingress
     component: controller
+    {{- end }}
   type: ClusterIP
 {{- end }}

charts/nginx-ingress-services/templates/ingress.yaml

@@ -5,7 +5,7 @@ kind: Ingress
 metadata:
   name: nginx-ingress
   annotations:
-    kubernetes.io/ingress.class: "nginx"
+    kubernetes.io/ingress.class: "{{ .Values.config.ingressClass }}"
 spec:
   # This assumes you have created the given cert (see secret.yaml)
   # https://github.com/kubernetes/ingress-nginx/blob/master/docs/examples/PREREQUISITES.md#tls-certificates

charts/nginx-ingress-services/templates/ingress_federator.yaml

@@ -8,7 +8,7 @@ kind: Ingress
 metadata:
   name: federator-ingress
   annotations:
-    kubernetes.io/ingress.class: "nginx"
+    kubernetes.io/ingress.class: "{{ .Values.config.ingressClass }}"
     nginx.ingress.kubernetes.io/ssl-redirect: "true"
     nginx.ingress.kubernetes.io/backend-protocol: "HTTP"
     nginx.ingress.kubernetes.io/auth-tls-verify-client: "on"

charts/nginx-ingress-services/values.yaml

@@ -91,8 +91,9 @@ service:
   accountPages:
     externalPort: 8080
 
+config:
+  ingressClass: "nginx"
 # You will need to supply some DNS names, namely
-# config:
 #   dns:
 #     https: nginz-https.<domain>
 #     ssl: nginz-ssl.<domain> # For websockets

hack/bin/integration-setup-federation.sh

@@ -20,13 +20,22 @@ ${DIR}/integration-cleanup.sh
 # script beforehand on all relevant charts to download the nested dependencies
 # (e.g. cassandra from underneath databases-ephemeral)
 echo "updating recursive dependencies ..."
-charts=(fake-aws databases-ephemeral redis-cluster wire-server nginx-ingress-controller nginx-ingress-services)
+charts=(fake-aws databases-ephemeral redis-cluster wire-server ingress-nginx-controller nginx-ingress-controller nginx-ingress-services)
 mkdir -p ~/.parallel && touch ~/.parallel/will-cite
 printf '%s\n' "${charts[@]}" | parallel -P "${HELM_PARALLELISM}" "$DIR/update.sh" "$CHARTS_DIR/{}"
 
 # FUTUREWORK: use helm functions instead, see https://wearezeta.atlassian.net/browse/SQPIT-723
 echo "Generating self-signed certificates..."
 
+KUBERNETES_VERSION_MAJOR="$(kubectl version -o json | jq -r .serverVersion.major)"
+KUBERNETES_VERSION_MINOR="$(kubectl version -o json | jq -r .serverVersion.minor)"
+export KUBERNETES_VERSION="$KUBERNETES_VERSION_MAJOR.$KUBERNETES_VERSION_MINOR"
+if (( KUBERNETES_VERSION_MAJOR > 1 || KUBERNETES_VERSION_MAJOR == 1 && KUBERNETES_VERSION_MINOR >= 23 )); then
+    export INGRESS_CHART="ingress-nginx-controller"
+else
+    export INGRESS_CHART="nginx-ingress-controller"
+fi
+echo "kubeVersion: $KUBERNETES_VERSION and ingress controller=$INGRESS_CHART"
 export NAMESPACE_1="$NAMESPACE"
 export FEDERATION_DOMAIN_BASE="$NAMESPACE_1.svc.cluster.local"
 export FEDERATION_DOMAIN_1="federation-test-helper.$FEDERATION_DOMAIN_BASE"

hack/bin/integration-setup.sh

@@ -14,10 +14,19 @@ HELM_PARALLELISM=${HELM_PARALLELISM:-1}
 "${DIR}/integration-cleanup.sh"
 
 echo "updating recursive dependencies ..."
-charts=(fake-aws databases-ephemeral redis-cluster wire-server nginx-ingress-controller nginx-ingress-services)
+charts=(fake-aws databases-ephemeral redis-cluster wire-server ingress-nginx-controller nginx-ingress-controller nginx-ingress-services)
 mkdir -p ~/.parallel && touch ~/.parallel/will-cite
 printf '%s\n' "${charts[@]}" | parallel -P "${HELM_PARALLELISM}" "$DIR/update.sh" "$CHARTS_DIR/{}"
 
+KUBERNETES_VERSION_MAJOR="$(kubectl version -o json | jq -r .serverVersion.major)"
+KUBERNETES_VERSION_MINOR="$(kubectl version -o json | jq -r .serverVersion.minor)"
+export KUBERNETES_VERSION="$KUBERNETES_VERSION_MAJOR.$KUBERNETES_VERSION_MINOR"
+if (( KUBERNETES_VERSION_MAJOR > 1 || KUBERNETES_VERSION_MAJOR == 1 && KUBERNETES_VERSION_MINOR >= 23 )); then
+    export INGRESS_CHART="ingress-nginx-controller"
+else
+    export INGRESS_CHART="nginx-ingress-controller"
+fi
+echo "kubeVersion: $KUBERNETES_VERSION and ingress controller=$INGRESS_CHART"
 echo "Generating self-signed certificates..."
 export FEDERATION_DOMAIN_BASE="$NAMESPACE.svc.cluster.local"
 export FEDERATION_DOMAIN="federation-test-helper.$FEDERATION_DOMAIN_BASE"

hack/bin/integration-teardown-federation.sh

@@ -12,6 +12,13 @@ export NAMESPACE_2="$NAMESPACE-fed2"
 export FEDERATION_DOMAIN_1="."
 export FEDERATION_DOMAIN_2="."
 
+KUBERNETES_VERSION_MINOR="$(kubectl version -o json | jq -r .serverVersion.minor)"
+if (( KUBERNETES_VERSION_MAJOR > 1 || KUBERNETES_VERSION_MAJOR == 1 && KUBERNETES_VERSION_MINOR >= 23 )); then
+    export INGRESS_CHART="ingress-nginx-controller"
+else
+    export INGRESS_CHART="nginx-ingress-controller"
+fi
+
 . "$DIR/helm_overrides.sh"
 helmfile --file "${TOP_LEVEL}/hack/helmfile.yaml" destroy
 

hack/bin/integration-teardown.sh

@@ -6,6 +6,12 @@ TOP_LEVEL="$DIR/../.."
 NAMESPACE=${NAMESPACE:-test-integration}
 # doesn't matter for destruction but needs to be set
 export FEDERATION_DOMAIN="."
+KUBERNETES_VERSION_MINOR="$(kubectl version -o json | jq -r .serverVersion.minor)"
+if (( KUBERNETES_VERSION_MAJOR > 1 || KUBERNETES_VERSION_MAJOR == 1 && KUBERNETES_VERSION_MINOR >= 23 )); then
+    export INGRESS_CHART="ingress-nginx-controller"
+else
+    export INGRESS_CHART="nginx-ingress-controller"
+fi
 
 set -ex
 

hack/bin/set-helm-chart-version.sh

@@ -12,7 +12,7 @@ tempfile=$(mktemp)
 
 function update_chart(){
     chart_file=$1
-    sed -e "s/version: .*/version: $target_version/g" "$chart_file" > "$tempfile" && mv "$tempfile" "$chart_file"
+    sed -e "s/^version: .*/version: $target_version/g" "$chart_file" > "$tempfile" && mv "$tempfile" "$chart_file"
 }
 
 function write_versions() {

hack/bin/update.sh

@@ -29,6 +29,12 @@ helmDepUp () {
       helm dep up
       echo "... updating in $path done."
     fi
+
+    if grep "dependencies:" Chart.yaml; then
+      echo "Updating dependencies (from Chart.yaml) in $path ..."
+      helm dep up
+      echo "... updating in $path done."
+    fi
 }
 
 helmDepUp "$dir"

hack/helm_vars/ingress-nginx-controller/values.yaml.gotmpl

@@ -0,0 +1,19 @@
+ingress-nginx:
+  controller:
+    ingressClassResource:
+        name: "nginx-{{ .Release.Namespace }}"
+        # -- Is this ingressClass enabled or not
+        enabled: true
+    ingressClass: "nginx-{{ .Release.Namespace }}"
+    kind: Deployment
+    replicaCount: 1
+    service:
+      nodePorts:
+        # choose a random free port
+        https: null
+        http: null
+      # in CI, do not use ValidatingWebhooks, as these, if not properly cleaned up
+      # (i.e. the ingress controller was deleted in another namespace but the webhook remains)
+      # prevent new kind:Ingress resources to be created in the cluster.
+      admissionWebhooks:
+        enabled: false

hack/helm_vars/nginx-ingress-controller/values.yaml → hack/helm_vars/nginx-ingress-controller/values.yaml.gotmpl

@@ -1,6 +1,8 @@
 nginx-ingress:
   controller:
-    kind: DaemonSet
+    kind: Deployment
+    replicaCount: 1
+    ingressClass: "nginx-{{ .Release.Namespace }}"
     service:
       type: NodePort
       externalTrafficPolicy: Local

hack/helm_vars/nginx-ingress-services/values.yaml → hack/helm_vars/nginx-ingress-services/values.yaml.gotmpl

@@ -9,6 +9,7 @@ tls:
   useCertManager: false
 
 config:
+  ingressClass: "nginx-{{ .Release.Namespace }}"
   dns:
     https: nginz-https.integration.example.com
     ssl: nginz-ssl.integration.example.com