Implement canonical changelog area hierarchy (/) with filename encoding (~)

py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py

@@ -32,6 +32,8 @@
 CHANGELOG_ENTRY_GLOB = "*/*.rst"
 CHANGELOG_CONFIG_PATH = "changelogs/changelogs.yaml"
 ENTRY_SEPARATOR = "__"
+AREA_SEPARATOR = "/"
+AREA_FILENAME_SEPARATOR = "~"
 CHANGELOG_SUMMARY_PATH = "changelogs/summary.md"
 CHANGELOG_ENTRY_URL_TPL = (
     "https://raw.githubusercontent.com/envoyproxy/envoy/"
@@ -53,6 +55,14 @@
 YAML_CHANGELOGS_VERSION = "1.23"
 
 
+def area_from_filename(stem_area: str) -> str:
+    return stem_area.replace(AREA_FILENAME_SEPARATOR, AREA_SEPARATOR)
+
+
+def area_to_filename(area: str) -> str:
+    return area.replace(AREA_SEPARATOR, AREA_FILENAME_SEPARATOR)
+
+
 class LegacyChangelog:
     # Parser for changelogs < 1.23.0
 
@@ -138,9 +148,11 @@ def data_from_entry_map(
                     f"Invalid entry filename "
                     f"(expected exactly one '{ENTRY_SEPARATOR}'): "
                     f"{entry_path}")
-            area, _slug = path.stem.split(ENTRY_SEPARATOR, 1)
+            stem_area, _slug = path.stem.split(ENTRY_SEPARATOR, 1)
             change = typing.Change(text)
-            entry: typing.ChangeDict = dict(area=area, change=change)
+            entry: typing.ChangeDict = dict(
+                area=area_from_filename(stem_area),
+                change=change)
             sections.setdefault(path.parent.name, []).append(entry)
         return cast(
             typing.ChangelogDict,

py/envoy.base.utils/tests/test_abstract_project_changelogs.py

@@ -1419,6 +1419,7 @@ def test_abstract_changelog_get_data(iters, patches, raises):
 def test_abstract_changelog_data_from_entry_map_happy_path():
     entries = {
         "new_features/grpc__cool.rst": "New feature.\n",
+        "new_features/dns~cares__preserve-qid.rst": "Preserve qid.\n",
         "bug_fixes/oauth2__foo_fix.rst": "Fixed oauth2.\n",
         "bug_fixes/jwt__bar_fix.rst": "Fixed jwt.\n"}
 
@@ -1430,6 +1431,7 @@ def test_abstract_changelog_data_from_entry_map_happy_path():
                 dict(area="jwt", change="Fixed jwt.\n"),
                 dict(area="oauth2", change="Fixed oauth2.\n")],
             new_features=[
+                dict(area="dns/cares", change="Preserve qid.\n"),
                 dict(area="grpc", change="New feature.\n")]))
 
 

py/envoy.code.check/README.rst

@@ -39,6 +39,19 @@ Run only against files changed since a given ref:
 
 See ``envoy.code.check --help`` for the full set of options.
 
+Changelog entry areas
+---------------------
+
+Changelog area IDs in ``changelogs/changelogs.yaml`` are canonical and may use
+``/`` to represent hierarchy (for example ``dns/cares``). Because ``/`` cannot
+be used in entry filenames, changelog entries encode ``/`` as ``~`` in
+``changelogs/current/<section>/<area>__<slug>.rst`` (for example
+``dns~cares__preserve-qid.rst``).
+
+Area keys must match ``[a-z0-9_\\-/]+`` (so ``~`` is not valid in keys). Area
+titles must not contain ``~``. If an area key contains ``/``, it must define an
+explicit non-empty ``title``.
+
 Links
 -----
 

py/envoy.code.check/envoy/code/check/abstract/changelog.py

@@ -20,14 +20,36 @@
 )
 from envoy.code.check import abstract, interface
 
+try:
+    from envoy.base.utils.abstract.project.changelog import (
+        AREA_FILENAME_SEPARATOR,
+        AREA_SEPARATOR,
+        area_from_filename,
+    )
+except ImportError:
+    AREA_SEPARATOR = "/"
+    AREA_FILENAME_SEPARATOR = "~"
+
+    def area_from_filename(stem_area: str) -> str:
+        return stem_area.replace(AREA_FILENAME_SEPARATOR, AREA_SEPARATOR)
+
 
 MAX_VERSION_FOR_CHANGES_SECTION = "1.16"
 VALID_CHANGELOG_AREA_RE = re.compile(r"^[a-z0-9_\-/]+$")
 VALID_CHANGELOG_AREA_PATTERN = r"[a-z0-9_\-/]+"
