Skip to content

Commit 01f7d25

Browse files
Kyle-NealeclaudeCopilot
authored
Guard build_agent.yaml updates on Agent branch existence (DataDog#23987)
* Guard build_agent.yaml updates on Agent branch existence Extract YAML helpers into a shared build_agent.py module so the inline update in `ddev release branch create` and the recovery path through the workflow share parsing logic. Gate both writers on the matching DataDog/datadog-agent branch existing: - `ensure_build_agent_yaml_updated` skips the rewrite (with a warning) when the upstream branch is missing, leaving `main` in place for the tag-time recovery path to handle later. - The `update-build-agent-yaml.yml` workflow now hard-fails instead of warning when the upstream branch is missing, so a fire-and-forget dispatch from `ddev release branch tag` is visible in Actions. - `bump_milestone` defensively restores build_agent.yaml from origin/master after checking out the milestone-bump branch so the release-branch edit cannot leak into the bump commit (see PR DataDog#23977 commit f71a89c). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Wrap warning message to satisfy ruff E501 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop unused re-exports and __all__ from create.py No external caller imports the two YAML-helper names from create.py; they live in build_agent.py and tag.py imports them from there directly. With those imports gone, __all__ has no purpose either. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Import build agent helper from owner module --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 1f1382c commit 01f7d25

6 files changed

Lines changed: 139 additions & 67 deletions

File tree

.github/workflows/update-build-agent-yaml.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ jobs:
6161
exit 0
6262
fi
6363
if ! git ls-remote --exit-code --heads https://github.com/DataDog/datadog-agent.git "$BRANCH" >/dev/null 2>&1; then
64-
echo "::warning::Agent branch '$BRANCH' does not exist in DataDog/datadog-agent yet. Creating the PR anyway."
64+
echo "::error::Agent branch '$BRANCH' does not exist in DataDog/datadog-agent. Refusing to open a PR pointing to a missing upstream branch. Re-dispatch this workflow once the upstream branch is cut."
65+
exit 1
6566
fi
6667
echo "needs_update=true" >> "$GITHUB_ENV"
6768

ddev/changelog.d/23987.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Gate `ddev release branch create` and `update-build-agent-yaml.yml` on the matching `DataDog/datadog-agent` branch existing so neither writer can produce a release-branch pointer to a missing upstream branch.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# (C) Datadog, Inc. 2024-present
2+
# All rights reserved
3+
# Licensed under a 3-clause BSD style license (see LICENSE)
4+
from __future__ import annotations
5+
6+
import re
7+
import subprocess
8+
from typing import TYPE_CHECKING
9+
10+
if TYPE_CHECKING:
11+
from ddev.cli.application import Application
12+
13+
BUILD_AGENT_YAML_PATH = '.gitlab/build_agent.yaml'
14+
BUILD_AGENT_TEMPLATE_PATTERN = r'^\.build-agent-tpl:\n(?:[^\S\n].*(?:\n|$))*'
15+
BUILD_AGENT_MAIN_BRANCH_PATTERN = r'^(\s+branch:\s+)main([^\S\n]*)$'
16+
BUILD_AGENT_TEMPLATE_REGEX = re.compile(BUILD_AGENT_TEMPLATE_PATTERN, re.MULTILINE)
17+
BUILD_AGENT_MAIN_BRANCH_REGEX = re.compile(BUILD_AGENT_MAIN_BRANCH_PATTERN, re.MULTILINE)
18+
19+
DATADOG_AGENT_REPO_URL = 'https://github.com/DataDog/datadog-agent.git'
20+
21+
22+
def agent_branch_exists(branch_name: str) -> bool:
23+
"""Return ``True`` if ``branch_name`` exists in ``DataDog/datadog-agent``."""
24+
result = subprocess.run(
25+
['git', 'ls-remote', '--exit-code', '--heads', DATADOG_AGENT_REPO_URL, branch_name],
26+
capture_output=True,
27+
check=False,
28+
)
29+
return result.returncode == 0
30+
31+
32+
def find_build_agent_template_main_branch_matches(content: str) -> list[re.Match[str]]:
33+
template_match = BUILD_AGENT_TEMPLATE_REGEX.search(content)
34+
if template_match is None:
35+
return []
36+
return list(BUILD_AGENT_MAIN_BRANCH_REGEX.finditer(template_match.group(0)))
37+
38+
39+
def replace_build_agent_template_main_branch(content: str, branch_name: str) -> tuple[str, int]:
40+
template_match = BUILD_AGENT_TEMPLATE_REGEX.search(content)
41+
if template_match is None:
42+
return content, 0
43+
44+
def replacement(match: re.Match[str]) -> str:
45+
return f'{match.group(1)}{branch_name}{match.group(2)}'
46+
47+
updated_template, replacement_count = BUILD_AGENT_MAIN_BRANCH_REGEX.subn(
48+
replacement, template_match.group(0), count=1
49+
)
50+
if replacement_count == 0:
51+
return content, 0
52+
53+
updated_content = content[: template_match.start()] + updated_template + content[template_match.end() :]
54+
return updated_content, replacement_count
55+
56+
57+
def ensure_build_agent_yaml_updated(app: Application, branch_name: str) -> bool:
58+
"""Update build_agent.yaml to point to the release branch when it still targets main.
59+
60+
Returns False without modifying the file when the matching ``DataDog/datadog-agent``
61+
branch does not exist yet, so callers can leave ``main`` in place for the recovery
62+
path (``ddev release branch tag`` -> ``update-build-agent-yaml.yml``) to run later.
63+
"""
64+
from ddev.utils.fs import Path
65+
66+
build_agent_yaml = Path(BUILD_AGENT_YAML_PATH)
67+
68+
if not build_agent_yaml.exists():
69+
app.display_warning(f'Warning: {build_agent_yaml} not found')
70+
return False
71+
72+
with open(build_agent_yaml, 'r') as f:
73+
content = f.read()
74+
75+
matches = find_build_agent_template_main_branch_matches(content)
76+
if not matches:
77+
return False
78+
if len(matches) > 1:
79+
app.abort(
80+
f'Expected exactly one `.build-agent-tpl` branch pointing to `main` in `{BUILD_AGENT_YAML_PATH}`; '
81+
f'found {len(matches)}.'
82+
)
83+
return False
84+
85+
if not agent_branch_exists(branch_name):
86+
app.display_warning(
87+
f'Unable to verify that agent branch `{branch_name}` exists in `DataDog/datadog-agent`. '
88+
f'Leaving `{BUILD_AGENT_YAML_PATH}` pointing to `main`. '
89+
f'Re-dispatch `update-build-agent-yaml.yml` (or re-run `ddev release branch tag`) '
90+
f'once the upstream branch exists.'
91+
)
92+
return False
93+
94+
updated_content, replacement_count = replace_build_agent_template_main_branch(content, branch_name)
95+
assert replacement_count == 1
96+
97+
with open(build_agent_yaml, 'w') as f:
98+
f.write(updated_content)
99+
100+
app.display_success(f'Updated build_agent.yaml file to use Agent branch: {branch_name}')
101+
return True

ddev/src/ddev/cli/release/branch/create.py

Lines changed: 5 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,13 @@
1212
from httpx import HTTPError, HTTPStatusError
1313
from packaging.version import Version
1414

15+
from .build_agent import BUILD_AGENT_YAML_PATH, ensure_build_agent_yaml_updated
16+
1517
if TYPE_CHECKING:
1618
from ddev.cli.application import Application
1719

1820
BRANCH_NAME_PATTERN = r"^\d+\.\d+\.x$"
1921
BRANCH_NAME_REGEX = re.compile(BRANCH_NAME_PATTERN)
20-
BUILD_AGENT_YAML_PATH = '.gitlab/build_agent.yaml'
21-
BUILD_AGENT_TEMPLATE_PATTERN = r'^\.build-agent-tpl:\n(?:[^\S\n].*(?:\n|$))*'
22-
BUILD_AGENT_MAIN_BRANCH_PATTERN = r'^(\s+branch:\s+)main([^\S\n]*)$'
23-
BUILD_AGENT_TEMPLATE_REGEX = re.compile(BUILD_AGENT_TEMPLATE_PATTERN, re.MULTILINE)
24-
BUILD_AGENT_MAIN_BRANCH_REGEX = re.compile(BUILD_AGENT_MAIN_BRANCH_PATTERN, re.MULTILINE)
2522
GITHUB_LABEL_COLOR = '5319e7'
2623

2724

@@ -108,6 +105,9 @@ def bump_milestone(app: Application, branch_name: str) -> None:
108105
app.display_waiting(f"Updating release.json with new milestone `{next_milestone}`...")
109106
app.repo.git.run('fetch', 'origin', 'master')
110107
app.repo.git.run('checkout', '-B', bump_branch, 'origin/master')
108+
# Force-restore build_agent.yaml from origin/master so any prior in-process edit on the release
109+
# branch cannot leak into the milestone-bump commit (root cause of the leak on PR #23977).
110+
app.repo.git.run('checkout', 'origin/master', '--', BUILD_AGENT_YAML_PATH)
111111
update_release_json(app.repo.path / 'release.json', next_milestone)
112112
app.repo.git.run('add', 'release.json')
113113
app.repo.git.run('commit', '-m', f'Update current_milestone to {next_milestone}')
@@ -179,62 +179,3 @@ def suggest_next_branch(app: Application) -> str:
179179
return suggestion
180180

181181
return f'{major}.{minors[-1] + 1}.x'
182-
183-
184-
def ensure_build_agent_yaml_updated(app: Application, branch_name: str) -> bool:
185-
"""Update build_agent.yaml to point to the release branch when it still targets main."""
186-
from ddev.utils.fs import Path
187-
188-
build_agent_yaml = Path(BUILD_AGENT_YAML_PATH)
189-
190-
if not build_agent_yaml.exists():
191-
app.display_warning(f'Warning: {build_agent_yaml} not found')
192-
return False
193-
194-
with open(build_agent_yaml, 'r') as f:
195-
content = f.read()
196-
197-
matches = find_build_agent_template_main_branch_matches(content)
198-
if not matches:
199-
return False
200-
if len(matches) > 1:
201-
app.abort(
202-
f'Expected exactly one `.build-agent-tpl` branch pointing to `main` in `{BUILD_AGENT_YAML_PATH}`; '
203-
f'found {len(matches)}.'
204-
)
205-
return False
206-
207-
updated_content, replacement_count = replace_build_agent_template_main_branch(content, branch_name)
208-
assert replacement_count == 1
209-
210-
with open(build_agent_yaml, 'w') as f:
211-
f.write(updated_content)
212-
213-
app.display_success(f'Updated build_agent.yaml file to use Agent branch: {branch_name}')
214-
return True
215-
216-
217-
def find_build_agent_template_main_branch_matches(content: str) -> list[re.Match[str]]:
218-
template_match = BUILD_AGENT_TEMPLATE_REGEX.search(content)
219-
if template_match is None:
220-
return []
221-
222-
return list(BUILD_AGENT_MAIN_BRANCH_REGEX.finditer(template_match.group(0)))
223-
224-
225-
def replace_build_agent_template_main_branch(content: str, branch_name: str) -> tuple[str, int]:
226-
template_match = BUILD_AGENT_TEMPLATE_REGEX.search(content)
227-
if template_match is None:
228-
return content, 0
229-
230-
def replacement(match: re.Match[str]) -> str:
231-
return f'{match.group(1)}{branch_name}{match.group(2)}'
232-
233-
updated_template, replacement_count = BUILD_AGENT_MAIN_BRANCH_REGEX.subn(
234-
replacement, template_match.group(0), count=1
235-
)
236-
if replacement_count == 0:
237-
return content, 0
238-
239-
updated_content = content[: template_match.start()] + updated_template + content[template_match.end() :]
240-
return updated_content, replacement_count

ddev/src/ddev/cli/release/branch/tag.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
from httpx import HTTPStatusError
99
from packaging.version import Version
1010

11-
from .create import BRANCH_NAME_REGEX, BUILD_AGENT_YAML_PATH, find_build_agent_template_main_branch_matches
11+
from .build_agent import BUILD_AGENT_YAML_PATH, find_build_agent_template_main_branch_matches
12+
from .create import BRANCH_NAME_REGEX
1213

1314
if TYPE_CHECKING:
1415
from ddev.cli.application import Application

ddev/tests/cli/release/branch/test_create.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
import pytest
1010

11-
from ddev.cli.release.branch.create import compute_next_milestone, ensure_build_agent_yaml_updated, update_release_json
11+
from ddev.cli.release.branch.build_agent import ensure_build_agent_yaml_updated
12+
from ddev.cli.release.branch.create import compute_next_milestone, update_release_json
1213
from ddev.utils.fs import Path
1314

1415

@@ -70,6 +71,9 @@ def test_create_branch(ddev, mocker, yaml_updated):
7071
# Milestone bump workflow
7172
run_mock.assert_any_call('fetch', 'origin', 'master')
7273
run_mock.assert_any_call('checkout', '-B', 'release/bump-milestone-5.6.0', 'origin/master')
74+
# Defensive restore prevents the build_agent.yaml change on the release branch
75+
# from leaking into the milestone-bump commit (see PR #23977 leak).
76+
run_mock.assert_any_call('checkout', 'origin/master', '--', '.gitlab/build_agent.yaml')
7377
run_mock.assert_any_call('add', 'release.json')
7478
run_mock.assert_any_call('commit', '-m', 'Update current_milestone to 5.6.0')
7579
run_mock.assert_any_call('push', 'origin', 'release/bump-milestone-5.6.0')
@@ -84,6 +88,7 @@ def test_ensure_build_agent_yaml_updated(mocker, tmp_path):
8488
build_agent_path.write_text('.build-agent-tpl:\n trigger:\n branch: main\n')
8589

8690
app_mock = mocker.MagicMock()
91+
mocker.patch('ddev.cli.release.branch.build_agent.agent_branch_exists', return_value=True)
8792

8893
with Path(tmp_path).as_cwd():
8994
result = ensure_build_agent_yaml_updated(app_mock, '7.99.x')
@@ -100,6 +105,7 @@ def test_ensure_build_agent_yaml_updated_ignores_unrelated_main_branch(mocker, t
100105
build_agent_path.write_text(content)
101106

102107
app_mock = mocker.MagicMock()
108+
mocker.patch('ddev.cli.release.branch.build_agent.agent_branch_exists', return_value=True)
103109

104110
with Path(tmp_path).as_cwd():
105111
result = ensure_build_agent_yaml_updated(app_mock, '7.99.x')
@@ -111,6 +117,27 @@ def test_ensure_build_agent_yaml_updated_ignores_unrelated_main_branch(mocker, t
111117
app_mock.abort.assert_not_called()
112118

113119

120+
def test_ensure_build_agent_yaml_updated_skips_when_agent_branch_missing(mocker, tmp_path):
121+
"""Leave `main` in place when the matching DataDog/datadog-agent branch does not exist yet."""
122+
build_agent_path = Path(tmp_path / '.gitlab' / 'build_agent.yaml')
123+
build_agent_path.parent.ensure_dir_exists()
124+
original_content = '.build-agent-tpl:\n trigger:\n branch: main\n'
125+
build_agent_path.write_text(original_content)
126+
127+
app_mock = mocker.MagicMock()
128+
mocker.patch('ddev.cli.release.branch.build_agent.agent_branch_exists', return_value=False)
129+
130+
with Path(tmp_path).as_cwd():
131+
result = ensure_build_agent_yaml_updated(app_mock, '7.99.x')
132+
133+
assert result is False
134+
assert build_agent_path.read_text() == original_content
135+
app_mock.display_warning.assert_called_once()
136+
warning_message = app_mock.display_warning.call_args[0][0]
137+
assert '7.99.x' in warning_message
138+
assert 'datadog-agent' in warning_message
139+
140+
114141
def test_ensure_build_agent_yaml_updated_aborts_on_multiple_template_main_branches(mocker, tmp_path):
115142
build_agent_path = Path(tmp_path / '.gitlab' / 'build_agent.yaml')
116143
build_agent_path.parent.ensure_dir_exists()

0 commit comments

Comments
 (0)