From 069b26949c95bf21fa5d181e1daa2971cb239f7b Mon Sep 17 00:00:00 2001
From: Adrian Moennich <adrian.moennich@cern.ch>
Date: Fri, 14 May 2021 12:39:46 +0200
Subject: [PATCH] Storage/S3: Fix dynamic bucket hashing

Also run the s3 plugin tests in CI...
---
 .github/workflows/ci.yml                |  1 +
 storage_s3/indico_storage_s3/storage.py |  2 +-
 storage_s3/pytest.ini                   | 16 ++++++++++++++++
 storage_s3/tests/plugin_test.py         |  4 ++--
 4 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 storage_s3/pytest.ini

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index bf62974..635a2d9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -193,6 +193,7 @@ jobs:
           - plugin: citadel
           - plugin: livesync
           - plugin: payment_paypal
+          - plugin: storage_s3
           - plugin: vc_zoom
 
     steps:
diff --git a/storage_s3/indico_storage_s3/storage.py b/storage_s3/indico_storage_s3/storage.py
index 2f83e43..151dae4 100644
--- a/storage_s3/indico_storage_s3/storage.py
+++ b/storage_s3/indico_storage_s3/storage.py
@@ -241,7 +241,7 @@ class DynamicS3Storage(S3StorageBase):
 
     def _get_bucket_name(self, date):
         name = self._replace_bucket_placeholders(self.bucket_name_template, date)
-        token = hmac.new(self.bucket_secret.encode(), name, hashlib.md5).hexdigest()
+        token = hmac.new(self.bucket_secret.encode(), name.encode(), hashlib.md5).hexdigest()
         return f'{name}-{token}'[:63]
 
     def _replace_bucket_placeholders(self, name, date):
diff --git a/storage_s3/pytest.ini b/storage_s3/pytest.ini
new file mode 100644
index 0000000..7990c25
--- /dev/null
+++ b/storage_s3/pytest.ini
@@ -0,0 +1,16 @@
+[pytest]
+; more verbose summary (include skip/fail/error/warning), coverage
+addopts = -rsfEw --cov . --cov-report html --no-cov-on-fail
+; only check for tests in suffixed files
+python_files = *_test.py
+; we need the storage_s3 plugin to be loaded
+indico_plugins = storage_s3
+; fail if there are warnings, but ignore ones that are likely just noise
+filterwarnings =
+    error
+    ignore::sqlalchemy.exc.SAWarning
+    ignore::UserWarning
+    # port_for via pytest-redis
+    ignore:Sampling from a set deprecated:DeprecationWarning:port_for
+; use redis-server from $PATH
+redis_exec = redis-server
diff --git a/storage_s3/tests/plugin_test.py b/storage_s3/tests/plugin_test.py
index 5b4f2ff..99b2719 100644
--- a/storage_s3/tests/plugin_test.py
+++ b/storage_s3/tests/plugin_test.py
@@ -40,7 +40,7 @@ def test_resolve_bucket_name_dynamic(freeze_time, date, name_template, expected_
     storage = plugin.DynamicS3Storage(f'bucket_template={name_template},bucket_secret=secret')
     name, token = storage._get_current_bucket_name().rsplit('-', 1)
     assert name == expected_name
-    assert token == hmac.new(b'secret', expected_name, hashlib.md5).hexdigest()
+    assert token == hmac.new(b'secret', expected_name.encode(), hashlib.md5).hexdigest()
 
 
 class MockConfig:
@@ -78,7 +78,7 @@ def test_dynamic_bucket_creation_task(freeze_time, mocker, date, name_template,
     else:
         create_bucket()
     if bucket_created:
-        token = hmac.new(b'secret', expected_name, hashlib.md5).hexdigest()
+        token = hmac.new(b'secret', expected_name.encode(), hashlib.md5).hexdigest()
         create_bucket_call.assert_called_with(f'{expected_name}-{token}')
     else:
         assert not create_bucket_call.called