Fix update checks for OCI tags and yanked chart versions

scripts/chart_ctl.py

@@ -236,7 +236,21 @@ def oci_list_tags(repository: str, chart: str) -> list:
     return tags
 
 
-def latest_stable_tag(tags: list) -> str:
+def tag_preference(tag: str, current_version: str) -> int:
+    """Rank otherwise equal tags, so a repeated run always picks the same one.
+
+    Registries can carry both spellings of one release (agentgateway has 1.1.0
+    and v1.1.0); without a tiebreak the winner would follow the order the
+    registry happens to return and flip between runs.
+    """
+    if current_version and tag == current_version:
+        return 2
+    prefixed = tag.startswith('v')
+    wanted = current_version.startswith('v') if current_version else False
+    return 1 if prefixed == wanted else 0
+
+
+def latest_stable_tag(tags: list, current_version: str = None) -> str:
     """Highest release tag, ignoring pre-releases and anything not version-like."""
     latest, latest_version = None, None
     for tag in tags:
@@ -248,6 +262,9 @@ def latest_stable_tag(tags: list) -> str:
             continue
         if latest_version is None or version > latest_version:
             latest, latest_version = tag, version
+        elif version == latest_version and \
+                tag_preference(tag, current_version) > tag_preference(latest, current_version):
+            latest = tag
     return latest
 
 
@@ -268,26 +285,33 @@ def get_latest_https_chart(chart: str, repository: str) -> dict:
         subprocess.run(["helm", "repo", "remove", chart], check=True)
 
 
-def get_latest_oci_chart(chart: str, repository: str) -> dict:
+def get_latest_oci_chart(chart: str, repository: str, current_version: str = None):
     """Resolve the newest release from the registry tag list.
 
     `helm show chart --version '>=0.0.0'` would be shorter, but Helm drops every
     'v'-prefixed tag when resolving a range, which silently returns a stale
     version (or no version at all) for charts tagged that way.
     """
-    tag = latest_stable_tag(oci_list_tags(repository, chart))
+    tag = latest_stable_tag(oci_list_tags(repository, chart), current_version)
     if tag is None:
         raise RuntimeError(f"no release tags found in '{oci_chart_ref(repository, chart)}'")
-    return show_chart(oci_chart_ref(repository, chart), tag)
+    return tag, show_chart(oci_chart_ref(repository, chart), tag)
 
 
-def get_latest_chart(chart: str, repository: str) -> dict:
+def get_latest_chart(chart: str, repository: str, current_version: str):
+    """Return (version to record, chart metadata) for the newest release."""
     if repository.startswith("https"):
-        return get_latest_https_chart(chart, repository)
+        metadata = get_latest_https_chart(chart, repository)
+        # An index is keyed by the chart version, so that is what we store.
+        try_ignore_prefix_v(metadata, current_version)
+        return metadata['version'], metadata
     if repository.startswith("oci://"):
-        return get_latest_oci_chart(chart, repository)
+        # A registry is keyed by tag, which is not always the chart version
+        # (lws tags 0.11.1 for chart v0.11.1, agentgateway tags v2.2.1). Storing
+        # anything but the tag breaks every later `helm ... --version` call.
+        return get_latest_oci_chart(chart, repository, current_version)
     print(f"Unsupported repo '{repository}' to automatically check updates, skipping.")
-    return None
+    return None, None
 
 
 def check_updates(args: str):
@@ -300,19 +324,18 @@ def check_updates(args: str):
     updates_dict = {}
     for chart, data in last_deps.items():
         try:
-            up_to_date_chart = get_latest_chart(chart, data['repository'])
+            latest_version, _ = get_latest_chart(chart, data['repository'], data['version'])
         except (subprocess.CalledProcessError, urllib.error.URLError, RuntimeError) as e:
             # One unreachable repo (private registry, outage) must not fail the whole app.
             print(f"::warning::Cannot check updates for '{chart}' in '{data['repository']}': {e}")
             continue
-        if up_to_date_chart is None:
+        if latest_version is None:
             continue
-        print(f"Last version found: {up_to_date_chart['version']}")
-        try_ignore_prefix_v(up_to_date_chart, data['version'])
-        if up_to_date_chart['version'] != data['version']:
-            print(f"::warning::Update found for '{chart}': {data['version']} -> {up_to_date_chart['version']}")
+        print(f"Last version found: {latest_version}")
+        if latest_version != data['version']:
+            print(f"::warning::Update found for '{chart}': {data['version']} -> {latest_version}")
             item = data.copy()
-            item['version'] = up_to_date_chart['version']
+            item['version'] = latest_version
             updates_list.append(item)
             updates_dict[item['name']] = item
     update_charts_cfg(args, updates_list, cfg)
@@ -328,6 +351,17 @@ def check_updates(args: str):
         update_example_chart(args, updates_dict)
 
 
+def read_known_app_versions(charts_file: str) -> dict:
+    """Map (chart name, version) -> appVersion already recorded in charts.yaml."""
+    if not os.path.exists(charts_file):
+        return {}
+    with open(charts_file, "r", encoding='utf-8') as file:
+        known = yaml.safe_load(file) or {}
+    return {(name, str(entry.get('version'))): entry.get('appVersion', '')
+            for name, entries in (known.get('charts') or {}).items()
+            for entry in entries or []}
+
+
 def generate_charts_info(app: str, cfg: dict, rewrite: bool):
     if cfg is None:
         print('Charts config not found.')
@@ -339,6 +373,7 @@ def generate_charts_info(app: str, cfg: dict, rewrite: bool):
     deps = cfg['st-charts']
     repos = dict()
     out_charts = dict()
+    known_app_versions = read_known_app_versions(charts_file)
     for data in deps:
         repo = data['dep_name']
         if data['repository'].startswith("http") and data['repository'] not in repos:
@@ -351,9 +386,17 @@ def generate_charts_info(app: str, cfg: dict, rewrite: bool):
         else:
             repo_name = repos.get(data['repository'])
         args = ["helm", "show", "chart", f"{repo_name}/{repo}", "--version", data['version']]
-        result = subprocess.run(args, check=True, capture_output=True, text=True)
-        up_to_date_chart = yaml.safe_load(result.stdout)
+        result = subprocess.run(args, check=False, capture_output=True, text=True)
         name = data['name']
+        if result.returncode != 0:
+            # Upstreams drop old versions from the index (mysql-operator keeps only
+            # the latest). Keep what we already recorded rather than failing the app.
+            previous = known_app_versions.get((name, str(data['version'])), '')
+            print(f"::warning::'{repo_name}/{repo}' no longer offers version "
+                  f"{data['version']}, keeping appVersion '{previous}'")
+            up_to_date_chart = {'appVersion': previous}
+        else:
+            up_to_date_chart = yaml.safe_load(result.stdout)
         out_chart = dict(
             version=str(data['version']),
             appVersion=up_to_date_chart.get('appVersion', '')

scripts/tests/test_chart_ctl.py

@@ -141,7 +141,7 @@ def test_check_updates_skips_generation_when_nothing_changed(monkeypatch, capsys
     monkeypatch.setattr(chart_ctl, "read_charts_cfg", lambda *a, **k: cfg)
     # Upstream reports the very version we already track.
     monkeypatch.setattr(chart_ctl, "get_latest_chart",
-                        lambda chart, repo: {"version": "1.0.0", "appVersion": "v1.0.0"})
+                        lambda chart, repo, current: ("1.0.0", {"appVersion": "v1.0.0"}))
     called = []
     monkeypatch.setattr(chart_ctl, "generate", lambda *a: called.append("generate"))
     monkeypatch.setattr(chart_ctl, "update_example_chart", lambda *a: called.append("example"))
@@ -159,7 +159,7 @@ def test_check_updates_generates_when_version_changed(monkeypatch):
                           "repository": "oci://example.com/charts"}]}
     monkeypatch.setattr(chart_ctl, "read_charts_cfg", lambda *a, **k: cfg)
     monkeypatch.setattr(chart_ctl, "get_latest_chart",
-                        lambda chart, repo: {"version": "1.1.0", "appVersion": "1.1.0"})
+                        lambda chart, repo, current: ("1.1.0", {"appVersion": "1.1.0"}))
     called = []
     monkeypatch.setattr(chart_ctl, "generate", lambda *a: called.append("generate"))
     monkeypatch.setattr(chart_ctl, "update_example_chart", lambda *a: called.append("example"))
@@ -168,3 +168,67 @@ def test_check_updates_generates_when_version_changed(monkeypatch):
     chart_ctl.check_updates(_Args("demo"))
 
     assert called == ["cfg", "generate", "example"]
+
+
+def test_get_latest_chart_oci_records_the_tag(monkeypatch):
+    """OCI is addressed by tag, which is not always the chart version."""
+    monkeypatch.setattr(chart_ctl, "oci_list_tags", lambda repo, chart: ["v2.2.0", "v2.2.1"])
+    monkeypatch.setattr(chart_ctl, "show_chart",
+                        lambda ref, version=None: {"version": "v2.2.1", "appVersion": "v2.2.1"})
+    version, _ = chart_ctl.get_latest_chart("agentgateway", "oci://cr.example.dev/charts", "1.1.0")
+    # Not '2.2.1': stripping the prefix yields a tag that does not exist.
+    assert version == "v2.2.1"
+
+
+def test_get_latest_chart_oci_keeps_tag_differing_from_chart_version(monkeypatch):
+    # lws publishes tag '0.11.1' for a chart whose version field is 'v0.11.1'.
+    monkeypatch.setattr(chart_ctl, "oci_list_tags", lambda repo, chart: ["0.11.0", "0.11.1"])
+    monkeypatch.setattr(chart_ctl, "show_chart",
+                        lambda ref, version=None: {"version": "v0.11.1", "appVersion": "v0.11.1"})
+    version, _ = chart_ctl.get_latest_chart("lws", "oci://registry.k8s.io/lws/charts", "0.7.0")
+    assert version == "0.11.1"
+
+
+def test_get_latest_chart_https_strips_prefix_v(monkeypatch):
+    monkeypatch.setattr(chart_ctl, "get_latest_https_chart",
+                        lambda chart, repo: {"version": "v1.2.3", "appVersion": "1.2.3"})
+    version, _ = chart_ctl.get_latest_chart("x", "https://example.com/charts", "1.2.2")
+    assert version == "1.2.3"
+
+
+def test_read_known_app_versions(tmp_path):
+    charts_file = tmp_path / "charts.yaml"
+    charts_file.write_text(
+        "charts:\n  mysql-operator:\n  - version: 2.2.3\n    appVersion: 9.2.0-2.2.3\n")
+    known = chart_ctl.read_known_app_versions(str(charts_file))
+    assert known[("mysql-operator", "2.2.3")] == "9.2.0-2.2.3"
+
+
+def test_read_known_app_versions_missing_file(tmp_path):
+    assert chart_ctl.read_known_app_versions(str(tmp_path / "nope.yaml")) == {}
+
+
+def test_latest_stable_tag_tiebreak_prefers_recorded_spelling():
+    # Both spellings of the same release exist; keep the one already in use.
+    assert chart_ctl.latest_stable_tag(["v1.1.0", "1.1.0"], "1.1.0") == "1.1.0"
+    assert chart_ctl.latest_stable_tag(["1.1.0", "v1.1.0"], "v1.1.0") == "v1.1.0"
+
+
+def test_latest_stable_tag_tiebreak_is_order_independent():
+    # The registry may return tags in any order; the pick must not depend on it.
+    assert (chart_ctl.latest_stable_tag(["v1.1.0", "1.1.0"], "0.9.0")
+            == chart_ctl.latest_stable_tag(["1.1.0", "v1.1.0"], "0.9.0"))
+
+
+def test_latest_stable_tag_tiebreak_follows_current_prefix_style():
+    # Not the recorded version itself, but the same style, so the entry stays consistent.
+    assert chart_ctl.latest_stable_tag(["1.2.0", "v1.2.0"], "v1.1.0") == "v1.2.0"
+    assert chart_ctl.latest_stable_tag(["v1.2.0", "1.2.0"], "1.1.0") == "1.2.0"
+
+
+def test_latest_stable_tag_tiebreak_without_current_version():
+    assert chart_ctl.latest_stable_tag(["v1.1.0", "1.1.0"]) == "1.1.0"
+
+
+def test_latest_stable_tag_tiebreak_does_not_beat_a_higher_version():
+    assert chart_ctl.latest_stable_tag(["1.1.0", "v2.0.0"], "1.1.0") == "v2.0.0"