Commit 5faffe07 authored by Sankalp's avatar Sankalp
Browse files

feat: add oneOf set-membership matcher to rate-limit rules

GitLab's rack rate limiting needs one skip rule matching the allowlisted
user IDs from GITLAB_THROTTLE_USER_ALLOWLIST. A regex union of IDs would
break the 200-char cap on the re string form, and per-ID eq rules
multiply with the allowlist size. oneOf matches when the identifier
value equals any array element, compared without coercion like eq.

The name follows the runbooks PromQL selector convention (oneOf in
selectors.libsonnet) rather than in, which is a keyword in other
languages labkit targets.

See gitlab-com/gl-infra/production-engineering#29320
parent af614963
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -174,10 +174,19 @@ A `match` hash gates whether a rule applies. Each value is normalised through
| `Regexp`          | `re`     | `match: { endpoint: %r{\A/api/} }`                     |
| `{ eq: <value> }` | `eq`     | `match: { method: { eq: "POST" } }` (YAML-friendly)    |
| `{ re: <source> }`| `re`     | `match: { endpoint: { re: '\A/api/' } }` (YAML-friendly) |
| `{ oneOf: <Array> }` | `oneOf` | `match: { user: { oneOf: ["7", "42"] } }` (YAML-friendly) |

`re` coerces the identifier value via `#to_s` before matching, so it can be
used against non-String values (e.g. matching a 503 status against `{ re: '^5' }`).

`oneOf` is set membership via `Set#include?` (`eql?`/`hash` equality, no
coercion): it agrees with `eq` for Strings, Symbols, booleans, nil, and
same-class numerics, but cross-type numerics differ (`1 == 1.0`, yet
`{ oneOf: [1] }` does not match `1.0`). The name follows the runbooks' PromQL
selector convention (`selectors.libsonnet`), avoiding the `in` keyword in
languages labkit targets (Python). A bare Array is rejected; membership must
be spelled `{ oneOf: [...] }` explicitly.

Glob and prefix matchers are intentionally out of scope.

### Evaluation flow
+28 −7
Original line number Diff line number Diff line
# frozen_string_literal: true

require "set"

module Labkit
  module RateLimit
    # Matcher is the internal representation of a single key/value predicate in
@@ -11,6 +13,14 @@ module Labkit
    #   - a Regexp instance                              -> :re matcher (Ruby convenience)
    #   - { eq: <value> }                                -> :eq matcher (canonical, YAML-compatible)
    #   - { re: <String|Regexp> }                        -> :re matcher (canonical, YAML-compatible)
    #   - { oneOf: <Array> }                             -> :oneOf matcher (set membership, YAML-compatible)
    #
    # A bare Array is rejected rather than treated as :oneOf, so a value that
    # was meant as a single matcher hash but arrived as an Array fails loudly
    # instead of silently becoming a membership test.
    #
    # :oneOf membership is Set#include? (eql?/hash equality), not :eq's ==, so
    # cross-type numerics differ (1 == 1.0, but {oneOf: [1]} does not match 1.0).
    #
    # Hash-key naming follows the metrics-catalog selector pattern. Glob,
    # prefix, and other matcher kinds are intentionally out of scope here; see
@@ -20,7 +30,7 @@ module Labkit
    # the regex, so callers can match non-String identifier values such as
    # Integer status codes (e.g. {status: { re: "^5" }} against status: 503).
    class Matcher < Data.define(:type, :value)
      KNOWN_HASH_KEYS = %i[eq re].freeze
      KNOWN_HASH_KEYS = %i[eq re oneOf].freeze
      MAX_REGEX_SOURCE_LENGTH = 200
      ERROR_INSPECT_LIMIT = 80

@@ -42,18 +52,14 @@ module Labkit
        when Hash
          from_hash(input)
        when Array
          raise ArgumentError,
            "rate-limit match value must be a single-key Hash like {re: \"...\"} or {eq: ...}, got #{truncate_for_error(input)}"
          raise ArgumentError, invalid_shape_error(input)
        else
          new(type: :eq, 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 {re: \"...\"} or {eq: ...}, got #{truncate_for_error(input)}"
        end
        raise ArgumentError, invalid_shape_error(input) if input.size != 1

        type, source = input.first
        type_sym = type.to_sym
@@ -71,6 +77,13 @@ module Labkit
        case type_sym
        when :eq
          new(type: :eq, value: source)
        when :oneOf
          unless source.is_a?(Array)
            raise ArgumentError,
              "rate-limit match value {oneOf: ...} must be an Array, got #{truncate_for_error(source)}"
          end

          new(type: :oneOf, value: Set.new(source).freeze)
        when :re
          if source.to_s.length > MAX_REGEX_SOURCE_LENGTH
            raise ArgumentError,
@@ -113,6 +126,12 @@ module Labkit

      private_class_method :with_match_timeout

      def self.invalid_shape_error(input)
        "rate-limit match value must be a single-key Hash like {re: \"...\"}, {eq: ...} or {oneOf: [...]}, " \
          "got #{truncate_for_error(input)}"
      end
      private_class_method :invalid_shape_error

      def self.truncate_for_error(value)
        s = value.inspect
        s.length > ERROR_INSPECT_LIMIT ? "#{s[0, ERROR_INSPECT_LIMIT]}...(truncated)" : s
@@ -123,6 +142,8 @@ module Labkit
        case type
        when :eq
          value == identifier_value
        when :oneOf
          value.include?(identifier_value)
        when :re
          value.match?(identifier_value.to_s)
        else
+59 −0
Original line number Diff line number Diff line
@@ -137,6 +137,36 @@ RSpec.describe Labkit::RateLimit::Matcher do
      end
    end

    context "with the canonical { oneOf: [...] } shape" do
      it "builds a :oneOf matcher holding the members as a Set" do
        m = described_class.build(oneOf: %w[7 42])
        expect(m.type).to eq(:oneOf)
        expect(m.value).to eq(Set.new(%w[7 42]))
        expect(m.value).to be_frozen
      end

      it "accepts a string-keyed hash too (mirrors YAML.safe_load output)" do
        m = described_class.build("oneOf" => %w[7 42])
        expect(m).to eq(described_class.build(oneOf: %w[7 42]))
      end

      it "accepts an empty Array (matches nothing)" do
        m = described_class.build(oneOf: [])
        expect(m.match?("7")).to be(false)
        expect(m.match?(nil)).to be(false)
      end

      it "raises ArgumentError when the source is not an Array" do
        expect { described_class.build(oneOf: "7,42") }
          .to raise_error(ArgumentError, /must be an Array/)
      end

      it "raises ArgumentError when the source is nil" do
        expect { described_class.build(oneOf: nil) }
          .to raise_error(ArgumentError, /must be an Array/)
      end
    end

    context "with unsupported shapes" do
      it "rejects an Array" do
        expect { described_class.build(["/a", "/b"]) }
@@ -261,6 +291,35 @@ RSpec.describe Labkit::RateLimit::Matcher do
      end
    end

    context "with a :oneOf matcher" do
      let(:matcher) { described_class.build(oneOf: ["7", "42", 99]) }

      it "returns true for a member value" do
        expect(matcher.match?("7")).to be(true)
        expect(matcher.match?(99)).to be(true)
      end

      it "returns false for a non-member value" do
        expect(matcher.match?("8")).to be(false)
      end

      it "compares without coercion" do
        # 99 is a member as an Integer; the String "99" is a different value.
        expect(matcher.match?("99")).to be(false)
        expect(matcher.match?(7)).to be(false)
      end

      it "uses eql?/hash equality, so cross-type numerics do not match (unlike :eq's ==)" do
        expect(described_class.build(eq: 1).match?(1.0)).to be(true)
        expect(described_class.build(oneOf: [1]).match?(1.0)).to be(false)
      end

      it "returns false for nil unless nil is a member" do
        expect(matcher.match?(nil)).to be(false)
        expect(described_class.build(oneOf: [nil]).match?(nil)).to be(true)
      end
    end

    context "with a :re matcher built from a bare Regexp" do
      it "matches identically to the canonical { re: \"...\" } form" do
        from_hash = described_class.build(re: "^/api/v\\d+/projects")