ci(ff): support feature freeze automation - #7339
Open
fr4nc1sc0-r4m0n wants to merge 30 commits into
Open
fr4nc1sc0-r4m0n wants to merge 30 commits into
fr4nc1sc0-r4m0n wants to merge 30 commits into
Conversation
Resolve go.mod/go.sum conflicts by keeping main dependency versions and retaining go-git/go-github deps required for release automation.
fr4nc1sc0-r4m0n
added
Team:Elastic-Agent-Control-Plane
fr4nc1sc0-r4m0n
added
Team:Elastic-Agent-Control-Plane
|
| Status | Scan Engine |  Critical |
 High |
 Medium |
 Low |
Total (0) |
|---|---|---|---|---|---|---|
| Open Source Security | 0 | 0 | 0 | 0 | See details | |
| Licenses | 0 | 0 | 0 | 0 | See details |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
Contributor
|
This pull request does not have a backport label. Could you fix it @fr4nc1sc0-r4m0n? 🙏
|
fr4nc1sc0-r4m0n
added
backport-active-9
8 tasks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Allow release workflows to be safely retriggered by no-oping when the target version is already applied, reusing existing branches and open PRs, and skipping empty commits.
Move release workflow logic into a dedicated package aligned with elastic-agent and beats, leaving thin mage release:* wrappers in magefile.go.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Enable gomoddirectives replace-local so the dev-tools submodule replace required by mage release imports passes golangci-lint.
The root go.mod replaces dev-tools with a local path. Docker layer caching must include dev-tools/go.mod and go.sum so go mod download can resolve the replaced module.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
dev-tools/mage/release/go.mod:5
- This nested module declares
go 1.26.5, which is inconsistent with the repo toolchain (.go-versionis 1.26.7, and the root/dev-tools modules usego 1.26.7). Keeping the go version aligned avoids unexpected toolchain/format churn when runninggo mod tidyacross modules.
go 1.26.5
dev-tools/mage/release/mergify.go:32
UpdateMergifyvalidateslen(parts) < 2, but the error message says it expectsX.Y.Z. This function is called withcfg.ReleaseBranch(e.g.9.6), so the message should reflect the accepted inputs (X.Y or X.Y.Z) to avoid confusing failures.
parts := strings.Split(version, ".")
if len(parts) < 2 {
return fmt.Errorf("invalid version format: %s (expected X.Y.Z)", version)
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
dev-tools/mage/release/mergify.go:27
- UpdateMergify unmarshals .mergify.yml into map[string]interface{} and then marshals it back, which will drop comments and likely reformat/reorder large parts of the file. In practice this makes PR-A very noisy and risks producing a YAML shape/style Mergify doesn’t accept (e.g., branch names like 8.16/9.5 may be emitted unquoted). Consider switching to an implementation that preserves the original file (e.g., append a properly-indented rule text block to the existing pull_request_rules list, or manipulate a yaml.Node tree and force branch scalars to be !!str/double-quoted) so the diff is limited to the new rule only.
var config map[string]interface{}
if err := yaml.Unmarshal(content, &config); err != nil {
return fmt.Errorf("failed to parse %s: %w", mergifyFile, err)
}
Comment on lines
+235
to
+255
| const prSection = "## PRs" | ||
| idx := strings.Index(body, prSection) | ||
| if idx < 0 { | ||
| updated := strings.TrimRight(body, "\n") + "\n\n" + prSection + "\n\n" + formatPRChecklist(allURLs, existingChecked) | ||
| return updated, true | ||
| } | ||
|
|
||
| before := body[:idx] | ||
| oldPRBlock := strings.TrimPrefix(body[idx+len(prSection):], "\n") | ||
| newPRBlock := formatPRChecklist(allURLs, existingChecked) | ||
|
|
||
| oldURLs := keys(extractPRCheckboxes(oldPRBlock)) | ||
| if !sameStringSet(oldURLs, allURLs) || normalizePRSection(oldPRBlock) != normalizePRSection(newPRBlock) { | ||
| changed = true | ||
| } | ||
| if !changed { | ||
| return existingBody, false | ||
| } | ||
|
|
||
| updated := strings.TrimRight(before, "\n") + "\n\n" + prSection + "\n\n" + newPRBlock | ||
| return updated, true |
Comment on lines
+401
to
+418
| func ensurePatchCurrentReleaseMatchesBranch(repo *GitRepo, cfg *ReleaseConfig) error { | ||
| if err := repo.CheckoutBranch(cfg.ReleaseBranch); err != nil { | ||
| return err | ||
| } | ||
| branchVersion, err := ReadFleetVersion() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if branchVersion != cfg.CurrentRelease { | ||
| return fmt.Errorf( | ||
| "CURRENT_RELEASE=%s does not match version on branch %s (%s in version/version.go); "+ | ||
| "set CURRENT_RELEASE to the version already on the release branch (the patch being released)", | ||
| cfg.CurrentRelease, cfg.ReleaseBranch, branchVersion, | ||
| ) | ||
| } | ||
| fmt.Printf("Verified CURRENT_RELEASE=%s matches %s on branch %s\n", cfg.CurrentRelease, branchVersion, cfg.ReleaseBranch) | ||
| return nil | ||
| } |
Comment on lines
+81
to
+83
| if err := g.CheckoutBranch(baseBranch); err != nil { | ||
| return fmt.Errorf("failed to checkout base branch %s: %w", baseBranch, err) | ||
| } |
Contributor
Author
|
buildkite test this |
Contributor
TL;DRBuildkite Remediation
Investigation detailsRoot CauseThe failing step ends in Terraform destroy output and then a generic shell exit status 1, with no preceding Fleet Server test assertion, panic, or compile/runtime error. The tail of the failing log shows only teardown of an Elastic Cloud deployment (
This indicates the failure occurred in the CI script lifecycle around teardown/command handling, not in Fleet Server product logic. Evidence
Verification
Follow-up
What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The Buildkite pipeline drops support for WORKFLOW=major, and the current .mergify.yml update implementation risks rewriting/invalidating the Mergify config.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
dev-tools/mage/release/mergify.go:27
- UpdateMergify unmarshals into
map[string]interface{}and re-marshals the entire .mergify.yml. That will typically rewrite/reorder the whole file (map iteration order is not stable) and can introduce YAML scalar typing issues (e.g. branch "9.5" emitted unquoted and parsed as a float), producing noisy or invalid Mergify configs. Prefer modifying only thepull_request_ruleslist while preserving the original YAML structure (e.g. parse intoyaml.Nodeand append a new node with quoted scalars, or append a formatted YAML snippet underpull_request_rules:).
var config map[string]interface{}
if err := yaml.Unmarshal(content, &config); err != nil {
return fmt.Errorf("failed to parse %s: %w", mergifyFile, err)
}
- Files reviewed: 26/27 changed files
- Comments generated: 4
- Review effort level: Lite
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed workflow-impacting bugs (team reviewer assignment, issue body merging discarding content, and fragile mergify rule detection) that should be fixed before relying on the automation in CI.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
dev-tools/mage/release/issue.go:239
- mergeReleaseIssueBody rebuilds the issue body as
before + PR section, which drops any content that comes after the existing "## PRs" section (e.g., additional headings/notes below PRs). This contradicts the goal of doing an idempotent merge while preserving the rest of the checklist body.
const prSection = "## PRs"
idx := strings.Index(body, prSection)
if idx < 0 {
updated := strings.TrimRight(body, "\n") + "\n\n" + prSection + "\n\n" + formatPRChecklist(allURLs, existingChecked)
return updated, true
dev-tools/mage/release/go.mod:5
- The nested release module declares
go 1.26.5, but the repository toolchain is pinned to Go 1.26.7 (see root go.mod and .go-version). Keeping the same Go version across modules avoids subtle behavior differences and makes CI/tooling consistent.
go 1.26.5
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
Correct team reviewer assignment, exact Mergify rule matching, and remote-only branch checkout so release workflows work reliably in CI.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The release workflows currently perform an unused “latest release” GitHub lookup and the Mergify updater risks rewriting the entire complex .mergify.yml, both of which can introduce avoidable external failures and config breakage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
dev-tools/mage/release/workflows.go:110
- cfg.EnsureLatestRelease() is invoked here but cfg.LatestRelease is never used anywhere in this workflow. This adds an unnecessary GitHub API dependency (including in DRY_RUN) and can fail due to network/rate limits; remove this call or defer it to the specific step that actually consumes LatestRelease.
This issue also appears on line 343 of the same file.
if err := cfg.EnsureLatestRelease(); err != nil {
return err
}
dev-tools/mage/release/workflows.go:345
- cfg.EnsureLatestRelease() is invoked here but cfg.LatestRelease is not used anywhere in the patch workflow. This forces a GitHub API lookup (even for DRY_RUN) that can fail due to network/rate limits; remove this call or only run it in the code path that actually needs LatestRelease.
if err := cfg.EnsureLatestRelease(); err != nil {
return err
}
dev-tools/mage/release/mergify.go:27
- UpdateMergify unmarshals .mergify.yml into a generic map and marshals it back. Given the current .mergify.yml includes many complex rules (multiline blocks, regex strings, and quoted branch names like "8.10"), this approach will rewrite large parts of the file and may change scalar rendering (e.g., turning quoted branch strings into YAML numbers), risking Mergify config breakage and noisy diffs. Prefer inserting the new rule with a minimal text append under pull_request_rules, or edit a yaml.Node tree while forcing branch values to be !!str/quoted and preserving existing structure as much as possible.
var config map[string]interface{}
if err := yaml.Unmarshal(content, &config); err != nil {
return fmt.Errorf("failed to parse %s: %w", mergifyFile, err)
}
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The new automation has correctness/operational issues (issue body merging can drop content, PR text is misleading in one workflow, and Mergify updates round-trip YAML in a way that can rewrite the entire config file).
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
dev-tools/mage/release/workflows.go:444
- prepPatchBeforeBuild uses the PR title/commit message "Update docs versions" even though the surrounding comment and PR body explicitly state Fleet Server has no docs/test-env updates in this step. This mismatch will make the generated PRs misleading during releases.
commitMsg := fmt.Sprintf("[Release %s] Update docs versions %s", cfg.CurrentRelease, cfg.CurrentRelease)
if _, err := repo.CommitAll(commitMsg, cfg.GitAuthorName, cfg.GitAuthorEmail); err != nil {
return workflowPR{}, err
dev-tools/mage/release/issue.go:246
- mergeReleaseIssueBody rebuilds the issue body starting at the "## PRs" header but drops any content that appears after the PRs section (because it replaces everything from idx onward with a newly generated PR checklist). That can unintentionally delete additional sections/notes someone may have added below the PR checklist.
before := body[:idx]
oldPRBlock := strings.TrimPrefix(body[idx+len(prSection):], "\n")
newPRBlock := formatPRChecklist(allURLs, existingChecked)
oldURLs := keys(extractPRCheckboxes(oldPRBlock))
dev-tools/mage/release/mergify.go:27
- UpdateMergify unmarshals the entire .mergify.yml into a map and then yaml.Marshal()s it back out. In the current repository, .mergify.yml contains many rules and multi-line block scalars (e.g. message: | …), so re-marshalling will likely rewrite the whole file (losing formatting/order/comments) and create very noisy PR diffs for each release; it also increases the risk of subtle YAML output differences changing behavior.
var config map[string]interface{}
if err := yaml.Unmarshal(content, &config); err != nil {
return fmt.Errorf("failed to parse %s: %w", mergifyFile, err)
}
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Lite
Contributor
Author
|
buildkite test this |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The release tooling currently has a destructive issue-body merge behavior and a push/auth implementation that can fail in common SSH-remote setups, both of which can break or corrupt release automation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
dev-tools/mage/release/issue.go:246
- mergeReleaseIssueBody() treats everything after the "## PRs" header as the PR checklist block and rewrites it entirely, which will drop any user-added content that appears after the PRs section (e.g., notes or additional headings). This makes issue updates potentially destructive.
before := body[:idx]
oldPRBlock := strings.TrimPrefix(body[idx+len(prSection):], "\n")
newPRBlock := formatPRChecklist(allURLs, existingChecked)
oldURLs := keys(extractPRCheckboxes(oldPRBlock))
- Files reviewed: 26/27 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+234
to
+258
| func (g *GitRepo) Push(remoteName string) error { | ||
| token := os.Getenv("GITHUB_TOKEN") | ||
| if token == "" { | ||
| return fmt.Errorf("GITHUB_TOKEN environment variable is required for pushing") | ||
| } | ||
|
|
||
| currentBranch, err := g.GetCurrentBranch() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| refSpec := config.RefSpec(fmt.Sprintf("refs/heads/%s:refs/heads/%s", currentBranch, currentBranch)) | ||
|
|
||
| err = g.repo.Push(&git.PushOptions{ | ||
| RemoteName: remoteName, | ||
| RefSpecs: []config.RefSpec{refSpec}, | ||
| Auth: &http.BasicAuth{ | ||
| Username: "git", | ||
| Password: token, | ||
| }, | ||
| }) | ||
| if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) { | ||
| return fmt.Errorf("failed to push: %w", err) | ||
| } | ||
|
|
| // enter the main fleet-server go.mod / NOTICE.txt. | ||
| module github.com/elastic/fleet-server/dev-tools/mage/release | ||
|
|
||
| go 1.26.7 |
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
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.
What is the problem this PR solves?
Fleet Server release automation still relies on Makefile-based scripts and external CLI tools. This PR migrates the feature-freeze and patch release workflows to pure Go Mage targets so release steps can run without depending on hub, gh, sed, yq, or Python.
This continues the work from #6584 and aligns the process with beats (#51831) and elastic-agent (#15433).
How does this PR solve the problem?
dev-tools/mage/release/withcmd/fleet-releaseCLI sogo-git/go-githubstay out of the rootgo.mod/NOTICE.txtmage release:*wrappers invoke the nested CLIrunNextRelease(next-patch prep is PR-D / patch PR-B)DRY_RUN=truemode for safe local testingRELEASE.mdanddev-tools/mage/release/README.mdWorkflow alignment
fleet-server.mak)prepare-major-minor-release+create-branch-major-minor-release(+ next-release steps)mage release:runMajorMinorX.Y; PR-A (main: mergify + next minor); PR-D (release branch: next patch)prepare-patch-release+create-prs-patch-releasemage release:runPatchFeature freeze (
CURRENT_RELEASEalready onmain)X.Yfrommainff-prep-main-{CURRENT}→mainmerge:1-ff-dayff-prep-next-patch-{NEXT}→X.Ymerge:4-after-releasePatch (
CURRENT_RELEASEalready on release branch)ff-prep-next-patch-{NEXT}→X.Ymerge:4-after-releaseFiles updated
version/version.go—DefaultVersion.mergify.yml— backport rule (PR-A)Idempotency
UpdateVersionUpdateMergifyCommitAllCreatePRHow to test this PR locally
Discard local workflow changes after review with
git reset --hard HEAD.Checklist
./changelog/fragmentsusing the changelog toolRelated issue: https://github.com/elastic/observability-robots/issues/3404
Validations
Minor releases
Former bot flow opened 3 PRs per minor FF (version bump + backport rule + next patch). The new
runMajorMinorflow groups bump + mergify into PR-A and opens PR-D for the next patch (PR-C omitted; no docs/test-env; PR-B is a no-op when the release branch already has the correct version):main+ backport rulePatch releases
Former bot flow opened 1 PR per patch (next-patch version bump only; fleet-server has no docs/manifests patch PR). The new
runPatchflow opens PR-D for the next patch (PR-A ensure-version is usually a no-op and was skipped in this run):