Added Config.GetAuthDetails #838

andrewnester · 2024-02-29T13:01:11Z

Changes

Added Config.GetAuthDetails and use it in config debug string

Needed for databricks/cli#1244

Tests

make test passing
make fmt applied
relevant integration tests applied

codecov-commenter · 2024-02-29T13:03:51Z

Codecov Report

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

Project coverage is 8.03%. Comparing base (28be042) to head (8657931).
Report is 5 commits behind head on main.

Files	Patch %	Lines
config/config_auth_details.go	87.67%	7 Missing and 2 partials ⚠️
config/config_attributes.go	80.95%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #838       +/-   ##
==========================================
- Coverage   18.33%   8.03%   -10.31%     
==========================================
  Files         118     257      +139     
  Lines       22166   63983    +41817     
==========================================
+ Hits         4064    5139     +1075     
- Misses      17851   58549    +40698     
- Partials      251     295       +44

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

pietern

I don't understand why the additional auth_group tag is needed. When I read the comment in the struct I kind of get it, but then I don't see where it has an effect in the rest of the PR. For example, in the Validate function the code falls back to using the AuthGroup field and the result is the same as before this PR. Am I missing anything?

config/config.go

config/config_attribute.go

config/config_attributes.go

config/auth_permutations_test.go

config/config_attribute.go

config/config_attributes.go

config/config_auth_details.go

config/config_attributes.go

config/config.go

config/config_auth_details.go

pietern · 2024-03-04T19:13:41Z

@mgyucht Could you take a look at this as well?

pietern

LGTM, thanks! Will defer to @mgyucht for the stamp.

pietern · 2024-03-05T11:11:51Z

config/config_attributes.go

 		if v == "" {
 			continue
 		}
 		err := attr.SetS(cfg, v)
+		cfg.SetAttrSource(&attr, Source{Type: SourceEnv, Name: envName})


Strictly speaking, this should only be set after the error check.

mgyucht

Small comment, but otherwise LGTM. Thanks for doing this! We'll need to update the error logging to take advantage of this change.

mgyucht · 2024-03-07T14:07:21Z

config/config_auth_details.go

