[ARVADOS] updated: 2.1.0-895-g0da30aa9a

Git user git at public.arvados.org
Tue Jun 15 20:59:56 UTC 2021


Summary of changes:
 sdk/go/arvadostest/fixtures.go    |   1 +
 services/keep-web/handler_test.go | 167 ++++++++++++--------------------------
 2 files changed, 55 insertions(+), 113 deletions(-)

       via  0da30aa9a699faa5e735c3aca2f2d6cd96429944 (commit)
       via  8697aedb4507a98ca5c459321271c2c004b38b5f (commit)
      from  0698f0ee6f2748016688b81e7bd53c8efa200648 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 0da30aa9a699faa5e735c3aca2f2d6cd96429944
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Jun 15 16:59:40 2021 -0400

    17464: Refactor tests and check that log events are posted
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index 16e209816..7829f0cf4 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -10,6 +10,7 @@ const (
 	ActiveToken             = "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
 	ActiveTokenUUID         = "zzzzz-gj3su-077z32aux8dg2s1"
 	ActiveTokenV2           = "v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
+	AdminUserUUID           = "zzzzz-tpzed-d9tiejq69daie8f"
 	AdminToken              = "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h"
 	AdminTokenUUID          = "zzzzz-gj3su-027z32aux8dg2s1"
 	AnonymousToken          = "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi"
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 9738c3ee9..31724dd5d 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1184,18 +1184,21 @@ func copyHeader(h http.Header) http.Header {
 	return hc
 }
 
-func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
-	h := handler{Config: newConfig(s.ArvConfig)}
-	u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
-	req := &http.Request{
-		Method:     "GET",
-		Host:       u.Host,
-		URL:        u,
-		RequestURI: u.RequestURI(),
-		Header: http.Header{
-			"Authorization": {"Bearer " + arvadostest.ActiveToken},
-		},
-	}
+func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, req *http.Request,
+	successCode int, direction string, perm bool, userUuid string, collectionUuid string, filepath string) {
+
+	client := s.testServer.Config.Client
+	client.AuthToken = arvadostest.AdminToken
+	var logentries arvados.LogList
+	limit1 := 1
+	err := client.RequestAndDecode(&logentries, "GET", "arvados/v1/logs", nil,
+		arvados.ResourceListParams{
+			Limit: &limit1,
+			Order: "created_at desc"})
+	c.Check(err, check.IsNil)
+	c.Check(logentries.Items, check.HasLen, 1)
+	lastLogId := logentries.Items[0].ID
+	nextLogId := lastLogId
 
 	var logbuf bytes.Buffer
 	logger := logrus.New()
@@ -1204,49 +1207,41 @@ func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
 	req = req.WithContext(ctxlog.Context(context.Background(), logger))
 	h.ServeHTTP(resp, req)
 
-	c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
-	c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-}
-
-func (s *IntegrationSuite) TestUploadLogging(c *check.C) {
-	defer func() {
-		client := s.testServer.Config.Client
-		client.AuthToken = arvadostest.AdminToken
-		client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
-	}()
+	if perm {
+		c.Check(resp.Result().StatusCode, check.Equals, successCode)
+		c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File `+direction+`".*`)
+		c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+
+		for nextLogId == lastLogId {
+			time.Sleep(50 * time.Millisecond)
+			err = client.RequestAndDecode(&logentries, "GET", "arvados/v1/logs", nil,
+				arvados.ResourceListParams{
+					Filters: []arvados.Filter{arvados.Filter{Attr: "event_type", Operator: "=", Operand: "file_" + direction}},
+					Limit:   &limit1,
+					Order:   "created_at desc",
+				})
+			c.Check(err, check.IsNil)
+			if len(logentries.Items) > 0 {
+				nextLogId = logentries.Items[0].ID
+			}
+		}
 
-	h := handler{Config: newConfig(s.ArvConfig)}
-	u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/bar")
-	req := &http.Request{
-		Method:     "PUT",
-		Host:       u.Host,
-		URL:        u,
-		RequestURI: u.RequestURI(),
-		Header: http.Header{
-			"Authorization": {"Bearer " + arvadostest.ActiveToken},
-		},
-		Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
+		c.Check(logentries.Items[0].ObjectUUID, check.Equals, userUuid)
+		c.Check(logentries.Items[0].Properties["collection_uuid"], check.Equals, collectionUuid)
+		c.Check(logentries.Items[0].Properties["collection_file_path"], check.Equals, filepath)
+	} else {
+		c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+		c.Check(logbuf.String(), check.Equals, "")
 	}
-
-	var logbuf bytes.Buffer
-	logger := logrus.New()
-	logger.Out = &logbuf
-	resp := httptest.NewRecorder()
-	req = req.WithContext(ctxlog.Context(context.Background(), logger))
-	h.ServeHTTP(resp, req)
-
-	c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
-	c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
 }
 
-func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
+func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) {
 	config := newConfig(s.ArvConfig)
 	h := handler{Config: config}
 	u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
 
 	for _, adminperm := range []bool{true, false} {
 		for _, userperm := range []bool{true, false} {
-
 			config.cluster.Collections.KeepWebPermission.Admin.Download = adminperm
 			config.cluster.Collections.KeepWebPermission.User.Download = userperm
 
@@ -1260,22 +1255,8 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
 					"Authorization": {"Bearer " + arvadostest.AdminToken},
 				},
 			}
-
-			var logbuf bytes.Buffer
-			logger := logrus.New()
-			logger.Out = &logbuf
-			resp := httptest.NewRecorder()
-			req = req.WithContext(ctxlog.Context(context.Background(), logger))
-			h.ServeHTTP(resp, req)
-
-			if adminperm {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
-				c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
-				c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-			} else {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
-				c.Check(logbuf.String(), check.Equals, "")
-			}
+			s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", adminperm,
+				arvadostest.AdminUserUUID, arvadostest.FooCollection, "foo")
 
 			// Test user permission
 			req = &http.Request{
@@ -1287,27 +1268,13 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
 					"Authorization": {"Bearer " + arvadostest.ActiveToken},
 				},
 			}
