Add support for tokenless authentication for GitHub Actions configured with OpenID Connect with Azure User Managed Identity (or Service Principal) by nfx · Pull Request #385 · databricks/databricks-sdk-py

nfx · 2023-10-04T19:57:47Z

Changes

See https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers and https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-azure

Technically, it should also work with Azure DevOps Workload Identity Federation, once we figure out the environment variables: https://techcommunity.microsoft.com/t5/azure-devops-blog/introduction-to-azure-devops-workload-identity-federation-oidc/ba-p/3908687

Tests

setup:

resource "github_actions_environment_secret" "scope" {
  for_each    = github_repository_environment.default
  repository  = each.value.repository
  environment = each.value.environment
  secret_name = "ARM_CLIENT_ID"
  # this value is not a secret as well
  plaintext_value = data.azurerm_user_assigned_identity.scopes[local.project_scopes[each.key]].client_id
}

...

resource "azurerm_federated_identity_credential" "oidc" {
  for_each            = github_repository_environment.default
  name                = "${local.org}-${each.value.repository}-${each.value.environment}-oidc"
  resource_group_name = local.resource_group_name
  audience            = ["api://AzureADTokenExchange"]
  issuer              = "https://token.actions.githubusercontent.com"
  parent_id           = data.azurerm_user_assigned_identity.scopes[local.project_scopes[each.key]].id
  subject             = "repo:${local.org}/${each.value.repository}:environment:${each.value.environment}"
}

...

resource "azurerm_user_assigned_identity" "scopes" {
  for_each = local.scopes
  name     = "labs-${each.key}-identity"

  resource_group_name = azurerm_resource_group.this.name
  location            = azurerm_resource_group.this.location
  // ...
}

...

resource "databricks_service_principal" "scopes" {
  provider = databricks.account
  for_each = local.scopes

  application_id = azurerm_user_assigned_identity.scopes[each.key].client_id
  display_name   = azurerm_user_assigned_identity.scopes[each.key].name
  external_id    = azurerm_user_assigned_identity.scopes[each.key].principal_id
}

result

_Experiment__Call_integration_tests_via_OIDC_·_databrickslabs_ucx_5a94b24

make test run locally
make fmt applied
relevant integration tests applied

…d with OpenID Connect with Azure User Managed Identity (or Service Principal) See https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-cloud-providers and https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-azure

mgyucht

Seems very reasonable, thanks for contributing. Added a new row to our unified auth template for SDKs to support this. Over time, we may want to generalize to support e.g. M2M SP via OIDC if Auth Platform is interested in supporting it; I asked a question on their channel. That said, I think this is a reasonable way to get started and help users who want a secret-free experience.

Can you add a couple of unit tests, esp. checking the case where the OIDC token endpoint times out or doesn't respond with a 200/JSON?

databricks/sdk/core.py

codecov-commenter · 2023-10-05T16:20:56Z

Codecov Report

Attention: 1377 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
databricks/sdk/azure.py	`100.00% <100.00%> (ø)`
databricks/sdk/mixins/files.py	`74.52% <100.00%> (+0.26%)`	⬆️
databricks/sdk/retries.py	`100.00% <100.00%> (ø)`
databricks/sdk/service/catalog.py	`50.22% <ø> (-2.85%)`	⬇️
databricks/sdk/service/compute.py	`51.13% <ø> (-2.73%)`	⬇️
databricks/sdk/service/iam.py	`41.85% <ø> (-6.36%)`	⬇️
databricks/sdk/service/ml.py	`47.36% <ø> (-2.47%)`	⬇️
databricks/sdk/service/sql.py	`52.75% <ø> (-3.27%)`	⬇️
databricks/sdk/version.py	`100.00% <100.00%> (ø)`
databricks/sdk/__init__.py	`74.80% <90.90%> (+1.46%)`	⬆️
... and 13 more

... and 3 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

mgyucht

Implementation seems fine to me, but let's not include test code paths in our main code. I think you just need to specify an azure resource ID for is_azure() to return true.

nfx · 2023-10-13T17:56:10Z

@mgyucht let's see if arm resource id helps

* Retry on all 429 and 503, even when missing Retry-After header ([#402](#402)). * Add support for tokenless authentication for GitHub Actions configured with OpenID Connect with Azure User Managed Identity (or Service Principal) ([#385](#385)). * Reduce redundant warning messages in azure-cli credential provider ([#410](#410)). API Changes: * Added `attributes`, `count`, `excluded_attributes`, `filter`, `sort_by`, `sort_order`, and `start_index` fields for `databricks.sdk.service.iam.GetAccountUserRequest` and `databricks.sdk.service.iam.GetUserRequest`. * Added `schemas` field for `databricks.sdk.service.iam.Group`, `databricks.sdk.service.iam.ListGroupsResponse`, `databricks.sdk.service.iam.ListServicePrincipalResponse`, `databricks.sdk.service.iam.ListUsersResponse`, `databricks.sdk.service.iam.ServicePrincipal`, and `databricks.sdk.service.iam.User`. * Added `databricks.sdk.service.iam.GetSortOrder`, `databricks.sdk.service.iam.GroupSchema`, `databricks.sdk.service.iam.ListResponseSchema`, `databricks.sdk.service.iam.ServicePrincipalSchema`, and `databricks.sdk.service.iam.UserSchema` dataclasses. * Added `webhook_notifications` field for `databricks.sdk.service.jobs.SubmitTask`. * Added [w.apps](https://databricks-sdk-py.readthedocs.io/en/latest/workspace/apps.html) workspace-level service and related dataclasses * Added [a.account_network_policy](https://databricks-sdk-py.readthedocs.io/en/latest/account/account_network_policy.html) account-level service and related dataclasses. OpenAPI SHA: 5903bb39137fd76ac384b2044e425f9c56840e00, Date: 2023-10-23

nfx requested review from hectorcast-db, mgyucht and pietern October 4, 2023 19:57

nfx added the enhancement New feature or request label Oct 4, 2023

fmt

c195207

mgyucht reviewed Oct 5, 2023

View reviewed changes

databricks/sdk/core.py Outdated Show resolved Hide resolved

databricks/sdk/core.py Outdated Show resolved Hide resolved

now with test coverage

b15eacd

nfx requested a review from mgyucht October 5, 2023 16:19

and a bit more resilient

219e2bb

nfx enabled auto-merge October 5, 2023 16:23

nfx linked an issue Oct 6, 2023 that may be closed by this pull request

[FEATURE] Native authentication support for GitHub Actions using the OpenID Connect from Azure #383

Closed

mgyucht reviewed Oct 12, 2023

View reviewed changes

use azure_workspace_resource_id for is_azure

f0a79a1

mgyucht approved these changes Oct 18, 2023

View reviewed changes

nfx added this pull request to the merge queue Oct 18, 2023

Merged via the queue into main with commit d4c160f Oct 18, 2023

nfx deleted the auth/github branch October 18, 2023 11:37

mgyucht mentioned this pull request Oct 24, 2023

Release v0.12.0 #412

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for tokenless authentication for GitHub Actions configured with OpenID Connect with Azure User Managed Identity (or Service Principal)#385

Add support for tokenless authentication for GitHub Actions configured with OpenID Connect with Azure User Managed Identity (or Service Principal)#385
nfx merged 5 commits intomainfrom
auth/github

nfx commented Oct 4, 2023 •

edited

Loading

Uh oh!

mgyucht left a comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Oct 5, 2023 •

edited

Loading

Uh oh!

mgyucht left a comment

Uh oh!

nfx commented Oct 13, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nfx commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mgyucht left a comment

Choose a reason for hiding this comment

Uh oh!

nfx commented Oct 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nfx commented Oct 4, 2023 •

edited

Loading

codecov-commenter commented Oct 5, 2023 •

edited

Loading

nfx commented Oct 13, 2023 •

edited

Loading