Commit 2d2b1f49 authored by Sam Wiskow's avatar Sam Wiskow
Browse files

feat(rate_limit): regex matchers in Rule#match 

Implements Spec 10 (gitlab-com/gl-infra/production-engineering#28855):
extends Labkit::RateLimit::Rule#match so callers can match on regex
patterns alongside the existing equality semantics. Unblocks Spec 9
(Stage 2b RackAttack migration, gitlab-com/gl-infra/production-engineering#28852).

Match-value shape:
  - any plain value -> :equality matcher (semantics unchanged)
  - { regex: "<source>" } single-key Hash -> :regex matcher (canonical,
    YAML-compatible)
  - bare Regexp instance -> :regex matcher (Ruby-side convenience,
    normalized internally)

Internals: a private Labkit::RateLimit::Matcher value object encapsulates
(type, value); Matcher.from is the single normalization entry point and
Matcher#match? the single matching entry point. Rule.new normalizes every
match value through Matcher.from at construction; Evaluator#rule_matches?
calls matcher.match?(identifier[key]).

Out of scope (per @reprazent review):
  - Glob matchers — { glob: "..." } is explicitly rejected at Rule.new
    with ArgumentError. Glob support is deferred to
    gitlab-com/gl-infra/production-engineering#28853 (config evolution).
  - Ruby-only typed-object constructors (e.g. Matcher.regex(...)) — push
    callers toward the YAML-compatible Hash form.

Behaviour notes:
  - Non-String identifier value vs. regex matcher: quiet skip (rule does
    not match, evaluation continues; no exception, no log).
  - Regex compilation happens once at Rule.new; counter-key derivation
    still uses the identifier value, never the pattern source.
  - Regex source strings are capped at 200 characters at Rule.new.

Specs cover Scenarios A–I from the spec acceptance criteria, including
Ruby<->YAML cross-format parity (Scenario I) and the explicit-rejection
guarantee for { glob: ... } and other unsupported shapes (Scenario H).

Co-Authored-By: default avatarClaude Opus 4.7 (1M context) <noreply@anthropic.com>
parent 96e48148

lib/labkit/rate_limit.rb

+1 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ module Labkit 
  module RateLimit

    autoload :Configuration, "labkit/rate_limit/configuration"

    autoload :Identifier, "labkit/rate_limit/identifier"

    autoload :Matcher, "labkit/rate_limit/matcher"

    autoload :Result, "labkit/rate_limit/result"

    autoload :Rule, "labkit/rate_limit/rule"

    autoload :Evaluator, "labkit/rate_limit/evaluator"

lib/labkit/rate_limit/evaluator.rb

+1 −1
Original line number Diff line number Diff line
@@ -41,7 +41,7 @@ module Labkit 
      end



      def rule_matches?(rule, identifier)

        rule.match.all? { |key, value| identifier[key] == value }

        rule.match.all? { |key, matcher| matcher.match?(identifier[key]) }

      end



      def evaluate_rule(rule, identifier)

lib/labkit/rate_limit/matcher.rb

0 → 100644
+85 −0
Original line number Diff line number Diff line 
# frozen_string_literal: true



module Labkit

  module RateLimit

    MATCHER_KNOWN_HASH_KEYS = %i[regex].freeze

    MATCHER_MAX_REGEX_SOURCE_LENGTH = 200



    # Matcher is the internal representation of a single key/value predicate in

    # a Rule#match hash. Rule.new normalizes every match value through

    # Matcher.from; the Evaluator calls Matcher#match? per identifier value.

    #

    # Accepted input shapes (everything else raises ArgumentError):

    #   - any plain value (String, Symbol, Integer, ...) -> :equality matcher

    #   - a Regexp instance                              -> :regex matcher (Ruby convenience)

    #   - { regex: "<source>" } single-key Hash          -> :regex matcher (canonical, YAML-compatible)

    #

    # See Spec 10 (gitlab-com/gl-infra/production-engineering#28855) for the

    # full contract; glob support is intentionally out of scope.

    # @api private

    Matcher = Data.define(:type, :value) do

      def self.from(input)

        case input

        when Regexp

          new(type: :regex, value: input)

        when Hash

          from_hash(input)

        when Array

          raise ArgumentError,

            "rate-limit match value must be a single-key Hash like {regex: \"...\"}, got #{input.inspect}"

        else

          new(type: :equality, value: input)

        end

      end



      def self.from_hash(input)

        if input.size != 1

          raise ArgumentError,

            "rate-limit match value must be a single-key Hash like {regex: \"...\"}, got #{input.inspect}"

        end



        type, source = input.first

        type_sym = type.to_sym



        unless MATCHER_KNOWN_HASH_KEYS.include?(type_sym)

          raise ArgumentError,

            "rate-limit match value has unknown type key #{type.inspect}; accepted: #{MATCHER_KNOWN_HASH_KEYS.inspect}"

        end



        compile(type_sym, source)

      end

      private_class_method :from_hash



      def self.compile(type_sym, source)

        case type_sym

        when :regex

          unless source.is_a?(String)

            raise ArgumentError,

              "rate-limit match value {regex: ...} requires a String source, got #{source.class}"

          end



          if source.length > MATCHER_MAX_REGEX_SOURCE_LENGTH

            raise ArgumentError,

              "rate-limit match value {regex: ...} source exceeds #{MATCHER_MAX_REGEX_SOURCE_LENGTH} characters"

          end



          begin

            new(type: :regex, value: Regexp.new(source))

          rescue RegexpError => e

            raise ArgumentError, "rate-limit match value {regex: #{source.inspect}} failed to compile: #{e.message}"

          end

        end

      end

      private_class_method :compile



      def match?(identifier_value)

        case type

        when :equality

          value == identifier_value

        when :regex

          identifier_value.is_a?(String) && value.match?(identifier_value)

        end

      end

    end

  end

end

lib/labkit/rate_limit/rule.rb

+1 −1
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ module Labkit 


        super(

          name: name_str.freeze,

          match: match.transform_keys(&:to_sym).freeze,

          match: match.transform_keys(&:to_sym).transform_values { |v| Matcher.from(v) }.freeze, # rubocop:disable CodeReuse/ActiveRecord

          limit: limit,

          period: period,

          action: action_sym,

spec/labkit/rate_limit/evaluator_spec.rb

+121 −0
Original line number Diff line number Diff line
@@ -168,4 +168,125 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      expect(result.matched?).to be(false)

    end

  end



  # Spec 10 acceptance criteria -- pattern matching for Rule#match

  # See gitlab-com/gl-infra/production-engineering#28855



  describe "Spec 10 Scenario A: equality semantics unchanged" do

    it "matches an :equality matcher and increments the counter" do

      rule = make_rule(name: "api_endpoint", match: { endpoint: "GET /api/v4/projects" }, characteristics: [:user])

      id = Labkit::RateLimit::Identifier.new(user: 42, endpoint: "GET /api/v4/projects")



      expect(redis).to receive(:incr).and_return(1)

      expect(redis).to receive(:expire)



      result = evaluator(rules: [rule]).check(id)

      expect(result.matched?).to be(true)

      expect(result.rule).to eq(rule)

    end

  end



  describe "Spec 10 Scenario B: regex matcher (canonical and Ruby-convenience forms)" do

    let(:id) { Labkit::RateLimit::Identifier.new(user: 42, path: "/api/v4/projects/42") }



    it "matches via { regex: \"...\" }" do

      rule = make_rule(name: "projects_api",

        match: { path: { regex: "^/api/v\\d+/projects" } },

        characteristics: [:user])

      expect(redis).to receive(:incr).and_return(1)

      expect(redis).to receive(:expire)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(true)

    end



    it "matches via a bare Regexp" do

      rule = make_rule(name: "projects_api",

        match: { path: %r{^/api/v\d+/projects} },

        characteristics: [:user])

      expect(redis).to receive(:incr).and_return(1)

      expect(redis).to receive(:expire)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(true)

    end

  end



  describe "Spec 10 Scenario C: regex non-match falls through" do

    it "does not match when the regex does not match the identifier value" do

      rule = make_rule(match: { path: { regex: "^/admin" } })

      id = Labkit::RateLimit::Identifier.new(user: 42, path: "/api/v4/projects")

      expect(redis).not_to receive(:incr)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(false)

    end

  end



  describe "Spec 10 Scenario D: AND across mixed equality + regex matchers" do

    let(:rule) do

      make_rule(name: "mixed",

        match: { request_type: "api", path: { regex: "^/api/v\\d+/projects" } },

        characteristics: [:user])

    end



    it "matches when both predicates pass" do

      id = Labkit::RateLimit::Identifier.new(user: 42, request_type: "api", path: "/api/v4/projects")

      expect(redis).to receive(:incr).and_return(1)

      expect(redis).to receive(:expire)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(true)

    end



    it "does not match when only the equality predicate passes" do

      id = Labkit::RateLimit::Identifier.new(user: 42, request_type: "api", path: "/admin")

      expect(redis).not_to receive(:incr)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(false)

    end



    it "does not match when only the regex predicate passes" do

      id = Labkit::RateLimit::Identifier.new(user: 42, request_type: "web", path: "/api/v4/projects")

      expect(redis).not_to receive(:incr)



      expect(evaluator(rules: [rule]).check(id).matched?).to be(false)

    end

  end



  describe "Spec 10 Scenario E: counter key uses the identifier value, not the regex source" do

    it "writes the identifier's path value into the compound key" do

      rule = make_rule(name: "projects_api",

        match: { path: { regex: "^/api/v\\d+/projects" } },

        characteristics: [:user, :path])

      id = Labkit::RateLimit::Identifier.new(user: 42, path: "/api/v4/projects")



      expect(redis).to receive(:incr)

        .with("labkit:rl:rack_request:projects_api:user:42:path:/api/v4/projects")

        .and_return(1)

      expect(redis).to receive(:expire)



      evaluator(rules: [rule]).check(id)

    end

  end



  describe "Spec 10 Scenario G: non-String identifier vs regex matcher (quiet skip)" do

    it "does not match and does not raise when the identifier value is an Integer" do

      rule = make_rule(match: { user: { regex: "42" } })

      id = Labkit::RateLimit::Identifier.new(user: 42)

      expect(redis).not_to receive(:incr)



      expect { evaluator(rules: [rule]).check(id) }.not_to raise_error

      expect(evaluator(rules: [rule]).check(id).matched?).to be(false)

    end



    it "falls through to the next rule when the regex matcher quiet-skips" do

      regex_rule = make_rule(name: "regex_first", match: { user: { regex: "42" } }, action: :block)

      eq_rule    = make_rule(name: "eq_second",   match: { user: 42 }, action: :log)

      id = Labkit::RateLimit::Identifier.new(user: 42)



      expect(redis).to receive(:incr).and_return(1)

      expect(redis).to receive(:expire)



      result = evaluator(rules: [regex_rule, eq_rule]).check(id)

      expect(result.matched?).to be(true)

      expect(result.rule).to eq(eq_rule)

    end

  end

end