refactor the s3 backend

This refactoring included breaking down several of the long methods in
the s3 backend to smaller functions in order to enhance unit testing and
ensure that this section of code remains solid.

This also updates moto from v4.x to v5.x to take advantage of new
functionality and simplified use.

There are many test additions for the s3 backend, but also a lot of
changes to try and reduce the reliance on complicated fixtures and
provide smaller more consise tests.

Makefile

@@ -12,11 +12,11 @@ format: init
 	poetry run isort tfworker tests
 
 test: init
-	poetry run pytest -p no:warnings
+	poetry run pytest -p no:warnings --disable-socket
 	poetry run coverage report --fail-under=60 -m --skip-empty
 
 dep-test: init
-	poetry run pytest
+	poetry run pytest --disable-socket
 	poetry run coverage report --fail-under=60 -m --skip-empty
 
 clean:

pyproject.toml

@@ -52,10 +52,11 @@ seed-isort-config = "^2.2.0"
 flake8 = "^6.0.0"
 wheel = "^0.40"
 pytest-depends = "^1.0.1"
+pytest-socket = "^0.7.0"
 pytest-lazy-fixture = "^0.6.3"
 coverage = "^7.2"
 pytest-cov = "^4.0.0"
-moto = {extras = ["sts"], version = "^4.1.4"}
+moto = {extras = ["sts","dynamodb", "s3"], version = "^5.0.8"}
 deepdiff = "^6.2.0"
 Sphinx = "5.1.1"
 

tests/authenticators/test_aws_auth.py

@@ -16,7 +16,7 @@
 
 import pytest
 from botocore.credentials import Credentials
-from moto import mock_sts
+from moto import mock_aws
 
 from tfworker.authenticators.aws import AWSAuthenticator, MissingArgumentException
 from tfworker.commands.root import RootCommand
@@ -68,7 +68,7 @@ def test_with_no_backend_bucket(self):
             AWSAuthenticator(state_args={}, deployment="deployfu")
             assert "backend_bucket" in str(e.value)
 
-    @mock_sts
+    @mock_aws
     def test_with_access_key_pair_creds(
         self, sts_client, state_args, aws_access_key_id, aws_secret_access_key
     ):
