Repository: hynek/environ-config

Issue:
Optional groups with secrets doesn't work
I would have expected this to work:

```python
import environ
from environ.secrets import DirectorySecrets

dir = DirectorySecrets.from_path("/what")


@environ.config
class Cfg:
    @environ.config
    class Sub:
        y = environ.var()
        dir = dir.secret()

    sub = environ.group(Sub, optional=True)


cfg = environ.to_config(Cfg, {})
assert cfg.sub is None
```

But it throws a `MissingSecretError`. Is there some clever thought behind it, or should it work?

If it should work, then I can look into fixing it.

Discussion:
@hynek: hm yeah I played with it a bit and it's more complicated that I'd wished for which reminds me, why I was rather unenthusiastic about adding it in the first place. Secrets and variables are handled somewhat differently and there's quite a bit of code that would need to mirror the optional logic (e.g., "fail if at least one of the vars in the optional group is present, but at least one is missing")@magnuswatn: Hmm, would https://github.com/hynek/environ-config/pull/100 not work? I don't think that's too big of a change. But it's a behaviour change, since it would now raise `MissingSecretError()` with *all* the missing secrets, instead of just the first. One could argue that that's better, though :-)

But of course, it's up to you, so if you rather not then I'll just close the issue, no problem!

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a7afd4..8bafd50 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,11 @@ Whenever there is a need to break compatibility, it is announced here in the cha
 - Type hints for `environ.group()` now respects the `optional` argument.
   [#98](https://github.com/hynek/environ-config/pull/98)
 
+### Changes
+
+- Secrets are now handled like environment variables in optional groups, so that if all secrets (and environment variables) are missing, the group becomes `None`.
+  This change also causes the `MissingSecretError` exceptions to include *all* missing secrets, not just the first one.
+  [#100](https://github.com/hynek/environ-config/pull/100)
 
 ## [24.1.0](https://github.com/hynek/environ-config/compare/23.2.0...24.1.0) - 2024-08-08
 
diff --git a/src/environ/_environ_config.py b/src/environ/_environ_config.py
index 88cc300..df1af6c 100644
--- a/src/environ/_environ_config.py
+++ b/src/environ/_environ_config.py
@@ -23,7 +23,7 @@ from typing import Any, Callable, Literal, TypeVar, overload
 
 import attr
 
-from .exceptions import MissingEnvValueError
+from .exceptions import MissingEnvValueError, MissingSecretError
 
 
 CNF_KEY = "environ_config"
@@ -294,6 +294,7 @@ def _to_config_recurse(config_cls, environ, prefixes, default=RAISE):
     got = {}
     defaulted = {}
     missing_vars = set()
+    missing_secrets = set()
 
     for attr_obj in attr.fields(config_cls):
         try:
@@ -311,9 +312,12 @@ def _to_config_recurse(config_cls, environ, prefixes, default=RAISE):
             getter = ce.callback or _default_getter
             try:
                 got[name] = getter(environ, attr_obj.metadata, prefixes, name)
-            except MissingEnvValueError as exc:
+            except (MissingEnvValueError, MissingSecretError) as exc:
                 if isinstance(ce.default, Raise):
-                    missing_vars |= set(exc.args)
+                    if isinstance(exc, MissingSecretError):
+                        missing_secrets |= set(exc.args)
+                    else:
+                        missing_vars |= set(exc.args)
                 else:
                     defaulted[name] = (
                         attr.NOTHING
@@ -321,10 +325,15 @@ def _to_config_recurse(config_cls, environ, prefixes, default=RAISE):
                         else ce.default
                     )
 
-    if missing_vars:
+    if missing_vars or missing_secrets:
         # If we were told to raise OR if we got *any* values for our attrs, we
-        # will raise a `MissingEnvValueError` with all the missing variables
+        # will raise a `Missing..Error` with all the missing variables
         if isinstance(default, Raise) or got:
+            # Raise MissingSecretError if there was any missing secrets, since
+            # that used to bubble all the way up, and thus was always prioritized
+            # over `MissingEnvValueError`.
+            if missing_secrets:
+                raise MissingSecretError(*missing_secrets) from None
             raise MissingEnvValueError(*missing_vars) from None
 
         # Otherwise we will simply use the default passed into this call.


Test patch:
diff --git a/tests/test_secrets.py b/tests/test_secrets.py
index 7181b79..0138005 100644
--- a/tests/test_secrets.py
+++ b/tests/test_secrets.py
@@ -19,7 +19,7 @@ import pytest
 
 import environ
 
-from environ.exceptions import MissingSecretError
+from environ.exceptions import MissingEnvValueError, MissingSecretError
 from environ.secrets import (
     DirectorySecrets,
     INISecrets,
@@ -176,6 +176,84 @@ class TestIniSecret:
 
         assert "foobar" == cfg.password
 
+    def test_group_missing_optional_var(self, ini):
+        """
+        If a missing env variable is part of an
+        optional group together with a secret,
+        raise MissingEnvValueError.
+        """
+
+        @environ.config
+        class Cfg:
+            @environ.config
+            class OptSub:
+                another_option = environ.var()
+                password = ini.secret()
+
+            db = environ.group(OptSub, optional=True)
+
+        with pytest.raises(MissingEnvValueError) as e:
+            environ.to_config(Cfg, {})
+        assert ("APP_DB_ANOTHER_OPTION",) == e.value.args
+
+    def test_group_missing_optional_secret(self, ini):
+        """
+        If secret is part of a group and is missing,
+        raise MissingSecretError.
+        """
+
+        @environ.config
+        class Cfg:
+            @environ.config
+            class OptSub:
+                another_option = environ.var()
+                secret = ini.secret()
+
+            opt_sub = environ.group(OptSub, optional=True)
+
+        with pytest.raises(MissingSecretError) as e:
+            environ.to_config(Cfg, {"APP_OPT_SUB_ANOTHER_OPTION": "23"})
+        assert ("opt_sub_secret",) == e.value.args
+
+    def test_group_missing_optional_both_env_and_secret(self, ini):
+        """
+        If both a secret and an env var is missing from an optional group
+        raise MissingSecretError.
+        """
+
+        @environ.config
+        class Cfg:
+            @environ.config
+            class OptSub:
+                first_option = environ.var()
+                another_option = environ.var()
+                secret = ini.secret()
+
+            opt_sub = environ.group(OptSub, optional=True)
+
+        with pytest.raises(MissingSecretError) as e:
+            environ.to_config(Cfg, {"APP_OPT_SUB_ANOTHER_OPTION": "23"})
+        assert ("opt_sub_secret",) == e.value.args
+
+    def test_optional_group_missing_all(self, ini):
+        """
+        If secret is part of an optional group and is missing,
+        use implicit default.
+        """
+
+        @environ.config
+        class Cfg:
+            @environ.config
+            class OptSub:
+                another_option = environ.var(default=42)
+                secret = ini.secret()
+
+            opt_sub = environ.group(OptSub, optional=True)
+
+        cfg = environ.to_config(Cfg, {})
+
+        assert None is cfg.opt_sub
+
 
 @pytest.fixture
 def vault():