Skip to content

Commit 9e5c600

Browse files
dgageotdgageot
authored
Merge pull request #826 from dgageot/fix-oci
Improve `cagent push` and `cagent pull`
2 parents 11d27b2 + c3aebc0 commit 9e5c600

13 files changed

Lines changed: 163 additions & 97 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ if err != nil {
277277
}
278278
279279
// For YAML errors, use formatted output
280-
if err := yaml.UnmarshalWithOptions(data, &raw); err != nil {
280+
if err := yaml.Unmarshal(data, &raw); err != nil {
281281
return nil, fmt.Errorf("parsing config:\n%s", yaml.FormatError(err, true, true))
282282
}
283283

‎cmd/root/api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (f *apiFlags) runAPICommand(cmd *cobra.Command, args []string) error {
126126
return
127127
case <-ticker.C:
128128
slog.Info("Auto-pulling OCI reference", "reference", agentsPath)
129-
if _, err := remote.Pull(ctx, agentsPath); err != nil {
129+
if _, err := remote.Pull(ctx, agentsPath, false); err != nil {
130130
slog.Error("Failed to auto-pull OCI reference", "reference", agentsPath, "error", err)
131131
continue
132132
}

‎cmd/root/pull.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,28 @@ import (
1515
"github.com/docker/cagent/pkg/telemetry"
1616
)
1717

18+
type pullFlags struct {
19+
force bool
20+
}
21+
1822
func newPullCmd() *cobra.Command {
19-
return &cobra.Command{
23+
var flags pullFlags
24+
25+
cmd := &cobra.Command{
2026
Use: "pull <registry-ref>",
2127
Short: "Pull an agent from an OCI registry",
2228
Long: "Pull an agent configuration file from an OCI registry",
2329
GroupID: "core",
2430
Args: cobra.ExactArgs(1),
25-
RunE: runPullCommand,
31+
RunE: flags.runPullCommand,
2632
}
33+
34+
cmd.PersistentFlags().BoolVar(&flags.force, "force", false, "Force pull even if the configuration already exists locally")
35+
36+
return cmd
2737
}
2838

29-
func runPullCommand(cmd *cobra.Command, args []string) error {
39+
func (f *pullFlags) runPullCommand(cmd *cobra.Command, args []string) error {
3040
telemetry.TrackCommand("pull", args)
3141

3242
ctx := cmd.Context()
@@ -37,7 +47,7 @@ func runPullCommand(cmd *cobra.Command, args []string) error {
3747
out.Println("Pulling agent", registryRef)
3848

3949
var opts []crane.Option
40-
_, err := remote.Pull(ctx, registryRef, opts...)
50+
_, err := remote.Pull(ctx, registryRef, f.force, opts...)
4151
if err != nil {
4252
return fmt.Errorf("failed to pull artifact: %w", err)
4353
}

‎cmd/root/push.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func newPushCmd() *cobra.Command {
2727
func runPushCommand(cmd *cobra.Command, args []string) error {
2828
telemetry.TrackCommand("push", args)
2929

30+
ctx := cmd.Context()
3031
filePath := args[0]
3132
tag := args[1]
3233
out := cli.NewPrinter(cmd.OutOrStdout())
@@ -36,7 +37,7 @@ func runPushCommand(cmd *cobra.Command, args []string) error {
3637
return err
3738
}
3839

39-
_, err = oci.PackageFileAsOCIToStore(filePath, tag, store)
40+
_, err = oci.PackageFileAsOCIToStore(ctx, filePath, tag, store)
4041
if err != nil {
4142
return fmt.Errorf("failed to build artifact: %w", err)
4243
}

‎pkg/agentfile/resolver.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func Resolve(ctx context.Context, out *cli.Printer, agentFilename string) (strin
105105
a, err := FromStore(agentFilename)
106106
if err != nil {
107107
out.Println("Pulling agent", agentFilename)
108-
if _, pullErr := remote.Pull(ctx, agentFilename); pullErr != nil {
108+
if _, pullErr := remote.Pull(ctx, agentFilename, false); pullErr != nil {
109109
return "", fmt.Errorf("failed to pull OCI image %s: %w", agentFilename, pullErr)
110110
}
111111
// Retry after pull

‎pkg/agentfile/resolver_test.go‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ agents:
112112

113113
// Package as OCI artifact
114114
ociRef := "test.registry.io/myorg/testagent:v1"
115-
_, err = oci.PackageFileAsOCIToStore(agentFile, ociRef, store)
115+
_, err = oci.PackageFileAsOCIToStore(t.Context(), agentFile, ociRef, store)
116116
require.NoError(t, err)
117117

118118
ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
@@ -125,7 +125,13 @@ agents:
125125

126126
content1, err := os.ReadFile(resolved1)
127127
require.NoError(t, err)
128-
assert.Equal(t, agentContent, string(content1))
128+
assert.Equal(t, `version: "1"
129+
agents:
130+
root:
131+
model: openai/gpt-4o
132+
description: Test OCI agent
133+
instruction: You are a test OCI agent
134+
`, string(content1))
129135

130136
// Expected filename based on OCI ref
131137
expectedFilename := OciRefToFilename(ociRef)
@@ -156,7 +162,7 @@ agents:
156162
`
157163
updatedFile := filepath.Join(t.TempDir(), "updated-agent.yaml")
158164
require.NoError(t, os.WriteFile(updatedFile, []byte(updatedContent), 0o644))
159-
_, err = oci.PackageFileAsOCIToStore(updatedFile, ociRef, store)
165+
_, err = oci.PackageFileAsOCIToStore(t.Context(), updatedFile, ociRef, store)
160166
require.NoError(t, err)
161167

162168
// Third resolution (simulating reload after update)
@@ -202,9 +208,9 @@ agents:
202208
// Package as different OCI artifacts
203209
ociRef1 := "test.io/org/agent1:v1"
204210
ociRef2 := "test.io/org/agent2:v1"
205-
_, err = oci.PackageFileAsOCIToStore(agent1File, ociRef1, store)
211+
_, err = oci.PackageFileAsOCIToStore(t.Context(), agent1File, ociRef1, store)
206212
require.NoError(t, err)
207-
_, err = oci.PackageFileAsOCIToStore(agent2File, ociRef2, store)
213+
_, err = oci.PackageFileAsOCIToStore(t.Context(), agent2File, ociRef2, store)
208214
require.NoError(t, err)
209215

210216
ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
@@ -252,7 +258,7 @@ agents:
252258

253259
// Package as OCI artifact
254260
ociRef := "test.io/cleanup/agent:v1"
255-
_, err = oci.PackageFileAsOCIToStore(agentFile, ociRef, store)
261+
_, err = oci.PackageFileAsOCIToStore(t.Context(), agentFile, ociRef, store)
256262
require.NoError(t, err)
257263

258264
ctx, cancel := context.WithCancel(t.Context())
@@ -454,3 +460,40 @@ func TestResolveAgentFile_ReplaceEmptyAliasWithActualFile(t *testing.T) {
454460
require.NoError(t, err)
455461
assert.Equal(t, aliasedAgentFile, resolved)
456462
}
463+
464+
func TestResolveAgentFile_OCIRef_HasAVersion(t *testing.T) {
465+
storeDir := t.TempDir()
466+
t.Setenv("CAGENT_CONTENT_STORE", storeDir)
467+
468+
store, err := content.NewStore()
469+
require.NoError(t, err)
470+
471+
agentContent := `agents:
472+
root:
473+
model: auto
474+
description: Test OCI agent
475+
instruction: You are a test OCI agent
476+
`
477+
agentFile := filepath.Join(t.TempDir(), "oci-agent.yaml")
478+
require.NoError(t, os.WriteFile(agentFile, []byte(agentContent), 0o644))
479+
480+
// Package as OCI artifact
481+
ociRef := "test.registry.io/myorg/testagent:v1"
482+
_, err = oci.PackageFileAsOCIToStore(t.Context(), agentFile, ociRef, store)
483+
require.NoError(t, err)
484+
485+
// First resolution
486+
resolved, err := Resolve(t.Context(), nil, ociRef)
487+
require.NoError(t, err)
488+
assert.NotEmpty(t, resolved)
489+
490+
storedContent, err := os.ReadFile(resolved)
491+
require.NoError(t, err)
492+
assert.Equal(t, `version: "2"
493+
agents:
494+
root:
495+
model: auto
496+
description: Test OCI agent
497+
instruction: You are a test OCI agent
498+
`, string(storedContent))
499+
}

‎pkg/config/config.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func LoadConfigBytes(ctx context.Context, data []byte) (*latest.Config, error) {
2828
var raw struct {
2929
Version string `yaml:"version,omitempty"`
3030
}
31-
if err := yaml.UnmarshalWithOptions(data, &raw); err != nil {
31+
if err := yaml.Unmarshal(data, &raw); err != nil {
3232
return nil, fmt.Errorf("looking for version in config file\n%s", yaml.FormatError(err, true, true))
3333
}
3434
if raw.Version == "" {

‎pkg/oci/package.go‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,29 @@
11
package oci
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"path/filepath"
78
"strings"
89
"time"
910

11+
"github.com/goccy/go-yaml"
1012
v1 "github.com/google/go-containerregistry/pkg/v1"
1113
"github.com/google/go-containerregistry/pkg/v1/empty"
1214
"github.com/google/go-containerregistry/pkg/v1/mutate"
1315
"github.com/google/go-containerregistry/pkg/v1/static"
1416
"github.com/google/go-containerregistry/pkg/v1/types"
1517

18+
"github.com/docker/cagent/pkg/config"
19+
latest "github.com/docker/cagent/pkg/config/v2"
1620
"github.com/docker/cagent/pkg/content"
1721
"github.com/docker/cagent/pkg/path"
22+
"github.com/docker/cagent/pkg/version"
1823
)
1924

2025
// PackageFileAsOCIToStore creates an OCI artifact from a file and stores it in the content store
21-
func PackageFileAsOCIToStore(filePath, artifactRef string, store *content.Store) (string, error) {
26+
func PackageFileAsOCIToStore(ctx context.Context, filePath, artifactRef string, store *content.Store) (string, error) {
2227
if !strings.Contains(artifactRef, ":") {
2328
artifactRef += ":latest"
2429
}
@@ -34,20 +39,47 @@ func PackageFileAsOCIToStore(filePath, artifactRef string, store *content.Store)
3439
return "", fmt.Errorf("reading file: %w", err)
3540
}
3641

37-
layer := static.NewLayer(data, types.OCIUncompressedLayer)
38-
39-
img := empty.Image
40-
41-
img, err = mutate.AppendLayers(img, layer)
42+
cfg, err := config.LoadConfigBytes(ctx, data)
4243
if err != nil {
43-
return "", fmt.Errorf("appending layer: %w", err)
44+
return "", fmt.Errorf("loading config: %w", err)
45+
}
46+
47+
var raw struct {
48+
Version string `yaml:"version,omitempty"`
49+
}
50+
if err := yaml.Unmarshal(data, &raw); err != nil {
51+
return "", fmt.Errorf("looking for version in config file\n%s", yaml.FormatError(err, true, true))
52+
}
53+
// Make sure we push a yaml with a version (Use latest if missing)
54+
if raw.Version == "" {
55+
cfg.Version = latest.Version
56+
data, err = yaml.MarshalWithOptions(cfg, yaml.Indent(2))
57+
if err != nil {
58+
return "", fmt.Errorf("marshaling config: %w", err)
59+
}
4460
}
4561

62+
// Prepare OCI annotations
4663
annotations := map[string]string{
64+
"io.docker.cagent.version": version.Version,
4765
"org.opencontainers.image.created": time.Now().Format(time.RFC3339),
4866
"org.opencontainers.image.description": fmt.Sprintf("OCI artifact containing %s", filepath.Base(validatedPath)),
4967
}
68+
if author := cfg.Metadata.Author; author != "" {
69+
annotations["org.opencontainers.image.authors"] = author
70+
}
71+
if license := cfg.Metadata.License; license != "" {
72+
annotations["org.opencontainers.image.licenses"] = license
73+
}
74+
75+
layer := static.NewLayer(data, types.OCIUncompressedLayer)
76+
img, err := mutate.AppendLayers(empty.Image, layer)
77+
if err != nil {
78+
return "", fmt.Errorf("appending layer: %w", err)
79+
}
5080

81+
// Convert to OCI manifest format to support annotations
82+
img = mutate.MediaType(img, types.OCIManifestSchema1)
5183
img = mutate.Annotations(img, annotations).(v1.Image)
5284

5385
digest, err := store.StoreArtifact(img, artifactRef)

‎pkg/oci/package_test.go‎

Lines changed: 14 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,20 @@ import (
1313

1414
func TestPackageFileAsOCIToStore(t *testing.T) {
1515
testFile := filepath.Join(t.TempDir(), "test.yaml")
16-
testContent := `name: test-app
17-
version: v1.0.0
18-
description: "Test application"
16+
testContent := `version: "2"
17+
agents:
18+
root:
19+
model: auto
20+
description: A helpful AI assistant
1921
`
2022
require.NoError(t, os.WriteFile(testFile, []byte(testContent), 0o644))
2123
store, err := content.NewStore(content.WithBaseDir(t.TempDir()))
2224
require.NoError(t, err)
2325

2426
tag := "test-app:v1.0.0"
25-
digest, err := PackageFileAsOCIToStore(testFile, tag, store)
27+
digest, err := PackageFileAsOCIToStore(t.Context(), testFile, tag, store)
2628
require.NoError(t, err)
27-
2829
assert.NotEmpty(t, digest)
29-
3030
t.Cleanup(func() {
3131
if err := store.DeleteArtifact(digest); err != nil {
3232
t.Logf("Failed to clean up artifact: %v", err)
@@ -35,20 +35,25 @@ description: "Test application"
3535

3636
img, err := store.GetArtifactImage(tag)
3737
require.NoError(t, err)
38-
3938
assert.NotNil(t, img)
4039

4140
metadata, err := store.GetArtifactMetadata(tag)
4241
require.NoError(t, err)
4342

4443
assert.Equal(t, tag, metadata.Reference)
4544
assert.Equal(t, digest, metadata.Digest)
45+
46+
// Verify annotations are present
47+
require.NotNil(t, metadata.Annotations)
48+
assert.Contains(t, metadata.Annotations, "org.opencontainers.image.created")
49+
assert.Contains(t, metadata.Annotations, "org.opencontainers.image.description")
50+
assert.Equal(t, "OCI artifact containing test.yaml", metadata.Annotations["org.opencontainers.image.description"])
4651
}
4752

4853
func TestPackageFileAsOCIToStoreMissingFile(t *testing.T) {
4954
store, err := content.NewStore(content.WithBaseDir(t.TempDir()))
5055
require.NoError(t, err)
51-
_, err = PackageFileAsOCIToStore("/non/existent/file.txt", "test:latest", store)
56+
_, err = PackageFileAsOCIToStore(t.Context(), "/non/existent/file.txt", "test:latest", store)
5257
require.Error(t, err)
5358
}
5459

@@ -58,64 +63,6 @@ func TestPackageFileAsOCIToStoreInvalidTag(t *testing.T) {
5863

5964
store, err := content.NewStore(content.WithBaseDir(t.TempDir()))
6065
require.NoError(t, err)
61-
_, err = PackageFileAsOCIToStore(testFile, "", store)
66+
_, err = PackageFileAsOCIToStore(t.Context(), testFile, "", store)
6267
require.Error(t, err)
6368
}
64-
65-
func TestPackageFileAsOCIToStoreDifferentFileTypes(t *testing.T) {
66-
testCases := []struct {
67-
name string
68-
filename string
69-
content string
70-
tag string
71-
}{
72-
{
73-
name: "yaml file",
74-
filename: "config.yaml",
75-
content: "key: value\nother: data",
76-
tag: "config:yaml",
77-
},
78-
{
79-
name: "json file",
80-
filename: "data.json",
81-
content: `{"key": "value", "number": 42}`,
82-
tag: "data:json",
83-
},
84-
{
85-
name: "text file",
86-
filename: "readme.txt",
87-
content: "This is a simple text file\nwith multiple lines",
88-
tag: "readme:txt",
89-
},
90-
}
91-
92-
store, err := content.NewStore(content.WithBaseDir(t.TempDir()))
93-
require.NoError(t, err)
94-
95-
var digests []string
96-
97-
for _, tc := range testCases {
98-
t.Run(tc.name, func(t *testing.T) {
99-
testFile := filepath.Join(t.TempDir(), tc.filename)
100-
require.NoError(t, os.WriteFile(testFile, []byte(tc.content), 0o644))
101-
102-
// Package the file as OCI artifact
103-
digest, err := PackageFileAsOCIToStore(testFile, tc.tag, store)
104-
require.NoError(t, err)
105-
106-
digests = append(digests, digest)
107-
108-
img, err := store.GetArtifactImage(tc.tag)
109-
require.NoError(t, err)
110-
assert.NotNil(t, img)
111-
})
112-
}
113-
114-
t.Cleanup(func() {
115-
for _, digest := range digests {
116-
if err := store.DeleteArtifact(digest); err != nil {
117-
t.Logf("Failed to clean up artifact %s: %v", digest, err)
118-
}
119-
}
120-
})
121-
}

0 commit comments

Comments
 (0)