Linting and code cleanup

.gitignore

@@ -44,7 +44,6 @@ pkg/
 .dev_builds
 .final_builds/jobs/**/*.tgz
 .final_builds/packages/**/*.tgz
-.idea
 .vscode
 blobs
 config/dev.yml

ci/dockerfiles/warden-cpi/warden-cloud-config.yml

@@ -17,7 +17,6 @@ networks:
   - azs: [z1, z2, z3]
     dns: [8.8.8.8]
     range: 10.245.0.0/24
-    dns: [8.8.8.8]
     # IPs that will not be used for anything
     reserved: [10.245.0.2-10.245.0.10]
     gateway: 10.245.0.1

spec/director.yml.erb_spec.rb

@@ -293,7 +293,7 @@
                 'enabled' => true,
                 'url' => 'https://config-server-host',
                 'uaa' => {
-                  'url' => 'http://something.com',
+                  'url' => 'https://something.com',
                   'client_secret' => 'secret',
                   'ca_cert_path' => '/var/vcap/blah/to/go'
                 }

spec/director_templates_spec.rb

@@ -329,7 +329,7 @@
           )
         end
 
-        it 'converts the adapter to `mysqql`' do
+        it 'converts the adapter to `mysql`' do
           bbr_config = JSON.parse(template.render(properties))
           expect(bbr_config['adapter']).to eq('mysql')
         end
@@ -375,16 +375,13 @@
       end
     end
 
-    # rubocop:disable Metrics/MethodLength
     describe 'certificate expiry template' do
       before do
         @key, @cert, @expiry = create_key_and_csr_cert
       end
 
-      # rubocop:disable Metrics/BlockLength
       it_should_behave_like 'a rendered file' do
         let(:file_name) { '../jobs/director/templates/certificate_expiry.json.erb' }
