[bitnami/keycloak] Add support for proxy-headers

[Kajot-dev]

Description of the change

Added support for proxy-headers via KEYCLOAK_PROXY_HEADERS. This also removes support for KEYCLOAK_PROXY
Implementing both this and KEYCLOAK_PROXY when the current default for KEYCLOAK_PROXY is passthorugh (and not empty) would be a mess since we won't really know if user meant to set "passthrough" or just did not specify this at all (unless we would change the default, but this would be a breaking change anyway) and creating to many possible combinations.

Benefits

Not using deprecated options

Possible drawbacks

Lack of KEYCLOAK_PROXY must be accounted for in helm charts etc.

Applicable issues

• fixes [bitnami/keycloak] Adapt scripts to proxy related changes in Keycloak 24 #65190

[juan131]

Hi @Kajot-dev

This isn't simply adding support for proxy-headers but dropping support for customizing the proxy mode (which is going to be deprecated). Based on the Upgrading Guide below:

• https://www.keycloak.org/docs/latest/upgrading/index.html#deprecated-proxy-option

I'm assuming you're not setting any proxy-header by default given it's the equivalent to the "passthrough" proxy mode, am I right?

[Kajot-dev]

@juan131 Yes, that's correct!

If you wish, I may try to implement backwards compatibility by introducing KEYCLOAK_LEGACY_PROXY which defaulta to the value of KEYCLOAK_PROXY (and is only a helper variable) before it's assigned the default "passthrough" value. That way the intentions of the user would be clear. Then if KEYCLOAK_LEGACY_PROXY is empty, we can use proxy-headers, else use proxy setting like before. Let me know what you think?

[juan131]

I don't think that's necessary, it's overcomplicated.

We should add some mechanism though in the associated chart. For instance, we could mark the proxy parameter as deprecated and introduce a backwards compatible mechanism:

• values.yaml:

+## @param proxyHeaders Set Keycloak proxy headers
+##
+proxyHeaders: ""

 ## @param proxy reverse Proxy mode edge, reencrypt, passthrough or none
+## DEPRECATED: use proxyHeaders instead
 ## ref: https://www.keycloak.org/server/reverseproxy
 ##
-proxy: passthrough
+proxy: ""

• templates/configmap-env-vars.yaml

+ {{- if and .Values.proxy (empty .Values.proxyHeaders }}
- KEYCLOAK_PROXY: {{ .Values.proxy | quote }}
+ KEYCLOAK_PROXY_HEADERS: {{ ternary "" "forwarded|xforwarded" (eq .Values.proxy "passthrough") }} 
+ {{- else }}
+ KEYCLOAK_PROXY_HEADERS: {{ .Values.proxyHeaders | quote }} 
+ {{- end }}

[Kajot-dev]

I will make counterpart PR for helm chart and link it here

[singhbaljit]

According to the docs, the edge mode (kc.sh --proxy edge) is equivalent to kc.sh --proxy-headers forwarded|xforwarded --http-enabled true. Don't we need to add another variable to also support the http-enabled option (KC_HTTP_ENABLED)?.. HTTP is disabled by default... or just rely on the Keycloak provided variable. Perhaps we keep the proxy mode still an option in the chart, but conditionally update the KEYCLOAK_PROXY_HEADERS and KC_HTTP_ENABLED based on the mode value, per that table.

[Kajot-dev]

According to the docs, the edge mode (kc.sh --proxy edge) is equivalent to kc.sh --proxy-headers forwarded|xforwarded --http-enabled true. Don't we need to add another variable to also support the http-enabled option (KC_HTTP_ENABLED)?.. HTTP is disabled by default... or just rely on the Keycloak provided variable. Perhaps we keep the proxy mode still an option in the chart, but conditionally update the KEYCLOAK_PROXY_HEADERS and KC_HTTP_ENABLED based on the mode value, per that table.

I don't think so, because of:
https://github.com/bitnami/containers/blob/main/bitnami/keycloak/24/debian-12/rootfs/opt/bitnami/scripts/libkeycloak.sh#L222

In bitnami containers http-enabled is always set to true unconditionally

In other words helm chart can just disable/disallow enabling https together with deprecated proxy in edge mode

[Kajot-dev]

@juan131 Please see: bitnami/charts#27890

[juan131]

LGTM

[juan131]

Hi @Kajot-dev

We released a new container image version (see 24.0.5-debian-12-r3) including your changes, see 536990b

Thanks for your contribution