Add identity and keycloak to charts #251

Zelldon · 2022-03-24T22:15:18Z

Breakthrough:

Manually tested via:

helm repo add bitnami https://charts.bitnami.com/bitnami
helm dependency update charts/ccsm-helm/charts/identity/
helm install zell-helm-test charts/ccsm-helm/ --set zeebe.enabled=false --set operate.enabled=false --set tasklist.enabled=false --set elasticsearch.enabled=false --set identity.image.tag=SNAPSHOT
k port-forward svc/zell-helm-test-identity 8080:80
k port-forward svc/zell-helm-test-keycl 18080:80 # without that we were not able to see the identity login page?

Added multiple new template tests. Integration tests verifies that pods become ready at least, more to come.

related #127

Add values related to secrets and documentation

Make a basic copy from operate and adjust names and references. Delete configmap since it doesn't seem to be necessary.

based on given values (and default values) keycloak secret will be deployed

Copy most of the values from operate and adjust them

Introduce the identity as sub chart into ccsm-helm

Add env vars, need to be values later

commons bitnami subchart brings several helper functions/templates which are quite useful.

Adjust identity deployment to reuse keycloak secrets and resolve secret names and service names etc.

The default LoadBalancer is not support on all cloud providers, which causes to not schedule the service

keycloak trim there service name to 20 chars, since wildfly only allows 23 (lol).

Identity depends on keycloak, and keycloak takes a while to start.

Zelldon · 2022-03-25T10:33:29Z

Don't worry about the size 😅 mostly because of golden files and tests. The core part is ~300 lines.

I kept the commits separated, to show my journey, but I can also rearrange and merge some together if this helps you on reviewing. Like merge CI and Test commits. Let me know if you need that :)

Integration tests seem to take longer than 10 minutes and go has a default limit of 10 minutes. We increase it to 1 hour. https://terratest.gruntwork.io/docs/testing-best-practices/timeouts-and-logging/

identity has no official image tag yet, this causes integration tests to fail. In order to run the test we set the snapshot tag on identity in values file.

npepinpe

👍

I may not be the right person to review this, to be honest. I tried it out, it works, but I can't really say much about the identity configuration itself 😄

🔧 The message I get when installing tells me I installed Tasklist, Operate, Zeebe, Elastic, but nothing about Identity and Keycloak or Postgres (even if it did install them). Would be nice to update the result message.
❓ When running, Operate and Tasklist kept crashing and restarting 🤷‍♂️ Not sure what to make of this. Identity works though 👍
❓ Why is keycloak a dependency of identity but not a dependency of the main chart (i.e. ccsm-helm/Chart.yaml)? What's the difference in putting it in one or the other?
🤡 I would've liked to see instead that people bring their own Keycloak, since there's no "official" chart for it and we're now coupled to the bitnami one, but imo that's more of a product decision. Bitnami is probably reliable anyway.
❓ Can we get rid of these warnings? e.g. Dependency zeebe did not declare a repository. Assuming it exists in the charts directory. It's one for each chart.

charts/ccsm-helm/charts/identity/Chart.yaml

charts/ccsm-helm/test/integration/integration_test.go

charts/ccsm-helm/values.yaml

menski

Looks good to me, just some missing replacements of operate with identity

charts/ccsm-helm/charts/identity/Chart.yaml

charts/ccsm-helm/charts/identity/templates/_helpers.tpl

charts/ccsm-helm/test/identity/deployment_test.go

charts/ccsm-helm/test/identity/goldenfiles_test.go

charts/ccsm-helm/test/identity/ingress_test.go

charts/ccsm-helm/test/identity/service_test.go

Zelldon · 2022-03-26T20:39:44Z

Thanks for your review @npepinpe

Regarding your comments:

I may not be the right person to review this, to be honest. I tried it out, it works, but I can't really say much about the identity configuration itself smile

🔧 The message I get when installing tells me I installed Tasklist, Operate, Zeebe, Elastic, but nothing about Identity and Keycloak or Postgres (even if it did install them). Would be nice to update the result message.

This makes sense, I missed that.

❓ When running, Operate and Tasklist kept crashing and restarting man_shrugging Not sure what to make of this. Identity works though +1

Hm never had this issue. For me identity crashloops, because of keycloaks unavailability.

❓ Why is keycloak a dependency of identity but not a dependency of the main chart (i.e. ccsm-helm/Chart.yaml)? What's the difference in putting it in one or the other?

I did this for a reason. First, Identity has the dependency to keycloak and not the others and secondly, otherwise the identity chart wouldn't have access to keycloak properties/values which makes the setup here much easier.

🤡 I would've liked to see instead that people bring their own Keycloak, since there's no "official" chart for it and we're now coupled to the bitnami one, but imo that's more of a product decision. Bitnami is probably reliable anyway.

Sorry, I might missed to point this out in the slack channel, I will do that next week. I think, I discussed that with @menski and we wanted to keep this simple for now and the decision was made to not bring an own keycloak. This would also make the setup harder, meaning the default configuration for identity and this would contradict a bit in my opinion our expected use case that the user directly want to set up ccsm and use it in prod (without too much configuration). I think what we can do is to add a condition for keycloak, to be able to disable it. But I wouldn't do that per default.

We decided for bitnami because we assume this to be stable / maintained enough.

❓ Can we get rid of these warnings? e.g. Dependency zeebe did not declare a repository. Assuming it exists in the charts directory. It's one for each chart.

Be aware that you see this only, because you directly installed the chart from the filesystem. This will not happen if the chart is packaged.

Co-authored-by: Sebastian Menski <sebastian@menski.org>

Zelldon · 2022-03-27T18:27:52Z

Hey @menski

thanks for your review!

Looks good to me, just some missing replacements of operate with identity

Yeah sorry, somehow missed that.

I have applied your suggestions, are you willing to accept the PR ? :)

Zelldon · 2022-03-28T06:55:53Z

I now added identity to the install (release) notes.

Default enabled:

NOTES:
______     ______     __    __     __  __     __   __     _____     ______        ______     __         ______     __  __     _____
/\  ___\   /\  __ \   /\ "-./  \   /\ \/\ \   /\ "-.\ \   /\  __-.  /\  __ \      /\  ___\   /\ \       /\  __ \   /\ \/\ \   /\  __-.
\ \ \____  \ \  __ \  \ \ \-./\ \  \ \ \_\ \  \ \ \-.  \  \ \ \/\ \ \ \  __ \     \ \ \____  \ \ \____  \ \ \/\ \  \ \ \_\ \  \ \ \/\ \
 \ \_____\  \ \_\ \_\  \ \_\ \ \_\  \ \_____\  \ \_\\"\_\  \ \____-  \ \_\ \_\     \ \_____\  \ \_____\  \ \_____\  \ \_____\  \ \____-
  \/_____/   \/_/\/_/   \/_/  \/_/   \/_____/   \/_/ \/_/   \/____/   \/_/\/_/      \/_____/   \/_____/   \/_____/   \/_____/   \/____/

(ccsm-helm - 0.0.26)

### Installed Services:

- Zeebe:
  - Docker Image used for Zeebe: camunda/zeebe:1.3.4
  - Zeebe Cluster Name: "ccsm-helm-test-zeebe"
  - Prometheus ServiceMonitor Enabled: false
- Operate:
  - Enabled: true
  - Docker Image used for Operate: camunda/operate:1.3.4
- Tasklist:
  - Enabled: true
  - Docker Image used for Tasklist: camunda/tasklist:1.3.4
- Identity:
  - Enabled: true
  - Docker Image used for Identity: camunda/identity:SNAPSHOT
  - Keycloak: bitnami/keycloak:16.1.1-debian-10-r52
- Elasticsearch:
  - Enabled: true
  - ElasticSearch URL: http://elasticsearch-master:9200

### Zeebe

The Cluster itself is not exposed as a service that means that you can use `kubectl port-forward` to access the Zeebe cluster from outside Kubernetes:

> kubectl port-forward svc/ccsm-helm-test-zeebe-gateway 26500:26500 -n zell-helm-test

Now you can connect your workers and clients to `localhost:26500`

