Keep the port protocol in generated snippets

dbuild/config.py

@@ -655,11 +655,20 @@ def _parse_service_data(
                 proto = "tcp"
                 if "/" in tgt:
                     tgt, proto = tgt.split("/", 1)
+                # The single-field short form is one string for BOTH ports, so
+                # `53/udp` leaves the protocol stuck on the published port too and
+                # the snippet renders `53/udp:53/udp`. Strip it here rather than in
+                # the template: `published` is documented as a port number, and
+                # every consumer of cfg.ports would otherwise need the same guard.
+                if "/" in pub:
+                    pub = pub.split("/", 1)[0]
             else:
                 pub = str(p.get("published", ""))
                 tgt = str(p.get("target", ""))
                 proto = p.get("protocol", "tcp")
-            ports.append({"published": pub, "target": tgt, "protocol": proto})
+            # Compose accepts `TCP`/`53:53/UDP`; every consumer compares or emits
+            # lowercase (README suffix, appjail `proto:`), so normalise once here.
+            ports.append({"published": pub, "target": tgt, "protocol": str(proto or "tcp").lower()})
 
     # 2. Fallback to legacy config.yaml (if compose missing or lists empty)
     if not env:
@@ -675,7 +684,7 @@ def _parse_service_data(
             ports.append({
                 "published": str(p.get("port", "")),
                 "target": str(p.get("port", "")),
-                "protocol": p.get("protocol", "tcp")
+                "protocol": str(p.get("protocol") or "tcp").lower()
             })
 
     return env, volumes, ports

dbuild/templates/README.j2

@@ -31,6 +31,31 @@
 {{ v.path }}{{ ":" ~ v.options if v.options else "" }}
 {%- endmacro -%}
 
+{#- A port's `/proto` suffix, emitted only when it is load-bearing.
+
+    tcp is the default for both compose and `podman run -p`, so a bare
+    `4000:4000` already means tcp and adding the suffix everywhere would churn
+    every README in the catalogue to no benefit.
+
+    It IS load-bearing when a service publishes the same port on both
+    protocols. Without the suffix the two entries render identically — blocky
+    declares `53:53/udp` and `53:53/tcp` and the snippet emitted `53:53` twice,
+    which podman rejects as a duplicate mapping. Where a runtime tolerates it,
+    the copy-pasted result is TCP-only DNS: a resolver that answers `dig +tcp`
+    and nothing a normal client sends.
+
+    The parameters table (`p.protocol|upper`) was always right; only the
+    copy-pasteable snippets dropped it.
+
+    Compared case-insensitively and emitted lowercased: compose accepts
+    `protocol: TCP` and `53:53/UDP`, and config.py stores whatever the author wrote
+    verbatim. A raw comparison would let `TCP` through as a non-default and append
+    `/TCP` to every port of an image that should have been left alone. -#}
+{%- macro proto_suffix(p) -%}
+{%- set proto = (p.protocol | default("", true) | lower) -%}
+{{ "/" ~ proto if proto and proto != "tcp" else "" }}
+{%- endmacro -%}
+
 {%- macro compose_snippet(override_port=None) -%}
 services:
   {{ name }}:
@@ -56,9 +81,9 @@ services:
     ports:
 {%- for p in ports %}
 {%- if loop.first %}
-      - "{{ override_port if override_port else port_var }}:{{ p.target }}"
+      - "{{ override_port if override_port else port_var }}:{{ p.target }}{{ proto_suffix(p) }}"
 {%- else %}
-      - "{{ p.port }}:{{ p.target }}"
+      - "{{ p.port }}:{{ p.target }}{{ proto_suffix(p) }}"
 {%- endif %}
 {%- endfor %}
 {%- endif %}
@@ -202,9 +227,9 @@ podman run -d --name {{ name }} \
 {%- endif %}
 {%- for p in ports %}
 {%- if loop.first %}
-  -p {{ override_port if override_port else port_var }}:{{ p.target }} \
+  -p {{ override_port if override_port else port_var }}:{{ p.target }}{{ proto_suffix(p) }} \
 {%- else %}
-  -p {{ p.port }}:{{ p.target }} \
+  -p {{ p.port }}:{{ p.target }}{{ proto_suffix(p) }} \
 {%- endif %}
 {%- endfor %}
 {%- for a in annotations %}
@@ -645,7 +670,26 @@ Open `http://<server>:{{ ports[0].port if render_mode != "mkdocs" else port_var
 {%- block implementation_details %}
 
 **Architectures:** {{ architectures | default(['amd64']) | join(', ') }}
+{#- PUID/PGID only mean something for an image whose init drops privilege. A
+    root image (`user: "root"` in x-daemonless) stays root on purpose — blocky
+    binds :53, and FreeBSD reserves ports below 1024 to root with no capability
+    mechanism to delegate one. Printing "root (UID/GID via PUID/PGID, defaults
+    to 1000:1000)" told the reader both that it runs as root and that it runs as
+    1000:1000.
+
+    The wording says what is certain — the PROCESS stays root — and deliberately
+    does NOT claim PUID/PGID are unused. A root image can still honour them for
+    the base init's chown of /config, and some READMEs print a PUID row in the
+    env table on the same page, so "not used" would contradict the document a few
+    lines up.
+
+    🔴 README.mkdocs.j2 OVERRIDES this whole block. A fix applied only here never
+    reaches the docs site. Both are updated; keep them in step. -#}
+{%- if user == "root" %}
+**User:** `root` — the service process stays root by design, not dropped to `bsd`
+{%- else %}
 **User:** `{{ user }}` (UID/GID via PUID/PGID, defaults to 1000:1000)
+{%- endif %}
 **Base:** FreeBSD {{ base_version }}
 {%- endblock %}
 {%- block community %}

dbuild/templates/README.mkdocs.j2

@@ -332,7 +332,14 @@ Save as `run.sh`, then run `sh run.sh`.
 !!! info "Implementation Details"
 
     - **Architectures:** {{ architectures | default(['amd64']) | join(', ') }}
+{#- Mirrors the same conditional in README.j2. This block OVERRIDES that one, so a fix
+    applied only there silently never reaches the docs site — which is the page most
+    readers actually see. Keep the two in step. -#}
+{%- if user == "root" %}
+    - **User:** `root` — the service process stays root by design, not dropped to `bsd`.
+{%- else %}
     - **User:** `{{ user }}` (UID/GID set via [PUID/PGID](../guides/permissions.md)). Defaults to `1000:1000`.
+{%- endif %}
     - **Base:** Built on `@REGISTRY@/base` (FreeBSD {{ base_version }}).
 {%- if mlock %}
     - **mlock:** Requires `--annotation 'org.freebsd.jail.allow.mlock=true'` ([ocijail 0.5.0+](/guides/ocijail-patch/))

tests/test_port_protocol.py

@@ -0,0 +1,108 @@
+"""Port protocol in generated snippets, and the root-user implementation line."""
+
+from __future__ import annotations
+
+import argparse
+import tempfile
+import unittest
+from pathlib import Path
+
+from dbuild import config as dbuild_config
+from dbuild import docs
+
+from tests.test_docs import CONTAINERFILE_J2, _chdir
+
+COMPOSE = """\
+name: dns
+x-daemonless:
+  title: "DNS"
+  description: "A resolver."
+  category: "Network"
+  icon: ":test:"
+  upstream_url: "https://example.com"
+  user: "{user}"
+services:
+  dns:
+    image: ghcr.io/daemonless/dns:latest
+    ports:
+{ports}
+"""
+
+
+def _render(ports: list[str], user: str = "bsd") -> tuple[dict, str, str, str]:
+    """Return (cfg.ports[0], README.md, docs-site page, podman run snippet).
+
+    The podman run snippet only reaches a page when there is no compose snippet
+    (CLI images), so it is rendered straight from its macro.
+    """
+    port_lines = "\n".join(f"      - {p}" for p in ports)
+    with tempfile.TemporaryDirectory() as d:
+        repo = Path(d)
+        (repo / "compose.yaml").write_text(COMPOSE.format(user=user, ports=port_lines))
+        (repo / "Containerfile.j2").write_text(CONTAINERFILE_J2)
+        args = argparse.Namespace(community=None)
+        with _chdir(repo):
+            cfg = dbuild_config.load(repo)
+            outputs, errors = docs.render_generated(cfg, args, repo)
+            context = docs._enrich_metadata(cfg, None)
+            env = docs._get_jinja_env(repo)
+            site = env.get_template("README.mkdocs.j2").render(context, render_mode="mkdocs")
+            run = env.get_template("README.j2").make_module(
+                dict(context, render_mode="github")
+            ).cli_snippet()
+    assert not errors, errors
+    return cfg.ports[0], outputs["README.md"], site, run
+
+
+class TestPortProtocol(unittest.TestCase):
+    def test_dual_protocol_keeps_both_mappings_distinct(self):
+        _, readme, site, run = _render(['"53:53/udp"', '"53:53/tcp"'])
+        # udp keeps its suffix; tcp stays bare because it is the default
+        self.assertIn('- "53:53/udp"\n      - "53:53"\n', readme)
+        self.assertIn(':53/udp"\n          - "53:53"\n', site)
+        self.assertIn("-p 53:53/udp \\\n  -p 53:53 \\", run)
+
+    def test_tcp_only_image_renders_unchanged(self):
+        _, readme, _, _ = _render(['"8080:8080"'])
+        self.assertNotIn("/tcp", readme)
+
+    def test_uppercase_tcp_is_still_the_default(self):
+        _, readme, site, _ = _render(['"8080:8080/TCP"'])
+        for page in (readme, site):
+            self.assertNotIn("/TCP", page)
+            self.assertNotIn("/tcp", page)
+            # the AppJail snippet emits the protocol verbatim, so it must arrive lowercase
+            self.assertNotIn("proto:TCP", page)
+            self.assertIn("proto:tcp", page)
+
+    def test_uppercase_udp_is_lowercased_everywhere(self):
+        _, readme, _, run = _render(['"53:53/UDP"'])
+        self.assertIn('- "53:53/udp"', readme)
+        self.assertIn("proto:udp", readme)
+        self.assertNotIn("UDP'", readme)
+        self.assertIn("-p 53:53/udp", run)
+
+    def test_single_field_short_form_strips_proto_from_published(self):
+        port, readme, _, _ = _render(['"53/udp"'])
+        self.assertIn('- "53:53/udp"', readme)
+        self.assertNotIn("53/udp:", readme)
+        self.assertEqual(port["published"], "53")
+        self.assertEqual(port["target"], "53")
+        self.assertEqual(port["protocol"], "udp")
+
+
+class TestRootUserLine(unittest.TestCase):
+    def test_root_image_does_not_claim_1000(self):
+        _, readme, site, _ = _render(['"53:53/udp"'], user="root")
+        for page in (readme, site):
+            self.assertIn("stays root by design", page)
+            self.assertNotIn("`root` (UID/GID", page)
+
+    def test_non_root_image_keeps_puid_line(self):
+        _, readme, site, _ = _render(['"8080:8080"'])
+        self.assertIn("**User:** `bsd` (UID/GID via PUID/PGID", readme)
+        self.assertIn("**User:** `bsd` (UID/GID set via", site)
+
+
+if __name__ == "__main__":
+    unittest.main()