scripts: add validate-runner-bump.py for Renovate diff validation#636
Open
jeanschmidt wants to merge 2 commits into
Open
scripts: add validate-runner-bump.py for Renovate diff validation#636jeanschmidt wants to merge 2 commits into
jeanschmidt wants to merge 2 commits into
Conversation
This was referenced May 27, 2026
malfet
suggested changes
May 27, 2026
malfet
left a comment
malfet
left a comment
Contributor
There was a problem hiding this comment.
Why it needs to be so complex? All this 400 line change does is checks whether some property in osdc/clusters.yaml contains positive version bump right? Why not do something like
current, old = json.read("osdc/clusters.yaml"), json.read_string(subprocess.check_output("git show HEAD~1 osdc/clusters.yml")) and just compare the appropriate key? And you can make a very simple gh api call to figure out whether diff is 1 line, can't you?
|
|
||
| if len(files) != 1: | ||
| _emit("close-wrong-file-count", f"expected exactly 1 file changed; got {len(files)}") | ||
| return 0 |
Contributor
There was a problem hiding this comment.
Why wrong file count is not an error?
|
|
||
| EXPECTED_FILE = "osdc/clusters.yaml" | ||
|
|
||
| # MUST remain bit-for-bit equivalent to the prior bash regex so the pre- |
Contributor
There was a problem hiding this comment.
MUST remain bit-for-bit equivalent to the prior bash regex
There are no prior, not sure what you are referring to...
| LINE_PATTERN = re.compile(r'^ runner_image_tag:[ \t]*"(?P<ver>\d+\.\d+\.\d+)"[ \t]*(#.*)?$') | ||
|
|
||
|
|
||
| def _emit(decision: str, reason: str, **extra: str) -> None: |
Contributor
There was a problem hiding this comment.
Extra is not an str, is it? Also traditionally this is called kwargs
Suggested change
| def _emit(decision: str, reason: str, **extra: str) -> None: | |
| def _emit(decision: str, reason: str, **extra: dict) -> None: |
Comment on lines
+43
to
+46
| print(f"decision={decision}") | ||
| print(f"reason={reason}") | ||
| for k, v in extra.items(): | ||
| print(f"{k}={v}") |
Contributor
There was a problem hiding this comment.
Nit
Suggested change
| print(f"decision={decision}") | |
| print(f"reason={reason}") | |
| for k, v in extra.items(): | |
| print(f"{k}={v}") | |
| print(*(f"{k}={v}" for k, v in {"decision": decision, "reason": reason, **extra}.items()), sep="\n") |
Comment on lines
+50
to
+51
| parts = version.split(".") | ||
| return (int(parts[0]), int(parts[1]), int(parts[2])) |
Contributor
There was a problem hiding this comment.
Nit
Suggested change
| parts = version.split(".") | |
| return (int(parts[0]), int(parts[1]), int(parts[2])) | |
| return tuple(int(x) for x in version.split(".")) |
malfet
reviewed
May 27, 2026
malfet
left a comment
malfet
left a comment
Contributor
There was a problem hiding this comment.
Asked claude to write succint diff for validating that something is positive semver and this is what it comeup with
#!/usr/bin/env python3
"""Exit 0 iff `git diff` is a single property line with a positive semver bump."""
import re
import subprocess
import sys
SEMVER = re.compile(r"(\d+)\.(\d+)\.(\d+)")
PROP = re.compile(r"^[-+]\s*([\w.-]+)\s*[:=]\s*['\"]?v?(\d+\.\d+\.\d+)['\"]?\s*,?\s*$")
def main() -> int:
diff = subprocess.check_output(["git", "diff", "--unified=0", "--no-color"], text=True)
hunks = [l for l in diff.splitlines() if l.startswith(("+", "-")) and not l.startswith(("+++", "---"))]
if len(hunks) != 2 or hunks[0][0] == hunks[1][0]:
return 1
old, new = sorted(hunks) # '-' < '+'
om, nm = PROP.match(old), PROP.match(new)
if not (om and nm) or om.group(1) != nm.group(1):
return 1
ov, nv = tuple(map(int, om.group(2).split("."))), tuple(map(int, nm.group(2).split(".")))
return 0 if nv > ov else 1
if __name__ == "__main__":
sys.exit(main())
Contributor
Author
|
Thanks — taking the nits at lines 37/42/46/51 in the next revision (kwargs rename, dict-merge On replacing the script with the ~25-line version: pushing back, three reasons. 1. Two callers, two diff shapes. 2. The regex is intentionally over-specified. 3. Structured
Non-zero exit aborts the job before the close-comment step runs. Exit codes are reserved for unrecoverable input errors (missing/malformed The script is ~130 LOC, another 300ish lines are tests to meet the 97% coverage and test things like pin every branch — downgrade, |
tofu plan — arc-cbr-production-uw1✅ Plan succeeded · commit Plan output |
tofu plan — arc-cbr-production✅ Plan succeeded · commit Plan output |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack from ghstack (oldest at bottom):
Impact: OSDC scripts — new standalone validator
Risk: low
What
Introduces a shared Python validator for Renovate-style runner-image bump
PRs. Confirms the diff touches exactly one file at exactly one hunk, that
the bumped value is strict semver, and that the new version is strictly
greater than the old.
Why
Both the pre-merge auto-approve gate and the post-merge auto-deploy
workflow need the same correctness guarantees on a Renovate diff before
they trust it. Centralizing the logic in one Python script keeps the YAML
workflows minimal and gives us proper unit-test coverage.
How
target file path from CLI args.
semver on both sides, new > old.
fast and the surfaced error tells reviewers exactly what is off.
rejection branch.
Changes
osdc/scripts/validate-runner-bump.py: new validator.osdc/scripts/test_validate_runner_bump.py: unit tests.Notes
No callers yet; wired up in later commits.
Testing
cd osdc && just testruns the new test suite.Signed-off-by: Jean Schmidt contato@jschmidt.me