fix: better checking of affected tasks for e2e (#785)

find-affected-tasks for e2e tests was just looking for strings in the
task yamls. This causes issues - an example is that updating the
tekton.py helper would trigger all tests because the string tekton is in
the apiVersion string in the task yamls. This fixes that to only check
certain parts of the yaml and have smarter checking when it does.

Assisted-By: Cursor

Signed-off-by: Johnny Bieren <jbieren@redhat.com>

Author: johnbieren

integration-tests/lib/find_catalog_suite_from_utils_diff.py

@@ -6,8 +6,9 @@
 1. Build search tokens from changed paths using the utils ``Dockerfile``
    (``COPY`` into ``/home``, ``PATH``) via :mod:`find_search_tokens_from_dockerfile`
    (the process cwd must be the utils repo root so ``./Dockerfile`` exists).
-2. Search catalog ``tasks/**/*.yaml`` for those search tokens (skips
-   ``tasks/**/tests/`` fixture YAML).
+2. Search catalog Task step ``command`` / ``args`` / ``script`` fields for those
+   tokens (skips ``tasks/**/tests/`` fixture YAML and Task metadata such as
+   ``apiVersion: tekton.dev``).
 3. Source catalog's ``find_release_pipelines_from_pr.sh`` and run
    ``_catalog_stdin_task_paths_to_testcase_tokens`` (same mapping as catalog PR
    tooling).
@@ -42,6 +43,8 @@
 import sys
 from pathlib import Path
 
+import yaml
+
 import find_search_tokens_from_dockerfile as fts
 import helper_task_import_graph as htig
 
@@ -153,6 +156,48 @@ def _changed_paths_trigger_global_catalog_run(changed: list[str]) -> bool:
     return False
 
 
+def _step_invocation_chunks(step: object) -> list[str]:
+    """Collect command/args/script strings from one Tekton Task step dict."""
+    if not isinstance(step, dict):
+        return []
+    chunks: list[str] = []
+    for key in ("command", "script"):
+        val = step.get(key)
+        if isinstance(val, str) and val:
+            chunks.append(val)
+        elif isinstance(val, list):
+            chunks.extend(str(item) for item in val if item is not None)
+    args = step.get("args")
+    if isinstance(args, list):
+        chunks.extend(str(item) for item in args if item is not None)
+    elif isinstance(args, str) and args:
+        chunks.append(args)
+    return chunks
+
+
+def _extract_task_step_invocation_text(task_yaml_text: str) -> str:
+    """Return concatenated step command/args/script text from a Task YAML document.
+
+    Ignores ``apiVersion``, annotations, and other metadata so tokens like ``tekton``
+    do not match every Task file.
+    """
+    try:
+        doc = yaml.safe_load(task_yaml_text)
+    except yaml.YAMLError:
+        return ""
+    if not isinstance(doc, dict):
+        return ""
+    spec = doc.get("spec")
+    if not isinstance(spec, dict):
+        return ""
+    chunks: list[str] = []
+    steps = spec.get("steps")
+    if isinstance(steps, list):
+        for step in steps:
+            chunks.extend(_step_invocation_chunks(step))
+    return "\n".join(chunks)
+
+
 def _is_under_task_tests_dir(path: Path, tasks_root: Path) -> bool:
     """Return whether ``path`` is under a ``tasks/.../tests/`` directory.
 
@@ -167,11 +212,12 @@ def _is_under_task_tests_dir(path: Path, tasks_root: Path) -> bool:
 
 
 def _find_tasks_referencing_search_tokens(catalog: Path, search_tokens: set[str]) -> set[str]:
-    """Find Task YAML files whose content mentions any search token.
+    """Find Task YAML files whose step invocations mention any search token.
 
     Search tokens are in-container paths (e.g. ``/home/pyxis/foo.py``) or command
     tokens (e.g. ``create_container_image``) from
-    :mod:`find_search_tokens_from_dockerfile`. We walk ``catalog/tasks``, skip
+    :mod:`find_search_tokens_from_dockerfile`. Only ``spec.steps`` ``command``,
+    ``args``, and ``script`` fields are searched. We walk ``catalog/tasks``, skip
     ``tasks/**/tests/**``, require ``kind: Task``, and return paths relative to
     ``catalog`` for any substring match.
     """
@@ -189,8 +235,11 @@ def _find_tasks_referencing_search_tokens(catalog: Path, search_tokens: set[str]
         # Ignore non-Task files (e.g. Pipeline snippets) that might live under tasks/.
         if not re.search(r"kind:\s*Task\b", text):
             continue
+        invocation_text = _extract_task_step_invocation_text(text)
+        if not invocation_text:
+            continue
         for token in search_tokens:
-            if token in text:
+            if token in invocation_text:
                 found.add(task_yaml.relative_to(catalog).as_posix())
                 break
     return found

integration-tests/lib/find_search_tokens_from_dockerfile.py

@@ -18,6 +18,8 @@
     r"^COPY\s+(?!--from=)(\S+)\s+(/home/\S+)\s*(?:#.*)?$",
     re.IGNORECASE,
 )
+# ``ENV PYTHONPATH`` also contains the substring PATH; match only executable PATH.
+_ENV_PATH = re.compile(r"^ENV\s+PATH\s*=", re.IGNORECASE)
 
 
 @dataclass(frozen=True)
@@ -50,13 +52,11 @@ def parse_dockerfile_home_layout(dockerfile_text: str) -> UtilsImageHomeLayout:
             continue
         segment_to_home[src] = dest.rstrip("/")
 
-    # Second pass: PATH adds e.g. /home/pyxis for basename search tokens.
+    # Second pass: ``ENV PATH=`` adds e.g. /home/pyxis for basename search tokens.
     path_dirs: set[str] = set()
     for line in dockerfile_text.splitlines():
         raw = line.split("#", 1)[0].strip()
-        if not raw.upper().startswith("ENV "):
-            continue
-        if "PATH" not in raw:
+        if not _ENV_PATH.match(raw):
             continue
         for hm in re.finditer(r"/home/[a-zA-Z0-9_.-]+", raw):
             path_dirs.add(hm.group(0))

integration-tests/lib/tests/test_find_catalog_suite_from_utils_diff.py

@@ -167,13 +167,135 @@ def test_find_tasks_referencing_search_tokens_skips_tests_and_non_task(
     fixture.parent.mkdir(parents=True, exist_ok=True)
     not_task.parent.mkdir(parents=True, exist_ok=True)
     path = "/home/scripts/utils/foo.sh"
-    good.write_text(f"kind: Task\nscript: {path}\n", encoding="utf-8")
+    good.write_text(
+        "kind: Task\nspec:\n  steps:\n    - name: run\n      script: |\n        "
+        + path
+        + "\n",
+        encoding="utf-8",
+    )
     fixture.write_text(f"kind: Task\n{path}\n", encoding="utf-8")
     not_task.write_text(f"kind: Pipeline\n{path}\n", encoding="utf-8")
     found = fc._find_tasks_referencing_search_tokens(tmp_path, {path})
     assert found == {"tasks/managed/t/task.yaml"}
 
 
+def test_find_tasks_referencing_search_tokens_ignores_tekton_in_metadata(
+    tmp_path: Path,
+) -> None:
+    """Do not match ``tekton`` from ``apiVersion`` when step scripts do not use it."""
+    tasks = tmp_path / "tasks"
+    with_cmd = tasks / "internal" / "embargo" / "task.yaml"
+    other = tasks / "managed" / "other" / "task.yaml"
+    with_cmd.parent.mkdir(parents=True, exist_ok=True)
+    other.parent.mkdir(parents=True, exist_ok=True)
+    script_path = "/home/scripts/python/tasks/internal/check_embargoed_cves.py"
+    with_cmd.write_text(
+        f"""\
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  name: check-embargoed-cves
+spec:
+  steps:
+    - name: run
+      command: ["{script_path}"]
+""",
+        encoding="utf-8",
+    )
+    other.write_text(
+        """\
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  name: other
+spec:
+  steps:
+    - name: noop
+      script: |
+        echo hello
+""",
+        encoding="utf-8",
+    )
+    found = fc._find_tasks_referencing_search_tokens(tmp_path, {"tekton"})
+    assert found == set()
+    found_path = fc._find_tasks_referencing_search_tokens(tmp_path, {script_path})
+    assert found_path == {"tasks/internal/embargo/task.yaml"}
+
+
+def test_extract_task_step_invocation_text_collects_command_args_script() -> None:
+    """Gather only step invocation fields from parsed Task YAML."""
+    text = """\
+kind: Task
+spec:
+  steps:
+    - name: a
+      command: ["/home/pyxis/foo.py"]
+      args: ["--verbose"]
+    - name: b
+      script: |
+        /home/utils/bar.sh
+"""
+    blob = fc._extract_task_step_invocation_text(text)
+    assert "/home/pyxis/foo.py" in blob
+    assert "--verbose" in blob
+    assert "/home/utils/bar.sh" in blob
+
+
+def test_step_invocation_chunks_non_dict_step_returns_empty() -> None:
+    """Non-dict step entries (malformed YAML) yield no invocation chunks."""
+    assert fc._step_invocation_chunks("not-a-step") == []
+    assert fc._step_invocation_chunks(None) == []
+
+
+def test_step_invocation_chunks_string_args() -> None:
+    """``args`` may be a single string instead of a list."""
+    chunks = fc._step_invocation_chunks(
+        {"command": ["/home/pyxis/run.py"], "args": "--dry-run"}
+    )
+    assert chunks == ["/home/pyxis/run.py", "--dry-run"]
+
+
+def test_extract_task_step_invocation_text_invalid_yaml_returns_empty() -> None:
+    """Invalid YAML returns empty invocation text."""
+    assert fc._extract_task_step_invocation_text("kind: Task\nsteps: [\n") == ""
+
+
+def test_extract_task_step_invocation_text_non_dict_document_returns_empty() -> None:
+    """YAML documents that are not mappings return empty invocation text."""
+    assert fc._extract_task_step_invocation_text("- item\n") == ""
+
+
+def test_extract_task_step_invocation_text_non_dict_spec_returns_empty() -> None:
+    """A Task with a non-mapping ``spec`` returns empty invocation text."""
+    text = "kind: Task\nspec: not-a-mapping\n"
+    assert fc._extract_task_step_invocation_text(text) == ""
+
+
+def test_find_tasks_referencing_search_tokens_skips_empty_invocation_text(
+    tmp_path: Path,
+) -> None:
+    """Tasks with no step command/args/script are skipped even if metadata matches."""
+    tasks = tmp_path / "tasks"
+    empty_steps = tasks / "managed" / "empty" / "task.yaml"
+    empty_steps.parent.mkdir(parents=True, exist_ok=True)
+    script = "/home/scripts/python/tasks/internal/check_embargoed_cves.py"
+    empty_steps.write_text(
+        f"""\
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  name: empty
+  annotations:
+    description: {script}
+spec:
+  steps: []
+""",
+        encoding="utf-8",
+    )
+    found = fc._find_tasks_referencing_search_tokens(tmp_path, {script})
+    assert found == set()
+
+
 def test_find_tasks_referencing_search_tokens_returns_empty_without_tasks_root(
     tmp_path: Path,
 ) -> None:
@@ -186,7 +308,10 @@ def test_find_tasks_referencing_search_tokens_skips_unreadable_yaml(tmp_path: Pa
     """Skip a task YAML file when read_text raises OSError."""
     task_yaml = tmp_path / "tasks" / "managed" / "t" / "task.yaml"
     task_yaml.parent.mkdir(parents=True, exist_ok=True)
-    task_yaml.write_text("kind: Task\nscript: /home/scripts/x.sh\n", encoding="utf-8")
+    task_yaml.write_text(
+        "kind: Task\nspec:\n  steps:\n    - name: run\n      script: /home/scripts/x.sh\n",
+        encoding="utf-8",
+    )
     with patch.object(Path, "read_text", side_effect=OSError("boom")):
         found = fc._find_tasks_referencing_search_tokens(tmp_path, {"/home/scripts/x.sh"})
     assert found == set()

integration-tests/lib/tests/test_find_search_tokens_from_dockerfile.py

@@ -23,6 +23,25 @@ def test_parse_dockerfile_home_layout_maps_copy_and_path() -> None:
     assert "/home/pyxis" in layout.path_home_dirs
 
 
+def test_parse_dockerfile_ignores_pythonpath_env() -> None:
+    """``ENV PYTHONPATH`` must not be treated as executable ``PATH``."""
+    text = """
+COPY scripts /home/scripts
+ENV PATH="$PATH:/home/pyxis"
+ENV PYTHONPATH="/home:/home/scripts/python/helpers:/home/scripts/python/tasks/internal"
+"""
+    layout = fts.parse_dockerfile_home_layout(text)
+    assert "/home/pyxis" in layout.path_home_dirs
+    assert "/home/scripts" not in layout.path_home_dirs
+
+
+def test_search_tokens_scripts_py_no_stem_when_scripts_not_on_path() -> None:
+    """``scripts/**/*.py`` only get a full path token when ``/home/scripts`` is not on PATH."""
+    layout = fts.parse_dockerfile_home_layout(MINIMAL_UTILS_DOCKERFILE)
+    tokens = fts.search_tokens_for_repo_path("scripts/python/helpers/tekton.py", layout)
+    assert tokens == frozenset({"/home/scripts/python/helpers/tekton.py"})
+
+
 def test_parse_skips_copy_from_stage() -> None:
     """``COPY --from=`` lines do not define repo layout."""
     text = """