+VALID_CHANGELOG_ENTRY_AREA_RE = re.compile(r"^[a-z0-9_\-~]+$")
+VALID_CHANGELOG_ENTRY_AREA_PATTERN = r"[a-z0-9_\-~]+"
 
 
 @abstracts.implementer(interface.IChangelogChangesChecker)
 class AChangelogChangesChecker(metaclass=abstracts.Abstraction):
+    """Changelog checker.
+
+    Changelog area IDs are canonicalized with ``/`` for hierarchy in
+    ``changelogs/changelogs.yaml`` while changelog entry filenames
+    encode ``/`` as ``~`` in ``<area>__<slug>.rst``.
+    """
+
     error_message = (
         "{version}/{section}/{entry[area]}: "
         "{error}\n{entry[change]}")
@@ -127,12 +149,21 @@ def check_entry_filename(
                 f"{path}: Filename stem must contain exactly one "
                 f"`{ENTRY_SEPARATOR}` separator "
                 f"(expected `<area>{ENTRY_SEPARATOR}<slug>`)")
-        area, slug = path.stem.split(ENTRY_SEPARATOR, 1)
-        if not area:
+        stem_area, slug = path.stem.split(ENTRY_SEPARATOR, 1)
+        if not stem_area:
             return f"{path}: Area part of filename is empty"
+        if not VALID_CHANGELOG_ENTRY_AREA_RE.match(stem_area):
+            return (
+                f"{path}: Invalid area '{stem_area}' "
+                f"(must match {VALID_CHANGELOG_ENTRY_AREA_PATTERN})")
+        area = area_from_filename(stem_area)
         if self.areas and area not in self.areas:
+            area_display = (
+                f"'{area}' (from filename '{stem_area}')"
+                if area != stem_area
+                else f"'{area}'")
             return (
-                f"{path}: Invalid area '{area}'. "
+                f"{path}: Invalid area {area_display}. "
                 f"Valid areas come from {self.config_path}")
         if not slug:
             return f"{path}: Slug part of filename is empty"
@@ -144,13 +175,23 @@ def check_areas_file(self) -> tuple[str, ...]:
         title_areas: dict[str, list[str]] = {}
         errors = []
         for area, area_data in self.areas.items():
-            title = area_data["title"]
+            title = area_data.get("title", "")
             title_areas.setdefault(title, []).append(area)
             if not VALID_CHANGELOG_AREA_RE.match(area):
                 errors.append(
                     f"{self.config_path}: "
                     f"Invalid area key '{area}' "
                     f"(must match {VALID_CHANGELOG_AREA_PATTERN})")
+            if AREA_FILENAME_SEPARATOR in title:
+                errors.append(
+                    f"{self.config_path}: "
+                    "Title must not contain '~' "
+                    "(it is only a filename escape for '/')")
+            if AREA_SEPARATOR in area and not title.strip():
+                errors.append(
+                    f"{self.config_path}: "
+                    f"Area '{area}' contains '/' and must set an explicit "
+                    "title")
             if not VALID_CHANGELOG_AREA_RE.match(title):
                 errors.append(
                     f"{self.config_path}: "

py/envoy.code.check/tests/test_abstract_changelog.py

@@ -890,6 +890,9 @@ def test_changeschecker_check_section_name(
     [(  # valid
         "bug_fixes", "myarea__myslug", ".rst",
         None),
+     (  # valid encoded nested area
+        "bug_fixes", "dns~cares__myslug", ".rst",
+        None),
      (  # invalid section
         "weird_section", "area__slug", ".rst",
         ("weird_section", "Invalid section")),
@@ -907,13 +910,17 @@ def test_changeschecker_check_section_name(
         ("Area", "empty")),
      (  # empty slug
         "bug_fixes", "area__", ".rst",
-        ("Slug", "empty"))])
+        ("Slug", "empty")),
+     (  # invalid filename area chars
+        "bug_fixes", "bad.area__slug", ".rst",
+        (r"[a-z0-9_\-~]+", ))])
 def test_changeschecker_check_entry_filename(
         section, stem, suffix, expected):
     changelog = DummyChangelogChangesChecker(
         {"bug_fixes": MagicMock()},
         {"myarea": {"title": "myarea"},
-         "area": {"title": "area"}})
+         "area": {"title": "area"},
+         "dns/cares": {"title": "dns/cares"}})
     path = MagicMock()
     path.parent.name = section
     path.stem = stem
