Create a method to generate OAuth tokens by hectorcast-db · Pull Request #886 · databricks/databricks-sdk-go

hectorcast-db · 2024-04-09T11:27:59Z

Changes

Create a method to generate OAuth tokens. This is required to later generate tokens to use in the DataPlane APIs. This PR introduces a breaking change by renaming CredentialsProvider -> CredentialsStrategy and changing the signature of the Configure method for such interface.

Tests

Manual test. No API returns the authorization_details for an object.
make test passing
make fmt applied
relevant integration tests applied

httpclient/oauth_token.go

codecov-commenter · 2024-04-09T11:58:19Z

Codecov Report

Attention: Patch coverage is 49.10714% with 57 lines in your changes are missing coverage. Please review.

Project coverage is 7.12%. Comparing base (27d08a6) to head (4214f2f).
Report is 2 commits behind head on main.

Files	Patch %	Lines
config/config.go	17.64%	11 Missing and 3 partials ⚠️
httpclient/oauth_token.go	0.00%	14 Missing ⚠️
httpclient/request.go	46.66%	7 Missing and 1 partial ⚠️
workspace_client.go	0.00%	5 Missing ⚠️
config/auth_gcp_google_id.go	20.00%	4 Missing ⚠️
config/api_client.go	0.00%	3 Missing ⚠️
examples/custom-auth/main.go	0.00%	3 Missing ⚠️
config/auth_azure_client_secret.go	33.33%	2 Missing ⚠️
config/auth_gcp_google_credentials.go	33.33%	2 Missing ⚠️
credentials/oauth_credentials_provider.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main    #886      +/-   ##
========================================
+ Coverage   7.08%   7.12%   +0.03%     
========================================
  Files        276     279       +3     
  Lines      64376   64446      +70     
========================================
+ Hits        4563    4590      +27     
- Misses     59502   59546      +44     
+ Partials     311     310       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

httpclient/oauth_token.go

mgyucht

Looking good, some initial comments.

workspace_client.go

httpclient/oauth_token.go

httpclient/request.go

httpclient/oauth_token.go

mgyucht · 2024-05-16T13:08:01Z

config/auth_m2m.go

 }

-func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (func(*http.Request) error, error) {
+func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {


minor suggestion: I would return the most specific type that you can. If someone uses this directly and wants to be able to call the Token method, they'll need to match on the type first. We already know that this is an OAuthCredentialsProvider.

Suggested change

func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {

func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials.OAuthCredentialsProvider, error) {

Then M2mCredentials does not fulfill the CredentialsStrategy interface. Not sure if there is a way around it.

config/auth_gcp_google_id.go

config/auth_azure_client_secret.go

mgyucht · 2024-05-16T13:10:45Z

config/config.go

+	if h, ok := c.credentialsProvider.(credentials.OAuthCredentialsProvider); ok {
+		return h.Token()
+	} else {
+		return nil, fmt.Errorf("OAuth Token can only be retrieved for OauthCredentialsProvider")


Slightly more useful error message for users

Suggested change

return nil, fmt.Errorf("OAuth Token can only be retrieved for OauthCredentialsProvider")

return nil, fmt.Errorf("OAuth Token not supported for current auth type %s", c.AuthType)

mgyucht · 2024-05-16T13:11:20Z

config/config.go

+
+// Authenticate returns an OAuth token for the current configuration.
+// It will return an error if the CredentialsStrategy does not support OAuth tokens.
+func (c *Config) GetToken() (*oauth2.Token, error) {


This is the method that will be used in the follow-up PR?

This is already being used in GetOAuthToken method in the Workspace Client
https://github.com/databricks/databricks-sdk-go/pull/886/files#diff-75f383f34048f57e9041c3a70e938f38345a52ca6473a7790c7069d801b9bacfR980
We will use that one in the follow-up PR

hectorcast-db added 11 commits April 4, 2024 14:06

Move create body method

f460176

New do option

316390f

WIP

0bc57e9

Merge remote-tracking branch 'origin/main' into urlencode-content

e9afac9

Fix test

6e5c5fb

Replace CredentialsProvider

6b34704

Replace usage

7c8c9ba

Test

c7e96e9

Cleanup

2d53aaa

Cleanup

d604fc0

More cleanup

dccbdcc

hectorcast-db added the do-not-merge label Apr 9, 2024

hectorcast-db requested a review from mgyucht April 9, 2024 11:28

hectorcast-db commented Apr 9, 2024

View reviewed changes

httpclient/oauth_token.go Outdated Show resolved Hide resolved

Move class

21591a6

hectorcast-db marked this pull request as draft April 9, 2024 11:32

hectorcast-db added 2 commits April 9, 2024 13:38

Oauth

fc09f08

Remove unused file

1ed4631

Remove unnecessary

56ff775

hectorcast-db commented Apr 11, 2024

View reviewed changes

httpclient/oauth_token.go Outdated Show resolved Hide resolved

hectorcast-db added 2 commits May 13, 2024 14:13

Merge branch 'main' into support-oauth-token

82c609e

Remove unused fields

b7c06af

hectorcast-db marked this pull request as ready for review May 13, 2024 12:24

Context

0df16da

hectorcast-db removed the do-not-merge label May 13, 2024

mgyucht reviewed May 16, 2024

View reviewed changes

Comments

4214f2f

mgyucht approved these changes May 17, 2024

View reviewed changes

mgyucht added this pull request to the merge queue May 17, 2024

Merged via the queue into main with commit 6106c39 May 17, 2024

mgyucht deleted the support-oauth-token branch May 17, 2024 13:34

hectorcast-db mentioned this pull request May 21, 2024

Release v0.41.0 #927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a method to generate OAuth tokens#886

Create a method to generate OAuth tokens#886
mgyucht merged 19 commits intomainfrom
support-oauth-token

hectorcast-db commented Apr 9, 2024 •

edited

Loading

Uh oh!

Uh oh!

codecov-commenter commented Apr 9, 2024 •

edited

Loading

Uh oh!

Uh oh!

mgyucht left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgyucht May 16, 2024

Uh oh!

hectorcast-db May 16, 2024

Uh oh!

Uh oh!

Uh oh!

mgyucht May 16, 2024

Uh oh!

mgyucht May 16, 2024

Uh oh!

hectorcast-db May 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials.CredentialsProvider, error) {
	func (c M2mCredentials) Configure(ctx context.Context, cfg *Config) (credentials.OAuthCredentialsProvider, error) {

	return nil, fmt.Errorf("OAuth Token can only be retrieved for OauthCredentialsProvider")
	return nil, fmt.Errorf("OAuth Token not supported for current auth type %s", c.AuthType)

Conversation

hectorcast-db commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

Uh oh!

codecov-commenter commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgyucht May 16, 2024

Choose a reason for hiding this comment

Uh oh!

hectorcast-db May 16, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mgyucht May 16, 2024

Choose a reason for hiding this comment

Uh oh!

mgyucht May 16, 2024

Choose a reason for hiding this comment

Uh oh!

hectorcast-db May 16, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hectorcast-db commented Apr 9, 2024 •

edited

Loading

codecov-commenter commented Apr 9, 2024 •

edited

Loading