Skip to content

Commit

Permalink
Resolve foodcritic and rubocop warnings
Browse files Browse the repository at this point in the history
I didn't want to have to do this, but I've added to the standard
.rubocop.yml file with some "todos" that we'll have to circle back on
later. There's just too much complexity in the provider ruby and a
complete lack of tests to refactor things with confidence.
  • Loading branch information
tas50 committed Sep 30, 2015
1 parent d0f8168 commit 4bb85b1
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 21 deletions.
1 change: 1 addition & 0 deletions .foodcritic
@@ -0,0 +1 @@
~FC048
12 changes: 12 additions & 0 deletions .rubocop.yml
Expand Up @@ -29,3 +29,15 @@ Style/ClassAndModuleChildren:
Enabled: false
Style/FileName:
Enabled: false

# Todo: These should are non-standard rubocop rules were should investigate and resolve
Style/ClassVars:
Enabled: false
Metrics/ParameterLists:
Enabled: false
Lint/Loop:
Enabled: false
Style/UnlessElse:
Enabled: false
Lint/UselessAssignment:
Enabled: false
2 changes: 1 addition & 1 deletion Rakefile
Expand Up @@ -14,7 +14,7 @@ namespace :style do
FoodCritic::Rake::LintTask.new(:chef) do |t|
t.options = {
fail_tags: ['any'],
tags: ['~FC005']
tags: ['~FC005', '~FC048']
}
end
end
Expand Down
4 changes: 2 additions & 2 deletions libraries/ec2.rb
Expand Up @@ -28,7 +28,7 @@ def find_snapshot_id(volume_id = '', find_most_recent = false)
ec2.describe_snapshots.sort { |a, b| a[:start_time] <=> b[:start_time] }
else
ec2.describe_snapshots.sort { |a, b| b[:start_time] <=> a[:start_time] }
end
end
response.each do |page|
page.snapshots.each do |snapshot|
if snapshot[:volume_id] == volume_id && snapshot[:state] == 'completed'
Expand Down Expand Up @@ -89,7 +89,7 @@ def query_instance_availability_zone
end

def query_mac_address(interface = 'eth0')
node[:network][:interfaces][interface][:addresses].select do |_, e|
node['network']['interfaces'][interface]['addresses'].select do |_, e|
e['family'] == 'lladdr'
end.keys.first.downcase
end
Expand Down
14 changes: 7 additions & 7 deletions providers/ebs_raid.rb
@@ -1,16 +1,16 @@
include Opscode::Aws::Ec2

action :auto_attach do
action :auto_attach do # ~FC017 https://github.com/acrmp/foodcritic/issues/387
package 'mdadm' do
action :install
end

# Baseline expectations.
node.set['aws'] ||= {}
node.set[:aws][:raid] ||= {}
node.set['aws']['raid'] ||= {}

# Mount point information.
node.set[:aws][:raid][@new_resource.mount_point] ||= {}
node.set['aws']['raid'][@new_resource.mount_point] ||= {}

