X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/3c23a0a57bde2978402e5d46ca2b003becee6d58..570af13f6dfd7f725abf49335b7abdb46aae5048:/services/keepproxy/keepproxy_test.go

diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index dc70d968e2..c49fbe0bb3 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -13,16 +13,20 @@ import (
 	"math/rand"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"strings"
 	"sync"
 	"testing"
 	"time"
 
-	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
-	"git.curoverse.com/arvados.git/sdk/go/arvadostest"
-	"git.curoverse.com/arvados.git/sdk/go/keepclient"
+	"git.arvados.org/arvados.git/lib/config"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/arvadosclient"
+	"git.arvados.org/arvados.git/sdk/go/arvadostest"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
+	"git.arvados.org/arvados.git/sdk/go/keepclient"
+	log "github.com/sirupsen/logrus"
 
+	"gopkg.in/check.v1"
 	. "gopkg.in/check.v1"
 )
 
@@ -37,6 +41,12 @@ var _ = Suite(&ServerRequiredSuite{})
 // Tests that require the Keep server running
 type ServerRequiredSuite struct{}
 
+// Gocheck boilerplate
+var _ = Suite(&ServerRequiredConfigYmlSuite{})
+
+// Tests that require the Keep servers running as defined in config.yml
+type ServerRequiredConfigYmlSuite struct{}
+
 // Gocheck boilerplate
 var _ = Suite(&NoKeepServerSuite{})
 
@@ -80,6 +90,21 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
 	arvadostest.StopAPI()
 }
 
+func (s *ServerRequiredConfigYmlSuite) SetUpSuite(c *C) {
+	arvadostest.StartAPI()
+	// config.yml defines 4 keepstores
+	arvadostest.StartKeep(4, false)
+}
+
+func (s *ServerRequiredConfigYmlSuite) SetUpTest(c *C) {
+	arvadostest.ResetEnv()
+}
+
+func (s *ServerRequiredConfigYmlSuite) TearDownSuite(c *C) {
+	arvadostest.StopKeep(4)
+	arvadostest.StopAPI()
+}
+
 func (s *NoKeepServerSuite) SetUpSuite(c *C) {
 	arvadostest.StartAPI()
 	// We need API to have some keep services listed, but the
@@ -96,14 +121,35 @@ func (s *NoKeepServerSuite) TearDownSuite(c *C) {
 	arvadostest.StopAPI()
 }
 
-func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient {
-	args = append([]string{"keepproxy"}, args...)
-	os.Args = append(args, "-listen=:0")
+func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool, kp *arvados.UploadDownloadRolePermissions) (*keepclient.KeepClient, *bytes.Buffer) {
+	cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
+	c.Assert(err, Equals, nil)
+	cluster, err := cfg.GetCluster("")
+	c.Assert(err, Equals, nil)
+
+	if !loadKeepstoresFromConfig {
+		// Do not load Keepstore InternalURLs from the config file
+		cluster.Services.Keepstore.InternalURLs = make(map[arvados.URL]arvados.ServiceInstance)
+	}
+
+	cluster.Services.Keepproxy.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Host: ":0"}: {}}
+
+	if kp != nil {
+		cluster.Collections.KeepproxyPermission = *kp
+	}
+
 	listener = nil
-	go main()
+	logbuf := &bytes.Buffer{}
+	logger := log.New()
+	logger.Out = logbuf
+	go func() {
+		run(logger, cluster)
+		defer closeListener()
+	}()
 	waitForListener()
 
