Support changing or removing Jira autolinks

README.md

@@ -509,7 +509,10 @@ github:
     - AMBARI
 ~~~
 
-The `autolink_jira` property can be a single string or a list of strings, each corresponding to a Jira project on `issues.apache.org`. It **must** adhere to the Jira project name syntax (uppercase alphabetical characters only).
+The `autolink_jira` property can be a single string or a list of strings, each corresponding to a Jira project on `issues.apache.org`. It **must** adhere to the Jira project name syntax (uppercase alphabetical characters only). Autolinks only match numeric ticket ids, e.g. `INFRA-123` but not `INFRA-FOO`.
+
+Once `autolink_jira` has been configured, it is the authoritative list of Jira autolinks for the repository: removing an entry from the list removes the corresponding autolink, removing the entire section removes all Jira autolinks, and any Jira autolink set up by other means (for instance via an INFRA ticket) is removed if not in the list. Autolinks pointing anywhere else than the ASF Jira are never touched, and if `autolink_jira` has never been configured, no autolinks are added or removed.
+
 We will evaluate the need for other autolink features.
 
 <h3 id="branchpro">Branch protection</h3>

asfyaml/feature/github/autolink.py

@@ -19,25 +19,57 @@
 
 from . import directive, ASFGitHubFeature
 
+JIRA_BROWSE_URL = "https://issues.apache.org/jira/browse/"
+
+
+def jira_autolink_url(jira_space: str) -> str:
+    return f"{JIRA_BROWSE_URL}{jira_space}-<num>"
+
+
+def listify(value) -> list:
+    # If not a list, assume a string and listify it (we'll validate shortly...)
+    if not value:
+        return []
+    return value if isinstance(value, list) else [value]
+
 
 @directive
 def autolink(self: ASFGitHubFeature):
-    # Jira auto-linking
-    autolink_jira = self.yaml.get("autolink_jira")
-    if autolink_jira:
-        # If not a list, assume a string and listify it (we'll validate shortly...)
-        if not isinstance(autolink_jira, list):
-            autolink_jira = [autolink_jira]
-        # Grab any existing auto-links (to ensure we don't recreate them over and over)
-        if not self.instance.no_cache:
-            existing_autolinks = [x for x in self.ghrepo.get_autolinks()]  # Paginated (Iter) result -> list
+    # Jira auto-linking. Once autolink_jira has been configured, it is the authoritative list of
+    # Jira auto-links for the repository: any ASF Jira auto-link not in the list is removed, no
+    # matter how it was created. If it has never been configured, nothing is added or removed.
+    previous_yaml = self.previous_yaml if isinstance(self.previous_yaml, dict) else {}
+    if "autolink_jira" not in self.yaml and "autolink_jira" not in previous_yaml:
+        return
+
+    desired_spaces = listify(self.yaml.get("autolink_jira"))
+    desired_urls = {jira_autolink_url(jira_space) for jira_space in desired_spaces}
+
+    # Grab any existing auto-links (to ensure we don't recreate them over and over)
+    if not self.instance.no_cache:
+        existing_autolinks = [x for x in self.ghrepo.get_autolinks()]  # Paginated (Iter) result -> list
+    else:
+        existing_autolinks = []
+
+    # Remove ASF Jira auto-links that are not (or no longer) in the config. Auto-links pointing
+    # anywhere else than the ASF Jira are left untouched. An auto-link matching a configured Jira
+    # space but with is_alphanumeric set is also removed, to be recreated correctly below.
+    matched_urls = set()
+    for existing in existing_autolinks:
+        if not existing.url_template.startswith(JIRA_BROWSE_URL):
+            continue
+        if existing.url_template in desired_urls and not existing.is_alphanumeric:
+            matched_urls.add(existing.url_template)
         else:
