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

Databricks CLI config reads #85

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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 59 additions & 1 deletion databricks/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import (
"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/terraform"

homedir "github.com/mitchellh/go-homedir"
ini "gopkg.in/ini.v1"
)

// Provider returns the entire terraform provider object
Expand Down Expand Up @@ -80,6 +83,22 @@ func Provider(version string) terraform.ResourceProvider {
},
ConflictsWith: []string{"token"},
},
"config_file": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("DATABRICKS_CONFIG_FILE", "~/.databrickscfg"),
Description: "Location of the Databricks CLI credentials file, that is created\n" +
"by `databricks configure --token` command. By default, it is located\n" +
"in ~/.databrickscfg. Check https://docs.databricks.com/dev-tools/cli/index.html#set-up-authentication for docs. Config\n" +
"file credetials will only be used when host/token are not provided.",
},
"profile": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Default: "DEFAULT",
Description: "Connection profile specified within ~/.databrickscfg. Please check\n" +
"https://docs.databricks.com/dev-tools/cli/index.html#connection-profiles for documentation.",
},
"azure_auth": &schema.Schema{
Type: schema.TypeMap,
Optional: true,
Expand Down Expand Up @@ -226,6 +245,41 @@ func providerConfigureAzureClient(d *schema.ResourceData, providerVersion string
return &dbClient, nil
}

// tryDatabricksCliConfigFile sets Host and Token from ~/.databrickscfg file if it exists
func tryDatabricksCliConfigFile(d *schema.ResourceData, config *service.DBApiClientConfig) error {
configFile, err := homedir.Expand(d.Get("config_file").(string))
nfx marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
cfg, err := ini.Load(configFile)
if err != nil {
return fmt.Errorf("Authentication is not configured for provider. Please configure it\n"+
"through one of the following options:\n"+
"1. DATABRICKS_HOST + DATABRICKS_TOKEN environment variables.\n"+
"2. host + token provider arguments.\n"+
"3. Run `databricks configure --token` that will create %s file.\n\n"+
"Please check https://docs.databricks.com/dev-tools/cli/index.html#set-up-authentication for details", configFile)
}
if profile, ok := d.GetOk("profile"); ok {
Copy link

Choose a reason for hiding this comment

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

Invert ok -> !ok to reduce nesting.

dbcliConfig := cfg.Section(profile.(string))
token := dbcliConfig.Key("token").String()
if "" == token {
return fmt.Errorf("config file %s is corrupt: cannot find token in %s profile",
configFile, profile)
}
config.Token = token

host := dbcliConfig.Key("host").String()
if "" == host {
return fmt.Errorf("config file %s is corrupt: cannot find host in %s profile",
configFile, profile)
}
config.Host = host
}

return nil
Copy link

Choose a reason for hiding this comment

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

schema.ResourceData does not contain required fields to fill. Is it really a positive outcome for this function?

}

func providerConfigure(d *schema.ResourceData, providerVersion string) (interface{}, error) {
nfx marked this conversation as resolved.
Show resolved Hide resolved
var config service.DBApiClientConfig
// Call setup to configure retryable httpclient
Expand All @@ -241,7 +295,11 @@ func providerConfigure(d *schema.ResourceData, providerVersion string) (interfac
if token, ok := d.GetOk("token"); ok {
config.Token = token.(string)
}

if config.Host == "" || config.Token == "" {
if err := tryDatabricksCliConfigFile(d, &config); err != nil {
return nil, fmt.Errorf("failed to get credentials from config file; error msg: %w", err)
}
}
// Basic authentication setup via username and password
if _, ok := d.GetOk("basic_auth"); ok {
username, userOk := d.GetOk("basic_auth.0.username")
Expand Down
91 changes: 89 additions & 2 deletions databricks/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,18 @@ package databricks
import (
"encoding/base64"
"fmt"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"log"
"os"
"testing"

"github.com/databrickslabs/databricks-terraform/client/service"
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/terraform"
"github.com/joho/godotenv"
"github.com/stretchr/testify/assert"

"github.com/databrickslabs/databricks-terraform/client/service"
)

var testAccProviders map[string]terraform.ResourceProvider
Expand Down Expand Up @@ -98,3 +101,87 @@ resource "databricks_scim_group" "my-group-azure3" {
}
`
}

func TestProvider(t *testing.T) {
if err := testAccProvider.InternalValidate(); err != nil {
t.Fatalf("err: %s", err)
}
}

func TestProvider_NoOptionsResultsInError(t *testing.T) {
var provider = Provider("")
var raw = make(map[string]interface{})
raw["config_file"] = "testdata/.databrickscfg_non_existant"
err := provider.Configure(terraform.NewResourceConfigRaw(raw))
assert.NotNil(t, err)
}

func TestProvider_HostTokensTakePrecedence(t *testing.T) {
var raw = make(map[string]interface{})
raw["host"] = "foo"
raw["token"] = "configured"
raw["config_file"] = "testdata/.databrickscfg"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
assert.Nil(t, err)

client := testAccProvider.Meta().(*service.DBApiClient).Config
assert.Equal(t, "configured", client.Token)
}

func TestProvider_MissingEnvMakesConfigRead(t *testing.T) {
var raw = make(map[string]interface{})
raw["token"] = "configured"
raw["config_file"] = "testdata/.databrickscfg"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
assert.Nil(t, err)

client := testAccProvider.Meta().(*service.DBApiClient).Config
assert.Equal(t, "PT0+IC9kZXYvdXJhbmRvbSA8PT0KYFZ", client.Token)
}

func TestProvider_NoHostGivesError(t *testing.T) {
var raw = make(map[string]interface{})
raw["config_file"] = "testdata/.databrickscfg"
raw["profile"] = "nohost"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
assert.NotNil(t, err)
}

func TestProvider_NoTokenGivesError(t *testing.T) {
var raw = make(map[string]interface{})
raw["config_file"] = "testdata/.databrickscfg"
raw["profile"] = "notoken"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
assert.NotNil(t, err)
}

func TestProvider_InvalidProfileGivesError(t *testing.T) {
var raw = make(map[string]interface{})
raw["config_file"] = "testdata/.databrickscfg"
raw["profile"] = "invalidhost"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
assert.NotNil(t, err)
Copy link

Choose a reason for hiding this comment

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

It might also be a good idea to actually check that the error message is the one that you expect and contains all the context required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

according to certain beliefs, asserting exception message was not considered a good practice - e.g. because it may change the content of the string or be translated to other language, therefore making test "more overfitting" and fragile to insignificant changes.

}

func TestProvider_InvalidConfigFilePath(t *testing.T) {
var raw = make(map[string]interface{})
raw["config_file"] = "testdata/.invalid file"
raw["profile"] = "invalidhost"
err := testAccProvider.Configure(terraform.NewResourceConfigRaw(raw))
log.Println(err)
assert.NotNil(t, err)
}

func TestAccDatabricksCliConfigWorks(t *testing.T) {
resource.Test(t,
resource.TestCase{
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: `provider "databricks" {}`,
ExpectNonEmptyPlan: true,
},
},
},
)
}
9 changes: 9 additions & 0 deletions databricks/testdata/.databrickscfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[DEFAULT]
host = https://dbc-XXXXXXXX-YYYY.cloud.databricks.com/
token = PT0+IC9kZXYvdXJhbmRvbSA8PT0KYFZ

[nohost]
token = PT0+IC9kZXYvdXJhbmRvbSA8PT0KYFZ

[notoken]
host = https://dbc-XXXXXXXX-YYYY.cloud.databricks.com/
12 changes: 12 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,26 @@ go 1.13
require (
github.com/Azure/go-autorest/autorest v0.10.1
github.com/Azure/go-autorest/autorest/adal v0.8.3
github.com/fatih/color v1.9.0 // indirect
github.com/google/go-querystring v1.0.0
github.com/hashicorp/go-retryablehttp v0.6.6
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/hashicorp/terraform-plugin-sdk v1.13.0
github.com/joho/godotenv v1.3.0
github.com/mattn/go-colorable v0.1.6 // indirect
github.com/mitchellh/go-homedir v1.1.0
github.com/pkg/errors v0.9.1 // indirect
github.com/r3labs/diff v0.0.0-20191120142937-b4ed99a31f5a
github.com/sergi/go-diff v1.1.0 // indirect
github.com/sirupsen/logrus v1.6.0 // indirect
github.com/smartystreets/goconvey v1.6.4 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/testify v1.5.1
golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 // indirect
golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a // indirect
golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 // indirect
gopkg.in/ini.v1 v1.57.0
gotest.tools/gotestsum v0.4.2 // indirect
)

replace github.com/Azure/go-autorest => github.com/tombuildsstuff/go-autorest v14.0.1-0.20200317095413-f2d2d0252c3c+incompatible
Expand Down