Skip to content

Commit 71fe45e

Browse files
committed
fix(overlay): resolve same-batch lockstep breaks/upgrades at install and preflight
Two related overlay-install correctness fixes for lockstep-versioned package sets (e.g. vim-runtime/vim-tiny), where a package Breaks: an older version of another that the overlay upgrades in the same batch: - install: pass --auto-deconfigure to dpkg -i so a transient Breaks: satisfied later in the same batch is deconfigured and reconfigured rather than aborting at unpack time, mirroring apt's behavior. - preflight: evaluate a versioned Conflicts:/Breaks: against the POST-INSTALL version of the target, not the baseline version. When the overlay upgrades the target past the break range in the same batch there is no real conflict, so it must not be flagged. Extract a shared postInstallVersionIndex helper reused by classifyConflicts and classifyUnsatisfiedDeps. Adds regression coverage for both paths. Signed-off-by: Tyagi, Yogesh <yogesh.tyagi@intel.com>
1 parent 422bbde commit 71fe45e

4 files changed

Lines changed: 98 additions & 24 deletions

File tree

internal/image/overlay/depcheck_test.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ func TestClassifyConflicts(t *testing.T) {
267267

268268
tests := []struct {
269269
name string
270+
resolved []ResolvedPackage // the overlay's to-install set (post-install state)
270271
conflicts []ArtifactConflict
271272
wantCount int
272273
wantTarget string
@@ -296,10 +297,20 @@ func TestClassifyConflicts(t *testing.T) {
296297
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "libbar", Constraint: &VersionConstraint{"<<", "2.0"}}}},
297298
wantCount: 0,
298299
},
300+
{
301+
// The overlay upgrades the target past the break range in the same batch,
302+
// so the versioned break no longer covers the post-install version and must
303+
// not be flagged (mirrors vim-runtime "Breaks: vim-tiny (<< X)" while the
304+
// overlay upgrades vim-tiny to X).
305+
name: "versioned break resolved by a same-batch upgrade is skipped",
306+
resolved: []ResolvedPackage{{Name: "libfoo", Version: "2.0"}},
307+
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "libfoo", Constraint: &VersionConstraint{"<<", "2.0"}}}},
308+
wantCount: 0,
309+
},
299310
}
300311
for _, tt := range tests {
301312
t.Run(tt.name, func(t *testing.T) {
302-
actions := classifyConflicts(PackageManagerAPT, sliceA, tt.conflicts)
313+
actions := classifyConflicts(PackageManagerAPT, sliceA, tt.resolved, tt.conflicts)
303314
if len(actions) != tt.wantCount {
304315
t.Fatalf("got %d actions, want %d: %+v", len(actions), tt.wantCount, actions)
305316
}

internal/image/overlay/install.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,23 @@ func (b *debInstallerBackend) install(req installRequest) error {
347347

348348
// Non-interactive install of the local artifacts. dpkg -i takes the prepared
349349
// files directly (no network, no repository resolution), keeping the install
350-
// strictly to the approved, pre-downloaded set. "--" terminates option parsing
351-
// so a URL-derived artifact basename beginning with '-' is treated as a file
352-
// path, not a dpkg option (shell-quoting stops word-splitting, not option parsing).
353-
cmd := "dpkg -i -- " + strings.Join(paths, " ")
350+
// strictly to the approved, pre-downloaded set.
351+
//
352+
// --auto-deconfigure lets dpkg temporarily deconfigure an installed package that
353+
// a to-be-unpacked artifact transiently Breaks, then reconfigure it once the
354+
// batch completes — mirroring what apt does. dpkg unpacks the artifacts in
355+
// command-line order, so when an upgraded package (e.g. vim-runtime) declares
356+
// `Breaks: <other> (<< newver)` against a baseline package that is ALSO being
357+
// upgraded to newver later in the same batch (e.g. vim-tiny), the old version is
358+
// still installed at unpack time and dpkg would otherwise abort with
359+
// "deconfiguration is not permitted". The break is self-resolving within the
360+
// batch (the satisfying version is in the same set), which is why the preflight
361+
// conflict gate correctly permits it; this flag lets dpkg carry it out.
362+
//
363+
// "--" terminates option parsing so a URL-derived artifact basename beginning
364+
// with '-' is treated as a file path, not a dpkg option (shell-quoting stops
365+
// word-splitting, not option parsing).
366+
cmd := "dpkg -i --auto-deconfigure -- " + strings.Join(paths, " ")
354367
envVars := []string{
355368
"DEBIAN_FRONTEND=noninteractive",
356369
"DEBCONF_NONINTERACTIVE_SEEN=true",

internal/image/overlay/install_test.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,7 @@ func TestInstallCommandsTerminateOptions(t *testing.T) {
758758
}{
759759
{
760760
name: "dpkg install",
761-
want: "dpkg -i -- ",
761+
want: "dpkg -i --auto-deconfigure -- ",
762762
run: func(shell.Executor) { _ = (&debInstallerBackend{}).install(req) },
763763
},
764764
{
@@ -797,3 +797,33 @@ func TestInstallCommandsTerminateOptions(t *testing.T) {
797797
})
798798
}
799799
}
800+
801+
// TestDebInstallUsesAutoDeconfigure guards that the deb install passes
802+
// --auto-deconfigure. Without it, dpkg unpacks artifacts in command-line order and
803+
// aborts ("deconfiguration is not permitted") when a to-be-unpacked artifact
804+
// transiently Breaks an installed package that is ALSO being upgraded later in the
805+
// same batch (the vim-runtime Breaks vim-tiny (<< newver) case): the old version is
806+
// still present at unpack time. The break is self-resolving within the batch, so
807+
// the preflight gate permits it; the flag lets dpkg complete it by temporarily
808+
// deconfiguring and then reconfiguring the affected package.
809+
func TestDebInstallUsesAutoDeconfigure(t *testing.T) {
810+
req := installRequest{
811+
chrootPath: "/mnt/root",
812+
artifactChrootDir: chrootArtifactDir,
813+
items: []plannedInstall{
814+
{pkg: ResolvedPackage{Name: "vim-runtime"}, artifact: "vim-runtime_9.deb"},
815+
{pkg: ResolvedPackage{Name: "vim-tiny"}, artifact: "vim-tiny_9.deb"},
816+
},
817+
}
818+
cap := &capturingExecutor{}
819+
stubShell(t, cap)
820+
if err := (&debInstallerBackend{}).install(req); err != nil {
821+
t.Fatalf("install: %v", err)
822+
}
823+
if len(cap.cmds) != 1 {
824+
t.Fatalf("expected exactly one command, got %v", cap.cmds)
825+
}
826+
if !strings.Contains(cap.cmds[0], "--auto-deconfigure") {
827+
t.Errorf("dpkg install command missing --auto-deconfigure: %q", cap.cmds[0])
828+
}
829+
}

internal/image/overlay/preflight.go

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ var simulateOverlayInstall = func(info *BaselineInfo, baseline []BaselinePackage
206206
if err != nil {
207207
return nil, err
208208
}
209-
return classifyConflicts(info.PackageManager, baselineVersionIndex(baseline), conflicts), nil
209+
return classifyConflicts(info.PackageManager, baselineVersionIndex(baseline), plan.ToInstall, conflicts), nil
210210
}
211211

212212
// Preflight runs the two-slice dependency/conflict preflight for an overlay
@@ -429,15 +429,7 @@ func classifyUnsatisfiedDeps(family PackageManager, sliceA map[string]BaselinePa
429429
// Post-install version index: the baseline overlaid with what to-install adds.
430430
// A dependency is checked against the state that will exist after install, so a
431431
// pin satisfied by a co-installed to-install package is correctly not flagged.
432-
postInstall := make(map[string]string, len(sliceA)+len(resolved))
433-
for name, bp := range sliceA {
434-
postInstall[name] = bp.Version
435-
}
436-
for _, rp := range resolved {
437-
if name := strings.TrimSpace(rp.Name); name != "" {
438-
postInstall[name] = rp.Version
439-
}
440-
}
432+
postInstall := postInstallVersionIndex(sliceA, resolved)
441433

442434
var actions []PlannedAction
443435
for _, dep := range deps {
@@ -499,11 +491,20 @@ func classifyObsoletions(family PackageManager, sliceA map[string]BaselinePackag
499491
// (rpm) on a present baseline package into an ActionConflict, so conflictPolicy
500492
// gates a conflict that the package manager would otherwise only reveal by
501493
// aborting at unpack time. A conflict whose target is absent from the baseline is
502-
// a no-op (nothing to clash with) and is skipped; a versioned conflict only fires
503-
// when the baseline version falls within the declared range, and an uncomparable
504-
// version is treated conservatively as a potential conflict (better to gate than
505-
// to miss it).
506-
func classifyConflicts(family PackageManager, sliceA map[string]BaselinePackage, conflicts []ArtifactConflict) []PlannedAction {
494+
// a no-op (nothing to clash with) and is skipped.
495+
//
496+
// A versioned conflict is evaluated against the POST-INSTALL version of the
497+
// target, not the baseline version: a Breaks:/Conflicts: bound to a version range
498+
// (e.g. vim-runtime's "Breaks: vim-tiny (<< 9.1.0016-1ubuntu7.17)") is a lockstep
499+
// upgrade marker, and when the overlay upgrades that target to a satisfying
500+
// version in the SAME batch the range no longer covers it, so there is no real
501+
// conflict — dpkg's --auto-deconfigure resolves the transient break at unpack
502+
// time. Checking the baseline version alone would spuriously block that upgrade.
503+
// An uncomparable version is treated conservatively as a potential conflict
504+
// (better to gate than to miss it).
505+
func classifyConflicts(family PackageManager, sliceA map[string]BaselinePackage, resolved []ResolvedPackage, conflicts []ArtifactConflict) []PlannedAction {
506+
postInstall := postInstallVersionIndex(sliceA, resolved)
507+
507508
var actions []PlannedAction
508509
for _, c := range conflicts {
509510
target := strings.TrimSpace(c.Conflicts.Name)
@@ -514,10 +515,12 @@ func classifyConflicts(family PackageManager, sliceA map[string]BaselinePackage,
514515
if !present {
515516
continue // nothing installed under this name to conflict with
516517
}
517-
// A versioned conflict only clashes when the baseline copy's version
518-
// satisfies the constraint; a version outside the range is not a conflict.
518+
// A versioned conflict only clashes when the version that will be present
519+
// after install falls within the declared range; a version outside it — most
520+
// commonly because the overlay upgrades the target in the same batch — is not
521+
// a conflict.
519522
if vc := c.Conflicts.Constraint; vc != nil {
520-
if cmp, err := comparePkgVersions(family, base.Version, vc.Ver); err == nil && !constraintSatisfied(vc.Op, cmp) {
523+
if cmp, err := comparePkgVersions(family, postInstall[target], vc.Ver); err == nil && !constraintSatisfied(vc.Op, cmp) {
521524
continue
522525
}
523526
}
@@ -533,6 +536,23 @@ func classifyConflicts(family PackageManager, sliceA map[string]BaselinePackage,
533536
return actions
534537
}
535538

539+
// postInstallVersionIndex builds a name→version map of the state that will exist
540+
// after the overlay install: the baseline overlaid with the versions the overlay
541+
// will install (which supersede the baseline copy for any upgraded package). It
542+
// backs the post-install checks in classifyConflicts and classifyUnsatisfiedDeps.
543+
func postInstallVersionIndex(sliceA map[string]BaselinePackage, resolved []ResolvedPackage) map[string]string {
544+
postInstall := make(map[string]string, len(sliceA)+len(resolved))
545+
for name, bp := range sliceA {
546+
postInstall[name] = bp.Version
547+
}
548+
for _, rp := range resolved {
549+
if name := strings.TrimSpace(rp.Name); name != "" {
550+
postInstall[name] = rp.Version
551+
}
552+
}
553+
return postInstall
554+
}
555+
536556
// unsatisfiedVersionedAlternative reports whether a dependency edge is blocked by
537557
// the present-but-wrong-version case, returning the offending alternative. An
538558
// edge holds if ANY alternative is satisfied, so it is unsatisfied only when

0 commit comments

Comments
 (0)