IPs and tokens update

app/models/host/managed.rb

@@ -557,7 +557,14 @@ def overwrite=(value)
   end
 
   def require_ip_validation?
-    managed? and !compute? or (compute? and !compute_resource.provided_attributes.keys.include?(:ip))
+    # if it's not managed there's nowhere to specify an IP anyway
+    return false unless managed?
+    ip_for_compute = (compute? && compute_resource.provided_attributes.keys.include?(:ip))
+    ip_for_dns     = (subnet.present? && subnet.dns_id.present?) || (domain.present? && domain.dns_id.present?)
+    ip_for_dhcp    = subnet.present? && subnet.dhcp_id.present?
+    ip_for_token   = Setting[:token_duration] == 0 && !capabilities.include?(:image)
+    # Any of these conditions will require an IP, so chain with OR
+    ip_for_compute or ip_for_dns or ip_for_dhcp or ip_for_token
   end
 
   # if certname does not exist, use hostname instead

app/models/setting/provisioning.rb

@@ -20,7 +20,7 @@ def self.load_defaults
         self.set('ignore_puppet_facts_for_provisioning', N_("Does not update ipaddress and MAC values from Puppet facts"), false),
         self.set('query_local_nameservers', N_("Should Foreman query the locally configured name server or the SOA/NS authorities"), false),
         self.set('remote_addr', N_("If Foreman is running behind Passenger or a remote load balancer, the IP should be set here. This is a regular expression, so it can support several load balancers, i.e: (10.0.0.1|127.0.0.1)"), "127.0.0.1"),
-        self.set('token_duration', N_("Time in minutes installation tokens should be valid for, 0 to disable"), 0),
+        self.set('token_duration', N_("Time in minutes installation tokens should be valid for, 0 to disable"), 60),
         self.set('libvirt_default_console_address', N_("The IP address that should be used for the console listen address when provisioning new virtual machines via Libvirt"), "0.0.0.0"),
         self.set('update_ip_from_built_request', N_("Should we use the originating IP of the built request to update the host's IP?"), false),
         self.set('use_shortname_for_vms', N_("Should Foreman use the short hostname instead of the FQDN for creating new virtual machines"), false)

app/views/hosts/_unattended.html.erb

@@ -20,7 +20,8 @@
         <span id="subnet_select">
           <%= render 'common/domain', :item => @host %>
         </span>
-        <%= text_f f, :ip, :help_inline => _("IP Address for this host, if DHCP Smart proxy is enabled, this should be auto suggested to you") , :autocomplete => 'off'%>
+        <%= text_f f, :ip, :autocomplete => 'off',
+          :help_inline => popover("?", _("The IP address will be auto-suggested if you have a DHCP-enabled Smart Proxy on the Subnet selected above.<br/><br/>The IP address can be left blank if:<br/><ul><li>Provisioning Tokens are being used</li><li>The Subnet does not use DNS</li><li>The Subnet does not use DHCP</li><li>The Domain does not use DNS</li></ul>"), :title => _("The IP Address for this host")).html_safe %>
         <%= f.fields_for :interfaces do |interfaces| %>
           <%= render 'hosts/interfaces', :f => interfaces  %>
         <% end %>

test/fixtures/subnets.yml

@@ -23,4 +23,11 @@ three:
    mask: 255.255.255.0
    dhcp: one
    tftp: two
-   vlanid: 43
+   vlanid: 43
+
+four:
+   name: four
+   network: 3.3.5.0
+   mask: 255.255.255.0
+   tftp: two
+   vlanid: 44

test/unit/host_test.rb

@@ -4,6 +4,7 @@ class HostTest < ActiveSupport::TestCase
   setup do
     disable_orchestration
     User.current = User.find_by_login "admin"
+    Setting[:token_duration] = 0
   end
 
   test "should not save without a hostname" do
@@ -907,6 +908,83 @@ class Host::Valid < Host::Base ; belongs_to :domain ; end
     assert_equal smart_proxies(:puppetmaster).id, Host.find_by_name('sinn1636.lan').puppet_proxy_id
   end
 
+  # Ip validations
+  test "unmanaged hosts don't require an IP" do
+    h=Host.new
+    refute h.require_ip_validation?
+  end
+
+  test "CR's without IP attribute don't require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the CR
+    h=Host.new :managed => true,
+      :compute_resource => compute_resources(:one),
+      :compute_attributes => {:fake => "data"}
+    refute h.require_ip_validation?
+  end
+
+  test "CR's with IP attribute do require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the CR
+    h=Host.new :managed => true,
+      :compute_resource => compute_resources(:openstack),
+      :compute_attributes => {:fake => "data"}
+    assert h.require_ip_validation?
+  end
+
+  test "hosts with a DNS-enabled Domain do require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the domain
+    h=Host.new :managed => true, :domain => domains(:mydomain)
+    assert h.require_ip_validation?
+  end
+
+  test "hosts without a DNS-enabled Domain don't require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the domain
+    h=Host.new :managed => true, :domain => domains(:useless)
+    refute h.require_ip_validation?
+  end
+
+  test "hosts with a DNS-enabled Subnet do require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the subnet
+    h=Host.new :managed => true, :subnet => subnets(:one)
+    assert h.require_ip_validation?
+  end
+
+  test "hosts without a DNS-enabled Subnet don't require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the subnet
+    h=Host.new :managed => true, :subnet => subnets(:four)
+    refute h.require_ip_validation?
+  end
+
+  test "hosts with a DHCP-enabled Subnet do require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the subnet
+    h=Host.new :managed => true, :subnet => subnets(:two)
+    assert h.require_ip_validation?
+  end
+
+  test "hosts without a DHCP-enabled Subnet don't require an IP" do
+    Setting[:token_duration] = 30 #enable tokens so that we only test the subnet
+    h=Host.new :managed => true, :subnet => subnets(:four)
+    refute h.require_ip_validation?
+  end
+
+  test "with tokens enabled hosts don't require an IP" do
+    Setting[:token_duration] = 30
+    h=Host.new :managed => true
+    refute h.require_ip_validation?
+  end
+
+  test "with tokens disabled hosts do require an IP" do
+    h=Host.new :managed => true
+    assert h.require_ip_validation?
+  end
+
+  test "tokens disabled doesn't require an IP for image hosts" do
+    h=Host.new :managed => true
+    h.stubs(:capabilities).returns([:image])
+    refute h.require_ip_validation?
+  end
+
+  private
+
   def parse_json_fixture(relative_path)
     return JSON.parse(File.read(File.expand_path(File.dirname(__FILE__) + relative_path)))
   end