18940: Go clients load $HOME/.config/arvados/settings.conf
authorTom Clegg <tom@curii.com>
Wed, 30 Mar 2022 18:40:25 +0000 (14:40 -0400)
committerTom Clegg <tom@curii.com>
Wed, 30 Mar 2022 18:40:25 +0000 (14:40 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

sdk/go/arvados/client.go
sdk/go/arvados/client_test.go

index 5ec828667fc940ace2c3f59b6cdc643139ae3b14..24d5ac3e335c824f5ea4a444c6066ce37f3cc86f 100644 (file)
@@ -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,
index df938008d49756b850ca6e5ce5abee8a0510e2a3..2363803cab1de157f4074d3a2770f2cc0c9201ca 100644 (file)
@@ -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)
+}