Hiera: Test for wrong interpolation syntax

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-03-08 12:59:05 UTC using RuboCop version 1.61.0.
+# on 2024-03-08 16:09:27 UTC using RuboCop version 1.61.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -86,7 +86,7 @@ RSpec/DescribedClass:
 # Offense count: 7
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
-  Max: 14
+  Max: 16
 
 # Offense count: 4
 # Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly.
@@ -100,7 +100,7 @@ RSpec/FilePath:
 
 # Offense count: 29
 RSpec/MultipleExpectations:
-  Max: 8
+  Max: 10
 
 # Offense count: 30
 # Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
@@ -244,7 +244,7 @@ Style/SymbolProc:
     - 'lib/puppet-syntax/manifests.rb'
     - 'lib/puppet-syntax/templates.rb'
 
-# Offense count: 2
+# Offense count: 3
 # This cop supports safe autocorrection (--autocorrect).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns.
 # URISchemes: http, https

README.md

@@ -85,6 +85,13 @@ This reports common mistakes in key names in Hiera files, such as:
 * Invalid camel casing, such as: `noCamelCase::warning3: true`.
 * Use of hyphens, such as: `no-hyphens::warning4: true`.
 
+
+* To enable a syntax check on Hiera values, set:
+
+```ruby
+PuppetSyntax.check_hiera_data = true
+```
+
 ## Usage
 
 * To enable Puppet::Syntax, include the following in your module's `Rakefile`:
@@ -139,6 +146,21 @@ By default, this rake task looks for all `.yaml` files in a single module under:
 * `hieradata/**/*.yaml`
 * `hiera*.yaml`
 
+It will validate the syntax of each Hiera *key*. for values, it will check if
+the interpolation function syntax is correct. Wrong:
+
+```
+foo:
+  "%{lookup('baz'):3306}": []
+```
+
+correct would be:
+
+```
+foo:
+  "%{lookup('baz')}:3306": []
+```
+
 ### manifests
 
 Checks all `.pp` files in the module for syntax errors.

lib/puppet-syntax.rb

@@ -20,6 +20,7 @@ module PuppetSyntax
   ]
   @fail_on_deprecation_notices = true
   @check_hiera_keys = false
+  @check_hiera_data = false
 
   class << self
     attr_accessor :exclude_paths,
@@ -28,6 +29,7 @@ class << self
                   :templates_paths,
                   :fail_on_deprecation_notices,
                   :epp_only,
-                  :check_hiera_keys
+                  :check_hiera_keys,
+                  :check_hiera_data
   end
 end

lib/puppet-syntax/hiera.rb

@@ -20,6 +20,16 @@ def check_hiera_key(key)
       end
     end
 
+    def check_hiera_data(_key, value)
+      # using filter_map to remove nil values
+      # there will be nil values if check_broken_function_call didn't return a string
+      # this is a shorthand for filter.compact
+      # https://blog.saeloun.com/2019/05/25/ruby-2-7-enumerable-filter-map/
+      keys_and_values(value).filter_map do |element|
+        check_broken_function_call(element)
+      end
+    end
+
     # Recurse through complex data structures.  Return on first error.
     def check_eyaml_data(name, val)
       error = nil
@@ -87,6 +97,11 @@ def check(filelist)
             key_msg = check_hiera_key(k)
             errors << "WARNING: #{hiera_file}: Key :#{k}: #{key_msg}" if key_msg
           end
+          if PuppetSyntax.check_hiera_data
+            check_hiera_data(k, v).each do |value_msg|
+              errors << "WARNING: #{hiera_file}: Key :#{k}: #{value_msg}"
+            end
+          end
           eyaml_msg = check_eyaml_data(k, v)
           errors << "WARNING: #{hiera_file}: #{eyaml_msg}" if eyaml_msg
         end
@@ -96,5 +111,51 @@ def check(filelist)
 
       errors
     end
+
+    private
+
+    # you can call functions in hiera, like this:
+    # "%{lookup('this_is_ok')}"
+    # you can do this in everywhere in a hiera value
+    # you cannot do string concatenation within {}:
+    # "%{lookup('this_is_ok'):3306}"
+    # You can do string concatenation outside of {}:
+    # "%{lookup('this_is_ok')}:3306"
+    def check_broken_function_call(element)
+      'string after a function call but before `}` in the value' if element.is_a?(String) && /%{.+\('.*'\).+}/.match?(element)
+    end
+
+    # gets a hash or array, returns all keys + values as array
+    def flatten_data(data, parent_key = [])
+      if data.is_a?(Hash)
+        data.flat_map do |key, value|
+          current_key = parent_key + [key.to_s]
+          if value.is_a?(Hash) || value.is_a?(Array)
+            flatten_data(value, current_key)
+          else
+            [current_key.join('.'), value]
+          end
+        end
+      elsif data.is_a?(Array) && !data.empty?
+        data.flat_map do |value|
+          if value.is_a?(Hash) || value.is_a?(Array)
+            flatten_data(value, parent_key)
+          else
+            [parent_key.join('.'), value]
+          end
+        end
+      else
+        [parent_key.join('.')]
+      end
+    end
+
+    # gets a string, hash or array, returns all keys + values as flattened + unique array
+    def keys_and_values(value)
+      if value.is_a?(Hash) || value.is_a?(Array)
+        flatten_data(value).flatten.uniq
+      else
+        [value]
+      end
+    end
   end
 end

spec/fixtures/hiera/hiera_badkey.yaml

@@ -9,3 +9,9 @@ typical:typo::warning1:   true
 noCamelCase::warning3:    true
 no-hyphens::warning4:     true
 :picky::warning5:         true
+this_is::warning6:
+  "%{lookup('foobar'):3306}": []
+this_is::warning7:
+  - "%{lookup('foobar'):3306}"
+this_is::warning8:
+  foo: "%{lookup('foobar'):3306}"

spec/puppet-syntax/hiera_spec.rb

@@ -24,11 +24,12 @@
   context 'check_hiera_keys = true' do
     before do
       PuppetSyntax.check_hiera_keys = true
+      PuppetSyntax.check_hiera_data = true
     end
 
     it 'returns warnings for invalid keys' do
       hiera_yaml = 'hiera_badkey.yaml'
-      examples = 5
+      examples = 8
       files = fixture_hiera(hiera_yaml)
       res = subject.check(files)
       (1..examples).each do |n|
@@ -40,6 +41,9 @@
       expect(res[2]).to match('Key :noCamelCase::warning3: Not a valid Puppet variable name for automatic lookup')
       expect(res[3]).to match('Key :no-hyphens::warning4: Not a valid Puppet variable name for automatic lookup')
       expect(res[4]).to match('Key :picky::warning5: Puppet automatic lookup will not look up symbols')
+      expect(res[5]).to match('Key :this_is::warning6: string after a function call but before `}` in the value')
+      expect(res[6]).to match('Key :this_is::warning7: string after a function call but before `}` in the value')
+      expect(res[7]).to match('Key :this_is::warning8: string after a function call but before `}` in the value')
     end
 
     it 'returns warnings for bad eyaml values' do