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: add console auth vars #1782

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

urbanisierung
Copy link
Member

@urbanisierung urbanisierung commented May 13, 2024

Which problem does the PR fix?

What's in this PR?

Running make go.update-golden-only leads to the following error:

make go.update-golden-only
Error: error unpacking identity in camunda-platform: Chart.yaml file is missing
make: *** [Makefile:97: helm.dependency-update] Error 1

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?

@aabouzaid aabouzaid changed the title feat(console): add auth vars feat: add console auth vars May 13, 2024
@aabouzaid aabouzaid added the kind/enhancement New feature or request label May 13, 2024
@urbanisierung urbanisierung marked this pull request as ready for review May 14, 2024 10:36
@urbanisierung urbanisierung enabled auto-merge (squash) May 14, 2024 10:37
@urbanisierung
Copy link
Member Author

@aabouzaid Thanks for fixing the PR! The introduced vars are needed for a feature for console self-managed: byo oidc. Would be great if you can review it.

@@ -63,3 +63,18 @@ Get the image pull secrets.
"context" $
) -}}
{{- end }}

{{/*
[console] Define variables related to authentication.
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to wrap the plain vars in the named templates?
If not, let's use them directly in the configmap.yaml

Copy link
Member Author

Choose a reason for hiding this comment

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

@aabouzaid Not sure what you mean ;) I just need the vars in the configmap.

If you could make the adjustments I'd be more than happy.

Copy link
Member

Choose a reason for hiding this comment

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

@urbanisierung my question about doing this:

clientId: {{ include "console.authClientId" . | quote }}

not this:

clientId: {{- .Values.global.identity.auth.console.clientId | quote -}}

the point here, it's working like a var, but used in 1 place only.
do you have any plans to use it in different places or extend the logic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants