Skip to content

Commit d58c20d

Browse files
authored
Merge pull request #957 from dgageot/always-use-source
Always use a Source to load a config
2 parents 10874ff + d20a9fd commit d58c20d

10 files changed

Lines changed: 56 additions & 86 deletions

File tree

cmd/root/debug.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (f *debugFlags) runDebugConfigCommand(cmd *cobra.Command, args []string) er
5656
return err
5757
}
5858

59-
cfg, err := config.LoadConfigFrom(ctx, agentSource)
59+
cfg, err := config.Load(ctx, agentSource)
6060
if err != nil {
6161
return err
6262
}

pkg/build/build.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func DockerImage(ctx context.Context, out Printer, agentFilename string, fs file
3939
return err
4040
}
4141

42-
cfg, err := config.LoadConfigFrom(ctx, agentSource)
42+
cfg, err := config.Load(ctx, agentSource)
4343
if err != nil {
4444
return err
4545
}

pkg/config/commands_test.go

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,60 @@
11
package config
22

33
import (
4+
"context"
5+
"os"
46
"testing"
57

68
"github.com/stretchr/testify/require"
79
)
810

911
func TestV2Commands_AllForms(t *testing.T) {
10-
cfg, err := LoadConfig(t.Context(), "commands_v2.yaml", openRoot(t, "testdata"))
12+
cfg, err := Load(t.Context(), fileSource("testdata/commands_v2.yaml"))
1113
require.NoError(t, err)
12-
// map form
14+
1315
cmdsMap := cfg.Agents["root"].Commands
1416
require.Equal(t, "check disk", cmdsMap["df"])
1517
require.Equal(t, "list files", cmdsMap["ls"])
16-
// list form
18+
1719
cmdsList := cfg.Agents["another_agent"].Commands
1820
require.Equal(t, "check disk", cmdsList["df"])
1921
require.Equal(t, "list files", cmdsList["ls"])
20-
// none
22+
2123
require.Empty(t, cfg.Agents["none_agent"].Commands)
2224
}
2325

2426
func TestMigrate_v1_Commands_AllForms(t *testing.T) {
25-
cfg, err := LoadConfig(t.Context(), "commands_v1.yaml", openRoot(t, "testdata"))
27+
cfg, err := Load(t.Context(), fileSource("testdata/commands_v1.yaml"))
2628
require.NoError(t, err)
27-
// map form
29+
2830
cmdsMap := cfg.Agents["root"].Commands
2931
require.Equal(t, "check disk", cmdsMap["df"])
3032
require.Equal(t, "list files", cmdsMap["ls"])
31-
// list form
33+
3234
cmdsList := cfg.Agents["another_agent"].Commands
3335
require.Equal(t, "check disk", cmdsList["df"])
3436
require.Equal(t, "list files", cmdsList["ls"])
35-
// none
37+
3638
require.Empty(t, cfg.Agents["yet_another_agent"].Commands)
3739
}
3840

3941
func TestMigrate_v0_Commands_AllForms(t *testing.T) {
40-
cfg, err := LoadConfig(t.Context(), "commands_v0.yaml", openRoot(t, "testdata"))
42+
cfg, err := Load(t.Context(), fileSource("testdata/commands_v0.yaml"))
4143
require.NoError(t, err)
42-
// map form
44+
4345
cmdsMap := cfg.Agents["root"].Commands
4446
require.Equal(t, "check disk", cmdsMap["df"])
4547
require.Equal(t, "list files", cmdsMap["ls"])
46-
// list form
48+
4749
cmdsList := cfg.Agents["another_agent"].Commands
4850
require.Equal(t, "check disk", cmdsList["df"])
4951
require.Equal(t, "list files", cmdsList["ls"])
50-
// none
52+
5153
require.Empty(t, cfg.Agents["yet_another_agent"].Commands)
5254
}
55+
56+
type fileSource string
57+
58+
func (s fileSource) Read(context.Context) ([]byte, error) {
59+
return os.ReadFile(string(s))
60+
}

pkg/config/config.go

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,18 @@ import (
99

1010
"github.com/docker/cagent/pkg/config/latest"
1111
"github.com/docker/cagent/pkg/environment"
12-
"github.com/docker/cagent/pkg/filesystem"
1312
)
1413

1514
type Source interface {
1615
Read(ctx context.Context) ([]byte, error)
1716
}
1817

