Skip to content

Commit

Permalink
Auto-configurable safely autocorrected offenses
Browse files Browse the repository at this point in the history
  • Loading branch information
LukasAud committed May 15, 2024
1 parent 25cc964 commit b0baf5b
Show file tree
Hide file tree
Showing 12 changed files with 172 additions and 268 deletions.
94 changes: 0 additions & 94 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,6 @@
# 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
# This cop supports safe autocorrection (--autocorrect).
Layout/RescueEnsureAlignment:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'

# Offense count: 10
# This cop supports safe autocorrection (--autocorrect).
Lint/RedundantCopDisableDirective:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'
- 'spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb'
- 'spec/unit/pwsh/util_spec.rb'

# Offense count: 1
# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns, inherit_mode.
# AllowedMethods: refine
Expand Down Expand Up @@ -57,13 +43,6 @@ RSpec/FilePath:
- 'spec/unit/pwsh/version_spec.rb'
- 'spec/unit/pwsh/windows_powershell_spec.rb'

# Offense count: 40
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: .
# SupportedStyles: implicit, each, example
RSpec/HookArgument:
EnforcedStyle: implicit

# Offense count: 1
RSpec/MultipleDescribes:
Exclude:
Expand All @@ -74,17 +53,6 @@ RSpec/SubjectStub:
Exclude:
- 'spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb'

# Offense count: 1
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle, ProceduralMethods, FunctionalMethods, AllowedMethods, AllowedPatterns, AllowBracesOnProceduralOneLiners, BracesRequiredMethods.
# SupportedStyles: line_count_based, semantic, braces_for_chaining, always_braces
# ProceduralMethods: benchmark, bm, bmbm, create, each_with_object, measure, new, realtime, tap, with_object
# FunctionalMethods: let, let!, subject, watch
# AllowedMethods: lambda, proc, it
Style/BlockDelimiters:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'

# Offense count: 3
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: EnforcedStyle.
Expand Down Expand Up @@ -116,65 +84,3 @@ Style/MethodCalledOnDoEndBlock:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'

# Offense count: 123
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle, AllowInnerSlashes.
# SupportedStyles: slashes, percent_r, mixed
Style/RegexpLiteral:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'
- 'lib/pwsh.rb'
- 'lib/pwsh/util.rb'
- 'spec/acceptance/dsc/basic.rb'
- 'spec/acceptance/dsc/cim_instances.rb'
- 'spec/acceptance/dsc/class.rb'
- 'spec/acceptance/dsc/complex.rb'
- 'spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb'
- 'spec/unit/pwsh/util_spec.rb'
- 'spec/unit/pwsh_spec.rb'

# Offense count: 18
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: MinSize.
# SupportedStyles: percent, brackets
Style/SymbolArray:
EnforcedStyle: percent

# Offense count: 5
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyle, AllowSafeAssignment.
# SupportedStyles: require_parentheses, require_no_parentheses, require_parentheses_when_complex
Style/TernaryParentheses:
Exclude:
- 'lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb'
- 'lib/pwsh.rb'
- 'spec/unit/pwsh_spec.rb'

# Offense count: 7
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyleForMultiline.
# SupportedStylesForMultiline: comma, consistent_comma, no_comma
Style/TrailingCommaInArguments:
Exclude:
- 'spec/unit/pwsh_spec.rb'

# Offense count: 23
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: EnforcedStyleForMultiline.
# SupportedStylesForMultiline: comma, consistent_comma, no_comma
Style/TrailingCommaInArrayLiteral:
Exclude:
- 'lib/pwsh.rb'
- 'spec/acceptance/dsc/basic.rb'
- 'spec/acceptance/dsc/class.rb'
- 'spec/spec_helper.rb'
- 'spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb'
- 'spec/unit/pwsh/util_spec.rb'
- 'spec/unit/pwsh_spec.rb'

# Offense count: 27
# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: MinSize, WordRegex.
# SupportedStyles: percent, brackets
Style/WordArray:
EnforcedStyle: percent
56 changes: 27 additions & 29 deletions lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
require 'pathname'
require 'json'

class Puppet::Provider::DscBaseProvider # rubocop:disable Metrics/ClassLength, Style/Documentation
class Puppet::Provider::DscBaseProvider # rubocop:disable Style/Documentation
# Initializes the provider, preparing the instance variables which cache:
# - the canonicalized resources across calls
# - query results
Expand All @@ -28,10 +28,10 @@ def initialize
# @param hashes [Array] the list of hashes to search the cache for
# @return [Array] an array containing the matching hashes for the search condition, if any
def fetch_cached_hashes(cache, hashes)
cache.reject do |item|
cache.reject { |item|
matching_hash = hashes.select { |hash| (item.to_a - hash.to_a).empty? || (hash.to_a - item.to_a).empty? }
matching_hash.empty?
end.flatten
}.flatten
end

# Implements the canonicalize feature of the Resource API; this method is called first against any resources
Expand All @@ -44,7 +44,6 @@ def fetch_cached_hashes(cache, hashes)
# @param context [Object] the Puppet runtime context to operate in and send feedback to
# @param resources [Hash] the hash of the resource to canonicalize from either manifest or invocation
# @return [Hash] returns a hash representing the current state of the object, if it exists
# rubocop:disable Metrics/BlockLength, Metrics/MethodLength
def canonicalize(context, resources)
canonicalized_resources = []
resources.collect do |r|
Expand All @@ -63,7 +62,6 @@ def canonicalize(context, resources)
canonicalized = invoke_get_method(context, r)
# If the resource could not be found or was returned as absent, skip case munging and
# treat the manifest values as canonical since the resource is being created.
# rubocop:disable Metrics/BlockNesting
if canonicalized.nil? || canonicalized[:dsc_ensure] == 'absent'
canonicalized = r.dup
@cached_canonicalized_resource << r.dup
Expand Down Expand Up @@ -268,7 +266,7 @@ def invoke_dsc_resource(context, name_hash, props, method)
return nil
end
context.debug("raw data received: #{data.inspect}")
collision_error_matcher = /The Invoke-DscResource cmdlet is in progress and must return before Invoke-DscResource can be invoked/
collision_error_matcher = %r{The Invoke-DscResource cmdlet is in progress and must return before Invoke-DscResource can be invoked}

error = data['errormessage']

Expand Down Expand Up @@ -351,7 +349,7 @@ def insync?(context, name, _property_name, _is_hash, should_hash)
# @param context [Object] the Puppet runtime context to operate in and send feedback to
# @param name_hash [Hash] the hash of namevars to be passed as properties to `Invoke-DscResource`
# @return [Hash] returns a hash representing the DSC resource munged to the representation the Puppet Type expects
def invoke_get_method(context, name_hash) # rubocop:disable Metrics/AbcSize
def invoke_get_method(context, name_hash)
context.debug("retrieving #{name_hash.inspect}")

query_props = name_hash.select { |k, v| mandatory_get_attributes(context).include?(k) || (k == :dsc_psdscrunascredential && !v.nil?) }
Expand All @@ -378,13 +376,13 @@ def invoke_get_method(context, name_hash) # rubocop:disable Metrics/AbcSize
end

# Convert DateTime back to appropriate type
if context.type.attributes[type_key][:mof_type] =~ /DateTime/i && !data[type_key].nil?
if context.type.attributes[type_key][:mof_type] =~ %r{DateTime}i && !data[type_key].nil?
data[type_key] = begin
Puppet::Pops::Time::Timestamp.parse(data[type_key]) if context.type.attributes[type_key][:mof_type] =~ /DateTime/i && !data[type_key].nil?
rescue ArgumentError, TypeError => e
# Catch any failures in the parse, output them to the context and then return nil
context.err("Value returned for DateTime (#{data[type_key].inspect}) failed to parse: #{e}")
nil
Puppet::Pops::Time::Timestamp.parse(data[type_key]) if context.type.attributes[type_key][:mof_type] =~ %r{DateTime}i && !data[type_key].nil?
rescue ArgumentError, TypeError => e
# Catch any failures in the parse, output them to the context and then return nil
context.err("Value returned for DateTime (#{data[type_key].inspect}) failed to parse: #{e}")
nil
end
end
# PowerShell does not distinguish between a return of empty array/string
Expand Down Expand Up @@ -424,7 +422,7 @@ def invoke_get_method(context, name_hash) # rubocop:disable Metrics/AbcSize
def invoke_set_method(context, name, should)
context.debug("Invoking Set Method for '#{name}' with #{should.inspect}")

apply_props = should.select { |k, _v| k.to_s =~ /^dsc_/ }
apply_props = should.select { |k, _v| k.to_s =~ %r{^dsc_} }
invoke_dsc_resource(context, should, apply_props, 'set')

# TODO: Implement this functionality for notifying a DSC reboot?
Expand All @@ -442,7 +440,7 @@ def invoke_test_method(context, name, should)
context.debug("Relying on DSC Test method for validating if '#{name}' is in the desired state")
context.debug("Invoking Test Method for '#{name}' with #{should.inspect}")