# we're done we successfully located what we needed
if !already_mounted(@new_resource.mount_point) && !locate_and_mount(@new_resource.mount_point, @new_resource.mount_point_owner,
Expand Down Expand Up @@ -88,8 +88,8 @@ def update_node_from_md_device(md_device, mount_point)
raid_devices = `#{command}`
Chef::Log.info("already found the mounted device, created from #{raid_devices}")

node.set[:aws][:raid][mount_point][:raid_dev] = md_device.sub(/\/dev\//, '')
node.set[:aws][:raid][mount_point][:devices] = raid_devices
node.set['aws']['raid'][mount_point]['raid_dev'] = md_device.sub(%r{/dev/}, '')
node.set['aws']['raid'][mount_point]['devices'] = raid_devices
node.save unless Chef::Config[:solo]
end

Expand Down Expand Up @@ -418,8 +418,8 @@ def create_raid_disks(mount_point, mount_point_owner, mount_point_group, mount_p
end

# Assemble all the data bag meta data
node.set[:aws][:raid][mount_point][:raid_dev] = raid_dev
node.set[:aws][:raid][mount_point][:device_map] = devices
node.set['aws']['raid'][mount_point]['raid_dev'] = raid_dev
node.set['aws']['raid'][mount_point]['device_map'] = devices
node.save unless Chef::Config[:solo]
end
end
Expand Down
6 changes: 3 additions & 3 deletions providers/instance_monitoring.rb
Expand Up @@ -5,7 +5,7 @@ def whyrun_supported?
end

action :enable do
if is_monitoring_enabled
if monitoring_enabled?
Chef::Log.debug('Monitoring is already enabled for this instance')
else
converge_by('enable monitoring for this instance') do
Expand All @@ -16,7 +16,7 @@ def whyrun_supported?
end

action :disable do
if is_monitoring_enabled
if monitoring_enabled?
converge_by('disable monitoring for this instance') do
Chef::Log.info('Disabling monitoring for this instance')
ec2.unmonitor_instances(instance_ids: [instance_id])
Expand All @@ -28,7 +28,7 @@ def whyrun_supported?

private

def is_monitoring_enabled
def monitoring_enabled?
monitoring_state = ec2.describe_instances(instance_ids: [instance_id])[:reservations][0][:instances][0][:monitoring][:state]
Chef::Log.info("Current monitoring state for this instance is #{monitoring_state}")
monitoring_state == 'enabled'
Expand Down
11 changes: 5 additions & 6 deletions providers/resource_tag.rb
Expand Up @@ -31,7 +31,7 @@
# tags that begin with "aws" are reserved
converge_by("Updating the following tags for resource #{resource_id} (skipping AWS tags): " + updated_tags.inspect) do
Chef::Log.info("AWS: Updating the following tags for resource #{resource_id} (skipping AWS tags): " + updated_tags.inspect)
updated_tags.delete_if { |key, _value| key.to_s.match /^aws/ }
updated_tags.delete_if { |key, _value| key.to_s.match /^aws/ } # rubocop: disable Lint/AmbiguousRegexpLiteral
ec2.create_tags(resources: [resource_id], tags: updated_tags.collect { |k, v| { key: k, value: v } })
end
else
Expand All @@ -49,11 +49,10 @@
tags_to_delete = @new_resource.tags.keys

tags_to_delete.each do |key|
if @current_resource.tags.keys.include?(key) && @current_resource.tags[key] == @new_resource.tags[key]
converge_by("delete tag '#{key}' on resource #{resource_id} with value '#{@current_resource.tags[key]}'") do
ec2.delete_tags(resources: [resource_id], tags: [{ key => @new_resource.tags[key] }])
Chef::Log.info("AWS: Deleted tag '#{key}' on resource #{resource_id} with value '#{@current_resource.tags[key]}'")
end
next unless @current_resource.tags.keys.include?(key) && @current_resource.tags[key] == @new_resource.tags[key]
converge_by("delete tag '#{key}' on resource #{resource_id} with value '#{@current_resource.tags[key]}'") do
ec2.delete_tags(resources: [resource_id], tags: [{ key => @new_resource.tags[key] }])
Chef::Log.info("AWS: Deleted tag '#{key}' on resource #{resource_id} with value '#{@current_resource.tags[key]}'")
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions providers/s3_file.rb
Expand Up @@ -28,14 +28,14 @@ def do_s3_file(resource_action)
end

remote_path = new_resource.remote_path
remote_path.sub!(/^\/*/, '')
remote_path.sub!(%r{^/*}, '')

obj = ::Aws::S3::Object.new(bucket_name: new_resource.bucket, key: remote_path, client: s3)
s3url = obj.presigned_url(:get, expires_in: 300)

remote_file new_resource.name do
path new_resource.path
source s3url.gsub(/https:\/\/([\w\.\-]*)\.{1}s3.amazonaws.com:443/, 'https://s3.amazonaws.com:443/\1') # Fix for ssl cert issue
source s3url.gsub(%r{https://([\w\.\-]*)\.\{1\}s3.amazonaws.com:443}, 'https://s3.amazonaws.com:443/\1') # Fix for ssl cert issue
owner new_resource.owner
group new_resource.group
mode new_resource.mode
Expand Down

0 comments on commit 4bb85b1

Please sign in to comment.