From 6a1f492c5002be458d28683c02bf1d61155ba69e Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 17 Apr 2026 13:26:58 -0700 Subject: [PATCH 1/3] Add dda inv github.bump-rshell task for automated rshell version bumps --- tasks/github_tasks.py | 203 +++++++++++++++++++++++++ tasks/unit_tests/github_tasks_tests.py | 168 +++++++++++++++++++- 2 files changed, 370 insertions(+), 1 deletion(-) diff --git a/tasks/github_tasks.py b/tasks/github_tasks.py index 57672c3e3b97..bd51bbbe5292 100644 --- a/tasks/github_tasks.py +++ b/tasks/github_tasks.py @@ -2,6 +2,7 @@ import json import os +import re from collections import Counter from invoke.context import Context @@ -689,3 +690,205 @@ def check_permissions( MAX_BLOCKS = 50 for idx in range(0, len(blocks), MAX_BLOCKS): client.chat_postMessage(channel=channel, blocks=blocks[idx : idx + MAX_BLOCKS], text=message) + + +RSHELL_MODULE = "github.com/DataDog/rshell" +_RSHELL_SEMVER_RE = re.compile(r"^v(\d+)\.(\d+)\.(\d+)$") + + +def _latest_rshell_semver_tag(): + """Return the highest `vX.Y.Z` tag on DataDog/rshell. + + Uses the git tags API rather than `releases/latest` because tags are the + source of truth: a Release object only exists once release.yml has run + to completion. + """ + from tasks.libs.ciproviders.github_api import GithubAPI + + rshell_gh = GithubAPI(repository="DataDog/rshell") + candidates = [] + for tag in rshell_gh._repository.get_tags(): + m = _RSHELL_SEMVER_RE.match(tag.name) + if m: + candidates.append((tag.name, tuple(int(g) for g in m.groups()))) + if not candidates: + raise Exit( + color_message("No semver tags found on DataDog/rshell", Color.RED), + code=1, + ) + return max(candidates, key=lambda pair: pair[1])[0] + + +def _current_rshell_version(ctx): + """Return the rshell version pinned in go.mod's require block, or None.""" + res = ctx.run("go mod edit -json", hide=True) + data = json.loads(res.stdout) + for req in data.get("Require") or []: + if req.get("Path") == RSHELL_MODULE: + return req.get("Version") + return None + + +def _drop_rshell_replaces(ctx): + """Remove every rshell `replace` directive in go.mod, in every valid form. + + Uses `go mod edit -dropreplace` (Go's own tool) rather than regex so we + handle single-line, versioned single-line, and block-form entries + uniformly. + """ + res = ctx.run("go mod edit -json", hide=True) + data = json.loads(res.stdout) + for entry in data.get("Replace") or []: + old = entry.get("Old", {}) + if old.get("Path") != RSHELL_MODULE: + continue + spec = RSHELL_MODULE + if old.get("Version"): + spec = f"{RSHELL_MODULE}@{old['Version']}" + ctx.run(f"go mod edit -dropreplace={spec}", hide=True) + + +def _classify_rshell_prs(gh, branch): + """Return (open, closed_unmerged, merged) PRs for the given head branch.""" + all_prs = list(gh._repository.get_pulls(state="all", head=f"DataDog:{branch}")) + open_prs = [p for p in all_prs if p.state == "open"] + closed_unmerged = [p for p in all_prs if p.state == "closed" and not p.merged] + merged = [p for p in all_prs if p.merged] + return open_prs, closed_unmerged, merged + + +@task +def bump_rshell(ctx, version=None, draft=True): + """ + Bump github.com/DataDog/rshell and open a draft PR on datadog-agent. + + With no --version flag, auto-discovers the latest semver tag on + DataDog/rshell. Explicit --version overrides that discovery (useful for + debugging or pinning to a non-latest release). + + Invoked from DataDog/rshell's GitLab bump pipeline against a fresh clone of + datadog-agent. Expects GITHUB_TOKEN in the environment (minted upstream + via dd-octo-sts with contents:write + pull-requests:write on + DataDog/datadog-agent). + """ + from tasks.libs.ciproviders.github_api import GithubAPI + from tasks.libs.common.git import check_clean_branch_state + from tasks.libs.common.utils import set_gitconfig_in_ci + + if version is None: + version = _latest_rshell_semver_tag() + print(color_message(f"Auto-discovered latest rshell version: {version}", Color.BLUE)) + elif not _RSHELL_SEMVER_RE.match(version): + raise Exit( + color_message(f"Invalid version {version!r}; expected vX.Y.Z", Color.RED), + code=1, + ) + else: + print(color_message(f"Using explicit rshell version: {version}", Color.BLUE)) + + gh = GithubAPI() + branch = f"bump-rshell-{version}" + + open_prs, closed_unmerged, merged_prs = _classify_rshell_prs(gh, branch) + if open_prs: + print(color_message(f"Open PR already exists: {open_prs[0].html_url}; nothing to do", Color.GREEN)) + return + if merged_prs: + print( + color_message( + f"Prior PR merged ({merged_prs[0].html_url}); " + "proceeding — tree diff will decide if there's anything to do", + Color.BLUE, + ) + ) + + # Scrub the token so `ctx.run` subprocesses (go, git, dda inv tidy) don't + # inherit a write-scoped credential. GithubAPI still holds its auth + # object; `git push` gets the token via the remote URL rewrite below. + os.environ.pop("GITHUB_TOKEN", None) + + previous_version = _current_rshell_version(ctx) + print(color_message(f"Current pinned rshell version: {previous_version or ''}", Color.BLUE)) + if previous_version == version: + print(color_message(f"datadog-agent already pins rshell at {version}; nothing to do", Color.GREEN)) + return + + _drop_rshell_replaces(ctx) + ctx.run(f"go get {RSHELL_MODULE}@{version}") + ctx.run("dda inv tidy") + + ctx.run("git add -A") + diff = ctx.run("git diff --cached --quiet", warn=True, hide=True) + if diff.ok: + print( + color_message( + f"No staged changes after bump; datadog-agent already resolves rshell to {version}", + Color.GREEN, + ) + ) + return + + # Release note via reno. + res = ctx.run(f"reno new bump-rshell-{version}", hide=True) + note_path = res.stdout.strip().split()[-1] + with open(note_path, "w") as f: + f.write( + "---\n" "enhancements:\n" " - |\n" f" Bump ``rshell`` to {version} for the Private Action Runner.\n" + ) + ctx.run(f"git add {note_path}") + + set_gitconfig_in_ci(ctx) + try: + check_clean_branch_state(ctx, gh, branch) + except Exit as e: + if "already exists locally" not in str(e): + raise + + ctx.run(f"git switch -c {branch}", warn=True, hide=True) + commit_msg = ( + f"Bump rshell dependency from {previous_version} to {version}" + if previous_version + else f"Bump rshell dependency to {version}" + ) + ctx.run(f'git commit -m "{commit_msg}" --no-verify') + + # Wire the token into the remote URL for push auth (mirrors + # tasks/pipeline.py:693). `hide=True` keeps it out of invoke's output. + ctx.run( + f"git remote set-url origin " f"https://x-access-token:{gh._auth.token}@github.com/DataDog/datadog-agent.git", + hide=True, + ) + # Force-push is safe: branch name is bot-exclusive. + ctx.run(f"git push --force -u origin {branch} --no-verify") + + pr_title = f"[automated] Bump rshell to {version}" + pr_body = ( + f"Automated bump of `{RSHELL_MODULE}` to " + f"[{version}](https://github.com/DataDog/rshell/releases/tag/{version}).\n" + ) + if closed_unmerged: + pr = closed_unmerged[0] + print(color_message(f"Reopening prior closed PR: {pr.html_url}", Color.BLUE)) + pr.edit(state="open", title=pr_title, body=pr_body) + else: + pr = gh.create_pr( + pr_title=pr_title, + pr_body=pr_body, + base_branch="main", + target_branch=branch, + draft=draft, + ) + print(color_message(f"Opened {'draft ' if draft else ''}PR: {pr.html_url}", Color.GREEN)) + + from github import GithubException + + for label in ("changelog/no-changelog", "ask-review"): + try: + gh.add_pr_label(pr.number, label) + except GithubException as e: + print(color_message(f"Warning: failed to add label {label!r}: {e}", Color.ORANGE)) + + try: + pr.create_review_request(team_reviewers=["action-platform"]) + except GithubException as e: + print(color_message(f"Warning: failed to request review from @DataDog/action-platform: {e}", Color.ORANGE)) diff --git a/tasks/unit_tests/github_tasks_tests.py b/tasks/unit_tests/github_tasks_tests.py index 9005cbe364f6..bd4527a9dfe8 100644 --- a/tasks/unit_tests/github_tasks_tests.py +++ b/tasks/unit_tests/github_tasks_tests.py @@ -1,8 +1,10 @@ from __future__ import annotations +import json +import os import unittest from dataclasses import dataclass -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import MagicMock, Mock, mock_open, patch from invoke.context import Context @@ -704,3 +706,167 @@ def test_return_all_teams_when_best_teams_only_false(self): _get_teams(changed_files, owners_file=self.CODEOWNERS_FILE, best_teams_only=False), ['@datadog/team-a', '@datadog/team-b', '@datadog/team-doc'], ) + + +def _fake_tag(name): + tag = MagicMock() + tag.name = name + return tag + + +class TestLatestRshellSemverTag(unittest.TestCase): + def test_picks_highest_semver_and_excludes_prereleases(self): + fake_tags = [_fake_tag(n) for n in ["v0.0.9", "v0.0.10", "v0.0.11", "v1.0.0-rc1", "main"]] + with patch('tasks.libs.ciproviders.github_api.GithubAPI') as gh_mock: + gh_instance = MagicMock() + gh_instance._repository.get_tags.return_value = fake_tags + gh_mock.return_value = gh_instance + result = tasks.github_tasks._latest_rshell_semver_tag() + self.assertEqual(result, "v0.0.11") + + def test_sorts_numerically_not_lexicographically(self): + # v0.0.9 > v0.0.11 as strings, but we want v0.0.11 as the higher version. + fake_tags = [_fake_tag(n) for n in ["v0.0.9", "v0.0.11"]] + with patch('tasks.libs.ciproviders.github_api.GithubAPI') as gh_mock: + gh_instance = MagicMock() + gh_instance._repository.get_tags.return_value = fake_tags + gh_mock.return_value = gh_instance + self.assertEqual(tasks.github_tasks._latest_rshell_semver_tag(), "v0.0.11") + + def test_raises_when_no_semver_tags(self): + fake_tags = [_fake_tag("main"), _fake_tag("v1.0.0-rc1"), _fake_tag("not-a-tag")] + with patch('tasks.libs.ciproviders.github_api.GithubAPI') as gh_mock: + gh_instance = MagicMock() + gh_instance._repository.get_tags.return_value = fake_tags + gh_mock.return_value = gh_instance + with self.assertRaises(Exit): + tasks.github_tasks._latest_rshell_semver_tag() + + +class TestBumpRshell(unittest.TestCase): + """Integration-ish tests: mock GithubAPI, ctx.run, and git helpers; verify flow.""" + + def _make_ctx(self, go_mod_json=None): + """Return a real invoke Context plus a Mock for its `run` method that serves canned + stdout per command. Using a real Context is required because the @task decorator + rejects non-Context first args. + """ + ctx = Context() + go_mod_json = go_mod_json or {"Require": [], "Replace": []} + + def run(cmd, *_args, **_kwargs): + result = MagicMock() + result.ok = True + if "go mod edit -json" in cmd: + result.stdout = json.dumps(go_mod_json) + elif "reno new" in cmd: + result.stdout = "Created new notes file in releasenotes/notes/bump-rshell-v0.0.11-abc.yaml\n" + elif "git diff --cached --quiet" in cmd: + result.ok = False # non-zero exit = there ARE changes + else: + result.stdout = "" + return result + + ctx.run = MagicMock(side_effect=run) + return ctx + + def _patch_github_api(self, open_prs=None, closed_unmerged=None, merged=None, rshell_tags=None, created_pr=None): + """Return a patch/context that replaces GithubAPI with a mock serving these PRs/tags.""" + open_prs = open_prs or [] + closed_unmerged = closed_unmerged or [] + merged = merged or [] + rshell_tags = rshell_tags or [_fake_tag("v0.0.11")] + + def gh_factory(*_args, **kwargs): + instance = MagicMock() + instance._auth.token = "fake-token" + if kwargs.get("repository") == "DataDog/rshell": + instance._repository.get_tags.return_value = rshell_tags + else: + instance._repository.get_pulls.return_value = open_prs + closed_unmerged + merged + if created_pr is not None: + instance.create_pr.return_value = created_pr + else: + new_pr = MagicMock() + new_pr.number = 42 + new_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/42" + instance.create_pr.return_value = new_pr + return instance + + return patch('tasks.libs.ciproviders.github_api.GithubAPI', side_effect=gh_factory) + + def test_rejects_invalid_explicit_version(self): + ctx = Context() + for bad in ("v0.0", "v1.2.3-rc1", "0.0.11"): + with self.subTest(version=bad): + with self.assertRaises(Exit): + tasks.github_tasks.bump_rshell(ctx, version=bad) + + def test_noop_when_open_pr_exists(self): + open_pr = MagicMock(state="open", merged=False) + open_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/999" + ctx = self._make_ctx() + with self._patch_github_api(open_prs=[open_pr]): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.99") + # Nothing should have been done after classification — no git, no go, no tidy. + commands = [call.args[0] for call in ctx.run.call_args_list] + self.assertFalse(any(c.startswith(("go ", "git ", "dda ", "reno ")) for c in commands)) + + def test_noop_when_already_pinned(self): + ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.11"}]}) + with patch.dict(os.environ, {"GITHUB_TOKEN": "t"}, clear=False): + with self._patch_github_api(): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.11") + # The task should return after the already-pinned check; `go get` / `dda inv tidy` never run. + commands = [call.args[0] for call in ctx.run.call_args_list] + self.assertFalse(any(c.startswith(("go get", "dda inv tidy", "reno new")) for c in commands)) + + def test_token_scrubbed_from_env_before_subprocess(self): + ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.11"}]}) + with patch.dict(os.environ, {"GITHUB_TOKEN": "fake-token"}, clear=False): + with self._patch_github_api(): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.11") + # After main(), the token must no longer be readable from env. + self.assertNotIn("GITHUB_TOKEN", os.environ) + + def test_reopens_closed_unmerged_pr(self): + closed_pr = MagicMock(state="closed", merged=False) + closed_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/123" + ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.9"}]}) + with ( + patch.dict(os.environ, {"GITHUB_TOKEN": "t"}, clear=False), + patch('tasks.libs.common.git.check_clean_branch_state'), + patch('tasks.libs.common.utils.set_gitconfig_in_ci'), + patch('builtins.open', mock_open()), + self._patch_github_api(closed_unmerged=[closed_pr]), + ): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.11") + + # Reopen was called; no new PR was created for this branch. + closed_pr.edit.assert_called_once() + kwargs = closed_pr.edit.call_args.kwargs + self.assertEqual(kwargs["state"], "open") + self.assertIn("v0.0.11", kwargs["title"]) + + def test_happy_path_creates_pr_with_labels_and_review(self): + ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.9"}]}) + new_pr = MagicMock() + new_pr.number = 42 + new_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/42" + with ( + patch.dict(os.environ, {"GITHUB_TOKEN": "t"}, clear=False), + patch('tasks.libs.common.git.check_clean_branch_state'), + patch('tasks.libs.common.utils.set_gitconfig_in_ci'), + patch('builtins.open', mock_open()), + self._patch_github_api(created_pr=new_pr), + ): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.11") + + # Validate the key side-effects on the PR object returned by create_pr. + new_pr.create_review_request.assert_called_once() + review_kwargs = new_pr.create_review_request.call_args.kwargs + self.assertEqual(review_kwargs.get("team_reviewers"), ["action-platform"]) + + # Branch push happened with force. + run_cmds = [c.args[0] for c in ctx.run.call_args_list] + self.assertTrue(any("git push --force" in c and "bump-rshell-v0.0.11" in c for c in run_cmds)) From 6e9b20f761c9220157f2e27ca5273f6df0e57d14 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 17 Apr 2026 14:20:11 -0700 Subject: [PATCH 2/3] tasks/github_tasks: tighten precondition to clean working tree only --- tasks/github_tasks.py | 24 ++++++++++++++++-------- tasks/unit_tests/github_tasks_tests.py | 21 +++++++++++++++++---- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/tasks/github_tasks.py b/tasks/github_tasks.py index bd51bbbe5292..424e55c2900f 100644 --- a/tasks/github_tasks.py +++ b/tasks/github_tasks.py @@ -772,7 +772,7 @@ def bump_rshell(ctx, version=None, draft=True): DataDog/datadog-agent). """ from tasks.libs.ciproviders.github_api import GithubAPI - from tasks.libs.common.git import check_clean_branch_state + from tasks.libs.common.git import check_uncommitted_changes from tasks.libs.common.utils import set_gitconfig_in_ci if version is None: @@ -786,6 +786,18 @@ def bump_rshell(ctx, version=None, draft=True): else: print(color_message(f"Using explicit rshell version: {version}", Color.BLUE)) + # Precondition: clean working tree. We force-push later, so stale local or + # remote branches are fine — but unrelated uncommitted changes would get + # swept into the bump commit by `git add -A`. + if check_uncommitted_changes(ctx): + raise Exit( + color_message( + "There are uncommitted changes in your repository. " "Please commit or stash them before trying again.", + Color.RED, + ), + code=1, + ) + gh = GithubAPI() branch = f"bump-rshell-{version}" @@ -838,13 +850,9 @@ def bump_rshell(ctx, version=None, draft=True): ctx.run(f"git add {note_path}") set_gitconfig_in_ci(ctx) - try: - check_clean_branch_state(ctx, gh, branch) - except Exit as e: - if "already exists locally" not in str(e): - raise - - ctx.run(f"git switch -c {branch}", warn=True, hide=True) + # `-C` (capital) creates or resets; handles a stale local branch from a + # prior partial run without needing a separate precondition check. + ctx.run(f"git switch -C {branch}", hide=True) commit_msg = ( f"Bump rshell dependency from {previous_version} to {version}" if previous_version diff --git a/tasks/unit_tests/github_tasks_tests.py b/tasks/unit_tests/github_tasks_tests.py index bd4527a9dfe8..f38841dc3738 100644 --- a/tasks/unit_tests/github_tasks_tests.py +++ b/tasks/unit_tests/github_tasks_tests.py @@ -763,6 +763,8 @@ def run(cmd, *_args, **_kwargs): result.stdout = "Created new notes file in releasenotes/notes/bump-rshell-v0.0.11-abc.yaml\n" elif "git diff --cached --quiet" in cmd: result.ok = False # non-zero exit = there ARE changes + elif "git --no-pager diff --name-only HEAD" in cmd: + result.stdout = "0" # `check_uncommitted_changes` precondition: clean tree else: result.stdout = "" return result @@ -802,15 +804,26 @@ def test_rejects_invalid_explicit_version(self): with self.assertRaises(Exit): tasks.github_tasks.bump_rshell(ctx, version=bad) + def test_aborts_if_working_tree_is_dirty(self): + ctx = Context() + # Mock check_uncommitted_changes to say the tree is dirty. + ctx.run = MagicMock( + return_value=MagicMock(stdout="3", ok=True) # wc -l reports 3 modified files + ) + with self.assertRaises(Exit): + tasks.github_tasks.bump_rshell(ctx, version="v0.0.11") + def test_noop_when_open_pr_exists(self): open_pr = MagicMock(state="open", merged=False) open_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/999" ctx = self._make_ctx() with self._patch_github_api(open_prs=[open_pr]): tasks.github_tasks.bump_rshell(ctx, version="v0.0.99") - # Nothing should have been done after classification — no git, no go, no tidy. + # No MUTATION commands should have run after classification. The + # pre-flight uncommitted-changes check is read-only and is fine. + mutations = ("go get", "git add", "git commit", "git push", "git switch", "dda inv tidy", "reno new") commands = [call.args[0] for call in ctx.run.call_args_list] - self.assertFalse(any(c.startswith(("go ", "git ", "dda ", "reno ")) for c in commands)) + self.assertFalse(any(c.startswith(mutations) for c in commands)) def test_noop_when_already_pinned(self): ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.11"}]}) @@ -835,7 +848,7 @@ def test_reopens_closed_unmerged_pr(self): ctx = self._make_ctx(go_mod_json={"Require": [{"Path": "github.com/DataDog/rshell", "Version": "v0.0.9"}]}) with ( patch.dict(os.environ, {"GITHUB_TOKEN": "t"}, clear=False), - patch('tasks.libs.common.git.check_clean_branch_state'), + patch('tasks.libs.common.git.check_uncommitted_changes', return_value=False), patch('tasks.libs.common.utils.set_gitconfig_in_ci'), patch('builtins.open', mock_open()), self._patch_github_api(closed_unmerged=[closed_pr]), @@ -855,7 +868,7 @@ def test_happy_path_creates_pr_with_labels_and_review(self): new_pr.html_url = "https://github.com/DataDog/datadog-agent/pull/42" with ( patch.dict(os.environ, {"GITHUB_TOKEN": "t"}, clear=False), - patch('tasks.libs.common.git.check_clean_branch_state'), + patch('tasks.libs.common.git.check_uncommitted_changes', return_value=False), patch('tasks.libs.common.utils.set_gitconfig_in_ci'), patch('builtins.open', mock_open()), self._patch_github_api(created_pr=new_pr), From 94f251975036fa712bbce3565e4b8ce0784ed953 Mon Sep 17 00:00:00 2001 From: Matthew DeGuzman Date: Fri, 17 Apr 2026 14:31:03 -0700 Subject: [PATCH 3/3] fix comment and add correct labels --- tasks/github_tasks.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/github_tasks.py b/tasks/github_tasks.py index 424e55c2900f..2829312b4a67 100644 --- a/tasks/github_tasks.py +++ b/tasks/github_tasks.py @@ -860,8 +860,8 @@ def bump_rshell(ctx, version=None, draft=True): ) ctx.run(f'git commit -m "{commit_msg}" --no-verify') - # Wire the token into the remote URL for push auth (mirrors - # tasks/pipeline.py:693). `hide=True` keeps it out of invoke's output. + # Wire the token into the remote URL for push auth. `hide=True` + # keeps it out of invoke's output. ctx.run( f"git remote set-url origin " f"https://x-access-token:{gh._auth.token}@github.com/DataDog/datadog-agent.git", hide=True, @@ -890,7 +890,7 @@ def bump_rshell(ctx, version=None, draft=True): from github import GithubException - for label in ("changelog/no-changelog", "ask-review"): + for label in ("qa/no-code-change", "team/action-platform"): try: gh.add_pr_label(pr.number, label) except GithubException as e: