remove event tags and contact tags, link check to rules only via tags

features/cli_flapjack-populator.feature

@@ -31,11 +31,7 @@ test:
         "interval": 7200,
         "rollup_threshold": null
       }
-    },
-    "tags": [
-      "legend",
-      "first computer programmer"
-    ]
+    }
   }
 ]
 """

features/notification_rules.feature

@@ -12,15 +12,15 @@ Feature: Notification rules on a per contact basis
       | c6  | Jive       | Smith     | jive@example.com  | +61400000006 | America/Los_Angeles |
 
     And the following checks exist:
-      | id  | name                       | contacts |
-      | 1   | foo:ping                   | c1       |
-      | 2   | bar:ping                   | c1,c2,c3 |
-      | 3   | baz:ping                   | c1,c3    |
-      | 4   | buf:ping                   | c1,c2,c3 |
-      | 5   | foo-app-01.xyz:Disk / Util | c4,c6    |
-      | 6   | foo-app-01.xyz:Memory Util | c4,c6    |
-      | 7   | foo-app-01.xyz:ping        | c4,c6    |
-      | 8   | blakes7:ping               | c2       |
+      | id  | name                       | contacts | tags                       |
+      | 1   | foo:ping                   | c1       | foo:ping                   |
+      | 2   | bar:ping                   | c1,c2,c3 | bar:ping                   |
+      | 3   | baz:ping                   | c1,c3    | baz:ping                   |
+      | 4   | buf:ping                   | c1,c2,c3 | buf:ping                   |
+      | 5   | foo-app-01.xyz:Disk / Util | c4,c6    | foo-app-01.xyz:Disk / Util |
+      | 6   | foo-app-01.xyz:Memory Util | c4,c6    | foo-app-01.xyz:Memory Util |
+      | 7   | foo-app-01.xyz:ping        | c4,c6    | foo-app-01.xyz:ping        |
+      | 8   | blakes7:ping               | c2       | blakes7:ping               |
 
     And user c1 has the following notification intervals:
       | email | sms |
@@ -39,21 +39,21 @@ Feature: Notification rules on a per contact basis
       | 15    | 60  |
 
     And user c1 has the following notification rules:
-      | checks    | unknown_media | warning_media | critical_media   | warning_blackhole | critical_blackhole | time_restrictions |
+      | tags    | unknown_media | warning_media | critical_media   | warning_blackhole | critical_blackhole | time_restrictions |
       |           |               | email         | sms,email        | true              | true               |                   |
       | foo:ping  |               | email         | sms,email        |                   |                    | 8-18 weekdays     |
       | bar:ping  | email         |               | sms,email        | true              |                    |                   |
       | baz:ping  |               | email         | sms,email        |                   |                    |                   |
 
     And user c2 has the following notification rules:
-      | checks                | tags | warning_media | critical_media   | warning_blackhole | critical_blackhole |
+      | tags                | tags | warning_media | critical_media   | warning_blackhole | critical_blackhole|
       |                       |      | email         | email            |                   |                    |
       |                       |      | sms           | sms              |                   |                    |
       | bar:ping,blakes7:ping |      | email         | email,sms        |                   |                    |
       | bar:ping,blakes7:ping | wags |               |                  | true              | true               |
 
     And user c3 has the following notification rules:
-      | checks    | warning_media | critical_media   | warning_blackhole | critical_blackhole |
+      | tags      | warning_media | critical_media   | warning_blackhole | critical_blackhole |
       |           | email         | email            |                   |                    |
       | baz:ping  | sms           | sms              |                   |                    |
       | buf:ping  | email         | email            |                   |                    |
@@ -71,7 +71,7 @@ Feature: Notification rules on a per contact basis
       | email         | email, sms     |
 
     And user c6 has the following notification rules:
-      | checks              | tags            | warning_media | critical_media   |
+      | tags              | tags            | warning_media | critical_media   |
       |                     |                 |               |                  |
       | foo-app-01.xyz:ping | check_disk      | email         | email            |
 

features/steps/events_steps.rb

@@ -306,12 +306,25 @@ def stringify(obj)
   checks.hashes.each do |check_data|
     check = find_or_create_check(check_data)
 
-    next if check_data['contacts'].nil?
-    check_data['contacts'].split(',').map(&:strip).each do |contact_id|
-      contact = Flapjack::Data::Contact.find_by_id(contact_id)
-      expect(contact).not_to be_nil
-      check.contacts << contact
+    unless check_data['contacts'].nil? || check_data['contacts'].strip.empty?
+      check_data['contacts'].split(',').map(&:strip).each do |contact_id|
+        contact = Flapjack::Data::Contact.find_by_id(contact_id)
+        expect(contact).not_to be_nil
+        check.contacts << contact
+      end
+    end
+
+    unless check_data['tags'].nil? || check_data['tags'].strip.empty?
+      check_data['tags'].split(',').map(&:strip).each do |tag_name|
+        tag = Flapjack::Data::Tag.intersect(:name => tag_name).all.first
+        if tag.nil?
+          tag = Flapjack::Data::Tag.new(:name => tag_name)
+          tag.save
+        end
+        check.tags << tag
+      end
     end
+
   end
 end
 
@@ -371,7 +384,6 @@ def stringify(obj)
   end
 
   rules.hashes.each do |rule|
-    checks           = rule['checks']      ? rule['checks'].split(',').map(&:strip)       : []
     tags             = rule['tags']        ? rule['tags'].split(',').map(&:strip)         : []
     media = {
       :unknown  => (rule['unknown_media']      ? rule['unknown_media'].split(',').map(&:strip)  : []),
@@ -399,15 +411,6 @@ def stringify(obj)
     new_rule = Flapjack::Data::NotificationRule.new(rule_data)
     expect(new_rule.save).to be true
 
-    checks.each do |check_name|
-      check = Flapjack::Data::Check.intersect(:name => check_name).all.first
-      if check.nil?
-        check = Flapjack::Data::Check.new(:name => check_name)
-        expect(check.save).to be true
-      end
-      new_rule.checks << check
-    end
-
     tags.each do |tag_name|
       tag = Flapjack::Data::Tag.intersect(:name => tag_name).all.first
       if tag.nil?

lib/flapjack/cli/import.rb

@@ -38,7 +38,6 @@ def contacts
               next
             end
             media_data = contact_data.delete('media')
-            tag_data   = contact_data.delete('tags')
             contact = Flapjack::Data::Contact.new(contact_data)
             contact.save
 
@@ -52,17 +51,6 @@ def contacts
                 contact.media << medium
               end
             end
-
-            unless tag_data.nil? || tag_data.empty?
-              tag_data.each do |tag_name|
-                tag = Flapjack::Data::Tag.intersect(:name => tag_name).all.first
-                if tag.nil?
-                  tag = Flapjack::Data::Tag.new(:name => tag_name)
-                  tag.save
-                end
-                contact.tags << tag
-              end
-            end
           end
         end
 

lib/flapjack/data/check.rb

@@ -55,20 +55,17 @@ class Check
       has_sorted_set :unscheduled_maintenances_by_end, :class_name => 'Flapjack::Data::UnscheduledMaintenance',
         :key => :end_time
 
-      has_sorted_set :notification_blocks, :class_name => 'Flapjack::Data::NotificationBlock',
-        :key => :expire_at
-
-      has_and_belongs_to_many :alerting_media, :class_name => 'Flapjack::Data::Medium',
-        :inverse_of => :alerting_checks
-
-      # the following 3 associations are used internally, for the notification
+      # the following associations are used internally, for the notification
       # and alert queue inter-pikelet workflow
       has_many :notifications, :class_name => 'Flapjack::Data::Notification'
       has_many :alerts, :class_name => 'Flapjack::Data::Alert'
       has_many :rollup_alerts, :class_name => 'Flapjack::Data::RollupAlert'
 
-      has_and_belongs_to_many :notification_rules, :class_name => 'Flapjack::Data::Check',
-        :inverse_of => :checks
+      has_sorted_set :notification_blocks, :class_name => 'Flapjack::Data::NotificationBlock',
+        :key => :expire_at
+
+      has_and_belongs_to_many :alerting_media, :class_name => 'Flapjack::Data::Medium',
+        :inverse_of => :alerting_checks
 
       validates :name, :presence => true
       validates :state,

lib/flapjack/data/contact.rb

@@ -35,9 +35,6 @@ class Contact
       has_and_belongs_to_many :checks, :class_name => 'Flapjack::Data::Check',
         :inverse_of => :contacts
 
-      has_and_belongs_to_many :tags, :class_name => 'Flapjack::Data::Tag',
-        :inverse_of => :contacts
-
       has_many :media, :class_name => 'Flapjack::Data::Medium'
       has_one :pagerduty_credentials, :class_name => 'Flapjack::Data::PagerdutyCredentials'
 
@@ -111,8 +108,7 @@ def as_json(opts = {})
             :checks                => opts[:check_ids] || [],
             :media                 => opts[:medium_ids] || [],
             :pagerduty_credentials => opts[:pagerduty_credentials_ids] || [],
-            :notification_rules    => opts[:notification_rule_ids] || [],
-            :tags                  => opts[:tag_ids] || [],
+            :notification_rules    => opts[:notification_rule_ids] || []
           }
         )
       end

lib/flapjack/data/event.rb

@@ -7,12 +7,12 @@ module Flapjack
   module Data
     class Event
 
-      attr_accessor :counter, :id_hash, :tags
+      attr_accessor :counter, :id_hash
 
       attr_reader :id, :summary, :details, :acknowledgement_id, :perfdata
 
       REQUIRED_KEYS = ['type', 'state', 'entity', 'check', 'summary']
-      OPTIONAL_KEYS = ['time', 'details', 'acknowledgement_id', 'duration', 'tags', 'perfdata']
+      OPTIONAL_KEYS = ['time', 'details', 'acknowledgement_id', 'duration', 'perfdata']
 
       VALIDATIONS = {
         proc {|e| e['type'].is_a?(String) &&
@@ -55,10 +55,6 @@ class Event
                  (e['duration'].is_a?(String) && !!(e['duration'] =~ /^\d+$/)) } =>
           "duration must be a positive integer, or a string castable to one",
 
-        proc {|e| e['tags'].nil? ||
-                  (e['tags'].is_a?(Array) &&
-                   e['tags'].all? {|tag| tag.is_a?(String)}) } =>
-          "tags must be an array of strings",
       }
 
       def self.parse_and_validate(raw, opts = {})
@@ -169,7 +165,7 @@ def self.test_notifications(queue, checks, opts = {})
       def initialize(attrs = {})
         @id = "#{attrs['entity']}:#{attrs['check']}"
         [:type, :state, :time, :summary,
-         :perfdata, :details, :acknowledgement_id, :duration, :tags].each do |key|
+         :perfdata, :details, :acknowledgement_id, :duration].each do |key|
           instance_variable_set("@#{key.to_s}", attrs[key.to_s])
         end
         # details and perfdata are optional. set to nil if they only contain whitespace

lib/flapjack/data/medium.rb

@@ -23,6 +23,9 @@ class Medium
 
       belongs_to :contact, :class_name => 'Flapjack::Data::Contact', :inverse_of => :media
 
+      has_and_belongs_to_many :notification_rule_states,
+        :class_name => 'Flapjack::Data::NotificationRuleState', :inverse_of => :media
+
       has_many :alerts, :class_name => 'Flapjack::Data::Alert'
 
       has_and_belongs_to_many :alerting_checks,
@@ -31,9 +34,6 @@ class Medium
       has_sorted_set :notification_blocks,
         :class_name => 'Flapjack::Data::NotificationBlock', :key => :expire_at
 
-      has_and_belongs_to_many :notification_rule_states,
-        :class_name => 'Flapjack::Data::NotificationRuleState', :inverse_of => :media
-
       index_by :type
 
       validates :type, :presence => true,
@@ -106,18 +106,13 @@ def update_sent_alert_keys(opts = {})
             block.save
           else
             # need to remove it from the sorted_set that uses expire_at as a key,
-            # change and re-add -- see https://github.com/ali-graham/sandstorm/issues/1
-            # TODO should this be in a multi/exec block?
+            # change and re-add -- see https://github.com/flapjack/sandstorm/issues/1
             self.notification_blocks.delete(block)
-            if check
-              check.notification_blocks.delete(block)
-            end
+            check.notification_blocks.delete(block) if check
             block.expire_at = expire_at
             block.save
           end
-          if check
-            check.notification_blocks << block
-          end
+          check.notification_blocks << block if check
           self.notification_blocks << block
         end
       end

lib/flapjack/data/notification_rule.rb

@@ -24,17 +24,14 @@ class NotificationRule
 
       define_attributes :time_restrictions_json => :string
 
-      has_and_belongs_to_many :checks, :class_name => 'Flapjack::Data::Check',
-        :inverse_of => :notification_rules
-
-      has_and_belongs_to_many :tags, :class_name => 'Flapjack::Data::Tag',
-        :inverse_of => :notification_rules
-
       belongs_to :contact, :class_name => 'Flapjack::Data::Contact',
         :inverse_of => :notification_rules
 
       has_many :states, :class_name => 'Flapjack::Data::NotificationRuleState'
 
+      has_and_belongs_to_many :tags, :class_name => 'Flapjack::Data::Tag',
+        :inverse_of => :notification_rules
+
       validates_each :time_restrictions_json do |record, att, value|
         unless value.nil?
           restrictions = JSON.parse(value)
@@ -81,12 +78,11 @@ def self.time_restriction_to_icecube_schedule(tr, timezone)
       end
 
       def is_specific?
-        !checks.empty? || !tags.empty?
+        !tags.empty?
       end
 
       def is_match?(check)
-        (self.checks.empty? || self.checks.ids.include?(check.id)) &&
-        (self.tags.empty? || (self.tags.ids - check.tags.ids).empty? )
+        self.tags.empty? || (self.tags.ids - check.tags.ids).empty?
       end
 
       # nil time_restrictions matches
@@ -114,9 +110,8 @@ def as_json(opts = {})
           :time_restrictions          => time_restrictions,
           :links => {
             :contacts                 => opts[:contact_ids] || [],
-            :notification_rule_states => opts[:states_ids] || [],
-            :checks                   => opts[:tag_ids] || [],
             :tags                     => opts[:tag_ids] || [],
+            :notification_rule_states => opts[:notification_rule_state_ids] || [],
           }
         )
       end

lib/flapjack/gateways/jsonapi/notification_rule_methods.rb

@@ -79,11 +79,13 @@ def self.registered(app)
 
             notification_rule_ids = notification_rules.map(&:id)
             linked_contact_ids = Flapjack::Data::NotificationRule.associated_ids_for_contact(*notification_rule_ids)
+            linked_tag_ids = Flapjack::Data::NotificationRule.associated_ids_for_tags(*notification_rule_ids)
             linked_notification_rule_states_ids = Flapjack::Data::NotificationRule.associated_ids_for_states(*notification_rule_ids)
 
             notification_rules_as_json = notification_rules.collect {|notification_rule|
               notification_rule.as_json(:contact_ids => [linked_contact_ids[notification_rule.id]],
-                                        :states_ids => linked_notification_rule_states_ids[notification_rule.id])
+                                        :tag_ids => linked_tag_ids,
+                                        :notification_rule_state_ids => linked_notification_rule_states_ids[notification_rule.id])
             }
 
             Flapjack.dump_json(:notification_rules => notification_rules_as_json)
@@ -101,18 +103,12 @@ def self.registered(app)
                   end
                 when 'add'
                   case linked
-                  when 'checks'
-                    check = Flapjack::Data::Check.find_by_id(value)
-                    notification_rule.checks << check unless check.nil?
                   when 'tags'
                     tag = Flapjack::Data::Tag.find_by_id(value)
                     notification_rule.tags << tag unless tag.nil?
                   end
                 when 'remove'
                   case linked
-                  when 'checks'
-                    check = Flapjack::Data::Check.find_by_id(value)
-                    notification_rule.delete(check) unless check.nil?
                   when 'tags'
                     tag = Flapjack::Data::Tag.find_by_id(value)
                     notification_rule.delete(tag) unless tag.nil?

lib/flapjack/processor.rb

@@ -232,7 +232,7 @@ def update_keys(event, check, timestamp)
         if previous_state.nil?
           @logger.info("No previous state for event #{event.id}")
 
-          if (@ncsm_duration > 0) && ((event.tags || []) & @ncsm_ignore_tags).empty?
+          if (@ncsm_duration > 0) && ((check.tags.all.map(&:name) || []) & @ncsm_ignore_tags).empty?
             @logger.info("Setting scheduled maintenance for #{time_period_in_words(@ncsm_duration)}")
 
             @ncsm_sched_maint = Flapjack::Data::ScheduledMaintenance.new(:start_time => timestamp,
@@ -318,13 +318,6 @@ def generate_notification(event, check, timestamp, previous_state)
 
       notification.save
 
-      unless event.tags.blank?
-        event_tags = Flapjack::Data::Tag.intersect(:name => event.tags)
-        notification.tags.add(*event_tags.all) unless event_tags.empty?
-      end
-
-      notification.tags.add(*check.tags.all) unless check.tags.empty?
-
       check.notifications << notification
       current_state.current_notifications << notification unless current_state.nil?
       previous_state.previous_notifications << notification unless previous_state.nil?

spec/lib/flapjack/data/event_spec.rb

@@ -18,8 +18,7 @@
                       'details'  => "couldn't access",
                       'perfdata' => "/=5504MB;5554;6348;0;7935",
                       'acknowledgement_id' => '1234',
-                      'duration' => (60 * 60),
-                      'tags'     => ['dev'] }
+                      'duration' => (60 * 60) }
                    }
 
   context 'class' do
@@ -99,7 +98,7 @@
     end
   end
 
-  ['time', 'details', 'perfdata', 'acknowledgement_id', 'duration', 'tags'].each do |optional_key|
+  ['time', 'details', 'perfdata', 'acknowledgement_id', 'duration'].each do |optional_key|
     it "rejects an event with invalid '#{optional_key}' key" do
       bad_event_data = event_data.clone
       bad_event_data[optional_key] = {'hello' => 'there'}