Repository: chainguard-dev/apko
Pull request #1327 (2024-10-02 18:32:21)

Make etag checks optional
This isn't perfect but I think it moves us closer in the right direction. Previously, we had a process-wide global etag cache that would prevent sending multiple HEAD requests to check the etag value of any URLs we fetched (keys and APKINDEX, mostly).

This is great for a local tool or even the terraform provider, but it's awful as a library used by a service because the lifetime of the process is liable to outlive the validity of the cach.

This hoists that global etag caching mechanism out into a little apk.Cache struct, which is where I'm hoping to put even more things that I'm ashamed of going forward.

Using the same apk.NewCache(true) instance across multiple builds will skip HEAD requests to the same URL and just assume that the tag hasn't changed. This was the previous default behavior.

Using the same apk.NewCache(false) instance across multiple builds (the default) will send a HEAD request prior to every key or index fetch in order to see if they've changed.

We still have some global caching going on for actually parsing the results, which keys off of etag values, so we shouldn't have any progressions re: the recent CPU savings changes I've made.

Another thing this introduces (because we dropped the more powerful etag-based caching) is a some singleflight instances for these outgoing requests. We don't want to cache the results indefinitely, but we do want to avoid sending redundant requests in every case, so apk.NewCache(false) will still coalesce multiple concurrent requests for the same URL, so there's still benefit to using this cache object even with the etag caching disabled.

One thing that's missing here is a distinction between ~request-scoped caching if e.g. I want to use an etag cache for any outgoing URLs for a single build, but I also want to use a global cache for coalescing requests. I'll probably split the singleflight stuff off from this struct at some point, but I want to measure first to see if that's really necessary.


Patch:
diff --git a/internal/cli/build.go b/internal/cli/build.go
index 51aed9d..fcccfd2 100644
--- a/internal/cli/build.go
+++ b/internal/cli/build.go
@@ -33,6 +33,7 @@ import (
 
 	"github.com/chainguard-dev/clog"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	"chainguard.dev/apko/pkg/build"
 	"chainguard.dev/apko/pkg/build/oci"
 	"chainguard.dev/apko/pkg/build/types"
@@ -108,7 +109,7 @@ Along the image, apko will generate SBOMs (software bill of materials) describin
 				build.WithTags(args[1]),
 				build.WithVCS(withVCS),
 				build.WithAnnotations(annotations),
-				build.WithCacheDir(cacheDir, offline),
+				build.WithCache(cacheDir, offline, apk.NewCache(true)),
 				build.WithLockFile(lockfile),
 				build.WithTempDir(tmp),
 				build.WithIncludePaths(includePaths),
diff --git a/internal/cli/dot.go b/internal/cli/dot.go
index 9c5fbe7..d1cd856 100644
--- a/internal/cli/dot.go
+++ b/internal/cli/dot.go
@@ -72,7 +72,7 @@ apko dot --web -S example.yaml
 				build.WithExtraKeys(extraKeys),
 				build.WithExtraBuildRepos(extraBuildRepos),
 				build.WithExtraRuntimeRepos(extraRuntimeRepos),
-				build.WithCacheDir(cacheDir, offline),
+				build.WithCache(cacheDir, offline, apk.NewCache(true)),
 			)
 		},
 	}
