(GH-146) Add ability to configure prefix hierarchy

README.md

@@ -295,6 +295,37 @@ How flexible can this get?  Below shows an example of how you "could" remove pro
 
 A lookup to 'profile::windows::sqlserver::azure_sql_user_password' or 'profile::linux::blah::blah_again::some_secret' would end up searching for secrets named azure_sql_user_password and some_secret in your Azure Key Vault named "prod-key-vault".
 
+### What is prefixes?
+
+Prefixes are an optional feature that allow you to create a "hierarchy" similar to the YAML backend built into Puppet. This enables you to configure node-specific secrets and/or create a customized lookup hierarchy. It is also useful if you are migrating from HashiCorp Hiera Vault to Azure Key Vault, as this behavior is similar to that of the [HashiCorp hiera_lookup puppet module](https://forge.puppet.com/modules/petems/hiera_vault/readme).
+
+
+Let's walk through an example to see how this works. If you wanted the ability to configure node-specific secrets but fall back on a "common/shared" secret when none are configured, you can set up the prefixes as shown below:
+
+```yaml
+  - 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 Puppet catalog is being compiled for a node with the `trusted.hostname` fact set to `WIN-SQL01.domain.com`, the following lookups would occur in the azure vault named `secrets-vault`:
+
+1. Check for a secret named `nodes--WIN-SQL-domain-com--profile--windows--sqlserver--sensitive-azure-sql-user-password`. If it exists, cache and return the secret. If not, continue to the next level in the hierarchy.
+
+2. Check for a secret named `common--profile--windows--sqlserver--sensitive-azure-sql-user-password`. If it exists, cache and return the secret. If it does not exist, return nothing and proceed to the next configured lookup if one exists in the hiera.yaml file.
+
+**NOTE: The prefixes are always normalized using the [key_replacement_token](#what-is-key_replacement_token). This is because certain facts (such as a machine's hostname) may contain characters that are not supported as part of a secret's name. This normalization prevents failures and avoids forcing users to make difficult decisions, such as changing a node's hostname, which is not ideal.**
+
+
 ## How it's secure by default
 
 In order to prevent accidental leakage of your secrets throughout all of the locations puppet stores information the returned value of the `azure_key_vault::secret` function & Hiera backend return a string wrapped in a Sensitive data type.  Lets look at an example of what this means and why it's important.  Below is an example of pulling a secret and trying to output the value in a notice function.

lib/puppet/functions/azure_key_vault/lookup.rb

@@ -11,7 +11,8 @@
       Optional[strip_from_keys] => Array[String],
       Optional[key_replacement_token] => String,
       Optional[service_principal_credentials] => String,
-      Optional[use_azure_arc_authentication] => Boolean
+      Optional[use_azure_arc_authentication] => Boolean,
+      Optional[prefixes] => Array[String],
     }]', :options
     param 'Puppet::LookupContext', :context
     return_type 'Variant[Sensitive, Undef]'
@@ -39,6 +40,11 @@ def lookup_key(secret_name, options, context)
       end
     end
 
+    prefixes = options['prefixes']
+    if prefixes
+      raise ArgumentError, 'prefixes must be an array' unless prefixes.is_a?(Array)
+    end
+
     strip_from_keys = options['strip_from_keys']
     if strip_from_keys
       raise ArgumentError, 'strip_from_keys must be an array' unless strip_from_keys.is_a?(Array)
@@ -53,9 +59,17 @@ def lookup_key(secret_name, options, context)
       end
     end
 
-    normalized_secret_name = TragicCode::Azure.normalize_object_name(secret_name, options['key_replacement_token'] || '-')
-    context.explain { "Using normalized KeyVault secret key for lookup: #{normalized_secret_name}" }
-    return Puppet::Pops::Types::PSensitiveType::Sensitive.new(context.cached_value(normalized_secret_name)) if context.cache_has_key(normalized_secret_name)
+    key_replacement_token = options['key_replacement_token'] || '-'
+    if prefixes
+      normalized_prefixed_keys = prefixes.map { |prefix| TragicCode::Azure.normalize_object_name(prefix + secret_name, key_replacement_token) }
+      normalized_prefixed_keys.each do |normalized_prefixed_key|
+        return Puppet::Pops::Types::PSensitiveType::Sensitive.new(context.cached_value(normalized_prefixed_key)) if context.cache_has_key(normalized_prefixed_key)
+      end
+    else
+      normalized_secret_name = TragicCode::Azure.normalize_object_name(secret_name, key_replacement_token)
+      context.explain { "Using normalized KeyVault secret key for lookup: #{normalized_secret_name}" }
+      return Puppet::Pops::Types::PSensitiveType::Sensitive.new(context.cached_value(normalized_secret_name)) if context.cache_has_key(normalized_secret_name)
+    end
     access_token = context.cached_value('access_token')
     if access_token.nil?
       metadata_api_version = options['metadata_api_version']
@@ -79,14 +93,28 @@ def lookup_key(secret_name, options, context)
       end
       context.cache('access_token', access_token)
     end
+    secret_value = nil
     begin
-      secret_value = TragicCode::Azure.get_secret(
-        options['vault_name'],
-        normalized_secret_name,
-        options['vault_api_version'],
-        access_token,
-        '',
-      )
+      if normalized_prefixed_keys
+        normalized_prefixed_keys.each do |normalized_prefixed_secret_key|
+          secret_value = TragicCode::Azure.get_secret(
+            options['vault_name'],
+            normalized_prefixed_secret_key,
+            options['vault_api_version'],
+            access_token,
+            '',
+          )
+          break unless secret_value.nil?
+        end
+      else
+        secret_value = TragicCode::Azure.get_secret(
+          options['vault_name'],
+          normalized_secret_name,
+          options['vault_api_version'],
+          access_token,
+          '',
+        )
+      end
     rescue RuntimeError => e
       Puppet.warning(e.message)
       secret_value = nil

spec/functions/azure_key_vault_lookup_spec.rb

@@ -60,7 +60,7 @@
   end
   # rubocop:enable RSpec/NamedSubject
 
-  it 'call context.not_found for the lookup_options key' do
+  it 'calls context.not_found for the lookup_options key' do
     expect(lookup_context).to receive(:not_found)
     is_expected.to run.with_params(
       'lookup_options', options, lookup_context
@@ -80,13 +80,13 @@
   end
   # rubocop:enable RSpec/NamedSubject
 
-  it 'errors when confine_to_keys is no array' do
+  it 'errors when `confine_to_keys` is no array' do
     is_expected.to run.with_params(
       'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'confine_to_keys' => '^vault.*$' }), lookup_context
     ).and_raise_error(ArgumentError, %r{'confine_to_keys' expects an Array value}i)
   end
 
-  it 'errors when strip_from_keys is no array' do
+  it 'errors when `strip_from_keys` is no array' do
     is_expected.to run.with_params(
       'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'strip_from_keys' => '^vault.*$' }), lookup_context
     ).and_raise_error(ArgumentError, %r{'strip_from_keys' expects an Array value}i)
@@ -98,6 +98,12 @@
     ).and_raise_error(ArgumentError, %r{metadata_api_version and service_principal_credentials cannot be used together}i)
   end
 
+  it 'errors when `prefixes` is no array' do
+    is_expected.to run.with_params(
+      'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => 'common--' }), lookup_context
+    ).and_raise_error(ArgumentError, %r{'prefixes' expects an Array value}i)
+  end
+
   it "errors when missing both 'metadata_api_version' and 'service_principal_credentials'" do
     bad_options = options
     bad_options.delete('metadata_api_version')
@@ -204,4 +210,92 @@
       .to be_an_instance_of(Puppet::Pops::Types::PSensitiveType::Sensitive)
   end
   # rubocop:enable RSpec/NamedSubject
+
+  describe 'prefixes' do
+    it 'calls context.not_found when secret is not found in vault' do
+      # Arrange
+      access_token_value = 'access_value'
+      expect(lookup_context).to receive(:not_found)
+      allow(TragicCode::Azure).to receive(:get_access_token).and_return(access_token_value)
+      allow(TragicCode::Azure).to receive(:get_secret).and_return(nil)
+      # This works but not sure if it's a good practice
+      # expect(TragicCode::Azure).to receive(:get_secret).exactly(2).times.and_return(nil)
+
+      expect(TragicCode::Azure).to receive(:get_secret).exactly(2).times
+      is_expected.to run.with_params(
+        'profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => ['nodes--%{trusted.hostname}--', 'common--'] }), lookup_context
+      )
+    end
+
+    # rubocop:disable RSpec/NamedSubject
+    it 'returns the key if found using the last prefix' do
+      access_token_value = 'access_value'
+      secret_value = 'secret_value'
+      allow(TragicCode::Azure).to receive(:get_access_token).and_return(access_token_value)
+      allow(TragicCode::Azure).to receive(:get_secret).and_return(nil)
+      allow(TragicCode::Azure).to receive(:get_secret).with(
+          options['vault_name'],
+          'common--profile--windows--sqlserver--sensitive-azure-sql-user-password',
+          options['vault_api_version'],
+          access_token_value,
+          '',
+        ).and_return('secret_value')
+
+      expect(subject.execute('profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => ['nodes--%{trusted.hostname}--', 'common--'] }), lookup_context).unwrap)
+        .to eq secret_value
+    end
+    # rubocop:enable RSpec/NamedSubject
+
+    # rubocop:disable RSpec/NamedSubject
+    it 'returns the key if found using the first prefix' do
+      access_token_value = 'access_value'
+      secret_value = 'secret_value'
+      allow(TragicCode::Azure).to receive(:get_access_token).and_return(access_token_value)
+      allow(TragicCode::Azure).to receive(:get_secret).and_return(nil)
+      allow(TragicCode::Azure).to receive(:get_secret).with(
+          options['vault_name'],
+          'nodes--WIN-P-01-domain-com--profile--windows--sqlserver--sensitive-azure-sql-user-password',
+          options['vault_api_version'],
+          access_token_value,
+          '',
+        ).and_return('secret_value')
+
+      expect(subject.execute('profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => ['nodes--WIN_P-01.domain.com--', 'common--'] }), lookup_context).unwrap)
+        .to eq secret_value
+    end
+    # rubocop:enable RSpec/NamedSubject
+
+    # rubocop:disable RSpec/NamedSubject
+    # If someone uses a puppet fact in their one of their prefixes (EX: 'nodes--%{trusted.hostname}--') instead of erroring out
+    # and forcing them to update the fact ( change the node name ) lets just normalize all prefixes
+    it 'normalizes prefixes to prevent issues for users' do
+      access_token_value = 'access_value'
+      secret_value = 'secret_value'
+      allow(TragicCode::Azure).to receive(:get_access_token).and_return(access_token_value)
+      expect(TragicCode::Azure).to receive(:get_secret).with(
+          options['vault_name'],
+          'nodes--WIN-P-01-domain-com--profile--windows--sqlserver--sensitive-azure-sql-user-password',
+          options['vault_api_version'],
+          access_token_value,
+          '',
+        ).and_return('secret_value')
+
+      expect(subject.execute('profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => ['nodes--WIN_P-01.domain.com--'] }), lookup_context).unwrap)
+        .to eq secret_value
+    end
+    # rubocop:enable RSpec/NamedSubject
+
+    # rubocop:disable RSpec/NamedSubject
+    it 'returns the key from the cache if it exists with the prefix' do
+      secret_value = 'secret_value'
+
+      allow(lookup_context).to receive(:cache_has_key).and_return(false)
+
+      expect(lookup_context).to receive(:cache_has_key).with('common--profile--windows--sqlserver--sensitive-azure-sql-user-password').and_return(true)
+      expect(lookup_context).to receive(:cached_value).with('common--profile--windows--sqlserver--sensitive-azure-sql-user-password').and_return(secret_value)
+      expect(subject.execute('profile::windows::sqlserver::sensitive_azure_sql_user_password', options.merge({ 'prefixes' => ['nodes--%{trusted.hostname}--', 'common--'] }),
+lookup_context).unwrap).to eq secret_value
+    end
+    # rubocop:enable RSpec/NamedSubject
+  end
 end

spec/functions/tragic_code/azure_spec.rb

@@ -27,7 +27,7 @@
 
   context '.get_access_token_azure_arc' do
     it 'returns a bearer token' do
-      File.stub(:read).and_return('magical-token-from-file')
+      allow(File).to receive(:read).and_return('magical-token-from-file')
 
       stub_request(:get, %r{127.0.0.1})
         .to_return(
@@ -56,7 +56,7 @@
     end
 
     it 'throws error with response body when response is not 2xx when getting the auth token' do
-      File.stub(:read).and_return('magical-token-from-file')
+      allow(File).to receive(:read).and_return('magical-token-from-file')
       # rubocop:disable Layout/LineLength
       stub_request(:get, %r{127.0.0.1})
         .to_return(body: '{"access_token": "token"}', status: 401, headers: { 'Www-Authenticate' => 'Basic realm=C:\\ProgramData\\AzureConnectedMachineAgent\\Tokens\\f1da0584-97f4-42fd-a671-879ad3de86fa.key' })