Skip to content

Commit 71eaa8f

Browse files
committed
feat: serve digest-pinned OCI references directly from cache
When an OCI reference includes a digest (e.g. repo@sha256:abc...), the content is immutable. If the artifact is already in the local content store, serve it directly without any network call to the registry, avoiding the ~500ms crane.Digest() round-trip. Also fix content store resolution to handle digest references of the form 'repo@sha256:...' by extracting the digest portion, so artifacts stored under a tag ref can be retrieved by digest. Assisted-By: docker-agent
1 parent cb4354c commit 71eaa8f

6 files changed

Lines changed: 167 additions & 45 deletions

File tree

pkg/config/sources.go

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ func (a ociSource) ParentDir() string {
110110
return ""
111111
}
112112

113-
// Read loads an agent configuration from an OCI artifact
113+
// Read loads an agent configuration from an OCI artifact.
114114
//
115-
// The OCI registry remains the source of truth
116-
// The local content store is used as a cache and fallback only
117-
// A forced re-pull is triggered exclusively when store corruption is detected
115+
// The OCI registry remains the source of truth.
116+
// The local content store is used as a cache and fallback only.
117+
// A forced re-pull is triggered exclusively when store corruption is detected.
118118
func (a ociSource) Read(ctx context.Context) ([]byte, error) {
119119
store, err := content.NewStore()
120120
if err != nil {
@@ -129,59 +129,63 @@ func (a ociSource) Read(ctx context.Context) ([]byte, error) {
129129
return nil, fmt.Errorf("normalizing OCI reference %s: %w", a.reference, err)
130130
}
131131

132-
tryLoad := func() ([]byte, error) {
133-
af, err := store.GetArtifact(storeKey)
134-
if err != nil {
135-
return nil, err
132+
// For digest references, the content is immutable. If we already have
133+
// the artifact locally, serve it directly without any network call.
134+
if remote.IsDigestReference(a.reference) {
135+
if data, loadErr := loadArtifact(store, storeKey); loadErr == nil {
136+
slog.Debug("Serving digest-pinned OCI artifact from cache", "ref", a.reference)
137+
return data, nil
136138
}
137-
return []byte(af), nil
138139
}
139140

140-
// Check if we have any local metadata
141-
_, metaErr := store.GetArtifactMetadata(storeKey)
142-
hasLocal := metaErr == nil
141+
// Check whether we have a local copy to fall back on.
142+
hasLocal := hasLocalArtifact(store, storeKey)
143143

144-
// Always try normal pull first (preserves pull-interval behavior)
144+
// Pull from registry (checks remote digest, skips download if unchanged).
145145
if _, pullErr := remote.Pull(ctx, a.reference, false); pullErr != nil {
146146
if !hasLocal {
147147
return nil, fmt.Errorf("failed to pull OCI image %s: %w", a.reference, pullErr)
148148
}
149-
150-
slog.Debug(
151-
"Failed to check for OCI reference updates, using cached version",
152-
"ref", a.reference,
153-
"error", pullErr,
154-
)
149+
slog.Debug("Failed to check for OCI reference updates, using cached version",
150+
"ref", a.reference, "error", pullErr)
155151
}
156152

157-
// Try loading from store
158-
data, err := tryLoad()
153+
// Try loading from store.
154+
data, err := loadArtifact(store, storeKey)
159155
if err == nil {
160156
return data, nil
161157
}
162158

163-
// If loading failed due to corruption, force re-pull
164-
if errors.Is(err, content.ErrStoreCorrupted) {
165-
slog.Warn(
166-
"Local OCI store corrupted, forcing re-pull",
167-
"ref", a.reference,
168-
)
159+
// If corrupted, force re-pull and try once more.
160+
if !errors.Is(err, content.ErrStoreCorrupted) {
161+
return nil, fmt.Errorf("failed to load agent from OCI source %s: %w", a.reference, err)
162+
}
169163

170-
if _, pullErr := remote.Pull(ctx, a.reference, true); pullErr != nil {
171-
return nil, fmt.Errorf("failed to force re-pull OCI image %s: %w", a.reference, pullErr)
172-
}
164+
slog.Warn("Local OCI store corrupted, forcing re-pull", "ref", a.reference)
165+
if _, pullErr := remote.Pull(ctx, a.reference, true); pullErr != nil {
166+
return nil, fmt.Errorf("failed to force re-pull OCI image %s: %w", a.reference, pullErr)
167+
}
173168

174-
data, err = tryLoad()
175-
if err == nil {
176-
return data, nil
177-
}
169+
data, err = loadArtifact(store, storeKey)
170+
if err != nil {
171+
return nil, fmt.Errorf("failed to load agent from OCI source %s: %w", a.reference, err)
178172
}
173+
return data, nil
174+
}
175+
176+
// loadArtifact reads the agent YAML from the content store.
177+
func loadArtifact(store *content.Store, storeKey string) ([]byte, error) {
178+
af, err := store.GetArtifact(storeKey)
179+
if err != nil {
180+
return nil, err
181+
}
182+
return []byte(af), nil
183+
}
179184

180-
return nil, fmt.Errorf(
181-
"failed to load agent from OCI source %s: %w",
182-
a.reference,
183-
err,
184-
)
185+
// hasLocalArtifact reports whether the content store has metadata for the given key.
186+
func hasLocalArtifact(store *content.Store, storeKey string) bool {
187+
_, err := store.GetArtifactMetadata(storeKey)
188+
return err == nil
185189
}
186190

187191
// urlSource is used to load an agent configuration from an HTTP/HTTPS URL.

pkg/config/sources_test.go

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,59 @@ import (
88
"sync/atomic"
99
"testing"
1010

11+
v1 "github.com/google/go-containerregistry/pkg/v1"
12+
"github.com/google/go-containerregistry/pkg/v1/empty"
13+
"github.com/google/go-containerregistry/pkg/v1/mutate"
14+
"github.com/google/go-containerregistry/pkg/v1/static"
1115
"github.com/stretchr/testify/assert"
1216
"github.com/stretchr/testify/require"
1317

18+
"github.com/docker/docker-agent/pkg/content"
1419
"github.com/docker/docker-agent/pkg/environment"
20+
"github.com/docker/docker-agent/pkg/remote"
1521
)
1622

23+
func TestOCISource_DigestReference_ServesFromCache(t *testing.T) {
24+
t.Parallel()
25+
26+
// Create a temporary content store and store a test artifact.
27+
storeDir := t.TempDir()
28+
store, err := content.NewStore(content.WithBaseDir(storeDir))
29+
require.NoError(t, err)
30+
31+
testData := []byte("version: v1\nname: test-agent")
32+
layer := static.NewLayer(testData, "application/yaml")
33+
img, err := mutate.AppendLayers(empty.Image, layer)
34+
require.NoError(t, err)
35+
img = mutate.Annotations(img, map[string]string{
36+
"io.docker.agent.version": "test",
37+
}).(v1.Image)
38+
39+
ref := "test-digest-cache/agent:latest"
40+
digest, err := store.StoreArtifact(img, ref)
41+
require.NoError(t, err)
42+
43+
// Build a digest reference using the stored digest.
44+
digestRef := "test-digest-cache/agent@" + digest
45+
46+
// Read via ociSource. Since the reference is pinned by digest and is
47+
// present in the local store, this must succeed without any network call.
48+
// We override the default store directory via an env-based approach;
49+
// instead, we directly exercise the cache-hit logic by verifying the
50+
// store lookup works with the normalized key.
51+
storeKey, err := remote.NormalizeReference(digestRef)
52+
require.NoError(t, err)
53+
54+
// Verify the store can resolve the digest key directly.
55+
data, err := store.GetArtifact(storeKey)
56+
require.NoError(t, err)
57+
assert.Equal(t, string(testData), data)
58+
59+
// Also verify that IsDigestReference correctly identifies this.
60+
assert.True(t, remote.IsDigestReference(digestRef))
61+
assert.False(t, remote.IsDigestReference(ref))
62+
}
63+
1764
func TestURLSource_Read(t *testing.T) {
1865
t.Parallel()
1966

@@ -204,11 +251,11 @@ func TestURLSource_Read_FallsBackToCacheOnHTTPError(t *testing.T) {
204251
func TestURLSource_Read_UpdatesCacheWhenContentChanges(t *testing.T) {
205252
// Not parallel - uses shared cache directory
206253

207-
var content atomic.Value
208-
content.Store("initial content update")
254+
var serverContent atomic.Value
255+
serverContent.Store("initial content update")
209256

210257
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
211-
currentContent := content.Load().(string)
258+
currentContent := serverContent.Load().(string)
212259
etag := `"etag-` + currentContent + `"`
213260

214261
if r.Header.Get("If-None-Match") == etag {
@@ -239,7 +286,7 @@ func TestURLSource_Read_UpdatesCacheWhenContentChanges(t *testing.T) {
239286
assert.Equal(t, "initial content update", string(data))
240287

241288
// Change content
242-
content.Store("updated content update")
289+
serverContent.Store("updated content update")
243290

244291
// Second read should get new content
245292
data, err = source.Read(t.Context())

pkg/content/store.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,12 +267,18 @@ func (s *Store) DeleteArtifact(identifier string) error {
267267
// resolveIdentifier resolves a user-provided identifier (digest or reference)
268268
// into a concrete content digest stored in the local artifact store.
269269
func (s *Store) resolveIdentifier(identifier string) (string, error) {
270-
// If the identifier is already a digest, we can return it directly.
271-
// This bypasses the refs lookup entirely.
270+
// If the identifier is already a bare digest, return it directly.
272271
if strings.HasPrefix(identifier, "sha256:") {
273272
return identifier, nil
274273
}
275274

275+
// If the identifier is a digest reference (e.g. "repo@sha256:abc..."),
276+
// extract and return the digest portion directly. Digest references
277+
// are content-addressable, so the digest alone identifies the artifact.
278+
if i := strings.LastIndex(identifier, "@sha256:"); i >= 0 {
279+
return identifier[i+1:], nil
280+
}
281+
276282
// If no tag is provided, default to ":latest".
277283
// This mirrors standard OCI reference semantics.
278284
if !strings.Contains(identifier, ":") {

pkg/content/store_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,34 @@ func TestStoreResolution(t *testing.T) {
109109
assert.NotNil(t, img)
110110
}
111111
}
112+
113+
func TestStoreResolution_DigestReference(t *testing.T) {
114+
store, err := NewStore(WithBaseDir(t.TempDir()))
115+
require.NoError(t, err)
116+
117+
testData := []byte("Digest resolution test")
118+
layer := static.NewLayer(testData, types.OCIUncompressedLayer)
119+
img := empty.Image
120+
img, err = mutate.AppendLayers(img, layer)
121+
require.NoError(t, err)
122+
123+
tagRef := "myrepo/agent:v1"
124+
digest, err := store.StoreArtifact(img, tagRef)
125+
require.NoError(t, err)
126+
127+
// Bare digest should resolve.
128+
retrievedImg, err := store.GetArtifactImage(digest)
129+
require.NoError(t, err)
130+
assert.NotNil(t, retrievedImg)
131+
132+
// Digest reference (repo@sha256:...) should also resolve.
133+
digestRef := "myrepo/agent@" + digest
134+
retrievedImg, err = store.GetArtifactImage(digestRef)
135+
require.NoError(t, err)
136+
assert.NotNil(t, retrievedImg)
137+
138+
// Metadata lookup via digest reference should work too.
139+
meta, err := store.GetArtifactMetadata(digestRef)
140+
require.NoError(t, err)
141+
assert.Equal(t, digest, meta.Digest)
142+
}

pkg/remote/pull.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@ func NormalizeReference(registryRef string) (string, error) {
2222
return ref.Context().RepositoryStr() + separator(ref) + ref.Identifier(), nil
2323
}
2424

25+
// IsDigestReference reports whether the given reference pins a specific
26+
// image digest (e.g. "repo@sha256:abc...").
27+
func IsDigestReference(registryRef string) bool {
28+
ref, err := name.ParseReference(registryRef)
29+
if err != nil {
30+
return false
31+
}
32+
_, ok := ref.(name.Digest)
33+
return ok
34+
}
35+
2536
// Pull pulls an artifact from a registry and stores it in the content store
2637
func Pull(ctx context.Context, registryRef string, force bool, opts ...crane.Option) (string, error) {
2738
opts = append(opts, crane.WithContext(ctx))

pkg/remote/pull_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,29 @@ func TestNormalizeReference_InvalidReference(t *testing.T) {
124124
require.Error(t, err)
125125
}
126126

127+
func TestIsDigestReference(t *testing.T) {
128+
t.Parallel()
129+
130+
tests := []struct {
131+
name string
132+
ref string
133+
expected bool
134+
}{
135+
{"tag reference", "agentcatalog/review-pr:latest", false},
136+
{"implicit tag", "agentcatalog/review-pr", false},
137+
{"digest reference", "agentcatalog/review-pr@sha256:0000000000000000000000000000000000000000000000000000000000000000", true},
138+
{"fully qualified digest", "index.docker.io/agentcatalog/review-pr@sha256:0000000000000000000000000000000000000000000000000000000000000000", true},
139+
{"invalid reference", ":::invalid", false},
140+
}
141+
142+
for _, tt := range tests {
143+
t.Run(tt.name, func(t *testing.T) {
144+
t.Parallel()
145+
assert.Equal(t, tt.expected, IsDigestReference(tt.ref))
146+
})
147+
}
148+
}
149+
127150
func TestSeparator(t *testing.T) {
128151
t.Parallel()
129152

0 commit comments

Comments
 (0)