(Feature) Add ability to optionally configure a lookup hierarchy

[TraGicCode]

Problem Statement

Currently, there is no way to allow node-specific secrets similar to how the YAML backend works in Puppet. This issue was brought to my attention through GitHub issue #142.

Proposal

Implement the ability to configure a custom "hierarchy" in which lookups can be performed to support node-specific secrets.

Key Considerations

Performance/Cost

The following key considerations must be taken into account for the implementation of this feature:

1. Reduce costs for large environments.
2. Keep catalog compilation/runs fast.
3. Reduce the rate of hitting rate limits.

Note: Currently, there is a maximum of 2,000 lookups every 10 seconds allowed against a Key Vault (Standard Tier) and 4,000 lookups every 10 seconds allowed against a Key Vault (Premium Tier).

Alternative to other usage

This is not meant to replace or deprecate the current functionality of Hiera in this module. Instead, it's meant to be another way in which this module can be used if more flexibility is needed.

Proposed Solution

  - name: 'Azure Key Vault Secrets'
    lookup_key: azure_key_vault::lookup
    options:
      vault_name: secrets-vault
      vault_api_version: '2016-10-01'
      metadata_api_version: '2018-04-02'
      key_replacement_token: '-'
      prefixes: 
        - nodes--%{trusted.hostname}--
        - common--
      confine_to_keys:
        - '^azure_.*'

[yakatz]

Looks good to me

[TraGicCode]

I did take a look at using YAML Anchors in aliases and it appears this "could" work if a new option was added called key_lookup_prefix. It feels complex/clunky but i did want to leave it here as a note for others in the future ( and for myself! ). This also goes back to what the original requestor was wanting.

I also didn't test and am not sure if hiera.yaml properly works with anchors and aliases.

With anchors and alises

version: 5
defaults:
  datadir: data
  data_hash: yaml_data
azure_key_vault_defaults: &azure_key_vault_defaults
  lookup_key: 'azure_key_vault::lookup'
  options:
    vault_api_version: '2016-10-01'
    service_principal_credentials: /etc/puppetlabs/puppet/azure_key_vault_credentials.yaml
    key_replacement_token: '-'
    key_lookup_prefix: ''
    confine_to_keys:
      - '^azure_*'
hierarchy:
  - name: Azure Key Vault Per-Node Secrets
    <<: *azure_key_vault_defaults
    options:
      key_lookup_prefix: 'nodes--%{trusted.hostname}--' # overrides `key_lookup_prefix` from `azure_key_vault_defaults`
  - name: Azure Key Vault Common Secrets
    <<: *azure_key_vault_defaults
    options:
      vault_name: secrets # overrides `vault_name` from `azure_key_vault_defaults`
  - name: Yaml backend
    paths:
      - 'nodes/%{::trusted.certname}.yaml'
      - 'osfamilies/%{::os.family}.yaml'
      - common.yaml

This is what the above expanded form of the anchors and aliases would result in

version: 5
defaults:
  datadir: data
  data_hash: yaml_data

hierarchy:
  - name: Azure Key Vault Per-Node Secrets
    lookup_key: 'azure_key_vault::lookup'
    options:
      vault_api_version: '2016-10-01'
      service_principal_credentials: /etc/puppetlabs/puppet/azure_key_vault_credentials.yaml
      key_replacement_token: '-'
      key_lookup_prefix: 'nodes--%{trusted.hostname}--'
      confine_to_keys:
        - '^azure_*'

  - name: Azure Key Vault Common Secrets
    lookup_key: 'azure_key_vault::lookup'
    options:
      vault_api_version: '2016-10-01'
      service_principal_credentials: /etc/puppetlabs/puppet/azure_key_vault_credentials.yaml
      key_replacement_token: '-'
      key_lookup_prefix: ''
      confine_to_keys:
        - '^azure_*'
      vault_name: secrets

  - name: Yaml backend
    paths:
      - 'nodes/%{::trusted.certname}.yaml'
      - 'osfamilies/%{::os.family}.yaml'
      - common.yaml

Another downside of this approach is you cannot do what @yakatz mentioned and "possibly get a list of secrets" beforehand to help reduce call count/cost/getting rate limited.

@yakatz @DennisTerrell and @sharumpe feedback is welcome here as well.

[sharumpe]

Your proposed solution with the “prefixes” looks great and gets me the ability to do what we need to do. No need to delve into the extra work for the key_lookup_prefix bit.

I’m assuming that the order that the prefixes are listed in will be the order they’re searched in? If so that’s perfect. Thanks!

[TraGicCode]

Hey @sharumpe ,

I actually messed up on the above example using YAML aliases and anchors. This has been fixed.

Your proposed solution with the “prefixes” looks great and gets me the ability to do what we need to do. No need to delve into the extra work for the key_lookup_prefix bit.

Got it! thanks

I’m assuming that the order that the prefixes are listed in will be the order they’re searched in? If so that’s perfect. Thanks!

Yep, this is how the yaml backend works and how other puppet modules are doing it.

[yakatz]

I think the anchors solution would be unnecessarily complex.

The only other change I could think of would be to allow the key name to be inserted anywhere in the string instead of only a prefix, but I don't think that is necessary.

    searches:
        # Prepend looked-up value instead of append (using string replacement KEY)
        - KEY--nodes--%{trusted.hostname}
        - KEY--common

[TraGicCode]

Hey @sharumpe and @yakatz ,

I should have this feature completed by end of next week. Will post back here once the PR is ready to go.

[TraGicCode]

Hey @sharumpe and @yakatz,

Can both of you give the PR below a spin? It should be pretty much code complete. Please let me know if you identify any bugs so I can write a unit test to resolve the issue.

#147

Please keep in mind the following:

• Documentation is still pending.
• I have not implemented what @yakatz mentioned regarding querying a list of secrets "ahead of time" to help reduce the number of calls to Azure Key Vault. I think this is a good idea but would like to implement it in a separate PR after this one is merged.
• All prefixes have been normalized. This might seem confusing, but I think it's ideal since erroring out on a node with an invalid character for a secret name is not something the user should be expected to fix. An example is shown below for clarity:

  - name: 'Azure Key Vault Secrets'
    lookup_key: azure_key_vault::lookup
    options:
      vault_name: secrets-vault
      vault_api_version: '2016-10-01'
      metadata_api_version: '2018-04-02'
      key_replacement_token: '-'
      prefixes: 
        - nodes--%{trusted.hostname}--
        - common--
      confine_to_keys:
        - '^azure_.*'

if a lookup for a key of profile::windows::sqlserver::sensitive_azure_sql_user_password occurs, and the host name of the machine happens to be WIN_P-01.domain.com, the normalized prefix and key will end up being turned into nodes--WIN-P-01-domain-com--profile--windows--sqlserver--sensitive-azure-sql-user-password (i.e., the name of the secret in the vault).

[sharumpe]

I'm closing up another task right now, but I'll give this a shot in the next hour or two.

[sharumpe]

I've tested this out using the following:

  - name: 'Azure Key Vault Secrets'
    lookup_key: azure_key_vault::lookup
    options:
      vault_name: puppet-linux-secrets
      vault_api_version: '2016-10-01'
      service_principal_credentials: '/etc/puppetlabs/puppet/azure_key_vault_credentials.yaml'
      key_replacement_token: '-'
      confine_to_keys:
        - '^secret::.*'
        - '^.*password$'
        - '^.*secret$'
      prefixes:
        - "%{::trusted.certname}--"
        - ""

I was able to retrieve the secrets the same way as I had been doing previously. Specifically, this means the node-specific lookup worked, overriding the not-node-specific one. I also tested not-node-specific lookups (the - "") and that worked as expected, as well. I think this looks good, at least for the types of things I do.

Thanks much!