-
Notifications
You must be signed in to change notification settings - Fork 59
support no-auth mode in k8s #2603
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
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
OriolMunoz-da
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.
tihi
|
When writing the docs, I also realized I have a small testing gap in Pulumi, since I'm still creating auth-related secrets which the docs will say you can skip for no-auth, will fix the tests (and anything they expose...) |
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
|
@OriolMunoz-da @martinflorian-da indeed that exposed some issues when those secrets did not exist. All fixed now. Mind taking another look before I merge please, for sanity? (successful test on the latest, FTR: https://app.circleci.com/pipelines/github/DACH-NY/canton-network-internal/35854/workflows/65f0f663-6c77-4290-beba-b4720d39c8fb) |
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
Signed-off-by: Itai Segall <itai.segall@digitalasset.com>
redeploy_sv_runbook (191761) failed, though at a quick glance I cannot tell if it's caused by this PR or just flaked |
|
Thanks @OriolMunoz-da |
martinflorian-da
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.
LGTM, thanks! (And sorry for ignoring for so long...)
OriolMunoz-da
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.
also LGTM provided that ci run passes 😄
Woohoo, that passed. |
| value: ledger-api-user | ||
| - name: ADDITIONAL_CONFIG_DISABLE_AUTH | ||
| value: | | ||
| canton.participants.participant.ledger-api.auth-services=[] |
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.
@isegall-da Sorry I only reviewed this now but I think this is the wrong approach. You now made the same mistake that we made in the docker compose deployment (but have an issue to fix and did fix for localnet): For the ledger API you disabled auth completely. For the validator we have hmac shared secret auth. This mismatch has caused confusion for a number of users. Imho we should use hmac shared secret auth for both.
Cluster test with no-auth validator1
(docs coming in a few minutes, sorry)
Pull Request Checklist
Cluster Testing
/cluster_teston this PR to request it, and ping someone with access to the DA-internal system to approve it./hdm_teston this PR to request it, and ping someone with access to the DA-internal system to approve it.PR Guidelines
Fixes #n, and mention issues worked on using#nMerge Guidelines