test_props = should.select { |k, _v| k.to_s =~ /^dsc_/ }
test_props = should.select { |k, _v| k.to_s =~ %r{^dsc_} }
data = invoke_dsc_resource(context, name, test_props, 'test')
# Something went wrong with Invoke-DscResource; fall back on property state comparisons
return nil if data.nil?
Expand Down Expand Up @@ -501,7 +499,7 @@ def vendored_modules_path(module_name)
# The new vendored_modules_path: puppet_x/<module_name>/dsc_resources
root_module_path = load_path.grep(%r{#{puppetize_name(module_name)}/lib}).first
vendored_path = if root_module_path.nil?
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + "puppet_x/#{puppetize_name(module_name)}/dsc_resources") # rubocop:disable Style/StringConcatenation
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + "puppet_x/#{puppetize_name(module_name)}/dsc_resources")
else
File.expand_path("#{root_module_path}/puppet_x/#{puppetize_name(module_name)}/dsc_resources")
end
Expand All @@ -510,7 +508,7 @@ def vendored_modules_path(module_name)
# checking for this first will always work and so the more specific search will never run.
unless File.exist? vendored_path
vendored_path = if root_module_path.nil?
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + 'puppet_x/dsc_resources') # rubocop:disable Style/StringConcatenation
File.expand_path(Pathname.new(__FILE__).dirname + '../../../' + 'puppet_x/dsc_resources')
else
File.expand_path("#{root_module_path}/puppet_x/dsc_resources")
end
Expand Down Expand Up @@ -538,9 +536,9 @@ def puppetize_name(name)
# Puppet module names must be lower case
name = name.downcase
# Puppet module names must only include lowercase letters, digits and underscores
name = name.gsub(/[^a-z0-9_]/, '_')
name = name.gsub(%r{[^a-z0-9_]}, '_')
# Puppet module names must not start with a number so if it does, append the letter 'a' to the name. Eg: '7zip' becomes 'a7zip'
name = name.match?(/^\d/) ? "a#{name}" : name # rubocop:disable Lint/UselessAssignment
name = name.match?(%r{^\d}) ? "a#{name}" : name # rubocop:disable Lint/UselessAssignment
end

# Return a UUID with the dashes turned into underscores to enable the specifying of guaranteed-unique
Expand Down Expand Up @@ -718,7 +716,7 @@ def enum_values(context, attribute)
return [] unless type_string&.include?('Enum[')

# Extract the enum values from the type string
enum_content = type_string.match(/Enum\[(.*?)\]/)&.[](1)
enum_content = type_string.match(%r{Enum\[(.*?)\]})&.[](1)

# Return an empty array if we couldn't find the enum values
return [] if enum_content.nil?
Expand Down Expand Up @@ -783,7 +781,7 @@ def prepare_credentials(resource)
instantiated_variables.merge!(variable_name => credential_hash)
end
credentials_block.join("\n")
credentials_block == [] ? '' : credentials_block
(credentials_block == []) ? '' : credentials_block
end

# Write a line of PowerShell which creates a PSCredential object and assigns it to a variable
Expand Down Expand Up @@ -825,7 +823,7 @@ def prepare_cim_instances(resource)
end
end
# We have to handle arrays of CIM instances slightly differently
if /\[\]$/.match?(property_hash[:mof_type])
if %r{\[\]$}.match?(property_hash[:mof_type])
class_name = property_hash[:mof_type].gsub('[]', '')
property_hash[:value].each do |hash|
variable_name = random_variable_name
Expand All @@ -839,7 +837,7 @@ def prepare_cim_instances(resource)
instantiated_variables.merge!(variable_name => property_hash[:value])
end
end
cim_instances_block == [] ? '' : cim_instances_block.join("\n")
(cim_instances_block == []) ? '' : cim_instances_block.join("\n")
end

# Recursively search for and return CIM instances nested in an enumerable
Expand Down Expand Up @@ -901,7 +899,7 @@ def invoke_params(resource)
end
resource[:parameters].each do |property_name, property_hash|
# strip dsc_ from the beginning of the property name declaration
name = property_name.to_s.gsub(/^dsc_/, '').to_sym
name = property_name.to_s.gsub(%r{^dsc_}, '').to_sym
params[:Property][name] = case property_hash[:mof_type]
when 'PSCredential'
# format can't unwrap Sensitive strings nested in arbitrary hashes/etc, so make
Expand All @@ -925,7 +923,7 @@ def invoke_params(resource)
# CIM instances do not do a consistent job of casting an empty array properly.
empty_array_parameters = resource[:parameters].select { |_k, v| v[:value].is_a?(Array) && v[:value].empty? }
empty_array_parameters.each do |name, properties|
param_block_name = name.to_s.gsub(/^dsc_/, '')
param_block_name = name.to_s.gsub(%r{^dsc_}, '')
params_block = params_block.gsub("#{param_block_name} = @()", "#{param_block_name} = [#{properties[:mof_type]}]@()")
end
# HACK: make CIM instances work:
Expand Down Expand Up @@ -1017,7 +1015,7 @@ def escape_quotes(text)
# With multiple methods which need to discover secrets it is necessary to keep a single regex
# which can discover them. This will lazily match everything in a single-quoted string which
# ends with the secret postfix id and mark the actual contents of the string as the secret.
SECRET_DATA_REGEX = /'(?<secret>[^']+)+?#{Regexp.quote(SECRET_POSTFIX)}'/.freeze
SECRET_DATA_REGEX = %r{'(?<secret>[^']+)+?#{Regexp.quote(SECRET_POSTFIX)}'}.freeze

