Giuseppe Lavagetto has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/382649 )
Change subject: Add checks for nodes
......................................................................
Add checks for nodes
* No hiera call in nodes
* No define or class inclusion/declaration
* one role per node definition
Change-Id: I796af8acb8a981b9a42bb739a43d4d075ef96b8e
---
M lib/puppet-lint/plugins/check_wmf_styleguide.rb
M spec/puppet-lint/plugins/check_wmf_styleguide_check_spec.rb
2 files changed, 130 insertions(+), 2 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
jenkins-bot: Verified
Volans: Looks good to me, but someone else must approve
diff --git a/lib/puppet-lint/plugins/check_wmf_styleguide.rb
b/lib/puppet-lint/plugins/check_wmf_styleguide.rb
index 11a2529..a5d243e 100644
--- a/lib/puppet-lint/plugins/check_wmf_styleguide.rb
+++ b/lib/puppet-lint/plugins/check_wmf_styleguide.rb
@@ -143,6 +143,14 @@
def declared_type?
@type == :NAME && @next_code_token.type == :LBRACE &&
@prev_code_token.type != :CLASS
end
+
+ def node_def?
+ [:SSTRING, :STRING, :NAME, :REGEX].include?(@type)
+ end
+
+ def role_keyword?
+ @type == :NAME && @value = 'role' && @next_code_token.type == :LPAREN
+ end
end
end
end
@@ -323,9 +331,86 @@
notify :error, msg
end
+# rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity,
Metrics/AbcSize, Metrics/CyclomaticComplexity
+def check_node(node)
+ title = node[:title_tokens].map(&:value).join(', ')
+ node[:tokens].each do |token|
+ msg = nil
+ if token.hiera?
+ msg = {
+ message: "wmf-style: Found hiera call in node '#{title}'",
+ line: token.line,
+ column: token.column
+ }
+
+ elsif token.class_include?
+ msg = {
+ message: "wmf-style: node '#{title}' includes class
#{token.included_class.value}",
+ line: token.line,
+ column: token.column
+ }
+ elsif token.declared_class
+ msg = {
+ message: "wmf-style: node '#{title}' declares class
#{token.declared_class.value}",
+ line: token.line,
+ column: token.column
+ }
+ elsif token.declared_type?
+ msg = {
+ message: "wmf-style: node '#{title}' declares #{token.value}",
+ line: token.line,
+ column: token.column
+ }
+ elsif token.role_keyword? &&
token.next_code_token.next_code_token.next_code_token.type != :RPAREN
+ msg = {
+ message: "wmf-style: node '#{title}' includes multiple roles",
+ line: token.line,
+ column: token.column
+ }
+ end
+ notify :error, msg if msg
+ end
+end
+
PuppetLint.new_check(:wmf_styleguide) do
- def check
- # Modules whose classes can be included elsewhere
+ def node_indexes
+ # Override the faulty "node_indexes" method from puppet-lint
+ result = []
+ in_node_def = false
+ braces_level = nil
+ start = 0
+ title_tokens = []
+ tokens.each_with_index do |token, i|
+ if token.type == :NODE
+ braces_level = 0
+ start = i
+ in_node_def = true
+ next
+ end
+ # If we're not within a node definition, skip this token
+ next unless in_node_def
+ case token.type
+ when :LBRACE
+ title_tokens = tokens[start + 1..(i - 1)].select(&:node_def?) if
braces_level.zero?
+ braces_level += 1
+ when :RBRACE
+ braces_level -= 1
+ if braces_level.zero?
+ result << {
+ start: start,
+ end: i,
+ tokens: tokens[start..i],
+ title_tokens: title_tokens
+ }
+ in_node_def = false
+ end
+ end
+ end
+ result
+ end
+ # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity,
Metrics/PercievedComplexity
+
+ def check_classes
class_indexes.each do |cl|
klass = PuppetResource.new(cl)
if klass.profile?
@@ -336,9 +421,16 @@
check_class klass
end
end
+ end
+
+ def check
+ check_classes
defined_type_indexes.each do |df|
define = PuppetResource.new(df)
check_define define
end
+ node_indexes.each do |node|
+ check_node node
+ end
end
end
diff --git a/spec/puppet-lint/plugins/check_wmf_styleguide_check_spec.rb
b/spec/puppet-lint/plugins/check_wmf_styleguide_check_spec.rb
index d848458..d6a0aad 100644
--- a/spec/puppet-lint/plugins/check_wmf_styleguide_check_spec.rb
+++ b/spec/puppet-lint/plugins/check_wmf_styleguide_check_spec.rb
@@ -71,6 +71,23 @@
}
EOF
+node_ok = <<-EOF
+node /^test1.*\.example\.com$/ {
+ role(spare::system)
+}
+EOF
+
+node_ko = <<-EOF
+node 'fixme' {
+ role(spare::system,
+ mediawiki::appserver)
+ include base::firewall
+ interface::mapped { 'eth0':
+ foo => 'bar'
+ }
+}
+EOF
+
describe 'wmf_styleguide' do
context 'class correctly written' do
let(:code) { class_ok }
@@ -141,4 +158,23 @@
expect(problems).to contain_error("wmf-style: defined type 'foo::fixme'
declares class bar from another module").on_line(3)
end
end
+
+ context 'node with no errors' do
+ let(:code) { node_ok }
+ it 'should not detect any problems' do
+ expect(problems).to have(0).problems
+ end
+ end
+ context 'node with violations' do
+ let(:code) { node_ko }
+ it 'should not have multiple roles applied' do
+ expect(problems).to contain_error("wmf-style: node 'fixme' includes
multiple roles").on_line(2)
+ end
+ it 'should not include classes directly' do
+ expect(problems).to contain_error("wmf-style: node 'fixme' includes
class base::firewall")
+ end
+ it 'should not declare any defined type' do
+ expect(problems).to contain_error("wmf-style: node 'fixme' declares
interface::mapped")
+ end
+ end
end
--
To view, visit https://gerrit.wikimedia.org/r/382649
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I796af8acb8a981b9a42bb739a43d4d075ef96b8e
Gerrit-PatchSet: 2
Gerrit-Project: operations/puppet-lint/wmf_styleguide-check
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: Volans <rcocci...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
