Rework how we cache APKs

pkg/apk/cache.go

@@ -22,6 +22,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+
+	"gitlab.alpinelinux.org/alpine/go/pkg/repository"
 )
 
 // cache
@@ -52,23 +54,21 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 	if request.URL == nil {
 		return nil, fmt.Errorf("no URL in request")
 	}
-	cacheFile, err := t.cachePathFromURL(*request.URL)
+	cacheFile, err := cachePathFromURL(t.root, *request.URL)
 	if err != nil {
 		return nil, fmt.Errorf("invalid cache path based on URL: %w", err)
 	}
 
-	// If an etag isn't required, then check the cache based on a simple
-	// filename-based naming scheme.
 	if !t.etagRequired {
-		// Try to open the file in the cache, and if we hit an error then
-		// try to populate the file in the cache.
+		// We don't cache the response for these because they get cached later in cachePackage.
+
+		// Try to open the file in the cache.
+		// If we hit an error, just send the request.
 		f, err := os.Open(cacheFile)
 		if err != nil {
-			return t.retrieveAndSaveFile(request, func(r *http.Response) (string, error) {
-				// On the non-etag path, we simply name files based on the URL.
-				return cacheFile, nil
-			})
+			return t.wrapped.Do(request)
 		}
+
 		return &http.Response{
 			StatusCode: http.StatusOK,
 			Body:       f,
@@ -87,7 +87,7 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 	}
 	// We simulate content-based addressing with the etag values using an .etag
 	// file extension.
-	etagFile := filepath.Join(filepath.Dir(cacheFile), initialEtag+".etag")
+	etagFile := cacheFileFromEtag(cacheFile, initialEtag)
 	f, err := os.Open(etagFile)
 	if err != nil {
 		return t.retrieveAndSaveFile(request, func(r *http.Response) (string, error) {
@@ -97,7 +97,8 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 			if !ok {
 				return "", fmt.Errorf("GET response did not contain an etag, but HEAD returned %q", initialEtag)
 			}
-			return filepath.Join(filepath.Dir(cacheFile), finalEtag+".etag"), nil
+
+			return cacheFileFromEtag(cacheFile, finalEtag), nil
 		})
 	}
 	return &http.Response{
@@ -106,6 +107,19 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 	}, nil
 }
 
