[Backport perf-v17] fix(integration-tests): align AWS keystore tests with the Secrets Manager backend

sdcm/keystore.py

@@ -42,6 +42,10 @@
 # client must always be pinned explicitly.
 KEYSTORE_SM_REGION = "us-east-1"
 
+# Namespace the mirrored keystore entries live under in Secrets Manager, so a
+# keystore file `foo.json` maps to the secret `sct/foo.json`.
+KEYSTORE_SM_PREFIX = "sct/"
+
 SSHKey = namedtuple("SSHKey", ["name", "public_key", "private_key"])
 
 BOTO3_CLIENT_CREATION_LOCK = threading.Lock()
@@ -98,7 +102,7 @@ def __init__(self, backend: str | None = None):
         self._cache: dict[str, bytes] = {}
         self._cache_lock = threading.Lock()
         self._backend = backend or os.environ.get("SCT_KEYSTORE_BACKEND") or "secretsmanager"
-        self._sm_prefix = os.environ.get("SCT_KEYSTORE_SM_PREFIX") or "sct/"
+        self._sm_prefix = os.environ.get("SCT_KEYSTORE_SM_PREFIX") or KEYSTORE_SM_PREFIX
         self._sm_region = os.environ.get("SCT_KEYSTORE_SM_REGION") or KEYSTORE_SM_REGION
 
     @property
@@ -350,9 +354,18 @@ def get_obj_if_needed(self, key, local_path, permissions):
             self.download_file(filename=key, dest_filename=path)
             os.chmod(path=path, mode=permissions)
             if self._backend == "secretsmanager":
-                with open(f"{path}.version", "w", encoding="utf-8") as vf:
+                with open(version_path, "w", encoding="utf-8") as vf:
                     vf.write(remote_version)
 
+        if self._backend == "secretsmanager":
+            # Match the key's permissions: the sidecar lands in the same
+            # directory (often ~/.ssh, where ssh rejects group/world readable
+            # content) and would otherwise inherit the umask. Enforced on every
+            # sync rather than only after a download, because a sidecar whose
+            # version is still current is never rewritten -- one left at 0o664
+            # by an earlier sync would keep those permissions forever.
+            os.chmod(path=version_path, mode=permissions)
+
     def sync(self, keys, local_path, permissions=0o777):
         """Syncs the local and remote copies from the configured backend."""
         if not keys:

test_sync.py

@@ -23,7 +23,7 @@ def is_using_aws_mock():
     if not is_using_aws_mock():
         key_store = KeyStore()
         key_store.sync(
-            keys=["scylla-qa-ec2", "scylla-test", "scylla_test_id_ed25519"] * 20,
+            keys=["scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"] * 30,
             local_path=Path("~/.ssh/").expanduser(),
             permissions=0o0600,
         )

unit_tests/test_aws_services.py

@@ -12,7 +12,7 @@
 os.environ["MOTO_AMIS_PATH"] = str(Path(__file__).parent / "test_data" / "mocked_ami_data.json")
 from moto.server import ThreadedMotoServer
 
-from sdcm.keystore import KeyStore
+from sdcm.keystore import KEYSTORE_S3_BUCKET, KEYSTORE_SM_PREFIX, KEYSTORE_SM_REGION, KeyStore
 from sdcm.utils.aws_region import AwsRegion
 from sdcm.sct_provision.common.layout import SCTProvisionLayout, create_sct_configuration
 from sdcm.utils.common import get_scylla_ami_versions
@@ -53,28 +53,27 @@ def moto_server():
 
 @pytest.fixture(scope="module", autouse=True)
 def keystore_configure(moto_server):
-    s3 = boto3.resource(service_name="s3", region_name=AWS_REGION, endpoint_url=moto_server)
+    entries = {
+        "gcp-sct-project-1.json": b"{}",
+        "aws_images_role.json": (
+            b'{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}'
+        ),
+    }
+    for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"):
+        entries[file] = (Path("~/.ssh").expanduser() / file).read_bytes()
 
