From: David Schmitt Date: Wed, 26 Jul 2017 15:36:56 +0000 (+0100) Subject: Run default pdk rubocop rules, update :kind to :behaviour X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=087de2d7ca559ea5e4d24ed709efc79c4b12e578;p=puppet-modules%2Fpuppetlabs-apt.git Run default pdk rubocop rules, update :kind to :behaviour --- diff --git a/.rubocop.yml b/.rubocop.yml index 5aadd1b..d66cbd6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,508 +1,96 @@ +--- require: rubocop-rspec AllCops: - TargetRubyVersion: 1.9 + TargetRubyVersion: '2.1' Include: - - ./**/*.rb + - "./**/*.rb" Exclude: - - vendor/**/* - - .vendor/**/* - - pkg/**/* - - spec/fixtures/**/* -Lint/ConditionPosition: - Enabled: True - -Lint/ElseLayout: - Enabled: True - -Lint/UnreachableCode: - Enabled: True - -Lint/UselessComparison: - Enabled: True - -Lint/EnsureReturn: - Enabled: True - -Lint/HandleExceptions: - Enabled: True - -Lint/LiteralInCondition: - Enabled: True - -Lint/ShadowingOuterLocalVariable: - Enabled: True - -Lint/LiteralInInterpolation: - Enabled: True - -Style/HashSyntax: - Enabled: True - -Style/RedundantReturn: - Enabled: True - -Lint/AmbiguousOperator: - Enabled: True - -Lint/AssignmentInCondition: - Enabled: True - -Style/SpaceBeforeComment: - Enabled: True - -Style/AndOr: - Enabled: True - -Style/RedundantSelf: - Enabled: True - -# Method length is not necessarily an indicator of code quality -Metrics/MethodLength: - Enabled: False - -# Module length is not necessarily an indicator of code quality -Metrics/ModuleLength: - Enabled: False - -Style/WhileUntilModifier: - Enabled: True - -Lint/AmbiguousRegexpLiteral: - Enabled: True - -Lint/Eval: - Enabled: True - -Lint/BlockAlignment: - Enabled: True - -Lint/DefEndAlignment: - Enabled: True - -Lint/EndAlignment: - Enabled: True - -Lint/DeprecatedClassMethods: - Enabled: True - -Lint/Loop: - Enabled: True - -Lint/ParenthesesAsGroupedExpression: - Enabled: True - -Lint/RescueException: - Enabled: True - -Lint/StringConversionInInterpolation: - Enabled: True - -Lint/UnusedBlockArgument: - Enabled: True - -Lint/UnusedMethodArgument: - Enabled: True - -Lint/UselessAccessModifier: - Enabled: True - -Lint/UselessAssignment: - Enabled: True - -Lint/Void: - Enabled: True - -Style/AccessModifierIndentation: - Enabled: True - -Style/AccessorMethodName: - Enabled: True - -Style/Alias: - Enabled: True - -Style/AlignArray: - Enabled: True - -Style/AlignHash: - Enabled: True - -Style/AlignParameters: - Enabled: True - -Metrics/BlockNesting: - Enabled: True - -Style/AsciiComments: - Enabled: True - -Style/Attr: - Enabled: True - -Style/BracesAroundHashParameters: - Enabled: True - -Style/CaseEquality: - Enabled: True - -Style/CaseIndentation: - Enabled: True - -Style/CharacterLiteral: - Enabled: True - -Style/ClassAndModuleCamelCase: - Enabled: True - -Style/ClassAndModuleChildren: - Enabled: False - -Style/ClassCheck: - Enabled: True - -# Class length is not necessarily an indicator of code quality -Metrics/ClassLength: - Enabled: False - -Style/ClassMethods: - Enabled: True - -Style/ClassVars: - Enabled: True - -Style/WhenThen: - Enabled: True - -Style/WordArray: - Enabled: True - -Style/UnneededPercentQ: - Enabled: True - -Style/Tab: - Enabled: True - -Style/SpaceBeforeSemicolon: - Enabled: True - -Style/TrailingBlankLines: - Enabled: True - -Style/SpaceInsideBlockBraces: - Enabled: True - -Style/SpaceInsideBrackets: - Enabled: True - -Style/SpaceInsideHashLiteralBraces: - Enabled: True - -Style/SpaceInsideParens: - Enabled: True - -Style/LeadingCommentSpace: - Enabled: True - -Style/SpaceBeforeFirstArg: - Enabled: True - -Style/SpaceAfterColon: - Enabled: True - -Style/SpaceAfterComma: - Enabled: True - -Style/SpaceAfterMethodName: - Enabled: True - -Style/SpaceAfterNot: - Enabled: True - -Style/SpaceAfterSemicolon: - Enabled: True - -Style/SpaceAroundEqualsInParameterDefault: - Enabled: True - -Style/SpaceAroundOperators: - Enabled: True - -Style/SpaceBeforeBlockBraces: - Enabled: True - -Style/SpaceBeforeComma: - Enabled: True - -Style/CollectionMethods: - Enabled: True - -Style/CommentIndentation: - Enabled: True - -Style/ColonMethodCall: - Enabled: True - -Style/CommentAnnotation: - Enabled: True - -# 'Complexity' is very relative -Metrics/CyclomaticComplexity: - Enabled: False - -Style/ConstantName: - Enabled: True - -Style/Documentation: - Enabled: False - -Style/DefWithParentheses: - Enabled: True - -Style/PreferredHashMethods: - Enabled: True - -Style/DotPosition: - EnforcedStyle: trailing - -Style/DoubleNegation: - Enabled: True - -Style/EachWithObject: - Enabled: True - -Style/EmptyLineBetweenDefs: - Enabled: True - -Style/IndentArray: - Enabled: True - -Style/IndentHash: - Enabled: True - -Style/IndentationConsistency: - Enabled: True - -Style/IndentationWidth: - Enabled: True - -Style/EmptyLines: - Enabled: True - -Style/EmptyLinesAroundAccessModifier: - Enabled: True - -Style/EmptyLiteral: - Enabled: True - -# Configuration parameters: AllowURI, URISchemes. + - bin/* + - ".vendor/**/*" + - Gemfile + - Rakefile + - pkg/**/* + - spec/fixtures/**/* + - vendor/**/* Metrics/LineLength: - Enabled: False - -Style/MethodCallParentheses: - Enabled: True - -Style/MethodDefParentheses: - Enabled: True - -Style/LineEndConcatenation: - Enabled: True - -Style/TrailingWhitespace: - Enabled: True - -Style/StringLiterals: - Enabled: True - -Style/TrailingCommaInArguments: - Enabled: True - -Style/TrailingCommaInLiteral: - Enabled: True - -Style/GlobalVars: - Enabled: True - -Style/GuardClause: - Enabled: True - -Style/IfUnlessModifier: - Enabled: True - -Style/MultilineIfThen: - Enabled: True - -Style/NegatedIf: - Enabled: True - -Style/NegatedWhile: - Enabled: True - -Style/Next: - Enabled: True - -Style/SingleLineBlockParams: - Enabled: True - -Style/SingleLineMethods: - Enabled: True - -Style/SpecialGlobalVars: - Enabled: True - -Style/TrivialAccessors: - Enabled: True - -Style/UnlessElse: - Enabled: True - -Style/VariableInterpolation: - Enabled: True - -Style/VariableName: - Enabled: True - -Style/WhileUntilDo: - Enabled: True - -Style/EvenOdd: - Enabled: True - -Style/FileName: - Enabled: True - -Style/For: - Enabled: True - -Style/Lambda: - Enabled: True - -Style/MethodName: - Enabled: True - -Style/MultilineTernaryOperator: - Enabled: True - -Style/NestedTernaryOperator: - Enabled: True - -Style/NilComparison: - Enabled: True - + Description: People have wide screens, use them. + Max: 200 +RSpec/BeforeAfterAll: + Description: Beware of using after(:all) as it may cause state to leak between tests. + A necessary evil in acceptance testing. + Exclude: + - spec/acceptance/**/*.rb +RSpec/HookArgument: + Description: Prefer explicit :each argument, matching existing module's style + EnforcedStyle: each +Style/BlockDelimiters: + Description: Prefer braces for chaining. Mostly an aesthetical choice. Better to + be consistent then. + EnforcedStyle: braces_for_chaining +Style/ClassAndModuleChildren: + Description: Compact style reduces the required amount of indentation. + EnforcedStyle: compact +Style/EmptyElse: + Description: Enforce against empty else clauses, but allow `nil` for clarity. + EnforcedStyle: empty Style/FormatString: - Enabled: True - -Style/MultilineBlockChain: - Enabled: True - -Style/Semicolon: - Enabled: True - -Style/SignalException: - Enabled: True - -Style/NonNilCheck: - Enabled: True - -Style/Not: - Enabled: True - -Style/NumericLiterals: - Enabled: True - -Style/OneLineConditional: - Enabled: True - -Style/OpMethod: - Enabled: True - -Style/ParenthesesAroundCondition: - Enabled: True - -Style/PercentLiteralDelimiters: - Enabled: True - -Style/PerlBackrefs: - Enabled: True - -Style/PredicateName: - Enabled: True - -Style/RedundantException: - Enabled: True - -Style/SelfAssignment: - Enabled: True - -Style/Proc: - Enabled: True - -Style/RaiseArgs: - Enabled: True - -Style/RedundantBegin: - Enabled: True - -Style/RescueModifier: - Enabled: True - -# based on https://github.com/voxpupuli/modulesync_config/issues/168 + Description: Following the main puppet project's style, prefer the % format format. + EnforcedStyle: percent +Style/FormatStringToken: + Description: Following the main puppet project's style, prefer the simpler template + tokens over annotated ones. + EnforcedStyle: template +Style/Lambda: + Description: Prefer the keyword for easier discoverability. + EnforcedStyle: literal Style/RegexpLiteral: + Description: Community preference. See https://github.com/voxpupuli/modulesync_config/issues/168 EnforcedStyle: percent_r - Enabled: True - -Lint/UnderscorePrefixedVariableName: - Enabled: True - -Metrics/ParameterLists: - Enabled: False - -Lint/RequireParentheses: - Enabled: True - -Style/SpaceBeforeFirstArg: - Enabled: True - -Style/ModuleFunction: - Enabled: True - -Lint/Debugger: - Enabled: True - -Style/IfWithSemicolon: - Enabled: True - -Style/Encoding: - Enabled: True - -Style/BlockDelimiters: - Enabled: True - -Style/MultilineBlockLayout: - Enabled: True - -# 'Complexity' is very relative +Style/TernaryParentheses: + Description: Checks for use of parentheses around ternary conditions. Enforce parentheses + on complex expressions for better readability, but seriously consider breaking + it up. + EnforcedStyle: require_parentheses_when_complex +Style/TrailingCommaInArguments: + Description: Prefer always trailing comma on multiline argument lists. This makes + diffs, and re-ordering nicer. + EnforcedStyleForMultiline: comma +Style/TrailingCommaInLiteral: + Description: Prefer always trailing comma on multiline literals. This makes diffs, + and re-ordering nicer. + EnforcedStyleForMultiline: comma +Style/SymbolArray: + Description: Using percent style obscures symbolic intent of array's contents. + EnforcedStyle: brackets +Style/CollectionMethods: + Enabled: true +Style/MethodCalledOnDoEndBlock: + Enabled: true +Style/StringMethods: + Enabled: true Metrics/AbcSize: - Enabled: False - -# 'Complexity' is very relative + Enabled: false +Metrics/BlockLength: + Enabled: false +Metrics/ClassLength: + Enabled: false +Metrics/CyclomaticComplexity: + Enabled: false +Metrics/MethodLength: + Enabled: false +Metrics/ModuleLength: + Enabled: false +Metrics/ParameterLists: + Enabled: false Metrics/PerceivedComplexity: - Enabled: False - -Lint/UselessAssignment: - Enabled: True - -Style/ClosingParenthesisIndentation: - Enabled: False - -# RSpec - -# We don't use rspec in this way + Enabled: false RSpec/DescribeClass: - Enabled: False - -# Example length is not necessarily an indicator of code quality -RSpec/ExampleLength: - Enabled: False - -RSpec/NamedSubject: - Enabled: False + Enabled: false +RSpec/MessageExpectation: + Enabled: false +Style/AsciiComments: + Enabled: false +Style/IfUnlessModifier: + Enabled: false +Style/SymbolProc: + Enabled: false diff --git a/lib/puppet/provider/apt_key2/apt_key2.rb b/lib/puppet/provider/apt_key2/apt_key2.rb index e08dfe5..aa21c6b 100644 --- a/lib/puppet/provider/apt_key2/apt_key2.rb +++ b/lib/puppet/provider/apt_key2/apt_key2.rb @@ -4,14 +4,6 @@ require 'open-uri' require 'net/ftp' require 'tempfile' -if RUBY_VERSION == '1.8.7' - # Mothers cry, puppies die and Ruby 1.8.7's open-uri needs to be - # monkeypatched to support passing in :ftp_passive_mode. - require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', '..', - 'puppet_x', 'apt_key', 'patch_openuri.rb')) - OpenURI::Options.merge!({:ftp_active_mode => false,}) -end - register_provider('apt_key2') do commands apt_key: 'apt-key' commands gpg: '/usr/bin/gpg' @@ -26,14 +18,14 @@ register_provider('apt_key2') do end end - def get(names = []) - cli_args = %w(adv --list-keys --with-colons --fingerprint --fixed-list-mode) + def get(_names = []) + cli_args = %w[adv --list-keys --with-colons --fingerprint --fixed-list-mode] key_output = apt_key_lines(*cli_args) pub_line = nil fpr_line = nil - result = key_output.collect do |line| - line = line.encode('UTF-8', 'binary', :invalid => :replace, :undef => :replace, :replace => '') + result = key_output.map { |line| + line = line.encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '') if line.start_with?('pub') pub_line = line elsif line.start_with?('fpr') @@ -41,7 +33,7 @@ register_provider('apt_key2') do end # puts "debug: parsing #{line}; fpr: #{fpr_line.inspect}; pub: #{pub_line.inspect}" - next unless (pub_line and fpr_line) + next unless pub_line && fpr_line # puts "debug: key_line_to_hash" @@ -52,7 +44,7 @@ register_provider('apt_key2') do fpr_line = nil hash - end.compact! + }.compact! result end @@ -63,16 +55,16 @@ register_provider('apt_key2') do # set key type based on types defined in /usr/share/doc/gnupg/DETAILS.gz key_type = case pub_split[3] - when '1' - :rsa - when '17' - :dsa - when '18' - :ecc - when '19' - :ecdsa - else - :unrecognized + when '1' + :rsa + when '17' + :dsa + when '18' + :ecc + when '19' + :ecdsa + else + :unrecognized end fingerprint = fpr_split.last @@ -95,7 +87,7 @@ register_provider('apt_key2') do def set(current_state, target_state, noop = false) target_state.each do |title, resource| logger.warning(title, 'The id should be a full fingerprint (40 characters) to avoid collision attacks, see the README for details.') if title.length < 40 - if resource[:source] and resource[:content] + if resource[:source] && resource[:content] logger.fail(title, 'The properties content and source are mutually exclusive') next end @@ -105,8 +97,8 @@ register_provider('apt_key2') do logger.deleting(title) do begin apt_key('del', resource[:short], noop: noop) - r = execute(["#{command(:apt_key)} list | grep '/#{resource[:short]}\s'"], :failonfail => false) - end while r.exitstatus == 0 + r = execute(["#{command(:apt_key)} list | grep '/#{resource[:short]}\s'"], failonfail: false) + end while r.exitstatus.zero? end elsif current && resource[:ensure].to_s == 'present' logger.warning(title, 'No updating implemented') @@ -119,7 +111,7 @@ register_provider('apt_key2') do def create(title, resource, noop = false) logger.creating(title) do |logger| - if resource[:source].nil? and resource[:content].nil? + if resource[:source].nil? && resource[:content].nil? # Breaking up the command like this is needed because it blows up # if --recv-keys isn't the last argument. args = ['adv', '--keyserver', resource[:server]] @@ -151,11 +143,12 @@ register_provider('apt_key2') do file.close if name.size == 40 if File.executable? command(:gpg) - extracted_key = execute(["#{command(:gpg)} --with-fingerprint --with-colons #{file.path} | awk -F: '/^fpr:/ { print $10 }'"], :failonfail => false) + extracted_key = execute(["#{command(:gpg)} --with-fingerprint --with-colons #{file.path} | awk -F: '/^fpr:/ { print $10 }'"], failonfail: false) extracted_key = extracted_key.chomp - unless extracted_key.match(/^#{name}$/) - logger.fail("The id in your manifest #{resource[:id]} and the fingerprint from content/source do not match. Please check there is not an error in the id or check the content/source is legitimate.") + unless extracted_key =~ %r{^#{name}$} + logger.fail("The id in your manifest #{resource[:id]} and the fingerprint from content/source do not match. "\ + ' Please check there is not an error in the id or check the content/source is legitimate.') end else logger.warning('/usr/bin/gpg cannot be found for verification of the id.') diff --git a/lib/puppet/type/apt_key2.rb b/lib/puppet/type/apt_key2.rb index 15a90bd..eb67117 100644 --- a/lib/puppet/type/apt_key2.rb +++ b/lib/puppet/type/apt_key2.rb @@ -1,8 +1,7 @@ require File.expand_path(File.join(File.dirname(__FILE__), '..', '..', 'puppet_x', 'apt_key', 'resource_api.rb')) -register_type({ - name: 'apt_key2', - docs: <<-EOS, +register_type(name: 'apt_key2', + docs: <<-EOS, This type provides Puppet with the capabilities to manage GPG keys needed by apt to perform package validation. Apt has it's own GPG keyring that can be manipulated through the `apt-key` command. @@ -15,79 +14,78 @@ register_type({ If Puppet is given the location of a key file which looks like an absolute path this type will autorequire that file. EOS - attributes: { - ensure: { - type: 'Enum[present, absent]', - docs: 'Whether this apt key should be present or absent on the target system.', - default: 'present', - }, - id: { - type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', - docs: 'The ID of the key you want to manage.', - namevar: true, - }, - content: { - type: 'Optional[String]', - docs: 'The content of, or string representing, a GPG key.', - }, - source: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - docs: 'Location of a GPG key file, /path/to/file, ftp://, http:// or https://', - }, - server: { - type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', - docs: 'The key server to fetch the key from based on the ID. It can either be a domain name or url.', - kind: :read_only, - default: :'keyserver.ubuntu.com', - }, - options: { - type: 'Optional[String]', - docs: 'Additional options to pass to apt-key\'s --keyserver-options.', - }, - fingerprint: { - type: 'Pattern[/[a-f]{40}/]', - docs: 'The 40-digit hexadecimal fingerprint of the specified GPG key.', - read_only: true, - }, - long: { - type: 'Pattern[/[a-f]{16}/]', - docs: 'The 16-digit hexadecimal id of the specified GPG key.', - read_only: true, - }, - short: { - type: 'Pattern[/[a-f]{8}/]', - docs: 'The 8-digit hexadecimal id of the specified GPG key.', - read_only: true, - }, - expired: { - type: 'Boolean', - docs: 'Indicates if the key has expired.', - read_only: true, - }, - expiry: { - # TODO: should be DateTime - type: 'String', - docs: 'The date the key will expire, or nil if it has no expiry date, in ISO format.', - read_only: true, - }, - size: { - type: 'Integer', - docs: 'The key size, usually a multiple of 1024.', - read_only: true, - }, - type: { - type: 'String', - docs: 'The key type, one of: rsa, dsa, ecc, ecdsa.', - read_only: true, - }, - created: { - type: 'String', - docs: 'Date the key was created, in ISO format.', - read_only: true, - }, + attributes: { + ensure: { + type: 'Enum[present, absent]', + docs: 'Whether this apt key should be present or absent on the target system.', + default: 'present', }, - autorequires: { - file: '$source', # will evaluate to the value of the `source` attribute - package: 'apt', + id: { + type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', + docs: 'The ID of the key you want to manage.', + behaviour: :namevar, }, -}) + content: { + type: 'Optional[String]', + docs: 'The content of, or string representing, a GPG key.', + }, + source: { + type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', + docs: 'Location of a GPG key file, /path/to/file, ftp://, http:// or https://', + }, + server: { + type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', + docs: 'The key server to fetch the key from based on the ID. It can either be a domain name or url.', + behaviour: :read_only, + default: :'keyserver.ubuntu.com', + }, + options: { + type: 'Optional[String]', + docs: 'Additional options to pass to apt-key\'s --keyserver-options.', + }, + fingerprint: { + type: 'Pattern[/[a-f]{40}/]', + docs: 'The 40-digit hexadecimal fingerprint of the specified GPG key.', + behaviour: :read_only, + }, + long: { + type: 'Pattern[/[a-f]{16}/]', + docs: 'The 16-digit hexadecimal id of the specified GPG key.', + behaviour: :read_only, + }, + short: { + type: 'Pattern[/[a-f]{8}/]', + docs: 'The 8-digit hexadecimal id of the specified GPG key.', + behaviour: :read_only, + }, + expired: { + type: 'Boolean', + docs: 'Indicates if the key has expired.', + behaviour: :read_only, + }, + expiry: { + # TODO: should be DateTime + type: 'String', + docs: 'The date the key will expire, or nil if it has no expiry date, in ISO format.', + behaviour: :read_only, + }, + size: { + type: 'Integer', + docs: 'The key size, usually a multiple of 1024.', + behaviour: :read_only, + }, + type: { + type: 'String', + docs: 'The key type, one of: rsa, dsa, ecc, ecdsa.', + behaviour: :read_only, + }, + created: { + type: 'String', + docs: 'Date the key was created, in ISO format.', + behaviour: :read_only, + }, + }, + autorequires: { + file: '$source', # will evaluate to the value of the `source` attribute + package: 'apt', + }) diff --git a/lib/puppet_x/apt_key/resource_api.rb b/lib/puppet_x/apt_key/resource_api.rb index 3b1cc07..d3aad0e 100644 --- a/lib/puppet_x/apt_key/resource_api.rb +++ b/lib/puppet_x/apt_key/resource_api.rb @@ -84,7 +84,7 @@ def register_type(definition) # TODO: using newparam everywhere would suppress change reporting # that would allow more fine-grained reporting through logger, # but require more invest in hooking up the infrastructure to emulate existing data - param_or_property = if options[:kind] == :read_only || options[:namevar] + param_or_property = if options[:behaviour] == :read_only || options[:behaviour] == :namevar :newparam else :newproperty @@ -100,7 +100,7 @@ def register_type(definition) warn("#{definition[:name]}.#{name} has no docs") end - if options[:namevar] + if options[:behaviour] == :namevar puts 'setting namevar' isnamevar has_namevar = true @@ -108,7 +108,7 @@ def register_type(definition) end # read-only values do not need type checking, but can have default values - if not options[:read_only] + if options[:behaviour] != :read_only # TODO: this should use Pops infrastructure to avoid hardcoding stuff, and enhance type fidelity # validate do |v| # type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type]).normalize