Skip to content

Add optional callback-style way to resolve secret values from config#111

Draft
akaIDIOT wants to merge 14 commits intomainfrom
feature/resolve-secrets
Draft

Add optional callback-style way to resolve secret values from config#111
akaIDIOT wants to merge 14 commits intomainfrom
feature/resolve-secrets

Conversation

@akaIDIOT
Copy link
Member

@akaIDIOT akaIDIOT commented Dec 6, 2024

Designed to let keyring.get_password drop into any load* function or Configuration instance, currently based on resolved a sing-magic-key mapping in to a secret value, resolved from the keys inside the mapping:

regular.configuration: 42
client:
  username: akaidiot
  password:
    $secret:
      service: github.com
      username: ${client.username}
config = load_name(..., secrets=keyring.get_password)
client = Client(**config.client)
  • badly needs tests
  • draft, subject to change

# wrap value in a Configuration
return self._wrap(value)
value = self._wrap(value)
if self._secrets and self._secrets.matches(value):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: this gets called every step along the config.deeply.nested.keys.foobar way; matches() needs to be cheap 😬

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this lend itself to a match statement, allowing the secrets instance to supply its own match args? 🤔

@@ -194,7 +199,13 @@ def get(self,
return as_type(value)
elif isinstance(value, Mapping):
Copy link
Member Author

@akaIDIOT akaIDIOT Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this has been hooked into here: current implementation requires any and all implementations to utilize at least some kind of mapping / dict / subtree to implement this. Do we want this kind restriction? Do we want to implement any alternatives right away or maybe at a later stage?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, yes, this need many comments on what / why the f things are as they are, soon™

callback=callback,
single_key=single_key,
args=args,
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very composition over inheritance approach to an implementation conforming to the Secrets protocol above, is this too much? 🤔

@akaIDIOT akaIDIOT marked this pull request as draft December 6, 2024 19:50
@akaIDIOT akaIDIOT changed the title Draft: Add optional callback-style way to resolve secret values from config Add optional callback-style way to resolve secret values from config Dec 6, 2024
def merge(*sources: typing.Mapping[str, typing.Any], missing: typing.Any = None) -> 'Configuration':
def merge(*sources: typing.Mapping[str, typing.Any],
missing: typing.Any = None,
secrets: typing.Any = None) -> 'Configuration':
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mypy fails to infer the set being constructed below correctly when this is set to typing.Optional[Secrets], meh 🤔

@caldaibis
Copy link

Okay I looked a little into this whole YAML loading stuff and I might have found a way to make the syntax more clean! You can use YAML tags to get this syntax:

regular.configuration: 42
client:
  username: akaidiot
  password: !secret
    service: github.com
    username: ${client.username}

This way, you get rid of the extra indentation and make it look a bit easier on the eye. You can then load it in python with something like this:

import yaml

class Secret:
    """Represents a !secret YAML tag."""
    def __init__(self, service: str, username: str):
        self.service = service
        self.username = username

    @classmethod
    def from_yaml(cls, loader: yaml.Loader, node: yaml.Node) -> 'Secret':
        if isinstance(node, yaml.MappingNode):
            mapping = loader.construct_mapping(node)
            return cls(mapping['service'], mapping['username'])
        raise ValueError("Invalid !secret tag format")


yaml.add_constructor('!secret', Secret.from_yaml, Loader=yaml.SafeLoader)

What do you think? I believe you can integrate something like this but let me know if this is not really what you were looking for :)

@akaIDIOT
Copy link
Member Author

akaIDIOT commented Jan 20, 2025

What do you think? I believe you can integrate something like this but let me know if this is not really what you were looking for :)

Interesting idea! I'll read up on the constructs a bit, definitely has potential to lose a level of indentation, provided users are going to 'get' the syntax. It is very explicit though, I like that :D

(and, just remembered there were thoughts on supporting toml, as that's becoming default in py-land, but not necessarily part of this PR)

@akaIDIOT akaIDIOT force-pushed the feature/resolve-secrets branch from c9bd939 to 4d0d029 Compare October 28, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants