From c5189c2da48c1bf291452f1ed3836cf16de5ee3a Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Sun, 15 Sep 2019 19:49:08 -0700 Subject: [PATCH] Update Chefstyle to 0.13.3 Fix most of the warnings. Signed-off-by: Tim Smith --- .rubocop.yml | 6 ++++ Rakefile | 4 +-- kitchen-ec2.gemspec | 2 +- lib/kitchen/driver/aws/instance_generator.rb | 2 +- .../driver/aws/standard_platform/centos.rb | 1 + .../driver/aws/standard_platform/freebsd.rb | 3 +- lib/kitchen/driver/ec2.rb | 33 ++++++++++--------- .../driver/aws/instance_generator_spec.rb | 14 ++++---- .../driver/aws/standard_platform_spec.rb | 12 +++---- spec/kitchen/driver/ec2_spec.rb | 17 ++++------ 10 files changed, 48 insertions(+), 46 deletions(-) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..e698f129 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,6 @@ +AllCops: + TargetRubyVersion: 2.3 + +Lint/IneffectiveAccessModifier: + Exclude: + - lib/kitchen/driver/aws/standard_platform.rb \ No newline at end of file diff --git a/Rakefile b/Rakefile index ab586123..0fb599ad 100644 --- a/Rakefile +++ b/Rakefile @@ -20,9 +20,9 @@ RuboCop::RakeTask.new(:style) do |task| end desc "Run all quality tasks" -task quality: [:style, :stats] +task quality: %i{style stats} require "yard" YARD::Rake::YardocTask.new -task default: [:test, :quality] +task default: %i{test quality} diff --git a/kitchen-ec2.gemspec b/kitchen-ec2.gemspec index 25b1f4cc..0d9a547e 100644 --- a/kitchen-ec2.gemspec +++ b/kitchen-ec2.gemspec @@ -33,6 +33,6 @@ Gem::Specification.new do |gem| # style and complexity libraries are tightly version pinned as newer releases # may introduce new and undesireable style choices which would be immediately # enforced in CI - gem.add_development_dependency "chefstyle", "= 0.6.0" + gem.add_development_dependency "chefstyle", "= 0.13.3" gem.add_development_dependency "climate_control" end diff --git a/lib/kitchen/driver/aws/instance_generator.rb b/lib/kitchen/driver/aws/instance_generator.rb index 5a3ba892..c84f1ac1 100644 --- a/lib/kitchen/driver/aws/instance_generator.rb +++ b/lib/kitchen/driver/aws/instance_generator.rb @@ -127,7 +127,7 @@ def ec2_instance_data # rubocop:disable Metrics/MethodLength, Metrics/AbcSize if config[:iam_profile_name] i[:iam_instance_profile] = { name: config[:iam_profile_name] } end - if !config.fetch(:associate_public_ip, nil).nil? + unless config.fetch(:associate_public_ip, nil).nil? i[:network_interfaces] = [{ device_index: 0, diff --git a/lib/kitchen/driver/aws/standard_platform/centos.rb b/lib/kitchen/driver/aws/standard_platform/centos.rb index bca38eb2..36b751b8 100644 --- a/lib/kitchen/driver/aws/standard_platform/centos.rb +++ b/lib/kitchen/driver/aws/standard_platform/centos.rb @@ -27,6 +27,7 @@ def username # Centos 6.x images use root as the username (but the "centos 6" # updateable image uses "centos") return "root" if version && version.start_with?("6.") + "centos" end diff --git a/lib/kitchen/driver/aws/standard_platform/freebsd.rb b/lib/kitchen/driver/aws/standard_platform/freebsd.rb index 539abf53..9cb90270 100644 --- a/lib/kitchen/driver/aws/standard_platform/freebsd.rb +++ b/lib/kitchen/driver/aws/standard_platform/freebsd.rb @@ -27,8 +27,7 @@ def username "ec2-user" end - def sudo_command - end + def sudo_command; end def image_search search = { diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index 8bc57f0d..d46a7158 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -56,9 +56,7 @@ class Ec2 < Kitchen::Driver::Base default_config :region, ENV["AWS_REGION"] || "us-east-1" default_config :shared_credentials_profile, ENV["AWS_PROFILE"] default_config :availability_zone, nil - default_config :instance_type do |driver| - driver.default_instance_type - end + default_config :instance_type, &:default_instance_type default_config :ebs_optimized, false default_config :security_group_ids, nil default_config :security_group_filter, nil @@ -80,9 +78,7 @@ class Ec2 < Kitchen::Driver::Base default_config :aws_secret_access_key, nil default_config :aws_session_token, nil default_config :aws_ssh_key_id, ENV["AWS_SSH_KEY_ID"] - default_config :image_id do |driver| - driver.default_ami - end + default_config :image_id, &:default_ami default_config :image_search, nil default_config :username, nil default_config :associate_public_ip, nil @@ -110,7 +106,7 @@ def self.validation_error(driver, old_key, new_key) end # TODO: remove these in 1.1 - deprecated_configs = [:ebs_volume_size, :ebs_delete_on_termination, :ebs_device_name] + deprecated_configs = %i{ebs_volume_size ebs_delete_on_termination ebs_device_name} deprecated_configs.each do |d| validations[d] = lambda do |attr, val, driver| unless val.nil? @@ -200,6 +196,7 @@ def self.validation_error(driver, old_key, new_key) def create(state) return if state[:server_id] + update_username(state) info(Kitchen::Util.outdent!(<<-END)) unless config[:skip_cost_warning] @@ -422,6 +419,7 @@ def submit_server def config return super unless @config + @config end @@ -429,7 +427,7 @@ def config def expand_config(conf, key) configs = [] - if conf[key] && conf[key].kind_of?(Array) + if conf[key] && conf[key].is_a?(Array) values = conf[key] values.each do |value| new_config = conf.clone @@ -446,7 +444,7 @@ def expand_config(conf, key) def submit_spots(state) configs = [config] expanded = [] - keys = [:instance_type, :subnet_id] + keys = %i{instance_type subnet_id} keys.each do |key| configs.each do |conf| @@ -492,7 +490,7 @@ def submit_spot(state) def create_spot_request request_duration = config[:spot_wait] config_spot_price = config[:spot_price].to_s - if ["ondemand", "on-demand"].include?(config_spot_price) + if %w{ondemand on-demand}.include?(config_spot_price) spot_price = "" else spot_price = config_spot_price @@ -543,8 +541,7 @@ def wait_until_volumes_ready(server, state) ready_volume_count = 0 if aws_instance.exists? described_volume_count = ec2.client.describe_volumes(filters: [ - { name: "attachment.instance-id", values: ["#{state[:server_id]}"] }] - ).volumes.length + { name: "attachment.instance-id", values: ["#{state[:server_id]}"] }]).volumes.length aws_instance.volumes.each { ready_volume_count += 1 } end (described_volume_count > 0) && (described_volume_count == ready_volume_count) @@ -626,6 +623,7 @@ def with_request_limit_backoff(state) yield rescue ::Aws::EC2::Errors::RequestLimitExceeded, ::Aws::Waiters::Errors::UnexpectedError => e raise unless retries < 5 && e.message.include?("Request limit exceeded") + retries += 1 info("Request limit exceeded for instance <#{state[:server_id]}>." \ " Trying again in #{retries**2} seconds.") @@ -779,11 +777,13 @@ def image_info(image) # @return [void] def create_security_group(state) return if state[:auto_security_group_id] + # Work out which VPC, if any, we are creating in. vpc_id = if config[:subnet_id] # Get the VPC ID for the subnet. subnets = ec2.client.describe_subnets(filters: [{ name: "subnet-id", values: [config[:subnet_id]] }]).subnets raise "Subnet #{config[:subnet_id]} not found during security group creation" if subnets.empty? + subnets.first.vpc_id else # Try to check for a default VPC. @@ -799,13 +799,13 @@ def create_security_group(state) # Create the SG. params = { group_name: "kitchen-#{Array.new(8) { rand(36).to_s(36) }.join}", - description: "Test Kitchen for #{instance.name} by #{Etc.getlogin || 'nologin'} on #{Socket.gethostname}", + description: "Test Kitchen for #{instance.name} by #{Etc.getlogin || "nologin"} on #{Socket.gethostname}", } params[:vpc_id] = vpc_id if vpc_id resp = ec2.client.create_security_group(params) state[:auto_security_group_id] = resp.group_id info("Created automatic security group #{state[:auto_security_group_id]}") - debug(" in VPC #{vpc_id || 'none'}") + debug(" in VPC #{vpc_id || "none"}") # Set up SG rules. ec2.client.authorize_security_group_ingress( group_id: state[:auto_security_group_id], @@ -828,6 +828,7 @@ def create_security_group(state) # @return [void] def create_key(state) return if state[:auto_key_id] + # Encode a bunch of metadata into the name because that's all we can # set for a key pair. name_parts = [ @@ -842,7 +843,7 @@ def create_key(state) # to rapidly exhaust local entropy by creating a lot of keys. So this is # probably fine. If you want very high security, probably don't use this # feature anyway. - resp = ec2.client.create_key_pair(key_name: "kitchen-#{name_parts.join('-')}") + resp = ec2.client.create_key_pair(key_name: "kitchen-#{name_parts.join("-")}") state[:auto_key_id] = resp.key_name info("Created automatic key pair #{state[:auto_key_id]}") # Write the key out with safe permissions @@ -862,6 +863,7 @@ def create_key(state) # @return [void] def delete_security_group(state) return unless state[:auto_security_group_id] + info("Removing automatic security group #{state[:auto_security_group_id]}") ec2.client.delete_security_group(group_id: state[:auto_security_group_id]) state.delete(:auto_security_group_id) @@ -874,6 +876,7 @@ def delete_security_group(state) # @return [void] def delete_key(state) return unless state[:auto_key_id] + info("Removing automatic key pair #{state[:auto_key_id]}") ec2.client.delete_key_pair(key_name: state[:auto_key_id]) state.delete(:auto_key_id) diff --git a/spec/kitchen/driver/aws/instance_generator_spec.rb b/spec/kitchen/driver/aws/instance_generator_spec.rb index d598abc1..fae75f7e 100644 --- a/spec/kitchen/driver/aws/instance_generator_spec.rb +++ b/spec/kitchen/driver/aws/instance_generator_spec.rb @@ -25,7 +25,7 @@ describe Kitchen::Driver::Aws::InstanceGenerator do - let(:config) { Hash.new } + let(:config) { {} } let(:resource) { instance_double(Aws::EC2::Resource) } let(:ec2) { instance_double(Kitchen::Driver::Aws::Client, resource: resource) } let(:logger) { instance_double(Logger) } @@ -115,12 +115,12 @@ it "returns the minimum data" do expect(generator.ec2_instance_data).to eq( - instance_type: "micro", - ebs_optimized: true, - image_id: "ami-123", - key_name: nil, - subnet_id: "s-456", - private_ip_address: "0.0.0.0" + instance_type: "micro", + ebs_optimized: true, + image_id: "ami-123", + key_name: nil, + subnet_id: "s-456", + private_ip_address: "0.0.0.0" ) end end diff --git a/spec/kitchen/driver/aws/standard_platform_spec.rb b/spec/kitchen/driver/aws/standard_platform_spec.rb index 35ea9fa7..ecee7eae 100644 --- a/spec/kitchen/driver/aws/standard_platform_spec.rb +++ b/spec/kitchen/driver/aws/standard_platform_spec.rb @@ -73,8 +73,7 @@ block_device_mappings: [], root_device_type: "other", virtualization_type: "other", - name: "ubuntu" - ) + name: "ubuntu") end let(:img2) do instance_double(::Aws::EC2::Image, @@ -83,8 +82,7 @@ block_device_mappings: [], root_device_type: "other", virtualization_type: "other", - name: "ubuntu" - ) + name: "ubuntu") end let(:img3) do instance_double(::Aws::EC2::Image, @@ -93,8 +91,7 @@ block_device_mappings: [], root_device_type: "ebs", virtualization_type: "other", - name: "ubuntu" - ) + name: "ubuntu") end let(:img4) do instance_double(::Aws::EC2::Image, @@ -103,8 +100,7 @@ block_device_mappings: [], root_device_type: "ebs", virtualization_type: "hvm", - name: "ubuntu" - ) + name: "ubuntu") end let(:images) { [img1, img2, img3, img4] } let(:sorted_images) { [img4, img3, img2, img1] } diff --git a/spec/kitchen/driver/ec2_spec.rb b/spec/kitchen/driver/ec2_spec.rb index e15889ef..3259016f 100644 --- a/spec/kitchen/driver/ec2_spec.rb +++ b/spec/kitchen/driver/ec2_spec.rb @@ -72,7 +72,8 @@ it "plugin_version is set to Kitchen::Vagrant::VERSION" do expect(driver.diagnose_plugin[:version]).to eq( - Kitchen::Driver::EC2_VERSION) + Kitchen::Driver::EC2_VERSION + ) end describe "default_config" do @@ -116,8 +117,7 @@ private_dns_name: private_dns_name, public_ip_address: public_ip_address, private_ip_address: private_ip_address, - id: id - ) + id: id) end it "returns nil if all sources are nil" do @@ -429,23 +429,20 @@ end it "second, it checks for prescence of described volumes" do expect(aws_instance).to receive(:exists?).and_return(true) - expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length - ).and_return(0) + expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length).and_return(0) expect(aws_instance).to receive(:volumes).and_return([]) expect(driver.wait_until_volumes_ready(server, state)).to eq(false) end it "third, it compares the described volumes and instance volumes" do expect(aws_instance).to receive(:exists?).and_return(true) - expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length - ).and_return(2) + expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length).and_return(2) expect(aws_instance).to receive(:volumes).and_return([volume]) expect(driver.wait_until_volumes_ready(server, state)).to eq(false) end context "when it exists, and both client and instance agree on volumes" do it "returns true" do expect(aws_instance).to receive(:exists?).and_return(true) - expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length - ).and_return(1) + expect(actual_client).to receive_message_chain(:describe_volumes, :volumes, :length).and_return(1) expect(aws_instance).to receive(:volumes).and_return([volume]) expect(driver.wait_until_volumes_ready(server, state)).to eq(true) end @@ -522,7 +519,7 @@ allow(instance).to receive(:name).and_return("instance_name") expect(actual_client).to receive(:create_key_pair).with(key_name: /kitchen-/).and_return(double(key_name: "expected-key-name", key_material: "RSA PRIVATE KEY")) - fake_file = double() + fake_file = double allow(File).to receive(:open).and_call_original expect(File).to receive(:open).with("/kitchen/.kitchen/instance_name.pem", kind_of(Numeric), kind_of(Numeric)).and_yield(fake_file) expect(fake_file).to receive(:write).with("RSA PRIVATE KEY")