Skip to content
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

Fix Azure SP Personal Access Token Duration #110

Merged
merged 8 commits into from
Jun 25, 2020
1 change: 1 addition & 0 deletions client/service/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type DBApiClientConfig struct {
// new token should be requested from
// the workspace before this time comes
// not yet used in the client but can be set
TokenCreateTime int64
TokenExpiryTime int64
AuthType AuthType
UserAgent string
Expand Down
5 changes: 3 additions & 2 deletions client/service/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"time"

"github.com/databrickslabs/databricks-terraform/client/model"
)
Expand All @@ -14,11 +15,11 @@ type TokensAPI struct {
}

// Create creates a api token given a expiration duration and a comment
func (a TokensAPI) Create(lifeTimeSeconds int32, comment string) (model.TokenResponse, error) {
func (a TokensAPI) Create(tokenLifetime time.Duration, comment string) (model.TokenResponse, error) {
var tokenData model.TokenResponse

tokenCreateRequest := model.TokenRequest{
LifetimeSeconds: lifeTimeSeconds,
LifetimeSeconds: int32(tokenLifetime.Seconds()),
Comment: comment,
}

Expand Down
3 changes: 2 additions & 1 deletion client/service/tokens_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package service

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand All @@ -13,7 +14,7 @@ func TestCreateToken(t *testing.T) {

client := GetIntegrationDBAPIClient()

lifeTimeSeconds := int32(30)
lifeTimeSeconds := time.Duration(30) * time.Second
comment := "Hello world"

token, err := client.Tokens().Create(lifeTimeSeconds, comment)
Expand Down
3 changes: 2 additions & 1 deletion client/service/tokens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package service
import (
"net/http"
"testing"
"time"

"github.com/databrickslabs/databricks-terraform/client/model"
)
Expand Down Expand Up @@ -63,7 +64,7 @@ func TestTokensAPI_Create(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
var input args
AssertRequestWithMockServer(t, &tt.args, http.MethodPost, "/api/2.0/token/create", &input, tt.response, tt.responseStatus, tt.want, tt.wantErr, func(client DBApiClient) (interface{}, error) {
return client.Tokens().Create(tt.args.LifetimeSeconds, tt.args.Comment)
return client.Tokens().Create(time.Duration(tt.args.LifetimeSeconds)*time.Second, tt.args.Comment)
})
})
}
Expand Down
10 changes: 7 additions & 3 deletions databricks/azure_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type TokenPayload struct {
ClientSecret string
ClientID string
TenantID string
PatTokenSeconds int32
}

type azureDatabricksWorkspace struct {
Expand Down Expand Up @@ -157,7 +158,7 @@ func (t *TokenPayload) getADBPlatformToken() (string, error) {
return platformToken.OAuthToken(), nil
}

func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceURL, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) {
func (t *TokenPayload) getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken, adbWorkspaceURL, adbWorkspaceResourceID, adbPlatformToken string) (*model.TokenResponse, error) {
log.Println("[DEBUG] Creating workspace token")
url := adbWorkspaceURL + "/api/2.0/token/create"
headers := map[string]string{
Expand All @@ -168,10 +169,11 @@ func getWorkspaceAccessToken(config *service.DBApiClientConfig, managementToken,
"Authorization": "Bearer " + adbPlatformToken,
}

tokenLifetimeSeconds := (time.Duration(t.PatTokenSeconds) * time.Second).Seconds()
var tokenResponse model.TokenResponse
resp, err := service.PerformQuery(config, http.MethodPost, url, "2.0",
headers, true, true, model.TokenRequest{
LifetimeSeconds: int32(3600),
LifetimeSeconds: int32(tokenLifetimeSeconds),
Comment: "Secret made via SP",
}, nil)
if err != nil {
Expand Down Expand Up @@ -210,14 +212,16 @@ func (t *TokenPayload) initWorkspaceAndGetClient(config *service.DBApiClientConf
}

// Get workspace personal access token
workspaceAccessTokenResp, err := getWorkspaceAccessToken(config, managementToken, adbWorkspaceURL, adbWorkspace.ID, adbPlatformToken)
workspaceAccessTokenResp, err := t.getWorkspaceAccessToken(config, managementToken, adbWorkspaceURL, adbWorkspace.ID, adbPlatformToken)
if err != nil {
return err
}

// Getting and creating this token happens in a mtx lock so this assignment should be safe
config.Host = adbWorkspaceURL
config.Token = workspaceAccessTokenResp.TokenValue
if workspaceAccessTokenResp.TokenInfo != nil {
config.TokenCreateTime = workspaceAccessTokenResp.TokenInfo.CreationTime
config.TokenExpiryTime = workspaceAccessTokenResp.TokenInfo.ExpiryTime
}

Expand Down
31 changes: 30 additions & 1 deletion databricks/azure_auth_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package databricks

import (
"os"
"testing"
"time"

"github.com/databrickslabs/databricks-terraform/client/model"
"github.com/databrickslabs/databricks-terraform/client/service"
Expand All @@ -14,11 +16,37 @@ func GetIntegrationDBClientOptions() *service.DBApiClientConfig {
return &config
}

func TestAccAzureAuth_TestPatTokenDuration(t *testing.T) {
if _, ok := os.LookupEnv("TF_ACC"); !ok {
t.Skip("Acceptance tests skipped unless env 'TF_ACC' set")
}

patTokenSeconds := (time.Duration(1) * time.Hour).Seconds()
tokenPayload := TokenPayload{
ManagedResourceGroup: getAndAssertEnv(t, "DATABRICKS_AZURE_MANAGED_RESOURCE_GROUP"),
AzureRegion: getAndAssertEnv(t, "AZURE_REGION"),
WorkspaceName: getAndAssertEnv(t, "DATABRICKS_AZURE_WORKSPACE_NAME"),
ResourceGroup: getAndAssertEnv(t, "DATABRICKS_AZURE_RESOURCE_GROUP"),
SubscriptionID: getAndAssertEnv(t, "DATABRICKS_AZURE_SUBSCRIPTION_ID"),
TenantID: getAndAssertEnv(t, "DATABRICKS_AZURE_TENANT_ID"),
ClientID: getAndAssertEnv(t, "DATABRICKS_AZURE_CLIENT_ID"),
ClientSecret: getAndAssertEnv(t, "DATABRICKS_AZURE_CLIENT_SECRET"),
PatTokenSeconds: int32(patTokenSeconds),
}
config := GetIntegrationDBClientOptions()
err := tokenPayload.initWorkspaceAndGetClient(config)
assert.NoError(t, err, err)
// Time in milliseconds
tokenActualDuration := config.TokenExpiryTime - config.TokenCreateTime
assert.Equal(t, patTokenSeconds, (time.Duration(tokenActualDuration) * time.Millisecond).Seconds(),
"duration should be the same")
}

func TestAzureAuthCreateApiToken(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode.")
}

patTokenSeconds := (time.Duration(1) * time.Hour).Seconds()
tokenPayload := TokenPayload{
ManagedResourceGroup: getAndAssertEnv(t, "DATABRICKS_AZURE_MANAGED_RESOURCE_GROUP"),
AzureRegion: getAndAssertEnv(t, "AZURE_REGION"),
Expand All @@ -28,6 +56,7 @@ func TestAzureAuthCreateApiToken(t *testing.T) {
TenantID: getAndAssertEnv(t, "DATABRICKS_AZURE_TENANT_ID"),
ClientID: getAndAssertEnv(t, "DATABRICKS_AZURE_CLIENT_ID"),
ClientSecret: getAndAssertEnv(t, "DATABRICKS_AZURE_CLIENT_SECRET"),
PatTokenSeconds: int32(patTokenSeconds),
}

config := GetIntegrationDBClientOptions()
Expand Down
22 changes: 22 additions & 0 deletions databricks/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"log"
"os"
"strconv"
"time"

"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
Expand Down Expand Up @@ -150,6 +152,12 @@ func Provider(version string) terraform.ResourceProvider {
Required: true,
DefaultFunc: schema.MultiEnvDefaultFunc([]string{"DATABRICKS_AZURE_TENANT_ID", "ARM_TENANT_ID"}, nil),
},
"pat_token_duration_seconds": {
Type: schema.TypeString,
Required: true,
Description: "Currently secret scopes are not accessible via AAD tokens so we will need to create a PAT token",
Default: durationToSecondsString(time.Hour),
},
},
},
},
Expand All @@ -169,6 +177,10 @@ func Provider(version string) terraform.ResourceProvider {
return provider
}

func durationToSecondsString(duration time.Duration) string {
return strconv.Itoa(int(duration.Seconds()))
}

func providerConfigureAzureClient(d *schema.ResourceData, config *service.DBApiClientConfig) (interface{}, error) {
log.Println("Creating db client via azure auth!")
azureAuth, _ := d.GetOk("azure_auth")
Expand Down Expand Up @@ -249,6 +261,16 @@ func providerConfigureAzureClient(d *schema.ResourceData, config *service.DBApiC
tokenPayload.TenantID = os.Getenv("ARM_TENANT_ID")
}

// no need to ok this value has a default
patTokenDurationSeconds, ok := azureAuthMap["pat_token_duration_seconds"].(string)
if ok {
patTokenDuration, err := strconv.Atoi(patTokenDurationSeconds)
if err != nil {
return nil, fmt.Errorf("failed to parse pat_token_duration_seconds, %w", err)
}
tokenPayload.PatTokenSeconds = int32(patTokenDuration)
}

// Setup the CustomAuthorizer Function to be called at API invoke rather than client invoke
config.CustomAuthorizer = tokenPayload.initWorkspaceAndGetClient
var dbClient service.DBApiClient
Expand Down
5 changes: 5 additions & 0 deletions databricks/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"log"
"os"
"testing"
"time"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"

Expand Down Expand Up @@ -198,6 +199,10 @@ func TestProvider_InvalidConfigFilePath(t *testing.T) {
assert.NotNil(t, err)
}

func TestProvider_DurationToSecondsString(t *testing.T) {
assert.Equal(t, durationToSecondsString(time.Hour), "3600")
}

func TestAccDatabricksCliConfigWorks(t *testing.T) {
resource.Test(t,
resource.TestCase{
Expand Down
4 changes: 3 additions & 1 deletion databricks/resource_databricks_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package databricks

import (
"log"
"time"

"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
Expand Down Expand Up @@ -49,7 +50,8 @@ func resourceTokenCreate(d *schema.ResourceData, m interface{}) error {
client := m.(*service.DBApiClient)
lifeTimeSeconds := d.Get("lifetime_seconds").(int)
comment := d.Get("comment").(string)
tokenResp, err := client.Tokens().Create(int32(lifeTimeSeconds), comment)
tokenDuration := time.Duration(lifeTimeSeconds) * time.Second
tokenResp, err := client.Tokens().Create(tokenDuration, comment)
if err != nil {
return err
}
Expand Down
12 changes: 11 additions & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ resource "databricks_cluster" "shared_autoscaling" {
!> **Warning** Please be aware that hard coding any credentials is not something that is recommended. It may be best if
you store the credentials environment variables, `~/.databrickscfg` file or use tfvars file.

!> **Warning** Please note that the azure service principal authentication currently uses a generated Databricks PAT token
and not a AAD token for the authentication. This is due to the Databricks AAD feature not yet supporting AAD tokens for
secret scopes. This will be refactored in a transparent manner when that support is enabled. The only field to be impacted
is `pat_token_duration_seconds` which will be deprecated and after AAD support is fully supported.

There are currently three supported methods [to authenticate into](https://docs.databricks.com/dev-tools/api/latest/authentication.html) the Databricks platform to create resources:

* [PAT Tokens](https://docs.databricks.com/dev-tools/api/latest/authentication.html)
Expand Down Expand Up @@ -122,7 +127,7 @@ resource "azurerm_databricks_workspace" "demo_test_workspace" {
}

provider "databricks" {
azure_auth {
azure_auth = {
managed_resource_group = azurerm_databricks_workspace.demo_test_workspace.managed_resource_group_name
azure_region = azurerm_databricks_workspace.demo_test_workspace.location
workspace_name = azurerm_databricks_workspace.demo_test_workspace.name
Expand Down Expand Up @@ -233,6 +238,11 @@ environment variable `DATABRICKS_AZURE_CLIENT_ID` or `ARM_CLIENT_ID`.
* `tenant_id` - (required) This is the Azure Active Directory Tenant id in which the Enterprise Application (Service Principal)
resides in. Alternatively you can provide this value as an environment variable `DATABRICKS_AZURE_TENANT_ID` or `ARM_TENANT_ID`.

* `pat_token_duration_seconds` - The current implementation of the azure auth via sp requires the provider to create a temporary
personal access token within Databricks. The current AAD implementation does not cover all the APIs for Authentication. This
field determines the duration in which that temporary PAT token is alive for. It is measured in seconds and will default to
`3600` seconds.

Where there are multiple environment variable options, the `DATABRICKS_AZURE_*` environment variables takes precedence
and the `ARM_*` environment variables provide a way to share authentication configuration when using the `databricks-terraform`
provider alongside the `azurerm` provider.
Expand Down
26 changes: 17 additions & 9 deletions website/content/Provider/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ resource "azurerm_databricks_workspace" "demo_test_workspace" {

provider "databricks" {
azure_auth = {
managed_resource_group = azurerm_databricks_workspace.demo_test_workspace.managed_resource_group_name
azure_region = azurerm_databricks_workspace.demo_test_workspace.location
workspace_name = azurerm_databricks_workspace.demo_test_workspace.name
resource_group = azurerm_databricks_workspace.demo_test_workspace.resource_group_name
client_id = var.client_id
client_secret = var.client_secret
tenant_id = var.tenant_id
subscription_id = var.subscription_id
managed_resource_group = azurerm_databricks_workspace.demo_test_workspace.managed_resource_group_name
azure_region = azurerm_databricks_workspace.demo_test_workspace.location
workspace_name = azurerm_databricks_workspace.demo_test_workspace.name
resource_group = azurerm_databricks_workspace.demo_test_workspace.resource_group_name
client_id = var.client_id
client_secret = var.client_secret
tenant_id = var.tenant_id
subscription_id = var.subscription_id
pat_token_duration_seconds = 3600
}
}

Expand Down Expand Up @@ -141,7 +142,8 @@ provider "databricks" {
client_id = "${var.client_id}"
client_secret = "${var.client_secret}"
tenant_id = "${var.tenant_id}"
subscription_id = "${var.subscription_id}"
subscription_id = var.subscription_id
pat_token_duration_seconds = 3600
}
}
```
Expand Down Expand Up @@ -265,5 +267,11 @@ environment variable `DATABRICKS_AZURE_CLIENT_ID` or `ARM_CLIENT_ID`.
* `tenant_id` - This is the Azure Active Directory Tenant id in which the Enterprise Application (Service Principal)
resides in. Alternatively you can provide this value as an environment variable `DATABRICKS_AZURE_TENANT_ID` or `ARM_TENANT_ID`.

* `pat_token_duration_seconds` - The current implementation of the azure auth via sp requires the provider to create a temporary
personal access token within Databricks. The current AAD implementation does not cover all the APIs for Authentication. This
field determines the duration in which that temporary PAT token is alive for. It is measured in seconds and will default to
`3600` seconds.


Where there are multiple environment variable options, the `DATABRICKS_AZURE_*` environment variables takes precedence and the `ARM_*` environment variables provide a way to share authentication configuration when using the `databricks-terraform` provider alongside the `azurerm` provider.
{{% /chevron %}}