Repository navigation
fix(tekton): correct tidb-binlog release branch filter - #5377
Conversation
The prod2-derived regex matched release-7.18.1/release-7.58.1 instead of release-7.1/release-7.5. Match the intended 6.5, 7.1, 7.5 and 8.1 release branches; tidb-binlog is deprecated at release-8.4.
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary:
This PR fixes a regex filter in a Tekton trigger for the tidb-binlog repository branches. The previous regex mistakenly matched unintended branches like release-7.18.1 and release-7.58.1, while the intended branches are release-6.5, release-7.1, release-7.5, and release-8.1. The author adjusted the regex to correctly match these specific branches and exclude deprecated or later branches. The fix is concise and focused on the regex pattern, with validation done through regex testing and kustomize build verification. Overall, the change is straightforward and appears correct.
Code Improvements
-
Regex specificity and clarity (file:
tekton/v1/triggers/triggers/env-gcp/pingcap/tidb-binlog/git-push.yaml, line ~17):
The updated regex'release-(6[.]5|7[.][15]|8[.]1)'matchesrelease-7.1andrelease-7.5as intended but could be clearer by explicitly listing these two branches instead of using a character class[15]. This reduces any chance of accidental matches (e.g., no7.8or7.0creeping in if regex is modified later).Suggested improvement:
body.ref.matches('^refs/heads/release-(6[.]5|7[.]1|7[.]5|8[.]1)$')This change trades the compactness of
[15]for explicitness and maintainability. -
Regex Anchors:
The regex uses^and$anchors, which is good. Just ensure these anchors are required and consistent with how thebody.refstring is formatted (which appears to be the full ref). No change needed unless string format changes.
Best Practices
-
Documentation clarity (file:
git-push.yaml, lines ~12-16):
The inline comment is helpful but could be expanded slightly to state the reason for excludingrelease-8.4explicitly (e.g., deprecated, no longer supported), and mention that only exact matches for the listed releases are allowed.Suggested comment enhancement:
# Supported release branches (exact matches only): # - release-6.5 # - release-7.1 # - release-7.5 # - release-8.1 # Note: tidb-binlog is deprecated starting at release-8.4, # so branches 8.4 and later are excluded intentionally.
-
Testing coverage:
The PR description mentions manual regex testing andkustomize buildsuccess. If possible, adding automated tests for this trigger (if the system supports it) or documenting manual test steps in the repo’s test docs would improve future confidence.
No critical issues found. The regex fix is precise and the PR well-scoped. The main suggestion is to prefer explicit branch matches in the regex for better clarity and maintainability.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wuhuizuo 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 |
Summary
The prod2-derived
tidb-binlogbranch regex was malformed:7[.][15]8[.]1matchedrelease-7.18.1/release-7.58.1instead ofrelease-7.1/release-7.5.Fix the filter to match the intended branches:
release-6.5release-7.1release-7.5release-8.1tidb-binlogis deprecated atrelease-8.4, so that branch and later are intentionally excluded.Testing
kustomize build tekton/v1/triggers/triggers/env-gcppasses.