### Operate

As part of the Operate HELM Chart an ingress definition can be deployed, but you require to have an Ingress Controller for that Ingress to be Exposed.
In order to deploy the ingress manifest, set `operate.ingress.enabled` to `true`.
If you don't have an Ingress Controller you can use kubectl port-forward to access Operate from outside the cluster:

> kubectl port-forward svc/ccsm-helm-test-operate 8080:80

Now you can point your browser to `http://localhost:8080`

Default user and password: "demo/demo"

### Tasklist

As part of Tasklist an ingress definition can be deployed, but you require to have an Ingress Contoller for that Ingress to be Exposed.
In order to deploy the ingress manifest, set `tasklist.ingress.enabled` to `true`.
If you don't have an Ingress Controller you can use kubectl port-forward to access tasklist from outside the cluster:

> kubectl port-forward svc/ccsm-helm-test-tasklist 8081:80

Now you can point your browser to `http://localhost:8080`

Default user and password: "demo/demo"

### Identity

As part of Identity an ingress definition can be deployed, but you require to have an Ingress Contoller for that Ingress to be Exposed.
In order to deploy the ingress manifest, set `identity.ingress.enabled` to `true`.

If you don't have an ingress controller you can use kubectl port-forward to access identity from outside the cluster:

> kubectl port-forward svc/ccsm-helm-test-identity 8080:80
> kubectl port-forward svc/ccsm-helm-test-keycl 18080:80

Now you can point your browser to `http://localhost:8080`. Identity will forward requests to keycloak (listening under `18080`).

Default user and password: "demo/demo"

Identity disabled:

NOTES:
______     ______     __    __     __  __     __   __     _____     ______        ______     __         ______     __  __     _____
/\  ___\   /\  __ \   /\ "-./  \   /\ \/\ \   /\ "-.\ \   /\  __-.  /\  __ \      /\  ___\   /\ \       /\  __ \   /\ \/\ \   /\  __-.
\ \ \____  \ \  __ \  \ \ \-./\ \  \ \ \_\ \  \ \ \-.  \  \ \ \/\ \ \ \  __ \     \ \ \____  \ \ \____  \ \ \/\ \  \ \ \_\ \  \ \ \/\ \
 \ \_____\  \ \_\ \_\  \ \_\ \ \_\  \ \_____\  \ \_\\"\_\  \ \____-  \ \_\ \_\     \ \_____\  \ \_____\  \ \_____\  \ \_____\  \ \____-
  \/_____/   \/_/\/_/   \/_/  \/_/   \/_____/   \/_/ \/_/   \/____/   \/_/\/_/      \/_____/   \/_____/   \/_____/   \/_____/   \/____/

(ccsm-helm - 0.0.26)

### Installed Services:

- Zeebe:
  - Docker Image used for Zeebe: camunda/zeebe:1.3.4
  - Zeebe Cluster Name: "ccsm-helm-test-zeebe"
  - Prometheus ServiceMonitor Enabled: false
- Operate:
  - Enabled: true
  - Docker Image used for Operate: camunda/operate:1.3.4
- Tasklist:
  - Enabled: true
  - Docker Image used for Tasklist: camunda/tasklist:1.3.4
- Identity:
  - Enabled: false
- Elasticsearch:
  - Enabled: true
  - ElasticSearch URL: http://elasticsearch-master:9200

### Zeebe

The Cluster itself is not exposed as a service that means that you can use `kubectl port-forward` to access the Zeebe cluster from outside Kubernetes:

> kubectl port-forward svc/ccsm-helm-test-zeebe-gateway 26500:26500 -n zell-helm-test

Now you can connect your workers and clients to `localhost:26500`

### Operate

As part of the Operate HELM Chart an ingress definition can be deployed, but you require to have an Ingress Controller for that Ingress to be Exposed.
In order to deploy the ingress manifest, set `operate.ingress.enabled` to `true`.
If you don't have an Ingress Controller you can use kubectl port-forward to access Operate from outside the cluster:

> kubectl port-forward svc/ccsm-helm-test-operate 8080:80