-
-			logbuf = bytes.Buffer{}
-			logger = logrus.New()
-			logger.Out = &logbuf
-			resp = httptest.NewRecorder()
-			req = req.WithContext(ctxlog.Context(context.Background(), logger))
-			h.ServeHTTP(resp, req)
-
-			if userperm {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
-				c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
-				c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-			} else {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
-				c.Check(logbuf.String(), check.Equals, "")
-			}
+			s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", userperm,
+				arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo")
 		}
 	}
 }
 
-func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
+func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) {
 	defer func() {
 		client := s.testServer.Config.Client
 		client.AuthToken = arvadostest.AdminToken
@@ -1316,11 +1283,10 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
 
 	config := newConfig(s.ArvConfig)
 	h := handler{Config: config}
-	u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+	u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/bar")
 
 	for _, adminperm := range []bool{true, false} {
 		for _, userperm := range []bool{true, false} {
-
 			config.cluster.Collections.KeepWebPermission.Admin.Upload = adminperm
 			config.cluster.Collections.KeepWebPermission.User.Upload = userperm
 
@@ -1335,22 +1301,8 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
 				},
 				Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
 			}
-
-			var logbuf bytes.Buffer
-			logger := logrus.New()
-			logger.Out = &logbuf
-			resp := httptest.NewRecorder()
-			req = req.WithContext(ctxlog.Context(context.Background(), logger))
-			h.ServeHTTP(resp, req)
-
-			if adminperm {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
-				c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
-				c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-			} else {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
-				c.Check(logbuf.String(), check.Equals, "")
-			}
+			s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", adminperm,
+				arvadostest.AdminUserUUID, arvadostest.FooCollection, "bar")
 
 			// Test user permission
 			req = &http.Request{
@@ -1363,22 +1315,8 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
 				},
 				Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
 			}
-
-			logbuf = bytes.Buffer{}
-			logger = logrus.New()
-			logger.Out = &logbuf
-			resp = httptest.NewRecorder()
-			req = req.WithContext(ctxlog.Context(context.Background(), logger))
-			h.ServeHTTP(resp, req)
-
-			if userperm {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
-				c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
-				c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-			} else {
-				c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
-				c.Check(logbuf.String(), check.Equals, "")
-			}
+			s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", userperm,
+				arvadostest.ActiveUserUUID, arvadostest.FooCollection, "bar")
 		}
 	}
 }

commit 8697aedb4507a98ca5c459321271c2c004b38b5f
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Jun 15 14:54:16 2021 -0400

    17464: Reset database after upload tests
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index ad674e4d1..9738c3ee9 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -89,8 +89,9 @@ func (s *UnitSuite) TestEmptyResponse(c *check.C) {
 
 		// If we return no content because the client sent an
 		// If-Modified-Since header, our response should be
-		// 304, and we should not emit a log message.
-		{true, true, http.StatusNotModified, ``},
+		// 304.  We still expect a "File download" log since it
+		// counts as a file access for auditing.
+		{true, true, http.StatusNotModified, `(?ms).*msg="File download".*`},
 	} {
 		c.Logf("trial: %+v", trial)
 		arvadostest.StartKeep(2, true)
@@ -1210,6 +1211,7 @@ func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
 func (s *IntegrationSuite) TestUploadLogging(c *check.C) {
 	defer func() {
 		client := s.testServer.Config.Client
+		client.AuthToken = arvadostest.AdminToken
 		client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
 	}()
 
@@ -1308,6 +1310,7 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
 func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
 	defer func() {
 		client := s.testServer.Config.Client
+		client.AuthToken = arvadostest.AdminToken
 		client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
 	}()
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list