From 54f974d44c3d740bf1a41624d5a001952561c3d2 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 26 Aug 2016 12:16:17 -0700 Subject: [PATCH] make IServer.get_serverid() use pubkey, not tubid This is a change I've wanted to make for many years, because when we get to HTTP-based servers, we won't have tubids for them. What held me back was that there's code all over the place that uses the serverid for various purposes, so I wasn't sure it was safe. I did a big push a few years ago to use IServer instances instead of serverids in most places (in #1363), and to split out the values that actually depend upon tubid into separate accessors (like get_lease_seed and get_foolscap_write_enabler_seed), which I think took care of all the important uses. There are a number of places that use get_serverid() as dictionary key to track shares (Checker results, mutable servermap). I believe these are happy to use pubkeys instead of tubids: the only thing they do with get_serverid() is to compare it to other values obtained from get_serverid(). A few places in the WUI used serverid to compute display values: these were fixed. The main trouble was the Helper: it returns a HelperUploadResults (a Copyable) with a share->server mapping that's keyed by whatever the Helper's get_serverid() returns. If the uploader and the helper are on different sides of this change, the Helper could return values that the uploader won't recognize. This is cosmetic: that mapping is only used to display the upload results on the "Recent and Active Operations" page. I've added code to StorageFarmBroker.get_stub_server() to fall back to tubids when looking up a server, so this should still work correctly when the uploader is new and the Helper is old. If the Helper is new and the uploader is old, the upload results will show unusual server ids. refs ticket:1363 --- src/allmydata/immutable/upload.py | 3 +++ src/allmydata/storage_client.py | 22 ++++++++++++++-------- src/allmydata/test/test_storage_client.py | 4 ++-- src/allmydata/test/test_system.py | 2 +- src/allmydata/web/root.py | 4 ++-- src/allmydata/web/status.py | 6 +++--- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/allmydata/immutable/upload.py b/src/allmydata/immutable/upload.py index 99168f4e0..d7a7365a6 100644 --- a/src/allmydata/immutable/upload.py +++ b/src/allmydata/immutable/upload.py @@ -1323,6 +1323,9 @@ class AssistedUploader: now = time.time() timings["total"] = now - self._started + # Note: older Helpers (<=1.11) sent tubids as serverids. Newer ones + # send pubkeys. get_stub_server() knows how to map both into + # IDisplayableServer instances. gss = self._storage_broker.get_stub_server sharemap = {} servermap = {} diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 3089c303d..a1285a099 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -42,12 +42,6 @@ from allmydata.util.observer import ObserverList from allmydata.util.rrefutil import add_version_to_remote_reference from allmydata.util.hashutil import sha1 -def get_serverid_from_furl(furl): - m = re.match(r'pb://(\w+)@', furl) - assert m, furl - id = m.group(1).lower() - return base32.a2b(id) - # who is responsible for de-duplication? # both? # IC remembers the unpacked announcements it receives, to provide for late @@ -135,7 +129,7 @@ class StorageFarmBroker(service.MultiService): self._threshold_listeners = remaining def got_static_announcement(self, key_s, ann, handlers): - server_id = get_serverid_from_furl(ann["anonymous-storage-FURL"]) + server_id = key_s assert server_id not in self.static_servers # XXX self.static_servers.append(server_id) self._got_announcement(key_s, ann, handlers=handlers) @@ -216,6 +210,18 @@ class StorageFarmBroker(service.MultiService): def get_stub_server(self, serverid): if serverid in self.servers: return self.servers[serverid] + # some time before 1.12, we changed "serverid" to be "key_s" (the + # printable verifying key, used in V2 announcements), instead of the + # tubid. When the immutable uploader delegates work to a Helper, + # get_stub_server() is used to map the returning server identifiers + # to IDisplayableServer instances (to get a name, for display on the + # Upload Results web page). If the Helper is running 1.12 or newer, + # it will send pubkeys, but if it's still running 1.11, it will send + # tubids. This clause maps the old tubids to our existing servers. + for s in self.servers.values(): + if isinstance(s, NativeStorageServer): + if serverid == s._tubid: + return s return StubServer(serverid) class StubServer: @@ -315,7 +321,7 @@ class NativeStorageServer(service.MultiService): def __repr__(self): return "" % self.get_name() def get_serverid(self): - return self._tubid # XXX replace with self.key_s + return self.key_s def get_permutation_seed(self): return self._permutation_seed def get_version(self): diff --git a/src/allmydata/test/test_storage_client.py b/src/allmydata/test/test_storage_client.py index 1b7926837..21eb3ac1c 100644 --- a/src/allmydata/test/test_storage_client.py +++ b/src/allmydata/test/test_storage_client.py @@ -49,8 +49,8 @@ class TestStorageFarmBroker(unittest.TestCase): } broker.got_static_announcement(key_s, ann, None) self.failUnlessEqual(len(broker.static_servers), 1) - self.failUnlessEqual(broker.servers['1'].announcement, ann) - self.failUnlessEqual(broker.servers['1'].key_s, key_s) + self.failUnlessEqual(broker.servers[key_s].announcement, ann) + self.failUnlessEqual(broker.servers[key_s].key_s, key_s) @inlineCallbacks def test_threshold_reached(self): diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index ddfb0524e..95e1901f3 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -2437,7 +2437,7 @@ class Connections(SystemTestMixin, unittest.TestCase): def _start(ign): self.c0 = self.clients[0] nonclients = [s for s in self.c0.storage_broker.get_connected_servers() - if s.get_serverid() != self.c0.nodeid] + if s.get_serverid() != self.c0.get_long_nodeid()] self.failUnlessEqual(len(nonclients), 1) self.s1 = nonclients[0] # s1 is the server, not c0 diff --git a/src/allmydata/web/root.py b/src/allmydata/web/root.py index 451b669a2..8b5f196a3 100644 --- a/src/allmydata/web/root.py +++ b/src/allmydata/web/root.py @@ -300,13 +300,13 @@ class Root(rend.Page): return sorted(sb.get_known_servers(), key=lambda s: s.get_serverid()) def render_service_row(self, ctx, server): - nodeid = server.get_serverid() + server_id = server.get_serverid() ctx.fillSlots("peerid", server.get_longname()) ctx.fillSlots("nickname", server.get_nickname()) rhost = server.get_remote_host() if server.is_connected(): - if nodeid == self.client.nodeid: + if server_id == self.client.get_long_nodeid(): rhost_s = "(loopback)" elif isinstance(rhost, address.IPv4Address): rhost_s = "%s:%d" % (rhost.host, rhost.port) diff --git a/src/allmydata/web/status.py b/src/allmydata/web/status.py index 4064e2820..49ea49a95 100644 --- a/src/allmydata/web/status.py +++ b/src/allmydata/web/status.py @@ -1,5 +1,5 @@ -import pprint, itertools +import pprint, itertools, hashlib import simplejson from twisted.internet import defer from nevow import rend, inevow, tags as T @@ -597,10 +597,10 @@ class DownloadStatusPage(DownloadResultsRendererMixin, rend.Page): return l def color(self, server): - peerid = server.get_serverid() # binary + h = hashlib.sha256(server.get_serverid()).digest() def m(c): return min(ord(c) / 2 + 0x80, 0xff) - return "#%02x%02x%02x" % (m(peerid[0]), m(peerid[1]), m(peerid[2])) + return "#%02x%02x%02x" % (m(h[0]), m(h[1]), m(h[2])) def render_results(self, ctx, data): d = self.download_results()