From 563bcb51966dc41641c4a97935a614a49747b16b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Th=C3=B6mmes?= Date: Tue, 16 Dec 2025 15:28:33 +0100 Subject: [PATCH 1/2] Use in-memory control data everywhere Currently, apko sometimes uses an in-memory version of the control file and sometimes reads the data from the control file itself. This makes all of those instances use the cached data. --- pkg/apk/apk/implementation.go | 24 ++++++++---------------- pkg/apk/apk/installed.go | 25 ++++++------------------- pkg/apk/apk/installed_test.go | 8 ++------ 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/pkg/apk/apk/implementation.go b/pkg/apk/apk/implementation.go index b267b608d..980c10988 100644 --- a/pkg/apk/apk/implementation.go +++ b/pkg/apk/apk/implementation.go @@ -1213,13 +1213,7 @@ func (a *APK) cachedPackage(ctx context.Context, pkg InstallablePackage, cacheDi exp.SignatureHash = signatureHash[:] } - f, err := os.Open(ctl) - if err != nil { - return nil, err - } - defer f.Close() - - datahash, err := a.datahash(f) + datahash, err := a.datahash(bytes.NewReader(control)) if err != nil { return nil, fmt.Errorf("datahash for %s: %w", pkg, err) } @@ -1486,29 +1480,27 @@ func (a *APK) installPackage(ctx context.Context, pkg *Package, expanded *expand } // update the scripts.tar - controlData, err := os.Open(expanded.ControlFile) + controlData, err := expanded.ControlData() if err != nil { return nil, fmt.Errorf("opening control file %q: %w", expanded.ControlFile, err) } - defer controlData.Close() - if err := a.updateScriptsTar(pkg, controlData, sourceDateEpoch); err != nil { + controlTar := bytes.NewReader(controlData) + if err := a.updateScriptsTar(pkg, controlTar, sourceDateEpoch); err != nil { return nil, fmt.Errorf("unable to update scripts.tar for pkg %s: %w", pkg.Name, err) } // update the triggers - if _, err := controlData.Seek(0, 0); err != nil { - return nil, fmt.Errorf("unable to seek to start of control data for pkg %s: %w", pkg.Name, err) - } - if err := a.updateTriggers(pkg, controlData); err != nil { + controlTar.Reset(controlData) + if err := a.updateTriggers(pkg, controlTar); err != nil { return nil, fmt.Errorf("unable to update triggers for pkg %s: %w", pkg.Name, err) } return installedFiles, nil } -func (a *APK) datahash(controlTarGz io.Reader) (string, error) { - values, err := a.controlValue(controlTarGz, "datahash") +func (a *APK) datahash(controlTar io.Reader) (string, error) { + values, err := a.controlValue(controlTar, "datahash") if err != nil { return "", fmt.Errorf("reading datahash from control: %w", err) } diff --git a/pkg/apk/apk/installed.go b/pkg/apk/apk/installed.go index 33fd33ff7..6a1815a84 100644 --- a/pkg/apk/apk/installed.go +++ b/pkg/apk/apk/installed.go @@ -28,8 +28,6 @@ import ( "strconv" "strings" "time" - - "github.com/klauspost/compress/gzip" ) type InstalledPackage struct { @@ -121,13 +119,8 @@ func (a *APK) isInstalledPackage(pkg string) (bool, error) { } // updateScriptsTar insert the scripts into the tarball -func (a *APK) updateScriptsTar(pkg *Package, controlTarGz io.Reader, sourceDateEpoch *time.Time) error { - gz, err := gzip.NewReader(controlTarGz) - if err != nil { - return fmt.Errorf("unable to gunzip control tar.gz file: %w", err) - } - defer gz.Close() - tr := tar.NewReader(gz) +func (a *APK) updateScriptsTar(pkg *Package, controlData io.Reader, sourceDateEpoch *time.Time) error { + tr := tar.NewReader(controlData) fi, err := a.fs.Stat(scriptsFilePath) if err != nil { return fmt.Errorf("unable to stat scripts file: %w", err) @@ -197,14 +190,8 @@ func (a *APK) readScriptsTar() (io.ReadCloser, error) { } // TODO: We should probably parse control section on the first pass and reuse it. -func (a *APK) controlValue(controlTarGz io.Reader, want string) ([]string, error) { - gz, err := gzip.NewReader(controlTarGz) - if err != nil { - return nil, fmt.Errorf("unable to gunzip control tar file: %w", err) - } - defer gz.Close() - - mapping, err := controlValue(gz, want) +func (a *APK) controlValue(controlTar io.Reader, want string) ([]string, error) { + mapping, err := controlValue(controlTar, want) if err != nil { return nil, err } @@ -217,14 +204,14 @@ func (a *APK) controlValue(controlTarGz io.Reader, want string) ([]string, error } // updateTriggers insert the triggers into the triggers file -func (a *APK) updateTriggers(pkg *Package, controlTarGz io.Reader) error { +func (a *APK) updateTriggers(pkg *Package, controlTar io.Reader) error { triggers, err := a.fs.OpenFile(triggersFilePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0) if err != nil { return fmt.Errorf("unable to open triggers file %s: %w", triggersFilePath, err) } defer triggers.Close() - values, err := a.controlValue(controlTarGz, "triggers") + values, err := a.controlValue(controlTar, "triggers") if err != nil { return fmt.Errorf("updating triggers for %s: %w", pkg.Name, err) } diff --git a/pkg/apk/apk/installed_test.go b/pkg/apk/apk/installed_test.go index 4840369d2..e6909e331 100644 --- a/pkg/apk/apk/installed_test.go +++ b/pkg/apk/apk/installed_test.go @@ -361,8 +361,7 @@ func TestUpdateScriptsTar(t *testing.T) { ".post-upgrade": []byte("echo 'post upgrade'"), } var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gw) + tw := tar.NewWriter(&buf) for name, content := range scripts { _ = tw.WriteHeader(&tar.Header{ Name: name, @@ -379,7 +378,6 @@ func TestUpdateScriptsTar(t *testing.T) { }) _, _ = tw.Write([]byte(pkginfo)) tw.Close() - gw.Close() // pass the controltargz to updateScriptsTar r := bytes.NewReader(buf.Bytes()) @@ -450,8 +448,7 @@ func TestUpdateTriggers(t *testing.T) { ".PKGINFO": []byte(pkginfo), } var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gw) + tw := tar.NewWriter(&buf) for name, content := range scripts { _ = tw.WriteHeader(&tar.Header{ Name: name, @@ -461,7 +458,6 @@ func TestUpdateTriggers(t *testing.T) { _, _ = tw.Write(content) } tw.Close() - gw.Close() // pass the controltargz to updateScriptsTar r := bytes.NewReader(buf.Bytes()) From c9fd45572273e115b8b220a5da28e3f6b5cab20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Th=C3=B6mmes?= Date: Tue, 16 Dec 2025 16:16:53 +0100 Subject: [PATCH 2/2] Use parsed ControlFS where possible --- pkg/apk/apk/implementation.go | 9 +++-- pkg/apk/apk/installed.go | 9 ++--- pkg/apk/apk/installed_test.go | 5 ++- pkg/apk/apk/util.go | 63 ++++++++++++++++------------------- 4 files changed, 41 insertions(+), 45 deletions(-) diff --git a/pkg/apk/apk/implementation.go b/pkg/apk/apk/implementation.go index 980c10988..8151e5fd0 100644 --- a/pkg/apk/apk/implementation.go +++ b/pkg/apk/apk/implementation.go @@ -1213,7 +1213,7 @@ func (a *APK) cachedPackage(ctx context.Context, pkg InstallablePackage, cacheDi exp.SignatureHash = signatureHash[:] } - datahash, err := a.datahash(bytes.NewReader(control)) + datahash, err := a.datahash(exp.ControlFS) if err != nil { return nil, fmt.Errorf("datahash for %s: %w", pkg, err) } @@ -1491,16 +1491,15 @@ func (a *APK) installPackage(ctx context.Context, pkg *Package, expanded *expand } // update the triggers - controlTar.Reset(controlData) - if err := a.updateTriggers(pkg, controlTar); err != nil { + if err := a.updateTriggers(pkg, expanded.ControlFS); err != nil { return nil, fmt.Errorf("unable to update triggers for pkg %s: %w", pkg.Name, err) } return installedFiles, nil } -func (a *APK) datahash(controlTar io.Reader) (string, error) { - values, err := a.controlValue(controlTar, "datahash") +func (a *APK) datahash(controlFS fs.FS) (string, error) { + values, err := a.controlValue(controlFS, "datahash") if err != nil { return "", fmt.Errorf("reading datahash from control: %w", err) } diff --git a/pkg/apk/apk/installed.go b/pkg/apk/apk/installed.go index 6a1815a84..480a6aaad 100644 --- a/pkg/apk/apk/installed.go +++ b/pkg/apk/apk/installed.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "sort" @@ -190,8 +191,8 @@ func (a *APK) readScriptsTar() (io.ReadCloser, error) { } // TODO: We should probably parse control section on the first pass and reuse it. -func (a *APK) controlValue(controlTar io.Reader, want string) ([]string, error) { - mapping, err := controlValue(controlTar, want) +func (a *APK) controlValue(controlFs fs.FS, want string) ([]string, error) { + mapping, err := controlValue(controlFs, want) if err != nil { return nil, err } @@ -204,14 +205,14 @@ func (a *APK) controlValue(controlTar io.Reader, want string) ([]string, error) } // updateTriggers insert the triggers into the triggers file -func (a *APK) updateTriggers(pkg *Package, controlTar io.Reader) error { +func (a *APK) updateTriggers(pkg *Package, controlFs fs.FS) error { triggers, err := a.fs.OpenFile(triggersFilePath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0) if err != nil { return fmt.Errorf("unable to open triggers file %s: %w", triggersFilePath, err) } defer triggers.Close() - values, err := a.controlValue(controlTar, "triggers") + values, err := a.controlValue(controlFs, "triggers") if err != nil { return fmt.Errorf("updating triggers for %s: %w", pkg.Name, err) } diff --git a/pkg/apk/apk/installed_test.go b/pkg/apk/apk/installed_test.go index e6909e331..acc28c2f6 100644 --- a/pkg/apk/apk/installed_test.go +++ b/pkg/apk/apk/installed_test.go @@ -35,6 +35,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "chainguard.dev/apko/internal/tarfs" "chainguard.dev/apko/pkg/apk/expandapk" ) @@ -461,7 +462,9 @@ func TestUpdateTriggers(t *testing.T) { // pass the controltargz to updateScriptsTar r := bytes.NewReader(buf.Bytes()) - err = a.updateTriggers(pkg, r) + fs, err := tarfs.New(r, int64(buf.Len())) + require.NoError(t, err, "unable to create tarfs: %v", err) + err = a.updateTriggers(pkg, fs) require.NoError(t, err, "unable to update triggers: %v", err) // successfully wrote it; not check that it was written correctly diff --git a/pkg/apk/apk/util.go b/pkg/apk/apk/util.go index a27dabd60..cc3420563 100644 --- a/pkg/apk/apk/util.go +++ b/pkg/apk/apk/util.go @@ -15,10 +15,10 @@ package apk import ( - "archive/tar" "errors" "fmt" "io" + "io/fs" "slices" "strings" ) @@ -38,48 +38,41 @@ func uniqify[T comparable](s []T) []T { return uniq } -func controlValue(controlTar io.Reader, want ...string) (map[string][]string, error) { - tr := tar.NewReader(controlTar) - for { - header, err := tr.Next() - if errors.Is(err, io.EOF) { +func controlValue(controlFs fs.FS, want ...string) (map[string][]string, error) { + f, err := controlFs.Open(".PKGINFO") + if err != nil { + if errors.Is(err, fs.ErrNotExist) { return nil, fmt.Errorf("control file not found") } - if err != nil { - return nil, err - } + return nil, fmt.Errorf("opening .PKGINFO: %w", err) + } + defer f.Close() - // ignore .PKGINFO as it is not a script - if header.Name != ".PKGINFO" { + b, err := io.ReadAll(f) + if err != nil { + return nil, fmt.Errorf("unable to read .PKGINFO from control tar.gz file: %w", err) + } + mapping := map[string][]string{} + lines := strings.SplitSeq(string(b), "\n") + for line := range lines { + parts := strings.Split(line, "=") + if len(parts) != 2 { continue } - - b, err := io.ReadAll(tr) - if err != nil { - return nil, fmt.Errorf("unable to read .PKGINFO from control tar.gz file: %w", err) + key := strings.TrimSpace(parts[0]) + if !slices.Contains(want, key) { + continue } - mapping := map[string][]string{} - lines := strings.SplitSeq(string(b), "\n") - for line := range lines { - parts := strings.Split(line, "=") - if len(parts) != 2 { - continue - } - key := strings.TrimSpace(parts[0]) - if !slices.Contains(want, key) { - continue - } - values, ok := mapping[key] - if !ok { - values = []string{} - } + values, ok := mapping[key] + if !ok { + values = []string{} + } - value := strings.TrimSpace(parts[1]) - values = append(values, value) + value := strings.TrimSpace(parts[1]) + values = append(values, value) - mapping[key] = values - } - return mapping, nil + mapping[key] = values } + return mapping, nil }