-
Notifications
You must be signed in to change notification settings - Fork 226
fix: helm production guide OIDC update #7440
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
Conversation
dd4fcfd to
1ef3f33
Compare
1ef3f33 to
57c315c
Compare
ThorbenLindhauer
left a comment
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 the PR. The change in itself looks fine to me. However, I have two questions before I can approve:
- Can you please clarify the reason why we recommended AWS Simple Active Directory and said it's compatible with Entra (we discussed this already on slack, just to make it visible here, too)?
- With this change, we change the nature of this guide a bit: In the previous state, it's an opinionated guide regarding all technology choices (essentially saying "set up everything with AWS"); now we leave the choice of OIDC provider open to the user; I have two potential concerns here:
- What is the goal of this guide? Should it show users how to set up C8 on AWS end-to-end? If yes, should we rather show how to set up concretely with an AWS OIDC provider (AWS Cognito as I understand)? I think PM would be the stakeholder here
- We are not able to give the user a full values file anymore (we have the placeholders for the Identity config); I think that makes the guide more error-prone, especially for less experienced people
Original intention seems to be to stick to an AWS specific setup; however it seems to have been missed that AWS Simple AD is not actually compatible with OIDC.
This is sadly a compromise. However the previous state of the guide was just straight up wrong, the current scope of the epic is just to correct the OIDC parts. However we do not run tests at the moment with AWS specific OIDC solutions (e.g. Cognito) so recommending one without a nightly test I am not a big fan of. For now, I believe that this is better than supplying a config that does not actually work (current state). Once merged, I will create a follow-up issue to re-visit the production guide, as we have received some other feedback to it too. |
57c315c to
e9d23db
Compare
ThorbenLindhauer
left a comment
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 the clarification. Looks good to me then. Agree we should review this at some point then.
e9d23db to
2d53fa8
Compare
Description
This PR makes the production Helm installation guide IdP-agnostic for authentication by removing incorrect AWS Simple AD references and replacing inline OIDC configuration with links to the authoritative authentication guides. The other AWS specific services stay.
Problem: The original guide incorrectly claimed AWS Simple Active Directory supports OIDC authentication (it doesn't: Simple AD is Samba 4-based LDAP). It also included ~150 lines of hardcoded Microsoft Entra ID configuration that duplicated content from the dedicated authentication guides and could become outdated.
When should this change go live?
bugorsupportlabel)available & undocumentedlabel)holdlabel)low priolabel)PR Checklist
{type}(scope): {description}commit message(s)/docsdirectory (version 8.9)./versioned_docsdirectory.@camunda/tech-writersunless working with an embedded writer.