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,37 @@
+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'
+```
+
+For more information read the documentation under https://docs.wire.com/how-to/install/ingress.html (or go to https://docs.wire.com and search for "ingress-nginx-controller")

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,58 @@
+# The following defaults apply to a cloud-like setup (in which you can ask your
+# kubernetes installation to give you a LoadBalancer setup).
+#
+# If you are on bare metal and wish an installation similiar in spirit as the
+# older similarly named wrapper chart 'nginx-ingress-controller' (note the
+# swapped words 'nginx' and 'ingress'), where we assume no load balancer support
+# and instead expose NodePorts on ports 31773 and 31772, and where you need to
+# ensure traffic gets to these ports in another way; then please read the
+# documentation on https://docs.wire.com/how-to/install/ingress.html (or go to
+# https://docs.wire.com and search for "ingress-nginx-controller")
+#
+# 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 kind: `DaemonSet` (when using NodePort) or `Deployment` (when using
+    # LoadBalancer)
+    kind: Deployment
+    service:
+      type: LoadBalancer  # or NodePort (then also use DaemonSet)
+      # set externalTrafficPolicy=Local to keep the source IP available in
+      # upstream services. Comes with tradeoff considerations, see
+      # documentation on "ingress" on docs.wire.com
+      externalTrafficPolicy: Local
+      nodePorts:
+        # If you set service.type = NodePort, then the nginx controller 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

docs/src/how-to/install/index.md

@@ -19,6 +19,7 @@ Infrastructure configuration <infrastructure-configuration>
 How to monitor wire-server <monitoring>
 How to see centralized logs for wire-server <logging>
 
+Ingress-controller (getting traffic in) <ingress>
 Web app settings <web-app-settings>
 sft
 restund

docs/src/how-to/install/ingress.md

@@ -0,0 +1,102 @@
+# Ingress traffic to wire-server (ingress-nginx-controller)
+
+*at the time of writing (2023-03), this section assumes you use a kubernetes
+version 1.23 or above (tested with 1.26)*
+
+## Installing in a cloud-like environment
+
+Install the ingress controller chart in your helmfile with the defaults, simply
+like this:
+
+```yaml
+# helmfile.yaml
+repositories:
+  - name: wire
+    url: 'https://s3-eu-west-1.amazonaws.com/public.wire.com/charts'
+
+releases:
+  - name: 'ingress-nginx-controller'
+    namespace: 'wire'
+    chart: 'wire/ingress-nginx-controller'
+    version: 'CHANGE_ME'
+
+# charts wire-server and nginx-ingress-services also need to be installed, see other
+# documentation
+#  - name: ...
+#    chart: ...
+```
+
+By default, the `wire/ingress-nginx-controller` chart will create a `Deployment`
+with services of type `LoadBalancer`, where your kubernetes installation needs
+to support dynamic LoadBalancers. If this is not possible, read the next section.
+
+By default three pods will come up and external traffic will be load balanced into these
+three pods, which will also do TLS termination and forward traffic to upstream
+services (`nginz` and others).
+
+To inspect default TLS settings, see [defaults in the latest code](https://github.com/wireapp/wire-server/blob/develop/charts/ingress-nginx-controller/values.yaml) and also see {ref}`tls`.
+
+## Installing on bare-metal without dynamic load balancer support
+
+In case you cannot create a `kind: service` of `type: LoadBalancer`, then you
+can fall back to manually ensure traffic reaches your installation:
+
+```yaml
+# helmfile.yaml
+releases:
+  - name: 'ingress-nginx-controller'
+    namespace: 'wire'
+    chart: 'wire/ingress-nginx-controller'
+    version: 'CHANGE_ME'
+    values:
+      - './helm_vars/ingress-nginx-controller/values.yaml'
+```
+
+Create this file with the following override values:
+
+```yaml
+# helm_vars/ingress-nginx-controller/values.yaml
+ingress-nginx:
+  controller:
+    kind: DaemonSet
+    service:
+      type: NodePort
+```
+
+Then, on each of your kubernetes worker nodes, two ports are exposed: ports
+31773 (https) and 31772 (http)
+
+You should add a port-forwarding rule on the node or on the loadbalancer that
+forwards ports 443 and 80 to these respective ports. Any traffic hitting the http port is simply getting a http 30x redirect to https.
+
+Downsides of this approach: The NodePort approach always requires manual configuration of some external load balancer/firewall to round-robin between node IPs and is error-prone. It's also a bit annoying to have to decide on some global ports that may not be used otherwise.
+
+Most managed K8s clusters have support for LoadBalancers, you can also get this for your own clusters in hcloud etc. It's even possible to do it for pure bare metal, without any "load balancer hardware", by using BGP or some leadership election over who's announcing the "load balancer ip" via ARP (https://metallb.universe.tf/configuration/_advanced_l2_configuration/).
+
+### Using NodePort (not the default) with externalTrafficPolicy=Local (the default)
+
+Normally, NodePort will listen to traffic on all nodes, and uses kube-proxy
+to redirect to the node that actually runs ingress-nginx-controller. However
+one problem with this is that this traffic is NAT'ed. This means that nginx
+will not have access to the source IP address from which the request
+originated.  We want to have this source IP address for potentially logging
+and rate-limiting based on it. By setting externalTrafficPolicy: local,
+nodes will no longer forward requests to other nodes if they receive a
+request that they themselves can not handle. Upside is that the traffic is
+now not NAT'ed anymore, and we get access to the source IP address. Downside
+is that you need to know beforehand which nodes run a certain pod. However,
+with kubernetes a pod can be rescheduled to any node at any time so we can
+not trust this.  We could do something with node affinities to decide apriori
+on what set of nodes will be publicly reachable and make sure the nginx
+controller pods are only ran on there but for now that sounds a bit overkill.
+Instead, we just simply run the ingress controller on each node using a
+daemonset. This means that any node in the cluster can receive requests and
+redirect them to the correct service, whilst maintaining the source ip
+address. The ingress controller is sort of taking over the role of what
+kube-proxy was doing before.
+More information:
+- https://kubernetes.io/docs/tutorials/services/source-ip/#source-ip-for-services-with-typenodeport
+- https://kubernetes.github.io/ingress-nginx/deploy/baremetal/
+
+There are also downsides to setting `externalTrafficPolicy: Local`, please look at the [following blog post](https://www.asykim.com/blog/deep-dive-into-kubernetes-external-traffic-policies), which very clearly explains the upsides and
+downsides of this setting

docs/src/how-to/install/tls.md

@@ -25,7 +25,7 @@ Therefore it is not necessary to add them to openssl based configurations.
 
 ## Ingress Traffic (wire-server)
 
-The list of TLS ciphers for incoming requests is limited by default to the [following](https://github.com/wireapp/wire-server/blob/master/charts/nginx-ingress-controller/values.yaml#L7) (for general server-certificates, both for federation and client API), and can be overridden on your installation if needed.
+The list of TLS ciphers for incoming requests is limited by default to the [following](https://github.com/wireapp/wire-server/blob/master/charts/ingress-nginx-controller/values.yaml#L41-45) (for general server-certificates, both for federation and client API), and can be overridden on your installation if needed.
 
 ## Egress Traffic (wire-server/federation)