Fixes #30627 - mac address is not required for virtual resources

theforeman · Aug 27, 2020 · 1ec987a · 1ec987a
1 parent 2801602
commit 1ec987a
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 18 deletions.
diff --git a/app/models/nic/base.rb b/app/models/nic/base.rb
@@ -18,7 +18,7 @@ class Base < ApplicationRecord
     validate :mac_uniqueness,
       :if => proc { |nic| nic.managed? && nic.host && nic.host.managed? && !nic.host.compute? && !nic.virtual? && nic.mac.present? }
     validates :mac, :presence => true,
-              :if => proc { |nic| nic.managed? && nic.host_managed? && !nic.host.compute? && !nic.virtual? && (nic.provision? || nic.subnet.present? || nic.subnet6.present?) }
+              :if => proc { |nic| nic.managed? && nic.host_managed? && !nic.host.compute? && !nic.host.compute_provides?(:mac) && !nic.virtual? && (nic.provision? || nic.subnet.present? || nic.subnet6.present?) }
     validate :validate_mac_is_unicast,
       :if => proc { |nic| nic.managed? && !nic.virtual? }
     validates :mac, :mac_address => true, :allow_blank => true
@@ -45,6 +45,8 @@ class Base < ApplicationRecord
     validates :subnet, :belongs_to_host_taxonomy => { :taxonomy => :organization }
     validates :subnet6, :belongs_to_host_taxonomy => { :taxonomy => :organization }
 
+    validate :check_blank_mac_for_virtual_resources, on: :create
+
     scope :bmc, -> { where(:type => "Nic::BMC") }
     scope :bonds, -> { where(:type => "Nic::Bond") }
     scope :bridges, -> { where(:type => "Nic::Bridge") }
@@ -358,6 +360,12 @@ def interface_attribute_uniqueness(attr, base = Nic::Base.where(nil))
       db_candidates = db_candidates.select { |c| c.id != id && in_memory_candidates.map(&:id).include?(c.id) }
       errors.add(attr, :taken) if db_candidates.present?
     end
+
+    def check_blank_mac_for_virtual_resources
+      if virtual? && host.try(:compute_provides?, :mac) && host.uuid.empty? && mac.present?
+        errors.add(:mac, _("can't be set for this interface because it's provided by the compute resource"))
+      end
+    end
   end
 end
 

diff --git a/test/controllers/api/v2/hosts_controller_test.rb b/test/controllers/api/v2/hosts_controller_test.rb
@@ -26,27 +26,21 @@ def basic_attrs
       :operatingsystem_id  => Operatingsystem.find_by_name('Redhat').id,
       :puppet_proxy_id     => smart_proxies(:puppetmaster).id,
       :compute_resource_id => compute_resources(:one).id,
-      :root_pass           => "xybxa6JUkz63w",
-      :location_id         => taxonomies(:location1).id,
-      :organization_id     => taxonomies(:organization1).id,
-    }
-  end
-
-  def valid_compute_attrs
-    {
       :compute_attributes => {
         :cpus => 4,
         :memory => 1024,
       },
+      :root_pass           => "xybxa6JUkz63w",
+      :location_id         => taxonomies(:location1).id,
+      :organization_id     => taxonomies(:organization1).id,
     }
   end
 
   def valid_attrs
     net_attrs = {
-      :ip  => '10.0.0.20',
-      :mac => '52:53:00:1e:85:93',
+      :ip => '10.0.0.20',
     }
-    basic_attrs.merge(net_attrs).merge(valid_compute_attrs)
+    basic_attrs.merge(net_attrs)
   end
 
   def valid_attrs_with_root(extra_attrs = {})
@@ -71,7 +65,6 @@ def nics_attrs
     [{
       :primary => true,
       :ip => '10.0.0.20',
-      :mac => '00:11:22:33:44:00',
     }, {
       :type => 'bmc',
       :provider => 'IPMI',
@@ -310,8 +303,8 @@ def last_record
     assert_response :created
     assert_equal 2, last_record.interfaces.count
 
-    assert last_record.interfaces.find_by_mac('00:11:22:33:44:00').primary?
-    assert_equal Nic::Managed, last_record.interfaces.find_by_mac('00:11:22:33:44:00').class
+    assert last_record.interfaces.find_by_ip('10.0.0.20').primary?
+    assert_equal Nic::Managed, last_record.interfaces.find_by_ip('10.0.0.20').class
     assert_equal Nic::BMC,     last_record.interfaces.find_by_mac('00:11:22:33:44:01').class
   end
 
@@ -325,8 +318,8 @@ def last_record
     assert_response :created
     assert_equal 2, last_record.interfaces.count
 
-    assert last_record.interfaces.find_by_mac('00:11:22:33:44:00').primary?
-    assert_equal Nic::Managed, last_record.interfaces.find_by_mac('00:11:22:33:44:00').class
+    assert last_record.interfaces.find_by_ip('10.0.0.20').primary?
+    assert_equal Nic::Managed, last_record.interfaces.find_by_ip('10.0.0.20').class
     assert_equal Nic::BMC,     last_record.interfaces.find_by_mac('00:11:22:33:44:01').class
   end
 
@@ -352,7 +345,7 @@ def last_record
       assert_equal compute_attrs.vm_interfaces.count,
         last_record.interfaces.count
       assert_equal expected_compute_attributes(compute_attrs, 0),
-        last_record.interfaces.find_by_mac('00:11:22:33:44:00').compute_attributes
+        last_record.interfaces.find_by_ip('10.0.0.20').compute_attributes
       assert_equal expected_compute_attributes(compute_attrs, 1),
         last_record.interfaces.find_by_mac('00:11:22:33:44:01').compute_attributes
     end