From 223ad4cb48aa6d2245e3ad9d4851904801ff3d5e Mon Sep 17 00:00:00 2001
From: Dragomir Penev <6687393+dragomirp@users.noreply.github.com>
Date: Thu, 2 Nov 2023 12:23:01 +0200
Subject: [PATCH] [DPE-2804] Conditionally skip backup tests (#310)

* Conditionally skip backup tests

* Wrong step output

* Bump libs
---
 .github/workflows/ci.yaml                     | 13 ++++++-
 .../data_platform_libs/v0/data_interfaces.py  | 34 +++++++++++++++----
 pyproject.toml                                |  3 ++
 tests/integration/test_backups.py             |  8 +++++
 4 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 5ed1dc2577..4751487d29 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -115,8 +115,19 @@ jobs:
             echo Skipping unstable tests
             echo "mark_expression=and not unstable" >> "$GITHUB_OUTPUT"
           fi
+      - name: Select test secret usage
+        id: select-test-secrets
+        run: |
+          if [[ "${{ github.event.pull_request.head.repo.full_name }}" == "canonical/postgresql-k8s-operator" ]]
+          then
+            echo Running tests using secrets
+            echo "mark_secrets=" >> "$GITHUB_OUTPUT"
+          else
+            echo Skipping tests using secrets
+            echo "mark_secrets=and not uses_secrets" >> "$GITHUB_OUTPUT"
+          fi
       - name: Run integration tests
-        run: tox run -e ${{ matrix.tox-environment }} -- -m 'not ${{ matrix.exclude-mark }} ${{ steps.select-test-stability.outputs.mark_expression }}' --keep-models
+        run: tox run -e ${{ matrix.tox-environment }} -- -m 'not ${{ matrix.exclude-mark }} ${{ steps.select-test-secrets.outputs.mark_secrets }} ${{ steps.select-test-stability.outputs.mark_expression }}' --keep-models
         env:
           SECRETS_FROM_GITHUB: |
             {
diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py
index df59585d53..5d1691d9f1 100644
--- a/lib/charms/data_platform_libs/v0/data_interfaces.py
+++ b/lib/charms/data_platform_libs/v0/data_interfaces.py
@@ -320,7 +320,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 22
+LIBPATCH = 23
 
 PYDEPS = ["ops>=2.0.0"]
 
@@ -807,6 +807,9 @@ def _fetch_relation_data_without_secrets(
         This is used typically when the Provides side wants to read the Requires side's data,
         or when the Requires side may want to read its own data.
         """
+        if app not in relation.data or not relation.data[app]:
+            return {}
+
         if fields:
             return {k: relation.data[app][k] for k in fields if k in relation.data[app]}
         else:
@@ -830,6 +833,9 @@ def _fetch_relation_data_with_secrets(
         normal_fields = []
 
         if not fields:
+            if app not in relation.data or not relation.data[app]:
+                return {}
+
             all_fields = list(relation.data[app].keys())
             normal_fields = [field for field in all_fields if not self._is_secret_field(field)]
 
@@ -853,8 +859,11 @@ def _fetch_relation_data_with_secrets(
 
     def _update_relation_data_without_secrets(
         self, app: Application, relation: Relation, data: Dict[str, str]
-    ):
+    ) -> None:
         """Updating databag contents when no secrets are involved."""
+        if app not in relation.data or relation.data[app] is None:
+            return
+
         if any(self._is_secret_field(key) for key in data.keys()):
             raise SecretsIllegalUpdateError("Can't update secret {key}.")
 
@@ -865,8 +874,19 @@ def _delete_relation_data_without_secrets(
         self, app: Application, relation: Relation, fields: List[str]
     ) -> None:
         """Remove databag fields 'fields' from Relation."""
+        if app not in relation.data or not relation.data[app]:
+            return
+
         for field in fields:
-            relation.data[app].pop(field)
+            try:
+                relation.data[app].pop(field)
+            except KeyError:
+                logger.debug(
+                    "Non-existing field was attempted to be removed from the databag %s, %s",
+                    str(relation.id),
+                    str(field),
+                )
+                pass
 
     # Public interface methods
     # Handling Relation Fields seamlessly, regardless if in databag or a Juju Secret
@@ -880,9 +900,6 @@ def get_relation(self, relation_name, relation_id) -> Relation:
                 "Relation %s %s couldn't be retrieved", relation_name, relation_id
             )
 
-        if not relation.app:
-            raise DataInterfacesError("Relation's application missing")
-
         return relation
 
     def fetch_relation_data(
@@ -1089,7 +1106,10 @@ def _delete_relation_secret(
         # Remove secret from the relation if it's fully gone
         if not new_content:
             field = self._generate_secret_field_name(group)
-            relation.data[self.local_app].pop(field)
+            try:
+                relation.data[self.local_app].pop(field)
+            except KeyError:
+                pass
 
         # Return the content that was removed
         return True
diff --git a/pyproject.toml b/pyproject.toml
index 0353c47090..4751f00533 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -102,6 +102,9 @@ target-version = ["py38"]
 
 # Linting tools configuration
 [tool.ruff]
+# preview and explicit preview are enabled for CPY001
+preview = true
+explicit-preview-rules = true
 target-version = "py38"
 src = ["src", "."]
 line-length = 99
diff --git a/tests/integration/test_backups.py b/tests/integration/test_backups.py
index a66de9b116..17eb9f8737 100644
--- a/tests/integration/test_backups.py
+++ b/tests/integration/test_backups.py
@@ -80,6 +80,12 @@ async def cloud_configs(ops_test: OpsTest, github_secrets) -> None:
             bucket_object.delete()
 
 
+async def test_none() -> None:
+    """Empty test so that the suite will not fail if all tests are skippedi."""
+    pass
+
+
+@pytest.mark.uses_secrets
 @pytest.mark.abort_on_fail
 async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> None:
     """Build and deploy two units of PostgreSQL and then test the backup and restore actions."""
@@ -203,6 +209,7 @@ async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict,
     await ops_test.model.remove_application(TLS_CERTIFICATES_APP_NAME, block_until_done=True)
 
 
+@pytest.mark.uses_secrets
 async def test_restore_on_new_cluster(ops_test: OpsTest) -> None:
     """Test that is possible to restore a backup to another PostgreSQL cluster."""
     database_app_name = f"new-{DATABASE_APP_NAME}"
@@ -275,6 +282,7 @@ async def test_restore_on_new_cluster(ops_test: OpsTest) -> None:
     connection.close()
 
 
+@pytest.mark.uses_secrets
 async def test_invalid_config_and_recovery_after_fixing_it(
     ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]
 ) -> None: