Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[incubator/keycloak] Add miscellaneous improvements #5372

Merged
merged 3 commits into from
May 18, 2018

Conversation

unguiculus
Copy link
Member

@unguiculus unguiculus commented May 3, 2018

  • Update Keycloak to 3.4.3.Final
  • Use nc to check for PostgreSQL availability in order to avoid
    RBAC configurations
  • Allow additional volumes and mounts to be configured
  • Allow custom pre-start script to be configured
  • Improve affinity configuration
  • Add security context
  • Update _helpers.tpl
  • Only add environment variables for Keycloak credentials on
    install. They are only needed for creating the intial user.
    This avoids an error in the logs
  • Add pod disruption budget
  • Make test more robust
  • Update Postgresql dependency

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 3, 2018
@unguiculus
Copy link
Member Author

cc @nexeck

@unguiculus unguiculus force-pushed the feature/keycloak branch 4 times, most recently from 0c224fa to 231f923 Compare May 3, 2018 12:51
* Update Keycloak to 3.4.3.Final
* Use nc to check for PostgreSQL availability in order to avoid
  RBAC configurations
* Allow additional volumes and mounts to be configured
* Allow custom pre-start script to be configured
* Improve affinity configuration
* Add security context
* Update _helpers.tpl
* Only add environment variables for Keycloak credentials on
  install. They are only needed for creating the intial user.
  This avoids an error in the logs
* Add pod disruption budget
* Make test more robust
@monotek
Copy link
Collaborator

monotek commented May 10, 2018

/assign lachie83

@arroo
Copy link

arroo commented May 11, 2018

why get rid of RBAC? if KUBE_PING gets adopted won't that be needed?

@unguiculus
Copy link
Member Author

@mozesmagyar The chart uses JDBC_PING which doesn't need RBAC.

@unguiculus
Copy link
Member Author

@nexeck Please review.


exec /opt/jboss/docker-entrypoint.sh "${args[@]}"
exec /opt/jboss/docker-entrypoint.sh -b 0.0.0.0 {{- if $highAvailability }} --server-config standalone-ha.xml{{ end }}
Copy link

@arroo arroo May 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible to include additional args? for example, it would be nice to include importing realm settings for an install...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me think about it. I'd say this could go into a separate PR.

@arroo
Copy link

arroo commented May 17, 2018

what's the likelihood of this getting merged before the 25th?

@lachie83
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 18, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lachie83, unguiculus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [lachie83,unguiculus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 8759f58 into helm:master May 18, 2018
@masonhensley
Copy link

masonhensley commented May 18, 2018

Thanks for the improvements unguiculus! The change below with tls is more consistent with other helm charts my team uses.

For future searchers (see error at end of comment). helm upgrade -f custom-values.yml incubator/keycloak will break for you if you where previously on version 0.3 and didn't pin your version.

1

This update breaks custom tls values, you will need to upgrade your custom-values.yaml if you specify one. Easy 30 second fix, see the before and after of the values in the next link...

https://github.com/kubernetes/charts/pull/5372/files#diff-7b15cc8622ffbf734ff8d6a926c2fbd0L140

For instance, my values changed from:

tls:
  enabled: true
  existingSecret: "my-auth-tls-secret"
  secretContents: {}
  secretAnnotations: {}

To

tls:
  - hosts:
    - my.super-awesome-custom-url.com
    secretName: my-auth-tls-secret

2

Also, additionalEnv changed to extraEnv.
https://github.com/kubernetes/charts/pull/5372/files#diff-979e22b13dd707c03ff2d3b1e417c56cL147

3

The following also changes, see

hyperkube:
  image:

https://github.com/kubernetes/charts/pull/5372/files#diff-7b15cc8622ffbf734ff8d6a926c2fbd0L1


How to pin your helm chart

helm upgrade -f custom-values.yml incubator/keycloak --version=0.X.0

Helm upgrade error I encountered to hopefully help with SEO:

2018/05/18 12:00:00 warning: destination for tls is a table. Ignoring non-table value []
Error: UPGRADE FAILED: render error in "keycloak/templates/ingress.yaml": template: keycloak/templates/ingress.yaml:21:16: executing "keycloak/templates/ingress.yaml" at <.hosts>: can't evaluate field hosts in type interface {}

@unguiculus unguiculus deleted the feature/keycloak branch June 4, 2018 06:49
or1can pushed a commit to or1can/charts that referenced this pull request Jul 10, 2018
* [incubator/keycloak] Add miscellaneous improvements

* Update Keycloak to 3.4.3.Final
* Use nc to check for PostgreSQL availability in order to avoid
  RBAC configurations
* Allow additional volumes and mounts to be configured
* Allow custom pre-start script to be configured
* Improve affinity configuration
* Add security context
* Update _helpers.tpl
* Only add environment variables for Keycloak credentials on
  install. They are only needed for creating the intial user.
  This avoids an error in the logs
* Add pod disruption budget
* Make test more robust

* Update Postgresql dependency

* Align ingress configuration with best practices
voron pushed a commit to dysnix/helm-charts that referenced this pull request Sep 5, 2018
* [incubator/keycloak] Add miscellaneous improvements

* Update Keycloak to 3.4.3.Final
* Use nc to check for PostgreSQL availability in order to avoid
  RBAC configurations
* Allow additional volumes and mounts to be configured
* Allow custom pre-start script to be configured
* Improve affinity configuration
* Add security context
* Update _helpers.tpl
* Only add environment variables for Keycloak credentials on
  install. They are only needed for creating the intial user.
  This avoids an error in the logs
* Add pod disruption budget
* Make test more robust

* Update Postgresql dependency

* Align ingress configuration with best practices

Signed-off-by: voron <av@arilot.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants