Skip to content

luci-app-pbr-1.2.3: Tor policy warning strings, drop dead iptables code - #39

Merged
egc112 merged 2 commits into
1.2.3from
luci-1.2.3-tor-policy-warnings
Aug 13, 2026
Merged

luci-app-pbr-1.2.3: Tor policy warning strings, drop dead iptables code#39
egc112 merged 2 commits into
1.2.3from
luci-1.2.3-tor-policy-warnings

Conversation

@egc112

@egc112 egc112 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

What

Companion to mossdef-org/pbr#150, which restores the Tor policy warnings.

pbr now emits warningTorUnsetSrcPort and warningTorUnsetDestPort in place of the single warningTorUnsetParams, so the WebUI needs the matching entries or it renders "Unknown warning".

  • status.js: replace warningTorUnsetParams with warningTorUnsetSrcPort and warningTorUnsetDestPort. The old string told users to unset src_addr, which is in fact the correct way to match a Tor policy, so it is not carried over. The two new strings state their differing consequence: src_port produces an invalid rule, dest_port is merely ignored.
  • compat 35 → 36 in status.js (LuciCompat) and luci.pbr (rpcdCompat).

Dead iptables code removed

The iptables backend was removed in pbr 1.1.7 and running_iptables has been hardcoded false ever since; pbr#150 drops it from the status payload entirely.

  • status.js: drop the running_iptables field and the "(iptables mode)" branch; drop errorNoIptables, errorNoIpset and errorIpsetNameTooLong, none of which exist in pbr's catalog; drop warningTorUnsetChainIpt, orphaned since the iptables variant was removed.
  • 72_pbr.js: drop the matching "(iptables mode)" branch.

Net effect on the WebUI is nil — every branch removed was unreachable and every string removed was for a code pbr cannot produce. The diff is a net deletion: +9 / −21.

Translations

po/templates/pbr.pot is deliberately untouched — it is xgettext output, regenerated from the luci tree (its source references carry applications/luci-app-pbr/ paths), not hand-maintained here. It currently still lists the removed Please unset 'src_addr', 'src_port' and 'dest_port'... string and does not yet carry the two new ones; that resolves on the next template regeneration. Translators will need the two new messages:

  • Please unset 'src_port' for policy '%s': it produces an invalid rule
  • Please unset 'dest_port' for policy '%s': it is ignored

Not touched

  • warningAGHVersionTooLow, dead in the same way but belonging to a separate AdGuardHome cleanup.
  • The resolver_ipsetresolver_set migration in pbr's uci-defaults, still needed for upgrades from old configs.

⚠️ Merge together with pbr#150

These two are a matched pair. Merging either alone leaves a compat mismatch — users would see the "WebUI application is outdated" / version-mismatch warning until both are in.

🤖 Generated with Claude Code

Companion to the pbr change that restores the Tor policy warnings. pbr now
emits warningTorUnsetSrcPort and warningTorUnsetDestPort in place of the
single warningTorUnsetParams, so the WebUI needs the matching entries or it
renders 'Unknown warning'.

 - status.js: replace warningTorUnsetParams with warningTorUnsetSrcPort and
   warningTorUnsetDestPort. The old string told users to unset 'src_addr',
   which is in fact the correct way to match a Tor policy, so it is not
   carried over. The two new strings also state the consequence, which
   differs: src_port produces an invalid rule, dest_port is merely ignored.
 - compat 35 -> 36 in status.js (LuciCompat) and luci.pbr (rpcdCompat).

Also removes render entries and branches for things pbr can no longer emit.
The iptables backend was removed in pbr 1.1.7 and 'running_iptables' has
been hardcoded false ever since; the companion PR drops it from the status
payload entirely.

 - status.js: drop the running_iptables field and the "(iptables mode)"
   branch; drop errorNoIptables, errorNoIpset and errorIpsetNameTooLong,
   none of which exist in pbr's catalog; drop warningTorUnsetChainIpt,
   orphaned since the iptables variant was removed.
 - 72_pbr.js: drop the matching "(iptables mode)" branch.

Net effect on the WebUI is nil -- every branch removed was unreachable and
every string removed was for a code pbr cannot produce.

Not touched: warningAGHVersionTooLow, which is dead in the same way but
belongs to a separate AdGuardHome cleanup, and the 'resolver_ipset' ->
'resolver_set' migration in pbr's uci-defaults, which is still needed for
upgrades from old configs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: Erik Conijn <egc112@msn.com>
@egc112

egc112 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

All three addressed. Comments added in e853b74; on the wording I took the intent but not the literal text — reasoning below.

1 & 3. Compat coupling comments — added at both declarations

Agreed the coupling was invisible in the source: both values read as arbitrary constants, and the five-location bump is easy to get half-right.

What I did not write is "DO NOT merge this PR without mossdef-org/pbr#150". That is PR-lifecycle state in a permanent source file — it goes stale the moment this merges, and every future reader sees a merge instruction for a PR that landed long ago. Worse, it would still be sitting there at the next compat bump, pointing at the wrong PR.

Instead both comments state the durable invariant, which applies to every future catalog change rather than just this one:

// Must equal pbr's pkg.compat and rpcdCompat in luci.pbr. Any change to
// pbr's error/warning catalog bumps all five in lockstep: pbr pkg.uc,
// pbr files/etc/init.d/pbr, pbr tests/04_policies/01_start_dynamic_routing,
// this getter, and rpcdCompat. A mismatch trips isVersionMismatch() below
// and tells the user their WebUI is outdated, so the two packages have to
// be released together.
get LuciCompat() {
// Must equal LuciCompat in pbr/status.js and pkg.compat in the pbr backend;
// a mismatch trips isVersionMismatch() and warns the user to update. Bump all
// five locations together whenever pbr's message catalog changes.
const rpcdCompat = 36; // ucode-lsp disable

Naming the five locations is the part that actually prevents the mistake — two of them are easy to miss with a plain grep (initCompat in a shell script, and LuciCompat, which is a multi-line getter).

The merge coordination for this PR is stated in both PR descriptions, where transient information belongs and where it does not rot.

2. Translations — pot is generated, not hand-maintained

po/templates/pbr.pot is deliberately untouched. It is xgettext output regenerated from the luci tree — its source references carry applications/luci-app-pbr/... paths, not paths from this repo — so hand-editing it here would be overwritten and would put a spurious hunk in the diff.

Current state, for the record: it still lists the removed string

#: .../status.js:...
msgid "Please unset 'src_addr', 'src_port' and 'dest_port' for policy '%s'"

and does not yet carry the two new ones. That resolves on the next template regeneration.

Added a Translations section to the PR description naming both new messages so translators have them:

  • Please unset 'src_port' for policy '%s': it produces an invalid rule
  • Please unset 'dest_port' for policy '%s': it is ignored

LuciCompat and rpcdCompat have to equal pbr's pkg.compat, and a mismatch
trips isVersionMismatch() and tells the user their WebUI is outdated. That
invariant was nowhere in the source, so the two values read as arbitrary
constants and the five-location bump is easy to get half-right.

States the invariant at both declarations rather than referencing a specific
PR: the coupling applies to every future catalog change, not just this one,
and merge-time coordination belongs in the PR description where it does not
go stale.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Signed-off-by: Erik Conijn <egc112@msn.com>
@egc112

egc112 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

Co-Pilot comment:

LGTM (looks good to me) after the inline comments you added — thanks, they make the coupling to pbr explicit. A couple small follow-ups remain (translations, explicit PR cross-reference, optional compatibility fallback). None are blocking.

@egc112
egc112 merged commit f8738d1 into 1.2.3 Aug 13, 2026
3 checks passed
@egc112
egc112 deleted the luci-1.2.3-tor-policy-warnings branch August 13, 2026 11:34
egc112 added a commit that referenced this pull request Aug 14, 2026
Lockstep release bump with pbr. Covers errorInterfacePriorityExhausted
(#37) and the Tor policy warning strings plus dead iptables cleanup
(#39). Compat moved 34 -> 36 across those changes, so both packages have
to be released together.

Signed-off-by: Erik Conijn <egc112@msn.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant