[hack] Add create-release-tag.py script#3913
[hack] Add create-release-tag.py script#3913mtnbikenc wants to merge 2 commits intoopenshift:masterfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Skipping CI for Draft Pull Request. |
|
@mtnbikenc thanks for working on this. there is a presedence to maintain the hack scripts in bash, consider implementing the script in bash to avoid introducing another dependency. i.e. python also, the script requires special permissions push the tag to the repository, please capture that requirement in the documentation/usage |
|
@jrvaldes Thanks for the feedback. I started out with a bash script but it became lengthy and cumbersome for the task at hand. I switched from bash to python for improved error handling and reducing the overall length of the script. I understand the desire for consistency for scripts, but I also balance that with the right tool for the job. I'd like feedback from the rest of the team if using python would be too burdensome or undesired. I'll add a note about the repo permissions required for pushing tags. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/create-release-tag.py`:
- Around line 224-230: The tag_exists function currently calls git("rev-parse",
tag) which resolves any ref (branch or tag); change it to verify the tag ref
explicitly by invoking git with "refs/tags/{tag}" (e.g., git("rev-parse",
f"refs/tags/{tag}") or equivalent) so only actual tags cause a True return, and
keep the existing try/except RuntimeError behavior to return False when the
refs/tags/... lookup fails; update the reference in the tag_exists function
accordingly.
- Around line 297-298: The current check only uses re.fullmatch on args.date and
allows invalid calendar dates (e.g., 2026-02-31); update the validation to
attempt parsing the ISO date string (use datetime.date.fromisoformat or
datetime.strptime) inside a try/except and if parsing raises ValueError call
parser.error(f"--date must be YYYY-MM-DD, got: {args.date!r}") so invalid
calendar dates are rejected at argument parsing; replace or augment the existing
re.fullmatch branch that references args.date and parser.error to perform the
parse-and-catch logic instead.
- Around line 320-346: fetch_bundle_info() and fetch_operator_push_date() call
_fetch_pages() and may raise network/JSON exceptions; wrap both calls in
try/except and surface a clean ERROR + sys.exit(1) like the git-error handling
already used: when calling fetch_bundle_info(version) and
fetch_operator_push_date(version) catch Exception as err, print a descriptive
ERROR to stderr that includes the function name and err (same style as the git
failure prints), then sys.exit(1); keep setting commit_sha/commit_source and
published_date/date_source only on success so behavior matches the manual
--commit/--date branches.
- Around line 214-221: The git() helper currently allows FileNotFoundError from
subprocess.run to leak; modify git() to resolve the git executable first (e.g.,
use shutil.which("git")) and if not found raise a RuntimeError with a clear
message, or wrap the subprocess.run call in a try/except that catches
FileNotFoundError and re-raises a RuntimeError with the original error text;
ensure the change is applied in the git(...) function so all callers continue to
receive RuntimeError instead of an unhandled FileNotFoundError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5eca9ed4-d5b5-4540-82fc-b372ad2e25fd
📒 Files selected for processing (1)
hack/create-release-tag.py
mtnbikenc
force-pushed
the
feature/create-release-tag
branch
from
fc5463b to
1f0d9b8
Compare
Adds a script to create consistent annotated release tags for WMCO. The commit SHA is resolved from the bundle image in the Red Hat Container Catalog, preferring the org.opencontainers.image.revision OCI label (full SHA) and falling back to the short-SHA image tag — matching the approach used by hack/verify-release.py. The tag date is set to the operator image's push_date so the timestamp reflects the actual release date. A --commit override is available for releases not present in the catalog (e.g. backport-only releases), and --date allows overriding the publish date. The script prompts for confirmation before creating the tag and prints the push command targeting the upstream repository. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mtnbikenc
force-pushed
the
feature/create-release-tag
branch
from
1f0d9b8 to
0f16e25
Compare
jrvaldes
left a comment
jrvaldes
left a comment
There was a problem hiding this comment.
@mtnbikenc thanks for working on this, LGTM. just minor suggestions, PTAL
| def resolve_commit(ref: str) -> str: | ||
| """Return the full commit SHA for ref, or raise RuntimeError.""" | ||
| try: | ||
| return git("rev-parse", f"{ref}^{{commit}}") |
There was a problem hiding this comment.
consider validating the --commit input; the user-supplied SHA is passed to git rev-parse without format checking, enabling argument injection.
There was a problem hiding this comment.
Addressed in commit ebb6fdb: added a _HEX_RE (^[0-9a-f]{7,40}$) check against the --commit value at argument-parse time via parser.error(). Any value that isn't a lowercase hex string of 7–40 characters is rejected before reaching git rev-parse, preventing git option injection (e.g. --upload-pack=evil-cmd or relative refs like HEAD~1). Note: since subprocess.run() uses a list rather than shell=True, shell metacharacter injection was never possible — the real risk was git option injection, which _HEX_RE now covers.
|
|
||
|
|
||
| # pylint: disable=too-many-locals,too-many-branches,too-many-statements | ||
| def main(): |
There was a problem hiding this comment.
this func is big and complex, consider generalizing orthogonal logic using helper functions, for example
resolve_commit_sha() and resolve_date()
There was a problem hiding this comment.
Addressed in commit ebb6fdb: extracted resolve_commit_sha(tag, version, override) and resolve_date(tag, version, override) from main(). Each helper encapsulates the catalog lookup, exception handling, and missing-value error messaging for its respective concern. The too-many-branches pylint suppression on main() was also removed as it is no longer needed.
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Catalog helpers (shared pattern with verify-release.py) |
There was a problem hiding this comment.
still have mixed feelings about introducing helper tools in Python; anyhow, consider extracting a shared module under hack/lib/ to avoid duplicating this implementation
There was a problem hiding this comment.
Agreed on the value. Deferring the hack/lib/ extraction until team direction on future hack script organization is decided.
|
Thanks for the feedback and suggestions, all good points. I'll look into these and come back with some updates. |
|
I didn't squash commits to allow easier review of the changes. If the team decides to move forward with using this script, and other python scripts, I can add python linting tests to this PR since it is the first. |
mtnbikenc
force-pushed
the
feature/create-release-tag
branch
from
ebb6fdb to
7cdb3c7
Compare
mansikulkarni96
left a comment
mansikulkarni96
left a comment
There was a problem hiding this comment.
I have used this script and it works for me. I am okay with having this in python as this is easy to maintain and right for the job. I agree with adding a linter like pylint to help further updates to this.
|
|
||
| # pylint: disable=too-many-locals,too-many-statements | ||
| def main(): | ||
| """Parse args, resolve tag details, confirm with user, create the tag.""" |
There was a problem hiding this comment.
I like that the script asks for confirmation to avoid accidental tagging.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mansikulkarni96, mtnbikenc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
- Validate --commit flag against _HEX_RE at parse time to prevent git option injection (e.g. --upload-pack=evil-cmd or HEAD~1 references) - Extract resolve_commit_sha() and resolve_date() helpers from main() to reduce branch count and remove too-many-branches pylint suppress Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mtnbikenc
force-pushed
the
feature/create-release-tag
branch
from
7cdb3c7 to
7950a38
Compare
|
As part of this PR I reviewed the script against what I believe should be a baseline standard for hack scripts. Noting it here for team visibility and feedback. Hack script checklist
The Why is the most important piece — these scripts codify team process decisions, and without the reasoning future maintainers have no basis for knowing whether a detail is intentional or accidental. Open to guidance from the team if there are other items that should be on this list. |
|
@mtnbikenc: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Summary
preferring the `org.opencontainers.image.revision` OCI label (full SHA) and falling
back to the short-SHA image tag
the actual release date rather than when the tag was created, consistent with the
commit source
option injection
and their sources
header: why the bundle image commit is tagged, why the OCI label is preferred over
the short SHA tag, why annotated tags, and why the push command is shown rather than
executed
environment expectations, and permissions
Usage
`--commit` is required for releases not present in the bundle catalog (e.g. backport-only
releases). `--date` allows overriding the publish date when needed.
Test plan