ManageIQ/httpd_configmap_generator

View on GitHub

Showing 26 of 26 total issues

Method configure_sssd has 40 lines of code (exceeds 25 allowed). Consider refactoring.
Open

    def configure_sssd
      info_msg("Configuring SSSD Service")
      sssd = Sssd.new(opts)
      sssd.load(SSSD_CONFIG)

Severity: Minor
Found in lib/httpd_configmap_generator/ldap.rb - About 1 hr to fix

Method file_binary? has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

      def file_binary?(file)
        data = File.read(file)
        ascii = control = binary = total = 0
        data[0..512].each_byte do |c|
          total += 1
Severity: Minor
Found in lib/httpd_configmap_generator/base/file_helper.rb - About 25 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method file_entry_spec has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

    def file_entry_spec(source_file, target_file = nil, mode = nil)
      target_file = source_file.dup unless target_file
      unless mode
        stat = File.stat(source_file)
        file_owner = Etc.getpwuid(stat.uid).name
Severity: Minor
Found in lib/httpd_configmap_generator/base/config_map.rb - About 25 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Method rename_mellon_configfiles has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
Open

    def rename_mellon_configfiles
      info_msg("Renaming mellon config files")
      Dir.chdir(SAML2_CONFIG_DIRECTORY) do
        Dir.glob("https_*.*") do |mellon_file|
          saml2_file = nil
Severity: Minor
Found in lib/httpd_configmap_generator/saml.rb - About 25 mins to fix

Cognitive Complexity

Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.

A method's cognitive complexity is based on a few simple rules:

  • Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
  • Code is considered more complex for each "break in the linear flow of the code"
  • Code is considered more complex when "flow breaking structures are nested"

Further reading

Use match? instead of =~ when MatchData is not used.
Open

      raise "Must specify a mode for URL file sources" if source_file =~ URI.regexp(%w(http https))

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

metadata['rubygems_mfa_required'] must be set to 'true'.
Open

Gem::Specification.new do |s|
  s.name          = "httpd_configmap_generator"
  s.version       = HttpdConfigmapGenerator::VERSION
  s.authors       = ["Httpd Auth Config Developers"]
  s.homepage      = "https://github.com/ManageIQ/httpd_configmap_generator"
Severity: Minor
Found in httpd_configmap_generator.gemspec by rubocop

Requires a gemspec to have rubygems_mfa_required metadata set.

This setting tells RubyGems that MFA (Multi-Factor Authentication) is required for accounts to be able perform privileged operations, such as (see RubyGems' documentation for the full list of privileged operations):

  • gem push
  • gem yank
  • gem owner --add/remove
  • adding or removing owners using gem ownership page

This helps make your gem more secure, as users can be more confident that gem updates were pushed by maintainers.

Example:

# bad
Gem::Specification.new do |spec|
  # no `rubygems_mfa_required` metadata specified
end

# good
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'true'
  }
end

# good
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'true'
end

# bad
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'false'
  }
end

# good
Gem::Specification.new do |spec|
  spec.metadata = {
    'rubygems_mfa_required' => 'true'
  }
end

# bad
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'false'
end

# good
Gem::Specification.new do |spec|
  spec.metadata['rubygems_mfa_required'] = 'true'
end

Variable @name used in void context.
Open

      @name

Checks for operators, variables, literals, lambda, proc and nonmutating methods used in void context.

Example: CheckForMethodsWithNoSideEffects: false (default)

# bad
def some_method
  some_num * 10
  do_something
end

def some_method(some_var)
  some_var
  do_something
end

Example: CheckForMethodsWithNoSideEffects: true

# bad
def some_method(some_array)
  some_array.sort
  do_something(some_array)
end

# good
def some_method
  do_something
  some_num * 10
end

def some_method(some_var)
  do_something
  some_var
end

def some_method(some_array)
  some_array.sort!
  do_something(some_array)
end

Use match? instead of =~ when MatchData is not used.
Open

      if key =~ /^domain\/.*$/

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

URI.regexp(%w(http https)) is obsolete and should not be used. Instead, use URI::DEFAULT_PARSER.make_regexp(%w(http https)).
Open

      raise "Must specify a mode for URL file sources" if source_file =~ URI.regexp(%w(http https))

