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

rubocop: various small cleanups part 1 #82

Merged
merged 11 commits into from
Jun 10, 2022
87 changes: 1 addition & 86 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,46 +1,11 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2022-06-10 12:48:42 UTC using RuboCop version 1.22.3.
# on 2022-06-10 13:03:56 UTC using RuboCop version 1.22.3.
# 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
# versions of RuboCop, may require this file to be generated again.

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces.
# SupportedStyles: space, no_space
# SupportedStylesForEmptyBraces: space, no_space
Layout/SpaceBeforeBlockBraces:
Exclude:
- 'spec/unit/puppet/catalog_diff/comparer_spec.rb'

# Offense count: 2
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters.
# SupportedStyles: space, no_space
# SupportedStylesForEmptyBraces: space, no_space
Layout/SpaceInsideBlockBraces:
Exclude:
- 'spec/unit/puppet/catalog_diff/comparer_spec.rb'

# Offense count: 3
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, EnforcedStyleForEmptyBraces.
# SupportedStyles: space, no_space, compact
# SupportedStylesForEmptyBraces: space, no_space
Layout/SpaceInsideHashLiteralBraces:
Exclude:
- 'lib/puppet/catalog-diff/compilecatalog.rb'
- 'lib/puppet/catalog-diff/searchfacts.rb'

# Offense count: 4
# Cop supports --auto-correct.
# Configuration parameters: AllowInHeredoc.
Layout/TrailingWhitespace:
Exclude:
- 'spec/unit/puppet/catalog_diff/comparer_spec.rb'

# Offense count: 21
# Cop supports --auto-correct.
Lint/AmbiguousOperatorPrecedence:
Expand Down Expand Up @@ -116,25 +81,11 @@ Style/CaseLikeIf:
- 'lib/puppet/catalog-diff/formater.rb'
- 'lib/puppet/face/catalog/diff.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: IgnoredMethods.
# IgnoredMethods: ==, equal?, eql?
Style/ClassEqualityComparison:
Exclude:
- 'lib/puppet/catalog-diff/comparer.rb'

# Offense count: 1
Style/ClassVars:
Exclude:
- 'lib/puppet/catalog-diff/comparer.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/CollectionCompact:
Exclude:
- 'lib/puppet/catalog-diff/searchfacts.rb'

# Offense count: 3
# Cop supports --auto-correct.
Style/ColonMethodCall:
Expand Down Expand Up @@ -190,14 +141,6 @@ Style/HashConversion:
- 'lib/puppet/face/catalog/diff.rb'
- 'lib/puppet/face/catalog/pull.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, UseHashRocketsWithSymbolValues, PreferHashRocketsForNonAlnumEndingSymbols.
# SupportedStyles: ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys
Style/HashSyntax:
Exclude:
- 'lib/puppet/catalog-diff/comparer.rb'

# Offense count: 11
# Cop supports --auto-correct.
Style/IfUnlessModifier:
Expand All @@ -220,41 +163,13 @@ Style/MultilineBlockChain:
Exclude:
- 'lib/puppet/face/catalog/diff.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: AllowMethodComparison.
Style/MultipleComparison:
Exclude:
- 'lib/puppet/catalog-diff/compilecatalog.rb'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, IgnoredMethods.
# SupportedStyles: predicate, comparison
Style/NumericPredicate:
Exclude:
- 'spec/**/*'
- 'lib/puppet/catalog-diff/differ.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/RedundantAssignment:
Exclude:
- 'lib/puppet/catalog-diff/searchfacts.rb'

# Offense count: 3
# Cop supports --auto-correct.
Style/RedundantParentheses:
Exclude:
- 'lib/puppet/catalog-diff/differ.rb'
- 'lib/puppet/catalog-diff/formater.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/RedundantSelfAssignment:
Exclude:
- 'lib/puppet/catalog-diff/searchfacts.rb'

# Offense count: 7
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, AllowInnerSlashes.
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/catalog-diff/comparer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def sort_dependencies!(params)
params.each do |x|
next unless %i[require before notify subscribe].include?(x[0])

if x[1].class == Array
if x[1].instance_of?(Array)
x[1].sort!
end
end
Expand Down Expand Up @@ -123,7 +123,7 @@ def do_str_diff(str1, str2)
def validate_encoding(str)
unless str.valid_encoding?
Puppet::debug("Detected that string used in diff had invalid #{str.encoding} encoding. Replacing invalid characters in diff output.")
str.encode!('UTF-8', 'UTF-8', :invalid => :replace)
str.encode!('UTF-8', 'UTF-8', invalid: :replace)
end
str
end
Expand Down
4 changes: 2 additions & 2 deletions lib/puppet/catalog-diff/compilecatalog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def get_catalog_from_puppetdb(node_name, server, puppetdb)
query.concat([['=', 'environment', environment]])
json_query = URI.encode_www_form_component(query.to_json)
request_url = URI("#{puppetdb}/pdb/query/v4/catalogs?query=#{json_query}")
headers = { 'Accept-Content' => 'application/json'}
headers = { 'Accept-Content' => 'application/json' }
ret = Puppet.runtime[:http].get(request_url, headers: headers)
unless ret.success?
raise "HTTP request to PuppetDB failed with: HTTP #{ret.code} - #{ret.reason}"
Expand Down Expand Up @@ -158,7 +158,7 @@ def clean_nested_sensitive_parameters!(catalog)
def redact_sensitive(data)
if data.is_a?(Hash) && data.key?('__ptype')
data[:catalog_diff_hash] = Digest::SHA256.hexdigest Marshal.dump(data['__pvalue'])
data.reject! { |k| k == '__ptype' || k == '__pvalue' }
data.reject! { |k| %w[__ptype __pvalue].include?(k) }
elsif data.is_a? Hash
data.each do |_k, v|
redact_sensitive(v) if v.is_a?(Hash) || v.is_a?(Array)
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/catalog-diff/differ.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def diff(options = {})
output[:added_and_removed_resources] = "#{(!additions.zero? && "+#{additions}" || 0)} / #{(!subtractions.zero? && "-#{subtractions}" || 0)}"

divide_by = (changes_percentage.zero? ? 0 : 1) + (additions_percentage.zero? ? 0 : 1) + (subtractions_percentage.zero? ? 0 : 1)
output[:node_percentage] = (divide_by == 0 && 0 || additions_percentage == 100 && 100 || (changes_percentage + additions_percentage + subtractions_percentage) / divide_by).to_f
output[:node_percentage] = (divide_by.zero? && 0 || additions_percentage == 100 && 100 || (changes_percentage + additions_percentage + subtractions_percentage) / divide_by).to_f
output[:node_differences] = (additions.abs.to_i + subtractions.abs.to_i + changes.abs.to_i)
output
end
Expand Down
11 changes: 5 additions & 6 deletions lib/puppet/catalog-diff/searchfacts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ def build_query(env, version)
base_query = ['and', ['=', %w[node active], true]]
query_field_catalog_environment = Puppet::Util::Package.versioncmp(version, '3') >= 0 ? 'catalog_environment' : 'catalog-environment'
base_query.concat([['=', query_field_catalog_environment, env]]) if env
real_facts = @facts.reject { |_k, v| v.nil? }
real_facts = @facts.compact
query = base_query.concat(real_facts.map { |k, v| ['=', ['fact', k], v] })
classes = Hash[@facts.select { |_k, v| v.nil? }].keys
classes.each do |c|
capit = c.split('::').map(&:capitalize).join('::')
query = query.concat(
query.concat(
[['in', 'certname',
['extract', 'certname',
['select-resources',
Expand All @@ -44,7 +44,7 @@ def build_query(env, version)
end

def get_puppetdb_version(server)
headers = { 'Accept' => 'application/json'}
headers = { 'Accept' => 'application/json' }
result = Puppet.runtime[:http].get(URI("#{server}/pdb/meta/v1/version"), headers: headers)
if result.code == 200
body = JSON.parse(result.body)
Expand All @@ -61,7 +61,7 @@ def find_nodes_puppetdb(env, puppetdb)
puppetdb_version = get_puppetdb_version(puppetdb)
query = build_query(env, puppetdb_version)
json_query = URI.escape(query.to_json)
headers = { 'Accept' => 'application/json'}
headers = { 'Accept' => 'application/json' }
Puppet.debug("Querying #{puppetdb} for environment #{env}")
begin
result = Puppet.runtime[:http].get(URI("#{puppetdb}/pdb/query/v4/nodes?query=#{json_query}"), headers: headers)
Expand All @@ -76,8 +76,7 @@ def find_nodes_puppetdb(env, puppetdb)
rescue PSON::ParserError => e
raise "Error parsing json output of puppet search: #{e.message}"
end
names = filtered.map { |node| node['certname'] }
names
filtered.map { |node| node['certname'] }
end
end
end
10 changes: 5 additions & 5 deletions spec/unit/puppet/catalog_diff/comparer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,18 @@
diffs = compare_resources(res1, res2, show_resource_diff: true)
expect(diffs[:string_diffs]['file.foo'][3]).to eq("-\t content => \"foo content\"")
end

it 'returns string_diffs with show_resource_diff with content encoded in ISO8859-1 (latin1)' do
# Without this the unit test fails when running on LC_ALL=C but works with LC_ALL=en_US.UTF-8
# With this it works on both.
Encoding.default_internal = 'UTF-8'
Encoding.default_external = 'UTF-8' # Needed because diff uses tempfile.

latin1_string = [246].pack('C*').force_encoding('UTF-8')
res1[0][:parameters][:content] = latin1_string
expect{compare_resources(res1, res2, show_resource_diff: true)}.not_to raise_error
res1[0][:parameters][:content] = latin1_string
expect { compare_resources(res1, res2, show_resource_diff: true) }.not_to raise_error
diffs = compare_resources(res1, res2, show_resource_diff: true)

ruby_default_replacement_string_for_invalid_characters = '�'
expect(diffs[:string_diffs]['file.foo'][3]).to \
eq("-\t content => \"" + ruby_default_replacement_string_for_invalid_characters + '"')
Expand Down