Skip to content

Commit 733616a

Browse files
committed
fix(ci): report malformed CTK redistrib metadata instead of tracebacking
fetch_ctk_redistrib.main() wraps everything in a deliberate handler: except (ValueError, KeyError, OSError, urllib.error.URLError, json.JSONDecodeError) as exc: print(f"ERROR: {exc}", file=sys.stderr) return 1 Several malformed-manifest shapes escape it because the guards check for absence, not type. 1. Real redistrib_*.json files carry string-valued top-level keys -- "release_date", "release_label", "release_product" -- next to the component objects. `metadata.get(component)` returns a str for those, and `component_info is None` does not reject it, so `component_info.get(...)` raises `AttributeError: 'str' object has no attribute 'get'`. 2. The manifest is downloaded with `curl -LSs` and no `--fail` (.github/actions/fetch_ctk/action.yml), so an error page or redirect body is written to the file. If that body is valid JSON but not an object, the failure surfaces frames later as `TypeError: argument of type 'NoneType' is not iterable` or `AttributeError: 'list' object has no attribute 'get'`. 3. A subdir entry that is a bare string rather than an object raises the same AttributeError from a different line. Reproduced via `main(argv)` with `--metadata-path`, so no network: --component release_label -> AttributeError: 'str' object has no attribute 'get' metadata is null -> TypeError: argument of type 'NoneType' is not iterable metadata is a JSON array -> AttributeError: 'list' object has no attribute 'get' subdir entry is a string -> AttributeError: 'str' object has no attribute 'get' An absent component, by contrast, already returns 1 with a clear message. Validate the manifest is a JSON object where it is loaded, and check the component and subdir entries are objects before reaching into them. Also replace `ctk_subdir in metadata.get(resolved_component, {})` in filter_components with an explicit dict check: on a string value that `in` silently becomes a substring test rather than a key lookup. Adds the first tests for this tool.
1 parent 3bd069a commit 733616a

2 files changed

Lines changed: 149 additions & 3 deletions

File tree

ci/tools/fetch_ctk_redistrib.py

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,25 @@ def load_metadata(*, metadata_path: str | None, metadata_url: str | None) -> dic
7070
raise ValueError("exactly one of --metadata-path or --metadata-url is required")
7171

7272
if metadata_path is not None:
73-
return json.loads(Path(metadata_path).read_text(encoding="utf-8"))
73+
return _as_metadata_object(json.loads(Path(metadata_path).read_text(encoding="utf-8")), metadata_path)
7474

7575
assert metadata_url is not None
7676
metadata_url = validate_metadata_url(metadata_url)
7777
with urllib.request.urlopen(metadata_url) as response: # noqa: S310 - scheme is restricted to https above
78-
return json.load(response)
78+
return _as_metadata_object(json.load(response), metadata_url)
79+
80+
81+
def _as_metadata_object(metadata: Any, source: str) -> dict[str, Any]:
82+
"""Reject JSON that parsed fine but is not a redistrib manifest.
83+
84+
The manifest is downloaded with ``curl -LSs`` (no ``--fail``), so an error
85+
page or a redirect body lands in the file and may still be valid JSON --
86+
just not an object. Without this the failure surfaces several frames later
87+
as ``TypeError: argument of type 'NoneType' is not iterable``.
88+
"""
89+
if not isinstance(metadata, dict):
90+
raise ValueError(f"CTK redistrib metadata from {source} must be a JSON object, got {type(metadata).__name__}")
91+
return metadata
7992

8093