19-
func LoadConfigFrom(ctx context.Context, source Source) (*latest.Config, error) {
18+
func Load(ctx context.Context, source Source) (*latest.Config, error) {
2019
data, err := source.Read(ctx)
2120
if err != nil {
2221
return nil, err
2322
}
2423

25-
return loadConfigBytes(data)
26-
}
27-
28-
func LoadConfig(ctx context.Context, path string, fs filesystem.FS) (*latest.Config, error) {
29-
data, err := fs.ReadFile(path)
30-
if err != nil {
31-
return nil, fmt.Errorf("reading config file %s: %w", path, err)
32-
}
33-
34-
return loadConfigBytes(data)
35-
}
36-
37-
func loadConfigBytes(data []byte) (*latest.Config, error) {
3824
var raw struct {
3925
Version string `yaml:"version,omitempty"`
4026
}

pkg/config/config_test.go

Lines changed: 15 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ import (
1515
func TestAutoRegisterModels(t *testing.T) {
1616
t.Parallel()
1717

18-
root := openRoot(t, "testdata")
19-
20-
cfg, err := LoadConfig(t.Context(), "autoregister.yaml", root)
18+
cfg, err := Load(t.Context(), fileSource("testdata/autoregister.yaml"))
2119
require.NoError(t, err)
2220

2321
assert.Len(t, cfg.Models, 2)
@@ -30,9 +28,7 @@ func TestAutoRegisterModels(t *testing.T) {
3028
func TestAutoRegisterAlloy(t *testing.T) {
3129
t.Parallel()
3230

33-
root := openRoot(t, "testdata")
34-
35-
cfg, err := LoadConfig(t.Context(), "autoregister_alloy.yaml", root)
31+
cfg, err := Load(t.Context(), fileSource("testdata/autoregister_alloy.yaml"))
3632
require.NoError(t, err)
3733

3834
assert.Len(t, cfg.Models, 2)
@@ -45,9 +41,7 @@ func TestAutoRegisterAlloy(t *testing.T) {
4541
func TestMigrate_v0_v1_provider(t *testing.T) {
4642
t.Parallel()
4743

48-
root := openRoot(t, "testdata")
49-
50-
cfg, err := LoadConfig(t.Context(), "provider_v0.yaml", root)
44+
cfg, err := Load(t.Context(), fileSource("testdata/provider_v0.yaml"))
5145
require.NoError(t, err)
5246

5347
assert.Equal(t, "openai", cfg.Models["gpt"].Provider)
@@ -56,9 +50,7 @@ func TestMigrate_v0_v1_provider(t *testing.T) {
5650
func TestMigrate_v1_provider(t *testing.T) {
5751
t.Parallel()
5852

59-
root := openRoot(t, "testdata")
60-
61-
cfg, err := LoadConfig(t.Context(), "provider_v1.yaml", root)
53+
cfg, err := Load(t.Context(), fileSource("testdata/provider_v1.yaml"))
6254
require.NoError(t, err)
6355

6456
assert.Equal(t, "openai", cfg.Models["gpt"].Provider)
@@ -67,9 +59,7 @@ func TestMigrate_v1_provider(t *testing.T) {
6759
func TestMigrate_v0_v1_todo(t *testing.T) {
6860
t.Parallel()
6961

70-
root := openRoot(t, "testdata")
71-
72-
cfg, err := LoadConfig(t.Context(), "todo_v0.yaml", root)
62+
cfg, err := Load(t.Context(), fileSource("testdata/todo_v0.yaml"))
7363
require.NoError(t, err)
7464

7565
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -81,9 +71,7 @@ func TestMigrate_v0_v1_todo(t *testing.T) {
8171
func TestMigrate_v1_todo(t *testing.T) {
8272
t.Parallel()
8373

84-
root := openRoot(t, "testdata")
85-
86-
cfg, err := LoadConfig(t.Context(), "todo_v1.yaml", root)
74+
cfg, err := Load(t.Context(), fileSource("testdata/todo_v1.yaml"))
8775
require.NoError(t, err)
8876

8977
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -95,9 +83,7 @@ func TestMigrate_v1_todo(t *testing.T) {
9583
func TestMigrate_v0_v1_shared_todo(t *testing.T) {
9684
t.Parallel()
9785

98-
root := openRoot(t, "testdata")
99-
100-
cfg, err := LoadConfig(t.Context(), "shared_todo_v0.yaml", root)
86+
cfg, err := Load(t.Context(), fileSource("testdata/shared_todo_v0.yaml"))
10187
require.NoError(t, err)
10288

10389
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -109,9 +95,7 @@ func TestMigrate_v0_v1_shared_todo(t *testing.T) {
10995
func TestMigrate_v1_shared_todo(t *testing.T) {
11096
t.Parallel()
11197

112-
root := openRoot(t, "testdata")
113-
114-
cfg, err := LoadConfig(t.Context(), "shared_todo_v1.yaml", root)
98+
cfg, err := Load(t.Context(), fileSource("testdata/shared_todo_v1.yaml"))
11599
require.NoError(t, err)
116100

117101
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -123,9 +107,7 @@ func TestMigrate_v1_shared_todo(t *testing.T) {
123107
func TestMigrate_v0_v1_think(t *testing.T) {
124108
t.Parallel()
125109

126-
root := openRoot(t, "testdata")
127-
128-
cfg, err := LoadConfig(t.Context(), "think_v0.yaml", root)
110+
cfg, err := Load(t.Context(), fileSource("testdata/think_v0.yaml"))
129111
require.NoError(t, err)
130112

131113
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -136,9 +118,7 @@ func TestMigrate_v0_v1_think(t *testing.T) {
136118
func TestMigrate_v1_think(t *testing.T) {
137119
t.Parallel()
138120

139-
root := openRoot(t, "testdata")
140-
141-
cfg, err := LoadConfig(t.Context(), "think_v1.yaml", root)
121+
cfg, err := Load(t.Context(), fileSource("testdata/think_v1.yaml"))
142122
require.NoError(t, err)
143123

144124
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -149,9 +129,7 @@ func TestMigrate_v1_think(t *testing.T) {
149129
func TestMigrate_v0_v1_memory(t *testing.T) {
150130
t.Parallel()
151131

152-
root := openRoot(t, "testdata")
153-
154-
cfg, err := LoadConfig(t.Context(), "memory_v0.yaml", root)
132+
cfg, err := Load(t.Context(), fileSource("testdata/memory_v0.yaml"))
155133
require.NoError(t, err)
156134

157135
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -163,9 +141,7 @@ func TestMigrate_v0_v1_memory(t *testing.T) {
163141
func TestMigrate_v1_memory(t *testing.T) {
164142
t.Parallel()
165143

166-
root := openRoot(t, "testdata")
167-
168-
cfg, err := LoadConfig(t.Context(), "memory_v1.yaml", root)
144+
cfg, err := Load(t.Context(), fileSource("testdata/memory_v1.yaml"))
169145
require.NoError(t, err)
170146

171147
assert.Len(t, cfg.Agents["root"].Toolsets, 2)
@@ -177,9 +153,7 @@ func TestMigrate_v1_memory(t *testing.T) {
177153
func TestMigrate_v1(t *testing.T) {
178154
t.Parallel()
179155

180-
root := openRoot(t, "testdata")
181-
182-
_, err := LoadConfig(t.Context(), "v1.yaml", root)
156+
_, err := Load(t.Context(), fileSource("testdata/v1.yaml"))
183157
require.NoError(t, err)
184158
}
185159

@@ -257,9 +231,7 @@ func TestCheckRequiredEnvVars(t *testing.T) {
257231
t.Run(test.yaml, func(t *testing.T) {
258232
t.Parallel()
259233

260-
root := openRoot(t, "testdata/env")
261-
262-
cfg, err := LoadConfig(t.Context(), test.yaml, root)
234+
cfg, err := Load(t.Context(), fileSource("testdata/env/"+test.yaml))
263235
require.NoError(t, err)
264236

265237
err = CheckRequiredEnvVars(t.Context(), cfg, "", &noEnvProvider{})
@@ -277,9 +249,7 @@ func TestCheckRequiredEnvVars(t *testing.T) {
277249
func TestCheckRequiredEnvVarsWithModelGateway(t *testing.T) {
278250
t.Parallel()
279251

280-
root := openRoot(t, "testdata/env")
281-
282-
cfg, err := LoadConfig(t.Context(), "all.yaml", root)
252+
cfg, err := Load(t.Context(), fileSource("testdata/env/all.yaml"))
283253
require.NoError(t, err)
284254

285255
err = CheckRequiredEnvVars(t.Context(), cfg, "gateway:8080", &noEnvProvider{})

pkg/config/examples_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/xeipuuv/gojsonschema"
1313

1414
"github.com/docker/cagent/pkg/config/latest"
15-
"github.com/docker/cagent/pkg/filesystem"
1615
"github.com/docker/cagent/pkg/modelsdev"
1716
)
1817

@@ -43,7 +42,7 @@ func TestParseExamples(t *testing.T) {
4342
t.Run(file, func(t *testing.T) {
4443
t.Parallel()
4544

46-
cfg, err := LoadConfig(t.Context(), file, filesystem.AllowAll)
45+
cfg, err := Load(t.Context(), fileSource(file))
4746

4847
require.NoError(t, err)
4948
require.Equal(t, latest.Version, cfg.Version, "Version should be %d in %s", latest.Version, file)

pkg/config/validation_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,36 @@
11
package config
22

33
import (
4+
"path/filepath"
45
"testing"
56

67
"github.com/stretchr/testify/require"
78
)
89

910
func TestLoadConfig_InvalidPath(t *testing.T) {
10-
tmp := openRoot(t, t.TempDir())
11+
tmp := t.TempDir()
12+
tmpRoot := openRoot(t, tmp)
1113

1214
validConfig := `version: 1
1315
agents:
1416
root:
1517
model: "openai/gpt-4"
1618
`
1719

18-
err := tmp.WriteFile("valid.yaml", []byte(validConfig), 0o644)
20+
err := tmpRoot.WriteFile("valid.yaml", []byte(validConfig), 0o644)
1921
require.NoError(t, err)
2022

21-
cfg, err := LoadConfig(t.Context(), "valid.yaml", tmp)
23+
cfg, err := Load(t.Context(), fileSource(filepath.Join(tmp, "valid.yaml")))
2224
require.NoError(t, err)
2325
require.NotNil(t, cfg)
2426

25-
_, err = LoadConfig(t.Context(), "../../../etc/passwd", tmp)
27+
_, err = Load(t.Context(), fileSource(filepath.Join(tmp, "../../../etc/passwd"))) //nolint: gocritic // testing invalid path
2628
require.Error(t, err)
2729
}
2830

2931
func TestValidationErrors(t *testing.T) {
32+
t.Parallel()
33+
3034
tests := []struct {
3135
name string
3236
path string
@@ -49,9 +53,7 @@ func TestValidationErrors(t *testing.T) {
4953
t.Run(tt.name, func(t *testing.T) {
5054
t.Parallel()
5155

52-
root := openRoot(t, "testdata")
53-
54-
_, err := LoadConfig(t.Context(), tt.path, root)
56+
_, err := Load(t.Context(), fileSource(filepath.Join("testdata", tt.path)))
5557
require.Error(t, err)
5658
})
5759
}

pkg/oci/package.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func PackageFileAsOCIToStore(ctx context.Context, agentFilename, artifactRef str
2929

3030
agentFilename = filepath.Clean(agentFilename)
3131
source := teamloader.NewFileSource(agentFilename)
32-
cfg, err := config.LoadConfigFrom(ctx, source)
32+
cfg, err := config.Load(ctx, source)
3333
if err != nil {
3434
return "", fmt.Errorf("loading config: %w", err)
3535
}

pkg/server/server.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,12 @@ func (s *Server) getAgentConfig(c echo.Context) error {
195195
agentID := c.Param("id")
196196
p := addYamlExt(agentID)
197197

198-
cfg, err := config.LoadConfig(c.Request().Context(), p, s.rootFS)
198+
data, err := s.rootFS.ReadFile(p)
199+
if err != nil {
200+
return fmt.Errorf("reading config file %s: %w", p, err)
201+
}
202+
203+
cfg, err := config.Load(c.Request().Context(), teamloader.NewBytesSource(data))
199204
if err != nil {
200205
slog.Error("Failed to load config", "error", err)
201206
return echo.NewHTTPError(http.StatusNotFound, "agent not found")

pkg/teamloader/teamloader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func LoadFrom(ctx context.Context, source AgentSource, runtimeConfig *config.Run
130130
}
131131

132132
// Load the agent's configuration
133-
cfg, err := config.LoadConfigFrom(ctx, source)
133+
cfg, err := config.Load(ctx, source)
134134
if err != nil {
135135
return nil, err
136136
}

0 commit comments

Comments
 (0)