Implement databricks creds manager #2123

m-abulazm · 2025-10-28T13:13:00Z

Changes

What does this PR do?

Support databricks credentials
Standardize usages of credentials manager

Linked issues

Progresses #1008

Functionality

added relevant user documentation
modified existing command: databricks labs lakebridge ...

Tests

manually tested
added unit tests
added integration tests

codecov · 2025-10-28T13:14:52Z

Codecov Report

❌ Patch coverage is 69.64286% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.37%. Comparing base (eebf284) to head (fef02c5).

Files with missing lines	Patch %	Lines
.../labs/lakebridge/connections/credential_manager.py	85.71%	3 Missing and 1 partial ⚠️
...abs/lakebridge/assessments/configure_assessment.py	33.33%	2 Missing ⚠️
src/databricks/labs/lakebridge/config.py	86.66%	1 Missing and 1 partial ⚠️
...s/assessments/synapse/dedicated_sqlpool_extract.py	0.00%	2 Missing ⚠️
.../assessments/synapse/monitoring_metrics_extract.py	0.00%	2 Missing ⚠️
.../assessments/synapse/serverless_sqlpool_extract.py	0.00%	2 Missing ⚠️
...resources/assessments/synapse/workspace_extract.py	0.00%	2 Missing ⚠️
...databricks/labs/lakebridge/assessments/profiler.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2123      +/-   ##
==========================================
+ Coverage   65.24%   65.37%   +0.12%     
==========================================
  Files         100      100              
  Lines        8506     8535      +29     
  Branches      876      878       +2     
==========================================
+ Hits         5550     5580      +30     
+ Misses       2769     2767       -2     
- Partials      187      188       +1

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-10-28T13:19:03Z

✅ 51/51 passed, 11 flaky, 3m53s total

Flaky tests:

🤪 test_validate_invalid_source_tech (183ms)
🤪 test_validate_table_not_found (1ms)
🤪 test_validate_non_empty_tables (7ms)
🤪 test_validate_mixed_checks (293ms)
🤪 test_validate_invalid_schema_path (1ms)
🤪 test_transpiles_informatica_to_sparksql_non_interactive[False] (20.681s)
🤪 test_transpiles_informatica_to_sparksql (22.239s)
🤪 test_transpile_teradata_sql_non_interactive[False] (22.59s)
🤪 test_transpile_teradata_sql_non_interactive[True] (25.232s)
🤪 test_transpiles_informatica_to_sparksql_non_interactive[True] (3.838s)
🤪 test_transpile_teradata_sql (6.324s)

