Merge branch 'master' into 7492-keepproxy-upstream-errors
[arvados.git] / services / keepproxy / keepproxy_test.go
index 8acf43abd4eba5094d89e4443a0d6a067139aa80..6303122e345414d11b8d75c0ebd5be3eb5c428be 100644 (file)
@@ -5,6 +5,7 @@ import (
        "crypto/tls"
        "fmt"
        "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
+       "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        "git.curoverse.com/arvados.git/sdk/go/keepclient"
        . "gopkg.in/check.v1"
        "io"
@@ -13,7 +14,6 @@ import (
        "net/http"
        "net/url"
        "os"
-       "os/exec"
        "strings"
        "testing"
        "time"
@@ -30,11 +30,6 @@ var _ = Suite(&ServerRequiredSuite{})
 // Tests that require the Keep server running
 type ServerRequiredSuite struct{}
 
-func pythonDir() string {
-       cwd, _ := os.Getwd()
-       return fmt.Sprintf("%s/../../sdk/python/tests", cwd)
-}
-
 // Wait (up to 1 second) for keepproxy to listen on a port. This
 // avoids a race condition where we hit a "connection refused" error
 // because we start testing the proxy too soon.
@@ -57,45 +52,17 @@ func closeListener() {
 }
 
 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
-       cwd, _ := os.Getwd()
-       defer os.Chdir(cwd)
-
-       os.Chdir(pythonDir())
-       {
-               cmd := exec.Command("python", "run_test_server.py", "start")
-               stderr, err := cmd.StderrPipe()
-               if err != nil {
-                       log.Fatalf("Setting up stderr pipe: %s", err)
-               }
-               go io.Copy(os.Stderr, stderr)
-               if err := cmd.Run(); err != nil {
-                       panic(fmt.Sprintf("'python run_test_server.py start' returned error %s", err))
-               }
-       }
-       {
-               cmd := exec.Command("python", "run_test_server.py", "start_keep")
-               stderr, err := cmd.StderrPipe()
-               if err != nil {
-                       log.Fatalf("Setting up stderr pipe: %s", err)
-               }
-               go io.Copy(os.Stderr, stderr)
-               if err := cmd.Run(); err != nil {
-                       panic(fmt.Sprintf("'python run_test_server.py start_keep' returned error %s", err))
-               }
-       }
+       arvadostest.StartAPI()
+       arvadostest.StartKeep(2, false)
+}
 
-       os.Setenv("ARVADOS_API_HOST", "localhost:3000")
-       os.Setenv("ARVADOS_API_TOKEN", "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h")
-       os.Setenv("ARVADOS_API_HOST_INSECURE", "true")
+func (s *ServerRequiredSuite) SetUpTest(c *C) {
+       arvadostest.ResetEnv()
 }
 
 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
-       cwd, _ := os.Getwd()
-       defer os.Chdir(cwd)
-
-       os.Chdir(pythonDir())
-       exec.Command("python", "run_test_server.py", "stop_keep").Run()
-       exec.Command("python", "run_test_server.py", "stop").Run()
+       arvadostest.StopKeep(2)
+       arvadostest.StopAPI()
 }
 
 func setupProxyService() {
@@ -136,27 +103,41 @@ func setupProxyService() {
        }
 }
 
