Allow deeper checking of hiera keys #73
Conversation
| @@ -7,12 +7,22 @@ | |||
| module PuppetSyntax | |||
| @exclude_paths = [] | |||
| @future_parser = false | |||
| @hieradata_paths = ["**/data/**/*.*yaml", "hieradata/**/*.*yaml", "hiera*.*yaml"] | |||
| @hieradata_paths = [ | |||
There was a problem hiding this comment.
I moved these to new lines to make the diffs nicer, but I can do it in a separate PR if needed
|
|
||
| class << self | ||
| attr_accessor :exclude_paths, :future_parser, :hieradata_paths, :fail_on_deprecation_notices, :epp_only | ||
| attr_accessor :exclude_paths, |
There was a problem hiding this comment.
Same here: I moved these to new lines to make the diffs nicer, but I can do it in a separate PR if needed
| @@ -21,5 +21,7 @@ Gem::Specification.new do |spec| | |||
| spec.add_dependency "rake" | |||
|
|
|||
| spec.add_development_dependency "rspec" | |||
| spec.add_development_dependency "pry" | |||
There was a problem hiding this comment.
I added these for debugging, but can move to a seperate PR if needed
There was a problem hiding this comment.
I dont think they need to be a separate PR but they should probably be separate commits within this PR.
lib/puppet-syntax/hiera.rb
Outdated
| end | ||
| yamldata.each do |k,v| | ||
| if PuppetSyntax.check_hiera_keys | ||
| errors += Array(check_hiera_key(k)).map do |msg| |
There was a problem hiding this comment.
The Array(..).map here is a bit confusing as check_hiera_key either returns a string or nil. Perhaps better to write it simply as:
key_msg = check_heira_key(k)
errors << "WARNING: [..] #{key_msg}" if key_msg
lib/puppet-syntax/hiera.rb
Outdated
|
|
||
| def check_hiera_key(key) | ||
| if key.is_a? Symbol | ||
| if key =~ /^:/ |
There was a problem hiding this comment.
Swap this regexp for better performance. Use key.to_s.start_with(':')
lib/puppet-syntax/hiera.rb
Outdated
| rescue Exception => error | ||
| errors << "ERROR: Failed to parse #{hiera_file}: #{error}" | ||
| next | ||
| end | ||
| yamldata.each do |k,v| |
There was a problem hiding this comment.
It'd be a good idea to check the data type here before trying to iterate as loading an empty YAML file will return false.
There was a problem hiding this comment.
Good catch, added a spec around this also
|
I'll run these against the govuk-puppet code base to see what it throws up and extract examples for further work. |
petems
force-pushed
the
expand_hiera_support
branch
2 times, most recently
from
07791ce
to
fa8eb87
Compare
Resurrected from voxpupuli#57 * Add regexes for checking hiera key formats * Enabled by flag 'check_hiera_keys' which is disabled by default as it's a breaking change * Adds spec to catch issues * Also move multiple entry arrays for attr_accesors into separate lines to make cleaner git history
* Makes diffs easier to read when new added elements
* Allows debugging
petems
force-pushed
the
expand_hiera_support
branch
from
fa8eb87
to
92a3f41
Compare
|
@domcleal Cool, we're pairing on it now so should be good to merge soonish 👍 |
|
SInce this is still an optional feature the warnings caused by https://github.com/alphagov/govuk-puppet/blob/master/hieradata/common.yaml#L2 and https://github.com/alphagov/govuk-puppet/blob/master/hieradata/common.yaml#L137 are things we should probably fix in https://github.com/alphagov/govuk-puppet/ anyway. |
Resurrected from #57
it's a breaking change
git history
development debugging easier