Use in-memory control data everywhere (#1986)

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.

Author: markusthoemmes

pkg/apk/apk/implementation.go

@@ -1224,13 +1224,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(exp.ControlFS)
 	if err != nil {
 		return nil, fmt.Errorf("datahash for %s: %w", pkg, err)
 	}
@@ -1497,29 +1491,26 @@ 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 {
+	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(controlTarGz io.Reader) (string, error) {
-	values, err := a.controlValue(controlTarGz, "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)
 	}

pkg/apk/apk/installed.go

@@ -22,14 +22,13 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
 	"time"
-
-	"github.com/klauspost/compress/gzip"
 )
 
 type InstalledPackage struct {
@@ -121,13 +120,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 +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(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(controlFs fs.FS, want string) ([]string, error) {
+	mapping, err := controlValue(controlFs, want)
 	if err != nil {
 		return nil, err
 	}
@@ -217,14 +205,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, 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(controlTarGz, "triggers")
+	values, err := a.controlValue(controlFs, "triggers")
 	if err != nil {
 		return fmt.Errorf("updating triggers for %s: %w", pkg.Name, err)
 	}

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"
 )
 
@@ -361,8 +362,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 +379,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 +449,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,11 +459,12 @@ func TestUpdateTriggers(t *testing.T) {
 		_, _ = tw.Write(content)
 	}
 	tw.Close()
-	gw.Close()
 
 	// 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

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
 }