-
Notifications
You must be signed in to change notification settings - Fork 46
Implement Databricks OIDC as Token Source #1204
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?
Conversation
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.
Looking great, few suggestions but otherwise very happy with this!
e2ff441
to
3e0b95b
Compare
3e0b95b
to
b09f4e8
Compare
config/oidc_github.go
Outdated
@@ -0,0 +1,42 @@ | |||
package config |
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.
Is this file still used?
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 was used by azure. Remove and replaced by the new Id TokenSource
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
// This is only used for Account level tokens. | ||
AccountID string | ||
// Host is the host of the Databricks account or workspace. | ||
Host string |
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 seems like this field is never used. Is there a reason why we have it as a field? If not, would it be possible to remove it?
DO NOTE MERGE
Waiting for server side update
What changes are proposed in this pull request?
Implement Databricks OIDC as Token Source
How is this tested?
Added unit and integration tests