Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(SDK-137) Add puppet syntax validation #105

Merged
merged 10 commits into from
Jun 28, 2017
5 changes: 5 additions & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ before_test:
test_script:
- bundle exec rake spec acceptance:local

# Uncomment this block to enable RDP access to the AppVeyor test instance for
# debugging purposes.
# on_finish:
# - ps: $blockRdp = $true; iex ((new-object net.webclient).DownloadString('https://raw.githubusercontent.com/appveyor/ci/master/scripts/enable-rdp.ps1'))

notifications:
- provider: HipChat
auth_token:
Expand Down
2 changes: 1 addition & 1 deletion lib/pdk/cli/util/option_normalizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def self.report_formats(formats)
target = $stderr
end

{ method: "to_#{format}".to_sym, target: target }
{ method: "write_#{format}".to_sym, target: target }
end
end

Expand Down
9 changes: 6 additions & 3 deletions lib/pdk/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def self.formats

# @return [Symbol] the method name of the default report format.
def self.default_format
:to_text
:write_text
end

# @return [#write] the default target to write the report to.
Expand Down Expand Up @@ -47,7 +47,7 @@ def add_event(data)
#
# @param target [#write] an IO object that the report will be written to.
# Defaults to PDK::Report.default_target.
def to_junit(target = self.class.default_target)
def write_junit(target = self.class.default_target)
document = REXML::Document.new
document << REXML::XMLDecl.new
testsuites = REXML::Element.new('testsuites')
Expand Down Expand Up @@ -84,7 +84,10 @@ def to_junit(target = self.class.default_target)
#
# @param target [#write] an IO object that the report will be written to.
# Defaults to PDK::Report.default_target.
def to_text(target = self.class.default_target)
def write_text(target = self.class.default_target)
# Extra defaulting here, b/c the Class.send method will pass in nil
target ||= self.class.default_target

events.each do |_tool, tool_events|
tool_events.each do |event|
target.puts(event.to_text) unless event.pass?
Expand Down
1 change: 1 addition & 0 deletions lib/pdk/report/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ def skipped?
# @return [String] The rendered event.
def to_text
location = [file, line, column].compact.join(':')
location = nil if location.empty?

# TODO: maybe add trace
[location, severity, message].compact.join(': ')
Expand Down
10 changes: 2 additions & 8 deletions lib/pdk/validators/base_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def self.invoke(report, options = {})

PDK::Util::Bundler.ensure_binstubs!(cmd)
cmd_argv = parse_options(options, targets).unshift(cmd_path)
cmd_argv.unshift('ruby') if Gem.win_platform?
cmd_argv.unshift('ruby', '-W0') if Gem.win_platform?

PDK.logger.debug(_('Running %{cmd}') % { cmd: cmd_argv.join(' ') })

Expand All @@ -56,13 +56,7 @@ def self.invoke(report, options = {})

result = command.execute!

begin
json_data = JSON.parse(result[:stdout])
rescue JSON::ParserError
json_data = []
end

parse_output(report, json_data, targets)
parse_output(report, result, targets)

result[:exit_code]
end
Expand Down
8 changes: 7 additions & 1 deletion lib/pdk/validators/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ def self.parse_options(_options, targets)
cmd_options.concat(targets)
end

def self.parse_output(report, json_data, _targets)
def self.parse_output(report, result, _targets)
begin
json_data = JSON.parse(result[:stdout])
rescue JSON::ParserError
json_data = []
end