-func runProxy(c *C, args []string, token string, port int) keepclient.KeepClient {
-       os.Args = append(args, fmt.Sprintf("-listen=:%v", port))
-       os.Setenv("ARVADOS_API_TOKEN", "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h")
-
-       listener = nil
-       go main()
-       time.Sleep(100 * time.Millisecond)
-
-       os.Setenv("ARVADOS_KEEP_PROXY", fmt.Sprintf("http://localhost:%v", port))
-       os.Setenv("ARVADOS_API_TOKEN", token)
+func runProxy(c *C, args []string, port int, bogusClientToken bool) keepclient.KeepClient {
+       if bogusClientToken {
+               os.Setenv("ARVADOS_API_TOKEN", "bogus-token")
+       }
        arv, err := arvadosclient.MakeArvadosClient()
        c.Assert(err, Equals, nil)
-       kc, err := keepclient.MakeKeepClient(&arv)
-       c.Assert(err, Equals, nil)
+       kc := keepclient.KeepClient{
+               Arvados:       &arv,
+               Want_replicas: 2,
+               Using_proxy:   true,
+               Client:        &http.Client{},
+       }
+       locals := map[string]string{
+               "proxy": fmt.Sprintf("http://localhost:%v", port),
+       }
+       writableLocals := map[string]string{
+               "proxy": fmt.Sprintf("http://localhost:%v", port),
+       }
+       kc.SetServiceRoots(locals, writableLocals, nil)
        c.Check(kc.Using_proxy, Equals, true)
-       c.Check(len(kc.ServiceRoots()), Equals, 1)
-       for _, root := range kc.ServiceRoots() {
+       c.Check(len(kc.LocalRoots()), Equals, 1)
+       for _, root := range kc.LocalRoots() {
                c.Check(root, Equals, fmt.Sprintf("http://localhost:%v", port))
        }
-       os.Setenv("ARVADOS_KEEP_PROXY", "")
        log.Print("keepclient created")
+       if bogusClientToken {
+               arvadostest.ResetEnv()
+       }
+
+       {
+               os.Args = append(args, fmt.Sprintf("-listen=:%v", port))
+               listener = nil
+               go main()
+       }
+
        return kc
 }
 
@@ -164,7 +145,6 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
        log.Print("TestPutAndGet start")
 
        os.Args = []string{"keepproxy", "-listen=:29950"}
-       os.Setenv("ARVADOS_API_TOKEN", "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h")
        listener = nil
        go main()
        time.Sleep(100 * time.Millisecond)
@@ -178,12 +158,11 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
        c.Assert(err, Equals, nil)
        c.Check(kc.Arvados.External, Equals, true)
        c.Check(kc.Using_proxy, Equals, true)
-       c.Check(len(kc.ServiceRoots()), Equals, 1)
-       for _, root := range kc.ServiceRoots() {
+       c.Check(len(kc.LocalRoots()), Equals, 1)
+       for _, root := range kc.LocalRoots() {
                c.Check(root, Equals, "http://localhost:29950")
        }
        os.Setenv("ARVADOS_EXTERNAL_CLIENT", "")
-       log.Print("keepclient created")
 
        waitForListener()
        defer closeListener()
@@ -194,9 +173,25 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
        {
                _, _, err := kc.Ask(hash)
                c.Check(err, Equals, keepclient.BlockNotFound)
-               log.Print("Ask 1")
+               log.Print("Finished Ask (expected BlockNotFound)")
        }
 
+       {
+               reader, _, _, err := kc.Get(hash)
+               c.Check(reader, Equals, nil)
+               c.Check(err, Equals, keepclient.BlockNotFound)
+               log.Print("Finished Get (expected BlockNotFound)")
+       }
+
+       // Note in bug #5309 among other errors keepproxy would set
+       // Content-Length incorrectly on the 404 BlockNotFound response, this
+       // would result in a protocol violation that would prevent reuse of the
+       // connection, which would manifest by the next attempt to use the
+       // connection (in this case the PutB below) failing.  So to test for
+       // that bug it's necessary to trigger an error response (such as
+       // BlockNotFound) and then do something else with the same httpClient
+       // connection.
+
        {
                var rep int
                var err error
@@ -204,14 +199,14 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
                c.Check(hash2, Matches, fmt.Sprintf(`^%s\+3(\+.+)?$`, hash))
                c.Check(rep, Equals, 2)
                c.Check(err, Equals, nil)
-               log.Print("PutB")
+               log.Print("Finished PutB (expected success)")
        }
 
        {
                blocklen, _, err := kc.Ask(hash2)
                c.Assert(err, Equals, nil)
                c.Check(blocklen, Equals, int64(3))
-               log.Print("Ask 2")
+               log.Print("Finished Ask (expected success)")
        }
 
        {
@@ -220,7 +215,7 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
                all, err := ioutil.ReadAll(reader)
                c.Check(all, DeepEquals, []byte("foo"))
                c.Check(blocklen, Equals, int64(3))
-               log.Print("Get")
+               log.Print("Finished Get (expected success)")
        }
 
        {
@@ -230,7 +225,7 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
                c.Check(hash2, Matches, `^d41d8cd98f00b204e9800998ecf8427e\+0(\+.+)?$`)
                c.Check(rep, Equals, 2)
                c.Check(err, Equals, nil)
-               log.Print("PutB zero block")
+               log.Print("Finished PutB zero block")
        }
 
        {
@@ -239,7 +234,7 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
                all, err := ioutil.ReadAll(reader)
                c.Check(all, DeepEquals, []byte(""))
                c.Check(blocklen, Equals, int64(0))
-               log.Print("Get zero block")
+               log.Print("Finished Get zero block")
        }
 
        log.Print("TestPutAndGet done")
@@ -248,17 +243,17 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
        log.Print("TestPutAskGetForbidden start")
 
-       kc := runProxy(c, []string{"keepproxy"}, "123abc", 29951)
+       kc := runProxy(c, []string{"keepproxy"}, 29951, true)
        waitForListener()
        defer closeListener()
 
-       log.Print("keepclient created")
-
        hash := fmt.Sprintf("%x", md5.Sum([]byte("bar")))
 
        {
                _, _, err := kc.Ask(hash)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                log.Print("Ask 1")
        }
 
@@ -272,14 +267,18 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
 
        {
                blocklen, _, err := kc.Ask(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Ask 2")
        }
 
        {
                _, blocklen, _, err := kc.Get(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Get")
        }
@@ -290,7 +289,7 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
 func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
        log.Print("TestGetDisabled start")
 
-       kc := runProxy(c, []string{"keepproxy", "-no-get"}, "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h", 29952)
+       kc := runProxy(c, []string{"keepproxy", "-no-get"}, 29952, false)
        waitForListener()
        defer closeListener()
 
@@ -298,7 +297,9 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
 
        {
                _, _, err := kc.Ask(hash)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                log.Print("Ask 1")
        }
 
@@ -312,14 +313,18 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
 
        {
                blocklen, _, err := kc.Ask(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Ask 2")
        }
 
        {
                _, blocklen, _, err := kc.Get(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Get")
        }
@@ -330,7 +335,7 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
 func (s *ServerRequiredSuite) TestPutDisabled(c *C) {
        log.Print("TestPutDisabled start")
 
-       kc := runProxy(c, []string{"keepproxy", "-no-put"}, "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h", 29953)
+       kc := runProxy(c, []string{"keepproxy", "-no-put"}, 29953, false)
        waitForListener()
        defer closeListener()
 
@@ -346,7 +351,7 @@ func (s *ServerRequiredSuite) TestPutDisabled(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
-       runProxy(c, []string{"keepproxy"}, "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h", 29954)
+       runProxy(c, []string{"keepproxy"}, 29954, false)
        waitForListener()
        defer closeListener()
 
@@ -378,7 +383,7 @@ func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPostWithoutHash(c *C) {
-       runProxy(c, []string{"keepproxy"}, "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h", 29955)
+       runProxy(c, []string{"keepproxy"}, 29955, false)
        waitForListener()
        defer closeListener()
 
@@ -393,7 +398,150 @@ func (s *ServerRequiredSuite) TestPostWithoutHash(c *C) {
                c.Check(err, Equals, nil)
                body, err := ioutil.ReadAll(resp.Body)
                c.Check(err, Equals, nil)
-               c.Check(string(body), Equals,
-                       fmt.Sprintf("%x+%d", md5.Sum([]byte("qux")), 3))
+               c.Check(string(body), Matches,
+                       fmt.Sprintf(`^%x\+3(\+.+)?$`, md5.Sum([]byte("qux"))))
+       }
+}
+
+func (s *ServerRequiredSuite) TestStripHint(c *C) {
+       c.Check(removeHint.ReplaceAllString("http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73+K@zzzzz", "$1"),
+               Equals,
+               "http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73")
+       c.Check(removeHint.ReplaceAllString("http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+K@zzzzz+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73", "$1"),
+               Equals,
+               "http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73")
+       c.Check(removeHint.ReplaceAllString("http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73+K@zzzzz-zzzzz-zzzzzzzzzzzzzzz", "$1"),
+               Equals,
+               "http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73+K@zzzzz-zzzzz-zzzzzzzzzzzzzzz")
+       c.Check(removeHint.ReplaceAllString("http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+K@zzzzz-zzzzz-zzzzzzzzzzzzzzz+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73", "$1"),
+               Equals,
+               "http://keep.zzzzz.arvadosapi.com:25107/2228819a18d3727630fa30c81853d23f+67108864+K@zzzzz-zzzzz-zzzzzzzzzzzzzzz+A37b6ab198qqqq28d903b975266b23ee711e1852c@55635f73")
+
+}
+
+// Test GetIndex
+//   Put one block, with 2 replicas
+//   With no prefix (expect the block locator, twice)
+//   With an existing prefix (expect the block locator, twice)
+//   With a valid but non-existing prefix (expect "\n")
+//   With an invalid prefix (expect error)
+func (s *ServerRequiredSuite) TestGetIndex(c *C) {
+       kc := runProxy(c, []string{"keepproxy"}, 28852, false)
+       waitForListener()
+       defer closeListener()
+
+       // Put "index-data" blocks
+       data := []byte("index-data")
+       hash := fmt.Sprintf("%x", md5.Sum(data))
+
+       hash2, rep, err := kc.PutB(data)
+       c.Check(hash2, Matches, fmt.Sprintf(`^%s\+10(\+.+)?$`, hash))
+       c.Check(rep, Equals, 2)
+       c.Check(err, Equals, nil)
+
+       reader, blocklen, _, err := kc.Get(hash)
+       c.Assert(err, Equals, nil)
+       c.Check(blocklen, Equals, int64(10))
+       all, err := ioutil.ReadAll(reader)
+       c.Check(all, DeepEquals, data)
+
+       // Put some more blocks
+       _, rep, err = kc.PutB([]byte("some-more-index-data"))
+       c.Check(err, Equals, nil)
+
+       // Invoke GetIndex
+       for _, spec := range []struct {
+               prefix         string
+               expectTestHash bool
+               expectOther    bool
+       }{
+               {"", true, true},         // with no prefix
+               {hash[:3], true, false},  // with matching prefix
+               {"abcdef", false, false}, // with no such prefix
+       } {
+               indexReader, err := kc.GetIndex("proxy", spec.prefix)
+               c.Assert(err, Equals, nil)
+               indexResp, err := ioutil.ReadAll(indexReader)
+               c.Assert(err, Equals, nil)
+               locators := strings.Split(string(indexResp), "\n")
+               gotTestHash := 0
+               gotOther := 0
+               for _, locator := range locators {
+                       if locator == "" {
+                               continue
+                       }
+                       c.Check(locator[:len(spec.prefix)], Equals, spec.prefix)
+                       if locator[:32] == hash {
+                               gotTestHash++
+                       } else {
+                               gotOther++
+                       }
+               }
+               c.Check(gotTestHash == 2, Equals, spec.expectTestHash)
+               c.Check(gotOther > 0, Equals, spec.expectOther)
+       }
+
+       // GetIndex with invalid prefix
+       _, err = kc.GetIndex("proxy", "xyz")
+       c.Assert((err != nil), Equals, true)
+}
+
+func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
+       kc := runProxy(c, []string{"keepproxy"}, 28852, false)
+       waitForListener()
+       defer closeListener()
+
+       // Put a test block
+       hash, rep, err := kc.PutB([]byte("foo"))
+       c.Check(err, Equals, nil)
+       c.Check(rep, Equals, 2)
+
+       for _, token := range []string{
+               "nosuchtoken",
+               "2ym314ysp27sk7h943q6vtc378srb06se3pq6ghurylyf3pdmx", // expired
+       } {
+               // Change token to given bad token
+               kc.Arvados.ApiToken = token
+
+               // Ask should result in error
+               _, _, err = kc.Ask(hash)
+               c.Check(err, NotNil)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound.Temporary(), Equals, false)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
+
+               // Get should result in error
+               _, _, _, err = kc.Get(hash)
+               c.Check(err, NotNil)
+               errNotFound, _ = err.(keepclient.ErrNotFound)
+               c.Check(errNotFound.Temporary(), Equals, false)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403 \"Missing or invalid Authorization header\""), Equals, true)
        }
 }
+
+func (s *ServerRequiredSuite) TestPutAskGetConnectionError(c *C) {
+       arv, err := arvadosclient.MakeArvadosClient()
+       c.Assert(err, Equals, nil)
+
+       // keepclient with no such keep server
+       kc := keepclient.New(&arv)
+       locals := map[string]string{
+               "proxy": "http://localhost:12345",
+       }
+       kc.SetServiceRoots(locals, nil, nil)
+
+       // Ask should result in temporary connection refused error
+       hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
+       _, _, err = kc.Ask(hash)
+       c.Check(err, NotNil)
+       errNotFound, _ := err.(*keepclient.ErrNotFound)
+       c.Check(errNotFound.Temporary(), Equals, true)
+       c.Assert(strings.Contains(err.Error(), "connection refused"), Equals, true)
+
+       // Get should result in temporary connection refused error
+       _, _, _, err = kc.Get(hash)
+       c.Check(err, NotNil)
+       errNotFound, _ = err.(*keepclient.ErrNotFound)
+       c.Check(errNotFound.Temporary(), Equals, true)
+       c.Assert(strings.Contains(err.Error(), "connection refused"), Equals, true)
+}