diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index aa11097ae705904c287babd26905ab05e6b52bf1..a2c8e1f596fdab64f01706a131b9973cf50a3947 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -32,7 +32,7 @@ from typing import Dict, List, Optional import attr from allmydata.interfaces import RIStorageServer, TestAndWriteVectorsForShares from allmydata.storage.common import storage_index_to_dir -from allmydata.storage.immutable import ShareFile +from allmydata.storage.immutable import ShareFile, FoolscapBucketWriter from allmydata.storage.lease import LeaseInfo from allmydata.storage.mutable import MutableShareFile from allmydata.storage.server import StorageServer @@ -253,7 +253,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): Pass-through without pass check to allow clients to learn about our version and configuration in case it helps them decide how to behave. """ - return self._original.remote_get_version() + return self._original._server.get_version() def remote_allocate_buckets( self, @@ -302,7 +302,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): allocated_size, ) - alreadygot, bucketwriters = self._original._allocate_buckets( + alreadygot, bucketwriters = self._original._server.allocate_buckets( storage_index, renew_secret, cancel_secret, @@ -338,7 +338,10 @@ class ZKAPAuthorizerStorageServer(Referenceable): self._public_key, validation.valid[:spent_passes], ) - return alreadygot, bucketwriters + return alreadygot, { + k: FoolscapBucketWriter(bw) + for (k, bw) in bucketwriters.items() + } def remote_get_buckets(self, storage_index): """ @@ -361,9 +364,9 @@ class ZKAPAuthorizerStorageServer(Referenceable): self._pass_value, storage_index, validation, - self._original, + self._original._server, ) - result = self._original.remote_add_lease(storage_index, *a, **kw) + result = self._original._server.add_lease(storage_index, *a, **kw) self._spender.mark_as_spent( self._public_key, validation.valid, @@ -376,7 +379,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): Pass-through without a pass check to let clients inform us of possible issues with the system without incurring any cost to themselves. """ - return self._original.remote_advise_corrupt_share(*a, **kw) + return self._original._server.advise_corrupt_share(*a, **kw) def remote_share_sizes(self, storage_index_or_slot, sharenums): with start_action( @@ -384,13 +387,13 @@ class ZKAPAuthorizerStorageServer(Referenceable): storage_index_or_slot=storage_index_or_slot, ): return dict( - get_share_sizes(self._original, storage_index_or_slot, sharenums) + get_share_sizes(self._original._server, storage_index_or_slot, sharenums) ) def remote_stat_shares(self, storage_indexes_or_slots): # type: (List[bytes]) -> List[Dict[int, ShareStat]] return list( - dict(get_share_stats(self._original, storage_index_or_slot, None)) + dict(get_share_stats(self._original._server, storage_index_or_slot, None)) for storage_index_or_slot in storage_indexes_or_slots ) @@ -469,7 +472,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): # Inspect the operation to determine its price based on any # allocations. required_new_passes = get_writev_price( - self._original, + self._original._server, self._pass_value, storage_index, tw_vectors, @@ -482,7 +485,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): validation.raise_for(required_new_passes) # Perform the operation. - result = self._original.slot_testv_and_readv_and_writev( + result = self._original._server.slot_testv_and_readv_and_writev( storage_index, secrets, tw_vectors, @@ -503,7 +506,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): # difference but this only grants storage for the remainder of the # existing lease period. This results in the client being overcharged # somewhat. - add_leases_for_writev(self._original, storage_index, secrets, tw_vectors, now) + add_leases_for_writev(self._original._server, storage_index, secrets, tw_vectors, now) self._spender.mark_as_spent( self._public_key, @@ -521,7 +524,7 @@ class ZKAPAuthorizerStorageServer(Referenceable): Pass-through without a pass check to let clients read mutable shares as long as those shares exist. """ - return self._original.remote_slot_readv(*a, **kw) + return self._original._server.slot_readv(*a, **kw) def check_pass_quantity(pass_value, validation, share_sizes): diff --git a/src/_zkapauthorizer/tests/fixtures.py b/src/_zkapauthorizer/tests/fixtures.py index 54d27b96c42014149e21d9cdcb83a6376d4df7f4..aa967e27fdcf791a14bd249f4c40af82634cfe50 100644 --- a/src/_zkapauthorizer/tests/fixtures.py +++ b/src/_zkapauthorizer/tests/fixtures.py @@ -19,7 +19,7 @@ Common fixtures to let the test suite focus on application logic. from base64 import b64encode import attr -from allmydata.storage.server import StorageServer +from allmydata.storage.server import StorageServer, FoolscapStorageServer from fixtures import Fixture, TempDir from twisted.internet.task import Clock from twisted.python.filepath import FilePath @@ -28,31 +28,39 @@ from ..controller import DummyRedeemer, PaymentController from ..model import VoucherStore, memory_connect, open_and_initialize -@attr.s +@attr.s(auto_attribs=True) class AnonymousStorageServer(Fixture): """ Supply an instance of allmydata.storage.server.StorageServer which implements anonymous access to Tahoe-LAFS storage server functionality. - :ivar FilePath tempdir: The path to the server's storage on the - filesystem. + :ivar tempdir: The path to the server's storage on the filesystem. + + :ivar backend: The protocol-agnostic storage server backend. - :ivar allmydata.storage.server.StorageServer storage_server: The storage - server. + :ivar anonymous_foolscap_server: The Foolscap-based server wrapped around + the backend. - :ivar twisted.internet.task.Clock clock: The ``IReactorTime`` provider to - supply to ``StorageServer`` for its time-checking needs. + :ivar clock: The ``IReactorTime`` provider to supply to ``StorageServer`` + for its time-checking needs. """ - clock = attr.ib() + clock: Clock = attr.ib() + + tempdir: FilePath = attr.ib(default=None) + backend: StorageServer = attr.ib(default=None) + anonymous_foolscap_server: FoolscapStorageServer = attr.ib(default=None) def _setUp(self): self.tempdir = FilePath(self.useFixture(TempDir()).join(u"storage")) - self.storage_server = StorageServer( + self.backend = StorageServer( self.tempdir.path, b"x" * 20, clock=self.clock, ) + self.anonymous_foolscap_server = FoolscapStorageServer( + self.backend, + ) @attr.s diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py index ceb34deb3d1143a55f8072de31f658992ccce226..b6a8c4dace48c96dedaf29c6de6da9d8c6e898aa 100644 --- a/src/_zkapauthorizer/tests/test_storage_protocol.py +++ b/src/_zkapauthorizer/tests/test_storage_protocol.py @@ -154,13 +154,13 @@ class ShareTests(TestCase): ) self.clock = Clock() - self.anonymous_storage_server = self.useFixture( + self.storage = self.useFixture( AnonymousStorageServer(self.clock), - ).storage_server + ) self.spending_recorder, spender = RecordingSpender.make() self.server = ZKAPAuthorizerStorageServer( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, self.pass_value, self.signing_key, spender, @@ -191,7 +191,7 @@ class ShareTests(TestCase): self.spending_recorder.reset() # And clean out any shares that might confuse things. - cleanup_storage_server(self.anonymous_storage_server) + cleanup_storage_server(self.storage.backend) def test_get_version(self): """ @@ -410,7 +410,7 @@ class ShareTests(TestCase): # Create some shares to alter the behavior of the next # allocate_buckets. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -480,7 +480,7 @@ class ShareTests(TestCase): ) self.assertThat( - dict(get_lease_grant_times(self.anonymous_storage_server, storage_index)), + dict(get_lease_grant_times(self.storage.backend, storage_index)), Equals(expected_leases), ) @@ -504,7 +504,7 @@ class ShareTests(TestCase): # Create a share we can toy with. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, add_lease_secret, cancel_secret, @@ -528,7 +528,7 @@ class ShareTests(TestCase): matches_spent_passes(self.public_key_hash, self.pass_factory.spent), ) - leases = list(self.anonymous_storage_server.get_leases(storage_index)) + leases = list(self.storage.backend.get_leases(storage_index)) self.assertThat(leases, HasLength(2)) def _stat_shares_immutable_test( @@ -541,7 +541,7 @@ class ShareTests(TestCase): # Create a share we can toy with. write_shares( - self.anonymous_storage_server, + self.storage, storage_index, {sharenum}, size, @@ -550,7 +550,7 @@ class ShareTests(TestCase): # Perhaps put some more leases on it. Leases might impact our # ability to determine share data size. for renew_secret in leases: - self.anonymous_storage_server.remote_add_lease( + self.storage.backend.add_lease( storage_index, renew_secret, cancel_secret, @@ -591,8 +591,8 @@ class ShareTests(TestCase): size, when, leases, - lambda storage_server, storage_index, sharenums, size, canary: write_toy_shares( - storage_server, + lambda storage, storage_index, sharenums, size, canary: write_toy_shares( + storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -619,7 +619,7 @@ class ShareTests(TestCase): """ assume(version not in (1, 2)) - sharedir = FilePath(self.anonymous_storage_server.sharedir).preauthChild( + sharedir = FilePath(self.storage.backend.sharedir).preauthChild( # storage_index_to_dir likes to return multiple segments # joined by pathsep storage_index_to_dir(storage_index), @@ -661,7 +661,7 @@ class ShareTests(TestCase): ``stat_shares`` declines to offer a result (by raising ``ValueError``). """ - sharedir = FilePath(self.anonymous_storage_server.sharedir).preauthChild( + sharedir = FilePath(self.storage.backend.sharedir).preauthChild( # storage_index_to_dir likes to return multiple segments # joined by pathsep storage_index_to_dir(storage_index), @@ -713,8 +713,8 @@ class ShareTests(TestCase): write real multi-gigabyte files to exercise the behavior. """ - def write_shares(storage_server, storage_index, sharenums, size, canary): - sharedir = FilePath(storage_server.sharedir).preauthChild( + def write_shares(storage, storage_index, sharenums, size, canary): + sharedir = FilePath(storage.backend.sharedir).preauthChild( # storage_index_to_dir likes to return multiple segments # joined by pathsep storage_index_to_dir(storage_index), @@ -819,7 +819,7 @@ class ShareTests(TestCase): """ # Create a share we can toy with. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -838,7 +838,7 @@ class ShareTests(TestCase): succeeded(Always()), ) self.assertThat( - FilePath(self.anonymous_storage_server.corruption_advisory_dir).children(), + FilePath(self.storage.backend.corruption_advisory_dir).children(), HasLength(1), ) @@ -928,7 +928,7 @@ class ShareTests(TestCase): self.assertThat( dict( get_lease_grant_times( - self.anonymous_storage_server, + self.storage.backend, storage_index, ) ), @@ -956,7 +956,7 @@ class ShareTests(TestCase): def leases(): return list( lease.to_mutable_data() - for lease in self.anonymous_storage_server.get_slot_leases( + for lease in self.storage.backend.get_slot_leases( storage_index ) ) diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index 73a0001fc38d5924d133d09682effa3c20727eb0..055a4be53be1b01e29c21432171e21f4648ad0a0 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -20,6 +20,7 @@ from random import shuffle from time import time from allmydata.storage.mutable import MutableShareFile +from allmydata.interfaces import NoSpace from challenge_bypass_ristretto import PublicKey, random_signing_key from foolscap.referenceable import LocalReferenceable from hypothesis import given, note @@ -193,15 +194,15 @@ class PassValidationTests(TestCase): # the same time so we can do lease expiration calculations more # easily. self.clock.advance(time()) - self.anonymous_storage_server = self.useFixture( + self.storage = self.useFixture( AnonymousStorageServer(self.clock), - ).storage_server + ) self.signing_key = random_signing_key() self.public_key_hash = PublicKey.from_signing_key( self.signing_key ).encode_base64() self.storage_server = ZKAPAuthorizerStorageServer( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, self.pass_value, self.signing_key, spender, @@ -221,7 +222,11 @@ class PassValidationTests(TestCase): # Hypothesis and testtools fixtures don't play nicely together in a # way that allows us to just move everything from `setUp` into this # method. - cleanup_storage_server(self.anonymous_storage_server) + cleanup_storage_server(self.storage.backend) + # One of the tests makes the server read-only partway through. Make + # sure that doesn't leak into other examples. + self.storage.backend.readonly_storage = False + self.spending_recorder.reset() # Reset all of the metrics, too, so the individual tests have a @@ -527,7 +532,7 @@ class PassValidationTests(TestCase): ) # Create some shares at a slot which will require lease renewal. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -801,7 +806,7 @@ class PassValidationTests(TestCase): # the subsequent `allocate_buckets` operation - but of which the # client is unaware. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -872,7 +877,7 @@ class PassValidationTests(TestCase): ): # Create some shares at a slot which will require lease renewal. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -919,21 +924,24 @@ class PassValidationTests(TestCase): @given( storage_index=storage_indexes(), - renew_secret=lease_renew_secrets(), + renew_secrets=lists(lease_renew_secrets(), min_size=2, max_size=2, unique=True), cancel_secret=lease_cancel_secrets(), sharenums=sharenum_sets(), allocated_size=sizes(), ) def test_add_lease_metrics_on_failure( - self, storage_index, renew_secret, cancel_secret, sharenums, allocated_size + self, storage_index, renew_secrets, cancel_secret, sharenums, allocated_size ): """ If the ``add_lease`` operation fails then the successful pass spending metric is not incremented. """ + # We have two renew secrets so we can operate on two distinct leases. + renew_secret, another_renew_secret = renew_secrets + # Put some shares up there to target with the add_lease operation. write_toy_shares( - self.anonymous_storage_server, + self.storage.anonymous_foolscap_server, storage_index, renew_secret, cancel_secret, @@ -951,12 +959,9 @@ class PassValidationTests(TestCase): self.signing_key, ) - # Tahoe doesn't make it very easy to make an add_lease operation fail - # so monkey-patch something broken in. After 1.17.0 we can set - # `reserved_space` on StorageServer to a very large number and the - # server should refuse to allocate space for a *new* lease (which - # means we need to use a different renew secret for the next step. - self.anonymous_storage_server.remote_add_lease = lambda *a, **kw: 1 / 0 + # Turn off space-allocating operations entirely. Since there will be + # no space for a new lease, the operation will fail. + self.storage.backend.readonly_storage = True try: self.storage_server.doRemoteCall( @@ -965,14 +970,14 @@ class PassValidationTests(TestCase): dict( passes=_encode_passes(valid_passes), storage_index=storage_index, - renew_secret=renew_secret, + renew_secret=another_renew_secret, cancel_secret=cancel_secret, ), ) - except ZeroDivisionError: + except NoSpace: pass else: - self.fail("expected our ZeroDivisionError to be raised") + self.fail("expected NoSpace to be raised") after_count = read_spending_success_histogram_total(self.storage_server) self.expectThat(