From e414bc59aa13982d4ce8a8077acc9f2690d2f208 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Fri, 9 Oct 2026 14:11:41 +0100 Subject: [PATCH 1/5] Run oauth2-proxy in proxy mode This circumvents cookie-setting problem --- sync/chart/templates/_helpers.tpl | 61 ++++++++------------------ sync/chart/templates/ingress-oidc.yaml | 42 ------------------ sync/chart/templates/ingress.yaml | 7 +++ sync/chart/values.yaml | 50 +++++++++++++++++---- sync/zenith/sync/processor/helm.py | 39 +++++++++++----- 5 files changed, 97 insertions(+), 102 deletions(-) delete mode 100644 sync/chart/templates/ingress-oidc.yaml diff --git a/sync/chart/templates/_helpers.tpl b/sync/chart/templates/_helpers.tpl index 5420af2aa..0a54a1b78 100644 --- a/sync/chart/templates/_helpers.tpl +++ b/sync/chart/templates/_helpers.tpl @@ -30,7 +30,11 @@ Annotations for the ingress resource. */}} {{- define "zenith-service.ingress.annotations" -}} {{ toYaml .Values.ingress.annotations }} -nginx.ingress.kubernetes.io/backend-protocol: {{ quote .Values.protocol }} +{{- /* + With OIDC, the ingress forwards to the OAuth2 proxy, which listens on HTTP and + forwards authenticated requests to the service using the service protocol +*/}} +nginx.ingress.kubernetes.io/backend-protocol: {{ ternary "http" .Values.protocol .Values.oidc.enabled | quote }} {{- with .Values.readTimeout }} nginx.ingress.kubernetes.io/proxy-read-timeout: {{ int64 . | quote }} {{- end }} @@ -40,9 +44,6 @@ nginx.ingress.kubernetes.io/proxy-read-timeout: {{ int64 . | quote }} {{- if .Values.externalAuth.enabled }} {{- include "zenith-service.ingress.auth.external.annotations" . }} {{- end }} -{{- if .Values.oidc.enabled }} -{{- include "zenith-service.ingress.auth.oidc.annotations" . }} -{{- end }} {{- end }} {{/* @@ -72,8 +73,7 @@ Annotations for external auth. {{- with .Values.externalAuth }} nginx.ingress.kubernetes.io/auth-url: {{ .url | required "external auth URL is required" | quote }} {{- if .signinUrl }} -nginx.ingress.kubernetes.io/auth-signin: {{ quote .signinUrl }} -nginx.ingress.kubernetes.io/auth-signin-redirect-param: {{ .nextUrlParam }} +nginx.ingress.kubernetes.io/auth-signin: {{ include "zenith-service.ingress.auth.external.signinUrl" . | quote }} {{- end }} {{- if or .requestHeaders .params }} nginx.ingress.kubernetes.io/auth-snippet: | @@ -91,43 +91,20 @@ nginx.ingress.kubernetes.io/auth-response-headers: {{ join "," .responseHeaders {{- end }} {{/* -Annotations for OIDC auth. +The signin URL for external auth, including the parameter for the original URL. */}} -{{- define "zenith-service.ingress.auth.oidc.annotations" -}} -{{- $scheme := ternary "https" "http" .Values.global.secure }} -{{- $host := include "zenith-service.ingress.host" . }} -{{- $oidcReleaseName := printf "%s-oidc" .Release.Name }} -{{- $prefix := tpl (index .Values.oidc.extraArgs "proxy-prefix") . }} -nginx.ingress.kubernetes.io/auth-url: >- - http://{{ $oidcReleaseName }}.{{ .Release.Namespace }}.svc.cluster.local{{ $prefix }}/auth -nginx.ingress.kubernetes.io/auth-signin: >- - {{ $scheme }}://{{ $host }}{{ $prefix }}/start?rd=$escaped_request_uri&$args -{{- with .Values.oidc.alphaConfig.configData.injectResponseHeaders }} -nginx.ingress.kubernetes.io/auth-response-headers: >- - {{ range $i, $rh := . }}{{ if $i }},{{ end }}{{ $rh.name }}{{ end }} -{{- end }} -nginx.ingress.kubernetes.io/configuration-snippet: | - auth_request_set $auth_cookie__oauth2_proxy_1 $upstream_cookie__oauth2_proxy_1; - auth_request_set $auth_cookie__oauth2_proxy_2 $upstream_cookie__oauth2_proxy_2; - auth_request_set $auth_cookie__oauth2_proxy_3 $upstream_cookie__oauth2_proxy_3; - - access_by_lua_block { - local auth_set_cookie = ngx.var.auth_cookie - - if ngx.var.auth_cookie__oauth2_proxy_1 ~= "" then - auth_set_cookie = "_oauth2_proxy_1=" .. ngx.var.auth_cookie__oauth2_proxy_1 .. "; " .. auth_set_cookie - end - if ngx.var.auth_cookie__oauth2_proxy_2 ~= "" then - auth_set_cookie = "_oauth2_proxy_2=" .. ngx.var.auth_cookie__oauth2_proxy_2 .. "; " .. auth_set_cookie - end - if ngx.var.auth_cookie__oauth2_proxy_3 ~= "" then - auth_set_cookie = "_oauth2_proxy_3=" .. ngx.var.auth_cookie__oauth2_proxy_3 .. "; " .. auth_set_cookie - end - - if auth_set_cookie ~= "" then - ngx.header["Set-Cookie"] = auth_set_cookie - end - } +{{- define "zenith-service.ingress.auth.external.signinUrl" -}} +{{- $query := get (urlParse .signinUrl) "query" }} +{{- if regexMatch (printf "(^|&)%s=" (regexQuoteMeta .nextUrlParam)) $query }} +{{- .signinUrl }} +{{- else }} +{{- + printf "%s%s%s=$scheme://$best_http_host$escaped_request_uri" + .signinUrl + (ternary "&" "?" (contains "?" .signinUrl)) + .nextUrlParam +}} +{{- end }} {{- end }} {{/* diff --git a/sync/chart/templates/ingress-oidc.yaml b/sync/chart/templates/ingress-oidc.yaml deleted file mode 100644 index 14c179296..000000000 --- a/sync/chart/templates/ingress-oidc.yaml +++ /dev/null @@ -1,42 +0,0 @@ -{{- - if and - (mustRegexMatch "^https?$" .Values.protocol) - .Values.ingress.enabled - .Values.oidc.enabled - (not .Values.oidc.ingress.enabled) -}} -{{- $ingressTLSRequired := and .Values.global.secure (not .Values.ingress.tls.terminatedAtProxy) }} -apiVersion: networking.k8s.io/v1 -kind: Ingress -metadata: - name: {{ .Release.Name }}-oidc - labels: {{ include "zenith-service.labels" . | nindent 4 }} - annotations: - {{- toYaml .Values.ingress.annotations | nindent 4 }} - {{- if $ingressTLSRequired }} - {{- if not .Values.ingress.tls.existingCertificate.cert }} - {{- with .Values.ingress.tls.annotations }} - {{- toYaml . | nindent 4 }} - {{- end }} - {{- end }} - {{- end }} -spec: - ingressClassName: {{ .Values.ingress.className }} - {{- if $ingressTLSRequired }} - tls: - - hosts: - - {{ include "zenith-service.ingress.host" . }} - secretName: {{ include "zenith-service.ingress.tls.secretName" . }} - {{- end }} - rules: - - host: {{ include "zenith-service.ingress.host" . }} - http: - paths: - - path: {{ tpl (index .Values.oidc.extraArgs "proxy-prefix") . }} - pathType: Prefix - backend: - service: - name: {{ printf "%s-oidc" .Release.Name }} - port: - name: http -{{- end }} diff --git a/sync/chart/templates/ingress.yaml b/sync/chart/templates/ingress.yaml index 4ef90f2e2..bc82af615 100644 --- a/sync/chart/templates/ingress.yaml +++ b/sync/chart/templates/ingress.yaml @@ -26,7 +26,14 @@ spec: pathType: Prefix backend: service: + {{- if .Values.oidc.enabled }} + # With OIDC, the OAuth2 proxy receives all traffic for the service + name: {{ .Release.Name }}-oidc + port: + name: http + {{- else }} name: {{ .Release.Name }} port: name: dynamic + {{- end }} {{- end }} diff --git a/sync/chart/values.yaml b/sync/chart/values.yaml index 7c351ee52..2947723d6 100644 --- a/sync/chart/values.yaml +++ b/sync/chart/values.yaml @@ -102,24 +102,58 @@ oidc: insecureAllowUnverifiedEmail: true groupsClaim: groups audienceClaims: ["aud"] + # The OAuth2 proxy is in the data path for the service, so run two replicas + # Sessions are stored in cookies, so the replicas don't need to share any state + replicaCount: 2 + # Prefer to run the replicas on different nodes + topologySpreadConstraints: + - maxSkew: 1 + topologyKey: kubernetes.io/hostname + whenUnsatisfiable: ScheduleAnyway + labelSelector: + matchLabels: + app.kubernetes.io/name: oidc + matchLabelKeys: + - app.kubernetes.io/instance + # The OAuth2 proxy receives all traffic for the service and forwards authenticated + # requests to the Zenith service + # These must match the protocol and readTimeout for the service, as the subchart + # cannot see them + upstream: + # The protocol for the service + protocol: http + # The time to wait for the response headers from the service, in seconds + # If not given, a long timeout is used so that the ingress controller's read timeout + # is the effective limit + readTimeout: # We use the alpha config in order to be able to use loginURLParameters alphaConfig: enabled: true # Static parts of the configuration configData: - # This is the claims to return as headers from the response - injectResponseHeaders: [] - # Just configure a "null" upstream as we are using it as an auth request - upstreamConfig: - upstreams: - - id: static - path: "/" - static: true + # These are the claims to inject as headers into the request to the service + # Any headers with the same names in the incoming request are removed + injectRequestHeaders: [] # We template the provider so that we can move the provider configuration # to a higher level and get the nice merging behaviour + # The upstream is templated so that it points at the service for the release configFile: | providers: - {{ toYaml .Values.provider | nindent 6 }} + upstreamConfig: + upstreams: + {{- $protocol := .Values.upstream.protocol }} + {{- $port := ternary 443 80 (eq $protocol "https") }} + - id: zenith-service + path: / + uri: {{ printf "%s://%s.%s.svc:%d" $protocol .Release.Name .Release.Namespace $port }} + passHostHeader: true + proxyWebSockets: true + timeout: {{ printf "%ds" (int64 (.Values.upstream.readTimeout | default 3600)) }} + {{- if eq .Values.upstream.protocol "https" }} + # Like the ingress controller, don't verify the certificate of the service + insecureSkipTLSVerify: true + {{- end }} # Disable the standard config config: configFile: "" diff --git a/sync/zenith/sync/processor/helm.py b/sync/zenith/sync/processor/helm.py index a81a86cab..e9de4267d 100644 --- a/sync/zenith/sync/processor/helm.py +++ b/sync/zenith/sync/processor/helm.py @@ -182,22 +182,36 @@ def _get_service_values(self, service: model.Service) -> dict[str, typing.Any]: "subdomainAsPathPrefix": self.config.ingress.subdomain_as_path_prefix, }, "endpoints": [dataclasses.asdict(ep) for ep in service.endpoints], - "protocol": service.config.get("backend-protocol", "http"), + "protocol": self._get_protocol(service), "ingress": { "annotations": self.config.ingress.annotations, }, } - read_timeout = service.config.get("read-timeout") + read_timeout = self._get_read_timeout(service) if read_timeout: - # Check that the read timeout is an int - if it isn't don't use it - try: - read_timeout = int(read_timeout) - except ValueError: - self.logger.warn("Given read timeout is not a valid integer") - else: - values["readTimeout"] = read_timeout + values["readTimeout"] = read_timeout return values + def _get_protocol(self, service: model.Service) -> str: + """ + Returns the protocol for the service. + """ + return service.config.get("backend-protocol", "http") + + def _get_read_timeout(self, service: model.Service) -> int | None: + """ + Returns the read timeout for the service, if one is given. + """ + read_timeout = service.config.get("read-timeout") + if not read_timeout: + return None + # Check that the read timeout is an int - if it isn't don't use it + try: + return int(read_timeout) + except ValueError: + self.logger.warning("Given read timeout is not a valid integer") + return None + def _get_ingress_enabled(self, service: model.Service) -> dict[str, typing.Any]: """ Returns the values for enabling or disabling ingress as required. @@ -276,9 +290,14 @@ async def _get_auth_values(self, service: model.Service) -> dict[str, typing.Any "issuerURL": issuer_url, }, }, + # The OAuth2 proxy forwards authenticated requests to the service + "upstream": { + "protocol": self._get_protocol(service), + "readTimeout": self._get_read_timeout(service), + }, "alphaConfig": { "configData": { - "injectResponseHeaders": [ + "injectRequestHeaders": [ {"name": h, "values": [{"claim": c}]} for h, c in ( self.config.ingress.oidc.inject_request_headers.items() From f1ea68322ca85740954d0782ca934225db71d03f Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Fri, 9 Oct 2026 14:13:02 +0100 Subject: [PATCH 2/5] Add tests for oauth2-proxy Assisted-by: Claude Sonnet 5 --- .../tests/__snapshot__/auth_test.yaml.snap | 75 +++++++ .../__snapshot__/oidc_proxy_test.yaml.snap | 33 +++ sync/chart/tests/auth_test.yaml | 100 +++++++++ sync/chart/tests/oidc_proxy_test.yaml | 209 ++++++++++++++++++ sync/tests/test_helm_processor.py | 83 +++++++ 5 files changed, 500 insertions(+) create mode 100644 sync/chart/tests/__snapshot__/auth_test.yaml.snap create mode 100644 sync/chart/tests/__snapshot__/oidc_proxy_test.yaml.snap create mode 100644 sync/chart/tests/auth_test.yaml create mode 100644 sync/chart/tests/oidc_proxy_test.yaml create mode 100644 sync/tests/test_helm_processor.py diff --git a/sync/chart/tests/__snapshot__/auth_test.yaml.snap b/sync/chart/tests/__snapshot__/auth_test.yaml.snap new file mode 100644 index 000000000..d64fa6ad6 --- /dev/null +++ b/sync/chart/tests/__snapshot__/auth_test.yaml.snap @@ -0,0 +1,75 @@ +client CA ingress should match snapshot: + 1: | + apiVersion: networking.k8s.io/v1 + kind: Ingress + metadata: + annotations: + nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream: "true" + nginx.ingress.kubernetes.io/auth-tls-secret: NAMESPACE/tls-client-ca-RELEASE-NAME + nginx.ingress.kubernetes.io/auth-tls-verify-client: optional + nginx.ingress.kubernetes.io/backend-protocol: http + nginx.ingress.kubernetes.io/proxy-body-size: "0" + nginx.ingress.kubernetes.io/proxy-buffer-size: 16k + nginx.ingress.kubernetes.io/proxy-buffering: "off" + labels: + app.kubernetes.io/managed-by: Helm + app.kubernetes.io/version: main + helm.sh/chart: zenith-service-0.1.0 + zenith.stackhpc.com/service-name: RELEASE-NAME + name: RELEASE-NAME + spec: + ingressClassName: nginx + rules: + - host: bar.foo.example.com + http: + paths: + - backend: + service: + name: RELEASE-NAME + port: + name: dynamic + path: / + pathType: Prefix + tls: + - hosts: + - bar.foo.example.com + secretName: tls-RELEASE-NAME +external auth ingress with signin URL should match snapshot: + 1: | + apiVersion: networking.k8s.io/v1 + kind: Ingress + metadata: + annotations: + nginx.ingress.kubernetes.io/auth-response-headers: X-Remote-User,X-Remote-Group + nginx.ingress.kubernetes.io/auth-signin: https://auth.example.com/login?next=$scheme://$best_http_host$escaped_request_uri + nginx.ingress.kubernetes.io/auth-snippet: | + proxy_set_header accept application/json; + proxy_set_header tenancy-id abcdef; + nginx.ingress.kubernetes.io/auth-url: http://auth.auth-system.svc.cluster.local/verify/ + nginx.ingress.kubernetes.io/backend-protocol: http + nginx.ingress.kubernetes.io/proxy-body-size: "0" + nginx.ingress.kubernetes.io/proxy-buffer-size: 16k + nginx.ingress.kubernetes.io/proxy-buffering: "off" + labels: + app.kubernetes.io/managed-by: Helm + app.kubernetes.io/version: main + helm.sh/chart: zenith-service-0.1.0 + zenith.stackhpc.com/service-name: RELEASE-NAME + name: RELEASE-NAME + spec: + ingressClassName: nginx + rules: + - host: bar.foo.example.com + http: + paths: + - backend: + service: + name: RELEASE-NAME + port: + name: dynamic + path: / + pathType: Prefix + tls: + - hosts: + - bar.foo.example.com + secretName: tls-RELEASE-NAME diff --git a/sync/chart/tests/__snapshot__/oidc_proxy_test.yaml.snap b/sync/chart/tests/__snapshot__/oidc_proxy_test.yaml.snap new file mode 100644 index 000000000..93de37c50 --- /dev/null +++ b/sync/chart/tests/__snapshot__/oidc_proxy_test.yaml.snap @@ -0,0 +1,33 @@ +ingress should match snapshot: + 1: | + apiVersion: networking.k8s.io/v1 + kind: Ingress + metadata: + annotations: + nginx.ingress.kubernetes.io/backend-protocol: http + nginx.ingress.kubernetes.io/proxy-body-size: "0" + nginx.ingress.kubernetes.io/proxy-buffer-size: 16k + nginx.ingress.kubernetes.io/proxy-buffering: "off" + labels: + app.kubernetes.io/managed-by: Helm + app.kubernetes.io/version: main + helm.sh/chart: zenith-service-0.1.0 + zenith.stackhpc.com/service-name: svc1 + name: svc1 + spec: + ingressClassName: nginx + rules: + - host: bar.foo.example.com + http: + paths: + - backend: + service: + name: svc1-oidc + port: + name: http + path: / + pathType: Prefix + tls: + - hosts: + - bar.foo.example.com + secretName: tls-svc1 diff --git a/sync/chart/tests/auth_test.yaml b/sync/chart/tests/auth_test.yaml new file mode 100644 index 000000000..9539ae3b7 --- /dev/null +++ b/sync/chart/tests/auth_test.yaml @@ -0,0 +1,100 @@ +# Tests for the external auth and TLS annotations on the ingress +# OIDC is tested in oidc_proxy_test.yaml +# These annotations must be supported by both ingress-nginx and Traefik's NGINX +# compatibility (kubernetesIngressNGINX provider) +# To update manifest snapshots run helm unittest plugin with -u option: +# docker run -i --rm -v $(pwd):/apps helmunittest/helm-unittest -u /path/to/chart +suite: Ingress auth annotations +values: + - values_test_overrides.yaml +templates: + - templates/ingress.yaml +tests: + - it: external auth ingress with signin URL should match snapshot + set: + externalAuth: &externalAuth + enabled: true + url: http://auth.auth-system.svc.cluster.local/verify/ + signinUrl: https://auth.example.com/login + requestHeaders: + accept: application/json + responseHeaders: + - X-Remote-User + - X-Remote-Group + params: + tenancy-id: abcdef + asserts: + - matchSnapshot: {} + + - it: external auth signin URL should embed the next URL parameter + set: + externalAuth: + <<: *externalAuth + nextUrlParam: next_url + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + value: https://auth.example.com/login?next_url=$scheme://$best_http_host$escaped_request_uri + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin-redirect-param"] + + - it: external auth signin URL should append to an existing query string + set: + externalAuth: + <<: *externalAuth + signinUrl: https://auth.example.com/login?kc_idp_hint=azimuth + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + value: https://auth.example.com/login?kc_idp_hint=azimuth&next=$scheme://$best_http_host$escaped_request_uri + + - it: external auth signin URL should not override an existing next URL parameter + set: + externalAuth: + <<: *externalAuth + signinUrl: https://auth.example.com/login?next=https://other.example.com + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + value: https://auth.example.com/login?next=https://other.example.com + + - it: external auth signin URL should not treat a similar parameter as the next URL parameter + set: + externalAuth: + <<: *externalAuth + signinUrl: https://auth.example.com/login?nextpage=1 + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + value: https://auth.example.com/login?nextpage=1&next=$scheme://$best_http_host$escaped_request_uri + + - it: external auth without signin URL should not set signin annotations + set: + externalAuth: + <<: *externalAuth + signinUrl: + asserts: + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin-redirect-param"] + + - it: client CA ingress should match snapshot + set: + ingress: + tls: + clientCA: | + -----BEGIN CERTIFICATE----- + notarealcertificate + -----END CERTIFICATE----- + asserts: + - matchSnapshot: {} + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-tls-secret"] + value: NAMESPACE/tls-client-ca-RELEASE-NAME + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-tls-verify-client"] + value: optional + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream"] + value: "true" diff --git a/sync/chart/tests/oidc_proxy_test.yaml b/sync/chart/tests/oidc_proxy_test.yaml new file mode 100644 index 000000000..5855738ea --- /dev/null +++ b/sync/chart/tests/oidc_proxy_test.yaml @@ -0,0 +1,209 @@ +# Tests for OIDC auth, where the OAuth2 proxy receives all traffic for the service and +# forwards authenticated requests to the service +# To update manifest snapshots run helm unittest plugin with -u option: +# docker run -i --rm -v $(pwd):/apps helmunittest/helm-unittest -u /path/to/chart +suite: OIDC proxy +release: + name: svc1 + namespace: zenith-services +values: + - values_test_overrides.yaml +templates: + - templates/ingress.yaml + # The deployment includes other templates to compute checksums + - charts/oidc/templates/*.yaml +set: + oidc: + enabled: true + provider: + clientID: client-id + clientSecret: client-secret + oidcConfig: + issuerURL: https://identity.example.com/realms/test + alphaConfig: + configData: + injectRequestHeaders: + - name: X-Remote-User + values: + - claim: preferred_username + - name: X-Remote-Group + values: + - claim: groups + extraArgs: + cookie-secret: cookie-secret-value +tests: + - it: ingress should match snapshot + template: templates/ingress.yaml + asserts: + - matchSnapshot: {} + + - it: ingress should send all traffic to the OAuth2 proxy + template: templates/ingress.yaml + asserts: + - equal: + path: spec.rules[0].http.paths + value: + - path: / + pathType: Prefix + backend: + service: + name: svc1-oidc + port: + name: http + + - it: ingress should not use auth annotations or snippets + template: templates/ingress.yaml + asserts: + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-url"] + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-signin"] + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/auth-response-headers"] + - notExists: + path: metadata.annotations["nginx.ingress.kubernetes.io/configuration-snippet"] + + - it: ingress should use HTTP to the OAuth2 proxy even if the service uses HTTPS + template: templates/ingress.yaml + set: + protocol: https + oidc: + upstream: + protocol: https + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/backend-protocol"] + value: http + + - it: ingress should keep the read timeout + template: templates/ingress.yaml + set: + readTimeout: 600 + oidc: + upstream: + readTimeout: 600 + asserts: + - equal: + path: metadata.annotations["nginx.ingress.kubernetes.io/proxy-read-timeout"] + value: "600" + + - it: ingress should send traffic for the path prefix to the OAuth2 proxy + template: templates/ingress.yaml + set: + global: + subdomainAsPathPrefix: true + asserts: + - equal: + path: spec.rules[0].host + value: foo.example.com + - equal: + path: spec.rules[0].http.paths[0].path + value: /bar + - equal: + path: spec.rules[0].http.paths[0].backend.service.name + value: svc1-oidc + + - it: OAuth2 proxy should forward requests to the service + template: charts/oidc/templates/secret-alpha.yaml + asserts: + - matchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: |- + upstreamConfig: + upstreams: + - id: zenith-service + path: / + uri: http://svc1.zenith-services.svc:80 + passHostHeader: true + proxyWebSockets: true + timeout: 3600s + - notMatchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: insecureSkipTLSVerify + - notMatchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: "static: true" + + - it: OAuth2 proxy should forward requests to an HTTPS service without verifying TLS + template: charts/oidc/templates/secret-alpha.yaml + set: + oidc: + upstream: + protocol: https + asserts: + - matchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: "uri: https://svc1.zenith-services.svc:443" + - matchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: "insecureSkipTLSVerify: true" + + - it: OAuth2 proxy should use the read timeout for the service + template: charts/oidc/templates/secret-alpha.yaml + set: + oidc: + upstream: + readTimeout: 600 + asserts: + - matchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: "timeout: 600s" + + - it: OAuth2 proxy should inject claims into the request to the service + template: charts/oidc/templates/secret-alpha.yaml + asserts: + - matchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: |- + injectRequestHeaders: + - name: X-Remote-User + values: + - claim: preferred_username + - name: X-Remote-Group + values: + - claim: groups + - notMatchRegex: + path: data["oauth2_proxy.yml"] + decodeBase64: true + pattern: injectResponseHeaders + + - it: OAuth2 proxy should run two replicas spread across nodes + template: charts/oidc/templates/deployment.yaml + asserts: + - equal: + path: spec.replicas + value: 2 + - equal: + path: spec.template.spec.topologySpreadConstraints + value: + - maxSkew: 1 + topologyKey: kubernetes.io/hostname + whenUnsatisfiable: ScheduleAnyway + labelSelector: + matchLabels: + app.kubernetes.io/name: oidc + matchLabelKeys: + - app.kubernetes.io/instance + # The constraint relies on these labels being on the pods + - equal: + path: spec.template.metadata.labels["app.kubernetes.io/name"] + value: oidc + - equal: + path: spec.template.metadata.labels["app.kubernetes.io/instance"] + value: svc1 + + - it: OAuth2 proxy should allow one replica to be disrupted at a time + template: charts/oidc/templates/poddisruptionbudget.yaml + asserts: + - hasDocuments: + count: 1 + - equal: + path: spec.minAvailable + value: 1 diff --git a/sync/tests/test_helm_processor.py b/sync/tests/test_helm_processor.py new file mode 100644 index 000000000..97132fac7 --- /dev/null +++ b/sync/tests/test_helm_processor.py @@ -0,0 +1,83 @@ +import logging +import unittest +from unittest import mock + +from zenith.sync import config, model +from zenith.sync.processor import helm + + +def make_processor(**oidc): + """ + Returns a processor with the given OIDC config, without connecting to Kubernetes. + """ + processor = helm.Processor.__new__(helm.Processor) + processor.config = config.KubernetesConfig( + self_namespace="zenith", + ingress=config.IngressConfig( + base_domain="apps.example.com", + oidc=config.OIDCConfig(discovery_enabled=True, **oidc), + ), + ) + processor.logger = logging.getLogger(__name__) + processor._reconcile_oidc_credentials = mock.AsyncMock( + return_value=("https://identity.example.com", "client-id", "secret", []) + ) + processor._reconcile_oidc_cookie_secret = mock.AsyncMock( + return_value="cookie-secret" + ) + return processor + + +class TestOIDCProxyValues(unittest.IsolatedAsyncioTestCase): + async def test_claims_are_injected_into_the_request(self): + processor = make_processor( + inject_request_headers={ + "X-Remote-User": "preferred_username", + "X-Remote-Group": "groups", + } + ) + values = await processor._get_auth_values(model.Service(name="svc1")) + config_data = values["oidc"]["alphaConfig"]["configData"] + self.assertEqual( + config_data["injectRequestHeaders"], + [ + {"name": "X-Remote-User", "values": [{"claim": "preferred_username"}]}, + {"name": "X-Remote-Group", "values": [{"claim": "groups"}]}, + ], + ) + self.assertNotIn("injectResponseHeaders", config_data) + + async def test_upstream_defaults(self): + processor = make_processor() + values = await processor._get_auth_values(model.Service(name="svc1")) + self.assertEqual( + values["oidc"]["upstream"], {"protocol": "http", "readTimeout": None} + ) + + async def test_upstream_matches_service(self): + processor = make_processor() + service = model.Service( + name="svc1", + config={"backend-protocol": "https", "read-timeout": "600"}, + ) + values = await processor._get_auth_values(service) + self.assertEqual( + values["oidc"]["upstream"], {"protocol": "https", "readTimeout": 600} + ) + # The values for the service itself should agree + service_values = processor._get_service_values(service) + self.assertEqual(service_values["protocol"], "https") + self.assertEqual(service_values["readTimeout"], 600) + + async def test_invalid_read_timeout_is_ignored(self): + processor = make_processor() + service = model.Service(name="svc1", config={"read-timeout": "ten"}) + values = await processor._get_auth_values(service) + self.assertIsNone(values["oidc"]["upstream"]["readTimeout"]) + self.assertNotIn("readTimeout", processor._get_service_values(service)) + + async def test_no_oidc_when_auth_skipped(self): + processor = make_processor() + service = model.Service(name="svc1", config={"skip-auth": True}) + values = await processor._get_auth_values(service) + self.assertEqual(values["oidc"], {"enabled": False}) From 75d50ec453024b3f0b6cace1c026cdd18b08c61d Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Mon, 12 Oct 2026 15:19:25 +0100 Subject: [PATCH 3/5] Fix jupyter session renewal Keeping a session open longer than 1 hour triggered Cross-origin errors due to the session timing out. I don't think this was caused by the changed I made in this PR --- sync/chart/values.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sync/chart/values.yaml b/sync/chart/values.yaml index 2947723d6..57df67b5b 100644 --- a/sync/chart/values.yaml +++ b/sync/chart/values.yaml @@ -176,7 +176,9 @@ oidc: # i.e. the length of time that a user *who already has a valid cookie* will be # able to continue using the service after their access has been removed at the # OIDC provider - cookie-refresh: 1h + # NB: must be shorter than the idle timeout for the session at the OIDC provider + # (Keycloak's default: 30m), otherwise the refresh token expires before it's used + cookie-refresh: 20m # If the ingress is not using TLS, we have to allow the cookie on insecure connections cookie-secure: '{{ ternary "true" "false" .Values.global.secure }}' # Allow redirections to the service domain only From 5e8fc7da6a6db6eb3ccdd585aef6f16efd6c1969 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Mon, 12 Oct 2026 16:18:03 +0100 Subject: [PATCH 4/5] Fix jupyter session renewal (2) Again I don't think this error is caused by the traefik work. --- sync/chart/values.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sync/chart/values.yaml b/sync/chart/values.yaml index 57df67b5b..278fc631f 100644 --- a/sync/chart/values.yaml +++ b/sync/chart/values.yaml @@ -92,6 +92,10 @@ oidc: allowedGroups: [] # Parameters that will be forwarded from the OIDC start URL to the IdP login URL loginURLParameters: [] + # Use PKCE to bind the authz code to the browser that started the login + # Replaces the code injection protection lost by skipping the nonce (see below) + # NB the key is snake_case in the oauth2-proxy alpha config + code_challenge_method: S256 oidcConfig: # The OIDC issuer URL issuerURL: @@ -102,6 +106,13 @@ oidc: insecureAllowUnverifiedEmail: true groupsClaim: groups audienceClaims: ["aud"] + # Don't verify the nonce claim of the ID token. + # This is oauth2-proxy's documented default, but it doesn't follow it's own + # default when using the alpha config + # The refreshed ID token from Keycloak does not carry the nonce from the original login + # so without this token referesh breaks + # https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-config?_highlight=nonce#oidcoptions + insecureSkipNonce: true # The OAuth2 proxy is in the data path for the service, so run two replicas # Sessions are stored in cookies, so the replicas don't need to share any state replicaCount: 2 From 8764b6bdd736e5ee322b0cf79f6cd9b1252ae606 Mon Sep 17 00:00:00 2001 From: Alex Manning Date: Mon, 12 Oct 2026 17:32:12 +0100 Subject: [PATCH 5/5] Spelling mistake --- sync/chart/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sync/chart/values.yaml b/sync/chart/values.yaml index 278fc631f..0591fba5c 100644 --- a/sync/chart/values.yaml +++ b/sync/chart/values.yaml @@ -110,7 +110,7 @@ oidc: # This is oauth2-proxy's documented default, but it doesn't follow it's own # default when using the alpha config # The refreshed ID token from Keycloak does not carry the nonce from the original login - # so without this token referesh breaks + # so without this token refresh breaks # https://oauth2-proxy.github.io/oauth2-proxy/configuration/alpha-config?_highlight=nonce#oidcoptions insecureSkipNonce: true # The OAuth2 proxy is in the data path for the service, so run two replicas