[ARVADOS] updated: 1.3.0-2135-gd8861941e

[Git user]

Summary of changes:
 lib/controller/federation/conn.go | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

       via  d8861941ef704dd729252ee1592e48a082798b65 (commit)
      from  14bb7aa24ed7a06ce94e59f64ab32bdb44641168 (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 d8861941ef704dd729252ee1592e48a082798b65
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Thu Feb 6 14:16:55 2020 -0500

    16133: Don't take multiple federation hops.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 2aebc0e97..42083cb83 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -120,8 +120,13 @@ func (conn *Conn) chooseBackend(id string) backend {
 // or "" for the local backend.
 //
 // A non-nil error means all backends failed.
-func (conn *Conn) tryLocalThenRemotes(ctx context.Context, fn func(context.Context, string, backend) error) error {
-	if err := fn(ctx, "", conn.local); err == nil || errStatus(err) != http.StatusNotFound {
+func (conn *Conn) tryLocalThenRemotes(ctx context.Context, forwardedFor string, fn func(context.Context, string, backend) error) error {
+	if err := fn(ctx, "", conn.local); err == nil || errStatus(err) != http.StatusNotFound || forwardedFor != "" {
+		// Note: forwardedFor != "" means this request came
+		// from a remote cluster, so we don't take a second
+		// hop. This avoids cycles, redundant calls to a
+		// mutually reachable remote, and use of double-salted
+		// tokens.
 		return err
 	}
 
@@ -218,8 +223,6 @@ func (conn *Conn) Login(ctx context.Context, options arvados.LoginOptions) (arva
 }
 
 func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions) (arvados.Collection, error) {
-	downstream := options.ForwardedFor
-	options.ForwardedFor = conn.cluster.ClusterID + "-" + downstream
 	if len(options.UUID) == 27 {
 		// UUID is really a UUID
 		c, err := conn.chooseBackend(options.UUID).CollectionGet(ctx, options)
@@ -230,17 +233,10 @@ func (conn *Conn) CollectionGet(ctx context.Context, options arvados.GetOptions)
 	} else {
 		// UUID is a PDH
 		first := make(chan arvados.Collection, 1)
-		err := conn.tryLocalThenRemotes(ctx, func(ctx context.Context, remoteID string, be backend) error {
-			if remoteID != "" && downstream != "" {
-				// If remoteID isn't in downstream, we
-				// might find the collection by taking
-				// another hop, but we don't bother:
-				// token salting and blob signature
-				// rewriting don't work over multiple
-				// hops.
-				return notFoundError{}
-			}
-			c, err := be.CollectionGet(ctx, options)
+		err := conn.tryLocalThenRemotes(ctx, options.ForwardedFor, func(ctx context.Context, remoteID string, be backend) error {
+			remoteOpts := options
+			remoteOpts.ForwardedFor = conn.cluster.ClusterID + "-" + options.ForwardedFor
+			c, err := be.CollectionGet(ctx, remoteOpts)
 			if err != nil {
 				return err
 			}

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


hooks/post-receive
--