Now you can point your browser to `http://localhost:8080`

Default user and password: "demo/demo"

### Tasklist

As part of Tasklist an ingress definition can be deployed, but you require to have an Ingress Contoller for that Ingress to be Exposed.
In order to deploy the ingress manifest, set `tasklist.ingress.enabled` to `true`.
If you don't have an Ingress Controller you can use kubectl port-forward to access tasklist from outside the cluster:

> kubectl port-forward svc/ccsm-helm-test-tasklist 8081:80

Now you can point your browser to `http://localhost:8080`

Default user and password: "demo/demo"

\cc @npepinpe

Bitnami Common is part of keycloak, so it doesn't need to be imported directly. Remove condition, identity is anyway not part of the installation if disabled

dlavrenuek

Some comments and questions from my side

dlavrenuek · 2022-03-28T12:21:53Z

charts/ccsm-helm/charts/identity/Chart.yaml

+dependencies:
+  - name: keycloak
+    repository: "https://charts.bitnami.com/bitnami"
+    version: 7.1.6


Suggested change

version: 7.1.6

version: 7.1.6

charts/ccsm-helm/charts/identity/Chart.yaml

charts/ccsm-helm/charts/identity/templates/_helpers.tpl

charts/ccsm-helm/charts/identity/templates/deployment.yaml

dlavrenuek · 2022-03-28T12:37:31Z

charts/ccsm-helm/charts/identity/templates/deployment.yaml

+                name: {{ include "common.secrets.name" (dict "existingSecret" .Values.keycloak.auth.existingSecret "context" $) }}
+                key: admin-password
+            {{- else }}
+            valueFrom:


Is it correct that there are two valueFrom: used here?

Yes this is depending on the .Values.keycloak.auth.existingSecret only one branch is taken here :)

charts/ccsm-helm/test/identity/golden/deployment.golden.yaml

charts/ccsm-helm/test/identity/golden/ingress-all-enabled.golden.yaml

charts/ccsm-helm/test/identity/golden/ingress.golden.yaml

Co-authored-by: dlavrenuek <20122620+dlavrenuek@users.noreply.github.com>

Zelldon · 2022-03-28T14:04:19Z

Thanks to all reviewers 🙇 👍

Zelldon added 15 commits March 24, 2022 15:19

feat: add first identity and keycloak values

3887a72

Add values related to secrets and documentation

feat: add identity sub chart

4fc8de1

Make a basic copy from operate and adjust names and references. Delete configmap since it doesn't seem to be necessary.

feat: add keycloak-secret

143f579

based on given values (and default values) keycloak secret will be deployed

fix: rm config map

85bd4b8

feat: define default values

b502c53

Copy most of the values from operate and adjust them

feat: add identity to Chart.yaml

e534fad

Introduce the identity as sub chart into ccsm-helm

feat: add keycloak dependency

b7f2235

feat: add missing env vars

976aae6

Add env vars, need to be values later

refactor: remove default secret

23dc7be

feat: add commons as deps

02acf2c

commons bitnami subchart brings several helper functions/templates which are quite useful.

feat: use keycloak secrets

fea6eaf

Adjust identity deployment to reuse keycloak secrets and resolve secret names and service names etc.

fix: use clusterIP for keycloak service

12ba32b

The default LoadBalancer is not support on all cloud providers, which causes to not schedule the service

fix: trim keycloak name

de79d01

keycloak trim there service name to 20 chars, since wildfly only allows 23 (lol).

ci: add instructions to get deps

e502672

ci: fix path for subchart get deps

2b1703f

Zelldon mentioned this pull request Mar 25, 2022

[EPIC]: Add Identity #127

Closed

22 tasks

Zelldon added 14 commits March 25, 2022 06:19

ci: use direct path

4d38f20

ci: get subchart deps in makefile

e8b1681

ci: add missing repo

be4b65d

ci: use makefile in gha

2ac31ef

docs: update development instructions

cf87922

ci: add deps as pre-requisite

62c1d0b

fix: set correct component name

22c9781

test: add golden test for identity

307d0ef

test: add service test

69666bb

test: add ingress golden test

9ced7f6

test: add deployment test

1ae6362

feat: allow template env vars

fc76d04

test: bring own secret

2a90c82

test: increase retries to 100

53fca07

Identity depends on keycloak, and keycloak takes a while to start.

Zelldon requested review from npepinpe and menski March 25, 2022 10:25

style: gofmt

4674859

Zelldon marked this pull request as ready for review March 25, 2022 10:27

Zelldon added 2 commits March 25, 2022 11:43

test: increate it timeout

70ec4d7

Integration tests seem to take longer than 10 minutes and go has a default limit of 10 minutes. We increase it to 1 hour. https://terratest.gruntwork.io/docs/testing-best-practices/timeouts-and-logging/

fix: use snapshot tag for now

13f0795

Zelldon requested a review from dlavrenuek March 25, 2022 10:49

test: use snapshot tag for now

8a9d137

identity has no official image tag yet, this causes integration tests to fail. In order to run the test we set the snapshot tag on identity in values file.

npepinpe approved these changes Mar 25, 2022

View reviewed changes

charts/ccsm-helm/charts/identity/Chart.yaml Outdated Show resolved Hide resolved

charts/ccsm-helm/test/integration/integration_test.go Outdated Show resolved Hide resolved

charts/ccsm-helm/values.yaml Show resolved Hide resolved

menski suggested changes Mar 25, 2022

View reviewed changes

Zelldon and others added 2 commits March 27, 2022 20:26

refactor: rename to identity

47e916c

Co-authored-by: Sebastian Menski <sebastian@menski.org>

test: reduce poll time and inc retries

b40befc

Zelldon force-pushed the zell-identity-keycloak branch from fd362c5 to b40befc Compare March 27, 2022 18:26

Merge branch 'main' into zell-identity-keycloak

91bc0f4

Zelldon added 2 commits March 28, 2022 08:56

feat: add identity to release notes

e2be717

build: remove common and condition

99ea530

Bitnami Common is part of keycloak, so it doesn't need to be imported directly. Remove condition, identity is anyway not part of the installation if disabled

dlavrenuek requested changes Mar 28, 2022

View reviewed changes

refactor: use iam logo

e1ae65a

Co-authored-by: dlavrenuek <20122620+dlavrenuek@users.noreply.github.com>

menski approved these changes Mar 28, 2022

View reviewed changes

refactor: replace operate with identity

0eb25f5

dlavrenuek approved these changes Mar 28, 2022

View reviewed changes

Zelldon merged commit ade7c3e into main Mar 28, 2022

Zelldon deleted the zell-identity-keycloak branch March 28, 2022 14:04

npepinpe assigned Zelldon Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add identity and keycloak to charts #251

Add identity and keycloak to charts #251

Zelldon commented Mar 24, 2022 •

edited

Zelldon commented Mar 25, 2022

npepinpe left a comment

menski left a comment

Zelldon commented Mar 26, 2022 •

edited

Zelldon commented Mar 27, 2022

Zelldon commented Mar 28, 2022

dlavrenuek left a comment

dlavrenuek Mar 28, 2022

dlavrenuek Mar 28, 2022

Zelldon Mar 28, 2022

Zelldon commented Mar 28, 2022

Add identity and keycloak to charts #251

Add identity and keycloak to charts #251

Conversation

Zelldon commented Mar 24, 2022 • edited

Zelldon commented Mar 25, 2022

npepinpe left a comment

Choose a reason for hiding this comment

menski left a comment

Choose a reason for hiding this comment

Zelldon commented Mar 26, 2022 • edited

Zelldon commented Mar 27, 2022

Zelldon commented Mar 28, 2022

dlavrenuek left a comment

Choose a reason for hiding this comment

dlavrenuek Mar 28, 2022

Choose a reason for hiding this comment

dlavrenuek Mar 28, 2022

Choose a reason for hiding this comment

Zelldon Mar 28, 2022

Choose a reason for hiding this comment

Zelldon commented Mar 28, 2022

Zelldon commented Mar 24, 2022 •

edited

Zelldon commented Mar 26, 2022 •

edited