+	}
+}
+
+func (c *Config) SetAttrSource(attr *ConfigAttribute, source Source) {


Does this need to be public?

@mgyucht yes, it's being used in CLI to set a different attribute source which does not exist in SDK (bundle to be specifc)

## Changes This command provide details on auth configuration user is using as well as authenticated user and auth mechanism used. Relies on databricks/databricks-sdk-go#838 (tests will fail until merged) Examples of output ``` Workspace: https://test.com User: [email protected] Authenticated with: pat ----- Configuration: ✓ auth_type: pat ✓ host: https://test.com (from bundle) ✓ profile: DEFAULT (from --profile flag) ✓ token: ******** (from /Users/andrew.nester/.databrickscfg config file) ``` ``` DATABRICKS_AUTH_TYPE=azure-msi databricks auth describe -p "Azure 2" Unable to authenticate: inner token: Post "https://foobar.com/oauth2/token": AADSTS900023: Specified tenant identifier foobar_aaaaaaa' is neither a valid DNS name, nor a valid external domain. See https://login.microsoftonline.com/error?code=900023 ----- Configuration: ✓ auth_type: azure-msi (from DATABRICKS_AUTH_TYPE environment variable) ✓ azure_client_id: 8470f3ba-aaaa-bbbb-cccc-xxxxyyyyzzzz (from /Users/andrew.nester/.databrickscfg config file) ~ azure_client_secret: ******** (from /Users/andrew.nester/.databrickscfg config file, not used for auth type azure-msi) ~ azure_tenant_id: foobar_aaaaaaa (from /Users/andrew.nester/.databrickscfg config file, not used for auth type azure-msi) ✓ azure_use_msi: true (from /Users/andrew.nester/.databrickscfg config file) ✓ host: https://foobar.com (from /Users/andrew.nester/.databrickscfg config file) ✓ profile: Azure 2 (from --profile flag) ``` For account ``` Unable to authenticate: default auth: databricks-cli: cannot get access token: Error: token refresh: Post "https://xxxxxxx.com/v1/token": http 400: {"error":"invalid_request","error_description":"Refresh token is invalid"} . Config: host=https://xxxxxxx.com, account_id=ed0ca3c5-fae5-4619-bb38-eebe04a4af4b, profile=ACCOUNT-ed0ca3c5-fae5-4619-bb38-eebe04a4af4b ----- Configuration: ✓ account_id: ed0ca3c5-fae5-4619-bb38-eebe04a4af4b (from /Users/andrew.nester/.databrickscfg config file) ✓ auth_type: databricks-cli (from /Users/andrew.nester/.databrickscfg config file) ✓ host: https://xxxxxxxxx.com (from /Users/andrew.nester/.databrickscfg config file) ✓ profile: ACCOUNT-ed0ca3c5-fae5-4619-bb38-eebe04a4af4b ``` ## Tests Added unit tests --------- Co-authored-by: Julia Crawford (Databricks) <[email protected]>

Added source tracking for config attributes

a89c992

andrewnester requested review from pietern and mgyucht February 29, 2024 13:01

andrewnester mentioned this pull request Feb 29, 2024

Added auth describe command databricks/cli#1244

Merged

pietern reviewed Mar 4, 2024

View reviewed changes

more auth details

7b9fe7e

andrewnester requested a review from pietern March 4, 2024 14:26

andrewnester changed the title ~~Added source tracking for config attributes~~ Added Config.GetAuthDetails and use it in config debug string Mar 4, 2024

fixed tests for go <1.20

fc3e6f5

pietern reviewed Mar 4, 2024

View reviewed changes

andrewnester changed the title ~~Added Config.GetAuthDetails and use it in config debug string~~ Added Config.GetAuthDetails Mar 4, 2024

fixes

eae0348

andrewnester requested a review from pietern March 4, 2024 16:45

pietern reviewed Mar 4, 2024

View reviewed changes

config/config_attributes.go Outdated Show resolved Hide resolved

config/config_attributes.go Outdated Show resolved Hide resolved

config/config.go Outdated Show resolved Hide resolved

config/config_auth_details.go Outdated Show resolved Hide resolved

fixes

8657931

andrewnester requested a review from pietern March 5, 2024 09:46

pietern reviewed Mar 5, 2024

View reviewed changes

mgyucht approved these changes Mar 7, 2024

View reviewed changes

andrewnester added this pull request to the merge queue Mar 8, 2024

Merged via the queue into main with commit bc9b34e Mar 8, 2024
5 checks passed

andrewnester deleted the improve-auth-details branch March 8, 2024 11:05

mgyucht mentioned this pull request Mar 19, 2024

Release v0.35.0 #860

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Config.GetAuthDetails #838

Added Config.GetAuthDetails #838

andrewnester commented Feb 29, 2024 •

edited

Loading

codecov-commenter commented Feb 29, 2024 •

edited

Loading

pietern left a comment

pietern commented Mar 4, 2024

pietern left a comment

pietern Mar 5, 2024

mgyucht left a comment

mgyucht Mar 7, 2024

andrewnester Mar 8, 2024

Added Config.GetAuthDetails #838

Added Config.GetAuthDetails #838

Conversation

andrewnester commented Feb 29, 2024 • edited Loading

Changes

Tests

codecov-commenter commented Feb 29, 2024 • edited Loading

Codecov Report

pietern left a comment

Choose a reason for hiding this comment

pietern commented Mar 4, 2024

pietern left a comment

Choose a reason for hiding this comment

pietern Mar 5, 2024

Choose a reason for hiding this comment

mgyucht left a comment

Choose a reason for hiding this comment

mgyucht Mar 7, 2024

Choose a reason for hiding this comment

andrewnester Mar 8, 2024

Choose a reason for hiding this comment

andrewnester commented Feb 29, 2024 •

edited

Loading

codecov-commenter commented Feb 29, 2024 •

edited

Loading