Identifies places where URI.regexp is obsolete and should not be used. Instead, use URI::DEFAULT_PARSER.make_regexp.

Example:

# bad
URI.regexp('http://example.com')

# good
URI::DEFAULT_PARSER.make_regexp('http://example.com')

Use filter_map instead.
Open

    constants.collect do |c|
      k = const_get(c)
      k::AUTH[:subtype] if k.kind_of?(Class) && k.constants.include?(:AUTH)
    end.compact
Severity: Minor
Found in lib/httpd_configmap_generator.rb by rubocop

Use atomic file operation method FileUtils.rm_f.
Open

        File.delete(path) if File.exist?(path)

Checks for non-atomic file operation. And then replace it with a nearly equivalent and atomic method.

These can cause problems that are difficult to reproduce, especially in cases of frequent file operations in parallel, such as test runs with parallel_rspec.

For examples: creating a directory if there is none, has the following problems

An exception occurs when the directory didn't exist at the time of exist?, but someone else created it before mkdir was executed.

Subsequent processes are executed without the directory that should be there when the directory existed at the time of exist?, but someone else deleted it shortly afterwards.

Safety:

This cop is unsafe, because autocorrection change to atomic processing. The atomic processing of the replacement destination is not guaranteed to be strictly equivalent to that before the replacement.

Example:

# bad - race condition with another process may result in an error in `mkdir`
unless Dir.exist?(path)
  FileUtils.mkdir(path)
end

# good - atomic and idempotent creation
FileUtils.mkdir_p(path)

# bad - race condition with another process may result in an error in `remove`
if File.exist?(path)
  FileUtils.remove(path)
end

# good - atomic and idempotent removal
FileUtils.rm_f(path)

Use find instead of select.first.
Open

        key = sssd.entries.collect(&:key).select { |k| k.downcase == key.downcase }.first || key

This cop is used to identify usages of select.first, select.last, find_all.first, and find_all.last and change them to use detect instead.

Example:

# bad
[].select { |item| true }.first
[].select { |item| true }.last
[].find_all { |item| true }.first
[].find_all { |item| true }.last

# good
[].detect { |item| true }
[].reverse.detect { |item| true }

ActiveRecord compatibility: ActiveRecord does not implement a detect method and find has its own meaning. Correcting ActiveRecord methods with this cop should be considered unsafe.

Socket.gethostbyname is deprecated in favor of Addrinfo#getaddrinfo.
Open

      Socket.gethostbyname(hostname)[0]

Checks for uses of the deprecated class method usages.

Example:

# bad
File.exists?(some_path)
Dir.exists?(some_path)
iterator?
attr :name, true
attr :name, false
ENV.freeze # Calling `Env.freeze` raises `TypeError` since Ruby 2.7.
ENV.clone
ENV.dup # Calling `Env.dup` raises `TypeError` since Ruby 3.1.
Socket.gethostbyname(host)
Socket.gethostbyaddr(host)

# good
File.exist?(some_path)
Dir.exist?(some_path)
block_given?
attr_accessor :name
attr_reader :name
ENV # `ENV.freeze` cannot prohibit changes to environment variables.
ENV.to_h
ENV.to_h # `ENV.dup` cannot dup `ENV`, use `ENV.to_h` to get a copy of `ENV` as a hash.
Addrinfo.getaddrinfo(nodename, service)
Addrinfo.tcp(host, port).getnameinfo

Call super to initialize state of the parent class.
Open

    def initialize(opts = {})
      @opts = opts
      @config_map = template
    end

Checks for the presence of constructors and lifecycle callbacks without calls to super.

This cop does not consider method_missing (and respond_to_missing?) because in some cases it makes sense to overtake what is considered a missing method. In other cases, the theoretical ideal handling could be challenging or verbose for no actual gain.

Autocorrection is not supported because the position of super cannot be determined automatically.

Object and BasicObject are allowed by this cop because of their stateless nature. However, sometimes you might want to allow other parent classes from this cop, for example in the case of an abstract class that is not meant to be called with super. In those cases, you can use the AllowedParentClasses option to specify which classes should be allowed in addition to Object and BasicObject.

