Add IPV4_PREFIX_BITS and IPV6_PREFIX_BITS options to keyed_by_ip

[adityachopra29]

• Solves For keyed_by_ip quotas, add a ipv4_prefix_bits and ipv6_prefix_bits options. #79858
• Enables support for ipv4 and ipv6 networks by creating prefix bits support when quotas using keyed_by_ip

CREATE QUOTA q_1 KEYED BY IP_ADDRESS IPV4_PREFIX_BITS 24 IPV6_PREFIX_BITS 64 

or

CREATE QUOTA q_2 KEYED BY FORWARDED_IP_ADDRESS IPV4_PREFIX_BITS 24 IPV6_PREFIX_BITS 64 

Then, if q_2 is queried by, say a user with forwarded_ip_address = '1.2.3.4' first, then the quota_key generated :

curl -H 'X-Forwarded-For: 1.2.3.4' sS 'http://localhost:8123?user=test_user' -d "
SELECT quota_key
FROM system.quota_usage
WHERE quota_name = 'q_2'
FORMAT TSV;
"

will give 1.2.3.0

Changelog category (leave one):

• New Feature

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

• Adds IPV4_PREFIX_BITS and IPV6_PREFIX_BITS flags when Keying done by IP_ADDRESS or FORWARDED_IP_ADDRESS

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

---

Note

Medium Risk
Changes quota key derivation and SQL parsing/serialization for IP- and forwarded-IP-keyed quotas, which can alter how limits are shared/enforced across clients/subnets. Includes new system table columns and validation logic; regressions could affect quota correctness but scope is contained to quota handling.

Overview
Adds optional IPV4_PREFIX_BITS/IPV6_PREFIX_BITS settings to quotas keyed by ip_address or forwarded_ip_address, allowing quota buckets to be shared by masked subnet instead of full IP.

Extends the Quota entity, SQL parser/formatter (CREATE/ALTER/SHOW CREATE incl. ON CLUSTER), users.xml quota parsing, and system.quotas (new nullable columns) to persist/expose these settings, and updates QuotaCache key calculation to apply masking (with a fast path and safe fallback for malformed X-Forwarded-For). Adds validation to reject prefix bits on non-IP key types and clears stale prefix bits when key type changes.

Updates docs and tests, including new stateless coverage for subnet-masked quota keys and adjusted integration queries to avoid SELECT * breakage from the new system.quotas columns.

^{Reviewed by Cursor Bugbot for commit b193898. Bugbot is set up for automated code reviews on this repo. Configure here.}

[CLAassistant]

All committers have signed the CLA.

[clickhouse-gh]

Workflow [PR], commit [5a7dad2]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR adds optional IPV4_PREFIX_BITS and IPV6_PREFIX_BITS support for quotas keyed by ip_address and forwarded_ip_address, covering SQL parsing/formatting, users.xml, runtime quota-key masking, SHOW CREATE, system.quotas, docs, and focused tests. The current code addresses the prior review concerns I rechecked, including parser order, non-integer rejection, /0 masking, stale prefix bits on non-IP key changes, XML parity, forwarded IPv6 and IPv4-mapped behavior, ON CLUSTER formatting, and system.quotas column ordering.

Missing context / blind spots

• ⚠️ I did not run the new stateless or integration tests locally. GitHub checks still show Fast test, Build (arm_tidy), and the aggregate PR report pending; a completed green CI run would close this.

Final Verdict

Status: ✅ Approve

[adityachopra29]

Hi @alexey-milovidov, any update on this pr's review? I was not able to clear the formatting issues, if any, since there are already a lot of changes that arise besides my additions when I run clang-format. Please tell if any other changes are required.

[adityachopra29]

Hi @alexey-milovidov, Is there any update on this PR? Please advise; I am keen to contribute further in ClickHouse. I will appreciate any feedback to merge this pr.

[alexey-milovidov]

It didn't pass the style check.

[pufit]

Nice feature — subnet-level quota keying is a solid addition. The core implementation (parser, serialization round-trip, system table, masking logic) looks good. Found a few issues though, one of which is a real semantic bug.

Bug: Prefix bits not cleared when key_type changes

InterpreterCreateQuotaQuery.cpp only sets prefix bits when the query specifies them — it never clears them:

if (query.ipv4_prefix_bits)
    quota.ipv4_prefix_bits = query.ipv4_prefix_bits;

This means changing key_type away from IP preserves stale bits:

CREATE QUOTA q KEYED BY ip_address IPV4_PREFIX_BITS 24 IPV6_PREFIX_BITS 64;
ALTER QUOTA q KEYED BY client_key;
-- SHOW CREATE hides the bits (formatter only shows them for IP key types)
-- But system.quotas still shows ipv4_prefix_bits=24, ipv6_prefix_bits=64

