Fix: improve Docker startup and compose validation #78
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| deployments_path: ./deployment | ||
|
||
| docker_socket: unix:///var/run/docker.sock | ||
| api: | ||
| host: 0.0.0.0 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,23 +47,25 @@ require ( | |
| github.com/go-playground/universal-translator v0.18.1 // indirect | ||
| github.com/go-playground/validator/v10 v10.14.0 // indirect | ||
| github.com/goccy/go-json v0.10.5 // indirect | ||
| github.com/google/go-cmp v0.7.0 // indirect | ||
| github.com/google/go-querystring v1.1.0 // indirect | ||
| github.com/hashicorp/go-cleanhttp v0.5.2 // indirect | ||
| github.com/hashicorp/go-retryablehttp v0.7.7 // indirect | ||
| github.com/json-iterator/go v1.1.12 // indirect | ||
| github.com/klauspost/cpuid/v2 v2.2.4 // indirect | ||
| github.com/kr/pretty v0.3.1 // indirect | ||
| github.com/leodido/go-urn v1.2.4 // indirect | ||
| github.com/mattn/go-isatty v0.0.20 // indirect | ||
| github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect | ||
| github.com/modern-go/reflect2 v1.0.2 // indirect | ||
| github.com/pelletier/go-toml/v2 v2.0.8 // indirect | ||
| github.com/rogpeppe/go-internal v1.13.1 // indirect | ||
| github.com/twitchyliquid64/golang-asm v0.15.1 // indirect | ||
| github.com/ugorji/go/codec v1.2.11 // indirect | ||
| golang.org/x/arch v0.3.0 // indirect | ||
| golang.org/x/net v0.34.0 // indirect | ||
| golang.org/x/sys v0.33.0 // indirect | ||
| golang.org/x/text v0.21.0 // indirect | ||
| golang.org/x/time v0.11.0 // indirect | ||
| google.golang.org/protobuf v1.34.2 // indirect | ||
| ) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,11 +70,9 @@ github.com/goccy/go-json v0.10.5 h1:Fq85nIqj+gXn/S5ahsiTlK3TmC85qgirsdTP/+DeaC4= | |
| github.com/goccy/go-json v0.10.5/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= | ||
| github.com/golang-jwt/jwt/v5 v5.3.0 h1:pv4AsKCKKZuqlgs5sUmn4x8UlGa0kEVt/puTpKx9vvo= | ||
| github.com/golang-jwt/jwt/v5 v5.3.0/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= | ||
| github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= | ||
| github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= | ||
| github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= | ||
| github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= | ||
| github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= | ||
| github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= | ||
|
|
@@ -93,8 +91,8 @@ github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHm | |
| github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= | ||
| github.com/klauspost/cpuid/v2 v2.2.4 h1:acbojRNwl3o09bUq+yDCtZFc1aiwaAAxtcn8YkZXnvk= | ||
| github.com/klauspost/cpuid/v2 v2.2.4/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY= | ||
| github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= | ||
| github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= | ||
| github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= | ||
| github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= | ||
| github.com/leodido/go-urn v1.2.4 h1:XlAE/cm/ms7TE/VMVoduSpNBoyc2dOxHs5MZSwAN63Q= | ||
|
|
@@ -114,12 +112,14 @@ github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9G | |
| github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= | ||
| github.com/pelletier/go-toml/v2 v2.0.8 h1:0ctb6s9mE31h0/lhu+J6OPmVeDxJn+kYnJc2jZR9tGQ= | ||
| github.com/pelletier/go-toml/v2 v2.0.8/go.mod h1:vuYfssBdrU2XDZ9bYydBu6t+6a6PYNcZljzZR9VXg+4= | ||
| github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= | ||
| github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= | ||
| github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= | ||
| github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= | ||
| github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= | ||
| github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= | ||
| github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= | ||
| github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= | ||
| github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= | ||
| github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= | ||
| github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= | ||
|
|
@@ -147,16 +147,15 @@ golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= | |
| golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= | ||
| golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= | ||
| golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= | ||
| golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= | ||
| golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= | ||
| golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= | ||
| golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= | ||
| golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0= | ||
| golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg= | ||
| golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= | ||
| google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= | ||
| google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= | ||
| gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= | ||
| gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= | ||
| gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os/exec" | ||
| "strings" | ||
| "sync" | ||
| "testing" | ||
|
|
||
| "github.com/flatrun/agent/pkg/config" | ||
| ) | ||
|
|
||
| var ( | ||
| dockerCheckOnce sync.Once | ||
| dockerCheckErr error | ||
| ) | ||
|
|
||
| // dockerAvailable ensures docker is available and the proxy network exists for CLI validation tests. | ||
| // Runs the check once; subsequent calls use the cached result. Fails when Docker is unavailable. | ||
| func dockerAvailable(t *testing.T) { | ||
| t.Helper() | ||
| dockerCheckOnce.Do(func() { | ||
| dockerCheckErr = checkDocker() | ||
| }) | ||
| if dockerCheckErr != nil { | ||
| t.Fatal(dockerCheckErr) | ||
| } | ||
| } | ||
|
|
||
| func checkDocker() error { | ||
| if testing.Short() { | ||
| return fmt.Errorf("compose CLI validation tests require Docker; do not use -short") | ||
| } | ||
| if _, err := exec.LookPath("docker"); err != nil { | ||
| return fmt.Errorf("docker not in PATH: %w", err) | ||
| } | ||
| if err := exec.Command("docker", "info").Run(); err != nil { | ||
| return fmt.Errorf("docker daemon not reachable: %w", err) | ||
| } | ||
| _ = exec.Command("docker", "network", "create", "proxy").Run() | ||
| if err := exec.Command("docker", "network", "inspect", "proxy").Run(); err != nil { | ||
| return fmt.Errorf("proxy network not found (run: docker network create proxy): %w", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func TestValidateComposeWithCLI_ValidCompose_Integration(t *testing.T) { | ||
| dockerAvailable(t) | ||
| validCompose := `name: test | ||
| services: | ||
| app: | ||
| image: nginx:alpine | ||
| ` | ||
| err := validateComposeWithCLI(validCompose) | ||
| if err != nil { | ||
| t.Errorf("validateComposeWithCLI(valid compose) = %v, want nil", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeWithCLI_InvalidCompose_Integration(t *testing.T) { | ||
| dockerAvailable(t) | ||
| // Invalid: service references network not defined in top-level networks | ||
| invalidCompose := `name: test | ||
| services: | ||
| app: | ||
| image: nginx | ||
| networks: | ||
| - undefined_network_xyz | ||
| ` | ||
| err := validateComposeWithCLI(invalidCompose) | ||
| if err == nil { | ||
| t.Error("validateComposeWithCLI(invalid compose) = nil, want error") | ||
| } | ||
| if err != nil && !strings.Contains(err.Error(), "invalid compose") { | ||
| t.Errorf("validateComposeWithCLI error should mention 'invalid compose', got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeWithCLI_InvalidYAML_Integration(t *testing.T) { | ||
| dockerAvailable(t) | ||
| // Invalid YAML - docker compose config will reject it | ||
| invalidCompose := `name: test | ||
| services: | ||
| app: | ||
| image: [broken yaml | ||
| ` | ||
| err := validateComposeWithCLI(invalidCompose) | ||
| if err == nil { | ||
| t.Error("validateComposeWithCLI(invalid YAML) = nil, want error") | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeContent_ValidCompose_Integration(t *testing.T) { | ||
| dockerAvailable(t) | ||
| cfg := &config.Config{ | ||
| Infrastructure: config.InfrastructureConfig{ | ||
| DefaultProxyNetwork: "proxy", | ||
| }, | ||
| } | ||
| s := &Server{config: cfg} | ||
|
|
||
| validCompose := `name: test | ||
| services: | ||
| app: | ||
| image: nginx:alpine | ||
| networks: | ||
| - proxy | ||
| networks: | ||
| proxy: | ||
| external: true | ||
| ` | ||
| err := s.validateComposeContent(validCompose, "test") | ||
| if err != nil { | ||
| t.Errorf("validateComposeContent(valid) = %v, want nil", err) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/flatrun/agent/pkg/config" | ||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
|
|
@@ -147,3 +149,37 @@ services: | |
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeContent_RejectsInvalidYAML(t *testing.T) { | ||
| cfg := &config.Config{ | ||
| Infrastructure: config.InfrastructureConfig{ | ||
| DefaultProxyNetwork: "proxy", | ||
| }, | ||
| } | ||
| s := &Server{config: cfg} | ||
|
|
||
| err := s.validateComposeContent("not valid: [ yaml", "test") | ||
| if err == nil { | ||
| t.Error("validateComposeContent(invalid YAML) = nil, want error") | ||
| } | ||
| if err != nil && !strings.Contains(err.Error(), "invalid YAML") { | ||
| t.Errorf("error should mention YAML, got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestValidateComposeContent_RejectsEmptyServices(t *testing.T) { | ||
| cfg := &config.Config{ | ||
| Infrastructure: config.InfrastructureConfig{ | ||
| DefaultProxyNetwork: "proxy", | ||
| }, | ||
| } | ||
| s := &Server{config: cfg} | ||
|
|
||
| err := s.validateComposeContent("name: test\nservices: {}", "test") | ||
| if err == nil { | ||
| t.Error("validateComposeContent(no services) = nil, want error") | ||
| } | ||
| if err != nil && !strings.Contains(err.Error(), "at least one service") { | ||
| t.Errorf("error should mention services, got: %v", err) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1255,6 +1255,13 @@ func (s *Server) updateDeployment(c *gin.Context) { | |||||||||||||||||
| return | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if err := s.validateComposeContent(req.ComposeContent, name); err != nil { | ||||||||||||||||||
| c.JSON(http.StatusBadRequest, gin.H{ | ||||||||||||||||||
| "error": err.Error(), | ||||||||||||||||||
| }) | ||||||||||||||||||
| return | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if err := s.manager.UpdateDeployment(name, req.ComposeContent); err != nil { | ||||||||||||||||||
| c.JSON(http.StatusInternalServerError, gin.H{ | ||||||||||||||||||
| "error": err.Error(), | ||||||||||||||||||
|
|
@@ -2777,7 +2784,7 @@ type composeNetwork struct { | |||||||||||||||||
| Name string `yaml:"name"` | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func (s *Server) validateComposeContent(content, _ string) error { | ||||||||||||||||||
| var compose composeFile | ||||||||||||||||||
| if err := yaml.Unmarshal([]byte(content), &compose); err != nil { | ||||||||||||||||||
| return fmt.Errorf("invalid YAML syntax: %w", err) | ||||||||||||||||||
|
|
@@ -2815,6 +2822,41 @@ func (s *Server) validateComposeContent(content, deploymentName string) error { | |||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if err := validateComposeWithCLI(content); err != nil { | ||||||||||||||||||
|
||||||||||||||||||
| return err | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return nil | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| func validateComposeWithCLI(content string) error { | ||||||||||||||||||
|
||||||||||||||||||
| func validateComposeWithCLI(content string) error { | |
| tmp, err := os.CreateTemp("", "compose-*.yml") | |
| if err != nil { | |
| log.Printf("Warning: skipping CLI validation, failed to create temp file: %v", err) | |
| return nil | |
| } | |
| defer os.Remove(tmp.Name()) | |
| defer tmp.Close() |
Outdated
There was a problem hiding this comment.
The current implementation writes to the file, syncs, closes it, and then re-opens it via the docker compose CLI. While functional, it's cleaner to handle errors on WriteString and Sync more strictly if you want to ensure validation occurs. More importantly, since the file is already closed on line 2853, the deferred tmp.Close() on line 2841 will return an error (which is ignored), but it's redundant.
Suggested change
| if _, err := tmp.WriteString(content); err != nil { | |
| if _, err := tmp.WriteString(content); err != nil { | |
| return fmt.Errorf("failed to write temp file for validation: %w", err) | |
| } | |
| if err := tmp.Close(); err != nil { | |
| return fmt.Errorf("failed to close temp file: %w", err) | |
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
exec.Command("docker", "info")relies on thedockerCLI being installed in the PATH. Since you've added themoby/mobyclient dependency, it is more efficient and reliable to check connectivity using the library'sPing()orInfo()methods instead of spawning a shell process.