fix(#1537): allow s3 stores without static credentials (#1538)

s3 was the only storage protocol that mandated access_key/secret_key; gcs and
azure already fall through to their default credential chains. Make the two
optional so an ambient AWS identity (instance profile, IRSA, ECS task role, SSO)
is used when no static keys are configured.

- settings.py: drop access_key/secret_key from required_keys["s3"].
- storage.py::_validate_spec: drop them from required; reject exactly one of the
  pair (botocore would otherwise fail late with PartialCredentialsError).
- storage.py::_create_filesystem: self.spec["access_key"] -> .get(...) or None
  so a missing OR empty-string credential is dropped and botocore resolves the
  chain (a forwarded "" is read as an explicit, invalid credential).

Both-present behavior is unchanged; backward compatible. Adds unit tests for the
ambient, both-present, empty-string, and partial-credential cases.

Author: dimitri-yatsenko

src/datajoint/settings.py

@@ -466,7 +466,7 @@ def get_store_spec(self, store: str | None = None, *, use_filepath_default: bool
         # Define required and allowed keys by protocol
         required_keys: dict[str, tuple[str, ...]] = {
             "file": ("protocol", "location"),
-            "s3": ("protocol", "endpoint", "bucket", "access_key", "secret_key", "location"),
+            "s3": ("protocol", "endpoint", "bucket", "location"),
             "gcs": ("protocol", "bucket", "location"),
             "azure": ("protocol", "container", "location"),
         }

src/datajoint/storage.py

@@ -340,10 +340,21 @@ def _validate_spec(self):
             if location and not Path(location).is_dir():
                 raise FileNotFoundError(f"Inaccessible local directory {location}")
         elif self.protocol == "s3":
-            required = ["endpoint", "bucket", "access_key", "secret_key"]
+            required = ["endpoint", "bucket"]
             missing = [k for k in required if not self.spec.get(k)]
             if missing:
                 raise errors.DataJointError(f"Missing S3 configuration: {', '.join(missing)}")
+            # access_key/secret_key are optional: when both are absent the
+            # underlying botocore credential chain resolves an ambient identity
+            # (instance profile, IRSA, ECS task role, SSO), matching gcs/azure.
+            # But botocore treats exactly one as a partial credential and fails
+            # late (PartialCredentialsError at first access), so reject that here
+            # with a clear message.
+            if bool(self.spec.get("access_key")) != bool(self.spec.get("secret_key")):
+                raise errors.DataJointError(
+                    "Incomplete S3 credentials: set both access_key and secret_key, "
+                    "or neither to use ambient AWS credentials."
+                )
 
     @property
     def fs(self) -> fsspec.AbstractFileSystem:
@@ -376,10 +387,14 @@ def _create_filesystem(self) -> fsspec.AbstractFileSystem:
             else:
                 endpoint_url = endpoint
 
+            # Coerce falsy (missing or empty-string) credentials to None so s3fs
+            # drops them and botocore falls through to the default chain. A
+            # forwarded "" is NOT equivalent: it survives s3fs's None-filter and
+            # botocore reads it as an explicit (invalid) credential.
             return fsspec.filesystem(
                 "s3",
-                key=self.spec["access_key"],
-                secret=self.spec["secret_key"],
+                key=self.spec.get("access_key") or None,
+                secret=self.spec.get("secret_key") or None,
                 client_kwargs={"endpoint_url": endpoint_url},
             )
 

tests/unit/test_settings.py

@@ -336,6 +336,37 @@ def test_get_store_spec_file_protocol(self):
         finally:
             dj.config.stores = original_stores
 
+    def test_get_store_spec_s3_without_credentials(self):
+        """s3 no longer requires access_key/secret_key (#1537) — matches gcs/azure."""
+        original_stores = dj.config.stores.copy()
+        try:
+            dj.config.stores["test_s3"] = {
+                "protocol": "s3",
+                "endpoint": "s3.amazonaws.com",
+                "bucket": "my-bucket",
+                "location": "prefix",
+            }
+            spec = dj.config.get_store_spec("test_s3")
+            assert spec["protocol"] == "s3"
+            assert "access_key" not in spec and "secret_key" not in spec
+        finally:
+            dj.config.stores = original_stores
+
+    def test_get_store_spec_s3_missing_bucket(self):
+        """endpoint/bucket/location stay required for s3."""
+        original_stores = dj.config.stores.copy()
+        try:
+            dj.config.stores["bad_s3"] = {
+                "protocol": "s3",
+                "endpoint": "s3.amazonaws.com",
+                "location": "prefix",
+                # missing bucket
+            }
+            with pytest.raises(DataJointError, match="missing"):
+                dj.config.get_store_spec("bad_s3")
+        finally:
+            dj.config.stores = original_stores
+
     def test_get_store_spec_missing_required(self):
         """Test missing required keys raises error."""
         original_stores = dj.config.stores.copy()

tests/unit/test_storage_adapter.py

@@ -336,3 +336,61 @@ def _fake_entry_points(*, group=None):
         assert adapter is not None
         assert sa_mod.get_storage_adapter("bad") is None
         assert any("bad" in rec.message and "boom" in rec.message for rec in caplog.records)
+
+
+class TestS3AmbientCredentials:
+    """s3 stores may omit access_key/secret_key and fall through to the
+    botocore credential chain, matching gcs/azure (#1537)."""
+
+    @staticmethod
+    def _backend(spec):
+        backend = StorageBackend.__new__(StorageBackend)
+        backend.spec = {"protocol": "s3", "endpoint": "s3.amazonaws.com", "bucket": "b", **spec}
+        backend.protocol = "s3"
+        backend._fs = None
+        return backend
+
+    def _captured_kwargs(self, monkeypatch, spec):
+        captured = {}
+
+        def fake_filesystem(protocol, **kwargs):
+            captured["protocol"] = protocol
+            captured.update(kwargs)
+            return object()
+
+        monkeypatch.setattr(storage.fsspec, "filesystem", fake_filesystem)
+        self._backend(spec)._create_filesystem()
+        return captured
+
+    def test_no_credentials_validates(self):
+        # both absent is valid — ambient identity resolves downstream
+        self._backend({})._validate_spec()
+
+    def test_no_credentials_forwards_none(self, monkeypatch):
+        kw = self._captured_kwargs(monkeypatch, {})
+        assert kw["key"] is None and kw["secret"] is None
+
+    def test_both_credentials_forwarded(self, monkeypatch):
+        kw = self._captured_kwargs(monkeypatch, {"access_key": "AK", "secret_key": "SK"})
+        assert kw["key"] == "AK" and kw["secret"] == "SK"
+
+    def test_empty_string_treated_as_absent(self, monkeypatch):
+        # "" survives s3fs's None-filter and botocore reads it as an explicit
+        # (invalid) credential, so it must be coerced to None
+        self._backend({"access_key": "", "secret_key": ""})._validate_spec()
+        kw = self._captured_kwargs(monkeypatch, {"access_key": "", "secret_key": ""})
+        assert kw["key"] is None and kw["secret"] is None
+
+    def test_partial_credentials_rejected(self):
+        with pytest.raises(DataJointError, match="Incomplete S3 credentials"):
+            self._backend({"access_key": "AK"})._validate_spec()
+        with pytest.raises(DataJointError, match="Incomplete S3 credentials"):
+            self._backend({"secret_key": "SK"})._validate_spec()
+
+    def test_missing_endpoint_or_bucket_still_required(self):
+        backend = StorageBackend.__new__(StorageBackend)
+        backend.spec = {"protocol": "s3", "bucket": "b"}  # no endpoint
+        backend.protocol = "s3"
+        backend._fs = None
+        with pytest.raises(DataJointError, match="Missing S3 configuration"):
+            backend._validate_spec()