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

Use new workspace_url format + fix azure test #114

Merged
merged 23 commits into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ RUN \
# --> Delve for debugging
go get github.com/go-delve/delve/cmd/dlv@v1.3.2 \
# --> Go language server
&& go get golang.org/x/tools/gopls@v0.2.1 \
&& go get golang.org/x/tools/gopls@v0.4.1 \
# --> Goimports
&& go get golang.org/x/tools/cmd/goimports \
# --> Gotestsum
&& go get gotest.tools/gotestsum@v0.4.2 \
# --> Go symbols and outline for go to symbol support and test support
Expand Down
21 changes: 12 additions & 9 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
{
"name": "databricks-terraform",
"dockerFile": "Dockerfile",
"mounts": [
// Mount go mod cache
"source=databricks-terraform-gomodcache,target=/go/pkg,type=volume",
// Keep command history
"source=databricks-terraform-bashhistory,target=/commandhistory,type=volume",
// Mount docker socket for docker builds
"source=/var/run/docker.sock,target=/var/run/docker.sock,type=bind",
],
"runArgs": [
// Uncomment the next line to use a non-root user. On Linux, this will prevent
// new files getting created as root, but you may need to update the USER_UID
Expand All @@ -12,14 +20,8 @@
"--security-opt",
"seccomp=unconfined",
"--privileged",

"--name", "databricks-terraform-devcontainer",
// Mount go mod cache
"-v", "databricks-terraform-gomodcache:/go/pkg",
// Keep command history
"-v", "databricks-terraform-bashhistory:/root/commandhistory",
// Mount docker socket for docker builds
"-v", "/var/run/docker.sock:/var/run/docker.sock",
"--name",
"databricks-terraform-devcontainer",
// Use host network
"--network=host",
],
Expand Down Expand Up @@ -51,8 +53,9 @@
// "postCreateCommand": "go version",
// Add the IDs of extensions you want installed when the container is created in the array below.
"extensions": [
"ms-vscode.go",
"golang.go",
"mauve.terraform",
"hashicorp.terraform",
"ms-vsliveshare.vsliveshare"
]
}
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ vendor:
@echo "==> Filling vendor folder with library code..."
@go mod vendor

local-install: build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not be fully idempotent, when there are other db provider files there already, possibly with different names. I'd suggest to clear all files starting with terraform-provider-databricks and then do the move.

i'd suggest to rename target to simply install, following make install mantra :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr has been merged. I can make these changes in another PR

Copy link
Contributor

@stikkireddy stikkireddy Jun 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this command in another pr #129 considering that the version is hardcoded there for local install. If we want to make an install target for the makefile we can work on that in another PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other providers have a script to create a binary from the local files as well. It's very useful for testing and trying out your changes. Why is the hard-coded version number a problem enough that you'd remove the addition? How would you like to set a version number instead?

mv terraform-provider-databricks $(HOME)/.terraform.d/plugins/terraform-provider-databricks_v0.2.0

# INTEGRATION TESTING WITH AZURE
terraform-acc-azure: lint
@echo "==> Running Terraform Acceptance Tests for Azure..."
@CLOUD_ENV="azure" TF_ACC=1 gotestsum --format short-verbose --raw-command go test -v -json -tags=azure -short -coverprofile=coverage.out ./...
@/bin/bash integration-environment-azure/run.sh

# INTEGRATION TESTING WITH AWS
terraform-acc-aws: lint
Expand Down
50 changes: 8 additions & 42 deletions databricks/azure_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"log"
"net/http"
urlParse "net/url"

"github.com/Azure/go-autorest/autorest/adal"
"github.com/Azure/go-autorest/autorest/azure"
Expand All @@ -24,6 +23,7 @@ type AzureAuth struct {
AdbWorkspaceResourceID string
AdbAccessToken string
AdbPlatformToken string
AdbWorkspaceUrl string
}

// TokenPayload contains all the auth information for azure sp authentication
Expand Down Expand Up @@ -74,38 +74,6 @@ func (a *AzureAuth) getManagementToken() error {
return nil
}

func (a *AzureAuth) getWorkspaceID(config *service.DBApiClientConfig) error {
log.Println("[DEBUG] Getting Workspace ID via management token.")
// Escape all the ids
url := fmt.Sprintf("https://management.azure.com/subscriptions/%s/resourceGroups/%s"+
"/providers/Microsoft.Databricks/workspaces/%s",
urlParse.PathEscape(a.TokenPayload.SubscriptionID),
urlParse.PathEscape(a.TokenPayload.ResourceGroup),
urlParse.PathEscape(a.TokenPayload.WorkspaceName))
headers := map[string]string{
"Content-Type": "application/json",
"cache-control": "no-cache",
"Authorization": "Bearer " + a.ManagementToken,
}
type apiVersion struct {
APIVersion string `url:"api-version"`
}
uriPayload := apiVersion{
APIVersion: "2018-04-01",
}
var responseMap map[string]interface{}
resp, err := service.PerformQuery(config, http.MethodGet, url, "2.0", headers, false, true, uriPayload, nil)
if err != nil {
return err
}
err = json.Unmarshal(resp, &responseMap)
if err != nil {
return err
}
a.AdbWorkspaceResourceID = responseMap["id"].(string)
return err
}

func (a *AzureAuth) getADBPlatformToken() error {
log.Println("[DEBUG] Creating Azure Databricks management OAuth token.")
platformTokenOAuthCfg, err := adal.NewOAuthConfigWithAPIVersion(azure.PublicCloud.ActiveDirectoryEndpoint,
Expand Down Expand Up @@ -133,9 +101,9 @@ func (a *AzureAuth) getADBPlatformToken() error {

func (a *AzureAuth) getWorkspaceAccessToken(config *service.DBApiClientConfig) error {
log.Println("[DEBUG] Creating workspace token")
apiLifeTimeInSeconds := int32(600)
apiLifeTimeInSeconds := int32(3600)
comment := "Secret made via SP"
url := "https://" + a.TokenPayload.AzureRegion + ".azuredatabricks.net/api/2.0/token/create"
url := a.AdbWorkspaceUrl + "/api/2.0/token/create"
payload := struct {
LifetimeSeconds int32 `json:"lifetime_seconds,omitempty"`
Comment string `json:"comment,omitempty"`
Expand Down Expand Up @@ -178,11 +146,10 @@ func (a *AzureAuth) initWorkspaceAndGetClient(config *service.DBApiClientConfig)
return err
}

// Get workspace access token
err = a.getWorkspaceID(config)
if err != nil {
return err
}
a.AdbWorkspaceResourceID = fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Databricks/workspaces/%s",
a.TokenPayload.SubscriptionID,
a.TokenPayload.ResourceGroup,
a.TokenPayload.WorkspaceName)

// Get platform token
err = a.getADBPlatformToken()
Expand All @@ -196,8 +163,7 @@ func (a *AzureAuth) initWorkspaceAndGetClient(config *service.DBApiClientConfig)
return err
}

//// TODO: Eventually change this to include new Databricks domain names. May have to add new vars and/or deprecate existing args.
config.Host = "https://" + a.TokenPayload.AzureRegion + ".azuredatabricks.net"
config.Host = a.AdbWorkspaceUrl
config.Token = a.AdbAccessToken

return nil
Expand Down
18 changes: 18 additions & 0 deletions databricks/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ func Provider(version string) terraform.ResourceProvider {
Required: true,
DefaultFunc: schema.EnvDefaultFunc("DATABRICKS_AZURE_WORKSPACE_NAME", nil),
},
"workspace_url": {
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("DATABRICKS_AZURE_WORKSPACE_URL", nil),
},
"resource_group": {
Type: schema.TypeString,
Required: true,
Expand Down Expand Up @@ -173,6 +178,8 @@ func providerConfigureAzureClient(d *schema.ResourceData, config *service.DBApiC
azureAuthMap := azureAuth.(map[string]interface{})
//azureAuth AzureAuth{}
tokenPayload := TokenPayload{}
adbWorkspaceURL := ""

// The if else is required for the reason that "azure_auth" schema object is not a block but a map
// Maps do not inherently auto populate defaults from environment variables unless we explicitly assign values
// This makes it very difficult to test
Expand All @@ -181,22 +188,31 @@ func providerConfigureAzureClient(d *schema.ResourceData, config *service.DBApiC
} else if os.Getenv("DATABRICKS_AZURE_MANAGED_RESOURCE_GROUP") != "" {
tokenPayload.ManagedResourceGroup = os.Getenv("DATABRICKS_AZURE_MANAGED_RESOURCE_GROUP")
}

if azureRegion, ok := azureAuthMap["azure_region"].(string); ok {
tokenPayload.AzureRegion = azureRegion
} else if os.Getenv("AZURE_REGION") != "" {
tokenPayload.AzureRegion = os.Getenv("AZURE_REGION")
}

if resourceGroup, ok := azureAuthMap["resource_group"].(string); ok {
tokenPayload.ResourceGroup = resourceGroup
} else if os.Getenv("DATABRICKS_AZURE_RESOURCE_GROUP") != "" {
tokenPayload.ResourceGroup = os.Getenv("DATABRICKS_AZURE_RESOURCE_GROUP")
}

if workspaceName, ok := azureAuthMap["workspace_name"].(string); ok {
tokenPayload.WorkspaceName = workspaceName
} else if os.Getenv("DATABRICKS_AZURE_WORKSPACE_NAME") != "" {
tokenPayload.WorkspaceName = os.Getenv("DATABRICKS_AZURE_WORKSPACE_NAME")
}

if workspaceURL, ok := azureAuthMap["workspace_url"].(string); ok {
EliiseS marked this conversation as resolved.
Show resolved Hide resolved
adbWorkspaceURL = workspaceURL
} else if os.Getenv("DATABRICKS_AZURE_WORKSPACE_URL") != "" {
adbWorkspaceURL = os.Getenv("DATABRICKS_AZURE_WORKSPACE_URL")
}

// This provider takes DATABRICKS_AZURE_* for client ID etc
// The azurerm provider uses ARM_* for the same values
// To make it easier to use the two providers together we use the following sources in order:
Expand Down Expand Up @@ -244,12 +260,14 @@ func providerConfigureAzureClient(d *schema.ResourceData, config *service.DBApiC
tokenPayload.TenantID = os.Getenv("ARM_TENANT_ID")
}

adbWorkspaceURL = "https://" + adbWorkspaceURL
azureAuthSetup := AzureAuth{
TokenPayload: &tokenPayload,
ManagementToken: "",
AdbWorkspaceResourceID: "",
AdbAccessToken: "",
AdbPlatformToken: "",
AdbWorkspaceUrl: adbWorkspaceURL,
}

// Setup the CustomAuthorizer Function to be called at API invoke rather than client invoke
Expand Down
26 changes: 5 additions & 21 deletions databricks/resource_databricks_azure_adls_gen2_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,34 +69,18 @@ func testAccAzureAdlsGen2MountCorrectlyMounts() string {
clientID := os.Getenv("ARM_CLIENT_ID")
clientSecret := os.Getenv("ARM_CLIENT_SECRET")
tenantID := os.Getenv("ARM_TENANT_ID")
subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
workspaceName := os.Getenv("TEST_WORKSPACE_NAME")
resourceGroupName := os.Getenv("TEST_RESOURCE_GROUP")
managedResourceGroupName := os.Getenv("TEST_MANAGED_RESOURCE_GROUP")
location := os.Getenv("TEST_LOCATION")
gen2AdalName := os.Getenv("TEST_GEN2_ADAL_NAME")

definition := fmt.Sprintf(`
provider "databricks" {
azure_auth = {
client_id = "%[1]s"
client_secret = "%[2]s"
tenant_id = "%[3]s"
subscription_id = "%[4]s"

workspace_name = "%[5]s"
resource_group = "%[6]s"
managed_resource_group = "%[7]s"
azure_region = "%[8]s"
}
}

resource "databricks_cluster" "cluster" {
num_workers = 1
spark_version = "6.4.x-scala2.11"
node_type_id = "Standard_D3_v2"
# Don't spend too much, turn off cluster after 15mins
autotermination_minutes = 15
spark_conf = {
"spark.databricks.delta.preview.enabled": "false"
}
}

resource "databricks_secret_scope" "terraform" {
Expand All @@ -115,7 +99,7 @@ func testAccAzureAdlsGen2MountCorrectlyMounts() string {
resource "databricks_azure_adls_gen2_mount" "mount" {
cluster_id = databricks_cluster.cluster.id
container_name = "dev" # Created by prereqs.tf
storage_account_name = "%[9]s"
storage_account_name = "%[4]s"
directory = ""
mount_name = "localdir${databricks_cluster.cluster.cluster_id}"
tenant_id = "%[3]s"
Expand All @@ -125,7 +109,7 @@ func testAccAzureAdlsGen2MountCorrectlyMounts() string {
initialize_file_system = true
}

`, clientID, clientSecret, tenantID, subscriptionID, workspaceName, resourceGroupName, managedResourceGroupName, location, gen2AdalName)
`, clientID, clientSecret, tenantID, gen2AdalName)
return definition
}

Expand Down
31 changes: 6 additions & 25 deletions databricks/resource_databricks_azure_blob_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,38 +127,19 @@ func testAccAzureBlobMountMountExists(n string, azureBlobMount *AzureBlobMount,
}

func testAccAzureBlobMountCorrectlyMounts() string {
clientID := os.Getenv("ARM_CLIENT_ID")
clientSecret := os.Getenv("ARM_CLIENT_SECRET")
tenantID := os.Getenv("ARM_TENANT_ID")
subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
workspaceName := os.Getenv("TEST_WORKSPACE_NAME")
resourceGroupName := os.Getenv("TEST_RESOURCE_GROUP")
managedResourceGroupName := os.Getenv("TEST_MANAGED_RESOURCE_GROUP")
location := os.Getenv("TEST_LOCATION")
blobAccountKey := os.Getenv("TEST_STORAGE_ACCOUNT_KEY")
blobAccountName := os.Getenv("TEST_STORAGE_ACCOUNT_NAME")

definition := fmt.Sprintf(`
provider "databricks" {
azure_auth = {
client_id = "%[1]s"
client_secret = "%[2]s"
tenant_id = "%[3]s"
subscription_id = "%[4]s"

workspace_name = "%[5]s"
resource_group = "%[6]s"
managed_resource_group = "%[7]s"
azure_region = "%[8]s"
}
}

EliiseS marked this conversation as resolved.
Show resolved Hide resolved
resource "databricks_cluster" "cluster" {
num_workers = 1
spark_version = "6.4.x-scala2.11"
node_type_id = "Standard_D3_v2"
# Don't spend too much, turn off cluster after 15mins
autotermination_minutes = 15
spark_conf = {
"spark.databricks.delta.preview.enabled": "false"
}
}

resource "databricks_secret_scope" "terraform" {
Expand All @@ -170,20 +151,20 @@ func testAccAzureBlobMountCorrectlyMounts() string {

resource "databricks_secret" "storage_key" {
key = "blob_storage_key"
string_value = "%[10]s"
string_value = "%[1]s"
scope = databricks_secret_scope.terraform.name
}

resource "databricks_azure_blob_mount" "mount" {
cluster_id = databricks_cluster.cluster.id
container_name = "dev" # Created by prereqs.tf
storage_account_name = "%[9]s"
storage_account_name = "%[2]s"
mount_name = "dev"
auth_type = "ACCESS_KEY"
token_secret_scope = databricks_secret_scope.terraform.name
token_secret_key = databricks_secret.storage_key.key
}

`, clientID, clientSecret, tenantID, subscriptionID, workspaceName, resourceGroupName, managedResourceGroupName, location, blobAccountName, blobAccountKey)
`, blobAccountKey, blobAccountName)
return definition
}
1 change: 1 addition & 0 deletions examples/simple-azure-tf-deployment/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ provider "databricks" {
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
workspace_url = azurerm_databricks_workspace.demo_test_workspace.workspace_url
client_id = var.client_id
client_secret = var.client_secret
tenant_id = var.tenant_id
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ golang.org/x/tools v0.0.0-20190425150028-36563e24a262/go.mod h1:RgjU9mgBXZiqYHBn
golang.org/x/tools v0.0.0-20190506145303-2d16b83fe98c/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135/go.mod h1:RgjU9mgBXZiqYHBnxXauZ1Gv1EHHAz9KjViQ78xBX0Q=
golang.org/x/tools v0.0.0-20190606124116-d0a3d012864b/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0 h1:Dh6fw+p6FyRl5x/FvNswO1ji0lIGzm3KP8Y9VkS9PTE=
golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE=
google.golang.org/api v0.7.0/go.mod h1:WtwebWUNSVBH/HAw79HIFXZNqEvBhG+Ra+ax0hx3E3M=
Expand Down
8 changes: 6 additions & 2 deletions integration-environment-azure/prereqs.tf
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
provider "azurerm" {
version = "~> 2.3"
version = "~> 2.14"
features {}
}

Expand All @@ -22,7 +22,7 @@ resource "azurerm_databricks_workspace" "example" {
name = "workspace${random_string.naming.result}"
resource_group_name = azurerm_resource_group.example.name
location = azurerm_resource_group.example.location
sku = "standard"
sku = "premium"
managed_resource_group_name = "workspace${random_string.naming.result}"
}

Expand Down Expand Up @@ -77,6 +77,10 @@ output "workspace_name" {
value = azurerm_databricks_workspace.example.name
}

output "workspace_url" {
value = azurerm_databricks_workspace.example.workspace_url
}

output "gen2_adal_name" {
value = azurerm_storage_account.adlsaccount.name
}
Expand Down
Loading