_{Running from acceptance #3107}

# Conflicts: # src/databricks/labs/lakebridge/reconcile/connectors/jdbc_reader.py # src/databricks/labs/lakebridge/reconcile/connectors/oracle.py

…e plus major cleanup

… in one test for it to be green

asnare

I've highlighted some style and design issues that I think need to be resolved, but appreciate that this is the start of what is needed for #1008. On the testing side I really like that we've eliminated some monkey-patching during tests. (Some integration tests would be nice.)

One big concern I have is that I don't see where we're using the new provider because we don't pass the WorkspaceClient in anywhere that I can see. Can you elaborate a bit on the situation there?

src/databricks/labs/lakebridge/connections/credential_manager.py

asnare · 2025-11-28T13:08:19Z

src/databricks/labs/lakebridge/connections/credential_manager.py

+            raise UnicodeDecodeError(
+                "utf-8",
+                key_only.encode(),
+                0,
+                1,
+                f"Secret {key} has Base64 bytes that cannot be decoded to utf-8 string: {e}.",


This should probably be ValueError (from e): we're signalling that there's a problem with a user-supplied argument (due to an underlying unicode issue).

I dont think this is on the user. databricks returned a malformed response and not the utf8 base64 value it should return

src/databricks/labs/lakebridge/connections/credential_manager.py

asnare · 2025-11-28T17:13:21Z

src/databricks/labs/lakebridge/config.py

+@dataclass
+class ReconcileCredentialConfig:
+    vault_type: str  # supports local, env, databricks creds.
+    source_creds: dict[str, str]


I think we need a better name for this: it doesn't hold the credentials… it's more of a vault configuration?

agree but renaming this will require changing across three PRs. I would rename it after reviewing all PRs

asnare · 2025-12-01T09:21:18Z

src/databricks/labs/lakebridge/connections/credential_manager.py

-        'local': LocalSecretProvider(),
-        'env': EnvSecretProvider(env_getter),
-        'databricks': DatabricksSecretProvider(),
+def create_credential_manager(creds_or_path: dict | Path, ws: WorkspaceClient | None = None) -> CredentialManager:


Although I can see some tests, I can't see where the ws argument is provided in any of the non-test call-sites. Aside from the tests, is it actually being used?

using credential manager in reconcile and supplying ws is done in #2159.

asnare · 2025-12-01T09:24:13Z

src/databricks/labs/lakebridge/reconcile/trigger_recon_service.py

            spark=spark,
            ws=ws_client,
-            secret_scope=reconcile_config.secret_scope,
+            secret_scope=reconcile_config.creds.source_creds["__secret_scope"],


Should this now be using the CredentialManager mechanism?

yes and it is implemented in #2159 to make the reviews more manageable.
And #2157 adds the prompts to configure the creds.

this current PR can go to main first since it is backwards-compatible without the other two.

src/databricks/labs/lakebridge/connections/credential_manager.py

asnare · 2025-12-01T09:33:46Z

src/databricks/labs/lakebridge/connections/credential_manager.py

+        except NotFound as e:
+            raise KeyError(f'Secret does not exist with scope: {scope} and key: {key_only} : {e}') from e


I think this is different to the other providers: they just return the key if the secret cannot be found, whereas here we raise an exception instead.

What do you think the providers should do? I think they need to be consistent.

we should not raise an error. the return type should be optional and it is up to the caller how to handle missing secrets.

I did not want to change lots of things in one go so the
the implementation you see here of DatabricksSecretProvider is copied from src/databricks/labs/lakebridge/reconcile/connectors/secrets.py without changing the way it works which led to some inconsistency.

I would address your comment here in a later PR if you dont mind

Co-authored-by: Andrew Snare <asnare@users.noreply.github.com>

Implement databricks creds manager

e145ef3

m-abulazm temporarily deployed to tool October 28, 2025 13:13 — with GitHub Actions Inactive

m-abulazm added 3 commits October 29, 2025 14:05

[WIP] use DatabricksSecretProvider in reconcile datasources

6c75f6c

Merge branch 'main' into refactor/creds-manager

8b7fb0b

# Conflicts: # src/databricks/labs/lakebridge/reconcile/connectors/jdbc_reader.py # src/databricks/labs/lakebridge/reconcile/connectors/oracle.py

update oracle data source after merge

3f81eb4

m-abulazm temporarily deployed to tool October 29, 2025 13:10 — with GitHub Actions Inactive

m-abulazm self-assigned this Oct 29, 2025

m-abulazm added the do-not-merge label Oct 29, 2025

Merge branch 'main' into refactor/creds-manager

4f037a5

m-abulazm temporarily deployed to tool November 7, 2025 12:00 — with GitHub Actions Inactive

add load_credentials interface for data sources and impl for snowflak…

374b63f

…e plus major cleanup

m-abulazm had a problem deploying to tool November 10, 2025 12:00 — with GitHub Actions Error

Merge branch 'main' into refactor/creds-manager

9a1286b

m-abulazm temporarily deployed to tool November 10, 2025 12:01 — with GitHub Actions Inactive

implement for tsql and oracle

5102b3b

m-abulazm temporarily deployed to tool November 10, 2025 13:31 — with GitHub Actions Inactive

improve snowflake load credentials

e07e443

m-abulazm temporarily deployed to tool November 10, 2025 14:17 — with GitHub Actions Inactive

m-abulazm marked this pull request as ready for review November 10, 2025 14:31

m-abulazm requested a review from a team as a code owner November 10, 2025 14:31

add check if user loaded credentials in snowflake and use local creds…

fb6692d

… in one test for it to be green

m-abulazm temporarily deployed to tool November 10, 2025 14:58 — with GitHub Actions Inactive

m-abulazm mentioned this pull request Nov 21, 2025

prompt for recon secrets #2157

Open

5 tasks

revert changes to reconcile before moving to a new branch

a16455d

m-abulazm had a problem deploying to tool November 21, 2025 08:52 — with GitHub Actions Error

Merge branch 'main' into refactor/creds-manager

a62b6e0

m-abulazm temporarily deployed to tool November 21, 2025 08:57 — with GitHub Actions Inactive

revert two more changes

79b0721

m-abulazm temporarily deployed to tool November 25, 2025 13:45 — with GitHub Actions Inactive

Merge branch 'main' into refactor/creds-manager

7264dda

gueniai temporarily deployed to tool November 26, 2025 22:15 — with GitHub Actions Inactive

Merge branch 'main' into refactor/creds-manager

6ceb7aa

gueniai temporarily deployed to tool November 26, 2025 22:49 — with GitHub Actions Inactive

Merge branch 'main' into refactor/creds-manager

75611fb

m-abulazm temporarily deployed to tool November 27, 2025 09:00 — with GitHub Actions Inactive

change hardcoded vault_type value (it is only used in a log line so far)

76b044e

m-abulazm temporarily deployed to tool November 28, 2025 14:54 — with GitHub Actions Inactive

asnare reviewed Dec 1, 2025

View reviewed changes

m-abulazm mentioned this pull request Dec 2, 2025

remove redundant reconcile config params #2150

Open

7 tasks

Merge branch 'main' into refactor/creds-manager

c0b1080

m-abulazm temporarily deployed to tool December 3, 2025 11:49 — with GitHub Actions Inactive

Apply suggestions from code review

87ff1a6

Co-authored-by: Andrew Snare <asnare@users.noreply.github.com>

m-abulazm temporarily deployed to tool December 3, 2025 11:56 — with GitHub Actions Inactive

fmt after applying suggestions

6d8230c

m-abulazm had a problem deploying to tool December 3, 2025 12:07 — with GitHub Actions Error

fix test

e31a720

m-abulazm temporarily deployed to tool December 3, 2025 12:10 — with GitHub Actions Inactive

do not lazy init and remove unused method

3ef81cc

m-abulazm temporarily deployed to tool December 3, 2025 12:42 — with GitHub Actions Inactive

add validation for vault_type

178fe0f

m-abulazm temporarily deployed to tool December 3, 2025 14:35 — with GitHub Actions Inactive

type test_credential_manager.py

0f3e3ff

m-abulazm temporarily deployed to tool December 3, 2025 16:10 — with GitHub Actions Inactive

m-abulazm requested a review from asnare December 3, 2025 16:44

add tests

2f66882

m-abulazm temporarily deployed to tool December 4, 2025 13:32 — with GitHub Actions Inactive

remove one unnecessary indirection

fef02c5

m-abulazm temporarily deployed to tool December 4, 2025 14:53 — with GitHub Actions Inactive

		except NotFound as e:
		raise KeyError(f'Secret does not exist with scope: {scope} and key: {key_only} : {e}') from e

Implement databricks creds manager #2123

Are you sure you want to change the base?

Implement databricks creds manager #2123

Uh oh!

Conversation

m-abulazm commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

What does this PR do?

Linked issues

Functionality

Tests

Uh oh!

codecov bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asnare left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

m-abulazm commented Oct 28, 2025 •

edited

Loading

codecov bot commented Oct 28, 2025 •

edited

Loading

github-actions bot commented Oct 28, 2025 •

edited

Loading