From 1356eb79ce35dc2086454196af2a9bb025e8c80e Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Mon, 10 Jan 2022 10:47:06 -0500
Subject: [PATCH] Account for StorageServer being split in two
---
src/_zkapauthorizer/_storage_server.py | 29 +++++++------
src/_zkapauthorizer/tests/fixtures.py | 28 +++++++-----
.../tests/test_storage_protocol.py | 40 ++++++++---------
.../tests/test_storage_server.py | 43 +++++++++++--------
4 files changed, 78 insertions(+), 62 deletions(-)
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index aa11097..a2c8e1f 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 54d27b9..aa967e2 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 ceb34de..b6a8c4d 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 73a0001..055a4be 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(
--
GitLab