+func cacheFileFromEtag(cacheFile, etag string) string {
+	cacheDir := filepath.Dir(cacheFile)
+	ext := ".etag"
+
+	// Keep all the index files under APKINDEX/ with appropriate file extension.
+	if strings.HasSuffix(cacheFile, "APKINDEX.tar.gz") {
+		cacheDir = filepath.Join(cacheDir, "APKINDEX")
+		ext = ".tar.gz"
+	}
+
+	return filepath.Join(cacheDir, etag+ext)
+}
+
 func etagFromResponse(resp *http.Response) (string, bool) {
 	remoteEtag, ok := resp.Header[http.CanonicalHeaderKey("etag")]
 	if !ok || len(remoteEtag) == 0 || remoteEtag[0] == "" {
@@ -168,8 +182,26 @@ func (t *cacheTransport) retrieveAndSaveFile(request *http.Request, cp cachePlac
 	return resp, nil
 }
 
+func cacheDirForPackage(root string, pkg *repository.RepositoryPackage) (string, error) {
+	u, err := packageAsURL(pkg)
+	if err != nil {
+		return "", err
+	}
+
+	p, err := cachePathFromURL(root, *u)
+	if err != nil {
+		return "", err
+	}
+
+	if ext := filepath.Ext(p); ext != ".apk" {
+		return "", fmt.Errorf("unexpected ext (%s) to cache dir: %q", ext, p)
+	}
+
+	return strings.TrimSuffix(p, ".apk"), nil
+}
+
 // cachePathFromURL given a URL, figure out what the cache path would be
-func (t *cacheTransport) cachePathFromURL(u url.URL) (string, error) {
+func cachePathFromURL(root string, u url.URL) (string, error) {
 	// the last two levels are what we append. For example https://example.com/foo/bar/x86_64/baz.apk
 	// means we want to append x86_64/baz.apk to our cache root
 	u2 := u
@@ -185,12 +217,12 @@ func (t *cacheTransport) cachePathFromURL(u url.URL) (string, error) {
 
 	// url encode it so it can be a single directory
 	repoDir = url.QueryEscape(u2.String())
-	cacheFile := filepath.Join(t.root, repoDir, dir, filename)
-	// validate it is within t.root
+	cacheFile := filepath.Join(root, repoDir, dir, filename)
+	// validate it is within root
 	cacheFile = filepath.Clean(cacheFile)
-	root := filepath.Clean(t.root)
-	if !strings.HasPrefix(cacheFile, root) {
-		return "", fmt.Errorf("cache file %s is not within root %s", cacheFile, root)
+	cleanroot := filepath.Clean(root)
+	if !strings.HasPrefix(cacheFile, cleanroot) {
+		return "", fmt.Errorf("cache file %s is not within root %s", cacheFile, cleanroot)
 	}
 	return cacheFile, nil
 }

pkg/apk/expandapk.go

@@ -36,22 +36,59 @@ type APKExpanded struct {
 	// The temporary parent directory containing all exploded .tar/.tar.gz contents
 	tempDir string
 
-	// The package signature (a.k.a. ".SIGN...") in tar.gz format
-	Signature []byte
+	// The package signature filename (a.k.a. ".SIGN...") in tar.gz format
+	SignatureFile string
 
-	// the control data (a.k.a. ".PKGINFO") in tar.gz format
-	ControlData []byte
+	// The control data filename (a.k.a. ".PKGINFO") in tar.gz format
+	ControlFile string
 
-	// A stream reader to the contents in tar.gz format
-	PackageData io.ReadCloser
+	// The package data filename in tar.gz format
+	PackageFile string
 }
 
-func (a *APKExpanded) Close() error {
-	if a.PackageData != nil {
-		if err := a.PackageData.Close(); err != nil {
-			return fmt.Errorf("APKExpanded.Close error 1: %v", err)
+func (a *APKExpanded) APK() (io.ReadCloser, error) {
+	rs := []io.Reader{}
+	cs := []io.Closer{}
+
+	for _, fn := range []string{a.SignatureFile, a.ControlFile, a.PackageFile} {
+		if fn != "" {
+			f, err := os.Open(fn)
+			if err != nil {
+				return nil, err
+			}
+			rs = append(rs, f)
+			cs = append(cs, f)
 		}
 	}
+
+	return &multiReadCloser{
+		r:       io.MultiReader(rs...),
+		closers: cs,
+	}, nil
+}
+
+type multiReadCloser struct {
+	r       io.Reader
+	closers []io.Closer
+}
+
+func (m *multiReadCloser) Read(p []byte) (int, error) {
+	return m.r.Read(p)
+}
+
+func (m *multiReadCloser) Close() error {
+	errs := make([]error, len(m.closers))
+	for i, closer := range m.closers {
+		errs[i] = closer.Close()
+	}
+	return errors.Join(errs...)
+}
+
+func (a *APKExpanded) Close() error {
+	if a.tempDir == "" {
+		return nil
+	}
+
 	return os.RemoveAll(a.tempDir)
 }
 
@@ -207,6 +244,7 @@ func ExpandApk(ctx context.Context, source io.Reader) (*APKExpanded, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	sw, err := newExpandApkWriter(dir, "stream", "tar.gz")
 	if err != nil {
 		return nil, fmt.Errorf("expandApk error 1: %w", err)
@@ -324,29 +362,15 @@ func ExpandApk(ctx context.Context, source io.Reader) (*APKExpanded, error) {
 		return nil, fmt.Errorf("invalid number of tar streams: %d", numGzipStreams)
 	}
 
-	controlData, err := os.ReadFile(gzipStreams[controlDataIndex])
-	if err != nil {
-		return nil, fmt.Errorf("unable to read control data: %w", err)
-	}
-
-	packageData, err := os.Open(gzipStreams[controlDataIndex+1])
-	if err != nil {
-		return nil, fmt.Errorf("could not open package data file %s for reading: %w", gzipStreams[controlDataIndex+1], err)
-	}
-
 	expanded := APKExpanded{
 		tempDir:     dir,
 		Signed:      signed,
 		Size:        totalSize,
-		ControlData: controlData,
-		PackageData: packageData,
+		ControlFile: gzipStreams[controlDataIndex],
+		PackageFile: gzipStreams[controlDataIndex+1],
 	}
 	if signed {
-		b, err := os.ReadFile(gzipStreams[0])
-		if err != nil {
-			return nil, fmt.Errorf("could not read signature file %s: %w", gzipStreams[0], err)
-		}
-		expanded.Signature = b
+		expanded.SignatureFile = gzipStreams[0]
 	}
 
 	return &expanded, nil