-
Notifications
You must be signed in to change notification settings - Fork 284
✨ Support Application Credential auth #1189
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
✨ Support Application Credential auth #1189
Conversation
|
Skipping CI for Draft Pull Request. |
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
/test all |
|
/retest Thanks for the extensive first round of review :) New commit with all requested changes is up. |
|
/hold cancel |
We currently require an explicit project_id field in the clouds.yaml. This conflicts with the usual fields for Application Credentials, as they already include a scope, and setting another conflicts with that. In this commit, we instead save the returned project_id from the initial auth api call, and pass it around to all services using the Scope struct.
|
Nice one. |
|
@apricote not sure it's eligible to add some test cases (with application ID) later on? /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apricote, jichenjc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@jichenjc I had a unit test for the In general I would appreciate a test for this, that would probably mean to extend our e2e tests to use an addition clouds.yaml secret that uses application credentials. Are these supported by devstack? |
What this PR does / why we need it:
We currently require an explicit
project_idfield in the clouds.yaml. This conflicts with the usual fields for Application Credentials, as they already include a scope, and setting another conflicts with that.In this commit, we instead save the returned
project_idfrom the initial auth api call, and pass it around to all services using theScopestruct.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #1146
Special notes for your reviewer:
/hold