Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Linux EC2 detection, fix false detection, and add Windows detection #793

Merged
merged 7 commits into from
Apr 8, 2016
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 42 additions & 26 deletions lib/ohai/plugins/ec2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Author:: Tim Dysinger (<tim@dysinger.net>)
# Author:: Benjamin Black (<bb@chef.io>)
# Author:: Christopher Brown (<cb@chef.io>)
# Author:: Tim Smith (<tsmith@chef.io>)
# Copyright:: Copyright (c) 2009-2016 Chef Software, Inc.
# License:: Apache License, Version 2.0
#
Expand All @@ -19,8 +20,9 @@

# How we detect EC2 from easiest to hardest & least reliable
# 1. Ohai ec2 hint exists. This always works
# 2. DMI data mentions amazon. This catches HVM instances in a VPC
# 3. Has a xen MAC + can connect to metadata. This catches paravirt instances not in a VPC
# 2. Xen hypervisor UUID starts with 'ec2'. This catches Linux HVM & paravirt instances
# 3. DMI data mentions amazon. This catches HVM instances in a VPC
# 4. Kernel data mentioned Amazon. This catches Windows HVM & paravirt instances

require "ohai/mixin/ec2_metadata"
require "base64"
Expand All @@ -30,29 +32,8 @@

provides "ec2"

depends "network/interfaces"
depends "dmi"

# look for xen arp address
# this gets us detection of paravirt instances that are NOT within a VPC
def has_xen_mac?
network[:interfaces].values.each do |iface|
unless iface[:arp].nil?
if iface[:arp].value?("fe:ff:ff:ff:ff:ff")
# using MAC addresses from ARP is unreliable because they could time-out from the table
# fe:ff:ff:ff:ff:ff is actually a sign of Xen, not specifically EC2
deprecation_message <<-EOM
ec2 plugin: Detected EC2 by the presence of fe:ff:ff:ff:ff:ff in the ARP table. This method is unreliable and will be removed in a future version of ohai. Bootstrap using knife-ec2 or create "/etc/chef/ohai/hints/ec2.json" instead.
EOM
Ohai::Log.warn(deprecation_message)
Ohai::Log.debug("ec2 plugin: has_xen_mac? == true")
return true
end
end
end
Ohai::Log.debug("ec2 plugin: has_xen_mac? == false")
false
end
depends "kernel"

# look for amazon string in dmi bios data
# this gets us detection of HVM instances that are within a VPC
Expand All @@ -62,18 +43,53 @@ def has_ec2_dmi?
if dmi[:bios][:all_records][0][:Version] =~ /amazon/
Ohai::Log.debug("ec2 plugin: has_ec2_dmi? == true")
true
else
Ohai::Log.debug("ec2 plugin: has_ec2_dmi? == false")
false
end
rescue NoMethodError
Ohai::Log.debug("ec2 plugin: has_ec2_dmi? == false")
false
end
end

# looks for a xen UUID that starts with ec2
# this gets us detection of Linux HVM and Paravirt hosts
def has_ec2_xen_uuid?
if ::File.exist?("/sys/hypervisor/uuid")
if ::File.read("/sys/hypervisor/uuid") =~ /^ec2/
Ohai::Log.debug("ec2 plugin: has_ec2_xen_uuid? == true")
return true
end
else
Ohai::Log.debug("ec2 plugin: has_ec2_xen_uuid? == false")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't show up if the file exists and does not contain ec2. If you just remove the else line and put this at the end of the method unconditionally, it will probably do what you want since the if statement terminates with a return.

return false
end
end

# looks for the Amazon.com Organization in Windows Kernel data
# this gets us detection of Windows systems
def has_amazon_org?
begin
# detect an Organization of 'Amazon.com'
if kernel[:os_info][:organization] =~ /Amazon/
Ohai::Log.debug("ec2 plugin: has_amazon_org? == true")
true
else
Ohai::Log.debug("ec2 plugin: has_amazon_org? == false")
false
end
rescue NoMethodError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly uncomfortable catching the nonexistance of kernel[:os_info][:organization] with an exception. If it's set to not Amazon, then you'll also miss the logging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was pretty aggressive there since the keys aren't going to exist on a lot of platforms. kernel[:os_info][:organization] only exists on Windows and I haven't checked, but I assume we don't have kernel on every platform. Do we have a better way to check for the existence of the complete path to that key and it's content?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a DSL method attribute? that may be helpful

ohai/lib/ohai/dsl/plugin.rb

Lines 124 to 128 in 8f48805

def has_key?(name)
@data.has_key?(name)
end
alias :attribute? :has_key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so switch that to this?:

if kernel.attribute?('os_info') && kernel[:os_info].attribute?('organization') && kernel[:os_info][:organization] =~ /Amazon/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some offline discussion we're going to stick with this for now and add a helper to allow better checking nested Mashes with logging #794

Ohai::Log.debug("ec2 plugin: has_amazon_org? == false")
false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do the same if else reduction here and save yourself a repeat of this debug line, but this one isn't as important since it works.

end
end

def looks_like_ec2?
return true if hint?("ec2")

# Even if it looks like EC2 try to connect first
if has_ec2_dmi? || has_xen_mac?
if has_ec2_xen_uuid? || has_ec2_dmi? || has_amazon_org?
return true if can_metadata_connect?(Ohai::Mixin::Ec2Metadata::EC2_METADATA_ADDR, 80)
end
end
Expand All @@ -90,7 +106,7 @@ def looks_like_ec2?
next if k == "iam" && !hint?("iam")
ec2[k] = v
end
ec2[:userdata] = self.fetch_userdata
ec2[:userdata] = fetch_userdata
# ASCII-8BIT is equivalent to BINARY in this case
if ec2[:userdata] && ec2[:userdata].encoding.to_s == "ASCII-8BIT"
Ohai::Log.debug("ec2 plugin: Binary UserData Found. Storing in base64")
Expand Down
123 changes: 69 additions & 54 deletions spec/unit/plugins/ec2_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,29 @@
require "base64"

describe Ohai::System, "plugin ec2" do

before(:each) do
@plugin = get_plugin("ec2")
@plugin[:network] = { :interfaces => { :eth0 => {} } }
allow(File).to receive(:exist?).with("/etc/chef/ohai/hints/ec2.json").and_return(false)
allow(File).to receive(:exist?).with('C:\chef\ohai\hints/ec2.json').and_return(false)
allow(File).to receive(:exist?).with("/sys/hypervisor/uuid").and_return(false)
end

shared_examples_for "!ec2" do
it "should NOT attempt to fetch the ec2 metadata or set ec2 attribute" do
expect(@plugin).not_to receive(:http_client)
expect(@plugin[:ec2]).to be_nil
@plugin.run
let(:plugin) { get_plugin("ec2") }

it "DOESN'T attempt to fetch the ec2 metadata or set ec2 attribute" do
expect(plugin).not_to receive(:http_client)
expect(plugin[:ec2]).to be_nil
plugin.run
end
end

shared_examples_for "ec2" do
let(:plugin) { get_plugin("ec2") }

before(:each) do
@http_client = double("Net::HTTP client")
allow(@plugin).to receive(:http_client).and_return(@http_client)
allow(plugin).to receive(:http_client).and_return(@http_client)
allow(IO).to receive(:select).and_return([[], [1], []])
t = double("connection")
allow(t).to receive(:connect_nonblock).and_raise(Errno::EINPROGRESS)
Expand Down Expand Up @@ -69,12 +73,12 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["instance_type"]).to eq("c1.medium")
expect(@plugin[:ec2]["ami_id"]).to eq("ami-5d2dc934")
expect(@plugin[:ec2]["security_groups"]).to eql %w{group1 group2}
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["instance_type"]).to eq("c1.medium")
expect(plugin[:ec2]["ami_id"]).to eq("ami-5d2dc934")
expect(plugin[:ec2]["security_groups"]).to eql %w{group1 group2}
end

it "fetches binary userdata opaquely" do
Expand All @@ -87,17 +91,17 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "^_<8B>^H^H<C7>U^@^Csomething^@KT<C8><C9>,)<C9>IU(I-.I<CB><CC>I<E5>^B^@^Qz<BF><B0>^R^@^@^@", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["instance_type"]).to eq("c1.medium")
expect(@plugin[:ec2]["ami_id"]).to eq("ami-5d2dc934")
expect(@plugin[:ec2]["security_groups"]).to eql %w{group1 group2}
expect(@plugin[:ec2]["userdata"]).to eq(Base64.decode64("Xl88OEI+XkheSDxDNz5VXkBeQ3NvbWV0aGluZ15AS1Q8Qzg+PEM5PiwpPEM5\nPklVKEktLkk8Q0I+PENDPkk8RTU+XkJeQF5RejxCRj48QjA+XlJeQF5AXkA="))
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["instance_type"]).to eq("c1.medium")
expect(plugin[:ec2]["ami_id"]).to eq("ami-5d2dc934")
expect(plugin[:ec2]["security_groups"]).to eql %w{group1 group2}
expect(plugin[:ec2]["userdata"]).to eq(Base64.decode64("Xl88OEI+XkheSDxDNz5VXkBeQ3NvbWV0aGluZ15AS1Q8Qzg+PEM5PiwpPEM5\nPklVKEktLkk8Q0I+PENDPkk8RTU+XkJeQF5RejxCRj48QjA+XlJeQF5AXkA="))
end
end

it "should parse ec2 network/ directory as a multi-level hash" do
it "parses ec2 network/ directory as a multi-level hash" do
expect(@http_client).to receive(:get).
with("/2012-01-12/meta-data/").
and_return(double("Net::HTTP Response", :body => "network/", :code => "200"))
Expand All @@ -120,10 +124,10 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["network_interfaces_macs"]["12:34:56:78:9a:bc"]["public_hostname"]).to eql("server17.opscode.com")
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["network_interfaces_macs"]["12:34:56:78:9a:bc"]["public_hostname"]).to eql("server17.opscode.com")
end # context with common metadata paths

context "with ec2_iam hint file" do
Expand All @@ -137,7 +141,7 @@
end
end

it "should parse ec2 iam/ directory and collect iam/security-credentials/" do
it "parses ec2 iam/ directory and collect iam/security-credentials/" do
expect(@http_client).to receive(:get).
with("/2012-01-12/meta-data/").
and_return(double("Net::HTTP Response", :body => "iam/", :code => "200"))
Expand All @@ -154,11 +158,11 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["iam"]["security-credentials"]["MyRole"]["Code"]).to eql "Success"
expect(@plugin[:ec2]["iam"]["security-credentials"]["MyRole"]["Token"]).to eql "12345678"
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["iam"]["security-credentials"]["MyRole"]["Code"]).to eql "Success"
expect(plugin[:ec2]["iam"]["security-credentials"]["MyRole"]["Token"]).to eql "12345678"
end
end

Expand All @@ -171,7 +175,7 @@
end
end

it "should parse ec2 iam/ directory and NOT collect iam/security-credentials/" do
it "parses ec2 iam/ directory and NOT collect iam/security-credentials/" do
expect(@http_client).to receive(:get).
with("/2012-01-12/meta-data/").
and_return(double("Net::HTTP Response", :body => "iam/", :code => "200"))
Expand All @@ -188,14 +192,14 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["iam"]).to be_nil
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["iam"]).to be_nil
end
end

it "should ignore \"./\" and \"../\" on ec2 metadata paths to avoid infinity loops" do
it "ignores \"./\" and \"../\" on ec2 metadata paths to avoid infinity loops" do
expect(@http_client).to receive(:get).
with("/2012-01-12/meta-data/").
and_return(double("Net::HTTP Response", :body => ".\n./\n..\n../\npath1/.\npath2/./\npath3/..\npath4/../", :code => "200"))
Expand Down Expand Up @@ -227,12 +231,12 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]).not_to be_nil
end

it "should complete the run despite unavailable metadata" do
it "completes the run despite unavailable metadata" do
expect(@http_client).to receive(:get).
with("/2012-01-12/meta-data/").
and_return(double("Net::HTTP Response", :body => "metrics/", :code => "200"))
Expand All @@ -246,38 +250,50 @@
with("/2012-01-12/user-data/").
and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))

@plugin.run
plugin.run

expect(@plugin[:ec2]).not_to be_nil
expect(@plugin[:ec2]["metrics"]).to be_nil
expect(@plugin[:ec2]["metrics_vhostmd"]).to be_nil
expect(plugin[:ec2]).not_to be_nil
expect(plugin[:ec2]["metrics"]).to be_nil
expect(plugin[:ec2]["metrics_vhostmd"]).to be_nil
end
end # shared examples for ec2

describe "without dmi data, kernel organization, with xen mac, and metadata address connected" do
describe "with ec2 dmi data" do
it_behaves_like "ec2"

before(:each) do
allow(IO).to receive(:select).and_return([[], [1], []])
@plugin[:network][:interfaces][:eth0][:arp] = { "169.254.1.0" => "fe:ff:ff:ff:ff:ff" }
plugin[:dmi] = { :bios => { :all_records => [ { :Version => "4.2.amazon" } ] } }
end
end

describe "with amazon kernel data" do
it_behaves_like "ec2"

before(:each) do
plugin[:kernel] = { :os_info => { :organization => "Amazon.com" } }
end
end

it_should_behave_like "ec2"
describe "with EC2 Xen UUID" do
it_behaves_like "ec2"

it "warns that the arp table method is deprecated" do
expect(Ohai::Log).to receive(:warn).with(/will be removed/)
@plugin.has_xen_mac?
before(:each) do
allow(File).to receive(:exist?).with("/sys/hypervisor/uuid").and_return(true)
allow(File).to receive(:read).with("/sys/hypervisor/uuid").and_return("ec2a0561-e4d6-8e15-d9c8-2e0e03adcde8")
end
end

describe "with ec2 dmi data" do
it_should_behave_like "ec2"
describe "with non-EC2 Xen UUID" do
it_behaves_like "!ec2"

before(:each) do
@plugin[:dmi] = { :bios => { :all_records => [ { :Version => "4.2.amazon" } ] } }
allow(File).to receive(:exist?).with("/sys/hypervisor/uuid").and_return(true)
allow(File).to receive(:read).with("/sys/hypervisor/uuid").and_return("123a0561-e4d6-8e15-d9c8-2e0e03adcde8")
end
end

describe "with ec2 hint file" do
it_should_behave_like "ec2"
it_behaves_like "ec2"

before(:each) do
if windows?
Expand All @@ -291,7 +307,7 @@
end

describe "with rackspace hint file" do
it_should_behave_like "!ec2"
it_behaves_like "!ec2"

before(:each) do
allow(File).to receive(:exist?).with("/etc/chef/ohai/hints/rackspace.json").and_return(true)
Expand All @@ -302,13 +318,12 @@
end

describe "without any hints that it is an ec2 system" do
it_should_behave_like "!ec2"
it_behaves_like "!ec2"

before(:each) do
allow(File).to receive(:exist?).with("/etc/chef/ohai/hints/ec2.json").and_return(false)
allow(File).to receive(:exist?).with('C:\chef\ohai\hints/ec2.json').and_return(false)
@plugin[:dmi] = nil
@plugin[:network][:interfaces][:eth0][:arp] = { "169.254.1.0" => "00:50:56:c0:00:08" }
plugin[:dmi] = nil
end
end

Expand Down