# Strings containing sensitive data have a secrets postfix. These strings cannot be passed
# directly either to debug streams or to PowerShell and must be handled; this method contains
Expand All @@ -1030,12 +1028,12 @@ def escape_quotes(text)
def handle_secrets(text, replacement, error_message)
# Every secret unwrapped in this module will unwrap as "'secret#{SECRET_POSTFIX}'"
# Currently, no known resources specify a SecureString instead of a PSCredential object.
return text unless /#{Regexp.quote(SECRET_POSTFIX)}/.match?(text)
return text unless %r{#{Regexp.quote(SECRET_POSTFIX)}}.match?(text)

# In order to reduce time-to-parse, look at each line individually and *only* attempt
# to substitute if a naive match for the secret postfix is found on the line.
modified_text = text.split("\n").map do |line|
if /#{Regexp.quote(SECRET_POSTFIX)}/.match?(line)
if %r{#{Regexp.quote(SECRET_POSTFIX)}}.match?(line)
line.gsub(SECRET_DATA_REGEX, replacement)
else
line
Expand All @@ -1045,7 +1043,7 @@ def handle_secrets(text, replacement, error_message)
modified_text = modified_text.join("\n")

# Something has gone wrong, error loudly
raise error_message if /#{Regexp.quote(SECRET_POSTFIX)}/.match?(modified_text)
raise error_message if %r{#{Regexp.quote(SECRET_POSTFIX)}}.match?(modified_text)

modified_text
end
Expand Down
8 changes: 4 additions & 4 deletions lib/pwsh.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def make_ps_code(powershell_code, timeout_ms = nil, working_dir = nil, environme
if (envlist = environment_variables)
envlist = [envlist] unless envlist.is_a? Array
envlist.each do |setting|
if setting =~ /^(\w+)=((.|\n)+)$/
if setting =~ %r{^(\w+)=((.|\n)+)$}
env_name = Regexp.last_match(1)
value = Regexp.last_match(2)
if environment.include?(env_name) || environment.include?(env_name.to_sym)
Expand Down Expand Up @@ -538,7 +538,7 @@ def drain_pipe_until_signaled(pipe, signal)
read_from_pipe(pipe, 0) { |s| output << s } while self.class.readable?(pipe)

# String has been binary up to this point, so force UTF-8 now
output == [] ? [] : [output.join.force_encoding(Encoding::UTF_8)]
(output == []) ? [] : [output.join.force_encoding(Encoding::UTF_8)]
end

# Open threads and pipes to read stdout and stderr from the PowerShell manager,
Expand Down Expand Up @@ -589,8 +589,8 @@ def read_streams

[
output,
stdout == [] ? nil : stdout.join, # native stdout
stderr_reader.value # native stderr
(stdout == []) ? nil : stdout.join, # native stdout
stderr_reader.value, # native stderr
]
ensure
# Failsafe if the prior unlock was never reached / Mutex wasn't unlocked
Expand Down
10 changes: 5 additions & 5 deletions lib/pwsh/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Util
# @return [Bool] true if on windows
def on_windows?
host_os = RbConfig::CONFIG['host_os']
!!(host_os =~ /mswin|mingw/)
!!(host_os =~ %r{mswin|mingw})
end

# Verify paths specified are valid directories which exist.
Expand Down Expand Up @@ -41,11 +41,11 @@ def snake_case(object)
raise "snake_case method only handles strings and symbols, passed a #{object.class}: #{object}" unless should_symbolize || object.is_a?(String)

text = object.to_s
.gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2')
.gsub(/([a-z\d])([A-Z])/, '\1_\2')
.gsub(%r{([A-Z]+)([A-Z][a-z])}, '\1_\2')
.gsub(%r{([a-z\d])([A-Z])}, '\1_\2')
.tr('-', '_')
.gsub(/\s/, '_')
.gsub(/__+/, '_')
.gsub(%r{\s}, '_')
.gsub(%r{__+}, '_')
.downcase
should_symbolize ? text.to_sym : text
end
Expand Down
Loading

0 comments on commit b0baf5b

Please sign in to comment.