Example:

# bad
class Employee < Person
  def initialize(name, salary)
    @salary = salary
  end
end

# good
class Employee < Person
  def initialize(name, salary)
    super(name)
    @salary = salary
  end
end

# bad
Employee = Class.new(Person) do
  def initialize(name, salary)
    @salary = salary
  end
end

# good
Employee = Class.new(Person) do
  def initialize(name, salary)
    super(name)
    @salary = salary
  end
end

# bad
class Parent
  def self.inherited(base)
    do_something
  end
end

# good
class Parent
  def self.inherited(base)
    super
    do_something
  end
end

# good
class ClassWithNoParent
  def initialize
    do_something
  end
end

Example: AllowedParentClasses: [MyAbstractClass]

# good
class MyConcreteClass < MyAbstractClass
  def initialize
    do_something
  end
end

Passing safe_level with the 2nd argument of ERB.new is deprecated. Do not use it, and specify other arguments as keyword arguments.
Open

          File.write(dest_path, ERB.new(File.read(src_path), nil, '-').result(binding))

Emulates the following Ruby warnings in Ruby 2.6.

$ cat example.rb
ERB.new('hi', nil, '-', '@output_buffer')
$ ruby -rerb example.rb
example.rb:1: warning: Passing safe_level with the 2nd argument of ERB.new is
deprecated. Do not use it, and specify other arguments as keyword arguments.
example.rb:1: warning: Passing trim_mode with the 3rd argument of ERB.new is
deprecated. Use keyword argument like ERB.new(str, trim_mode:...) instead.
example.rb:1: warning: Passing eoutvar with the 4th argument of ERB.new is
deprecated. Use keyword argument like ERB.new(str, eoutvar: ...) instead.

Now non-keyword arguments other than first one are softly deprecated and will be removed when Ruby 2.5 becomes EOL. ERB.new with non-keyword arguments is deprecated since ERB 2.2.0. Use :trim_mode and :eoutvar keyword arguments to ERB.new. This cop identifies places where ERB.new(str, trim_mode, eoutvar) can be replaced by ERB.new(str, :trim_mode: trim_mode, eoutvar: eoutvar).

Example:

# Target codes supports Ruby 2.6 and higher only
# bad
ERB.new(str, nil, '-', '@output_buffer')

# good
ERB.new(str, trim_mode: '-', eoutvar: '@output_buffer')

# Target codes supports Ruby 2.5 and lower only
# good
ERB.new(str, nil, '-', '@output_buffer')

# Target codes supports Ruby 2.6, 2.5 and lower
# bad
ERB.new(str, nil, '-', '@output_buffer')

# good
# Ruby standard library style
# https://github.com/ruby/ruby/commit/3406c5d
if ERB.instance_method(:initialize).parameters.assoc(:key) # Ruby 2.6+
  ERB.new(str, trim_mode: '-', eoutvar: '@output_buffer')
else
  ERB.new(str, nil, '-', '@output_buffer')
end

# good
# Use `RUBY_VERSION` style
if RUBY_VERSION >= '2.6'
  ERB.new(str, trim_mode: '-', eoutvar: '@output_buffer')
else
  ERB.new(str, nil, '-', '@output_buffer')
end

Remove unnecessary existence check Dir.exist?.
Open

      FileUtils.rm_rf(SAML2_CONFIG_DIRECTORY) if Dir.exist?(SAML2_CONFIG_DIRECTORY)

Checks for non-atomic file operation. And then replace it with a nearly equivalent and atomic method.

These can cause problems that are difficult to reproduce, especially in cases of frequent file operations in parallel, such as test runs with parallel_rspec.

For examples: creating a directory if there is none, has the following problems

An exception occurs when the directory didn't exist at the time of exist?, but someone else created it before mkdir was executed.

Subsequent processes are executed without the directory that should be there when the directory existed at the time of exist?, but someone else deleted it shortly afterwards.

Safety:

This cop is unsafe, because autocorrection change to atomic processing. The atomic processing of the replacement destination is not guaranteed to be strictly equivalent to that before the replacement.

Example:

# bad - race condition with another process may result in an error in `mkdir`
unless Dir.exist?(path)
  FileUtils.mkdir(path)
