Skip to content

Commit 422bbde

Browse files
committed
feat(overlay): implement conflict-detection simulate in preflight
The simulateOverlayInstall seam was a no-op, so a to-install package declaring Conflicts:/Breaks: against a present baseline package was not caught by the preflight gate: conflictPolicy=fail had no conflict source to act on, and the clash only surfaced as an opaque dpkg -i / rpm -i abort at unpack time. Implement the seam as a metadata-based simulation: read each to-install artifact's declared conflicts on the host (dpkg -f Conflicts/Breaks, rpm -qp --conflicts) and classify each one that names a package present in the baseline (at a version the conflict's range covers) as an ActionConflict, gated by conflictPolicy. It runs without a chroot or a real package-manager invocation, so it is deterministic and always executes. Undeclared file-level collisions are not covered here and still fail loudly at install time; a future live simulator can augment this. Widen the seam signature to (info, baseline, plan) since a metadata simulation needs the baseline installed set to know what is present. Signed-off-by: Tyagi, Yogesh <yogesh.tyagi@intel.com>
1 parent dc94a92 commit 422bbde

4 files changed

Lines changed: 344 additions & 11 deletions

File tree

internal/image/overlay/depcheck.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,134 @@ var readOverlayArtifactObsoletes = func(family PackageManager, plan *ResolutionP
122122
return obs, nil
123123
}
124124

