codecentric / helm-charts

A curated set of Helm charts brought to you by codecentric
Apache License 2.0
626 stars 608 forks source link

Can I still use base path after I upgrade to keycloak 9.0.1 #283

Closed Jaorji closed 4 years ago

Jaorji commented 4 years ago

keycloak: 9.0.1 docker image: 11.0.0


scheme: http
enabled: true
fullnameOverride: keycloak
namespace:
  name: keycloak-{{.NAMESPACE}}
replicas: 2
image:
  repository: docker.io/jboss/keycloak
  tag: "11.0.0"
username: admin
password: admin
livenessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/
    port: 8080
  initialDelaySeconds: 60
  timeoutSeconds: 5
readinessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/realms/master
    port: 8080
  initialDelaySeconds: 60
  timeoutSeconds: 1
service:
  httpPort: 8080
pgchecker:
  image:
    # Docker image used to check Postgresql readiness at startup
    repository: docker.io/busybox
    # Image tag for the pgchecker image
    tag: 1.32
    # Image pull policy for the pgchecker image
    pullPolicy: IfNotPresent
extraEnv: |
  - name: PROXY_ADDRESS_FORWARDING
    value: "true"
  - name: JGROUPS_DISCOVERY_PROTOCOL
    value: dns.DNS_PING
  - name: JGROUPS_DISCOVERY_PROPERTIES
    value: 'dns_query={{ include "keycloak.serviceDnsName" . }}'
  - name: CACHE_OWNERS_COUNT
    value: "2"
  - name: CACHE_OWNERS_AUTH_SESSIONS_COUNT
    value: "2"
  - name: KEYCLOAK_STATISTICS
    value: all
  - name: KEYCLOAK_USER_FILE
    value: /secrets/admin-creds/user
  - name: KEYCLOAK_PASSWORD_FILE
    value: /secrets/admin-creds/password
  - name: DB_ADDR
    value: mns-k8s-rdsstack-uklzuz2g2ji6-auroradbcluster-mhaszntypx4.cluster-czgg7xuryu0e.us-east-1.rds.amazonaws.com
  - name: DB_DATABASE
    value: xji
  - name: DB_USER_FILE
    value: /secrets/db-creds/user
  - name: DB_PASSWORD_FILE
    value: /secrets/db-creds/password
  - name: JAVA_OPTS
    value: >-
      -XX:+UseContainerSupport
      -XX:MaxRAMPercentage=50.0
      -Djava.net.preferIPv4Stack=true
      -Djboss.modules.system.pkgs=$JBOSS_MODULES_SYSTEM_PKGS
      -Djava.awt.headless=true
secrets:
  db-creds:
    stringData:
      user: postgres
      password: postgres
  admin-creds:
    stringData:
      user: admin
      password: admin
extraVolumeMounts: |
  - name: admin-creds
    mountPath: /secrets/admin-creds
    readOnly: true
  - name: db-creds
    mountPath: /secrets/db-creds
    readOnly: true
  - name: data
    mountPath: /opt/jboss/keycloak/standalone/deployments
extraVolumes: |
  - name: admin-creds
    secret:
      secretName: '{{ include "keycloak.fullname" . }}-admin-creds'
  - name: db-creds
    secret:
      secretName: '{{ include "keycloak.fullname" . }}-db-creds'
  - name: data
    emptyDir: {}
# extraArgs: -Djgroups.bind_addr=global -Dkeycloak.profile.feature.upload_scripts=enabled
# cli:
#   custom: |
#     /subsystem=keycloak-server/spi=hostname/provider=default:write-attribute(name=properties.forceBackendUrlToFrontendUrl,value="true")
startupScripts:
# WildFly CLI script for configuring the node-identifier
  keycloak.cli: |
    {{- .Files.Get "scripts/keycloak.cli" }}
nodeSelector:
  load_type: application
podAnnotations:
  sidecar.istio.io/inject: "false"
resources:
  requests:
    cpu: "1000m"
    memory: "1024Mi"
test:
  enabled: false
enableServiceLinks: false
serviceAccount:
  # Specifies whether a ServiceAccount should be created
  create: true
  # The name of the service account to use.
  # If not set and create is true, a name is generated using the fullname template
  name: ""
  # Additional annotations for the ServiceAccount
  annotations: {}
  # Additional labels for the ServiceAccount
  labels: {}
  # Image pull secrets that are attached to the ServiceAccount
  imagePullSecrets: []
service:
  # Annotations for headless and HTTP Services
  annotations: {}
  # Additional labels for headless and HTTP Services
  labels: {}
  # key: value
  # The Service type
  type: ClusterIP
  # Optional IP for the load balancer. Used for services of type LoadBalancer only
  loadBalancerIP: ""
  # The http Service port
  httpPort: 8080
  # The HTTP Service node port if type is NodePort
  httpNodePort: null
  # The HTTPS Service port
  httpsPort: 8443
  # The HTTPS Service node port if type is NodePort
  httpsNodePort: null
  # The WildFly management Service port
  httpManagementPort: 9990
  # The WildFly management Service node port if type is NodePort
  httpManagementNodePort: null
  # Additional Service ports, e. g. for custom admin console
  extraPorts: []
