[arvados] created: 2.6.0-566-g8aeb3c81d6

[git repository hosting]

        at  8aeb3c81d60d665a1ab83684c1615b003c1ebbca (commit)


commit 8aeb3c81d60d665a1ab83684c1615b003c1ebbca
Author: Tom Clegg <tom at curii.com>
Date:   Fri Sep 8 16:06:57 2023 -0400

    20930: Fix passing arvados.Client (with a sync.Mutex) by value.
    
    Copying could occur while the original arvados.Client was being used
    to fetch the discovery doc and had its Mutex locked.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/ws/permission.go b/services/ws/permission.go
index 78158b12a1..f71ca61167 100644
--- a/services/ws/permission.go
+++ b/services/ws/permission.go
@@ -24,10 +24,10 @@ type permChecker interface {
 	Check(ctx context.Context, uuid string) (bool, error)
 }
 
-func newPermChecker(ac arvados.Client) permChecker {
-	ac.AuthToken = ""
+func newPermChecker(ac *arvados.Client) permChecker {
 	return &cachingPermChecker{
-		Client:     &ac,
+		ac:         ac,
+		token:      "-",
 		cache:      make(map[string]cacheEnt),
 		maxCurrent: 16,
 	}
@@ -39,7 +39,8 @@ type cacheEnt struct {
 }
 
 type cachingPermChecker struct {
-	*arvados.Client
+	ac         *arvados.Client
+	token      string
 	cache      map[string]cacheEnt
 	maxCurrent int
 
@@ -49,17 +50,17 @@ type cachingPermChecker struct {
 }
 
 func (pc *cachingPermChecker) SetToken(token string) {
-	if pc.Client.AuthToken == token {
+	if pc.token == token {
 		return
 	}
-	pc.Client.AuthToken = token
+	pc.token = token
 	pc.cache = make(map[string]cacheEnt)
 }
 
 func (pc *cachingPermChecker) Check(ctx context.Context, uuid string) (bool, error) {
 	pc.nChecks++
 	logger := ctxlog.FromContext(ctx).
-		WithField("token", pc.Client.AuthToken).
+		WithField("token", pc.token).
 		WithField("uuid", uuid)
 	pc.tidy()
 	now := time.Now()
@@ -67,17 +68,19 @@ func (pc *cachingPermChecker) Check(ctx context.Context, uuid string) (bool, err
 		logger.WithField("allowed", perm.allowed).Debug("cache hit")
 		return perm.allowed, nil
 	}
-	var buf map[string]interface{}
-	path, err := pc.PathForUUID("get", uuid)
+
+	path, err := pc.ac.PathForUUID("get", uuid)
 	if err != nil {
 		pc.nInvalid++
 		return false, err
 	}
 
 	pc.nMisses++
-	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
+	ctx = arvados.ContextWithAuthorization(ctx, "Bearer "+pc.token)
+	ctx, cancel := context.WithDeadline(ctx, time.Now().Add(time.Minute))
 	defer cancel()
-	err = pc.RequestAndDecodeContext(ctx, &buf, "GET", path, nil, url.Values{
+	var buf map[string]interface{}
+	err = pc.ac.RequestAndDecodeContext(ctx, &buf, "GET", path, nil, url.Values{
 		"include_trash": {"true"},
 		"select":        {`["uuid"]`},
 	})
@@ -88,6 +91,9 @@ func (pc *cachingPermChecker) Check(ctx context.Context, uuid string) (bool, err
 	} else if txErr, ok := err.(*arvados.TransactionError); ok && pc.isNotAllowed(txErr.StatusCode) {
 		allowed = false
 	} else {
+		// If "context deadline exceeded", "client
+		// disconnected", HTTP 5xx, network error, etc., don't
+		// cache the result.
 		logger.WithError(err).Error("lookup error")
 		return false, err
 	}
diff --git a/services/ws/permission_test.go b/services/ws/permission_test.go
index 5e90881a03..2a22eae609 100644
--- a/services/ws/permission_test.go
+++ b/services/ws/permission_test.go
@@ -21,7 +21,7 @@ func (s *permSuite) TestCheck(c *check.C) {
 	// Disable auto-retry
 	client.Timeout = 0
 
-	pc := newPermChecker(*client).(*cachingPermChecker)
+	pc := newPermChecker(client).(*cachingPermChecker)
 	setToken := func(label, token string) {
 		c.Logf("...%s token %q", label, token)
 		pc.SetToken(token)
@@ -73,7 +73,7 @@ func (s *permSuite) TestCheck(c *check.C) {
 	pc.SetToken(arvadostest.ActiveToken)
 
 	c.Log("...network error")
-	pc.Client.APIHost = "127.0.0.1:9"
+	pc.ac.APIHost = "127.0.0.1:9"
 	wantError(arvadostest.UserAgreementCollection)
 	wantError(arvadostest.FooBarDirCollection)
 
diff --git a/services/ws/service.go b/services/ws/service.go
index d6501e0771..9a4a239ea1 100644
--- a/services/ws/service.go
+++ b/services/ws/service.go
@@ -47,7 +47,7 @@ func newHandler(ctx context.Context, cluster *arvados.Cluster, token string, reg
 		cluster:        cluster,
 		client:         client,
 		eventSource:    eventSource,
-		newPermChecker: func() permChecker { return newPermChecker(*client) },
+		newPermChecker: func() permChecker { return newPermChecker(client) },
 		done:           done,
 		reg:            reg,
 	}

commit 53592764d7a3299857adb6db6cad2eba29788428
Author: Tom Clegg <tom at curii.com>
Date:   Fri Sep 8 16:31:39 2023 -0400

    20930: Fix unhelpful test failure output.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/ws/session_v0_test.go b/services/ws/session_v0_test.go
index d7c9edb24f..bec05474e7 100644
--- a/services/ws/session_v0_test.go
+++ b/services/ws/session_v0_test.go
@@ -339,14 +339,16 @@ func (s *v0Suite) expectLog(c *check.C, r *json.Decoder) *arvados.Log {
 	lg := &arvados.Log{}
 	ok := make(chan struct{})
 	go func() {
+		defer close(ok)
 		for lg.ID <= s.ignoreLogID {
-			c.Check(r.Decode(lg), check.IsNil)
+			c.Assert(r.Decode(lg), check.IsNil)
 		}
-		close(ok)
 	}()
 	select {
 	case <-time.After(10 * time.Second):
-		panic("timed out")
+		c.Error("timed out")
+		c.FailNow()
+		return lg
 	case <-ok:
 		return lg
 	}

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


hooks/post-receive
--