-
Notifications
You must be signed in to change notification settings - Fork 223
6671 adjust properties for identity initialization #6917
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
base: main
Are you sure you want to change the base?
6671 adjust properties for identity initialization #6917
Conversation
👋 🤖 ✅ Looks like the changes were ported across versions, nice job! 🎉 You can read more about the versioning within our docs in our documentation guidelines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for preparing, please see my detailed comments.
In addition, on the initialization page please also add a Helm tab to the example for the default user configuration.
docs/self-managed/components/orchestration-cluster/identity/overview.md
Outdated
Show resolved
Hide resolved
docs/self-managed/components/orchestration-cluster/core-settings/configuration/properties.md
Outdated
Show resolved
Hide resolved
|
||
| Property | Description | Default value | | ||
| ---------------------------------------------------------- | ---------------------------------------------------------------- | ------------------------------------ | | ||
| `orchestration.security.authentication.oidc.audiences` | Comma-separated list of audiences to validate in the OIDC token. | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| `orchestration.security.authentication.oidc.audiences` | Comma-separated list of audiences to validate in the OIDC token. | | | |
| `orchestration.security.authentication.oidc.audience` | Comma-separated list of audiences to validate in the OIDC token. | | |
And I believe that takes a single value (might be wrong though). See also the Helm value orchestration.security.authentication.oidc.backwardsCompatibleAudiences
that we could mention here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to not document this property here, its a property that is automatically set by the helm charts when required and I'm not sure its inclusion here adds much value in typical operation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of the migration guide here: https://docs.camunda.io/docs/next/self-managed/deployment/helm/upgrade/upgrade-hc-870-880/#values-key-changes
Anyway, I don't mind too much not documenting it, you can decide.
docs/self-managed/components/orchestration-cluster/core-settings/configuration/properties.md
Outdated
Show resolved
Hide resolved
aca6e97
to
68ae2fd
Compare
@Ben-Sheppard Just let me know when you are ready for final review 👍 |
68ae2fd
to
1f1cb03
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me.
Re the one open discussion, feel free to decide in either direction.
Ah, and one more comment: Now that the 8.8 docs are released, this needs a "backport" into the 8.8 directory. |
Thank you @ThorbenLindhauer - the changes are already applied to the 8.8 docs 👍🏻 This is ready for a final review @christinaausley :) |
Description
Closes #6671
This PR updates the overview page for Orchestration Identity focusing on renaming the existing
Helm
tab to beapplication.yaml
and adding a helm tab where necessary.Additionally I spotted some inconsistencies in the property reference recently change too so included a cleanup here.
As per the linked issue, I looked for additional places but couldn't immediately spot any relevant configuration for the new architecture where the config is and didn't want to block this PR because of that. If there are additional places I will follow up with them.
When should this change go live?
bug
orsupport
label)available & undocumented
label)hold
label)low prio
label)PR Checklist
{type}(scope): {description}
commit message(s)/docs
directory (version 8.8)./versioned_docs
directory.@camunda/tech-writers
unless working with an embedded writer.