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

Always resolve .databrickscfg file #659

Merged
merged 2 commits into from
Aug 14, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/auth/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/url"
"strings"

"github.com/databricks/cli/libs/databrickscfg"
"github.com/databricks/databricks-sdk-go/config"
"github.com/spf13/cobra"
"gopkg.in/ini.v1"
Expand All @@ -28,7 +29,7 @@ func canonicalHost(host string) (string, error) {

var ErrNoMatchingProfiles = errors.New("no matching profiles found")

func resolveSection(cfg *config.Config, iniFile *ini.File) (*ini.Section, error) {
func resolveSection(cfg *config.Config, iniFile *config.File) (*ini.Section, error) {
var candidates []*ini.Section
configuredHost, err := canonicalHost(cfg.Host)
if err != nil {
Expand Down Expand Up @@ -68,7 +69,7 @@ func resolveSection(cfg *config.Config, iniFile *ini.File) (*ini.Section, error)
}

func loadFromDatabricksCfg(cfg *config.Config) error {
iniFile, err := getDatabricksCfg()
iniFile, err := databrickscfg.Get()
if errors.Is(err, fs.ErrNotExist) {
// it's fine not to have ~/.databrickscfg
return nil
Expand Down
2 changes: 1 addition & 1 deletion cmd/auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func newLoginCommand(persistentAuth *auth.PersistentAuth) *cobra.Command {
}

// If the chosen profile has a hostname and the user hasn't specified a host, infer the host from the profile.
_, profiles, err := databrickscfg.LoadProfiles(databrickscfg.DefaultPath, func(p databrickscfg.Profile) bool {
_, profiles, err := databrickscfg.LoadProfiles(func(p databrickscfg.Profile) bool {
return p.Name == profileName
})
if err != nil {
Expand Down
24 changes: 5 additions & 19 deletions cmd/auth/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,16 @@ import (
"fmt"
"net/http"
"os"
"path/filepath"
"strings"
"sync"

"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/databrickscfg"
"github.com/databricks/databricks-sdk-go"
"github.com/databricks/databricks-sdk-go/config"
"github.com/spf13/cobra"
"gopkg.in/ini.v1"
)

func getDatabricksCfg() (*ini.File, error) {
configFile := os.Getenv("DATABRICKS_CONFIG_FILE")
if configFile == "" {
configFile = "~/.databrickscfg"
}
if strings.HasPrefix(configFile, "~") {
homedir, err := os.UserHomeDir()
if err != nil {
return nil, fmt.Errorf("cannot find homedir: %w", err)
}
configFile = filepath.Join(homedir, configFile[1:])
}
return ini.Load(configFile)
}

type profileMetadata struct {
Name string `json:"name"`
Host string `json:"host,omitempty"`
Expand Down Expand Up @@ -111,10 +95,12 @@ func newProfilesCommand() *cobra.Command {

cmd.RunE = func(cmd *cobra.Command, args []string) error {
var profiles []*profileMetadata
iniFile, err := getDatabricksCfg()
iniFile, err := databrickscfg.Get()
if os.IsNotExist(err) {
// return empty list for non-configured machines
iniFile = ini.Empty()
iniFile = &config.File{
File: &ini.File{},
}
} else if err != nil {
return fmt.Errorf("cannot parse config file: %w", err)
}
Expand Down
19 changes: 11 additions & 8 deletions cmd/root/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error {
// 1. only admins will have account configured
// 2. 99% of admins will have access to just one account
// hence, we don't need to create a special "DEFAULT_ACCOUNT" profile yet
_, profiles, err := databrickscfg.LoadProfiles(
databrickscfg.DefaultPath,
databrickscfg.MatchAccountProfiles,
)
_, profiles, err := databrickscfg.LoadProfiles(databrickscfg.MatchAccountProfiles)
if err != nil {
return err
}
Expand Down Expand Up @@ -124,8 +121,11 @@ func transformLoadError(path string, err error) error {
}

func askForWorkspaceProfile() (string, error) {
path := databrickscfg.DefaultPath
file, profiles, err := databrickscfg.LoadProfiles(path, databrickscfg.MatchWorkspaceProfiles)
path, err := databrickscfg.GetPath()
if err != nil {
return "", fmt.Errorf("cannot determine Databricks config file path: %w", err)
}
file, profiles, err := databrickscfg.LoadProfiles(databrickscfg.MatchWorkspaceProfiles)
if err != nil {
return "", transformLoadError(path, err)
}
Expand Down Expand Up @@ -156,8 +156,11 @@ func askForWorkspaceProfile() (string, error) {
}

func askForAccountProfile() (string, error) {
path := databrickscfg.DefaultPath
file, profiles, err := databrickscfg.LoadProfiles(path, databrickscfg.MatchAccountProfiles)
path, err := databrickscfg.GetPath()
if err != nil {
return "", fmt.Errorf("cannot determine Databricks config file path: %w", err)
}
file, profiles, err := databrickscfg.LoadProfiles(databrickscfg.MatchAccountProfiles)
if err != nil {
return "", transformLoadError(path, err)
}
Expand Down
34 changes: 29 additions & 5 deletions libs/databrickscfg/profiles.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package databrickscfg

import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/databricks/databricks-sdk-go/config"
Expand Down Expand Up @@ -64,12 +66,34 @@ func MatchAllProfiles(p Profile) bool {
return true
}

const DefaultPath = "~/.databrickscfg"
// Get the path to the .databrickscfg file, falling back to the default in the current user's home directory.
func GetPath() (string, error) {
configFile := os.Getenv("DATABRICKS_CONFIG_FILE")
if configFile == "" {
configFile = "~/.databrickscfg"
}
if strings.HasPrefix(configFile, "~") {
homedir, err := os.UserHomeDir()
if err != nil {
return "", fmt.Errorf("cannot find homedir: %w", err)
}
configFile = filepath.Join(homedir, configFile[1:])
}
return configFile, nil
}

func LoadProfiles(path string, fn ProfileMatchFunction) (file string, profiles Profiles, err error) {
f, err := config.LoadFile(path)
func Get() (*config.File, error) {
configFile, err := GetPath()
if err != nil {
return
return nil, fmt.Errorf("cannot determine Databricks config file path: %w", err)
}
return config.LoadFile(configFile)
}

func LoadProfiles(fn ProfileMatchFunction) (file string, profiles Profiles, err error) {
f, err := Get()
if err != nil {
return "", nil, fmt.Errorf("cannot load Databricks config file: %w", err)
}

homedir, err := os.UserHomeDir()
Expand Down Expand Up @@ -106,7 +130,7 @@ func LoadProfiles(path string, fn ProfileMatchFunction) (file string, profiles P
}

func ProfileCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
_, profiles, err := LoadProfiles(DefaultPath, MatchAllProfiles)
_, profiles, err := LoadProfiles(MatchAllProfiles)
if err != nil {
return nil, cobra.ShellCompDirectiveError
}
Expand Down
9 changes: 6 additions & 3 deletions libs/databrickscfg/profiles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,22 @@ func TestLoadProfilesReturnsHomedirAsTilde(t *testing.T) {
} else {
t.Setenv("HOME", "./testdata")
}
file, _, err := LoadProfiles("./testdata/databrickscfg", func(p Profile) bool { return true })
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/databrickscfg")
file, _, err := LoadProfiles(func(p Profile) bool { return true })
require.NoError(t, err)
assert.Equal(t, "~/databrickscfg", file)
}

func TestLoadProfilesMatchWorkspace(t *testing.T) {
_, profiles, err := LoadProfiles("./testdata/databrickscfg", MatchWorkspaceProfiles)
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/databrickscfg")
_, profiles, err := LoadProfiles(MatchWorkspaceProfiles)
require.NoError(t, err)
assert.Equal(t, []string{"DEFAULT", "query", "foo1", "foo2"}, profiles.Names())
}

func TestLoadProfilesMatchAccount(t *testing.T) {
_, profiles, err := LoadProfiles("./testdata/databrickscfg", MatchAccountProfiles)
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/databrickscfg")
_, profiles, err := LoadProfiles(MatchAccountProfiles)
require.NoError(t, err)
assert.Equal(t, []string{"acc"}, profiles.Names())
}