Handle exceptions when updating LF opt-ins (#292)

* Handle exceptions when updating LF opt-ins

- Allow revoking an opt-in which does not exist to continue
- Allow granting an opt-in which already exists to continue
- This fixes bugs when there the state of the DB and LF gets out of sync

* Add unit tests

* Bump chart patch version

ap/aws/lakeformation.py

@@ -173,10 +173,17 @@ def create_lake_formation_opt_in(
                     "CatalogId": resource_catalog_id or self.catalog_id,
                 },
             }
-
-        client.create_lake_formation_opt_in(
-            Principal={"DataLakePrincipalIdentifier": principal}, Resource=resource
-        )
+        try:
+            return client.create_lake_formation_opt_in(
+                Principal={"DataLakePrincipalIdentifier": principal}, Resource=resource
+            )
+        except botocore.exceptions.ClientError as error:
+            code = error.response["Error"]["Code"]
+            msg = error.response["Error"]["Message"]
+            if code == "InvalidInputException" and "already exists" in msg:
+                logger.info("Lake Formation opt-in already exists, continuing")
+                return
+            raise error
 
     def delete_lake_formation_opt_in(
         self,
@@ -203,9 +210,16 @@ def delete_lake_formation_opt_in(
                 },
             }
 
-        client.delete_lake_formation_opt_in(
-            Principal={"DataLakePrincipalIdentifier": principal}, Resource=resource
-        )
+        try:
+            return client.delete_lake_formation_opt_in(
+                Principal={"DataLakePrincipalIdentifier": principal}, Resource=resource
+            )
+        except botocore.exceptions.ClientError as error:
+            msg = error.response["Error"]["Message"]
+            code = error.response["Error"]["Code"]
+            if code == "InvalidInputException" and "does not exist" in msg:
+                return logger.info("Lake Formation opt-in does not exist, continuing")
+            raise error
 
     def list_permissions(self, principal, resource):
         logger.info(f"Getting permissions for {principal} on {resource}")

ap/database_access/views.py

@@ -169,6 +169,15 @@ def form_valid(self, form: BaseModelForm) -> HttpResponse:
             form.add_error(None, "An error occured granting permissions")
             return self.form_invalid(form)
 
+    def get_success_url(self) -> str:
+        return reverse(
+            "database_access:table_detail",
+            kwargs={
+                "database_name": self.kwargs["database_name"],
+                "table_name": self.kwargs["table_name"],
+            },
+        )
+
 
 class GrantTableAccessView(OIDCLoginRequiredMixin, TableAccessMixin, BreadcrumbsMixin, CreateView):
     template_name = "database_access/database/grant_access.html"

chart/Chart.yaml

@@ -3,8 +3,8 @@ apiVersion: v2
 name: analytical-platform-ui
 description: Analytical Platform UI
 type: application
-version: 0.2.5
-appVersion: 0.2.5
+version: 0.2.6
+appVersion: 0.2.6
 icon: https://upload.wikimedia.org/wikipedia/en/thumb/4/4a/Ministry_of_Justice_logo_%28United_Kingdom%29.svg/611px-Ministry_of_Justice_logo_%28United_Kingdom%29.svg.png
 maintainers:
   - name: moj-data-platform-robot

tests/aws/test_lakeformation.py

@@ -91,3 +91,51 @@ def test_revoke_table_raises_error(self):
                 lf.revoke_table_permissions(
                     database="db_without_access", table="table_without_access", principal="user"
                 )
+
+
+class TestHybridOptIn:
+    @pytest.mark.parametrize(
+        "method_name", ["delete_lake_formation_opt_in", "create_lake_formation_opt_in"]
+    )
+    def test_raises_error(self, method_name):
+        lf = LakeFormationService()
+        with patch.object(lf, "get_client") as mock_get_client:
+            mock_client = MagicMock()
+            getattr(mock_client, method_name).side_effect = botocore.exceptions.ClientError(
+                {
+                    "Error": {
+                        "Code": "SomeOtherError",
+                        "Message": "Some other error message",
+                    }
+                },
+                method_name,
+            )
+            mock_get_client.return_value = mock_client
+            with pytest.raises(botocore.exceptions.ClientError):
+                method = getattr(lf, method_name)
+                method(database="db_without_opt_in", principal="user")
+
+    @pytest.mark.parametrize(
+        "method_name, error_msg",
+        [
+            ("delete_lake_formation_opt_in", "Opt-in does not exist"),
+            ("create_lake_formation_opt_in", "Opt-in already exists"),
+        ],
+    )
+    def test_does_not_raise_error(self, method_name, error_msg):
+        lf = LakeFormationService()
+        with patch.object(lf, "get_client") as mock_get_client:
+            mock_client = MagicMock()
+            getattr(mock_client, method_name).side_effect = botocore.exceptions.ClientError(
+                {
+                    "Error": {
+                        "Code": "InvalidInputException",
+                        "Message": error_msg,  # noqa
+                    }
+                },
+                method_name,
+            )
+            mock_get_client.return_value = mock_client
+            method = getattr(lf, method_name)
+            result = method(database="db_with_opt_in", principal="user")
+            assert result is None