end

# good - atomic and idempotent creation
FileUtils.mkdir_p(path)

# bad - race condition with another process may result in an error in `remove`
if File.exist?(path)
  FileUtils.remove(path)
end

# good - atomic and idempotent removal
FileUtils.rm_f(path)

Use match? instead of =~ when MatchData is not used.
Open

      if source_file =~ URI.regexp(%w(http https))

In Ruby 2.4, String#match?, Regexp#match? and Symbol#match? have been added. The methods are faster than match. Because the methods avoid creating a MatchData object or saving backref. So, when MatchData is not used, use match? instead of match.

Example:

# bad
def foo
  if x =~ /re/
    do_something
  end
end

# bad
def foo
  if x.match(/re/)
    do_something
  end
end

# bad
def foo
  if /re/ === x
    do_something
  end
end

# good
def foo
  if x.match?(/re/)
    do_something
  end
end

# good
def foo
  if x =~ /re/
    do_something(Regexp.last_match)
  end
end

# good
def foo
  if x.match(/re/)
    do_something($~)
  end
end

# good
def foo
  if /re/ === x
    do_something($~)
  end
end

Remove unnecessary existence check File.exist?.
Open

        File.delete(path) if File.exist?(path)

Checks for non-atomic file operation. And then replace it with a nearly equivalent and atomic method.

These can cause problems that are difficult to reproduce, especially in cases of frequent file operations in parallel, such as test runs with parallel_rspec.

For examples: creating a directory if there is none, has the following problems

An exception occurs when the directory didn't exist at the time of exist?, but someone else created it before mkdir was executed.

Subsequent processes are executed without the directory that should be there when the directory existed at the time of exist?, but someone else deleted it shortly afterwards.

Safety:

This cop is unsafe, because autocorrection change to atomic processing. The atomic processing of the replacement destination is not guaranteed to be strictly equivalent to that before the replacement.

Example:

# bad - race condition with another process may result in an error in `mkdir`
unless Dir.exist?(path)
  FileUtils.mkdir(path)
end

# good - atomic and idempotent creation
FileUtils.mkdir_p(path)

# bad - race condition with another process may result in an error in `remove`
if File.exist?(path)
  FileUtils.remove(path)
end

# good - atomic and idempotent removal
FileUtils.rm_f(path)

Call super to initialize state of the parent class.
Open

    def initialize(options = {})
      options.each { |n, v| public_send("#{n}=", v) }
      @realm = @realm.upcase if @realm
      @name ||= "#{service}/#{hostname}@#{realm}"
      @name

Checks for the presence of constructors and lifecycle callbacks without calls to super.

This cop does not consider method_missing (and respond_to_missing?) because in some cases it makes sense to overtake what is considered a missing method. In other cases, the theoretical ideal handling could be challenging or verbose for no actual gain.

Autocorrection is not supported because the position of super cannot be determined automatically.

Object and BasicObject are allowed by this cop because of their stateless nature. However, sometimes you might want to allow other parent classes from this cop, for example in the case of an abstract class that is not meant to be called with super. In those cases, you can use the AllowedParentClasses option to specify which classes should be allowed in addition to Object and BasicObject.

Example:

# bad
class Employee < Person
  def initialize(name, salary)
    @salary = salary
  end
end

# good
class Employee < Person
  def initialize(name, salary)
    super(name)
    @salary = salary
  end
end

# bad
Employee = Class.new(Person) do
  def initialize(name, salary)
    @salary = salary
  end
end

# good
Employee = Class.new(Person) do
  def initialize(name, salary)
    super(name)
    @salary = salary
  end
end

# bad
class Parent
  def self.inherited(base)
    do_something
  end
end

# good
class Parent
  def self.inherited(base)
    super
    do_something
  end
end

# good
class ClassWithNoParent
  def initialize
    do_something
  end
end

Example: AllowedParentClasses: [MyAbstractClass]

# good
class MyConcreteClass < MyAbstractClass
  def initialize
    do_something
  end
end

Use filter_map instead.
Open

      opts[:ldap_basedn].split(",").collect do |p|
        p.split('dc=')[1]
      end.compact.join('.')
Severity
Category
Status
Source
Language