-    bucket = s3.Bucket("scylla-qa-keystore")
+    s3 = boto3.resource(service_name="s3", region_name=AWS_REGION, endpoint_url=moto_server)
+    bucket = s3.Bucket(KEYSTORE_S3_BUCKET)
     bucket.create()
-    bucket.put_object(Key="gcp-sct-project-1.json", Body=b"{}")
-    bucket.put_object(
-        Key="aws_images_role.json",
-        Body=b'{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}',
-    )
-    for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"):
-        bucket.upload_file(Filename=str(Path("~/.ssh").expanduser() / file), Key=file)
-
-    # The Secrets Manager backend (the default) reads `sct/<name>` secrets,
-    # so mirror the same entries into moto's Secrets Manager as well.
-    sm = boto3.client("secretsmanager", region_name=AWS_REGION, endpoint_url=moto_server)
-    sm.create_secret(Name="sct/gcp-sct-project-1.json", SecretString="{}")
-    sm.create_secret(
-        Name="sct/aws_images_role.json",
-        SecretString='{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}',
-    )
-    for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"):
-        sm.create_secret(Name=f"sct/{file}", SecretBinary=(Path("~/.ssh").expanduser() / file).read_bytes())
+    for key, body in entries.items():
+        bucket.put_object(Key=key, Body=body)
+
+    # KeyStore reads from Secrets Manager by default, so mirror the same entries
+    # there under the `sct/` prefix - seeding only the bucket would leave every
+    # lookup failing with ResourceNotFoundException.
+    sm = boto3.client("secretsmanager", region_name=KEYSTORE_SM_REGION, endpoint_url=moto_server)
+    for key, body in entries.items():
+        sm.create_secret(Name=f"{KEYSTORE_SM_PREFIX}{key}", SecretBinary=body)
 
 
 @pytest.fixture(scope="module")

unit_tests/unit/test_keystore.py

@@ -22,6 +22,7 @@
 import json
 import logging
 import os
+import stat
 import time
 from concurrent.futures.thread import ThreadPoolExecutor
 from unittest.mock import MagicMock, PropertyMock, patch
@@ -884,6 +885,24 @@ def test_sync_from_sm(self, sm_ks, tmp_path):
         # Version sidecar written alongside the key
         assert (tmp_path / "scylla_test_id_ed25519.version").exists()
 
+    def test_sync_from_sm_applies_permissions_to_sidecar(self, sm_ks, tmp_path):
+        """The sidecar must not inherit the umask - ssh rejects a group/world readable ~/.ssh."""
+        sm_ks.sync(["scylla_test_id_ed25519"], str(tmp_path), 0o600)
+        for name in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.version"):
+            assert stat.S_IMODE((tmp_path / name).stat().st_mode) == 0o600, f"{name} has wrong permissions"
+
+    def test_sync_from_sm_repairs_sidecar_permissions_on_cache_hit(self, sm_ks, tmp_path):
+        """A sidecar left too permissive by an earlier sync is repaired without a re-download."""
+        sm_ks.get_obj_if_needed("scylla_test_id_ed25519", str(tmp_path), 0o600)
+        sidecar = tmp_path / "scylla_test_id_ed25519.version"
+        sidecar.chmod(0o664)
+
+        # Version is unchanged, so this must fix the permissions without downloading again
+        with patch.object(sm_ks, "download_file") as mock_dl:
+            sm_ks.get_obj_if_needed("scylla_test_id_ed25519", str(tmp_path), 0o600)
+            mock_dl.assert_not_called()
+        assert stat.S_IMODE(sidecar.stat().st_mode) == 0o600
+
     def test_get_sm_version_id_returns_awscurrent(self, sm_ks):
         version_id = sm_ks.get_sm_version_id("scylla_test_id_ed25519")
         assert version_id  # non-empty UUID-like string