@@ -77,7 +77,7 @@ def test_with_access_key_pair_creds(
         assert auth.secret_access_key == aws_secret_access_key
         assert auth.session_token is None
 
-    @mock_sts
+    @mock_aws
     def test_with_access_key_pair_creds_and_role_arn(
         self, sts_client, state_args_with_role_arn, aws_secret_access_key
     ):
@@ -109,7 +109,7 @@ def test_with_profile(
         assert auth.secret_access_key == aws_credentials_instance.secret_key
         assert auth.session_token is None
 
-    @mock_sts
+    @mock_aws
     def test_with_prefix(self, state_args):
         auth = AWSAuthenticator(state_args, deployment="deployfu")
         assert auth.prefix == DEFAULT_BACKEND_PREFIX.format(deployment="deployfu")

tests/backends/test_s3.py

@@ -11,12 +11,19 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import os
 import random
 import string
+from unittest.mock import MagicMock, patch
 
 import pytest
+from botocore.exceptions import ClientError
+from moto import mock_aws
 
+from tests.conftest import MockAWSAuth
+from tfworker.backends import S3Backend
 from tfworker.backends.base import BackendError
+from tfworker.handlers import HandlerError
 
 STATE_BUCKET = "test_bucket"
 STATE_PREFIX = "terraform"
@@ -25,6 +32,7 @@
 EMPTY_STATE = f"{STATE_PREFIX}/{STATE_DEPLOYMENT}/empty/terraform.tfstate"
 OCCUPIED_STATE = f"{STATE_PREFIX}/{STATE_DEPLOYMENT}/occupied/terraform.tfstate"
 LOCK_DIGEST = "1234123412341234"
+NO_SUCH_BUCKET = "no_such_bucket"
 
 
 @pytest.fixture(scope="class")
@@ -159,6 +167,191 @@ def test_clean_locking_state(self, basec, state_setup, dynamodb_client):
         )
 
 
+class TestS3BackendInit:
+    def setup_method(self, method):
+        self.authenticators = {"aws": MockAWSAuth()}
+        self.definitions = {}
+
+    def test_no_session(self):
+        self.authenticators["aws"]._session = None
+        with pytest.raises(BackendError):
+            result = S3Backend(self.authenticators, self.definitions)
+
+    def test_no_backend_session(self):
+        self.authenticators["aws"]._backend_session = None
+        with pytest.raises(BackendError):
+            result = S3Backend(self.authenticators, self.definitions)
+
+    @patch("tfworker.backends.S3Backend._ensure_locking_table", return_value=None)
+    @patch("tfworker.backends.S3Backend._ensure_backend_bucket", return_value=None)
+    @patch("tfworker.backends.S3Backend._get_bucket_files", return_value={})
+    def test_deployment_undefined(
+        self,
+        mock_get_bucket_files,
+        mock_ensure_backend_bucket,
+        mock_ensure_locking_table,
+    ):
+        # arrange
+        result = S3Backend(self.authenticators, self.definitions)
+        assert result._deployment == "undefined"
+        assert mock_get_bucket_files.called
+        assert mock_ensure_backend_bucket.called
+        assert mock_ensure_locking_table.called
+
+    @patch("tfworker.backends.S3Backend._ensure_locking_table", return_value=None)
+    @patch("tfworker.backends.S3Backend._ensure_backend_bucket", return_value=None)
+    @patch("tfworker.backends.S3Backend._get_bucket_files", return_value={})
+    @patch("tfworker.backends.s3.S3Handler", side_effect=HandlerError("message"))
+    def test_handler_error(
+        self,
+        mock_get_bucket_files,
+        mock_ensure_backend_bucket,
+        mock_ensure_locking_table,
+        mock_handler,
+    ):
+        with pytest.raises(SystemExit):
+            result = S3Backend(self.authenticators, self.definitions)
+
+
+class TestS3BackendEnsureBackendBucket:
+    from botocore.exceptions import ClientError
+
+    @pytest.fixture(autouse=True)
+    def setup_class(self, state_setup):
+        pass
+
+    @patch("tfworker.backends.S3Backend._ensure_locking_table", return_value=None)
+    @patch("tfworker.backends.S3Backend._ensure_backend_bucket", return_value=None)
+    @patch("tfworker.backends.S3Backend._get_bucket_files", return_value={})
+    def setup_method(
+        self,
+        method,
+        mock_get_bucket_files,
+        mock_ensure_backend_bucket,
+        mock_ensure_locking_table,
+    ):
+        with mock_aws():
+            self.authenticators = {"aws": MockAWSAuth()}
+            self.definitions = {}
+            self.backend = S3Backend(self.authenticators, self.definitions)
+            self.backend._authenticator.bucket = STATE_BUCKET
+            self.backend._authenticator.backend_region = STATE_REGION
+
+    def teardown_method(self, method):
+        with mock_aws():
+            try:
+                self.backend._s3_client.delete_bucket(Bucket=NO_SUCH_BUCKET)
+            except Exception:
+                pass
+
+    @mock_aws
+    def test_check_bucket_does_not_exist(self):
+        result = self.backend._check_bucket_exists(NO_SUCH_BUCKET)
+        assert result is False
+
+    @mock_aws
+    def test_check_bucket_exists(self):
+        result = self.backend._check_bucket_exists(STATE_BUCKET)
+        assert result is True
+
+    @mock_aws
+    def test_check_bucket_exists_error(self):
+        self.backend._s3_client = MagicMock()
+        self.backend._s3_client.head_bucket.side_effect = ClientError(
+            {"Error": {"Code": "403", "Message": "Unauthorized"}}, "head_bucket"
+        )
+
+        with pytest.raises(ClientError):
+            result = self.backend._check_bucket_exists(STATE_BUCKET)
+            assert self.backend._s3_client.head_bucket.called
+
+    @mock_aws
+    def test_bucket_not_exist_no_create(self, capfd):
+        self.backend._authenticator.create_backend_bucket = False
+        self.backend._authenticator.bucket = NO_SUCH_BUCKET
+        with pytest.raises(BackendError):
+            result = self.backend._ensure_backend_bucket()
+            assert (
+                "Backend bucket not found and --no-create-backend-bucket specified."
+                in capfd.readouterr().out
+            )
+
+    @mock_aws
+    def test_create_bucket(self):
+        self.backend._authenticator.create_backend_bucket = True
+        self.backend._authenticator.bucket = NO_SUCH_BUCKET
+        assert NO_SUCH_BUCKET not in [
+            x["Name"] for x in self.backend._s3_client.list_buckets()["Buckets"]
+        ]
+        result = self.backend._ensure_backend_bucket()
+        assert result is None
+        assert NO_SUCH_BUCKET in [
+            x["Name"] for x in self.backend._s3_client.list_buckets()["Buckets"]
+        ]
+
+    @mock_aws
+    def test_create_bucket_invalid_location_constraint(self, capsys):
+        self.backend._authenticator.create_backend_bucket = True
+        self.backend._authenticator.bucket = NO_SUCH_BUCKET
+        self.backend._authenticator.backend_region = "us-west-1"
+        # moto doesn't properly raise a location constraint when the session doesn't match the region
+        # so we'll just do it manually
+        assert self.backend._authenticator.backend_session.region_name != "us-west-1"
+        assert self.backend._authenticator.backend_region == "us-west-1"
+        assert NO_SUCH_BUCKET not in [
+            x["Name"] for x in self.backend._s3_client.list_buckets()["Buckets"]
+        ]
+        self.backend._s3_client = MagicMock()
+        self.backend._s3_client.create_bucket.side_effect = ClientError(
+            {
+                "Error": {
+                    "Code": "InvalidLocationConstraint",
+                    "Message": "InvalidLocationConstraint",
+                }
+            },
+            "create_bucket",
+        )
+
+        with pytest.raises(SystemExit):
+            result = self.backend._create_bucket(NO_SUCH_BUCKET)
+            assert "InvalidLocationConstraint" in capsys.readouterr().out
+
+        assert NO_SUCH_BUCKET not in [
+            x["Name"] for x in self.backend._s3_client.list_buckets()["Buckets"]
+        ]
+
+    # This test can not be enabled until several other tests are refactored to not create the bucket needlessly
+    # as the method itself skips this check when being run through a test, the same also applies to "BucketAlreadyOwnedByYou"
+    # @mock_aws
+    # def test_create_bucket_already_exists(self, capsys):
+    #     self.backend._authenticator.create_backend_bucket = True
+    #     self.backend._authenticator.bucket = STATE_BUCKET
+    #     assert STATE_BUCKET in [ x['Name'] for x in self.backend._s3_client.list_buckets()['Buckets'] ]
+
+    #     with pytest.raises(SystemExit):
+    #         result = self.backend._create_bucket(STATE_BUCKET)
+    #         assert f"Bucket {STATE_BUCKET} already exists" in capsys.readouterr().out
+
+    def test_create_bucket_error(self):
+        self.backend._authenticator.create_backend_bucket = True
+        self.backend._authenticator.bucket = NO_SUCH_BUCKET
+        self.backend._s3_client = MagicMock()
+        self.backend._s3_client.create_bucket.side_effect = ClientError(
+            {"Error": {"Code": "403", "Message": "Unauthorized"}}, "create_bucket"
+        )
+
+        with pytest.raises(ClientError):
+            result = self.backend._create_bucket(NO_SUCH_BUCKET)
+            assert self.backend._s3_client.create_bucket.called
+
+
+def test_backend_remotes(basec, state_setup):
+    remotes = basec.backend.remotes()
+    assert len(remotes) == 2
+    assert "empty" in remotes
+    assert "occupied" in remotes
+
+
 def test_backend_clean_all(basec, request, state_setup, dynamodb_client, s3_client):
     # this function should trigger an exit
     with pytest.raises(SystemExit):

tests/conftest.py

@@ -20,7 +20,7 @@
 
 import boto3
 import pytest
-from moto import mock_dynamodb, mock_s3, mock_sts
+from moto import mock_aws
 from pytest_lazyfixture import lazy_fixture
 
 import tfworker
@@ -70,19 +70,19 @@ def aws_credentials():
 
 @pytest.fixture(scope="class")
 def s3_client(aws_credentials):
-    with mock_s3():
+    with mock_aws():
         yield boto3.client("s3", region_name="us-west-2")
 
 
 @pytest.fixture(scope="class")
 def dynamodb_client(aws_credentials):
-    with mock_dynamodb():
+    with mock_aws():
         yield boto3.client("dynamodb", region_name="us-west-2")
 
 
 @pytest.fixture(scope="class")
 def sts_client(aws_credentials):
-    with mock_sts():
+    with mock_aws():
         yield boto3.client("sts", region_name="us-west-2")
 
 
@@ -93,15 +93,21 @@ class MockAWSAuth:
     (cross account assumed roles, user identity, etc...)
     """
 
+    @mock_aws
     def __init__(self):
         self._session = boto3.Session()
+        self._backend_session = self._session
         self.bucket = "test_bucket"
         self.prefix = "terraform/test-0001"
 
     @property
     def session(self):
         return self._session
 
+    @property
+    def backend_session(self):
+        return self._backend_session
+
 
 @pytest.fixture()
 def grootc():

tests/test_plugins.py

@@ -68,6 +68,7 @@ def does_not_raise():
 
 
 class TestPlugins:
+    @pytest.mark.enable_socket
     @pytest.mark.depends(on="get_url")
     def test_plugin_download(self, rootc):
         plugins = tfworker.plugins.PluginsCollection(

tests/util/test_system.py

@@ -17,7 +17,7 @@
 
 import pytest
 
-from tfworker.util.system import pipe_exec, which, get_version, strip_ansi
+from tfworker.util.system import get_version, pipe_exec, strip_ansi, which
 
 
 # context manager to allow testing exceptions in parameterized tests
@@ -33,11 +33,13 @@ def mock_pipe_exec(args, stdin=None, cwd=None, env=None):
 def mock_tf_version(args: str):
     return (0, args.encode(), "".encode())
 
+
 def mock_distribution(*args, **kwargs):
     Class = mock.MagicMock()
     Class.version = "1.2.3"
     return Class
 
+
 class TestUtilSystem:
     @pytest.mark.parametrize(
         "commands, exit_code, cwd, stdin, stdout, stderr, stream_output",
@@ -139,6 +141,7 @@ def test_get_version(self):
 
     def test_get_version_unknown(self):
         from pkg_resources import DistributionNotFound
+
         with mock.patch(
             "tfworker.util.system.get_distribution",
             side_effect=DistributionNotFound,

tfworker/backends/base.py

@@ -18,7 +18,16 @@
 
 
 class BackendError(Exception):
-    pass
+    # add custom "help" parameter to the exception
+    def __init__(self, message, help=None):
+        super().__init__(message)
+        self._help = help
+
+    @property
+    def help(self):
+        if self._help is None:
+            return "No help available"
+        return self._help
 
 
 class BaseBackend(metaclass=ABCMeta):