@@ -935,13 +942,13 @@ def test_changeschecker_check_entry_filename_invalid_area():
         {"some_other_area": {"title": "some_other_area"}})
     path = MagicMock()
     path.parent.name = "bug_fixes"
-    path.stem = "myarea__myslug"
+    path.stem = "dns~cares__myslug"
     path.suffix = ".rst"
 
     result = changelog.check_entry_filename(path)
 
     assert result is not None
-    assert "Invalid area" in result
+    assert "Invalid area 'dns/cares' (from filename 'dns~cares')" in result
     assert "changelogs.yaml" in result
 
 
@@ -955,6 +962,21 @@ def test_changeschecker_check_entry_filename_without_areas():
     assert changelog.check_entry_filename(path) is None
 
 
+def test_changeschecker_check_entry_filename_decoded_area_mismatch():
+    changelog = DummyChangelogChangesChecker(
+        {"bug_fixes": MagicMock()},
+        {"dns/cares": {"title": "dns/cares"}})
+    path = MagicMock()
+    path.parent.name = "bug_fixes"
+    path.stem = "dns_cares__myslug"
+    path.suffix = ".rst"
+
+    result = changelog.check_entry_filename(path)
+
+    assert result is not None
+    assert "Invalid area 'dns_cares'" in result
+
+
 def test_changeschecker_check_areas_file_empty():
     changelog = DummyChangelogChangesChecker("SECTIONS", {})
     assert changelog.check_areas_file() == ()
@@ -992,17 +1014,45 @@ def test_changeschecker_check_areas_file_invalid_keys():
     changelog = DummyChangelogChangesChecker(
         "SECTIONS",
         {"Bad": {"title": "bad"},
-         "bad.dot": {"title": "also_bad"}})
+         "dns~cares": {"title": "dns/cares"}})
     assert (
         changelog.check_areas_file()
         == (
             "changelogs/changelogs.yaml: Invalid area key 'Bad' "
             "(must match [a-z0-9_\\-/]+)",
-            "changelogs/changelogs.yaml: Invalid area key 'bad.dot' "
+            "changelogs/changelogs.yaml: Invalid area key 'dns~cares' "
             "(must match [a-z0-9_\\-/]+)",
         ))
 
 
+def test_changeschecker_check_areas_file_tilde_in_title():
+    changelog = DummyChangelogChangesChecker(
+        "SECTIONS",
+        {"dns/cares": {"title": "dns~cares"}})
+    assert (
+        changelog.check_areas_file()
+        == (
+            "changelogs/changelogs.yaml: Title must not contain '~' "
+            "(it is only a filename escape for '/')",
+            "changelogs/changelogs.yaml: Invalid title 'dns~cares' "
+            "for area 'dns/cares' (must match [a-z0-9_\\-/]+)",
+        ))
+
+
+def test_changeschecker_check_areas_file_slash_key_requires_title():
+    changelog = DummyChangelogChangesChecker(
+        "SECTIONS",
+        {"dns/cares": {"title": ""}})
+    assert (
+        changelog.check_areas_file()
+        == (
+            "changelogs/changelogs.yaml: Area 'dns/cares' contains '/' "
+            "and must set an explicit title",
+            "changelogs/changelogs.yaml: Invalid title '' "
+            "for area 'dns/cares' (must match [a-z0-9_\\-/]+)",
+        ))
+
+
 def test_changeschecker_check_areas_file_valid():
     changelog = DummyChangelogChangesChecker(
         "SECTIONS",
@@ -1161,9 +1211,13 @@ async def test_changelogstatus_errors_invalid_area_from_entry_files(tmp_path):
         "    title: Bug fixes\n"
         "areas:\n"
         "  known:\n"
-        "    title: known\n")
+        "    title: known\n"
+        "  dns/cares:\n"
+        "    title: dns/cares\n")
     current_dir = tmp_path / "changelogs" / "current"
     (current_dir / "bug_fixes").mkdir(parents=True)
+    (current_dir / "bug_fixes" / "dns~cares__slug.rst").write_text(
+        "Valid nested area content.")
     (current_dir / "bug_fixes" / "unknown__slug.rst").write_text(
         "Valid content.")
     config = utils.from_yaml(config_path, utils.typing.ChangelogConfigDict)
@@ -1192,3 +1246,4 @@ async def _execute(func, *args):
     result = await status.errors
 
     assert any("Invalid area 'unknown'" in error for error in result)
+    assert not any("dns/cares" in error for error in result)