From: Tom Clegg Date: Wed, 30 Mar 2022 18:40:25 +0000 (-0400) Subject: 18940: Go clients load $HOME/.config/arvados/settings.conf X-Git-Tag: 2.4.0~11 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/b7eab29935feef0a7bdfe35fa092118a7dd7e668 18940: Go clients load $HOME/.config/arvados/settings.conf Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go index 5ec828667f..24d5ac3e33 100644 --- a/sdk/go/arvados/client.go +++ b/sdk/go/arvados/client.go @@ -12,6 +12,7 @@ import ( "errors" "fmt" "io" + "io/fs" "io/ioutil" "log" "net/http" @@ -102,11 +103,60 @@ func NewClientFromConfig(cluster *Cluster) (*Client, error) { } // NewClientFromEnv creates a new Client that uses the default HTTP -// client with the API endpoint and credentials given by the -// ARVADOS_API_* environment variables. +// client, and loads API endpoint and credentials from ARVADOS_* +// environment variables (if set) and +// $HOME/.config/arvados/settings.conf (if readable). +// +// If a config exists in both locations, the environment variable is +// used. +// +// If there is an error (other than ENOENT) reading settings.conf, +// NewClientFromEnv logs the error to log.Default(), then proceeds as +// if settings.conf did not exist. +// +// Space characters are trimmed when reading the settings file, so +// these are equivalent: +// +// ARVADOS_API_HOST=localhost\n +// ARVADOS_API_HOST=localhost\r\n +// ARVADOS_API_HOST = localhost \n +// \tARVADOS_API_HOST = localhost\n func NewClientFromEnv() *Client { + vars := map[string]string{} + home := os.Getenv("HOME") + conffile := home + "/.config/arvados/settings.conf" + if home == "" { + // no $HOME => just use env vars + } else if settings, err := os.ReadFile(conffile); errors.Is(err, fs.ErrNotExist) { + // no config file => just use env vars + } else if err != nil { + // config file unreadable => log message, then use env vars + log.Printf("continuing without loading %s: %s", conffile, err) + } else { + for _, line := range bytes.Split(settings, []byte{'\n'}) { + kv := bytes.SplitN(line, []byte{'='}, 2) + k := string(bytes.TrimSpace(kv[0])) + if len(kv) != 2 || !strings.HasPrefix(k, "ARVADOS_") { + // Same behavior as python sdk: + // silently skip leading # (comments), + // blank lines, typos, and non-Arvados + // vars. + continue + } + vars[k] = string(bytes.TrimSpace(kv[1])) + } + } + for _, env := range os.Environ() { + if !strings.HasPrefix(env, "ARVADOS_") { + continue + } + kv := strings.SplitN(env, "=", 2) + if len(kv) == 2 { + vars[kv[0]] = kv[1] + } + } var svcs []string - for _, s := range strings.Split(os.Getenv("ARVADOS_KEEP_SERVICES"), " ") { + for _, s := range strings.Split(vars["ARVADOS_KEEP_SERVICES"], " ") { if s == "" { continue } else if u, err := url.Parse(s); err != nil { @@ -118,13 +168,13 @@ func NewClientFromEnv() *Client { } } var insecure bool - if s := strings.ToLower(os.Getenv("ARVADOS_API_HOST_INSECURE")); s == "1" || s == "yes" || s == "true" { + if s := strings.ToLower(vars["ARVADOS_API_HOST_INSECURE"]); s == "1" || s == "yes" || s == "true" { insecure = true } return &Client{ Scheme: "https", - APIHost: os.Getenv("ARVADOS_API_HOST"), - AuthToken: os.Getenv("ARVADOS_API_TOKEN"), + APIHost: vars["ARVADOS_API_HOST"], + AuthToken: vars["ARVADOS_API_TOKEN"], Insecure: insecure, KeepServiceURIs: svcs, Timeout: 5 * time.Minute, diff --git a/sdk/go/arvados/client_test.go b/sdk/go/arvados/client_test.go index df938008d4..2363803cab 100644 --- a/sdk/go/arvados/client_test.go +++ b/sdk/go/arvados/client_test.go @@ -10,9 +10,12 @@ import ( "io/ioutil" "net/http" "net/url" + "os" + "strings" "sync" - "testing" "testing/iotest" + + check "gopkg.in/check.v1" ) type stubTransport struct { @@ -68,43 +71,36 @@ func (stub *timeoutTransport) RoundTrip(req *http.Request) (*http.Response, erro }, nil } -func TestCurrentUser(t *testing.T) { - t.Parallel() +var _ = check.Suite(&clientSuite{}) + +type clientSuite struct{} + +func (*clientSuite) TestCurrentUser(c *check.C) { stub := &stubTransport{ Responses: map[string]string{ "/arvados/v1/users/current": `{"uuid":"zzzzz-abcde-012340123401234"}`, }, } - c := &Client{ + client := &Client{ Client: &http.Client{ Transport: stub, }, APIHost: "zzzzz.arvadosapi.com", AuthToken: "xyzzy", } - u, err := c.CurrentUser() - if err != nil { - t.Fatal(err) - } - if x := "zzzzz-abcde-012340123401234"; u.UUID != x { - t.Errorf("got uuid %q, expected %q", u.UUID, x) - } - if len(stub.Requests) < 1 { - t.Fatal("empty stub.Requests") - } + u, err := client.CurrentUser() + c.Check(err, check.IsNil) + c.Check(u.UUID, check.Equals, "zzzzz-abcde-012340123401234") + c.Check(stub.Requests, check.Not(check.HasLen), 0) hdr := stub.Requests[len(stub.Requests)-1].Header - if hdr.Get("Authorization") != "OAuth2 xyzzy" { - t.Errorf("got headers %+q, expected Authorization header", hdr) - } + c.Check(hdr.Get("Authorization"), check.Equals, "OAuth2 xyzzy") - c.Client.Transport = &errorTransport{} - u, err = c.CurrentUser() - if err == nil { - t.Errorf("got nil error, expected something awful") - } + client.Client.Transport = &errorTransport{} + u, err = client.CurrentUser() + c.Check(err, check.NotNil) } -func TestAnythingToValues(t *testing.T) { +func (*clientSuite) TestAnythingToValues(c *check.C) { type testCase struct { in interface{} // ok==nil means anythingToValues should return an @@ -158,17 +154,66 @@ func TestAnythingToValues(t *testing.T) { ok: nil, }, } { - t.Logf("%#v", tc.in) + c.Logf("%#v", tc.in) out, err := anythingToValues(tc.in) - switch { - case tc.ok == nil: - if err == nil { - t.Errorf("got %#v, expected error", out) - } - case err != nil: - t.Errorf("got err %#v, expected nil", err) - case !tc.ok(out): - t.Errorf("got %#v but tc.ok() says that is wrong", out) + if tc.ok == nil { + c.Check(err, check.NotNil) + continue } + c.Check(err, check.IsNil) + c.Check(tc.ok(out), check.Equals, true) } } + +func (*clientSuite) TestLoadConfig(c *check.C) { + oldenv := os.Environ() + defer func() { + os.Clearenv() + for _, s := range oldenv { + i := strings.IndexRune(s, '=') + os.Setenv(s[:i], s[i+1:]) + } + }() + + tmp := c.MkDir() + os.Setenv("HOME", tmp) + for _, s := range os.Environ() { + if strings.HasPrefix(s, "ARVADOS_") { + i := strings.IndexRune(s, '=') + os.Unsetenv(s[:i]) + } + } + os.Mkdir(tmp+"/.config", 0777) + os.Mkdir(tmp+"/.config/arvados", 0777) + + // Use $HOME/.config/arvados/settings.conf if no env vars are + // set + os.WriteFile(tmp+"/.config/arvados/settings.conf", []byte(` + ARVADOS_API_HOST = localhost:1 + ARVADOS_API_TOKEN = token_from_settings_file1 + `), 0777) + client := NewClientFromEnv() + c.Check(client.AuthToken, check.Equals, "token_from_settings_file1") + c.Check(client.APIHost, check.Equals, "localhost:1") + c.Check(client.Insecure, check.Equals, false) + + // ..._INSECURE=true, comments, ignored lines in settings.conf + os.WriteFile(tmp+"/.config/arvados/settings.conf", []byte(` + (ignored) = (ignored) + #ARVADOS_API_HOST = localhost:2 + ARVADOS_API_TOKEN = token_from_settings_file2 + ARVADOS_API_HOST_INSECURE = true + `), 0777) + client = NewClientFromEnv() + c.Check(client.AuthToken, check.Equals, "token_from_settings_file2") + c.Check(client.APIHost, check.Equals, "") + c.Check(client.Insecure, check.Equals, true) + + // Environment variables override settings.conf + os.Setenv("ARVADOS_API_HOST", "[::]:3") + os.Setenv("ARVADOS_API_HOST_INSECURE", "0") + client = NewClientFromEnv() + c.Check(client.AuthToken, check.Equals, "token_from_settings_file2") + c.Check(client.APIHost, check.Equals, "[::]:3") + c.Check(client.Insecure, check.Equals, false) +}