Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: execution identity integration #2247

Merged
merged 17 commits into from
Sep 4, 2024

Conversation

drodriguez-305
Copy link
Contributor

@drodriguez-305 drodriguez-305 commented Aug 20, 2024

Which problem does the PR fix?

Related to #2195

This is the first iteration of adding Execution identity component to the clusters.

What's in this PR?

see related issue

Checklist

Please make sure to follow our Contributing Guide.

Before opening the PR:

  • In the repo's root dir, run make go.update-golden-only.
  • There is no other open pull request for the same update/change.
  • Tests for charts are added (if needed).
  • In-repo documentation are updated (if needed).

After opening the PR:

  • Did you sign our CLA (Contributor License Agreement)? It will show once you open the PR.
  • Did all checks/tests pass in the PR?

@drodriguez-305 drodriguez-305 added kind/enhancement New feature or request cycle/alpha5 Tasks will be done in alpha5 cycle labels Aug 20, 2024
@drodriguez-305 drodriguez-305 self-assigned this Aug 20, 2024
@drodriguez-305 drodriguez-305 linked an issue Aug 20, 2024 that may be closed by this pull request
@drodriguez-305 drodriguez-305 changed the title [ENHANCEMENT] Execution Identity integration [feat] Execution Identity integration Aug 20, 2024
@drodriguez-305 drodriguez-305 changed the title [feat] Execution Identity integration feat: Execution Identity integration Aug 20, 2024
@drodriguez-305
Copy link
Contributor Author

@aabouzaid can you double check this please?

On the configmap, I wasn't sure about passing the member-id: identity I did find an example but that is referencing the metadata.name which would be incorrect in this case.

- name: ZEEBE_GATEWAY_CLUSTER_MEMBERID
valueFrom:
fieldRef:
fieldPath: metadata.name

For Camunda database configuration I saw we had this already in a few other configmaps I just left out the opensearch portion as the ask was only for elasticsearch. Let me know I can add it if needed.

Also for the spring portion I just hard coded it as others have it set the same way.

@drodriguez-305 drodriguez-305 changed the title feat: Execution Identity integration feat: execution identity integration Aug 20, 2024
@drodriguez-305 drodriguez-305 added the feature Indicates a new feature request label Aug 20, 2024
@drodriguez-305
Copy link
Contributor Author

drodriguez-305 commented Aug 21, 2024

Hey Team @megglos @Ben-Sheppard

Can you check to see if any other env vars are needed?

env:
- name: CAMUNDA_LICENSE_KEY
valueFrom:
secretKeyRef:
name: {{ include "camundaPlatform.licenseSecretName" . }}
key: {{ include "camundaPlatform.licenseSecretKey" . }}
{{- if .Values.executionIdentity.env}}
{{ .Values.executionIdentity.env | toYaml | nindent 12 }}
{{- end }}
{{- if .Values.executionIdentity.envFrom }}
envFrom:
{{- .Values.executionIdentity.envFrom | toYaml | nindent 12 }}

The ones requested in the main issue was added via configmap

https://github.com/camunda/camunda-platform-helm/blob/2195-enhancement-execution-identity-integration/charts/camunda-platform-alpha/templates/execution-identity/configmap.yaml

@Ben-Sheppard
Copy link
Contributor

Hey @drodriguez-305 :).-

Looking at the configuration added it seems like thats enough, until the recent license changes I've only ran the app with the config we shared in the main issue so should be good! To answer the other questions:

I wasn't sure about passing the member-id: identity
My understanding here is that the application needs to tell Zeebe who it is thats connecting so here we opt for identity as its closest, I'm not sure if its handled in a different way but I also think that the controller repo may have more examples

For Camunda database configuration I saw we had this already in a few other configmaps I just left out the opensearch portion as the ask was only for elasticsearch. Let me know I can add it if needed.

Thats fine for now, we will want OpenSearch at some point but we can add it later

@megglos
Copy link
Contributor

megglos commented Aug 26, 2024

For Camunda database configuration I saw we had this already in a few other configmaps I just left out the opensearch portion as the ask was only for elasticsearch. Let me know I can add it if needed.

Thats fine for now, we will want OpenSearch at some point but we can add it later

Can you please create a follow-up for adding opensearch support @drodriguez-305 ?

@drodriguez-305 drodriguez-305 force-pushed the 2195-enhancement-execution-identity-integration branch from 1ee521d to a63c64c Compare August 28, 2024 18:18
@drodriguez-305 drodriguez-305 added test-persistent Don't delete the Helm deployment created by the PR and use fixed names and removed test-persistent Don't delete the Helm deployment created by the PR and use fixed names labels Aug 28, 2024
@drodriguez-305 drodriguez-305 requested review from a team and removed request for jessesimpson36 September 3, 2024 12:48
Copy link
Member

@aabouzaid aabouzaid left a comment

Choose a reason for hiding this comment

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

LGTM, thank you 👏

@aabouzaid aabouzaid merged commit 31a8ab1 into main Sep 4, 2024
19 checks passed
@aabouzaid aabouzaid deleted the 2195-enhancement-execution-identity-integration branch September 4, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cycle/alpha5 Tasks will be done in alpha5 cycle feature Indicates a new feature request kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Execution Identity integration
4 participants