saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
13.98k stars 5.47k forks source link

[FEATURE REQUEST] Support pillar dictionary.keys() usage in salt/utils/vault.py `expand_pattern_lists` for existing pillar data formatted as dictionary keys. #64619

Open ipaqmaster opened 12 months ago

ipaqmaster commented 12 months ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Since 3006 a salt minion or master's vault.conf can now refer to pillar data for minion roles (Thanks!). Unfortunately this does not seem to support pillar data formatted as dictionaries with additional data underneath role names. Due to the design of expand_pattern_lists you also cannot use {pillar[roles].keys()} as a policy option when the referred role pillar data involves dictionaries and sub items pertaining to a role.

Describe the solution you'd like Supporting the use of example {pillar[roles]} where the roles: pillar contains roles in the form of named dictionaries with their own elements or sub-dictionaries. It would be nice if the module could notice this and extract the keys instead of treating the entire dictionary of dictionaries as a single policy name string.

Describe alternatives you've considered

  1. Rebuilding and entire multi-megabyte salt pillar to take advantage of this new vault module feature supporting pillar data as roles (Not ideal)
  2. Creating another pillar source which enumerates all roles into a list format compatible with expand_pattern_lists in salt/utils/vault.py (Also not ideal)
  3. Modifying expand_pattern_lists in salt/utils/vault.py to support dictionaries when encountered as a pillar path as just the keys only such as with the dict.items() attribute.

Additional context As designed when vault.conf is configured with example minion policy {pillar[roles]} it will accept the pillar if they're formatted as elements.

If they're formatted as dictionaries with their own sub-elements or sub-dictionaries for additional metadata then running salt-call vault.read_secret auth/token/lookup-self on the minion reveals a target policy named: {'default': none, 'role1': none, 'role2': none}

Documentation currently accurately advises what I'm asking for is not supported and "will error". This is a request to potentially enhance this feature.

Please Note If this feature request would be considered a substantial change or addition, this should go through a SEP process here https://github.com/saltstack/salt-enhancement-proposals, instead of a feature request.

dmurphy18 commented 11 months ago

@ipaqmaster There is a major re-write of Salt's support for vault by this PR https://github.com/saltstack/salt/pull/62684 which is being moved to a salt extension for the Salt 3007 release.

Can you take a look at that PR and see if the changes there address some or any of your needs. Either way it will be easier to update vault functionality once it is a salt-extension and not tied to a release of Salt, but it's own schedule.

ipaqmaster commented 10 months ago

I've heard about this one. Looks good.

I went through some of the dot points but its unclear at a glance whether this will solve what I'm talking about here or just change the way it would be fixed. I'll have a look through the commit once I have time later and maybe even try running it in a test environment just to see.

dmurphy18 commented 10 months ago

@ipaqmaster Heads up it will take a while to get through that PR, lot of changes and files, took me a good few hours.

lkubb commented 10 months ago

This is not supported by the PR currently, but should be trivial to include by

- if isinstance(value, list):
+ if isinstance(value, (dict, list)):

in expand_pattern_lists.

lkubb commented 2 months ago

This has been added to saltext-vault in https://github.com/salt-extensions/saltext-vault/pull/27, which just had its first release today.