rbac:
  create: false
  rules: []
ingress:
  # If `true`, an Ingress is created
  enabled: false
  # The Service port targeted by the Ingress
  servicePort: http
  # Ingress annotations
  annotations: {}
  # Additional Ingress labels
  labels: {}
   # List of rules for the Ingress
  rules:
    -
      # Ingress host
      host: '{{ .Release.Name }}.keycloak.example.com'
      # Paths for the host
      paths:
        - /
postgresql:
  enabled: false
basepath: xji/auth```
Jaorji commented 4 years ago

I got error when I use basePath here

unguiculus commented 4 years ago

basePath´ was removed. It was only used in liveness and readiness probes, which are freely configurable anyways. However, since probe configurations are passed through thetplfunction, you are free to configurebasepath` on your own and use it. It seems you are exactly doing this which should work. I just tested your configuration with out any issues. What error are you getting?

damianjaniszewski commented 4 years ago

I think the issue is that in previous versions of this helm chart basepath was used to not only change liveness and readiness probe url but also default context of the Keycloak server from auth to something else like in the example above to xji/auth. In 9.0.1 basepath is not used chart templates and regardless of its setting Keycloak server context is always /auth.

Jaorji commented 4 years ago

@unguiculus @damianjaniszewski Thank you so much for answer my question. I'm trying to upgrade keycloak from 7.3.0 to 9.0.1 since we have sync problems between two keycloak pods. However the basepath we have is /{{release.namespace}}/auth. Is there any way to set bathpath like that? Here is the liviness prob and readiness prob link: image And I got 404 👍 image

unguiculus commented 4 years ago

@damianjaniszewski basepath was not used for anything else than liveness and readiness probes in previous releases. If you want to have a different path, you'd have to configure that using a WildFly CLI script.

unguiculus commented 4 years ago

@Jaorji You can do something like this:

livenessProbe: |
  httpGet:
    path: /{{ .Release.Namespace }}/auth/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5
Jaorji commented 4 years ago

@unguiculus I have tried this:

  httpGet:
    path: /xji/auth/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5

However the log shows: (ServerService Thread Pool -- 66) WFLYUT0022: Unregistered web context: '/auth' from server 'default-server'

And I got 404 when I access the endpoint: image

If I can use WildFly CLI script., can you please give me an example on this.

zodiac1214 commented 4 years ago

@damianjaniszewski basepath was not used for anything else than liveness and readiness probes in previous releases. If you want to have a different path, you'd have to configure that using a WildFly CLI script.

cli:
    custom: |

is removed too?

unguiculus commented 4 years ago

Please read upgrade notes for changes and why they were done. https://github.com/codecentric/helm-charts/tree/master/charts/keycloak#from-chart-versions--900

The charts supports the startup script functionality of the official Docker image.

See https://github.com/keycloak/keycloak-containers/tree/master/server#running-custom-scripts-on-startup

startupScripts:
  mycustom.cli: |
    # CLI contents here
rexroof commented 4 years ago

removing this basepath setting is fairly disruptive for our use. we use different basepaths depending on our software configuration. now we have to manually add the liveness, readiness, ingress rule and the custom cli script every time we want to change the basepath.

rexroof commented 4 years ago

when we create these wildfly scripts that change our web-context / basepath, do we also need to determine if we are running with replicas > 1 in order to decide if we're using standalone.xml or standalone-ha.xml? the old chart setup was able to handle that difference for us.

danvy commented 4 years ago

I am impacted by the removal of basepath. I used it to shorten the url, removing the /auth/ part.

danvy commented 4 years ago

