Prevent queueing things for a zone that doesn't exist in the region

https://bugzilla.redhat.com/show_bug.cgi?id=1628658

app/models/miq_queue.rb

@@ -94,6 +94,8 @@ def self.lower_priority?(p1, p2)
   serialize :args, Array
   serialize :miq_callback, Hash
 
+  validates :zone, :inclusion => {:in => proc { Zone.in_my_region.pluck(:name) }}, :allow_nil => true
+
   STATE_READY   = 'ready'.freeze
   STATE_DEQUEUE = 'dequeue'.freeze
   STATE_WARN    = 'warn'.freeze

spec/lib/extensions/ar_nested_count_by_spec.rb

@@ -1,44 +1,45 @@
 describe "AR Nested Count By extension" do
   context "miq_queue with messages" do
+    let(:zone1) { EvmSpecHelper.local_miq_server.zone }
+    let(:zone2) { FactoryGirl.create(:zone) }
+    let(:zone3) { FactoryGirl.create(:zone) }
     before do
-      @zone = EvmSpecHelper.local_miq_server.zone
-
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role2", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => "east",     :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => "west",     :state => MiqQueue::STATE_READY,    :role => "role3", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_ERROR,    :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => @zone.name, :state => MiqQueue::STATE_WARN,     :role => "role3", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => "east",     :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
-      FactoryGirl.create(:miq_queue, :zone => "west",     :state => MiqQueue::STATE_ERROR,    :role => "role2", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role2", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_READY,    :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone2.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone3.name, :state => MiqQueue::STATE_READY,    :role => "role3", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_ERROR,    :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone1.name, :state => MiqQueue::STATE_WARN,     :role => "role3", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone2.name, :state => MiqQueue::STATE_DEQUEUE,  :role => "role1", :priority => 20)
+      FactoryGirl.create(:miq_queue, :zone => zone3.name, :state => MiqQueue::STATE_ERROR,    :role => "role2", :priority => 20)
     end
 
     it "should count by state, zone and role" do
       expect(MiqQueue.nested_count_by(%w(state zone role))).to eq(
         MiqQueue::STATE_READY   => {
-          @zone.name => {"role1" => 3},
-          "west"     => {"role3" => 1},
+          zone1.name => {"role1" => 3},
+          zone3.name => {"role3" => 1},
         },
         MiqQueue::STATE_DEQUEUE => {
-          @zone.name => {"role1" => 2, "role2" => 1},
-          "east"     => {"role1" => 2},
+          zone1.name => {"role1" => 2, "role2" => 1},
+          zone2.name => {"role1" => 2},
         },
         MiqQueue::STATE_WARN    => {
-          @zone.name => {"role3" => 1},
+          zone1.name => {"role3" => 1},
         },
         MiqQueue::STATE_ERROR   => {
-          @zone.name => {"role1" => 1},
-          "west"     => {"role2" => 1},
+          zone1.name => {"role1" => 1},
+          zone3.name => {"role2" => 1},
         }
       )
     end
 
     it "should respect nested where, and support individual args (vs an array)" do
-      expect(MiqQueue.where(:zone => "west").nested_count_by("role", "state")).to eq(
+      expect(MiqQueue.where(:zone => zone3.name).nested_count_by("role", "state")).to eq(
         "role3" => {MiqQueue::STATE_READY => 1},
         "role2" => {MiqQueue::STATE_ERROR => 1},
       )

spec/lib/tasks/evm_application_spec.rb

@@ -202,17 +202,18 @@ def pad(val, col, adjust = :rjust)
   context ".queue_status" do
     it "calculates oldest and counts" do
       tgt_time = 2.hours.ago
-      MiqQueue.put(:zone => "zone1", :class_name => "X", :method_name => "x", :created_on => 1.hour.ago)
-      MiqQueue.put(:zone => "zone1", :class_name => "X", :method_name => "x", :created_on => tgt_time)
-      MiqQueue.put(:zone        => "zone1",
+      zone = FactoryGirl.create(:zone)
+      MiqQueue.put(:zone => zone.name, :class_name => "X", :method_name => "x", :created_on => 1.hour.ago)
+      MiqQueue.put(:zone => zone.name, :class_name => "X", :method_name => "x", :created_on => tgt_time)
+      MiqQueue.put(:zone        => zone.name,
                    :class_name  => "X",
                    :method_name => "x",
                    :created_on  => 5.hours.ago,
                    :deliver_on  => 1.hour.from_now)
       expect(described_class.queue_status).to eq(
         [
           {
-            "Zone"   => "zone1",
+            "Zone"   => zone.name,
             "Queue"  => "generic",
             "Role"   => nil,
             "method" => "X.x",
@@ -225,20 +226,22 @@ def pad(val, col, adjust = :rjust)
 
     it "groups zone together" do
       tgt_time = 2.hours.ago
-      MiqQueue.put(:zone => "zone1", :class_name => "X", :method_name => "x", :created_on => tgt_time)
-      MiqQueue.put(:zone => "zone2", :class_name => "X", :method_name => "x", :created_on => tgt_time)
+      zone1 = FactoryGirl.create(:zone)
+      zone2 = FactoryGirl.create(:zone)
+      MiqQueue.put(:zone => zone1.name, :class_name => "X", :method_name => "x", :created_on => tgt_time)
+      MiqQueue.put(:zone => zone2.name, :class_name => "X", :method_name => "x", :created_on => tgt_time)
       expect(described_class.queue_status).to eq(
         [
           {
-            "Zone"   => "zone1",
+            "Zone"   => zone1.name,
             "Queue"  => "generic",
             "Role"   => nil,
             "method" => "X.x",
             "oldest" => tgt_time.strftime("%Y-%m-%d"),
             "count"  => 1,
           },
           {
-            "Zone"   => "zone2",
+            "Zone"   => zone2.name,
             "Queue"  => "generic",
             "Role"   => nil,
             "method" => "X.x",

spec/models/miq_action_spec.rb

@@ -178,8 +178,8 @@
 
     it "asynchronous" do
       input = {:synchronous => false}
-      zone  = 'Test Zone'
-      allow(@vm).to receive_messages(:my_zone => zone)
+      zone  = FactoryGirl.create(:zone)
+      allow(@vm).to receive_messages(:my_zone => zone.name)
 
       Timecop.freeze do
         date   = Time.now.utc - 1.day
@@ -190,7 +190,7 @@
         expect(msg.class_name).to eq(@vm.class.name)
         expect(msg.method_name).to eq('retire')
         expect(msg.args).to eq([[@vm], :date => date])
-        expect(msg.zone).to eq(zone)
+        expect(msg.zone).to eq(zone.name)
       end
     end
   end

spec/models/miq_queue_spec.rb

@@ -240,7 +240,7 @@
     ]
 
     message_parms.each do |mparms|
-      msg = FactoryGirl.create(:miq_queue, mparms)
+      msg = FactoryGirl.build(:miq_queue, mparms)
       expect(MiqQueue.format_short_log_msg(msg)).to eq("Message id: [#{msg.id}]")
       expect(MiqQueue.format_full_log_msg(msg)).to eq("Message id: [#{msg.id}], #{msg.handler_type} id: [#{msg.handler_id}], Zone: [#{msg.zone}], Role: [#{msg.role}], Server: [#{msg.server_guid}], MiqTask id: [#{msg.miq_task_id}], Ident: [#{msg.queue_name}], Target id: [#{msg.target_id}], Instance id: [#{msg.instance_id}], Task id: [#{msg.task_id}], Command: [#{msg.class_name}.#{msg.method_name}], Timeout: [#{msg.msg_timeout}], Priority: [#{msg.priority}], State: [#{msg.state}], Deliver On: [#{msg.deliver_on}], Data: [#{msg.data.nil? ? "" : "#{msg.data.length} bytes"}], Args: #{args_cleaned_password.inspect}")
     end
@@ -771,10 +771,11 @@ def self.some_method(single_arg)
     end
 
     it "should not unqueue a message from a different zone" do
+      zone = FactoryGirl.create(:zone)
       MiqQueue.put(
         :class_name  => 'MyClass',
         :method_name => 'method1',
-        :zone        => 'other_zone'
+        :zone        => zone.name
       )
 
       expect(MiqQueue.unqueue(
@@ -840,4 +841,19 @@ def self.some_method(single_arg)
       expect(MiqQueue.where(:id => q.id).count).to eq(0)
     end
   end
+
+  context "validates that the zone exists in the current region" do
+    it "with a matching region" do
+      zone = FactoryGirl.create(:zone)
+      expect(MiqQueue.create!(:state => "ready", :zone => zone.name)).to be_kind_of(MiqQueue)
+    end
+
+    it "without a matching region" do
+      expect { MiqQueue.create!(:state => "ready", :zone => "Missing Zone") }.to raise_error(ActiveRecord::RecordInvalid)
+    end
+
+    it "without a zone" do
+      expect(MiqQueue.create!(:state => "ready")).to be_kind_of(MiqQueue)
+    end
+  end
 end

spec/models/miq_request_spec.rb

@@ -68,7 +68,8 @@
     end
 
     it "#call_automate_event_queue" do
-      allow(MiqServer).to receive(:my_zone).and_return("New York")
+      zone = FactoryGirl.create(:zone)
+      allow(MiqServer).to receive(:my_zone).and_return(zone.name)
 
       expect(MiqQueue.count).to eq(0)
 
@@ -79,7 +80,7 @@
       expect(msg.class_name).to  eq(request.class.name)
       expect(msg.instance_id).to eq(request.id)
       expect(msg.method_name).to eq("call_automate_event")
-      expect(msg.zone).to        eq("New York")
+      expect(msg.zone).to        eq(zone.name)
       expect(msg.args).to        eq([event_name])
       expect(msg.msg_timeout).to eq(1.hour)
     end

spec/models/miq_task_spec.rb

@@ -177,8 +177,8 @@
     end
 
     it "should properly process MiqTask#generic_action_with_callback" do
-      zone = 'New York'
-      allow(MiqServer).to receive(:my_zone).and_return(zone)
+      zone = FactoryGirl.create(:zone)
+      allow(MiqServer).to receive(:my_zone).and_return(zone.name)
       opts = {
         :action => 'Feed',
         :userid => 'Flintstone'
@@ -201,7 +201,7 @@
       expect(message.class_name).to eq("MyClass")
       expect(message.method_name).to eq("my_method")
       expect(message.args).to eq([1, 2, 3])
-      expect(message.zone).to eq(zone)
+      expect(message.zone).to eq(zone.name)
       expect(message.miq_task_id).to eq(tid)
     end
   end
@@ -210,9 +210,9 @@
     let(:miq_task1) { FactoryGirl.create(:miq_task_plain) }
     let(:miq_task2) { FactoryGirl.create(:miq_task_plain) }
     let(:miq_task3) { FactoryGirl.create(:miq_task_plain) }
-    let(:zone) { 'New York' }
+    let(:zone)      { FactoryGirl.create(:zone) }
     before do
-      allow(MiqServer).to receive(:my_zone).and_return(zone)
+      allow(MiqServer).to receive(:my_zone).and_return(zone.name)
     end
 
     it "should queue up deletes when calling MiqTask.delete_by_id" do
@@ -225,7 +225,7 @@
       expect(message.args).to        be_kind_of(Array)
       expect(message.args.length).to eq(1)
       expect(message.args.first).to match_array([miq_task1.id, miq_task3.id])
-      expect(message.zone).to eq(zone)
+      expect(message.zone).to eq(zone.name)
     end
 
     it "should queue up proper deletes when calling MiqTask.delete_older" do
@@ -244,7 +244,7 @@
       expect(message.args.length).to eq(2)
       expect(message.args.first).to eq(date_5_minutes_ago)
       expect(message.args.second).to eq(condition)
-      expect(message.zone).to eq(zone)
+      expect(message.zone).to eq(zone.name)
 
       message.destroy
 
@@ -260,7 +260,7 @@
       expect(message.args.length).to eq(2)
       expect(message.args.first).to eq(date_11_minutes_ago)
       expect(message.args.second).to be nil
-      expect(message.zone).to eq(zone)
+      expect(message.zone).to eq(zone.name)
     end
   end
 

spec/models/mixins/authentication_mixin_spec.rb

@@ -90,7 +90,8 @@ def self.name; "TestClass"; end
 
   context "authorization event and check for container providers" do
     before do
-      allow(MiqServer).to receive(:my_zone).and_return("default")
+      zone = FactoryGirl.create(:zone)
+      allow(MiqServer).to receive(:my_zone).and_return(zone.name)
     end
 
     it "should be triggered for kubernetes" do