-	arv, err := arvadosclient.MakeArvadosClient()
+	client := arvados.NewClientFromEnv()
+	arv, err := arvadosclient.New(client)
 	c.Assert(err, Equals, nil)
 	if bogusClientToken {
 		arv.ApiToken = "bogus-token"
@@ -115,11 +161,11 @@ func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient
 	kc.SetServiceRoots(sr, sr, sr)
 	kc.Arvados.External = true
 
-	return kc
+	return kc, logbuf
 }
 
 func (s *ServerRequiredSuite) TestResponseViaHeader(c *C) {
-	runProxy(c, nil, false)
+	runProxy(c, false, false, nil)
 	defer closeListener()
 
 	req, err := http.NewRequest("POST",
@@ -130,6 +176,7 @@ func (s *ServerRequiredSuite) TestResponseViaHeader(c *C) {
 	resp, err := (&http.Client{}).Do(req)
 	c.Assert(err, Equals, nil)
 	c.Check(resp.Header.Get("Via"), Equals, "HTTP/1.1 keepproxy")
+	c.Assert(resp.StatusCode, Equals, http.StatusOK)
 	locator, err := ioutil.ReadAll(resp.Body)
 	c.Assert(err, Equals, nil)
 	resp.Body.Close()
@@ -145,7 +192,7 @@ func (s *ServerRequiredSuite) TestResponseViaHeader(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestLoopDetection(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	sr := map[string]string{
@@ -163,7 +210,7 @@ func (s *ServerRequiredSuite) TestLoopDetection(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestStorageClassesHeader(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	// Set up fake keepstore to record request headers
@@ -189,8 +236,30 @@ func (s *ServerRequiredSuite) TestStorageClassesHeader(c *C) {
 	c.Check(hdr.Get("X-Keep-Storage-Classes"), Equals, "secure")
 }
 
+func (s *ServerRequiredSuite) TestStorageClassesConfirmedHeader(c *C) {
+	runProxy(c, false, false, nil)
+	defer closeListener()
+
+	content := []byte("foo")
+	hash := fmt.Sprintf("%x", md5.Sum(content))
+	client := &http.Client{}
+
+	req, err := http.NewRequest("PUT",
+		fmt.Sprintf("http://%s/%s", listener.Addr().String(), hash),
+		bytes.NewReader(content))
+	c.Assert(err, IsNil)
+	req.Header.Set("X-Keep-Storage-Classes", "default")
+	req.Header.Set("Authorization", "OAuth2 "+arvadostest.ActiveToken)
+	req.Header.Set("Content-Type", "application/octet-stream")
+
+	resp, err := client.Do(req)
+	c.Assert(err, IsNil)
+	c.Assert(resp.StatusCode, Equals, http.StatusOK)
+	c.Assert(resp.Header.Get("X-Keep-Storage-Classes-Confirmed"), Equals, "default=2")
+}
+
 func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	content := []byte("TestDesiredReplicas")
@@ -207,7 +276,7 @@ func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	content := []byte("TestPutWrongContentLength")
@@ -218,7 +287,8 @@ func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
 	// fixes the invalid Content-Length header. In order to test
 	// our server behavior, we have to call the handler directly
 	// using an httptest.ResponseRecorder.
-	rtr := MakeRESTRouter(true, true, kc, 10*time.Second, "")
+	rtr, err := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{}, log.New())
+	c.Assert(err, check.IsNil)
 
 	type testcase struct {
 		sendLength   string
@@ -246,7 +316,7 @@ func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestManyFailedPuts(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 	router.(*proxyHandler).timeout = time.Nanosecond
 
@@ -273,7 +343,7 @@ func (s *ServerRequiredSuite) TestManyFailedPuts(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, logbuf := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
@@ -309,6 +379,9 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 		c.Check(rep, Equals, 2)
 		c.Check(err, Equals, nil)
 		c.Log("Finished PutB (expected success)")
+
+		c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+		logbuf.Reset()
 	}
 
 	{
@@ -316,6 +389,8 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 		c.Assert(err, Equals, nil)
 		c.Check(blocklen, Equals, int64(3))
 		c.Log("Finished Ask (expected success)")
+		c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+		logbuf.Reset()
 	}
 
 	{
@@ -326,6 +401,8 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 		c.Check(all, DeepEquals, []byte("foo"))
 		c.Check(blocklen, Equals, int64(3))
 		c.Log("Finished Get (expected success)")
+		c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+		logbuf.Reset()
 	}
 
 	{
@@ -350,7 +427,7 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
-	kc := runProxy(c, nil, true)
+	kc, _ := runProxy(c, true, false, nil)
 	defer closeListener()
 
 	hash := fmt.Sprintf("%x+3", md5.Sum([]byte("bar")))
@@ -365,69 +442,116 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
 
 	blocklen, _, err := kc.Ask(hash)
 	c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
-	c.Check(err, ErrorMatches, ".*not found.*")
+	c.Check(err, ErrorMatches, ".*HTTP 403.*")
 	c.Check(blocklen, Equals, int64(0))
 
 	_, blocklen, _, err = kc.Get(hash)
 	c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
-	c.Check(err, ErrorMatches, ".*not found.*")
+	c.Check(err, ErrorMatches, ".*HTTP 403.*")
 	c.Check(blocklen, Equals, int64(0))
-
 }
 
-func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
-	kc := runProxy(c, []string{"-no-get"}, false)
+func testPermission(c *C, admin bool, perm arvados.UploadDownloadPermission) {
+	kp := arvados.UploadDownloadRolePermissions{}
+	if admin {
+		kp.Admin = perm
+		kp.User = arvados.UploadDownloadPermission{Upload: true, Download: true}
+	} else {
+		kp.Admin = arvados.UploadDownloadPermission{Upload: true, Download: true}
+		kp.User = perm
+	}
+
+	kc, logbuf := runProxy(c, false, false, &kp)
 	defer closeListener()
+	if admin {
+		kc.Arvados.ApiToken = arvadostest.AdminToken
+	} else {
+		kc.Arvados.ApiToken = arvadostest.ActiveToken
+	}
 
-	hash := fmt.Sprintf("%x", md5.Sum([]byte("baz")))
+	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
+	var hash2 string
 
 	{
-		_, _, err := kc.Ask(hash)
-		errNotFound, _ := err.(keepclient.ErrNotFound)
-		c.Check(errNotFound, NotNil)
-		c.Assert(err, ErrorMatches, `.*HTTP 405.*`)
-		c.Log("Ask 1")
-	}
+		var rep int
+		var err error
+		hash2, rep, err = kc.PutB([]byte("foo"))
 
-	{
-		hash2, rep, err := kc.PutB([]byte("baz"))
-		c.Check(hash2, Matches, fmt.Sprintf(`^%s\+3(\+.+)?$`, hash))
-		c.Check(rep, Equals, 2)
-		c.Check(err, Equals, nil)
-		c.Log("PutB")
-	}
+		if perm.Upload {
+			c.Check(hash2, Matches, fmt.Sprintf(`^%s\+3(\+.+)?$`, hash))
+			c.Check(rep, Equals, 2)
+			c.Check(err, Equals, nil)
+			c.Log("Finished PutB (expected success)")
+			if admin {
+				c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+			} else {
 
-	{
-		blocklen, _, err := kc.Ask(hash)
-		errNotFound, _ := err.(keepclient.ErrNotFound)
-		c.Check(errNotFound, NotNil)
-		c.Assert(err, ErrorMatches, `.*HTTP 405.*`)
-		c.Check(blocklen, Equals, int64(0))
-		c.Log("Ask 2")
+				c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="Active User" user_uuid=zzzzz-tpzed-xurymjxw79nv3jz.*`)
+			}
+		} else {
+			c.Check(hash2, Equals, "")
+			c.Check(rep, Equals, 0)
+			c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
+		}
+		logbuf.Reset()
 	}
+	if perm.Upload {
+		// can't test download without upload.
 
-	{
-		_, blocklen, _, err := kc.Get(hash)
-		errNotFound, _ := err.(keepclient.ErrNotFound)
-		c.Check(errNotFound, NotNil)
-		c.Assert(err, ErrorMatches, `.*HTTP 405.*`)
-		c.Check(blocklen, Equals, int64(0))
-		c.Log("Get")
+		reader, blocklen, _, err := kc.Get(hash2)
+		if perm.Download {
+			c.Assert(err, Equals, nil)
+			all, err := ioutil.ReadAll(reader)
+			c.Check(err, IsNil)
+			c.Check(all, DeepEquals, []byte("foo"))
+			c.Check(blocklen, Equals, int64(3))
+			c.Log("Finished Get (expected success)")
+			if admin {
+				c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+			} else {
+				c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="Active User" user_uuid=zzzzz-tpzed-xurymjxw79nv3jz.*`)
+			}
+		} else {
+			c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+			c.Check(err, ErrorMatches, ".*Missing or invalid Authorization header, or method not allowed.*")
+			c.Check(blocklen, Equals, int64(0))
+		}
+		logbuf.Reset()
 	}
-}
 
-func (s *ServerRequiredSuite) TestPutDisabled(c *C) {
-	kc := runProxy(c, []string{"-no-put"}, false)
-	defer closeListener()
+}
 
-	hash2, rep, err := kc.PutB([]byte("quux"))
-	c.Check(hash2, Equals, "")
-	c.Check(rep, Equals, 0)
-	c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
+func (s *ServerRequiredSuite) TestPutGetPermission(c *C) {
+
+	for _, adminperm := range []bool{true, false} {
+		for _, userperm := range []bool{true, false} {
+
+			testPermission(c, true,
+				arvados.UploadDownloadPermission{
+					Upload:   adminperm,
+					Download: true,
+				})
+			testPermission(c, true,
+				arvados.UploadDownloadPermission{
+					Upload:   true,
+					Download: adminperm,
+				})
+			testPermission(c, false,
+				arvados.UploadDownloadPermission{
+					Upload:   true,
+					Download: userperm,
+				})
+			testPermission(c, false,
+				arvados.UploadDownloadPermission{
+					Upload:   true,
+					Download: userperm,
+				})
+		}
+	}
 }
 
 func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
-	runProxy(c, nil, false)
+	runProxy(c, false, false, nil)
 	defer closeListener()
 
 	{
@@ -458,7 +582,7 @@ func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPostWithoutHash(c *C) {
-	runProxy(c, nil, false)
+	runProxy(c, false, false, nil)
 	defer closeListener()
 
 	{
@@ -501,7 +625,22 @@ func (s *ServerRequiredSuite) TestStripHint(c *C) {
 //   With a valid but non-existing prefix (expect "\n")
 //   With an invalid prefix (expect error)
 func (s *ServerRequiredSuite) TestGetIndex(c *C) {
-	kc := runProxy(c, nil, false)
+	getIndexWorker(c, false)
+}
+
+// Test GetIndex
+//   Uses config.yml
+//   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 *ServerRequiredConfigYmlSuite) TestGetIndex(c *C) {
+	getIndexWorker(c, true)
+}
+
+func getIndexWorker(c *C, useConfig bool) {
+	kc, _ := runProxy(c, false, useConfig, nil)
 	defer closeListener()
 
 	// Put "index-data" blocks
@@ -524,7 +663,7 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
 	_, _, err = kc.PutB([]byte("some-more-index-data"))
 	c.Check(err, IsNil)
 
-	kc.Arvados.ApiToken = arvadostest.DataManagerToken
+	kc.Arvados.ApiToken = arvadostest.SystemRootToken
 
 	// Invoke GetIndex
 	for _, spec := range []struct {
@@ -564,7 +703,7 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 	hash, _, err := kc.PutB([]byte("shareddata"))
 	c.Check(err, IsNil)
@@ -577,7 +716,7 @@ func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	// Put a test block
@@ -605,16 +744,16 @@ func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
 			_, _, _, err = kc.Get(hash)
 			c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
 			c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
-			c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header\".*")
+			c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header, or method not allowed\".*")
 		}
 
 		_, _, err = kc.PutB([]byte("foo"))
-		c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header")
+		c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header, or method not allowed")
 	}
 }
 
 func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	// Point keepproxy at a non-existent keepstore
@@ -640,7 +779,7 @@ func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
 }
 
 func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
 	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
@@ -663,10 +802,11 @@ func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
 }
 
 func (s *ServerRequiredSuite) TestPing(c *C) {
-	kc := runProxy(c, nil, false)
+	kc, _ := runProxy(c, false, false, nil)
 	defer closeListener()
 
-	rtr := MakeRESTRouter(true, true, kc, 10*time.Second, arvadostest.ManagementToken)
+	rtr, err := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{ManagementToken: arvadostest.ManagementToken}, log.New())
+	c.Assert(err, check.IsNil)
 
 	req, err := http.NewRequest("GET",
 		"http://"+listener.Addr().String()+"/_health/ping",