ALTER QUOTA q KEYED BY ip_address;  -- switch back, no bits specified
-- SHOW CREATE now shows IPV4_PREFIX_BITS 24 IPV6_PREFIX_BITS 64 — ghost values resurrected

The bits silently survive a round-trip through a non-IP key type. Fix: when query.key_type is set and is not IP_ADDRESS/FORWARDED_IP_ADDRESS, explicitly reset both:

if (query.key_type)
{
    quota.key_type = *query.key_type;
    if (*query.key_type != QuotaKeyType::IP_ADDRESS
        && *query.key_type != QuotaKeyType::FORWARDED_IP_ADDRESS)
    {
        quota.ipv4_prefix_bits.reset();
        quota.ipv6_prefix_bits.reset();
    }
}

Same issue in the parser: ALTER QUOTA q IPV4_PREFIX_BITS 16 (without KEYED BY) succeeds even if the quota is keyed by user_name. The bits get persisted on an incompatible key type — system.quotas shows them, SHOW CREATE hides them.

Minor: IPV4_PREFIX_BITS 0 is a no-op at runtime

QuotaCache.cpp has:

if (quota->ipv4_prefix_bits && *quota->ipv4_prefix_bits > 0)

So IPV4_PREFIX_BITS 0 is accepted, persisted, displayed in SHOW CREATE — but silently does nothing. A /0 prefix means "all IPs share one bucket" which is valid behavior. Either support it (remove the > 0 check) or reject it in the parser.

Tested all of the above against the arm_release CI build (bb26c10b).

[pufit]

Generated by Nerve

[adityachopra29]

Hi, I will look into the errors and complete this pr shortly.

[clickhouse-gh]

Dear @pufit, you haven't been active on this PR for 30 days. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself.

[adityachopra29]

The failing test seems to be a borderline flaky-test, and looks like a job-timeout artifact.....rerunning the CI should fix this.

[alexey-milovidov]

Merged the latest master into the branch (clean, no conflicts) to refresh the base and clear the chronic Server died CI flake on a fresh build.

I rebuilt locally on the merged base and verified the three affected tests against a fresh binary:

• 01297_create_quota — matches the reference for all prefix-bits behavior, including the stale-bits-cleared fix (KEYED BY client_key then back to ip_address keeps ipv4/ipv6_prefix_bits as NULL), the /0 prefix, and the rejection cases. (The single ON CLUSTER line could only be skipped in my minimal local config because it has no ZooKeeper — it passes in CI.)
• 01600_quota_by_prefix_forwarded_ip — full match, exercising the runtime subnet masking over HTTP with X-Forwarded-For.
• 02117_show_create_table_system — the new ipv4_prefix_bits/ipv6_prefix_bits columns on system.quotas match.

All 15 review threads are resolved. The only CI failures are Server died, a chronic master-wide flake unrelated to this quota-only change (see #103235).

[alexey-milovidov]

Continued work on this PR:

• Addressed the remaining unresolved review thread (mask_address in QuotaCache.cpp): an IPv4-mapped IPv6 address (e.g. ::ffff:192.0.2.10) was being normalized to native IPv4 before checking whether IPV4_PREFIX_BITS was configured, which silently changed the quota key for existing IP-keyed quotas that have no prefix bits. It is now treated as an IPv4 client governed solely by IPV4_PREFIX_BITS: normalized and masked only when that prefix is set, otherwise kept as-is (so IPV6_PREFIX_BITS never collapses it). Added a 01600_quota_by_prefix_forwarded_ip regression for the no-IPV4_PREFIX_BITS case.
• Merged master (the branch was ~695 commits behind with an 02117_show_create_table_system.reference conflict; resolved cleanly).

Verified locally against a server built from this branch:

• 01600_quota_by_prefix_forwarded_ip passes, including the new mapped-address case.
• ALTER QUOTA q IPV4_PREFIX_BITS 16 without KEYED BY round-trips through SHOW CREATE (the key_type == std::nullopt formatImpl branch).
• Prefix bits are cleared when key_type changes away from IP (no ghost values resurrected).
• Prefix bits on a non-IP key type are rejected with BAD_ARGUMENTS; IPV4_PREFIX_BITS 33 is rejected with SYNTAX_ERROR.
• The ON CLUSTER part of 01297_create_quota requires ZooKeeper and is covered by CI.

Pushed bb91e9e9777 (fix + test) and the master merge.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.50%	84.50%	+0.00%
Functions	92.30%	92.30%	+0.00%
Branches	77.20%	77.20%	+0.00%

Changed lines: Changed C/C++ lines covered by tests: 233/244 (95.49%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 2 line(s) · Uncovered code

Full report · Diff report

[alexey-milovidov]

@groeneai, only after I merge this PR, send another PR that will edit 'Ip' to 'IP' everywhere according to our code style.