V3: filter audit events: multiple timestamps

Users can filter audit events with several timestamps, e.g.

  `GET /v3/audit_events?created_ats=2020-06-30T12:01:05Z,2020-07-02T12:00:00Z`

-  We feel comfortable extending the `to_array!` of the base class to
   return nil when passed a hash (which means, "don't change this!"). Prior
   to the recent addition of inequality filters, we did not pass hashes,
   and now we do, and it's appropriate to accommodate that.

-  For our newer methods, we add another `9` to our timespan (i.e.
   `0.99999` (five nines) is increased to `0.999999` (six nines).
   PostgreSQL stores timestamps in microseconds, and there is an obscure
   corner case where the query will unexpected fail if the timestamp is
   barely just before the second begins. We should probably backfill
   this.

[finishes #173575312]

Co-authored-by: Reid Mitchell <rmitchell@pivotal.io>
Co-authored-by: Brian Cunnie <bcunnie@pivotal.io>

Author: reidmit

app/fetchers/event_list_fetcher.rb

@@ -37,25 +37,32 @@ def filter(message, dataset)
               normalized_timestamp = Time.parse(given_timestamp).utc
               dataset = dataset.where(Sequel.lit('created_at < ?', normalized_timestamp))
             elsif operator == Event::LESS_THAN_OR_EQUAL_COMPARATOR
-              normalized_timestamp = (Time.parse(given_timestamp).utc + 0.99999).utc
+              normalized_timestamp = (Time.parse(given_timestamp).utc + 0.999999).utc
               dataset = dataset.where(Sequel.lit('created_at <= ?', normalized_timestamp))
             elsif operator == Event::GREATER_THAN_COMPARATOR
-              normalized_timestamp = (Time.parse(given_timestamp).utc + 0.99999).utc
+              normalized_timestamp = (Time.parse(given_timestamp).utc + 0.999999).utc
               dataset = dataset.where(Sequel.lit('created_at > ?', normalized_timestamp))
             elsif operator == Event::GREATER_THAN_OR_EQUAL_COMPARATOR
               normalized_timestamp = Time.parse(given_timestamp).utc
               dataset = dataset.where(Sequel.lit('created_at >= ?', normalized_timestamp))
             end
           else
             # Gotcha: unlike the other relational operators, which are hashes such as
-            # { lt: '2020-06-30T12:34:56Z' }, the equals operator is simply a value, e.g.
-            # '2020-06-30T12:34:56Z'.
+            # { lt: '2020-06-30T12:34:56Z' }, the equals operator is simply an array, e.g.
+            # [ '2020-06-30T12:34:56Z' ].
             # Gotcha: the equals operator returns all resources occurring within
             # the span of the second (e.g. "12:34:56.00-12:34:56.9999999"), for databases store
             # timestamps in sub-second accuracy (PostgreSQL stores in microseconds, for example)
-            lower_bound = Time.parse(message.created_ats).utc
-            upper_bound = Time.at(lower_bound + 0.99999).utc
-            dataset = dataset.where(Sequel.lit('created_at BETWEEN ? AND ?', lower_bound, upper_bound))
+            sequel_query =
+              (['created_at BETWEEN ? AND ?'] * message.created_ats.size).join(' OR ')
+
+            times = message.created_ats.map do |created_at|
+              lower_bound = Time.parse(created_at).utc
+              upper_bound = Time.at(lower_bound + 0.999999).utc
+              [lower_bound, upper_bound]
+            end.flatten
+
+            dataset = dataset.where(Sequel.lit(sequel_query, *times))
           end
         end
 

app/messages/base_message.rb

@@ -79,6 +79,7 @@ def self.from_params(params, to_array_keys, fields: [])
 
     def self.to_array!(params, key)
       return if params[key].nil?
+      return if params[key].is_a?(Hash)
 
       params[key] = if params[key] == ''
                       ['']

app/messages/events_list_message.rb

@@ -5,37 +5,41 @@ class EventsListMessage < ListMessage
     class CreatedAtValidator < ActiveModel::Validator
       def validate(record)
         if record.requested?(:created_ats)
-          if record.created_ats.is_a?(String)
-            timestamp = record.created_ats
+          if record.created_ats.is_a?(Array)
+            record.created_ats.each do |timestamp|
+              opinionated_iso_8601(timestamp, record)
+            end
           else
             unless record.created_ats.is_a?(Hash)
               record.errors[:created_ats] << 'comparison operator and timestamp must be specified'
               return
             end
 
-            comparison_operator = record.created_ats.keys[0]
             valid_comparision_operators = [
               Event::LESS_THAN_COMPARATOR,
               Event::GREATER_THAN_COMPARATOR,
               Event::LESS_THAN_OR_EQUAL_COMPARATOR,
               Event::GREATER_THAN_OR_EQUAL_COMPARATOR,
             ]
-            unless valid_comparision_operators.include?(comparison_operator)
-              record.errors[:created_ats] << "Invalid comparison operator: '#{comparison_operator}'"
-            end
 
-            timestamp = record.created_ats.values[0]
-          end
-          begin
-            raise ArgumentError.new('invalid date') unless timestamp =~ /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\Z/
+            record.created_ats.each do |comparison_operator, timestamp|
+              unless valid_comparision_operators.include?(comparison_operator)
+                record.errors[:created_ats] << "Invalid comparison operator: '#{comparison_operator}'"
+              end
 
-            Time.iso8601(timestamp)
-          rescue
-            record.errors[:created_ats] << "has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'"
-            return
+              opinionated_iso_8601(timestamp, record)
+            end
           end
         end
       end
+
+      private
+
+      def opinionated_iso_8601(timestamp, record)
+        if timestamp !~ /\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z\Z/
+          record.errors[:created_ats] << "has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'"
+        end
+      end
     end
 
     register_allowed_keys [
@@ -55,7 +59,7 @@ def validate(record)
     validates :organization_guids, array: true, allow_nil: true
 
     def self.from_params(params)
-      super(params, %w(types target_guids space_guids organization_guids))
+      super(params, %w(types target_guids space_guids organization_guids created_ats))
     end
   end
 end

docs/v3/source/includes/concepts/_filters.md.erb

@@ -15,6 +15,12 @@ This will return all apps with name `the_name`.
 
 This will return all apps with name `the_name` OR `second_name`.
 
+In the case of audit events, multiple timestamps can be requested, which will return all audit
+events that occurred at those timestamps. In the following request, all audit events that occurred
+New Year's just before midnight and July 4th at noon will be returned:
+
+`GET /v3/audit_events?created_ats=2019-12-31T23:59:59Z,2020-07-04T12:00:00Z`
+
 ###### **Exception**
 The `label_selector` query parameter will act as AND function, not an OR.
 

docs/v3/source/includes/resources/audit_events/_list.md.erb

@@ -30,7 +30,7 @@ Retrieve all audit events the user has access to.
 
 Name | Type | Description
 ---- | ---- | ------------
-**created_ats** (*experimental*)| _[timestamp](#timestamps)_ | Timestamp to filter by. Only supports relational operators, see [relational operators](#relational-operators-experimental)
+**created_ats** (*experimental*)| _[timestamp](#timestamps)_ | Timestamp to filter by. When filtering on equality, several comma-delimited timestamps may be passed. Also supports filtering with [relational operators](#relational-operators-experimental)
 **types** | _list of strings_ | Comma-delimited list of event types to filter by
 **target_guids** | _list of strings_ | Comma-delimited list of target guids to filter by
 **space_guids** | _list of strings_ | Comma-delimited list of space guids to filter by

spec/request/events_spec.rb

@@ -343,11 +343,12 @@
         it 'returns events at the given timestamp' do
           get "/v3/audit_events?created_ats=#{timestamp}", nil, admin_header
 
+          expect(last_response).to have_status_code(200)
           expect(
             resources: parsed_response['resources']
           ).to match_json_response(
             resources: [same_time_event_json]
-               )
+          )
         end
       end
 

spec/unit/fetchers/event_list_fetcher_spec.rb

@@ -148,7 +148,7 @@ module VCAP::CloudController
 
         context 'requesting events equal to a timestamp' do
           let(:filters) do
-            { created_ats: event_3.created_at.iso8601 }
+            { created_ats: [event_3.created_at.iso8601] }
           end
 
           it 'returns events with a created_at timestamp at or after a given timestamp' do
@@ -163,6 +163,25 @@ module VCAP::CloudController
             end
           end
         end
+
+        context 'requesting events equal to several timestamps' do
+          let!(:event_5) { Event.make(guid: '5', created_at: '2020-05-26T18:47:05Z') }
+          let(:filters) do
+            { created_ats: [event_2.created_at.iso8601, event_4.created_at.iso8601] }
+          end
+
+          it 'returns events with a created_at timestamp at or after a given timestamp' do
+            expect(subject).to match_array([event_2, event_4])
+          end
+
+          context 'when there are events with subsecond timestamps' do
+            let!(:event_between_2_and_3) { Event.make(guid: '2.5', created_at: '2020-05-26T18:47:02.5Z') }
+
+            it 'returns events with a created_at timestamp at a given timestamp' do
+              expect(subject).to match_array([event_2, event_between_2_and_3, event_4])
+            end
+          end
+        end
       end
     end
   end

spec/unit/messages/base_message_spec.rb

@@ -132,6 +132,7 @@ class ParamsClass < BaseMessage
           string_field: 'stringval&',
           nil_field: nil,
           empty_field: '',
+          hash_not_array: { pi: 3.141592653589792 }
         }
       end
 
@@ -166,6 +167,10 @@ class ParamsClass < BaseMessage
       it 'handles empty values' do
         expect(BaseMessage.to_array!(params, :empty_field)).to eq([''])
       end
+
+      it "doesn't try to convert a hash to an array" do
+        expect(BaseMessage.to_array!(params, :hash_not_array)).to eq(nil)
+      end
     end
 
     describe 'additional keys validation' do

spec/unit/messages/events_list_message_spec.rb

@@ -65,8 +65,8 @@ module VCAP::CloudController
         end
 
         context 'validates the created_ats filter' do
-          it 'requires a hash or a timestamp' do
-            message = EventsListMessage.from_params({ created_ats: [Time.now.utc.iso8601] })
+          it 'requires a hash or an array of timestamps' do
+            message = EventsListMessage.from_params({ created_ats: 47 })
             expect(message).not_to be_valid
             expect(message.errors[:created_ats]).to include('comparison operator and timestamp must be specified')
           end
@@ -78,23 +78,36 @@ module VCAP::CloudController
           end
 
           context 'requires a valid timestamp' do
+            it "won't accept a malformed timestamp" do
+              message = EventsListMessage.from_params({ 'created_ats' => "#{Time.now.utc.iso8601},bogus" })
+              expect(message).not_to be_valid
+              expect(message.errors[:created_ats]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
+            end
+
             it "won't accept garbage" do
               message = EventsListMessage.from_params({ created_ats: { gt: 123 } })
               expect(message).not_to be_valid
               expect(message.errors[:created_ats]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
             end
+
             it "won't accept fractional seconds even though it's ISO 8601-compliant" do
               message = EventsListMessage.from_params({ created_ats: { gt: '2020-06-30T12:34:56.78Z' } })
               expect(message).not_to be_valid
               expect(message.errors[:created_ats]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
             end
+
             it "won't accept local time zones even though it's ISO 8601-compliant" do
               message = EventsListMessage.from_params({ created_ats: { gt: '2020-06-30T12:34:56.78-0700' } })
               expect(message).not_to be_valid
               expect(message.errors[:created_ats]).to include("has an invalid timestamp format. Timestamps should be formatted as 'YYYY-MM-DDThh:mm:ssZ'")
             end
           end
 
+          it 'allows comma-separated timestamps' do
+            message = EventsListMessage.from_params({ 'created_ats' => "#{Time.now.utc.iso8601},#{Time.now.utc.iso8601}" })
+            expect(message).to be_valid
+          end
+
           it 'allows the lt operator' do
             message = EventsListMessage.from_params({ created_ats: { lt: Time.now.utc.iso8601 } })
             expect(message).to be_valid
@@ -117,14 +130,9 @@ module VCAP::CloudController
 
           context 'when the operator is an equals operator' do
             it 'allows the equals operator' do
-              message = EventsListMessage.from_params({ created_ats: Time.now.utc.iso8601 })
+              message = EventsListMessage.from_params({ 'created_ats' => Time.now.utc.iso8601 })
               expect(message).to be_valid
             end
-
-            it 'errors on invalid (non-ISO 8601) timestamps' do
-              message = EventsListMessage.from_params({ created_ats: 'yesterday' })
-              expect(message).to be_invalid
-            end
           end
         end
       end