From 262a3ccbaee775c851c0bfcd2fdce45640beaaaa Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Tue, 4 Apr 2023 13:55:10 +0200 Subject: [PATCH] Hiera: Test for wrong interpolation syntax This commit adds new base functions to validate hiera *data*. Previously we only tested hiera *keys* and eyaml. This also adds one specific check, to validate the correct syntax for lookup function interpolation. --- .rubocop_todo.yml | 8 ++-- README.md | 22 ++++++++++ lib/puppet-syntax.rb | 4 +- lib/puppet-syntax/hiera.rb | 61 +++++++++++++++++++++++++++ spec/fixtures/hiera/hiera_badkey.yaml | 6 +++ spec/puppet-syntax/hiera_spec.rb | 6 ++- 6 files changed, 101 insertions(+), 6 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 51a8dd6..bdbbf18 100644 --- a/.rubocop_todo.yml +++ b/.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 diff --git a/README.md b/README.md index 1197d5e..99eb5dc 100644 --- a/README.md +++ b/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. diff --git a/lib/puppet-syntax.rb b/lib/puppet-syntax.rb index 8b352da..6a624e9 100644 --- a/lib/puppet-syntax.rb +++ b/lib/puppet-syntax.rb @@ -20,6 +20,7 @@ module PuppetSyntax ] @fail_on_deprecation_notices = true @check_hiera_keys = false + @check_hiera_data = true 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 diff --git a/lib/puppet-syntax/hiera.rb b/lib/puppet-syntax/hiera.rb index 5b32cf7..6bcdd10 100644 --- a/lib/puppet-syntax/hiera.rb +++ b/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 diff --git a/spec/fixtures/hiera/hiera_badkey.yaml b/spec/fixtures/hiera/hiera_badkey.yaml index d1f45f4..da1665f 100644 --- a/spec/fixtures/hiera/hiera_badkey.yaml +++ b/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('this_is_ok'):3306}": [] +this_is::warning7: + - "%{lookup('this_is_ok'):3306}" +this_is::warning8: + foo: "%{lookup('this_is_ok'):3306}" diff --git a/spec/puppet-syntax/hiera_spec.rb b/spec/puppet-syntax/hiera_spec.rb index 1c182bb..1436fbc 100644 --- a/spec/puppet-syntax/hiera_spec.rb +++ b/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