8194
def resolve_component_name(metadata: dict[str, Any], component: str) -> str:
@@ -101,7 +114,11 @@ def filter_components(
101114
skipped = []
102115
for component in filter_static_components(split_components(components), host_platform, cuda_version):
103116
resolved_component = resolve_component_name(metadata, component)
104-
if ctk_subdir in metadata.get(resolved_component, {}):
117+
# Guard the type: a top-level key such as "release_label" holds a
118+
# string, and ``ctk_subdir in "13.0.0"`` is a substring test rather
119+
# than the intended key lookup.
120+
component_info = metadata.get(resolved_component)
121+
if isinstance(component_info, dict) and ctk_subdir in component_info:
105122
filtered.append(resolved_component)
106123
else:
107124
skipped.append(component)
@@ -114,10 +131,22 @@ def get_component_relative_path(metadata: dict[str, Any], *, host_platform: str,
114131
component_info = metadata.get(component)
115132
if component_info is None:
116133
raise KeyError(f"unknown CTK component {component!r}")
134+
if not isinstance(component_info, dict):
135+
# Real manifests carry string-valued top-level keys ("release_date",
136+
# "release_label", "release_product") alongside the component objects,
137+
# so "present" is not the same as "is a component".
138+
raise KeyError(
139+
f"CTK metadata entry {component!r} is not a component object (got {type(component_info).__name__})"
140+
)
117141

118142
subdir_info = component_info.get(ctk_subdir)
119143
if subdir_info is None:
120144
raise KeyError(f"CTK component {component!r} is not available for redistrib subdir {ctk_subdir!r}")
145+
if not isinstance(subdir_info, dict):
146+
raise KeyError(
147+
f"CTK component {component!r} entry for redistrib subdir {ctk_subdir!r} "
148+
f"is not an object (got {type(subdir_info).__name__})"
149+
)
121150

122151
relative_path = subdir_info.get("relative_path")
123152
if relative_path is None:
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
from __future__ import annotations
5+
6+
import json
7+
import os
8+
import sys
9+
10+
import pytest
11+
12+
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
13+
from fetch_ctk_redistrib import main
14+
15+
# Shaped like a real redistrib_*.json: string-valued release keys sit at the
16+
# top level alongside the component objects.
17+
METADATA = {
18+
"release_date": "2026-01-01",
19+
"release_label": "13.0.0",
20+
"release_product": "cuda",
21+
"cuda_nvcc": {
22+
"linux-x86_64": {"relative_path": "cuda_nvcc/linux-x86_64/cuda_nvcc-linux-x86_64.tar.xz"},
23+
},
24+
}
25+
26+
27+
def write_metadata(tmp_path, payload):
28+
path = tmp_path / "redistrib.json"
29+
path.write_text(json.dumps(payload), encoding="utf-8")
30+
return str(path)
31+
32+
33+
def relpath_argv(metadata_path, component):
34+
return [
35+
"component-relative-path",
36+
"--host-platform",
37+
"linux-64",
38+
"--component",
39+
component,
40+
"--metadata-path",
41+
metadata_path,
42+
]
43+
44+
45+
def filter_argv(metadata_path, components="cuda_nvcc"):
46+
return [
47+
"filter-components",
48+
"--host-platform",
49+
"linux-64",
50+
"--cuda-version",
51+
"13.0.0",
52+
"--components",
53+
components,
54+
"--metadata-path",
55+
metadata_path,
56+
]
57+
58+
59+
@pytest.mark.agent_authored(model="claude-opus-5")
60+
def test_valid_component_is_resolved(tmp_path, capsys):
61+
assert main(relpath_argv(write_metadata(tmp_path, METADATA), "cuda_nvcc")) == 0
62+
assert capsys.readouterr().out.strip() == "cuda_nvcc/linux-x86_64/cuda_nvcc-linux-x86_64.tar.xz"
63+
64+
65+
@pytest.mark.agent_authored(model="claude-opus-5")
66+
@pytest.mark.parametrize("component", ["release_label", "release_date", "release_product"])
67+
def test_string_valued_top_level_key_is_not_a_component(tmp_path, capsys, component):
68+
"""`is None` only rejects an absent key, not a wrongly-typed one.
69+
70+
Every real manifest carries these string-valued keys next to the component
71+
objects, so asking for one used to reach `component_info.get(...)` and die
72+
with `AttributeError: 'str' object has no attribute 'get'` instead of the
73+
tool's own diagnostic.
74+
"""
75+
assert main(relpath_argv(write_metadata(tmp_path, METADATA), component)) == 1
76+
assert "ERROR:" in capsys.readouterr().err
77+
78+
79+
@pytest.mark.agent_authored(model="claude-opus-5")
80+
def test_absent_component_still_reports_cleanly(tmp_path, capsys):
81+
assert main(relpath_argv(write_metadata(tmp_path, METADATA), "not_a_component")) == 1
82+
assert "unknown CTK component" in capsys.readouterr().err
83+
84+
85+
@pytest.mark.agent_authored(model="claude-opus-5")
86+
def test_non_object_subdir_entry_is_reported(tmp_path, capsys):
87+
metadata = {"cuda_nvcc": {"linux-x86_64": "cuda_nvcc/linux-x86_64/x.tar.xz"}}
88+
assert main(relpath_argv(write_metadata(tmp_path, metadata), "cuda_nvcc")) == 1
89+
assert "ERROR:" in capsys.readouterr().err
90+
91+
92+
@pytest.mark.agent_authored(model="claude-opus-5")
93+
@pytest.mark.parametrize(
94+
"payload",
95+
[
96+
pytest.param(None, id="null"),
97+
pytest.param([1, 2], id="array"),
98+
pytest.param("13.0.0", id="string"),
99+
],
100+
)
101+
@pytest.mark.parametrize("argv_builder", [relpath_argv, filter_argv], ids=["relative-path", "filter"])
102+
def test_metadata_that_is_not_an_object_is_reported(tmp_path, capsys, payload, argv_builder):
103+
"""The manifest is downloaded with `curl -LSs` (no --fail), so an error
104+
page or redirect body can parse as valid JSON that is not an object."""
105+
path = write_metadata(tmp_path, payload)
106+
argv = argv_builder(path, "cuda_nvcc") if argv_builder is relpath_argv else argv_builder(path)
107+
108+
assert main(argv) == 1
109+
assert "must be a JSON object" in capsys.readouterr().err
110+
111+
112+
@pytest.mark.agent_authored(model="claude-opus-5")
113+
def test_filter_skips_a_string_valued_top_level_key(tmp_path, capsys):
114+
assert main(filter_argv(write_metadata(tmp_path, METADATA), "release_label")) == 0
115+
captured = capsys.readouterr()
116+
assert captured.out.strip() == ""
117+
assert "Skipping unsupported CTK component 'release_label'" in captured.err

0 commit comments

Comments
 (0)