basePath´ was removed. It was only used in liveness and readiness probes, which are freely configurable anyways. However, since probe configurations are passed through thetplfunction, you are free to configurebasepath` on your own and use it. It seems you are exactly doing this which should work. I just tested your configuration with out any issues. What error are you getting?

@unguiculus It was also used here : https://github.com/codecentric/helm-charts/blob/keycloak-8.3.0/charts/keycloak/templates/configmap-startup.yaml

rexroof commented 4 years ago

@danvy 100%. this was super important. having this single tuneable option for the basepath really made this helm chart worthwhile for us.

unguiculus commented 4 years ago

You're correct. I was actually used to reconfigure Wildfly. Sorry for my confusion. As mentioned, it is possible to configure the chart for a different basepath. Here's how:

basepath: foo

startupScripts:
  basepath.cli: |
    embed-server {{- if gt (int .Values.replicas) 1 }} --server-config=standalone-ha.xml{{ end }} --std-out=echo
    batch
    {{- if ne .Values.basepath "auth" }}
      /subsystem=keycloak-server:write-attribute(name=web-context,value={{ if eq .Values.basepath "" }}ROOT{{ else }}{{ .Values.basepath }}{{ end }})
      {{- if eq .Values.basepath "" }}
        /subsystem=undertow/server=default-server/host=default-host:write-attribute(name=default-web-module,value=keycloak-server.war)
      {{- end }}
    {{ end }}
    run-batch
    stop-embedded-server

livenessProbe: |
  httpGet:
    path: /{{ .Values.basepath }}/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5

readinessProbe: |
  httpGet:
    path: /{{ .Values.basepath }}/realms/master
    port: http
  initialDelaySeconds: 30
  timeoutSeconds: 1

I can add a section to the readme. Would that work for everyone?

Jaorji commented 4 years ago

@unguiculus that would be great if you can add this to readme.

danvy commented 4 years ago

You're correct. I was actually used to reconfigure Wildfly. Sorry for my confusion. As mentioned, it is possible to configure the chart for a different basepath. Here's how:

basepath: foo

startupScripts:
  basepath.cli: |
    embed-server {{- if gt (int .Values.replicas) 1 }} --server-config=standalone-ha.xml{{ end }} --std-out=echo
    batch
    {{- if ne .Values.basepath "auth" }}
      /subsystem=keycloak-server:write-attribute(name=web-context,value={{ if eq .Values.basepath "" }}ROOT{{ else }}{{ .Values.basepath }}{{ end }})
      {{- if eq .Values.basepath "" }}
        /subsystem=undertow/server=default-server/host=default-host:write-attribute(name=default-web-module,value=keycloak-server.war)
      {{- end }}
    {{ end }}
    run-batch
    stop-embedded-server

livenessProbe: |
  httpGet:
    path: /{{ .Values.basepath }}/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5

readinessProbe: |
  httpGet:
    path: /{{ .Values.basepath }}/realms/master
    port: http
  initialDelaySeconds: 30
  timeoutSeconds: 1

I can add a section to the readme. Would that work for everyone? @unguiculus It partialy works. I edited probes like this so it works with an empty basepath

livenessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5
readinessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/realms/master
    port: http
  initialDelaySeconds: 30
  timeoutSeconds: 1

the startup creates a file called /opt/jboss/startup-scripts/basepath.cli containing :

embed-server --std-out=echo
batch
  /subsystem=keycloak-server:write-attribute(name=web-context,value=ROOT)
    /subsystem=undertow/server=default-server/host=default-host:write-attribute(name=default-web-module,value=keycloak-server.war)
run-batch
stop-embedded-server

I confirm that the standalone.xml file is modified but the service is still bound to /auth

danvy commented 4 years ago

I found that I have to modify the standalone-ha.xml file even if I only have 1 replica. https://hub.docker.com/r/jboss/keycloak/ says : "It's also worth to mention, that by default, keycloak uses standalone-ha.xml configuration (unless other server configuration is specified)." In the end I used :

basepath: ""
startupScripts:
  basepath.cli: |
    embed-server --server-config=standalone-ha.xml --std-out=echo
    batch
    {{- if ne .Values.basepath "auth" }}
    /subsystem=keycloak-server/:write-attribute(name=web-context,value={{ if eq .Values.basepath "" }}/{{ else }}{{ .Values.basepath }}{{ end }})
      {{- if eq .Values.basepath "" }}
    /subsystem=undertow/server=default-server/host=default-host:write-attribute(name=default-web-module,value=keycloak-server.war)
      {{- end }}
    {{- end }}
    run-batch
    stop-embedded-server
livenessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/
    port: http
  initialDelaySeconds: 300
  timeoutSeconds: 5
readinessProbe: |
  httpGet:
    path: {{ if ne .Values.basepath "" }}/{{ .Values.basepath }}{{ end }}/realms/master
    port: http
  initialDelaySeconds: 30
  timeoutSeconds: 1

And now, it works.

unguiculus commented 4 years ago

@danvy You right, the Docker image automatically uses standalone-ha.xmlunless you specify something else. So, my check for the number of replicas was unnecessary as you realized correctly.

See https://github.com/keycloak/keycloak-containers/blob/master/server/tools/docker-entrypoint.sh#L103-L105

rexroof commented 4 years ago

@danvy is this in a values file that you pass to this helm chart? or are you altering this helm chart with that values file? either way, i wasn't aware that you could use if statements inside the values you pass into a helm chart. or are you preprocessing this as a template somehow?

(edit) oh! I missed that these are string blocks that get templated again inside the chart. at least, I think thats what I'm seeing.

unguiculus commented 4 years ago

@rexroof Exactly, they are templated using the tpl function.

unguiculus commented 4 years ago

I added documentation here: https://github.com/codecentric/helm-charts/pull/298

Please have a look and review. Thanks.

@Jaorji @damianjaniszewski @rexroof @danvy @zodiac1214