chore: refactor fetch calls to be more oras compatible

oci/fetch.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2"
@@ -63,59 +64,64 @@ func (o *OrasRemote) FetchRoot(ctx context.Context) (*Manifest, error) {
 
 // FetchManifest fetches the manifest with the given descriptor from the remote repository.
 func (o *OrasRemote) FetchManifest(ctx context.Context, desc ocispec.Descriptor) (manifest *Manifest, err error) {
-	return FetchUnmarshal[*Manifest](ctx, o.FetchLayer, json.Unmarshal, desc)
+	return FetchUnmarshal[*Manifest](ctx, o, json.Unmarshal, desc)
 }
 
-// FetchLayer fetches the layer with the given descriptor from the remote repository.
-func (o *OrasRemote) FetchLayer(ctx context.Context, desc ocispec.Descriptor) (bytes []byte, err error) {
-	var src oras.ReadOnlyTarget
-	src = o.repo
+// src returns the read target for layer fetches, wrapping the repository with the
+// layer cache when one is configured.
+func (o *OrasRemote) src() oras.ReadOnlyTarget {
 	if o.cache != nil {
-		src = orasCache.New(o.repo, o.cache)
+		return orasCache.New(o.repo, o.cache)
 	}
-	return content.FetchAll(ctx, src, desc)
+	return o.repo
+}
+
+// Fetch fetches the content for the given descriptor, honoring the layer cache
+// when configured. This satisfies oras content.Fetcher, so an OrasRemote can be
+// passed directly to oras helpers such as content.FetchAll and FetchJSONFile.
+func (o *OrasRemote) Fetch(ctx context.Context, desc ocispec.Descriptor) (io.ReadCloser, error) {
+	return o.src().Fetch(ctx, desc)
+}
+
+// FetchLayer fetches (and digest-verifies) the layer with the given descriptor.
+func (o *OrasRemote) FetchLayer(ctx context.Context, desc ocispec.Descriptor) (bytes []byte, err error) {
+	return content.FetchAll(ctx, o, desc)
 }
 
 // FetchLayerReader fetches the layer with the given descriptor from the remote repository.
 func (o *OrasRemote) FetchLayerReader(ctx context.Context, desc ocispec.Descriptor) (*content.VerifyReader, error) {
-	var src oras.ReadOnlyTarget
-	src = o.repo
-	if o.cache != nil {
-		src = orasCache.New(o.repo, o.cache)
-	}
-	r, err := src.Fetch(ctx, desc)
+	r, err := o.Fetch(ctx, desc)
 	if err != nil {
 		return nil, err
 	}
 	return content.NewVerifyReader(r, desc), nil
 }
 
-// FetchJSONFile fetches the given JSON file from the remote repository.
-func FetchJSONFile[T any](ctx context.Context, fetcher func(ctx context.Context, desc ocispec.Descriptor) (bytes []byte, err error), manifest *Manifest, path string) (result T, err error) {
+// FetchJSONFile fetches and unmarshals the JSON file at path, located via the manifest.
+func FetchJSONFile[T any](ctx context.Context, fetcher content.Fetcher, manifest *Manifest, path string) (result T, err error) {
 	descriptor := manifest.Locate(path)
 	if IsEmptyDescriptor(descriptor) {
 		return result, fmt.Errorf("unable to find %s in the manifest", path)
 	}
 	return FetchUnmarshal[T](ctx, fetcher, json.Unmarshal, descriptor)
 }
 
-// FetchYAMLFile fetches the given YAML file from the remote repository.
-func FetchYAMLFile[T any](ctx context.Context, fetcher func(ctx context.Context, desc ocispec.Descriptor) (bytes []byte, err error), manifest *Manifest, path string) (result T, err error) {
+// FetchYAMLFile fetches and unmarshals the YAML file at path, located via the manifest.
+func FetchYAMLFile[T any](ctx context.Context, fetcher content.Fetcher, manifest *Manifest, path string) (result T, err error) {
 	descriptor := manifest.Locate(path)
 	if IsEmptyDescriptor(descriptor) {
 		return result, fmt.Errorf("unable to find %s in the manifest", path)
 	}
 	return FetchUnmarshal[T](ctx, fetcher, goyaml.Unmarshal, descriptor)
 }
 
-// FetchUnmarshal fetches the given descriptor from the remote repository and unmarshals it.
-func FetchUnmarshal[T any](ctx context.Context, fetcher func(ctx context.Context, desc ocispec.Descriptor) (bytes []byte, err error), unmarshaler func(data []byte, v any) error, descriptor ocispec.Descriptor) (result T, err error) {
-	bytes, err := fetcher(ctx, descriptor)
+// FetchUnmarshal fetches (and digest-verifies, via content.FetchAll) the descriptor and unmarshals it.
+func FetchUnmarshal[T any](ctx context.Context, fetcher content.Fetcher, unmarshaler func(data []byte, v any) error, descriptor ocispec.Descriptor) (result T, err error) {
+	b, err := content.FetchAll(ctx, fetcher, descriptor)
 	if err != nil {
 		return result, err
 	}
-	err = unmarshaler(bytes, &result)
-	if err != nil {
+	if err := unmarshaler(b, &result); err != nil {
 		return result, err
 	}
 	return result, nil

oci/fetch_test.go

@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: 2024-Present Defense Unicorns
+
+package oci
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"path/filepath"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2/content/file"
+	ocistore "oras.land/oras-go/v2/content/oci"
+
+	"github.com/defenseunicorns/pkg/helpers/v2"
+)
+
+type cachePayload struct {
+	Name string `json:"name" yaml:"name"`
+}
+
+// warm hits the origin; offline's origin is empty, so any read it serves came from
+// the shared cache - returning a value without error proves the path went through src().
+func (suite *OCISuite) cachedAndOfflineRemotes(ctx context.Context) (*ocistore.Store, *OrasRemote, *OrasRemote) {
+	suite.T().Helper()
+	store, err := ocistore.New(suite.T().TempDir())
+	suite.NoError(err)
+	warm, err := NewOrasRemote("oci://"+suite.remote.Repo().Reference.String(),
+		PlatformForArch(testArch), WithPlainHTTP(true), WithCache(store))
+	suite.NoError(err)
+	offline, err := NewOrasRemote(suite.setupInMemoryRegistry(ctx),
+		PlatformForArch(testArch), WithPlainHTTP(true), WithCache(store))
+	suite.NoError(err)
+	return store, warm, offline
+}
+
+func (suite *OCISuite) TestFetchLayerCache() {
+	ctx := context.TODO()
+
+	srcTempDir := suite.T().TempDir()
+	fileName := "cached-file"
+	fileContents := "cache me if you can"
+	path := filepath.Join(srcTempDir, fileName)
+	suite.NoError(os.WriteFile(path, []byte(fileContents), helpers.ReadWriteUser))
+	src, err := file.New(srcTempDir)
+	suite.NoError(err)
+	desc, err := src.Add(ctx, fileName, ocispec.MediaTypeImageLayer, path)
+	suite.NoError(err)
+	suite.publishPackage(src, []ocispec.Descriptor{desc})
+
+	store, warm, offline := suite.cachedAndOfflineRemotes(ctx)
+
+	exists, err := store.Exists(ctx, desc)
+	suite.NoError(err)
+	suite.False(exists)
+
+	b, err := warm.FetchLayer(ctx, desc)
+	suite.NoError(err)
+	suite.Equal(fileContents, string(b))
+
+	exists, err = store.Exists(ctx, desc)
+	suite.NoError(err)
+	suite.True(exists) // populated by the fetch
+
+	b, err = offline.FetchLayer(ctx, desc)
+	suite.NoError(err)
+	suite.Equal(fileContents, string(b)) // served from cache
+}
+
+func (suite *OCISuite) TestFetchersUseCache() {
+	ctx := context.TODO()
+
+	srcTempDir := suite.T().TempDir()
+	writeLayer := func(name, contents string) {
+		suite.NoError(os.WriteFile(filepath.Join(srcTempDir, name), []byte(contents), helpers.ReadWriteUser))
+	}
+	writeLayer("data.json", `{"name":"json-layer"}`)
+	writeLayer("data.yaml", "name: yaml-layer\n")
+
+	src, err := file.New(srcTempDir)
+	suite.NoError(err)
+	var descs []ocispec.Descriptor
+	for _, name := range []string{"data.json", "data.yaml"} {
+		desc, err := src.Add(ctx, name, ocispec.MediaTypeImageLayer, filepath.Join(srcTempDir, name))
+		suite.NoError(err)
+		descs = append(descs, desc)
+	}
+	suite.publishPackage(src, descs)
+
+	rootDesc, err := suite.remote.ResolveRoot(ctx)
+	suite.NoError(err)
+	root, err := suite.remote.FetchRoot(ctx)
+	suite.NoError(err)
+	jsonDesc := root.Locate("data.json")
+
+	_, warm, offline := suite.cachedAndOfflineRemotes(ctx)
+
+	// warm each fetcher through the origin, then assert offline returns the same
+	// value from cache without returning an error (which it would w/o pre-warming)
+	warmM, err := warm.FetchManifest(ctx, rootDesc)
+	suite.NoError(err)
+	offlineM, err := offline.FetchManifest(ctx, rootDesc)
+	suite.NoError(err)
+	suite.Equal(warmM, offlineM)
+
+	warmJSON, err := FetchJSONFile[cachePayload](ctx, warm, root, "data.json")
+	suite.NoError(err)
+	offlineJSON, err := FetchJSONFile[cachePayload](ctx, offline, root, "data.json")
+	suite.NoError(err)
+	suite.Equal(warmJSON, offlineJSON)
+
+	warmYAML, err := FetchYAMLFile[cachePayload](ctx, warm, root, "data.yaml")
+	suite.NoError(err)
+	offlineYAML, err := FetchYAMLFile[cachePayload](ctx, offline, root, "data.yaml")
+	suite.NoError(err)
+	suite.Equal(warmYAML, offlineYAML)
+
+	warmUn, err := FetchUnmarshal[cachePayload](ctx, warm, json.Unmarshal, jsonDesc)
+	suite.NoError(err)
+	offlineUn, err := FetchUnmarshal[cachePayload](ctx, offline, json.Unmarshal, jsonDesc)
+	suite.NoError(err)
+	suite.Equal(warmUn, offlineUn)
+}

oci/pull.go

@@ -15,8 +15,6 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras-go/v2"
 
-	orasCache "github.com/defenseunicorns/pkg/oci/cache"
-
 	"github.com/defenseunicorns/pkg/helpers/v2"
 )
 
@@ -73,13 +71,7 @@ func (o *OrasRemote) CopyToTarget(ctx context.Context, layers []ocispec.Descript
 		return oras.SkipNode
 	}
 
-	var src oras.ReadOnlyTarget
-	src = o.repo
-	if o.cache != nil {
-		src = orasCache.New(o.repo, o.cache)
-	}
-
-	_, err := oras.Copy(ctx, src, o.repo.Reference.String(), target, o.repo.Reference.String(), copyOpts)
+	_, err := oras.Copy(ctx, o.src(), o.repo.Reference.String(), target, o.repo.Reference.String(), copyOpts)
 	if err != nil {
 		return err
 	}