diff --git a/internal/cli/publish.go b/internal/cli/publish.go
index 3748b77..b734b5e 100644
--- a/internal/cli/publish.go
+++ b/internal/cli/publish.go
@@ -31,6 +31,7 @@ import (
 
 	"github.com/chainguard-dev/clog"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	"chainguard.dev/apko/pkg/build"
 	"chainguard.dev/apko/pkg/build/oci"
 	"chainguard.dev/apko/pkg/build/types"
@@ -116,7 +117,7 @@ in a keychain.`,
 					build.WithTags(args[1:]...),
 					build.WithVCS(withVCS),
 					build.WithAnnotations(annotations),
-					build.WithCacheDir(cacheDir, offline),
+					build.WithCache(cacheDir, offline, apk.NewCache(true)),
 					build.WithLockFile(lockfile),
 					build.WithTempDir(tmp),
 					build.WithIgnoreSignatures(ignoreSignatures),
diff --git a/internal/cli/show-config.go b/internal/cli/show-config.go
index 9d34bc7..69a739e 100644
--- a/internal/cli/show-config.go
+++ b/internal/cli/show-config.go
@@ -23,6 +23,7 @@ import (
 	"github.com/spf13/cobra"
 	"gopkg.in/yaml.v3"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	apkfs "chainguard.dev/apko/pkg/apk/fs"
 	"chainguard.dev/apko/pkg/build"
 )
@@ -49,7 +50,7 @@ The derived configuration is rendered in YAML.
 				build.WithExtraKeys(extraKeys),
 				build.WithExtraBuildRepos(extraBuildRepos),
 				build.WithExtraRuntimeRepos(extraRuntimeRepos),
-				build.WithCacheDir(cacheDir, offline),
+				build.WithCache(cacheDir, offline, apk.NewCache(true)),
 			)
 		},
 	}
diff --git a/internal/cli/show-packages.go b/internal/cli/show-packages.go
index c3d04b2..a61418c 100644
--- a/internal/cli/show-packages.go
+++ b/internal/cli/show-packages.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/chainguard-dev/clog"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	"chainguard.dev/apko/pkg/build"
 	"chainguard.dev/apko/pkg/build/types"
 )
@@ -108,7 +109,7 @@ packagelock and packagelock-source are particularly useful for inserting back in
 				build.WithExtraKeys(extraKeys),
 				build.WithExtraBuildRepos(extraBuildRepos),
 				build.WithExtraRuntimeRepos(extraRuntimeRepos),
-				build.WithCacheDir(cacheDir, offline),
+				build.WithCache(cacheDir, offline, apk.NewCache(true)),
 			)
 		},
 	}
diff --git a/pkg/apk/apk/cache.go b/pkg/apk/apk/cache.go
index 1a999f4..f1a204f 100644
--- a/pkg/apk/apk/cache.go
+++ b/pkg/apk/apk/cache.go
@@ -26,133 +26,74 @@ import (
 	"sync"
 
 	"go.opentelemetry.io/otel"
+	"golang.org/x/sync/singleflight"
 )
 
-// This is terrible but simpler than plumbing around a cache for now.
-// We will assume that for a given process, we want to reuse etag values.
-// Doing this cuts down on the number of requests we send for index and keys.
-var globalEtagCache = &etagCache{}
-
-type etagResp struct {
-	resp      *http.Response
-	err       error
-	cacheFile string
+type Cache struct {
+	etagCache  *sync.Map
+	headFlight *singleflight.Group
+	getFlight  *singleflight.Group
 }
 
-type etagCache struct {
-	// url -> *sync.Once
-	etags sync.Map
-
-	// url -> etagResp
-	resps sync.Map
-}
-
-// get dedupes incoming etag-based requests by url (using a sync.Map[string]sync.Once) and stores the results
-// in a sync.Map[string]etagResp. If we request the same URL multiple times, we will only ever reach out to
-// the internet for the first once and reuse the results for all subsequent calls (unless the response does
-// not have an etag).
-func (e *etagCache) get(ctx context.Context, t *cacheTransport, request *http.Request, cacheFile string) (*http.Response, error) {
-	ctx, span := otel.Tracer("go-apk").Start(ctx, fmt.Sprintf("etagCache.get(%q)", request.URL.String()))
-	defer span.End()
-
-	url := request.URL.String()
-
-	// Do all the expensive things inside the once.
-	once, _ := e.etags.LoadOrStore(url, &sync.Once{})
-	once.(*sync.Once).Do(func() {
-		ctx, span := otel.Tracer("go-apk").Start(ctx, "once.Do")
-		defer span.End()
-
-		req := request.Clone(request.Context())
-		req.Method = http.MethodHead
-		resp, rerr := t.wrapped.Do(req)
-		if resp != nil {
-			// We don't expect any body from a HEAD so just always close it to appease the linter.
-			resp.Body.Close()
-		}
-		if rerr != nil || resp.StatusCode != 200 {
-			e.resps.Store(url, etagResp{
-				resp: resp,
-				err:  rerr,
-			})
-			return
-		}
-
-		initialEtag, ok := etagFromResponse(resp)
-		if !ok {
-			return
-		}
-
-		// We simulate content-based addressing with the etag values using an .etag
-		// file extension.
-		etagFile := cacheFileFromEtag(cacheFile, initialEtag)
-		if _, err := os.Stat(etagFile); err == nil {
-			e.resps.Store(url, etagResp{
-				cacheFile: etagFile,
-			})
-			return
-		}
+// NewCache returns a new Cache, which allows us to persist the results of HEAD requests
+// for a given URL across multiple builds. This is generally desirable when building many images
+// all at once, since we can eliminate round trips to HEAD the same e.g. APKINDEX repeatedly.
+// However, this is not desirable for long-lived processes where we expected the APKINDEX to change
+// over time and want to observe those changes and re-fetch the new APKINDEX. Since apko is used
+// in both contexts, this knob is exposed, but very few people actually should be using apko as a library,
+// so I'm mostly just talking to myself here. This used to be a process-wide global cache, which was
+// great for the short-lived terraform module, but a terrible default behavior for a library.
+//
+// Even if you don't want to cache HEAD requests, this is still useful for coalescing concurrent
+// requests for the same resource when passing etag=false.
+func NewCache(etag bool) *Cache {
+	c := &Cache{
+		headFlight: &singleflight.Group{},
+		getFlight:  &singleflight.Group{},
+	}
 
-		// Only download the index once.
-		etagFile, err := t.retrieveAndSaveFile(ctx, request, func(r *http.Response) (string, error) {
-			_, span := otel.Tracer("go-apk").Start(ctx, "callback")
-			defer span.End()
-			// On the etag path, use the etag from the actual response to
-			// compute the final file name.
-			finalEtag, ok := etagFromResponse(r)
-			if !ok {
-				return "", fmt.Errorf("GET response did not contain an etag, but HEAD returned %q", initialEtag)
-			}
+	if etag {
+		c.etagCache = &sync.Map{}
+	}
 
-			return cacheFileFromEtag(cacheFile, finalEtag), nil
-		})
-		e.resps.Store(url, etagResp{
-			err:       err,
-			cacheFile: etagFile,
-		})
-	})
+	return c
+}
 
-	v, ok := e.resps.Load(url)
-	if !ok {
-		// If the server doesn't return etags, and we require them,
-		// then do not cache.
-		return t.wrapped.Do(request)
+func (c *Cache) load(cacheFile string) (*http.Response, bool) {
+	if c == nil || c.etagCache == nil {
+		return nil, false
 	}
-	resp := v.(etagResp)
 
-	// If we didn't manage to cache it, return the response and/or error.
-	if resp.cacheFile == "" {
-		return resp.resp, resp.err
+	v, ok := c.etagCache.Load(cacheFile)
+	if !ok {
+		return nil, false
 	}
 
-	f, err := os.Open(resp.cacheFile)
-	if err != nil {
-		return nil, fmt.Errorf("open(%q): %w", resp.cacheFile, err)
-	}
+	return v.(*http.Response), true
+}
 
-	fi, err := f.Stat()
-	if err != nil {
-		return nil, fmt.Errorf("stat(%q): %w", resp.cacheFile, err)
+func (c *Cache) store(cacheFile string, resp *http.Response) {
+	if c == nil || c.etagCache == nil {
+		return
 	}
 
-	return &http.Response{
-		StatusCode:    http.StatusOK,
-		Body:          f,
-		ContentLength: fi.Size(),
-	}, nil
+	c.etagCache.Store(cacheFile, resp)
 }
 
 // cache
 type cache struct {
 	dir     string
 	offline bool
+
+	shared *Cache
 }
 
 // client return an http.Client that knows how to read from and write to the cache
 // key is in the implementation of https://pkg.go.dev/net/http#RoundTripper
-func (c cache) client(wrapped *http.Client, etagRequired bool) *http.Client {
+func (c *cache) client(wrapped *http.Client, etagRequired bool) *http.Client {
 	return &http.Client{
 		Transport: &cacheTransport{
+			cache:        c.shared,
 			wrapped:      wrapped,
 			root:         c.dir,
 			offline:      c.offline,
@@ -162,6 +103,7 @@ func (c cache) client(wrapped *http.Client, etagRequired bool) *http.Client {
 }
 
 type cacheTransport struct {
+	cache        *Cache
 	wrapped      *http.Client
 	root         string
 	offline      bool
@@ -172,20 +114,12 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 	ctx, span := otel.Tracer("go-apk").Start(request.Context(), "cacheTransport.RoundTrip")
 	defer span.End()
 
-	// do we have the file in the cache?
-	if request.URL == nil {
-		return nil, fmt.Errorf("no URL in request")
-	}
 	cacheFile, err := cachePathFromURL(t.root, *request.URL)
 	if err != nil {
 		return nil, fmt.Errorf("invalid cache path based on URL: %w", err)
 	}
 
 	if !t.etagRequired {
-		// We don't cache the response for these because they get cached later in cachePackage.
-
-		ctx, span := otel.Tracer("go-apk").Start(ctx, fmt.Sprintf("Open(%q)", cacheFile))
-		defer span.End()
 		// Try to open the file in the cache.
 		// If we hit an error, just send the request.
 		f, err := os.Open(cacheFile)
@@ -193,9 +127,11 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 			if t.offline {
 				return nil, fmt.Errorf("failed to read %q in offline cache: %w", cacheFile, err)
 			}
+
 			_, span := otel.Tracer("go-apk").Start(ctx, fmt.Sprintf("Request(%q)", request.URL.String()))
 			defer span.End()
 
+			// We don't cache the response for these because they get cached later in cachePackage.
 			return t.wrapped.Do(request)
 		}
 
@@ -206,45 +142,157 @@ func (t *cacheTransport) RoundTrip(request *http.Request) (*http.Response, error
 	}
 
 	if t.offline {
-		cacheDir := cacheDirFromFile(cacheFile)
-		des, err := os.ReadDir(cacheDir)
+		return t.fetchOffline(cacheFile)
+	}
+
+	return t.fetchAndCache(ctx, request, cacheFile)
+}
+
+func (t *cacheTransport) head(request *http.Request, cacheFile string) (*http.Response, error) {
+	resp, ok := t.cache.load(cacheFile)
+	if ok {
+		return resp, nil
+	}
+
+	v, err, _ := t.cache.headFlight.Do(cacheFile, func() (interface{}, error) {
+		req := request.Clone(request.Context())
+		req.Method = http.MethodHead
+		resp, err := t.wrapped.Do(req)
 		if err != nil {
-			return nil, fmt.Errorf("listing %q for offline cache: %w", cacheDir, err)
+			return nil, err
 		}
 
-		if len(des) == 0 {
-			return nil, fmt.Errorf("no offline cached entries for %s", cacheDir)
+		// HEAD shouldn't have a body. Make sure we close it so we can reuse the connection.
+		defer resp.Body.Close()
+
+		t.cache.store(cacheFile, resp)
+
+		return resp, nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return v.(*http.Response), nil
+}
+
+func (t *cacheTransport) get(ctx context.Context, request *http.Request, cacheFile, initialEtag string) (string, error) {
+	v, err, _ := t.cache.getFlight.Do(cacheFile, func() (interface{}, error) {
+		// We simulate content-based addressing with the etag values using an .etag file extension.
+		etagFile, err := cacheFileFromEtag(cacheFile, initialEtag)
+		if err != nil {
+			return "", err
 		}
+		if _, err := os.Stat(etagFile); err == nil {
+			return etagFile, nil
+		}
+
+		// Only download the index once.
+		return t.retrieveAndSaveFile(ctx, request, func(r *http.Response) (string, error) {
+			_, span := otel.Tracer("go-apk").Start(ctx, "callback")
+			defer span.End()
+
+			// On the etag path, use the etag from the actual response to compute the final file name.
+			finalEtag, ok := etagFromResponse(r)
+			if !ok {
+				return "", fmt.Errorf("GET response did not contain an etag, but HEAD returned %q", initialEtag)
+			}
+
+			return cacheFileFromEtag(cacheFile, finalEtag)
+		})
+	})
+	if err != nil {
+		return "", err
+	}
 
-		newest, err := des[0].Info()
+	return v.(string), nil
+}
+
+func (t *cacheTransport) fetchAndCache(ctx context.Context, request *http.Request, cacheFile string) (*http.Response, error) {
+	initialEtag := request.Header.Get("If-None-Match")
+	if initialEtag == "" {
+		resp, err := t.head(request, cacheFile)
 		if err != nil {
 			return nil, err
 		}
 
-		for _, de := range des[1:] {
-			fi, err := de.Info()
-			if err != nil {
-				return nil, err
-			}
+		if request.Method == http.MethodHead {
+			return resp, err
+		}
 
-			if fi.ModTime().After(newest.ModTime()) {
-				newest = fi
-			}
+		etag, ok := etagFromResponse(resp)
+		if !ok {
+			return t.wrapped.Do(request)
 		}
 
-		f, err := os.Open(filepath.Join(cacheDir, newest.Name()))
+		initialEtag = etag
+	}
+
+	// This is a bit of a hack. We cache parsed APKINDEX files in globalIndexCache, which needs the etag as a cache key.
+	// Since we already send a HEAD request to get that etag, we want to avoid sending a redundant HEAD request above if we can.
+	// However, we don't actually want to pass along this header because it's using the "parsed" version of the Etag from
+	// etagFromResponse and doesn't actually attempt to follow HTTP semantics, so we remove it here to avoid any confusion.
+	request.Header.Del("If-None-Match")
+
+	etagFile, err := t.get(ctx, request, cacheFile, initialEtag)
+	if err != nil {
+		return nil, err
+	}
+
+	f, err := os.Open(etagFile)
+	if err != nil {
+		return nil, fmt.Errorf("open(%q): %w", etagFile, err)
+	}
+
+	fi, err := f.Stat()
+	if err != nil {
+		return nil, fmt.Errorf("stat(%q): %w", etagFile, err)
+	}
+
+	return &http.Response{
+		StatusCode:    http.StatusOK,
+		Body:          f,
+		ContentLength: fi.Size(),
+	}, nil
+}
+
+func (t *cacheTransport) fetchOffline(cacheFile string) (*http.Response, error) {
+	cacheDir := cacheDirFromFile(cacheFile)
+	des, err := os.ReadDir(cacheDir)
+	if err != nil {
+		return nil, fmt.Errorf("listing %q for offline cache: %w", cacheDir, err)
+	}
+
+	if len(des) == 0 {
+		return nil, fmt.Errorf("no offline cached entries for %s", cacheDir)
+	}
+
+	newest, err := des[0].Info()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, de := range des[1:] {
+		fi, err := de.Info()
 		if err != nil {
 			return nil, err
 		}
 
-		return &http.Response{
-			StatusCode:    http.StatusOK,
-			Body:          f,
-			ContentLength: newest.Size(),
-		}, nil
+		if fi.ModTime().After(newest.ModTime()) {
+			newest = fi
+		}
 	}
 
-	return globalEtagCache.get(ctx, t, request, cacheFile)
+	f, err := os.Open(filepath.Join(cacheDir, newest.Name()))
+	if err != nil {
+		return nil, err
+	}
+
+	return &http.Response{
+		StatusCode:    http.StatusOK,
+		Body:          f,
+		ContentLength: newest.Size(),
+	}, nil
 }
 
 func cacheDirFromFile(cacheFile string) string {
@@ -255,7 +303,7 @@ func cacheDirFromFile(cacheFile string) string {
 	return filepath.Dir(cacheFile)
 }
 
-func cacheFileFromEtag(cacheFile, etag string) string {
+func cacheFileFromEtag(cacheFile, etag string) (string, error) {
 	cacheDir := filepath.Dir(cacheFile)
 	ext := ".etag"
 
@@ -265,7 +313,15 @@ func cacheFileFromEtag(cacheFile, etag string) string {
 		ext = ".tar.gz"
 	}
 
-	return filepath.Join(cacheDir, etag+ext)
+	absPath, err := filepath.Abs(filepath.Join(cacheDir, etag+ext))
+	if err != nil {
+		return "", err
+	}
+	if !strings.HasPrefix(absPath, cacheDir) {
+		return "", fmt.Errorf("unsafe etag value: %q", etag)
+	}
+
+	return absPath, nil
 }
 
 func etagFromResponse(resp *http.Response) (string, bool) {
diff --git a/pkg/apk/apk/index.go b/pkg/apk/apk/index.go
index 05599a7..8d127e3 100644
--- a/pkg/apk/apk/index.go
+++ b/pkg/apk/apk/index.go
@@ -31,6 +31,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/chainguard-dev/clog"
 	"github.com/klauspost/compress/gzip"
 	"go.lsp.dev/uri"
 	"go.opentelemetry.io/otel"
@@ -47,7 +48,8 @@ var signatureFileRegex = regexp.MustCompile(`^\.SIGN\.(DSA|RSA|RSA256|RSA512)\.(
 // We just hold the parsed index in memory rather than re-parsing it every time,
 // which requires gunzipping, which is (somewhat) expensive.
 var globalIndexCache = &indexCache{
-	modtimes: map[string]time.Time{},
+	modtimes:  map[string]time.Time{},
+	urlToEtag: map[string]string{},
 }
 
 type indexResult struct {
@@ -57,16 +59,39 @@ type indexResult struct {
 
 type indexCache struct {
 	// For remote indexes.
-	onces sync.Map
+	onces     sync.Map
+	urlToEtag map[string]string
 
 	// For local indexes.
 	sync.Mutex
 	modtimes map[string]time.Time
 
-	// repoBase -> indexResult
+	// etag|filename -> indexResult
 	indexes sync.Map
 }
 
+func (i *indexCache) forget(key string) {
+	i.onces.Delete(key)
+	i.indexes.Delete(key)
+}
+
+func (i *indexCache) store(key string, idx NamedIndex, err error) {
+	i.indexes.Store(key, indexResult{
+		idx: idx,
+		err: err,
+	})
+}
+
+func (i *indexCache) load(key string) (NamedIndex, error) {
+	v, ok := i.indexes.Load(key)
+	if !ok {
+		return nil, fmt.Errorf("indexCache did not see key %q after writing it", key)
+	}
+	result := v.(indexResult)
+
+	return result.idx, result.err
+}
+
 func (i *indexCache) get(ctx context.Context, repoName, repoURL string, keys map[string][]byte, arch string, opts *indexOpts) (NamedIndex, error) {
 	u := IndexURL(repoURL, arch)
 
@@ -77,20 +102,69 @@ func (i *indexCache) get(ctx context.Context, repoName, repoURL string, keys map
 	repoRef := Repository{URI: repoBase}
 
 	if strings.HasPrefix(u, "https://") || strings.HasPrefix(u, "http://") {
-		// We don't want remote indexes to change while we're running.
-		once, _ := i.onces.LoadOrStore(u, &sync.Once{})
-		once.(*sync.Once).Do(func() {
-			idx, err := getRepositoryIndex(ctx, u, keys, arch, opts)
+		asURL, err := url.Parse(u)
+		if err != nil {
+			return nil, fmt.Errorf("parsing repo: %w", err)
+		}
+
+		// We usually don't want remote indexes to change while we're running.
+		// But sometimes, we do, in which case we want to key off of the etag.
+		// We can use a separate etag cache in the httpClient to avoid the HEAD
+		// if it's set, but if it's not set we need to do a HEAD each time.
+		client := opts.httpClient
+		head, err := http.NewRequestWithContext(ctx, http.MethodHead, u, nil)
+		if err != nil {
+			return nil, err
+		}
+		if opts.auth == nil {
+			opts.auth = auth.DefaultAuthenticators
+		}
+		if err := opts.auth.AddAuth(ctx, head); err != nil {
+			return nil, fmt.Errorf("unable to add auth to request: %w", err)
+		}
+
+		resp, err := client.Do(head)
+		if err != nil {
+			return nil, err
+		}
+
+		fetchAndParse := func(etag string) (NamedIndex, error) {
+			b, err := fetchRepositoryIndex(ctx, u, etag, opts)
 			if err != nil {
-				i.indexes.Store(u, indexResult{
-					err: err,
-				})
-			} else {
-				i.indexes.Store(u, indexResult{
-					idx: NewNamedRepositoryWithIndex(repoName, repoRef.WithIndex(idx)),
-				})
+				return nil, fmt.Errorf("fetching %s: %w", asURL.Redacted(), err)
+			}
+			idx, err := parseRepositoryIndex(ctx, u, keys, arch, b, opts)
+			if err != nil {
+				return nil, fmt.Errorf("parsing %s: %w", asURL.Redacted(), err)
+			}
+			return NewNamedRepositoryWithIndex(repoName, repoRef.WithIndex(idx)), nil
+		}
+
+		etag, ok := etagFromResponse(resp)
+		if !ok {
+			// If there's no etag, we can't cache it, so just return the result.
+			return fetchAndParse(etag)
+		}
+
+		key := fmt.Sprintf("%s@%s", u, etag)
+
+		once, _ := i.onces.LoadOrStore(key, &sync.Once{})
+		once.(*sync.Once).Do(func() {
+			// If we've seen this URL before, delete any references to old indexes so we can GC them.
+			prev, ok := i.urlToEtag[u]
+			if ok {
+				prevKey := fmt.Sprintf("%s@%s", u, prev)
+				i.forget(prevKey)
 			}
+
+			idx, err := fetchAndParse(etag)
+			i.store(key, idx, err)
+
+			// Record the current etag for this URL so we can GC it later.
+			i.urlToEtag[u] = etag
 		})
+
+		return i.load(key)
 	} else {
 		i.Lock()
 		defer i.Unlock()
@@ -98,35 +172,28 @@ func (i *indexCache) get(ctx context.Context, repoName, repoURL string, keys map
 		// We do expect local indexes to change, so we check modtimes.
 		stat, err := os.Stat(u)
 		if err != nil {
-			return nil, nil
+			return nil, fmt.Errorf("stat: %w", err)
 		}
 
 		mod := stat.ModTime()
 		before, ok := i.modtimes[u]
 		if !ok || mod.After(before) {
+			b, err := os.ReadFile(u)
+			if err != nil {
+				return nil, fmt.Errorf("reading file: %w", err)
+			}
 			// If this is the first time or it has changed since the last time...
-			idx, err := getRepositoryIndex(ctx, u, keys, arch, opts)
+			idx, err := parseRepositoryIndex(ctx, u, keys, arch, b, opts)
 			if err != nil {
-				i.indexes.Store(u, indexResult{
-					err: err,
-				})
+				i.store(u, nil, err)
 			} else {
-				i.indexes.Store(u, indexResult{
-					idx: NewNamedRepositoryWithIndex(repoName, repoRef.WithIndex(idx)),
-				})
+				i.store(u, NewNamedRepositoryWithIndex(repoName, repoRef.WithIndex(idx)), nil)
 			}
 			i.modtimes[u] = mod
 		}
-	}
 
-	v, ok := i.indexes.Load(u)
-	if !ok {
-		asURL, _ := url.Parse(u)
-		panic(fmt.Errorf("did not see index %q after writing it", asURL.Redacted()))
+		return i.load(u)
 	}
-	result := v.(indexResult)
-
-	return result.idx, result.err
 }
 
 // IndexURL full URL to the index file for the given repo and arch
@@ -171,15 +238,16 @@ func GetRepositoryIndexes(ctx context.Context, repos []string, keys map[string][
 
 			index, err := globalIndexCache.get(ctx, repoName, repoURL, keys, arch, opts)
 			if err != nil {
-				u := IndexURL(repoURL, arch)
-				asURL, _ := url.Parse(u)
-				return fmt.Errorf("reading index %s: %w", asURL.Redacted(), err)
-			}
+				redacted := redact(IndexURL(repoURL, arch))
+				if errors.Is(err, fs.ErrNotExist) {
+					// This can happen for local repos, just log and continue.
+					clog.WarnContextf(ctx, "getting local index %s: %v", redacted, err)
+					return nil
+				}
 
-			// Can happen for fs.ErrNotExist in file scheme, we just ignore it.
-			if index == nil {
-				return nil
+				return fmt.Errorf("reading index %s: %w", redacted, err)
 			}
+
 			indexes[i] = index
 			return nil
 		})
@@ -208,75 +276,51 @@ func shouldCheckSignatureForIndex(index string, arch string, opts *indexOpts) bo
 	return true
 }
 
-func getRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, arch string, opts *indexOpts) (*APKIndex, error) { //nolint:gocyclo
-	ctx, span := otel.Tracer("go-apk").Start(ctx, "getRepositoryIndex")
-	defer span.End()
+func fetchRepositoryIndex(ctx context.Context, u string, etag string, opts *indexOpts) ([]byte, error) { //nolint:gocyclo
+	client := opts.httpClient
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
+	if err != nil {
+		return nil, err
+	}
 
-	// Normalize the repo as a URI, so that local paths
-	// are translated into file:// URLs, allowing them to be parsed
-	// into a url.URL{}.
-	var (
-		b     []byte
-		asURL *url.URL
-		err   error
-	)
-	if strings.HasPrefix(u, "https://") || strings.HasPrefix(u, "http://") {
-		asURL, err = url.Parse(u)
-	} else {
-		// Attempt to parse non-https elements into URI's so they are translated into
-		// file:// URLs allowing them to parse into a url.URL{}
-		asURL, err = url.Parse(string(uri.New(u)))
+	if etag != "" {
+		// This is a hack, I'm sorry, but it's the only way to avoid modifying our weird transportCache too much.
+		// Earlier, we check the etag value to avoid re-parsing the APKINDEX if it hasn't changed, but the transport
+		// also really wants to do a HEAD request in order to do etag-based caching itself. To avoid the double HEAD,
+		// I'm stuffing the etag into the If-None-Match header, which isn't exactly correct semantics but it rhymes.
+		// The alternative is to rewrite everything, which I don't have time to do right now, so it is what it is.
+		req.Header.Set("If-None-Match", etag)
 	}
+
+	if opts.auth == nil {
+		opts.auth = auth.DefaultAuthenticators
+	}
+	if err := opts.auth.AddAuth(ctx, req); err != nil {
+		return nil, fmt.Errorf("unable to add auth to request: %w", err)
+	}
+
+	// This will return a body that retries requests using Range requests if Read() hits an error.
+	rrt := newRangeRetryTransport(ctx, client)
+	res, err := rrt.RoundTrip(req)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse repo as URI: %w", err)
+		return nil, err
+	}
+	if res.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("unexpected status code %d", res.StatusCode)
 	}
+	defer res.Body.Close()
 
-	switch asURL.Scheme {
-	case "file":
-		b, err = os.ReadFile(u)
-		if err != nil {
-			if !errors.Is(err, fs.ErrNotExist) {
-				return nil, fmt.Errorf("failed to read repository %s: %w", asURL.Redacted(), err)
-			}
-			return nil, nil
-		}
-	case "https", "http":
-		client := opts.httpClient
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, asURL.String(), nil)
-		if err != nil {
-			return nil, err
-		}
+	b, err := io.ReadAll(res.Body)
+	if err != nil {
+		return nil, fmt.Errorf("reading body: %w", err)
+	}
 
-		if opts.auth == nil {
-			opts.auth = auth.DefaultAuthenticators
-		}
-		if err := opts.auth.AddAuth(ctx, req); err != nil {
-			return nil, fmt.Errorf("unable to add auth to request: %w", err)
-		}
+	return b, nil
+}
 
-		// This will return a body that retries requests using Range requests if Read() hits an error.
-		rrt := newRangeRetryTransport(ctx, client)
-		res, err := rrt.RoundTrip(req)
-		if err != nil {
-			return nil, fmt.Errorf("unable to get repository index at %s: %w", asURL.Redacted(), err)
-		}
-		switch res.StatusCode {
-		case http.StatusOK:
-			// this is fine
-		case http.StatusNotFound:
-			return nil, fmt.Errorf("repository index not found for architecture %s at %s", arch, asURL.Redacted())
-		default:
-			return nil, fmt.Errorf("unexpected status code %d when getting repository index for architecture %s at %s", res.StatusCode, arch, asURL.Redacted())
-		}
-		defer res.Body.Close()
-		buf := bytes.NewBuffer(nil)
-		if _, err := io.Copy(buf, res.Body); err != nil {
-			return nil, fmt.Errorf("unable to read repository index at %s: %w", asURL.Redacted(), err)
-		}
-		b = buf.Bytes()
-	default:
-		return nil, fmt.Errorf("repository scheme %s not supported", asURL.Scheme)
-	}
+func parseRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, arch string, b []byte, opts *indexOpts) (*APKIndex, error) { //nolint:gocyclo
+	_, span := otel.Tracer("go-apk").Start(ctx, "parseRepositoryIndex")
+	defer span.End()
 
 	// validate the signature
 	if shouldCheckSignatureForIndex(u, arch, opts) {
@@ -371,7 +415,7 @@ func getRepositoryIndex(ctx context.Context, u string, keys map[string][]byte, a
 	// with a valid signature, convert it to an ApkIndex
 	index, err := IndexFromArchive(io.NopCloser(bytes.NewReader(b)))
 	if err != nil {
-		return nil, fmt.Errorf("unable to read convert repository index bytes to index struct at %s: %w", asURL.Redacted(), err)
+		return nil, fmt.Errorf("unable to read convert repository index bytes to index struct: %w", err)
 	}
 
 	return index, err
@@ -408,3 +452,19 @@ func WithIndexAuthenticator(a auth.Authenticator) IndexOption {
 		o.auth = a
 	}
 }
+
+func redact(in string) string {
+	asURL, err := url.Parse(in)
+	if err != nil {
+		// Attempt to parse non-https elements into URI's so they are translated into
+		// file:// URLs allowing them to parse into a url.URL{}
+		asURL, err := url.Parse(string(uri.New(in)))
+		if err != nil {
+			return in
+		}
+
+		return asURL.Redacted()
+	}
+
+	return asURL.Redacted()
+}
diff --git a/pkg/apk/apk/options.go b/pkg/apk/apk/options.go
index 54949ab..223103d 100644
--- a/pkg/apk/apk/options.go
+++ b/pkg/apk/apk/options.go
@@ -83,7 +83,7 @@ func WithFS(fs apkfs.FullFS) Option {
 //
 // If offline is true, only read from the cache and do not make any network requests to
 // populate it.
-func WithCache(cacheDir string, offline bool) Option {
+func WithCache(cacheDir string, offline bool, shared *Cache) Option {
 	return func(o *opts) error {
 		var err error
 		if cacheDir == "" {
@@ -96,6 +96,7 @@ func WithCache(cacheDir string, offline bool) Option {
 		o.cache = &cache{
 			dir:     cacheDir,
 			offline: offline,
+			shared:  shared,
 		}
 		return nil
 	}
diff --git a/pkg/build/build.go b/pkg/build/build.go
index a723ad5..7cbabf6 100644
--- a/pkg/build/build.go
+++ b/pkg/build/build.go
@@ -257,9 +257,9 @@ func New(ctx context.Context, fs apkfs.FullFS, opts ...Option) (*Context, error)
 	// note that this is not easy to do in a switch statement, because of the second
 	// condition, if err := ...; err == nil {}
 	if bc.o.CacheDir != "" {
-		apkOpts = append(apkOpts, apk.WithCache(bc.o.CacheDir, bc.o.Offline))
+		apkOpts = append(apkOpts, apk.WithCache(bc.o.CacheDir, bc.o.Offline, bc.o.SharedCache))
 	} else if _, err := os.UserCacheDir(); err == nil {
-		apkOpts = append(apkOpts, apk.WithCache(bc.o.CacheDir, bc.o.Offline))
+		apkOpts = append(apkOpts, apk.WithCache(bc.o.CacheDir, bc.o.Offline, bc.o.SharedCache))
 	} else {
 		log.Warnf("cache disabled because cache dir was not set, and cannot determine system default: %v", err)
 	}
diff --git a/pkg/build/options.go b/pkg/build/options.go
index d91c97d..2045768 100644
--- a/pkg/build/options.go
+++ b/pkg/build/options.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"time"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	"chainguard.dev/apko/pkg/apk/auth"
 	"chainguard.dev/apko/pkg/build/types"
 
@@ -189,11 +190,12 @@ func WithAnnotations(annotations map[string]string) Option {
 	}
 }
 
-// WithCacheDir set the cache directory to use
-func WithCacheDir(cacheDir string, offline bool) Option {
+// WithCache set the cache directory to use
+func WithCache(cacheDir string, offline bool, shared *apk.Cache) Option {
 	return func(bc *Context) error {
 		bc.o.CacheDir = cacheDir
 		bc.o.Offline = offline
+		bc.o.SharedCache = shared
 		return nil
 	}
 }
diff --git a/pkg/options/options.go b/pkg/options/options.go
index 8ca7692..9bce658 100644
--- a/pkg/options/options.go
+++ b/pkg/options/options.go
@@ -21,6 +21,7 @@ import (
 	"runtime"
 	"time"
 
+	"chainguard.dev/apko/pkg/apk/apk"
 	"chainguard.dev/apko/pkg/apk/auth"
 	"chainguard.dev/apko/pkg/build/types"
 )
@@ -49,6 +50,7 @@ type Options struct {
 	Local                   bool               `json:"local,omitempty"`
 	CacheDir                string             `json:"cacheDir,omitempty"`
 	Offline                 bool               `json:"offline,omitempty"`
+	SharedCache             *apk.Cache         `json:"-"`
 	Lockfile                string             `json:"lockfile,omitempty"`
 	Auth                    auth.Authenticator `json:"-"`
 	IncludePaths            []string           `json:"includePaths,omitempty"`
@@ -61,6 +63,7 @@ var Default = Options{
 	Arch:            types.ParseArchitecture(runtime.GOARCH),
 	SourceDateEpoch: time.Unix(0, 0).UTC(),
 	Auth:            auth.DefaultAuthenticators,
+	SharedCache:     apk.NewCache(false),
 }
 
 // Tempdir returns the temporary directory where apko will create


Test patch:
diff --git a/pkg/apk/apk/implementation_test.go b/pkg/apk/apk/implementation_test.go
index c81778a..9e3ce78 100644
--- a/pkg/apk/apk/implementation_test.go
+++ b/pkg/apk/apk/implementation_test.go
@@ -421,7 +421,7 @@ func TestFetchPackage(t *testing.T) {
 
 		opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors)}
 		if cache != "" {
-			opts = append(opts, WithCache(cache, false))
+			opts = append(opts, WithCache(cache, false, NewCache(false)))
 		}
 		a, err := New(opts...)
 		require.NoError(t, err, "unable to create APK")
diff --git a/pkg/apk/apk/repo_test.go b/pkg/apk/apk/repo_test.go
index 27886ee..b8a57ab 100644
--- a/pkg/apk/apk/repo_test.go
+++ b/pkg/apk/apk/repo_test.go
@@ -26,7 +26,6 @@ import (
 	"reflect"
 	"sort"
 	"strings"
-	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -92,8 +91,6 @@ guyM+Ks3c29KlRf3iX35Gt0CAwEAAQ==
 )
 
 func TestGetRepositoryIndexes(t *testing.T) {
-	var mu sync.Mutex
-	counter := 0
 	prepLayout := func(t *testing.T, cache string, repos []string) *APK {
 		src := apkfs.NewMemFS()
 		err := src.MkdirAll("etc/apk", 0o755)
@@ -111,18 +108,13 @@ func TestGetRepositoryIndexes(t *testing.T) {
 			err = src.WriteFile(reposFilePath, []byte(strings.Join(repos, "\n")), 0o644)
 			require.NoErrorf(t, err, "unable to write repositories")
 		} else {
-			// Append an int to the testAlpineRepos path to bust the global cache if repos are unspecified.
-			// TODO: Stop being lazy and remove the global caches.
-			mu.Lock()
-			counter++
-			err = src.WriteFile(reposFilePath, []byte(fmt.Sprintf("%s/%d", testAlpineRepos, counter)), 0o644)
-			mu.Unlock()
+			err = src.WriteFile(reposFilePath, []byte(testAlpineRepos), 0o644)
 			require.NoErrorf(t, err, "unable to write repositories")
 		}
 
 		opts := []Option{WithFS(src), WithIgnoreMknodErrors(ignoreMknodErrors)}
 		if cache != "" {
-			opts = append(opts, WithCache(cache, false))
+			opts = append(opts, WithCache(cache, false, NewCache(false)))
 		}
 		a, err := New(opts...)
 		require.NoError(t, err, "unable to create APK")
@@ -149,9 +141,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.Greater(t, len(indexes), 0, "no indexes found")
 	})
 	t.Run("cache miss no network", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache, globalIndexCache = &etagCache{}, &indexCache{}
-
 		// we use a transport that always returns a 404 so we know we're not hitting the network
 		// it should fail for a cache hit
 		tmpDir := t.TempDir()
@@ -163,9 +152,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.Error(t, err, "should fail when no cache and no network")
 	})
 	t.Run("we can fetch, but do not cache indices without etag", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache, globalIndexCache = &etagCache{}, &indexCache{}
-
 		// we use a transport that can read from the network
 		// it should fail for a cache hit
 		tmpDir := t.TempDir()
@@ -186,9 +172,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		}))
 	})
 	t.Run("cache miss network should fill cache", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache, globalIndexCache = &etagCache{}, &indexCache{}
-
 		// we use a transport that can read from the network
 		// it should fail for a cache hit
 		tmpDir := t.TempDir()
@@ -216,9 +199,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.Equal(t, index1, index2, "index files do not match")
 	})
 	t.Run("repo url with http basic auth", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache = &etagCache{}
-
 		tmpDir := t.TempDir()
 		a := prepLayout(t, tmpDir, []string{"https://user:pass@dl-cdn.alpinelinux.org/alpine/v3.16/main"})
 
@@ -235,9 +215,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.Greater(t, len(indexes), 0, "no indexes found")
 	})
 	t.Run("cache hit etag match", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache = &etagCache{}
-
 		// it should succeed for a cache hit
 		tmpDir := t.TempDir()
 		testEtag := "test-etag"
@@ -270,9 +247,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.Equal(t, index1, index2, "index files do not match")
 	})
 	t.Run("cache hit etag miss", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache, globalIndexCache = &etagCache{}, &indexCache{}
-
 		// it should succeed for a cache hit
 		tmpDir := t.TempDir()
 		testEtag := "test-etag"
@@ -297,9 +271,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 			Transport: &testLocalTransport{root: testAlternatePkgDir, basenameOnly: true, headers: map[string][]string{http.CanonicalHeaderKey("etag"): {testEtag + "change"}}},
 		})
 
-		// Reset etag cache.
-		globalEtagCache, globalIndexCache = &etagCache{}, &indexCache{}
-
 		indexes, err = a.GetRepositoryIndexes(context.TODO(), false)
 		require.NoErrorf(t, err, "unable to get indexes")
 		require.Greater(t, len(indexes), 0, "no indexes found")
@@ -310,9 +281,6 @@ func TestGetRepositoryIndexes(t *testing.T) {
 		require.NotEqual(t, index1, index2, "index files do not match")
 	})
 	t.Run("test cache concurrency", func(t *testing.T) {
-		// Reset etag cache so we have isolated tests.
-		globalEtagCache = &etagCache{}
-
 		// Use the same temp directory for the cache.
 		tmpDir := t.TempDir()
 
diff --git a/pkg/build/build_test.go b/pkg/build/build_test.go
index 3cbbf3c..c663cc1 100644
--- a/pkg/build/build_test.go
+++ b/pkg/build/build_test.go
@@ -112,7 +112,7 @@ func TestAuth_good(t *testing.T) {
 	called := false
 	testUser, testPass := "user", "pass"
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("ETag", "123")
+		w.Header().Set("ETag", "234")
 		if r.Method == http.MethodHead {
 			return
 		}
@@ -151,7 +151,7 @@ func TestAuth_bad(t *testing.T) {
 	called := false
 	testUser, testPass := "user", "pass"
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("ETag", "123")
+		w.Header().Set("ETag", "345")
 		if r.Method == http.MethodHead {
 			return
 		}