if json_data.empty?
report.add_event(
file: 'metadata.json',
Expand Down
8 changes: 7 additions & 1 deletion lib/pdk/validators/puppet/puppet_lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ def self.parse_options(_options, targets)
cmd_options.concat(targets)
end

def self.parse_output(report, json_data, targets)
def self.parse_output(report, result, targets)
begin
json_data = JSON.parse(result[:stdout])
rescue JSON::ParserError
json_data = []
end

# puppet-lint does not include files without problems in its JSON
# output, so we need to go through the list of targets and add passing
# events to the report for any target not listed in the JSON output.
Expand Down
34 changes: 0 additions & 34 deletions lib/pdk/validators/puppet/puppet_parser.rb

This file was deleted.

82 changes: 82 additions & 0 deletions lib/pdk/validators/puppet/puppet_syntax.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
require 'pdk'
require 'pdk/cli/exec'
require 'pdk/validators/base_validator'

module PDK
module Validate
class PuppetSyntax < BaseValidator
def self.name
'puppet-syntax'
end

def self.cmd
'puppet'
end

def self.pattern
'**/**.pp'
end

def self.spinner_text
_('Checking Puppet manifest syntax')
end

def self.parse_options(_options, targets)
%w[parser validate].concat(targets)
end

def self.parse_output(report, result, targets)
# Due to PUP-7504, we will have to programmatically construct the json
# object from the text output for now.
output = result[:stderr].split("\n")

results_data = []
output.each do |offense|
sanitize_console_output(offense)
message, _at, location_raw = offense.partition(' at ')

# Parse the offense type and msg
severity, _colon, message = message.rpartition(': ')

# Parse the offense location info
location = location_raw.strip.match(%r{\A(?<file>.+):(?<line>\d+):(?<column>\d+)\Z}) unless location_raw.nil?

attributes = {
source: name,
message: message.strip,
state: 'failure',
}
attributes[:severity] = severity.strip unless severity.nil?

unless location.nil?
attributes[:file] = location[:file]
attributes[:line] = location[:line]
attributes[:column] = location[:column]
end

results_data << attributes
end

# puppet-lint does not include files without problems in its JSON
# output, so we need to go through the list of targets and add passing
# events to the report for any target not listed in the JSON output.
targets.reject { |target| results_data.any? { |j| j[:file] == target } }.each do |target|
report.add_event(
file: target,
source: 'puppet-syntax',
severity: 'ok',
state: :passed,
)
end

results_data.each do |offense|
report.add_event(offense)
end
end

def self.sanitize_console_output(line)
line.gsub!(%r{\e\[([;\d]+)?m}, '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be avoided by setting Puppet[:color] = false in https://github.com/voxpupuli/puppet-syntax/blob/master/lib/puppet-syntax/manifests.rb#L19 , or a similar place ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't want to change the default behavior of puppet-syntax itself and strip the colors, since users of this tool outside of PDK may still want the distinct console colors. I could add a Rakefile option to disable the colors in puppet-syntax and toggle it on or off based on that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good enough for now.

end
end
end
end
4 changes: 2 additions & 2 deletions lib/pdk/validators/puppet_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
require 'pdk/cli/exec'
require 'pdk/validators/base_validator'
require 'pdk/validators/puppet/puppet_lint'
require 'pdk/validators/puppet/puppet_parser'
require 'pdk/validators/puppet/puppet_syntax'

module PDK
module Validate
Expand All @@ -12,7 +12,7 @@ def self.name
end

def self.puppet_validators
[PuppetLint, PuppetParser]
[PuppetSyntax, PuppetLint]
end

def self.invoke(report, options = {})
Expand Down
8 changes: 7 additions & 1 deletion lib/pdk/validators/ruby/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ def self.parse_options(_options, targets)
cmd_options.concat(targets)
end

def self.parse_output(report, json_data, _targets)
def self.parse_output(report, result, _targets)
begin
json_data = JSON.parse(result[:stdout])
rescue JSON::ParserError
json_data = []
end

return unless json_data.key?('files')

json_data['files'].each do |file_info|
Expand Down
34 changes: 34 additions & 0 deletions spec/acceptance/validate_all_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require 'spec_helper_acceptance'

describe 'Running all validations' do
let(:junit_xsd) { File.join(RSpec.configuration.fixtures_path, 'JUnit.xsd') }

context 'with a fresh module' do
include_context 'in a new module', 'foo'

init_pp = File.join('manifests', 'init.pp')

before(:all) do
File.open(init_pp, 'w') do |f|
f.puts <<-EOS
# foo
class foo { }
EOS
end
end

describe command('pdk validate --list') do
its(:exit_status) { is_expected.to eq(0) }
its(:stdout) { is_expected.to match(%r{Available validators: metadata, puppet, ruby}i) }
end

describe command('pdk validate') do
its(:exit_status) { is_expected.to eq(0) }
its(:stdout) { is_expected.to match(%r{Running all available validators}i) }
its(:stderr) { is_expected.to match(%r{Checking metadata.json}i) }
its(:stderr) { is_expected.to match(%r{Checking Puppet manifest syntax}i) }
its(:stderr) { is_expected.to match(%r{Checking Puppet manifest style}i) }
its(:stderr) { is_expected.to match(%r{Checking Ruby code style}i) }
end
end
end
Loading