From f637be16b85e3b8c5fb0319c0cae5109ed812f9f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 5 Aug 2026 12:23:24 +0200 Subject: [PATCH] fix(backports/apply): scope cherry-pick to target package, ignore other-package conflicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a commit on main touches multiple packages and is cherry-picked onto a backport branch that does not include all of those packages, git cherry-pick produces modify/delete (DU) conflicts for absent packages, or UU conflicts when the branch evolved those files independently. The apply command was surfacing these as status: conflict, blocking the backport even when the target package's changes had applied cleanly. Added resetNonPackageChanges() which, after every cherry-pick, iterates git status --porcelain and resets every file outside the target package directory back to HEAD. Files present in HEAD are restored via git checkout HEAD; files absent from HEAD (modify/delete conflicts) are removed via git rm --force. Errors are non-fatal — conflictingFiles() is the authoritative gate for genuine remaining conflicts. Three integration tests added: - TestApplyIntegration_IgnoresConflictsInOtherPackages: DU conflict in absent package resolved via git rm → status: success - TestApplyIntegration_IgnoresRegularConflictInOtherPackage: UU conflict in present package reset to branch version → status: success - TestApplyIntegration_DiscardsCleanChangesInOtherPackage: clean change in other package discarded → status: success Co-Authored-By: Claude Sonnet 4.6 --- dev/backports/apply/apply.go | 67 +++- dev/backports/apply/apply_integration_test.go | 362 ++++++++++++++++++ 2 files changed, 426 insertions(+), 3 deletions(-) diff --git a/dev/backports/apply/apply.go b/dev/backports/apply/apply.go index ece0505f62f..b8b35c44902 100644 --- a/dev/backports/apply/apply.go +++ b/dev/backports/apply/apply.go @@ -305,9 +305,12 @@ func (a applier) prepareWorkingBranch(remote, branchName, workingBranch string) // cherryPickOrConflict attempts the cherry-pick. changelog.yml is always // restored to HEAD afterwards — its entries are fully regenerated by this // pipeline (see extractChangelogFields/InsertEntry), so cherry-picked changes -// to it are redundant. manifest.yml is left as cherry-pick merges it: any -// legitimate, non-version content change is preserved, and a conflict that is -// purely a "version:" line difference (expected, since each backport branch +// to it are redundant. All files outside the target package directory are also +// reset to HEAD unconditionally, scoping the backport to the target package +// only: changes (or conflicts) the source commit introduced in other packages +// are irrelevant and discarded. manifest.yml is left as cherry-pick merges it: +// any legitimate, non-version content change is preserved, and a conflict that +// is purely a "version:" line difference (expected, since each backport branch // bumps its own version independently) is auto-resolved in favor of the // current branch — bumpPatchVersion recomputes the version afterwards. A // manifest.yml conflict block containing anything else is left as a genuine @@ -340,6 +343,21 @@ func (a applier) cherryPickOrConflict(sha, branchName, pkg, changelogPath, manif return nil, fmt.Errorf("restoring changelog after cherry-pick: %w", err) } + // Scope the cherry-pick to the target package: reset every file outside + // pkgDir to HEAD, discarding both clean changes and conflicts that belong to + // other packages. This also resolves modify/delete conflicts in other + // packages (e.g. a package absent from this backport branch) in favor of + // the branch state, so conflictingFiles() below only surfaces issues that + // actually require the contributor's attention. + pkgDir := filepath.Dir(manifestPath) + relPkgDir, err := filepath.Rel(a.workDir, pkgDir) + if err != nil { + a.abortCherryPick() + return nil, fmt.Errorf("computing relative package dir: %w", err) + } + relPkgDir = filepath.ToSlash(relPkgDir) + a.resetNonPackageChanges(relPkgDir) + if conflict := a.manifestMissingConflict(sha, branchName, pkg, manifestPath); conflict != nil { return conflict, nil } @@ -394,6 +412,49 @@ func (a applier) abortCherryPick() { _ = a.git.RunToStderr("reset", "--hard", "HEAD") } +// resetNonPackageChanges resets all index and working-tree changes outside +// relPkgDir to HEAD. For each such file it first tries "git checkout HEAD -- +// ", which resolves both clean staged changes and most conflict types. +// When that fails (the file does not exist in HEAD — i.e. a modify/delete +// conflict where HEAD removed the file), it falls back to "git rm --force", +// which removes the file from both the index and the working tree. Errors from +// both commands are non-fatal: conflictingFiles() is authoritative about what +// remains unresolved. +func (a applier) resetNonPackageChanges(relPkgDir string) { + out, err := a.git.Output("status", "--porcelain") + if err != nil { + return + } + var outside []string + for _, line := range strings.Split(out, "\n") { + if len(line) < 4 { + continue + } + file := strings.TrimSpace(line[3:]) + // Renamed entries look like "old -> new"; use only the new path. + if idx := strings.Index(file, " -> "); idx != -1 { + file = file[idx+4:] + } + fileFwd := filepath.ToSlash(file) + pkgPrefix := relPkgDir + "/" + if fileFwd == relPkgDir || strings.HasPrefix(fileFwd, pkgPrefix) { + continue + } + outside = append(outside, file) + } + if len(outside) == 0 { + return + } + fmt.Fprintf(os.Stderr, "note: cherry-pick touched %d file(s) outside %s — resetting to HEAD to scope backport to target package\n", len(outside), relPkgDir) + for _, file := range outside { + if err := a.git.RunToStderr("checkout", "HEAD", "--", file); err != nil { + // HEAD does not have this file (e.g. modify/delete conflict where the + // backport branch removed it): remove it from index and working tree. + _ = a.git.RunToStderr("rm", "--force", "--", file) + } + } +} + // manifestMissingConflict reports a conflict Result if manifestPath does not // exist in the working tree. Called both before the cherry-pick (the // package doesn't exist on the target backport branch at all yet) and after diff --git a/dev/backports/apply/apply_integration_test.go b/dev/backports/apply/apply_integration_test.go index 9888203056c..4c906214e5e 100644 --- a/dev/backports/apply/apply_integration_test.go +++ b/dev/backports/apply/apply_integration_test.go @@ -1029,6 +1029,368 @@ func TestApplyIntegration_ContinuesWhenMainOwnerUnreadable(t *testing.T) { assert.Contains(t, lines[1], "Add backports config") } +// setupIntegrationRepoWithOtherPackageConflict creates a repo where the fix +// commit touches both the target package (kubernetes) and a second package +// (security_detection_engine) that was removed from the backport branch. +// Cherry-picking such a commit produces a modify/delete conflict for +// security_detection_engine/manifest.yml: the file was modified by the +// commit but is absent from the backport branch HEAD. Apply() must succeed +// by scoping the cherry-pick to the target package only, discarding the +// conflict in the other package. +func setupIntegrationRepoWithOtherPackageConflict(t *testing.T) (workDir, fixSHA string) { + t.Helper() + + run := func(dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) + return strings.TrimRight(string(out), "\n") + } + + remoteDir := t.TempDir() + run(remoteDir, "init", "--bare", "-q") + + workDir = t.TempDir() + run(workDir, "clone", "-q", remoteDir, ".") + run(workDir, "config", "user.email", "test@test.com") + run(workDir, "config", "user.name", "Test") + run(workDir, "config", "commit.gpgsign", "false") + + write := func(rel, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(workDir, rel), []byte(content), 0o644)) + } + + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "kubernetes"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "security_detection_engine"), 0o755)) + + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.0\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.0\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Initial release") + baseCommit := run(workDir, "rev-parse", "--short=10", "HEAD") + + write(".backports.yml", "backports:\n"+ + " - package: kubernetes\n"+ + " branch: backport-kubernetes-1.x\n"+ + " base_version: \"1.0.0\"\n"+ + " base_commit: \""+baseCommit+"\"\n"+ + " maintained_until: null\n"+ + " archived: false\n"+ + " remove_other_packages: false\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Add backports config") + run(workDir, "push", "-q", "origin", "HEAD:main") + + // Create the backport branch without security_detection_engine: it is not + // part of this backport, so it is removed from the branch, leaving its + // manifest.yml absent from the branch HEAD. + run(workDir, "checkout", "-q", "-b", "backport-kubernetes-1.x") + run(workDir, "rm", "-r", "-q", "packages/security_detection_engine") + run(workDir, "commit", "-q", "-m", "Remove non-backported packages") + run(workDir, "push", "-q", "origin", "backport-kubernetes-1.x") + run(workDir, "checkout", "-q", "main") + + // Fix commit on main touches both packages. When cherry-picked onto + // backport-kubernetes-1.x, the kubernetes changes apply cleanly but + // security_detection_engine/manifest.yml produces a modify/delete conflict. + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.1\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.1\"\n"+ + " changes:\n"+ + " - description: Fix timeout in metrics collection.\n"+ + " type: bugfix\n"+ + " link: https://github.com/elastic/integrations/pull/999\n"+ + "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.1\n") + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Fix timeout in metrics collection") + fixSHA = run(workDir, "rev-parse", "HEAD") + + return workDir, fixSHA +} + +func TestApplyIntegration_IgnoresConflictsInOtherPackages(t *testing.T) { + workDir, fixSHA := setupIntegrationRepoWithOtherPackageConflict(t) + + result, err := Apply(Options{ + SHA: fixSHA, + Package: "kubernetes", + Target: "backport-kubernetes-1.x", + Remote: "origin", + DryRun: true, + PackagesDir: "packages", + Repository: "elastic/integrations", + WorkDir: workDir, + }) + require.NoError(t, err) + require.NotNil(t, result) + + // The modify/delete conflict in security_detection_engine must not block + // the backport — Apply() scopes the cherry-pick to the target package only. + assert.Equal(t, "success", result.Status) + assert.Equal(t, "1.0.1", result.NewVersion) + + // The kubernetes package must be correctly backported. + manifestData, err := os.ReadFile(filepath.Join(workDir, "packages", "kubernetes", "manifest.yml")) + require.NoError(t, err) + assert.Contains(t, string(manifestData), "version: 1.0.1") + + // security_detection_engine must remain absent from the backport branch — + // the conflict was resolved in favor of the branch state (deleted). + _, statErr := os.Stat(filepath.Join(workDir, "packages", "security_detection_engine")) + assert.True(t, os.IsNotExist(statErr), "security_detection_engine must not be present on the backport branch") +} + +// setupIntegrationRepoWithOtherPackageRegularConflict creates a repo where +// both packages exist on the backport branch, but the backport branch has an +// independent change to security_detection_engine/manifest.yml (description +// changed) that conflicts with the fix commit's change to the same field. +// This produces a UU (both modified) conflict in the other package. +// Apply() must succeed by resetting that file to HEAD, keeping only the +// kubernetes changes. +func setupIntegrationRepoWithOtherPackageRegularConflict(t *testing.T) (workDir, fixSHA string) { + t.Helper() + + run := func(dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) + return strings.TrimRight(string(out), "\n") + } + + remoteDir := t.TempDir() + run(remoteDir, "init", "--bare", "-q") + + workDir = t.TempDir() + run(workDir, "clone", "-q", remoteDir, ".") + run(workDir, "config", "user.email", "test@test.com") + run(workDir, "config", "user.name", "Test") + run(workDir, "config", "commit.gpgsign", "false") + + write := func(rel, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(workDir, rel), []byte(content), 0o644)) + } + + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "kubernetes"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "security_detection_engine"), 0o755)) + + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.0\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", + "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.0\ndescription: Base description.\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Initial release") + baseCommit := run(workDir, "rev-parse", "--short=10", "HEAD") + + write(".backports.yml", "backports:\n"+ + " - package: kubernetes\n"+ + " branch: backport-kubernetes-1.x\n"+ + " base_version: \"1.0.0\"\n"+ + " base_commit: \""+baseCommit+"\"\n"+ + " maintained_until: null\n"+ + " archived: false\n"+ + " remove_other_packages: false\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Add backports config") + run(workDir, "push", "-q", "origin", "HEAD:main") + + // The backport branch keeps security_detection_engine but changes its + // description independently, setting up a genuine UU conflict later. + run(workDir, "checkout", "-q", "-b", "backport-kubernetes-1.x") + write("packages/security_detection_engine/manifest.yml", + "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.0\ndescription: Branch description.\n") + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Branch-specific change to security_detection_engine") + run(workDir, "push", "-q", "origin", "backport-kubernetes-1.x") + run(workDir, "checkout", "-q", "main") + + // Fix commit on main modifies both packages; it also changes the description + // of security_detection_engine — conflicting with the branch's own edit. + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.1\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.1\"\n"+ + " changes:\n"+ + " - description: Fix timeout in metrics collection.\n"+ + " type: bugfix\n"+ + " link: https://github.com/elastic/integrations/pull/999\n"+ + "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", + "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.0\ndescription: Main description.\n") + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Fix timeout in metrics collection") + fixSHA = run(workDir, "rev-parse", "HEAD") + + return workDir, fixSHA +} + +func TestApplyIntegration_IgnoresRegularConflictInOtherPackage(t *testing.T) { + workDir, fixSHA := setupIntegrationRepoWithOtherPackageRegularConflict(t) + + result, err := Apply(Options{ + SHA: fixSHA, + Package: "kubernetes", + Target: "backport-kubernetes-1.x", + Remote: "origin", + DryRun: true, + PackagesDir: "packages", + Repository: "elastic/integrations", + WorkDir: workDir, + }) + require.NoError(t, err) + require.NotNil(t, result) + + // The UU conflict in security_detection_engine must not block the backport. + assert.Equal(t, "success", result.Status) + assert.Equal(t, "1.0.1", result.NewVersion) + + // security_detection_engine must be reset to the branch's own state, not the + // cherry-picked version — the conflict is resolved in favor of HEAD. + sdeData, err := os.ReadFile(filepath.Join(workDir, "packages", "security_detection_engine", "manifest.yml")) + require.NoError(t, err) + assert.Contains(t, string(sdeData), "Branch description") + assert.NotContains(t, string(sdeData), "Main description") +} + +// setupIntegrationRepoWithOtherPackageCleanApply creates a repo where both +// packages exist on the backport branch and the fix commit modifies +// security_detection_engine cleanly (no conflict). Apply() must discard the +// clean change to the other package and succeed using only the kubernetes +// changes. +func setupIntegrationRepoWithOtherPackageCleanApply(t *testing.T) (workDir, fixSHA string) { + t.Helper() + + run := func(dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v: %s", args, out) + return strings.TrimRight(string(out), "\n") + } + + remoteDir := t.TempDir() + run(remoteDir, "init", "--bare", "-q") + + workDir = t.TempDir() + run(workDir, "clone", "-q", remoteDir, ".") + run(workDir, "config", "user.email", "test@test.com") + run(workDir, "config", "user.name", "Test") + run(workDir, "config", "commit.gpgsign", "false") + + write := func(rel, content string) { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(workDir, rel), []byte(content), 0o644)) + } + + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "kubernetes"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(workDir, "packages", "security_detection_engine"), 0o755)) + + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.0\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", + "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.0\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Initial release") + baseCommit := run(workDir, "rev-parse", "--short=10", "HEAD") + + write(".backports.yml", "backports:\n"+ + " - package: kubernetes\n"+ + " branch: backport-kubernetes-1.x\n"+ + " base_version: \"1.0.0\"\n"+ + " base_commit: \""+baseCommit+"\"\n"+ + " maintained_until: null\n"+ + " archived: false\n"+ + " remove_other_packages: false\n") + + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Add backports config") + run(workDir, "push", "-q", "origin", "HEAD:main") + + // The backport branch keeps security_detection_engine unchanged. + run(workDir, "checkout", "-q", "-b", "backport-kubernetes-1.x") + run(workDir, "push", "-q", "origin", "backport-kubernetes-1.x") + run(workDir, "checkout", "-q", "main") + + // Fix commit on main modifies both packages; security_detection_engine + // applies cleanly (no conflict) because the branch didn't change it. + write("packages/kubernetes/manifest.yml", "format_version: \"3.0.0\"\nname: kubernetes\ntype: integration\nversion: 1.0.1\n") + write("packages/kubernetes/changelog.yml", "- version: \"1.0.1\"\n"+ + " changes:\n"+ + " - description: Fix timeout in metrics collection.\n"+ + " type: bugfix\n"+ + " link: https://github.com/elastic/integrations/pull/999\n"+ + "- version: \"1.0.0\"\n"+ + " changes:\n"+ + " - description: Initial release.\n"+ + " type: enhancement\n"+ + " link: https://github.com/elastic/integrations/pull/1\n") + write("packages/security_detection_engine/manifest.yml", + "format_version: \"3.0.0\"\nname: security_detection_engine\ntype: integration\nversion: 1.0.1\n") + run(workDir, "add", ".") + run(workDir, "commit", "-q", "-m", "Fix timeout in metrics collection") + fixSHA = run(workDir, "rev-parse", "HEAD") + + return workDir, fixSHA +} + +func TestApplyIntegration_DiscardsCleanChangesInOtherPackage(t *testing.T) { + workDir, fixSHA := setupIntegrationRepoWithOtherPackageCleanApply(t) + + result, err := Apply(Options{ + SHA: fixSHA, + Package: "kubernetes", + Target: "backport-kubernetes-1.x", + Remote: "origin", + DryRun: true, + PackagesDir: "packages", + Repository: "elastic/integrations", + WorkDir: workDir, + }) + require.NoError(t, err) + require.NotNil(t, result) + + // The clean cherry-pick change to security_detection_engine must be discarded. + assert.Equal(t, "success", result.Status) + assert.Equal(t, "1.0.1", result.NewVersion) + + // security_detection_engine must remain at the branch's HEAD version (1.0.0), + // not the cherry-picked version (1.0.1). + sdeData, err := os.ReadFile(filepath.Join(workDir, "packages", "security_detection_engine", "manifest.yml")) + require.NoError(t, err) + assert.Contains(t, string(sdeData), "version: 1.0.0") + assert.NotContains(t, string(sdeData), "version: 1.0.1") +} + func TestApplyIntegration_DryRun(t *testing.T) { workDir, fixSHA := setupIntegrationRepo(t)