Browse Source

fix(balancer): pin loopback routing rules ahead of general rules (#6054)

* fix(balancer): pin loopback routing rules ahead of general rules

ensureBalancerLoopback appended a balancer's fallback loopback rule
({ inboundTag: ['_bl_<target>'], balancerTag: <target> }) to the END of
routing.rules via Array.push, and only updated balancerTag in place when
the rule already existed. Xray evaluates routing rules top-to-bottom,
first match wins, and a general domain rule (no inboundTag restriction)
matches every inbound including the loopback one. So once such a general
rule targeting the parent balancer ended up earlier in the array — added
before the fallback was configured, or recreated later by an edit or by
ensureMissingBalancerLoopbacks — the fallback re-entered routing through
the _bl_<target> loopback outbound, the general rule matched again, and
traffic routed straight back into the parent balancer: an unbounded loop
and the CPU spike in the report. detectBalancerCycles only catches
mutual fallback cycles, so the plain acyclic Balancer1 -> Balancer2
chain passed silently.

Insert the loopback rule before the first general (no-inboundTag) rule
instead of pushing to the end, and reposition any existing loopback rule
that already landed after a general rule (preserving its other fields,
mirroring the EnsureStatsRouting hoist used for the same class of bug).
Rules with an inboundTag restriction (api, real inbounds) are left in
place — they can never match loopback traffic, so only the unrestricted
rules are dangerous. ensureMissingBalancerLoopbacks runs on every Save
All and calls ensureBalancerLoopback per fallback, so this also repairs
configs loaded from the DB with the wrong order.

Fixes #5960

* ci: re-trigger checks after flaky FuzzDecodeCertPin timeout

---------

Co-authored-by: x06579 <x06579@ai-dashboard>
H-TTTTT 18 hours ago
parent
commit
a9d5d9afdb

+ 22 - 4
frontend/src/pages/xray/balancers/balancer-loopback.ts

@@ -52,6 +52,18 @@ function countLoopbackRefs(settings: XraySettingsValue, targetTag: string): numb
   return count;
 }
 
+function isGeneralRoutingRule(rule: Record<string, unknown>): boolean {
+  const inboundTag = rule.inboundTag;
+  return !Array.isArray(inboundTag) || inboundTag.length === 0;
+}
+
+function firstGeneralRoutingRuleIndex(rules: Array<Record<string, unknown>>): number {
+  for (let i = 0; i < rules.length; i += 1) {
+    if (isGeneralRoutingRule(rules[i])) return i;
+  }
+  return rules.length;
+}
+
 export function ensureBalancerLoopback(
   settings: XraySettingsValue,
   targetBalancerTag: string,
@@ -72,13 +84,19 @@ export function ensureBalancerLoopback(
   if (!settings.routing) settings.routing = { rules: [], balancers: [] };
   if (!Array.isArray(settings.routing.rules)) settings.routing.rules = [];
 
-  const existingRuleIdx = (settings.routing.rules as Array<{ inboundTag?: string[] }>).findIndex(
-    (r) => Array.isArray(r.inboundTag) && r.inboundTag.includes(lbTag),
+  const rules = settings.routing.rules as Array<Record<string, unknown>>;
+
+  const existingRuleIdx = rules.findIndex(
+    (r) => Array.isArray(r.inboundTag) && (r.inboundTag as string[]).includes(lbTag),
   );
+
   if (existingRuleIdx >= 0) {
-    (settings.routing.rules as Record<string, unknown>[])[existingRuleIdx].balancerTag = targetBalancerTag;
+    const existing = rules[existingRuleIdx];
+    existing.balancerTag = targetBalancerTag;
+    rules.splice(existingRuleIdx, 1);
+    rules.splice(firstGeneralRoutingRuleIndex(rules), 0, existing);
   } else {
-    (settings.routing.rules as Record<string, unknown>[]).push({
+    rules.splice(firstGeneralRoutingRuleIndex(rules), 0, {
       type: 'field',
       inboundTag: [lbTag],
       balancerTag: targetBalancerTag,

+ 100 - 0
frontend/src/test/balancer-loopback.test.ts

@@ -24,6 +24,7 @@ interface RuleEntry {
   type?: string;
   inboundTag?: string[];
   balancerTag?: string;
+  domain?: string[];
 }
 interface BalancerEntry {
   tag?: string;
@@ -140,6 +141,105 @@ describe('ensureBalancerLoopback dedup', () => {
   });
 });
 
+describe('ensureBalancerLoopback rule ordering', () => {
+  function loopbackRuleIndex(settings: XraySettingsValue, lbTag: string): number {
+    return ruleEntries(settings).findIndex(
+      (r) => Array.isArray(r.inboundTag) && r.inboundTag.includes(lbTag),
+    );
+  }
+  function generalRuleIndex(settings: XraySettingsValue): number {
+    return ruleEntries(settings).findIndex(
+      (r) => !Array.isArray(r.inboundTag) || r.inboundTag.length === 0,
+    );
+  }
+
+  it('inserts a new loopback rule ahead of a general (no inboundTag) rule', () => {
+    const settings = makeSettings({
+      rules: [{ type: 'field', domain: ['example.com'], balancerTag: 'parent' }],
+      balancers: [{ tag: 'parent', selector: [] }],
+    });
+
+    ensureBalancerLoopback(settings, 'target');
+
+    expect(loopbackRuleIndex(settings, '_bl_target')).toBeLessThan(
+      generalRuleIndex(settings),
+    );
+  });
+
+  it('repositions an existing loopback rule that landed after a general rule', () => {
+    const settings = makeSettings({
+      rules: [
+        { type: 'field', domain: ['example.com'], balancerTag: 'parent' },
+        { type: 'field', inboundTag: ['_bl_target'], balancerTag: 'stale' },
+      ],
+      balancers: [{ tag: 'parent', selector: [] }],
+    });
+
+    ensureBalancerLoopback(settings, 'target');
+
+    const lbIdx = loopbackRuleIndex(settings, '_bl_target');
+    expect(lbIdx).toBeLessThan(generalRuleIndex(settings));
+    expect(ruleEntries(settings)[lbIdx].balancerTag).toBe('target');
+  });
+
+  it('leaves inboundTag-restricted rules in place and slots loopback ahead of general rules only', () => {
+    const settings = makeSettings({
+      rules: [
+        { type: 'field', inboundTag: ['api'], balancerTag: 'stats' },
+        { type: 'field', domain: ['example.com'], balancerTag: 'parent' },
+      ],
+      balancers: [{ tag: 'parent', selector: [] }],
+    });
+
+    ensureBalancerLoopback(settings, 'target');
+
+    const entries = ruleEntries(settings);
+    expect(entries[0].inboundTag).toEqual(['api']);
+    const lbIdx = loopbackRuleIndex(settings, '_bl_target');
+    const generalIdx = generalRuleIndex(settings);
+    expect(lbIdx).toBeLessThan(generalIdx);
+    expect(lbIdx).toBeGreaterThan(0);
+  });
+
+  it('ensureMissingBalancerLoopbacks repositions every mis-ordered loopback rule', () => {
+    const settings = makeSettings({
+      rules: [
+        { type: 'field', domain: ['example.com'], balancerTag: 'B1' },
+        { type: 'field', inboundTag: ['_bl_B2'], balancerTag: 'B2' },
+      ],
+      balancers: [
+        { tag: 'B1', selector: [], fallbackTag: '_bl_B2' },
+        { tag: 'B2', selector: [] },
+      ],
+    });
+
+    ensureMissingBalancerLoopbacks(settings);
+
+    expect(loopbackRuleIndex(settings, '_bl_B2')).toBeLessThan(
+      generalRuleIndex(settings),
+    );
+  });
+
+  it('keeps the loopback rule ahead of the general rule after a second ensureBalancerLoopback call', () => {
+    const settings = makeSettings({
+      rules: [{ type: 'field', domain: ['example.com'], balancerTag: 'parent' }],
+      balancers: [{ tag: 'parent', selector: [] }],
+    });
+
+    ensureBalancerLoopback(settings, 'target');
+    ensureBalancerLoopback(settings, 'target');
+
+    expect(loopbackRuleIndex(settings, '_bl_target')).toBeLessThan(
+      generalRuleIndex(settings),
+    );
+    expect(
+      ruleEntries(settings).filter(
+        (r) => Array.isArray(r.inboundTag) && r.inboundTag.includes('_bl_target'),
+      ),
+    ).toHaveLength(1);
+  });
+});
+
 describe('detectBalancerCycles', () => {
   const cases: Array<{ name: string; balancers: BalancerEntry[]; expected: string[][] }> = [
     {