Conversation
📝 WalkthroughWalkthroughAdds ADR 0011 for chunked Kubernetes router configuration. The proposal keeps small configurations inline and stores larger configurations as deterministic, content-addressed chunks with a digest-based head document. ChangesChunked router configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Mergeability Score: 🟡 Moderate · up to The proposed configuration design still leaves unresolved risks around chunk deletion, size limits, lookup behavior, and deterministic output, which could cause configuration loss, publication failures, or inconsistent router configuration. The ADR is not ready to merge until these bounded correctness and implementation-contract issues are addressed. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: Christian Kruse <christian@c-kruse.com>
fefddb9 to
691e22e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6000947a-156a-4c3b-82ca-69d4e1b32c75
📒 Files selected for processing (1)
doc/adr/0011-chunked-router-configuration.md
| * Configuration is decomposed into keyed records (one per router entity). | ||
| * Records are partitioned deterministically into chunks by hashing their keys; | ||
| chunk count is derived from total size. | ||
| * Chunks are immutable (`ConfigMap.immutable: true`) and content-addressed: named and | ||
| referenced by the digest of their content. | ||
| * The head lists the digests of the complete chunk set. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- ADR outline ---'
ast-grep outline doc/adr/0011-chunked-router-configuration.md 2>/dev/null || true
printf '%s\n' '--- ADR lines 1-111 ---'
cat -n doc/adr/0011-chunked-router-configuration.md | sed -n '1,111p'
printf '%s\n' '--- ConfigMap-related implementation and tests ---'
rg -n -C 3 'ConfigMap|immutable|chunk|digest|1 ?MiB|1048576|config.*size|size.*config' internal doc test tests 2>/dev/null | sed -n '1,260p'Repository: skupperproject/skupper
Length of output: 28693
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Chunking and router-configuration design references ---'
rg -n -C 4 'partitioned|hashing|chunk count|chunk size|head document|chunk ConfigMap|skrouterd.json|TransportConfigFile|1MiB|1 MiB|1048576' . \
-g '!vendor/**' -g '!node_modules/**' -g '!dist/**' | sed -n '1,320p'
printf '%s\n' '--- Deterministic counterexample: count-based partitioning ---'
python3 - <<'PY'
import hashlib
limit = 1024 * 1024
records = {
"small-a": b"x" * 100,
"small-b": b"x" * 100,
"large": b"x" * (limit + 1),
}
total = sum(len(v) for v in records.values())
chunk_count = max(1, (total + limit - 1) // limit)
buckets = [[] for _ in range(chunk_count)]
for key, value in records.items():
bucket = int.from_bytes(hashlib.sha256(key.encode()).digest(), "big") % chunk_count
buckets[bucket].append((key, len(value)))
print({"total_bytes": total, "chunk_count": chunk_count, "buckets": buckets})
print("oversized_bucket_possible:", any(sum(size for _, size in b) > limit for b in buckets))
PYRepository: skupperproject/skupper
Length of output: 22620
🌐 Web query:
Kubernetes ConfigMap maximum size 1 MiB data field official documentation
💡 Result:
The official Kubernetes documentation explicitly states that a ConfigMap is not designed to hold large chunks of data and that the data stored in a ConfigMap cannot exceed 1 MiB [1][2]. This size limit applies to the entire object, including its metadata and all contained key-value pairs [3]. This constraint exists primarily because ConfigMaps are stored in etcd, the backing key-value store for Kubernetes, which has historically enforced size limits to ensure performance and stability [4][3]. If you need to store configuration data exceeding this 1 MiB limit, the official documentation recommends alternative approaches such as mounting a volume, using a separate database, or utilizing an external file service [1][2].
Citations:
- 1: https://kubernetes.io/docs/concepts/configuration/configmap/
- 2: https://kubernetes.website.cncfstack.com/docs/concepts/configuration/configmap/
- 3: https://spacelift.io/blog/kubernetes-configmap
- 4: Size limit for ConfigMap kubernetes/kubernetes#19781
Bound each chunk and the head by encoded size.
Hashing records into a count derived from total size does not bound any bucket. A large record can exceed Kubernetes’ 1 MiB ConfigMap limit. The head’s digest list can also exceed the limit as the chunk count grows. Size each serialized document with envelope overhead and margin, and define handling for records that cannot fit. Otherwise publication can fail, and the claim that transport no longer bounds configuration size is incorrect.
Source: MCP tools
| **Publishing is ordered for atomicity.** The controller creates any missing | ||
| chunks first, then updates the head in a single object write. The head update | ||
| is the atomic boundary: a reader that follows head → chunks always assembles | ||
| one consistent generation. Readers treat a missing chunk (deleted under them) | ||
| as a signal to re-read the head and retry. | ||
|
|
||
| **Unreferenced chunks are garbage-collected** after a grace period. | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Protect pending and cross-head chunk references during garbage collection.
The publisher creates chunks before the head references them. A concurrent collector can classify a pending chunk as unreferenced and delete it before the head update. If digest-only names allow sharing, the collector must also scan every router-group head, including skupper-router and skupper-router-2. Add a pending-publication lease or marker and re-check all heads immediately before deletion.
| **Serialization becomes deterministic.** One logical configuration has exactly | ||
| one byte representation: Records are serialized as compact JSON in a fixed | ||
| order by key. Readers merge records from all chunks and apply this ordering, so | ||
| the inline and chunked representations produce identical skrouterd.json bytes. | ||
| Deterministic chunk serialization ensures logically identical chunk content has | ||
| the same digest. | ||
|
|
||
| Config-init and the kube-adaptor understand both representations; their | ||
| downstream behavior (complete config file, ordered AMQP reconciliation) is |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- ADR ---'
cat -n doc/adr/0011-chunked-router-configuration.md | sed -n '1,115p'
printf '%s\n' '--- candidate files ---'
fd -t f . | rg '(^|/)(config_init|config_sync|.*router.*config|.*chunk.*|.*config.*test).*'
printf '%s\n' '--- symbols and references ---'
rg -n -C 4 'MarshalRouterConfig|skrouterd\.json|SslProfiles|ProxyProfiles|RouterConfig|chunk|Chunk' internal doc --glob '*.{go,md}' | sed -n '1,260p'
printf '%s\n' '--- outlines ---'
for f in $(fd -t f . internal | rg 'config_init\.go$|config_sync\.go$|.*config.*_test\.go$'); do
echo "### $f"
ast-grep outline "$f" 2>/dev/null || true
doneRepository: skupperproject/skupper
Length of output: 29973
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- config_init.go ---'
cat -n internal/kube/adaptor/config_init.go
printf '%s\n' '--- serializer definitions and calls ---'
rg -n -C 6 'func .*MarshalRouterConfig|MarshalRouterConfig|type RouterConfig struct|RouterConfig struct|json\.Marshal|Marshal.*Config' . --glob '!vendor/**' --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | sed -n '1,320p'
printf '%s\n' '--- RouterConfig files ---'
fd -t f . | rg '(^|/)(router|.*qdr.*|.*config.*)\.(go|json|yaml|yml)$' | sed -n '1,220p'Repository: skupperproject/skupper
Length of output: 25694
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- RouterConfig definition ---'
cat -n internal/qdr/qdr.go | sed -n '1,180p'
printf '%s\n' '--- ConfigMap parsing and serialization ---'
cat -n internal/qdr/qdr.go | sed -n '780,1010p'
printf '%s\n' '--- serializer tests ---'
cat -n internal/qdr/qdr_test.go | sed -n '180,340p'
cat -n internal/qdr/qdr_test.go | sed -n '440,490p'
cat -n internal/qdr/qdr_test.go | sed -n '640,700p'
printf '%s\n' '--- relevant type definitions ---'
rg -n -C 3 'type (RouterMetadata|SslProfile|ProxyProfile|Listener|Connector|Address|RouterAccess|SiteConfig|RouterLogging|LogConfig) struct|json:".*omitempty|map\[string\]' internal/qdr/qdr.go | sed -n '1,360p'Repository: skupperproject/skupper
Length of output: 35934
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("internal/qdr/qdr.go").read_text()
marshal = source[source.index("func MarshalRouterConfig"):source.index("func AsConfigMapData")]
map_ranges = re.findall(r'for\s+_,\s+e\s*:=\s*range\s+([A-Za-z0-9_.]+)\s*\{', marshal)
records = {
"SslProfiles": [["sslProfile", {"name": "a"}], ["sslProfile", {"name": "b"}]],
"ProxyProfiles": [["proxyProfile", {"name": "a"}], ["proxyProfile", {"name": "b"}]],
}
def assembled(order):
return '[\n' + ',\n'.join(f' ["{kind}", {value}]' for kind, value in order) + '\n]'
forward = assembled(records["SslProfiles"])
reverse = assembled(list(reversed(records["SslProfiles"])))
print("MarshalRouterConfig map ranges:", ", ".join(map_ranges))
print("uses json.MarshalIndent:", "json.MarshalIndent(" in marshal)
print("sorts map keys:", bool(re.search(r'\b(sort|Sort|Sorted)\b', marshal)))
print("SslProfiles forward bytes:", repr(forward))
print("SslProfiles reverse bytes:", repr(reverse))
print("different record order produces different bytes:", forward != reverse)
tests = "\n".join(str(p) for p in Path(".").rglob("*_test.go") if p.is_file())
exact_byte_patterns = [
r'bytes\.Equal',
r'assert\.Equal\(.*\[\]byte',
r'cmp\.Diff\(.*\[\]byte',
r'expected.*skrouterd\.json',
]
print("exact-byte test indicators:", [
pattern for pattern in exact_byte_patterns if re.search(pattern, tests, re.DOTALL)
])
print("adaptor test file exists:", Path("internal/kube/adaptor/config_init_test.go").exists())
PYRepository: skupperproject/skupper
Length of output: 786
Define and test one canonical serializer for both representations.
qdr.MarshalRouterConfig iterates over configuration maps without sorting keys and uses json.MarshalIndent, so output bytes can vary. Specify the canonical formatting, record order, field omission, defaults, and escaping rules. Add a golden test that compares exact skrouterd.json bytes for inline and chunked inputs, including SslProfiles and ProxyProfiles.
| limits are also impractical: only 13 MultiKeyListeners, each using the maximum | ||
| 256 routing keys of 64 bytes, exceed the 1 MiB limit. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the MultiKeyListener capacity calculation.
13 × 256 × 64 equals 212,992 bytes, not more than 1 MiB. Raw key bytes reach 1 MiB at 64 such listeners, before serialization overhead. Correct the listener count or provide a calculation based on the actual serialized configuration size. (kubernetes.io)
Source: MCP tools
|
Minor thing to keep in mind: there is an ADR in #2541 with the same id: |
|
Thanks @nluaces, closing this issue. It looks like compressing the configuration alone is going to get us enough to fit any reasonable kubernetes Site. Following up with a replacement ADR. |
2 participants
Part of #2556
Summary by CodeRabbit