-        # rubocop:disable Metrics/BlockLength
         let(:properties) do
           {
             'properties' => {
@@ -445,8 +442,6 @@
           JSON
         end
       end
-      # rubocop:enable Metrics/BlockLength
-      # rubocop:enable Metrics/MethodLength
     end
   end
 end
@@ -465,7 +460,7 @@ def new_csr(key, subject)
   csr.version = 0
   csr.subject = subject
   csr.public_key = key.public_key
-  csr.sign key, OpenSSL::Digest::SHA1.new
+  csr.sign key, OpenSSL::Digest.new('SHA1')
 
   csr
 end
@@ -475,13 +470,13 @@ def new_csr_certificate(key, csr)
   csr_cert.serial = 0
   csr_cert.version = 2
   csr_cert.not_before = Time.now - 60 * 60 * 24
-  csr_cert.not_after = Time.now + 94608000
+  csr_cert.not_after = Time.now + 94_608_000
 
   csr_cert.subject = csr.subject
   csr_cert.public_key = csr.public_key
   csr_cert.issuer = csr.subject
 
-  csr_cert.sign key, OpenSSL::Digest::SHA1.new
+  csr_cert.sign key, OpenSSL::Digest.new('SHA1')
 
   csr_cert
 end

src/bosh-dev/lib/bosh/dev/sandbox/mysql.rb

@@ -58,9 +58,7 @@ def current_tasks
 
     def current_locked_jobs
       jobs_cmd = %Q{mysql -h #{@host} -P #{@port} --user=#{@username} --password=#{@password} -e "select * from delayed_jobs where locked_by is not null;" #{db_name} 2> /dev/null}
-      job_lines = `#{jobs_cmd}`.lines.to_a[1..-1] || []
-
-      job_lines
+      `#{jobs_cmd}`.lines.to_a[1..-1] || []
     end
 
     def truncate_db

src/bosh-dev/lib/bosh/dev/sandbox/services/director_service.rb

@@ -141,8 +141,7 @@ def db_config
       end
 
       connection_config.delete_if { |_, v| v.to_s.empty? }
-      connection_config = connection_config.merge(custom_connection_options)
-      connection_config
+      connection_config.merge(custom_connection_options)
     end
 
     def read_log

src/bosh-dev/lib/bosh/dev/tasks/migrations.rake

@@ -56,7 +56,7 @@ EOF
     migration_digest = Digest::SHA1.hexdigest(File.read(new_migration_path))
 
     digest_migration_json = JSON.parse(File.read(migration_digests))
-    if digest_migration_json[name] != nil then
+    if digest_migration_json[name] != nil
       puts '
         YOU ARE MODIFIFYING A DB MIGRATION DIGEST.
         IF THIS MIGRATION HAS ALREADY BEEN RELEASED, IT MIGHT RESULT IN UNDESIRABLE BEHAVIOR.

src/bosh-dev/lib/bosh/dev/test_runner.rb

@@ -41,15 +41,17 @@ def unit_exec(build, log_file: nil, parallel: false)
         lines.append "            #{command}"
         lines.append(File.read(log_file)) if log_file && File.exist?(log_file)
         lines.append "----- END   #{build}\n\n"
-        return {:lines => lines, :error => false}
+
+        {:lines => lines, :error => false}
       else
         lines.append "----- BEGIN #{build}"
         lines.append "            #{command}"
         error_message = "#{build} failed to build or run unit tests"
         error_message += ": #{File.read(log_file)}" if log_file && File.exist?(log_file)
         lines.append "            #{error_message}\n"
         lines.append "----- END   #{build}\n\n"
-        return {:lines => lines, :error => true}
+
+        {:lines => lines, :error => true}
       end
     end
 

src/bosh-director-core/lib/bosh/director/core/templates/template_blob_cache.rb

@@ -1,7 +1,7 @@
 module Bosh::Director::Core::Templates
   class TemplateBlobCache
     def self.with_fresh_cache
-      cache = new()
+      cache = new
       yield cache
     ensure
       cache.clean_cache!

src/bosh-director/lib/bosh/director.rb

@@ -13,7 +13,6 @@ module Director
 require 'optparse'
 require 'ostruct'
 require 'pathname'
-require 'pp'
 require 'tmpdir'
 require 'yaml'
 require 'time'
@@ -27,14 +26,11 @@ module Director
 require 'bcrypt'
 require 'netaddr'
 require 'delayed_job'
-#TODO: remove when https://github.com/collectiveidea/delayed_job/pull/1185 is merged
-require "active_support/core_ext/array/extract_options"
 require 'sequel'
 require 'sinatra/base'
 require 'securerandom'
 require 'nats/io/client'
 require 'openssl'
-require 'securerandom'
 require 'delayed_job_sequel'
 
 require 'common/thread_formatter'

src/bosh-director/lib/bosh/director/agent_client.rb

@@ -205,7 +205,7 @@ def wait_for_task(agent_task_id, timeout = nil, &blk)
       task = get_task_status(agent_task_id)
       timed_out = false
 
-      until task['state'] != 'running' || (timeout && timed_out = timeout.timed_out?)
+      until task['state'] != 'running' || (timeout && (timed_out = timeout.timed_out?))
         blk.call if block_given?
         sleep(DEFAULT_POLL_INTERVAL)
         task = get_task_status(agent_task_id)
@@ -324,7 +324,7 @@ def format_exception(exception)
     def inject_compile_log(response)
       if response['value'] && response['value'].is_a?(Hash) &&
         response['value']['result'].is_a?(Hash) &&
-        blob_id = response['value']['result']['compile_log_id']
+        (blob_id = response['value']['result']['compile_log_id'])
         compile_log = download_and_delete_blob(blob_id)
         response['value']['result']['compile_log'] = compile_log
       end

src/bosh-director/lib/bosh/director/api/config_manager.rb

@@ -16,7 +16,7 @@ def create(type, name, config_yaml, team_id = nil)
           def deploy_config_enabled?
             deploy_config = find(type: 'deploy')
 
-            return !deploy_config.empty?
+            !deploy_config.empty?
           end
 
           def find(type: nil, name: nil, limit: 1)

src/bosh-director/lib/bosh/director/api/controllers/tasks_controller.rb

@@ -52,11 +52,11 @@ def self.authorization(perm)
           ])
         end
 
-        if context_id = params['context_id']
+        if (context_id = params['context_id'])
           dataset = dataset.filter(:context_id => context_id)
         end
 
-        if limit = params['limit']
+        if (limit = params['limit'])
           limit = limit.to_i
           limit = 1 if limit < 1
         end

src/bosh-director/lib/bosh/director/api/release_manager.rb

@@ -4,15 +4,13 @@ class ReleaseManager
       include ApiHelper
 
       def get_all_releases
-        releases = Models::Release.order_by(Sequel.asc(:name)).map do |release|
+        Models::Release.order_by(Sequel.asc(:name)).map do |release|
           release_versions = sorted_release_versions(release)
           {
             'name' => release.name,
             'release_versions' => release_versions
           }
         end
-
-        releases
       end
 
       def sorted_release_versions(release, prefix = nil)

src/bosh-director/lib/bosh/director/api/stemcell_manager.rb

@@ -19,7 +19,7 @@ def find_by_name_and_version_and_cpi(name, version, cpi)
         matched_cpis = found_cpis & cpi_aliases
         raise StemcellNotFound, "Stemcell '#{name}/#{version}' and cpi #{cpi} doesn't exist" if matched_cpis.empty?
 
-        return Models::Stemcell[:name => name, :version => version, :cpi => matched_cpis[0]]
+        Models::Stemcell[:name => name, :version => version, :cpi => matched_cpis[0]]
       end
 
       def find_all_stemcells
@@ -112,12 +112,10 @@ def find_latest(stemcells, prefix = nil)
 
         latest_version = Bosh::Common::Version::StemcellVersionList.parse(versions).latest.to_s
 
-        latest_stemcell = stemcells.find do |stemcell|
+        stemcells.find do |stemcell|
           parsed_version = Bosh::Common::Version::StemcellVersion.parse(stemcell.version).to_s
           parsed_version == latest_version
         end
-
-        latest_stemcell
       end
     end
   end

src/bosh-director/lib/bosh/director/api/user/config_user_manager.rb

@@ -21,7 +21,8 @@ def authenticate(username, password)
       def user_scopes(username)
         user = @users.find { |u| u['name'] == username }
         raise "User #{username} not found in ConfigUserManager" if user.nil?
-        return user.fetch('scopes', ['bosh.admin'])
+
+        user.fetch('scopes', ['bosh.admin'])
       end
 
       def delete_user(_)

src/bosh-director/lib/bosh/director/cidr_range_combiner.rb

@@ -40,11 +40,11 @@ def combine_adjacent_ranges(range_tuples)
               can_combine = false
               break
             end
-            if (range_tuple[1].succ == next_range_tuple[0])
+            if range_tuple[1].succ == next_range_tuple[0]
               range_tuple[1] = next_range_tuple[1]
               i += 1
             # does not cover all cases: 10/32, 10/8
-            elsif ((range_tuple[0] < next_range_tuple[0]) && (range_tuple[1] > next_range_tuple[1]))
+            elsif (range_tuple[0] < next_range_tuple[0]) && (range_tuple[1] > next_range_tuple[1])
               i += 1
             else
               can_combine = false

src/bosh-director/lib/bosh/director/config_server/client.rb

@@ -403,7 +403,7 @@ def get_variable_id_and_value_by_name(name)
         raise Bosh::Director::ConfigServerFetchError, "Failed to fetch variable '#{name_root}' from config server: Expected data[0] to have key 'id'" unless fetched_variable.key?('id')
         raise Bosh::Director::ConfigServerFetchError, "Failed to fetch variable '#{name_root}' from config server: Expected data[0] to have key 'value'" unless fetched_variable.key?('value')
 
-        return fetched_variable['id'], extract_variable_value(name, fetched_variable['value'])
+        [fetched_variable['id'], extract_variable_value(name, fetched_variable['value'])]
       elsif response.is_a? Net::HTTPNotFound
         raise Bosh::Director::ConfigServerMissingName, "Failed to find variable '#{name_root}' from config server: HTTP Code '404', Error: '#{response_body['error']}'"
       else

src/bosh-director/lib/bosh/director/config_server/deep_hash_replacement.rb

@@ -7,11 +7,9 @@ def variables_path(obj, subtrees_to_ignore = [])
       map = []
       construct_variables_paths(map, obj)
 
-      result = map.select do |elem|
+      map.select do |elem|
         !path_matches_subtrees_to_ignore?(subtrees_to_ignore, elem['path'])
       end
-
-      result
     end
 
     def replace_variables(obj_to_be_resolved, variables_paths, variable_values)

src/bosh-director/lib/bosh/director/deployment_plan/agent_state_migrator.rb

@@ -1,3 +1,5 @@
+require 'pp' # for #pretty_inspect
+
 module Bosh::Director
   module DeploymentPlan
     class AgentStateMigrator

src/bosh-director/lib/bosh/director/deployment_plan/compiled_package_finder.rb

@@ -12,8 +12,8 @@ def find_compiled_package(package:, stemcell:, exported_from: [], dependency_key
         return compiled_package if compiled_package
 
         compiled_package = find_newest_match(package, stemcell, dependency_key) unless package.source?
-        return compiled_package if compiled_package
 
+        compiled_package if compiled_package
       end
 
       private

src/bosh-director/lib/bosh/director/deployment_plan/ip_provider/ip_repo.rb

@@ -146,7 +146,7 @@ def validate_instance_and_update_reservation_type(instance_model, ip, ip_address
           ip_address.update(static: is_static, network_name: network_name)
         end
 
-        return ip_address
+        ip_address
       elsif reserved_instance.nil?
         raise Bosh::Director::NetworkReservationAlreadyInUse,
               "Failed to reserve IP '#{ip}' for instance '#{instance_model}': " \

src/bosh-director/lib/bosh/director/deployment_plan/job.rb

@@ -140,7 +140,8 @@ def links_of_kind_for_instance_group_name(instance_group_name, kind)
         if link_infos.has_key?(instance_group_name) && link_infos[instance_group_name].has_key?(kind)
           return link_infos[instance_group_name][kind]
         end
-        return []
+
+        []
       end
     end
   end

src/bosh-director/lib/bosh/director/deployment_plan/manual_network.rb

@@ -76,7 +76,7 @@ def ip_type(cidr_ip)
       def find_az_names_for_ip(ip)
         subnet = find_subnet_containing(ip)
         if subnet
-          return subnet.availability_zone_names
+          subnet.availability_zone_names
         end
       end
 

src/bosh-director/lib/bosh/director/deployment_plan/network_parser/name_servers_parser.rb

@@ -5,7 +5,7 @@ class NameServersParser
 
         include ValidationHelper
 
-        def initialize()
+        def initialize
           dns_config = Config.dns || {}
           @include_power_dns_server_addr = !!Config.dns_db
           @default_server = dns_config['server']

src/bosh-director/lib/bosh/director/deployment_plan/network_settings.rb

@@ -86,7 +86,7 @@ def link_network_addresses(link_def, prefer_dns_entry)
 
     def get_address(network_name, network, prefer_dns_entry = true)
       if should_use_dns?(network, prefer_dns_entry)
-        return @dns_encoder.encode_query({
+        @dns_encoder.encode_query({
           group_type: Models::LocalDnsEncodedGroup::Types::INSTANCE_GROUP,
           group_name: @instance_group_name,
           root_domain: @root_domain,
@@ -95,13 +95,13 @@ def get_address(network_name, network, prefer_dns_entry = true)
           uuid: @instance_id,
         })
       else
-        return network['ip']
+        network['ip']
       end
     end
 
     def get_link_address(link_def, network_name, network, prefer_dns_entry = true)
       if should_use_dns?(network, prefer_dns_entry)
-        return @dns_encoder.encode_link(
+        @dns_encoder.encode_link(
           link_def,
           {
             root_domain: @root_domain,
@@ -110,7 +110,7 @@ def get_link_address(link_def, network_name, network, prefer_dns_entry = true)
           }
         )
       else
-        return network['ip']
+        network['ip']
       end
     end
 

src/bosh-director/lib/bosh/director/deployment_plan/persistent_disk_collection.rb

@@ -123,7 +123,7 @@ def to_s
           { name: name, size: size, cloud_properties: cloud_properties }.inspect
         end
 
-        def to_json
+        def to_json(*_args)
           { name: name, size: size, cloud_properties: cloud_properties }.to_json
         end