-            existing_autolinks = []
-        # Now add the autolink if not already there
-        for jira_space in autolink_jira:
-            jira_url = f"https://issues.apache.org/jira/browse/{jira_space}-<num>"
-            # Check whether the url_template matches an existing auto-link. If not, create the auto-link entry.
-            if not any(jira_url == al.url_template for al in existing_autolinks):
-                print(f"Setting up new auto-link for {jira_space}-<num> -> {jira_url}")
-                if not self.noop("autolink_jira"):
-                    self.ghrepo.create_autolink(key_prefix=f"{jira_space}-", url_template=jira_url)
+            print(f"Removing auto-link for {existing.key_prefix}<num> -> {existing.url_template}")
+            if not self.noop("autolink_jira"):
+                self.ghrepo.remove_autolink(existing)
+
+    # Now add the autolink if not already there. is_alphanumeric=False ensures the auto-link only
+    # matches numeric ticket ids, e.g. SOLR-123 but not SOLR-FOO.
+    for jira_space in desired_spaces:
+        jira_url = jira_autolink_url(jira_space)
+        if jira_url not in matched_urls:
+            print(f"Setting up new auto-link for {jira_space}-<num> -> {jira_url}")
+            if not self.noop("autolink_jira"):
+                self.ghrepo.create_autolink(key_prefix=f"{jira_space}-", url_template=jira_url, is_alphanumeric=False)

tests/github_autolink.py

@@ -17,9 +17,13 @@
 
 """Unit tests for .asf.yaml GitHub autolink features"""
 
+from types import SimpleNamespace
+from typing import Any
+
 from helpers import YamlTest
 import asfyaml.asfyaml
 import asfyaml.dataobjects
+from asfyaml.feature.github.autolink import autolink as configure_autolinks
 
 # Set .asf.yaml to debug mode
 asfyaml.asfyaml.DEBUG = True
@@ -76,7 +80,7 @@ def test_basic_yaml(test_repo: asfyaml.dataobjects.Repository):
         valid_github_autolink,
         valid_github_autolink_single,
         invalid_github_autolink_not_upperalpha,
-        invalid_github_autolink_not_list
+        invalid_github_autolink_not_list,
     )
 
     for test in tests_to_run:
@@ -87,3 +91,185 @@ def test_basic_yaml(test_repo: asfyaml.dataobjects.Repository):
             a.environments_enabled.add("noop")
             a.no_cache = True
             a.run_parts()