125+
// ArtifactConflict records that a to-install package declares a conflict with
126+
// another package via its deb Conflicts:/Breaks: fields or its rpm Conflicts:.
127+
// When the conflicted package is present in the baseline, installing the
128+
// artifact would abort at the package-manager's unpack/configure step (dpkg -i
129+
// refuses a Conflicts, rpm -i/-U refuses a conflicting file/capability), so the
130+
// preflight classifies it as an ActionConflict gated by conflictPolicy — turning
131+
// an opaque mid-install failure into an up-front, actionable block.
132+
type ArtifactConflict struct {
133+
// Package is the to-install package declaring the conflict.
134+
Package string
135+
// Conflicts is the (name + optional version constraint) of the package it
136+
// conflicts with. Constraint is nil for an unversioned conflict (any version).
137+
Conflicts DependencyAlternative
138+
}
139+
140+
// readOverlayArtifactConflicts is the impure seam that reads the conflict
141+
// declarations of every plan.ToInstall artifact (deb Conflicts:/Breaks:, rpm
142+
// Conflicts:). Like the dependency and Obsoletes readers it is a best-effort
143+
// validation aid feeding the pure preflight, so a read failure is non-fatal: the
144+
// preflight simply loses this one net and the install would instead fail loudly
145+
// at unpack time. Tests override it to inject synthetic conflicts.
146+
var readOverlayArtifactConflicts = func(family PackageManager, plan *ResolutionPlan) ([]ArtifactConflict, error) {
147+
if plan == nil || len(plan.ToInstall) == 0 {
148+
return nil, nil
149+
}
150+
if strings.TrimSpace(plan.DownloadDir) == "" {
151+
return nil, fmt.Errorf("overlay conflict check: plan has packages to install but no artifact download directory")
152+
}
153+
154+
var conflicts []ArtifactConflict
155+
for _, rp := range plan.ToInstall {
156+
artifact, err := artifactFileFor(rp)
157+
if err != nil {
158+
return nil, err
159+
}
160+
hostPath := joinArtifactPath(plan.DownloadDir, artifact)
161+
162+
var edges []ArtifactConflict
163+
switch family {
164+
case PackageManagerDNF:
165+
edges, err = readRPMArtifactConflicts(rp.Name, hostPath)
166+
default:
167+
edges, err = readDebArtifactConflicts(rp.Name, hostPath)
168+
}
169+
if err != nil {
170+
// Best-effort: a single unreadable artifact must not fail the preflight;
171+
// the two-slice model and the remaining artifacts still gate the build.
172+
log.Warnf("Overlay conflict check: failed to read conflicts of %q from %s (continuing): %v", rp.Name, hostPath, err)
173+
continue
174+
}
175+
conflicts = append(conflicts, edges...)
176+
}
177+
return conflicts, nil
178+
}
179+
180+
// readDebArtifactConflicts reads the Conflicts and Breaks control fields of a
181+
// prepared .deb with `dpkg -f` and parses their (optionally versioned) entries.
182+
// Both fields are read because dpkg -i refuses to unpack over either one. The
183+
// file is read on the host, so no chroot is entered.
184+
func readDebArtifactConflicts(pkgName, hostPath string) ([]ArtifactConflict, error) {
185+
var conflicts []ArtifactConflict
186+
for _, field := range []string{"Conflicts", "Breaks"} {
187+
// hostPath is a URL-derived artifact path; quote it before interpolating it
188+
// into the bash -c command so metacharacters can't alter execution.
189+
out, err := shell.ExecCmdSilent(fmt.Sprintf("dpkg -f %s %s", shell.QuoteArg(hostPath), field), true, shell.HostPath, nil)
190+
if err != nil {
191+
return nil, fmt.Errorf("reading %s of %s: %w", field, hostPath, err)
192+
}
193+
conflicts = append(conflicts, parseDebConflictsField(pkgName, out)...)
194+
}
195+
return conflicts, nil
196+
}
197+
198+
// readRPMArtifactConflicts reads a prepared .rpm's Conflicts with
199+
// `rpm -qp --conflicts` (rpm is on the shell allowlist) and parses each entry.
200+
func readRPMArtifactConflicts(pkgName, hostPath string) ([]ArtifactConflict, error) {
201+
// hostPath is a URL-derived artifact path; quote it before interpolating it
202+
// into the bash -c command so metacharacters can't alter execution.
203+
out, err := shell.ExecCmdSilent(fmt.Sprintf("rpm -qp --conflicts %s", shell.QuoteArg(hostPath)), true, shell.HostPath, nil)
204+
if err != nil {
205+
return nil, fmt.Errorf("reading conflicts of %s: %w", hostPath, err)
206+
}
207+
return parseRPMConflicts(pkgName, out), nil
208+
}
209+
210+
// parseDebConflictsField parses a deb Conflicts/Breaks field value into conflict
211+
// entries. The field is a comma-separated list of "name[:arch] [(op ver)]" terms;
212+
// unlike Depends it carries no "a | b" alternatives (Debian policy forbids them
213+
// in Conflicts/Breaks), so each comma term is a single conflicted package. It
214+
// reuses parseDebAlternative for the name/version parsing.
215+
func parseDebConflictsField(pkgName, field string) []ArtifactConflict {
216+
var conflicts []ArtifactConflict
217+
for _, term := range strings.Split(field, ",") {
218+
term = strings.TrimSpace(term)
219+
if term == "" {
220+
continue
221+
}
222+
if a, ok := parseDebAlternative(term); ok {
223+
conflicts = append(conflicts, ArtifactConflict{Package: pkgName, Conflicts: a})
224+
}
225+
}
226+
return conflicts
227+
}
228+
229+
// parseRPMConflicts parses `rpm -qp --conflicts` output. Each non-empty line is
230+
// either a bare capability name or "name op version" (a versioned conflict).
231+
// File and rpmlib entries are skipped as in parseRPMObsoletes.
232+
func parseRPMConflicts(pkgName, out string) []ArtifactConflict {
233+
var conflicts []ArtifactConflict
234+
for _, line := range strings.Split(out, "\n") {
235+
line = strings.TrimSpace(line)
236+
if line == "" || strings.HasPrefix(line, "/") || strings.HasPrefix(line, "rpmlib(") {
237+
continue
238+
}
239+
fields := strings.Fields(line)
240+
switch len(fields) {
241+
case 1:
242+
// Unversioned conflict: conflicts with the named package at any version.
243+
conflicts = append(conflicts, ArtifactConflict{Package: pkgName, Conflicts: DependencyAlternative{Name: fields[0]}})
244+
case 3:
245+
if c, ok := parseConstraint(fields[1] + " " + fields[2]); ok {
246+
conflicts = append(conflicts, ArtifactConflict{Package: pkgName, Conflicts: DependencyAlternative{Name: fields[0], Constraint: &c}})
247+
}
248+
}
249+
}
250+
return conflicts
251+
}
252+
125253
// readRPMArtifactObsoletes reads a prepared .rpm's Obsoletes with
126254
// `rpm -qp --obsoletes` (rpm is on the shell allowlist) and parses each entry.
127255
func readRPMArtifactObsoletes(pkgName, hostPath string) ([]ArtifactObsoletion, error) {

internal/image/overlay/depcheck_test.go

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package overlay
22

33
import (
44
"reflect"
5+
"strings"
56
"testing"
67

78
"github.com/open-edge-platform/image-composer-tool/internal/config"
@@ -215,3 +216,148 @@ func TestEvaluatePreflight_UnsatisfiedDepAlternativeRescues(t *testing.T) {
215216
t.Errorf("expected no block when an alternative satisfies the edge, got %+v", report)
216217
}
217218
}
219+
220+
func TestParseDebConflictsField(t *testing.T) {
221+
// A realistic Conflicts line: a bare conflict, a versioned conflict, and a
222+
// multiarch-qualified one. Conflicts/Breaks carry no "a | b" alternatives.
223+
field := "oldpkg, libfoo (<< 2.0), bar:amd64 (= 1.0)"
224+
conflicts := parseDebConflictsField("mypkg", field)
225+
if len(conflicts) != 3 {
226+
t.Fatalf("got %d conflicts, want 3: %+v", len(conflicts), conflicts)
227+
}
228+
if conflicts[0].Package != "mypkg" || conflicts[0].Conflicts.Name != "oldpkg" || conflicts[0].Conflicts.Constraint != nil {
229+
t.Errorf("conflict[0] = %+v, want bare oldpkg", conflicts[0])
230+
}
231+
if conflicts[1].Conflicts.Name != "libfoo" || conflicts[1].Conflicts.Constraint == nil ||
232+
conflicts[1].Conflicts.Constraint.Op != "<<" || conflicts[1].Conflicts.Constraint.Ver != "2.0" {
233+
t.Errorf("conflict[1] = %+v, want libfoo (<< 2.0)", conflicts[1])
234+
}
235+
if conflicts[2].Conflicts.Name != "bar" || conflicts[2].Conflicts.Constraint == nil {
236+
t.Errorf("conflict[2] = %+v, want bar (= 1.0) with arch stripped", conflicts[2])
237+
}
238+
}
239+
240+
func TestParseRPMConflicts(t *testing.T) {
241+
out := "oldpkg\nlibfoo < 2.0\n/some/file\nrpmlib(Something) <= 4.0\n"
242+
conflicts := parseRPMConflicts("mypkg", out)
243+
// The bare name and the versioned conflict are kept; the file and rpmlib
244+
// entries are skipped.
245+
if len(conflicts) != 2 {
246+
t.Fatalf("got %d conflicts, want 2: %+v", len(conflicts), conflicts)
247+
}
248+
if conflicts[0].Conflicts.Name != "oldpkg" || conflicts[0].Conflicts.Constraint != nil {
249+
t.Errorf("conflict[0] = %+v, want bare oldpkg", conflicts[0])
250+
}
251+
if conflicts[1].Conflicts.Name != "libfoo" || conflicts[1].Conflicts.Constraint == nil ||
252+
conflicts[1].Conflicts.Constraint.Op != "<" {
253+
t.Errorf("conflict[1] = %+v, want libfoo < 2.0", conflicts[1])
254+
}
255+
}
256+
257+
// TestClassifyConflicts covers the pure conflict classifier: a declared conflict
258+
// against a present baseline package fires (bare and versioned-in-range), and a
259+
// conflict against an absent package or a versioned range that excludes the
260+
// baseline version does not.
261+
func TestClassifyConflicts(t *testing.T) {
262+
sliceA := baselineVersionIndex([]BaselinePackage{
263+
installedDeb("oldpkg", "1.0"),
264+
installedDeb("libfoo", "1.5"),
265+
installedDeb("libbar", "3.0"),
266+
})
267+
268+
tests := []struct {
269+
name string
270+
conflicts []ArtifactConflict
271+
wantCount int
272+
wantTarget string
273+
wantConflict string // the declaring artifact (ConflictWith)
274+
}{
275+
{
276+
name: "bare conflict on present package fires",
277+
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "oldpkg"}}},
278+
wantCount: 1,
279+
wantTarget: "oldpkg",
280+
wantConflict: "newpkg",
281+
},
282+
{
283+
name: "conflict on absent package is skipped",
284+
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "absent-pkg"}}},
285+
wantCount: 0,
286+
},
287+
{
288+
name: "versioned conflict in range fires",
289+
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "libfoo", Constraint: &VersionConstraint{"<<", "2.0"}}}},
290+
wantCount: 1,
291+
wantTarget: "libfoo",
292+
wantConflict: "newpkg",
293+
},
294+
{
295+
name: "versioned conflict out of range is skipped",
296+
conflicts: []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "libbar", Constraint: &VersionConstraint{"<<", "2.0"}}}},
297+
wantCount: 0,
298+
},
299+
}
300+
for _, tt := range tests {
301+
t.Run(tt.name, func(t *testing.T) {
302+
actions := classifyConflicts(PackageManagerAPT, sliceA, tt.conflicts)
303+
if len(actions) != tt.wantCount {
304+
t.Fatalf("got %d actions, want %d: %+v", len(actions), tt.wantCount, actions)
305+
}
306+
if tt.wantCount == 0 {
307+
return
308+
}
309+
if actions[0].Type != ActionConflict {
310+
t.Errorf("type = %s, want %s", actions[0].Type, ActionConflict)
311+
}
312+
if actions[0].Package != tt.wantTarget {
313+
t.Errorf("target = %q, want %q", actions[0].Package, tt.wantTarget)
314+
}
315+
if actions[0].ConflictWith != tt.wantConflict {
316+
t.Errorf("conflictWith = %q, want %q", actions[0].ConflictWith, tt.wantConflict)
317+
}
318+
})
319+
}
320+
}
321+
322+
// TestSimulateOverlayInstall_DeclaredConflictBlocked exercises the full seam:
323+
// the default simulateOverlayInstall reads artifact conflicts (overridden here to
324+
// avoid a real dpkg call) and feeds them through Preflight, which must block the
325+
// declared conflict against a present baseline package under the default fail
326+
// conflict policy — the end-to-end regression for the "conflict slips past the
327+
// gate" gap.
328+
func TestSimulateOverlayInstall_DeclaredConflictBlocked(t *testing.T) {
329+
origRead := readOverlayArtifactConflicts
330+
defer func() { readOverlayArtifactConflicts = origRead }()
331+
readOverlayArtifactConflicts = func(PackageManager, *ResolutionPlan) ([]ArtifactConflict, error) {
332+
return []ArtifactConflict{{Package: "newpkg", Conflicts: DependencyAlternative{Name: "oldpkg"}}}, nil
333+
}
334+
335+
info := &BaselineInfo{OS: "ubuntu", Arch: "amd64", PackageManager: PackageManagerAPT}
336+
baseline := []BaselinePackage{installedDeb("oldpkg", "1.0")}
337+
plan := &ResolutionPlan{
338+
DownloadDir: "/tmp/does-not-matter", // the reader is stubbed
339+
ToInstall: []ResolvedPackage{{Name: "newpkg", Version: "2.0", Arch: "amd64", URL: "https://x/newpkg.deb"}},
340+
}
341+
342+
report, err := Preflight(info, baseline, plan, &config.OverlayPolicy{})
343+
if err == nil || report == nil || !report.Blocked {
344+
t.Fatalf("expected a blocked conflict, err=%v report=%+v", err, report)
345+
}
346+
if report.Conflicts != 1 {
347+
t.Fatalf("conflicts = %d, want 1: %+v", report.Conflicts, report.Actions)
348+
}
349+
if report.Violations[0].Rule != ruleConflictPolicyFail {
350+
t.Errorf("rule = %s, want %s", report.Violations[0].Rule, ruleConflictPolicyFail)
351+
}
352+
if report.Violations[0].Action.ConflictWith != "newpkg" {
353+
t.Errorf("conflicting artifact = %q, want newpkg", report.Violations[0].Action.ConflictWith)
354+
}
355+
if report.Violations[0].Action.CurrentVersion != "1.0" {
356+
t.Errorf("current version = %q, want 1.0 (from baseline)", report.Violations[0].Action.CurrentVersion)
357+
}
358+
for _, want := range []string{"newpkg", "oldpkg", "conflict"} {
359+
if !strings.Contains(err.Error(), want) {
360+
t.Errorf("error %q missing %q", err, want)
361+
}
362+
}
363+
}