+
+
+def make_autolink(autolink_id: int, jira_space: str, is_alphanumeric: bool = False) -> SimpleNamespace:
+    return SimpleNamespace(
+        id=autolink_id,
+        key_prefix=f"{jira_space}-",
+        url_template=f"https://issues.apache.org/jira/browse/{jira_space}-<num>",
+        is_alphanumeric=is_alphanumeric,
+    )
+
+
+def expected_create(jira_space: str) -> dict:
+    return {
+        "key_prefix": f"{jira_space}-",
+        "url_template": f"https://issues.apache.org/jira/browse/{jira_space}-<num>",
+        "is_alphanumeric": False,
+    }
+
+
+class FakeGHRepo:
+    def __init__(self, existing_autolinks: list[SimpleNamespace] | None = None):
+        self.existing_autolinks = existing_autolinks or []
+        self.created: list[dict] = []
+        self.removed: list[SimpleNamespace] = []
+
+    def get_autolinks(self):
+        return list(self.existing_autolinks)
+
+    def create_autolink(self, key_prefix: str, url_template: str, is_alphanumeric: bool):
+        self.created.append(
+            {"key_prefix": key_prefix, "url_template": url_template, "is_alphanumeric": is_alphanumeric}
+        )
+
+    def remove_autolink(self, autolink: SimpleNamespace) -> bool:
+        self.removed.append(autolink)
+        return True
+
+
+class FakeFeature:
+    def __init__(
+        self,
+        *,
+        yaml: dict[str, Any],
+        previous_yaml: dict[str, Any],
+        ghrepo: FakeGHRepo,
+        noop_enabled: bool = False,
+    ):
+        self.yaml = yaml
+        self.previous_yaml = previous_yaml
+        self.ghrepo = ghrepo
+        self.instance = SimpleNamespace(no_cache=False)
+        self._noop_enabled = noop_enabled
+
+    def noop(self, directive: str) -> bool:
+        if self._noop_enabled:
+            print(f"[github::{directive}] Not applying changes, noop mode active.")
+            return True
+        return False
+
+
+def test_autolink_create_new():
+    ghrepo = FakeGHRepo()
+    feature = FakeFeature(yaml={"autolink_jira": ["FOO", "BAR"]}, previous_yaml={}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == [expected_create("FOO"), expected_create("BAR")]
+    assert ghrepo.removed == []
+
+
+def test_autolink_existing_not_recreated():
+    ghrepo = FakeGHRepo(existing_autolinks=[make_autolink(1, "FOO")])
+    feature = FakeFeature(yaml={"autolink_jira": ["FOO"]}, previous_yaml={"autolink_jira": ["FOO"]}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == []
+
+
+def test_autolink_section_removed_removes_all():
+    foo = make_autolink(1, "FOO")
+    bar = make_autolink(2, "BAR")
+    ghrepo = FakeGHRepo(existing_autolinks=[foo, bar])
+    feature = FakeFeature(yaml={}, previous_yaml={"autolink_jira": ["FOO", "BAR"]}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == [foo, bar]
+
+
+def test_autolink_entry_removed_removes_only_that_autolink():
+    foo = make_autolink(1, "FOO")
+    bar = make_autolink(2, "BAR")
+    ghrepo = FakeGHRepo(existing_autolinks=[foo, bar])
+    feature = FakeFeature(
+        yaml={"autolink_jira": ["FOO"]}, previous_yaml={"autolink_jira": ["FOO", "BAR"]}, ghrepo=ghrepo
+    )
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == [bar]
+
+
+def test_autolink_single_string_previous_removed():
+    infra = make_autolink(1, "INFRA")
+    ghrepo = FakeGHRepo(existing_autolinks=[infra])
+    feature = FakeFeature(yaml={}, previous_yaml={"autolink_jira": "INFRA"}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == [infra]
+
+
+def test_autolink_config_is_authoritative_for_jira_autolinks():
+    # A Jira autolink set up outside .asf.yaml (e.g. via INFRA ticket) is removed once
+    # autolink_jira is configured and doesn't list it
+    manual = make_autolink(1, "OTHER")
+    foo = make_autolink(2, "FOO")
+    ghrepo = FakeGHRepo(existing_autolinks=[manual, foo])
+    feature = FakeFeature(yaml={"autolink_jira": ["FOO"]}, previous_yaml={"autolink_jira": ["FOO"]}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == [manual]
+
+
+def test_autolink_non_jira_autolinks_untouched():
+    # Autolinks pointing anywhere else than the ASF Jira are never touched
+    other = SimpleNamespace(
+        id=1, key_prefix="GH-", url_template="https://github.com/apache/foo/issues/<num>", is_alphanumeric=False
+    )
+    ghrepo = FakeGHRepo(existing_autolinks=[other])
+    feature = FakeFeature(yaml={}, previous_yaml={"autolink_jira": ["FOO"]}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == []
+
+
+def test_autolink_alphanumeric_autolink_recreated():
+    # Autolinks created with is_alphanumeric=true (matching e.g. SOLR-FOO) are recreated
+    # with is_alphanumeric=false so they only match numeric ticket ids
+    foo = make_autolink(1, "FOO", is_alphanumeric=True)
+    ghrepo = FakeGHRepo(existing_autolinks=[foo])
+    feature = FakeFeature(yaml={"autolink_jira": ["FOO"]}, previous_yaml={"autolink_jira": ["FOO"]}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.removed == [foo]
+    assert ghrepo.created == [expected_create("FOO")]
+
+
+def test_autolink_never_configured_does_nothing():
+    ghrepo = FakeGHRepo(existing_autolinks=[make_autolink(1, "FOO")])
+    feature = FakeFeature(yaml={}, previous_yaml={}, ghrepo=ghrepo)
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == []
+
+
+def test_autolink_noop_mode_makes_no_changes():
+    bar = make_autolink(1, "BAR")
+    ghrepo = FakeGHRepo(existing_autolinks=[bar])
+    feature = FakeFeature(
+        yaml={"autolink_jira": ["FOO"]},
+        previous_yaml={"autolink_jira": ["BAR"]},
+        ghrepo=ghrepo,
+        noop_enabled=True,
+    )
+
+    configure_autolinks(feature)
+
+    assert ghrepo.created == []
+    assert ghrepo.removed == []