internal/image/overlay/preflight.go

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,31 @@ type PreflightInput struct {
182182
Policy config.OverlayPolicy
183183
}
184184

185-
// simulateOverlayInstall is a seam over the optional package-manager simulate
186-
// step (apt-get install --simulate / dnf install --assumeno). Its output is a
187-
// validation aid only — the two-slice model drives the policy decision — so the
188-
// default is a no-op. The install-wiring story can plug a real simulator in, and
189-
// tests override it to exercise the remove/conflict policy paths.
190-
var simulateOverlayInstall = func(info *BaselineInfo, plan *ResolutionPlan) ([]PlannedAction, error) {
191-
return nil, nil
185+
// simulateOverlayInstall simulates the overlay install and returns the
186+
// remove/conflict actions it would trigger, for the policy gate to enforce. Its
187+
// output is a validation aid — the two-slice model still drives add/upgrade/
188+
// downgrade decisions — so a failure here is non-fatal (see Preflight).
189+
//
190+
// The default implementation is a METADATA-based simulation: it reads the
191+
// declared Conflicts:/Breaks: (deb) and Conflicts: (rpm) of every to-install
192+
// artifact on the host and reports each one that names a package present in the
193+
// baseline (at a version the conflict's range covers) as an ActionConflict. This
194+
// catches a declared conflict — which dpkg -i / rpm -i would otherwise only
195+
// reveal by aborting at unpack time — up front, without entering a chroot or
196+
// running the package manager, so it is deterministic and always executes.
197+
//
198+
// It does NOT catch a file-level collision that no package declares (two packages
199+
// shipping the same path): nothing in the artifact metadata expresses that, so it
200+
// still surfaces as a loud install-time failure. A future live simulator
201+
// (apt-get install --simulate / dnf install --assumeno, run in the mounted chroot
202+
// during the install phase) could augment this to cover those cases. Tests
203+
// override this seam to exercise the remove/conflict policy paths directly.
204+
var simulateOverlayInstall = func(info *BaselineInfo, baseline []BaselinePackage, plan *ResolutionPlan) ([]PlannedAction, error) {
205+
conflicts, err := readOverlayArtifactConflicts(info.PackageManager, plan)
206+
if err != nil {
207+
return nil, err
208+
}
209+
return classifyConflicts(info.PackageManager, baselineVersionIndex(baseline), conflicts), nil
192210
}
193211

194212
// Preflight runs the two-slice dependency/conflict preflight for an overlay
@@ -223,8 +241,11 @@ func Preflight(info *BaselineInfo, baseline []BaselinePackage, plan *ResolutionP
223241

224242
// The simulate step is an optional validation aid; its failure must not mask
225243
// the authoritative two-slice decision, so a simulate error is logged and the
226-
// preflight continues on the two-slice model alone.
227-
simulated, err := simulateOverlayInstall(info, plan)
244+
// preflight continues on the two-slice model alone. The default simulator reads
245+
// the to-install artifacts' declared conflicts against the baseline (see
246+
// simulateOverlayInstall), catching a declared Conflicts:/Breaks: before the
247+
// install would abort at unpack time.
248+
simulated, err := simulateOverlayInstall(info, baseline, plan)
228249
if err != nil {
229250
log.Warnf("Overlay preflight: package-manager simulation unavailable, continuing on two-slice model only: %v", err)
230251
simulated = nil
@@ -474,6 +495,44 @@ func classifyObsoletions(family PackageManager, sliceA map[string]BaselinePackag
474495
return actions
475496
}
476497

498+
// classifyConflicts turns each declared Conflicts:/Breaks: (deb) or Conflicts:
499+
// (rpm) on a present baseline package into an ActionConflict, so conflictPolicy
500+
// gates a conflict that the package manager would otherwise only reveal by
501+
// 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 {
507+
var actions []PlannedAction
508+
for _, c := range conflicts {
509+
target := strings.TrimSpace(c.Conflicts.Name)
510+
if target == "" {
511+
continue
512+
}
513+
base, present := sliceA[target]
514+
if !present {
515+
continue // nothing installed under this name to conflict with
516+
}
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.
519+
if vc := c.Conflicts.Constraint; vc != nil {
520+
if cmp, err := comparePkgVersions(family, base.Version, vc.Ver); err == nil && !constraintSatisfied(vc.Op, cmp) {
521+
continue
522+
}
523+
}
524+
actions = append(actions, PlannedAction{
525+
Type: ActionConflict,
526+
Package: target,
527+
CurrentVersion: base.Version,
528+
Arch: base.Arch,
529+
ConflictWith: c.Package,
530+
Detail: fmt.Sprintf("declared as a conflict by %q, which would abort the install at unpack time", c.Package),
531+
})
532+
}
533+
return actions
534+
}
535+
477536
// unsatisfiedVersionedAlternative reports whether a dependency edge is blocked by
478537
// the present-but-wrong-version case, returning the offending alternative. An
479538
// edge holds if ANY alternative is satisfied, so it is unsatisfied only when

0 commit comments

Comments
 (0)