From b445ff2d87528c82e9277bd48650340a78033672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Fri, 22 May 2026 10:38:04 +0000 Subject: [PATCH 1/2] feat(compose): add 'compose fix' command for auto-fixing compose files --- src/cmd/cli/command/compose.go | 79 +++++ src/pkg/cli/compose/fix.go | 291 ++++++++++++++++++ src/pkg/cli/compose/fix_test.go | 198 ++++++++++++ src/testdata/compose-fix/compose.yaml | 12 + src/testdata/compose-fix/compose.yaml.fixup | 36 +++ src/testdata/compose-fix/compose.yaml.golden | 23 ++ .../compose-fix/compose.yaml.warnings | 3 + 7 files changed, 642 insertions(+) create mode 100644 src/pkg/cli/compose/fix.go create mode 100644 src/pkg/cli/compose/fix_test.go create mode 100644 src/testdata/compose-fix/compose.yaml create mode 100644 src/testdata/compose-fix/compose.yaml.fixup create mode 100644 src/testdata/compose-fix/compose.yaml.golden create mode 100644 src/testdata/compose-fix/compose.yaml.warnings diff --git a/src/cmd/cli/command/compose.go b/src/cmd/cli/command/compose.go index 53d7167de..4a1ac2d26 100644 --- a/src/cmd/cli/command/compose.go +++ b/src/cmd/cli/command/compose.go @@ -553,6 +553,84 @@ func makeComposeConfigCmd() *cobra.Command { } } +func makeComposeFixCmd() *cobra.Command { + var dryRun bool + cmd := &cobra.Command{ + Use: "fix", + Args: cobra.NoArgs, + Short: "Reads a Compose file and applies safe mechanical fixes", + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + + sessionx, err := newCommandSessionWithOpts(cmd, commandSessionOpts{ + CheckAccountInfo: false, + }) + if err != nil { + term.Warn("unable to load stack:", err, "- using local compose configuration") + sessionx = &session.Session{ + Loader: newLoaderForCommand(cmd), + } + } + + project, loadErr := sessionx.Loader.LoadProject(ctx) + if loadErr != nil { + return handleInvalidComposeFileErr(ctx, loadErr) + } + + fixes := compose.FixProject(project) + printFixResults(fixes) + if dryRun { + return nil + } + + data, err := compose.MarshalYAML(project) + if err != nil { + return err + } + term.Println() + _, err = term.Print(string(data)) + return err + }, + } + cmd.Flags().BoolVar(&dryRun, "dry-run", false, "show fixes without outputting YAML") + return cmd +} + +func printFixResults(fixes []compose.FixResult) { + term.Println("Fixes applied:") + for _, fix := range fixes { + term.Printf(" service %q: %s\n", fix.Service, describeFixResult(fix)) + } + term.Println() + term.Printf("%d fixes applied.\n", len(fixes)) +} + +func describeFixResult(fix compose.FixResult) string { + switch fix.Action { + case "removed": + return fmt.Sprintf("removed unsupported directive: %s", fix.Field) + case "changed": + if fix.Field == "domainname" { + return fmt.Sprintf("moved hostname %q to domainname", fix.After) + } + if fix.Before != "" { + return fmt.Sprintf("changed %s: %s to %s (%s)", fix.Field, fix.Before, fix.After, fix.Reason) + } + return fmt.Sprintf("changed %s: %s (%s)", fix.Field, fix.After, fix.Reason) + default: + if fix.Field == "mode" { + return fmt.Sprintf("added mode: %s to %s", fix.After, fix.Reason) + } + if fix.Field == "healthcheck" { + return fmt.Sprintf("added healthcheck for %s", fix.Reason) + } + if fix.Reason != "" { + return fmt.Sprintf("added %s: %s (%s)", fix.Field, fix.After, fix.Reason) + } + return fmt.Sprintf("added %s: %s", fix.Field, fix.After) + } +} + func makeComposePsCmd() *cobra.Command { getServicesCmd := &cobra.Command{ Use: "ps", @@ -763,6 +841,7 @@ services: composeCmd.PersistentFlags().StringVar(&byoc.DefangPulumiBackend, "pulumi-backend", "", `specify an alternate Pulumi backend URL or "pulumi-cloud"`) composeCmd.AddCommand(makeComposeUpCmd()) composeCmd.AddCommand(makeComposeConfigCmd()) + composeCmd.AddCommand(makeComposeFixCmd()) composeCmd.AddCommand(makeComposeDownCmd()) composeCmd.AddCommand(makeComposePsCmd()) composeCmd.AddCommand(makeLogsCmd()) diff --git a/src/pkg/cli/compose/fix.go b/src/pkg/cli/compose/fix.go new file mode 100644 index 000000000..37d76cc65 --- /dev/null +++ b/src/pkg/cli/compose/fix.go @@ -0,0 +1,291 @@ +package compose + +import ( + "fmt" + "sort" + "strings" + + composeTypes "github.com/compose-spec/compose-go/v2/types" +) + +const defaultRestartPolicy = "unless-stopped" + +type FixResult struct { + Service string + Field string + Action string // "added", "removed", "changed" + Before string + After string + Reason string +} + +func FixProject(project *Project) []FixResult { + if project == nil { + return nil + } + + var results []FixResult + for _, name := range sortedServiceNames(project) { + service := project.Services[name] + results = append(results, fixService(&service)...) + project.Services[name] = service + } + return results +} + +func sortedServiceNames(project *Project) []string { + names := make([]string, 0, len(project.Services)) + for name := range project.Services { + names = append(names, name) + } + sort.Strings(names) + return names +} + +func fixService(service *composeTypes.ServiceConfig) []FixResult { + var results []FixResult + repo := GetImageRepo(service.Image) + isManagedStoreImage := IsPostgresRepo(repo) || IsRedisRepo(repo) || IsMongoRepo(repo) + + results = append(results, fixPorts(service, isManagedStoreImage)...) + results = append(results, fixManagedServiceExtensions(service, repo)...) + results = append(results, fixResources(service)...) + results = append(results, fixRestart(service)...) + results = append(results, fixHostname(service)...) + results = append(results, fixUnsupportedDirectives(service)...) + results = append(results, fixIngressHealthcheck(service)...) + + return results +} + +func fixPorts(service *composeTypes.ServiceConfig, isManagedStoreImage bool) []FixResult { + var results []FixResult + for i := range service.Ports { + port := &service.Ports[i] + if port.Mode != "" { + continue + } + + mode := Mode_INGRESS + reason := "" + if port.Protocol == Protocol_UDP { + mode = Mode_HOST + reason = "UDP port" + } else if isManagedStoreImage { + mode = Mode_HOST + reason = "database image" + } + port.Mode = mode + results = append(results, FixResult{ + Service: service.Name, + Field: "mode", + Action: "added", + After: mode, + Reason: portReason(port.Target, reason), + }) + } + return results +} + +func portReason(target uint32, reason string) string { + if reason == "" { + return fmt.Sprintf("port %d", target) + } + return fmt.Sprintf("port %d (%s)", target, reason) +} + +func fixManagedServiceExtensions(service *composeTypes.ServiceConfig, repo string) []FixResult { + switch { + case IsPostgresRepo(repo): + return addManagedServiceExtension(service, "x-defang-postgres", "postgres image detected") + case IsRedisRepo(repo): + return addManagedServiceExtension(service, "x-defang-redis", "redis image detected") + case IsMongoRepo(repo): + return addManagedServiceExtension(service, "x-defang-mongodb", "mongo image detected") + default: + return nil + } +} + +func addManagedServiceExtension(service *composeTypes.ServiceConfig, key, reason string) []FixResult { + if service.Extensions == nil { + service.Extensions = composeTypes.Extensions{} + } + if _, ok := service.Extensions[key]; ok { + return nil + } + service.Extensions[key] = true + return []FixResult{{ + Service: service.Name, + Field: key, + Action: "added", + After: "true", + Reason: reason, + }} +} + +func fixResources(service *composeTypes.ServiceConfig) []FixResult { + var results []FixResult + if service.Deploy == nil { + service.Deploy = &composeTypes.DeployConfig{} + } + if service.Deploy.Resources.Limits != nil && service.Deploy.Resources.Reservations == nil { + limits := *service.Deploy.Resources.Limits + service.Deploy.Resources.Reservations = &limits + results = append(results, FixResult{ + Service: service.Name, + Field: "deploy.resources.reservations", + Action: "added", + After: "deploy.resources.limits", + Reason: "limits used as reservations", + }) + } + if service.Deploy.Resources.Reservations == nil { + service.Deploy.Resources.Reservations = &composeTypes.Resource{} + } + if service.Deploy.Resources.Reservations.MemoryBytes == 0 { + memory := composeTypes.UnitBytes(512 * MiB) + after := "512M" + if service.Extensions["x-defang-static-files"] != nil { + memory = composeTypes.UnitBytes(256 * MiB) + after = "256M" + } + service.Deploy.Resources.Reservations.MemoryBytes = memory + results = append(results, FixResult{ + Service: service.Name, + Field: "deploy.resources.reservations.memory", + Action: "added", + After: after, + Reason: "missing memory reservation", + }) + } + return results +} + +func fixRestart(service *composeTypes.ServiceConfig) []FixResult { + restart := restartFromDeployPolicy(service) + if restart == "" && isSupportedRestart(service.Restart) { + return nil + } + + before := service.Restart + if restart == "" { + restart = defaultRestartPolicy + } + service.Restart = restart + + reason := "unsupported restart policy" + if service.Deploy != nil && service.Deploy.RestartPolicy != nil { + reason = "deploy.restart_policy is unsupported" + } else if before == "" { + reason = "missing restart policy" + } + + if service.Deploy != nil && service.Deploy.RestartPolicy != nil { + service.Deploy.RestartPolicy = nil + } + + action := "changed" + if before == "" { + action = "added" + } + return []FixResult{{ + Service: service.Name, + Field: "restart", + Action: action, + Before: before, + After: restart, + Reason: reason, + }} +} + +func restartFromDeployPolicy(service *composeTypes.ServiceConfig) string { + if service.Deploy == nil || service.Deploy.RestartPolicy == nil { + return "" + } + switch service.Deploy.RestartPolicy.Condition { + case "", "any": + return "always" + default: + return defaultRestartPolicy + } +} + +func isSupportedRestart(restart string) bool { + return restart == "always" || restart == defaultRestartPolicy +} + +func fixHostname(service *composeTypes.ServiceConfig) []FixResult { + if service.Hostname == "" { + return nil + } + before := service.Hostname + service.DomainName = service.Hostname + service.Hostname = "" + return []FixResult{{ + Service: service.Name, + Field: "domainname", + Action: "changed", + Before: before, + After: service.DomainName, + Reason: "hostname is unsupported", + }} +} + +func fixUnsupportedDirectives(service *composeTypes.ServiceConfig) []FixResult { + var results []FixResult + if len(service.DNS) != 0 { + service.DNS = nil + results = append(results, removedDirective(service.Name, "dns")) + } + if len(service.DNSSearch) != 0 { + service.DNSSearch = nil + results = append(results, removedDirective(service.Name, "dns_search")) + } + if len(service.Devices) != 0 { + service.Devices = nil + results = append(results, removedDirective(service.Name, "devices")) + } + if len(service.DeviceCgroupRules) != 0 { + service.DeviceCgroupRules = nil + results = append(results, removedDirective(service.Name, "device_cgroup_rules")) + } + if len(service.GroupAdd) != 0 { + service.GroupAdd = nil + results = append(results, removedDirective(service.Name, "group_add")) + } + return results +} + +func removedDirective(service, field string) FixResult { + return FixResult{ + Service: service, + Field: field, + Action: "removed", + Before: "present", + Reason: "unsupported directive", + } +} + +func fixIngressHealthcheck(service *composeTypes.ServiceConfig) []FixResult { + if service.HealthCheck != nil && !service.HealthCheck.Disable { + return nil + } + for _, port := range service.Ports { + if port.Mode != Mode_INGRESS { + continue + } + url := fmt.Sprintf("http://localhost:%d/", port.Target) + service.HealthCheck = &composeTypes.HealthCheckConfig{ + Test: composeTypes.HealthCheckTest{"CMD", "curl", "-f", url}, + } + return []FixResult{{ + Service: service.Name, + Field: "healthcheck", + Action: "added", + After: strings.Join(service.HealthCheck.Test, " "), + Reason: fmt.Sprintf("ingress port %d", port.Target), + }} + } + return nil +} diff --git a/src/pkg/cli/compose/fix_test.go b/src/pkg/cli/compose/fix_test.go new file mode 100644 index 000000000..1da95151f --- /dev/null +++ b/src/pkg/cli/compose/fix_test.go @@ -0,0 +1,198 @@ +package compose + +import ( + "reflect" + "testing" + + "github.com/DefangLabs/defang/src/pkg/modes" + composeTypes "github.com/compose-spec/compose-go/v2/types" +) + +func TestFixProject(t *testing.T) { + tests := []struct { + name string + project *Project + want []FixResult + check func(*testing.T, *Project) + }{ + { + name: "web service defaults", + project: &Project{Services: Services{ + "web": { + Name: "web", + Image: "nginx", + Ports: []composeTypes.ServicePortConfig{{Target: 8080}}, + }, + }}, + want: []FixResult{ + {Service: "web", Field: "mode", Action: "added", After: Mode_INGRESS, Reason: "port 8080"}, + {Service: "web", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, + {Service: "web", Field: "restart", Action: "added", After: defaultRestartPolicy, Reason: "missing restart policy"}, + {Service: "web", Field: "healthcheck", Action: "added", After: "CMD curl -f http://localhost:8080/", Reason: "ingress port 8080"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["web"] + if service.Ports[0].Mode != Mode_INGRESS { + t.Fatalf("port mode = %q, want %q", service.Ports[0].Mode, Mode_INGRESS) + } + if service.HealthCheck == nil { + t.Fatal("healthcheck was not added") + } + if service.Deploy.Resources.Reservations.MemoryBytes != 512*MiB { + t.Fatalf("memory = %d, want %d", service.Deploy.Resources.Reservations.MemoryBytes, 512*MiB) + } + }, + }, + { + name: "managed postgres defaults to host", + project: &Project{Services: Services{ + "db": { + Name: "db", + Image: "postgres:16", + Ports: []composeTypes.ServicePortConfig{{Target: 5432}}, + }, + }}, + want: []FixResult{ + {Service: "db", Field: "mode", Action: "added", After: Mode_HOST, Reason: "port 5432 (database image)"}, + {Service: "db", Field: "x-defang-postgres", Action: "added", After: "true", Reason: "postgres image detected"}, + {Service: "db", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, + {Service: "db", Field: "restart", Action: "added", After: defaultRestartPolicy, Reason: "missing restart policy"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["db"] + if service.Ports[0].Mode != Mode_HOST { + t.Fatalf("port mode = %q, want %q", service.Ports[0].Mode, Mode_HOST) + } + if service.Extensions["x-defang-postgres"] != true { + t.Fatal("x-defang-postgres was not added") + } + }, + }, + { + name: "limits copied to reservations", + project: &Project{Services: Services{ + "api": { + Name: "api", + Image: "api", + Restart: defaultRestartPolicy, + Deploy: &composeTypes.DeployConfig{Resources: composeTypes.Resources{ + Limits: &composeTypes.Resource{MemoryBytes: 1024 * MiB}, + }}, + }, + }}, + want: []FixResult{ + {Service: "api", Field: "deploy.resources.reservations", Action: "added", After: "deploy.resources.limits", Reason: "limits used as reservations"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["api"] + if service.Deploy.Resources.Reservations == nil { + t.Fatal("reservations were not added") + } + if service.Deploy.Resources.Reservations.MemoryBytes != 1024*MiB { + t.Fatalf("memory = %d, want %d", service.Deploy.Resources.Reservations.MemoryBytes, 1024*MiB) + } + }, + }, + { + name: "unsupported directives removed", + project: &Project{Services: Services{ + "worker": { + Name: "worker", + Image: "worker", + Restart: defaultRestartPolicy, + DNS: composeTypes.StringList{"1.1.1.1"}, + DNSSearch: composeTypes.StringList{"example.com"}, + Devices: []composeTypes.DeviceMapping{{Source: "/dev/null", Target: "/dev/null"}}, + DeviceCgroupRules: []string{"c 1:3 mr"}, + GroupAdd: []string{"audio"}, + }, + }}, + want: []FixResult{ + {Service: "worker", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, + {Service: "worker", Field: "dns", Action: "removed", Before: "present", Reason: "unsupported directive"}, + {Service: "worker", Field: "dns_search", Action: "removed", Before: "present", Reason: "unsupported directive"}, + {Service: "worker", Field: "devices", Action: "removed", Before: "present", Reason: "unsupported directive"}, + {Service: "worker", Field: "device_cgroup_rules", Action: "removed", Before: "present", Reason: "unsupported directive"}, + {Service: "worker", Field: "group_add", Action: "removed", Before: "present", Reason: "unsupported directive"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["worker"] + if len(service.DNS) != 0 || len(service.DNSSearch) != 0 || len(service.Devices) != 0 || len(service.DeviceCgroupRules) != 0 || len(service.GroupAdd) != 0 { + t.Fatal("unsupported directives were not removed") + } + }, + }, + { + name: "hostname and deploy restart policy", + project: &Project{Services: Services{ + "api": { + Name: "api", + Image: "api", + Hostname: "api.example.com", + Deploy: &composeTypes.DeployConfig{ + RestartPolicy: &composeTypes.RestartPolicy{Condition: "any"}, + }, + }, + }}, + want: []FixResult{ + {Service: "api", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, + {Service: "api", Field: "restart", Action: "added", After: "always", Reason: "deploy.restart_policy is unsupported"}, + {Service: "api", Field: "domainname", Action: "changed", Before: "api.example.com", After: "api.example.com", Reason: "hostname is unsupported"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["api"] + if service.Hostname != "" || service.DomainName != "api.example.com" { + t.Fatalf("hostname/domainname = %q/%q", service.Hostname, service.DomainName) + } + if service.Deploy.RestartPolicy != nil { + t.Fatal("deploy.restart_policy was not removed") + } + }, + }, + { + name: "static files memory default", + project: &Project{Services: Services{ + "cdn": { + Name: "cdn", + Image: "nginx", + Restart: defaultRestartPolicy, + Extensions: composeTypes.Extensions{"x-defang-static-files": "./public"}, + }, + }}, + want: []FixResult{ + {Service: "cdn", Field: "deploy.resources.reservations.memory", Action: "added", After: "256M", Reason: "missing memory reservation"}, + }, + check: func(t *testing.T, project *Project) { + service := project.Services["cdn"] + if service.Deploy.Resources.Reservations.MemoryBytes != 256*MiB { + t.Fatalf("memory = %d, want %d", service.Deploy.Resources.Reservations.MemoryBytes, 256*MiB) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FixProject(tt.project) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("FixProject() = %#v, want %#v", got, tt.want) + } + tt.check(t, tt.project) + }) + } +} + +func TestFixProjectOutputValidates(t *testing.T) { + loader := NewLoader(WithPath("../../../testdata/compose-fix/compose.yaml")) + project, err := loader.LoadProject(t.Context()) + if err != nil { + t.Fatalf("LoadProject() failed: %v", err) + } + + if fixes := FixProject(project); len(fixes) == 0 { + t.Fatal("expected fixes") + } + if err := ValidateProject(project, modes.ModeUnspecified); err != nil { + t.Fatalf("ValidateProject() after FixProject() failed: %v", err) + } +} diff --git a/src/testdata/compose-fix/compose.yaml b/src/testdata/compose-fix/compose.yaml new file mode 100644 index 000000000..e24f8632d --- /dev/null +++ b/src/testdata/compose-fix/compose.yaml @@ -0,0 +1,12 @@ +name: compose-fix +services: + web: + image: nginx + ports: + - target: 8080 + dns: + - 1.1.1.1 + db: + image: postgres:16 + ports: + - target: 5432 diff --git a/src/testdata/compose-fix/compose.yaml.fixup b/src/testdata/compose-fix/compose.yaml.fixup new file mode 100644 index 000000000..16f9a6fba --- /dev/null +++ b/src/testdata/compose-fix/compose.yaml.fixup @@ -0,0 +1,36 @@ +{ + "db": { + "command": null, + "entrypoint": null, + "image": "postgres:16", + "networks": { + "default": null + }, + "ports": [ + { + "mode": "host", + "target": 5432, + "protocol": "tcp" + } + ] + }, + "web": { + "command": null, + "dns": [ + "1.1.1.1" + ], + "entrypoint": null, + "image": "nginx", + "networks": { + "default": null + }, + "ports": [ + { + "mode": "ingress", + "target": 8080, + "protocol": "tcp", + "app_protocol": "http" + } + ] + } +} \ No newline at end of file diff --git a/src/testdata/compose-fix/compose.yaml.golden b/src/testdata/compose-fix/compose.yaml.golden new file mode 100644 index 000000000..ea34d6304 --- /dev/null +++ b/src/testdata/compose-fix/compose.yaml.golden @@ -0,0 +1,23 @@ +name: compose-fix +services: + db: + image: postgres:16 + networks: + default: null + ports: + - mode: ingress + target: 5432 + protocol: tcp + web: + dns: + - 1.1.1.1 + image: nginx + networks: + default: null + ports: + - mode: ingress + target: 8080 + protocol: tcp +networks: + default: + name: compose-fix_default diff --git a/src/testdata/compose-fix/compose.yaml.warnings b/src/testdata/compose-fix/compose.yaml.warnings new file mode 100644 index 000000000..d31dc75d6 --- /dev/null +++ b/src/testdata/compose-fix/compose.yaml.warnings @@ -0,0 +1,3 @@ + ! service "db": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "db": stateful service will lose data on restart; use a managed service instead +Error: service "web": unsupported compose directive: dns From 9e5611e8aa14cf7f04dfbebe8cc450595f8cf6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Titsworth-Morin?= Date: Fri, 22 May 2026 11:19:23 +0000 Subject: [PATCH 2/2] refactor(compose): remove aggressive auto-fixes from compose fix Remove fixes that guess at values users should set themselves: - Memory reservation defaults (512M/256M) - Healthcheck generation (curl-based) - Managed service extension injection (x-defang-postgres etc.) - Hostname-to-domainname conversion Keep only safe, deterministic fixes: - Port mode assignment (ingress/host based on protocol/image) - Limits-to-reservations copy - Restart policy normalization - Unsupported directive removal (dns, devices, etc.) Also makes compose fix fully offline (no session/auth needed), handles zero-fix case gracefully, and rewrites tests to match. Co-Authored-By: Claude Opus 4.6 --- src/cmd/cli/command/compose.go | 39 ++++------- src/pkg/cli/compose/fix.go | 119 +++----------------------------- src/pkg/cli/compose/fix_test.go | 69 +++++++++--------- 3 files changed, 58 insertions(+), 169 deletions(-) diff --git a/src/cmd/cli/command/compose.go b/src/cmd/cli/command/compose.go index 4a1ac2d26..e9d5e5360 100644 --- a/src/cmd/cli/command/compose.go +++ b/src/cmd/cli/command/compose.go @@ -558,28 +558,18 @@ func makeComposeFixCmd() *cobra.Command { cmd := &cobra.Command{ Use: "fix", Args: cobra.NoArgs, - Short: "Reads a Compose file and applies safe mechanical fixes", + Short: "Apply safe mechanical fixes to a Compose file", RunE: func(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - - sessionx, err := newCommandSessionWithOpts(cmd, commandSessionOpts{ - CheckAccountInfo: false, - }) - if err != nil { - term.Warn("unable to load stack:", err, "- using local compose configuration") - sessionx = &session.Session{ - Loader: newLoaderForCommand(cmd), - } - } + loader := newLoaderForCommand(cmd) - project, loadErr := sessionx.Loader.LoadProject(ctx) + project, loadErr := loader.LoadProject(cmd.Context()) if loadErr != nil { - return handleInvalidComposeFileErr(ctx, loadErr) + return handleInvalidComposeFileErr(cmd.Context(), loadErr) } fixes := compose.FixProject(project) printFixResults(fixes) - if dryRun { + if dryRun || len(fixes) == 0 { return nil } @@ -597,33 +587,30 @@ func makeComposeFixCmd() *cobra.Command { } func printFixResults(fixes []compose.FixResult) { + if len(fixes) == 0 { + term.Info("No fixes needed.") + return + } term.Println("Fixes applied:") for _, fix := range fixes { term.Printf(" service %q: %s\n", fix.Service, describeFixResult(fix)) } - term.Println() - term.Printf("%d fixes applied.\n", len(fixes)) + term.Printf("\n%d fix(es) applied.\n", len(fixes)) } func describeFixResult(fix compose.FixResult) string { switch fix.Action { case "removed": - return fmt.Sprintf("removed unsupported directive: %s", fix.Field) + return fmt.Sprintf("removed unsupported directive: %s (%s)", fix.Field, fix.Reason) case "changed": - if fix.Field == "domainname" { - return fmt.Sprintf("moved hostname %q to domainname", fix.After) - } if fix.Before != "" { - return fmt.Sprintf("changed %s: %s to %s (%s)", fix.Field, fix.Before, fix.After, fix.Reason) + return fmt.Sprintf("changed %s from %q to %q (%s)", fix.Field, fix.Before, fix.After, fix.Reason) } - return fmt.Sprintf("changed %s: %s (%s)", fix.Field, fix.After, fix.Reason) + return fmt.Sprintf("changed %s to %q (%s)", fix.Field, fix.After, fix.Reason) default: if fix.Field == "mode" { return fmt.Sprintf("added mode: %s to %s", fix.After, fix.Reason) } - if fix.Field == "healthcheck" { - return fmt.Sprintf("added healthcheck for %s", fix.Reason) - } if fix.Reason != "" { return fmt.Sprintf("added %s: %s (%s)", fix.Field, fix.After, fix.Reason) } diff --git a/src/pkg/cli/compose/fix.go b/src/pkg/cli/compose/fix.go index 37d76cc65..b3aaf72bd 100644 --- a/src/pkg/cli/compose/fix.go +++ b/src/pkg/cli/compose/fix.go @@ -3,7 +3,6 @@ package compose import ( "fmt" "sort" - "strings" composeTypes "github.com/compose-spec/compose-go/v2/types" ) @@ -48,12 +47,9 @@ func fixService(service *composeTypes.ServiceConfig) []FixResult { isManagedStoreImage := IsPostgresRepo(repo) || IsRedisRepo(repo) || IsMongoRepo(repo) results = append(results, fixPorts(service, isManagedStoreImage)...) - results = append(results, fixManagedServiceExtensions(service, repo)...) - results = append(results, fixResources(service)...) + results = append(results, fixLimitsToReservations(service)...) results = append(results, fixRestart(service)...) - results = append(results, fixHostname(service)...) results = append(results, fixUnsupportedDirectives(service)...) - results = append(results, fixIngressHealthcheck(service)...) return results } @@ -94,74 +90,24 @@ func portReason(target uint32, reason string) string { return fmt.Sprintf("port %d (%s)", target, reason) } -func fixManagedServiceExtensions(service *composeTypes.ServiceConfig, repo string) []FixResult { - switch { - case IsPostgresRepo(repo): - return addManagedServiceExtension(service, "x-defang-postgres", "postgres image detected") - case IsRedisRepo(repo): - return addManagedServiceExtension(service, "x-defang-redis", "redis image detected") - case IsMongoRepo(repo): - return addManagedServiceExtension(service, "x-defang-mongodb", "mongo image detected") - default: +func fixLimitsToReservations(service *composeTypes.ServiceConfig) []FixResult { + if service.Deploy == nil { return nil } -} - -func addManagedServiceExtension(service *composeTypes.ServiceConfig, key, reason string) []FixResult { - if service.Extensions == nil { - service.Extensions = composeTypes.Extensions{} - } - if _, ok := service.Extensions[key]; ok { + if service.Deploy.Resources.Limits == nil || service.Deploy.Resources.Reservations != nil { return nil } - service.Extensions[key] = true + limits := *service.Deploy.Resources.Limits + service.Deploy.Resources.Reservations = &limits return []FixResult{{ Service: service.Name, - Field: key, + Field: "deploy.resources.reservations", Action: "added", - After: "true", - Reason: reason, + After: "copied from deploy.resources.limits", + Reason: "Defang uses reservations for scheduling, not limits", }} } -func fixResources(service *composeTypes.ServiceConfig) []FixResult { - var results []FixResult - if service.Deploy == nil { - service.Deploy = &composeTypes.DeployConfig{} - } - if service.Deploy.Resources.Limits != nil && service.Deploy.Resources.Reservations == nil { - limits := *service.Deploy.Resources.Limits - service.Deploy.Resources.Reservations = &limits - results = append(results, FixResult{ - Service: service.Name, - Field: "deploy.resources.reservations", - Action: "added", - After: "deploy.resources.limits", - Reason: "limits used as reservations", - }) - } - if service.Deploy.Resources.Reservations == nil { - service.Deploy.Resources.Reservations = &composeTypes.Resource{} - } - if service.Deploy.Resources.Reservations.MemoryBytes == 0 { - memory := composeTypes.UnitBytes(512 * MiB) - after := "512M" - if service.Extensions["x-defang-static-files"] != nil { - memory = composeTypes.UnitBytes(256 * MiB) - after = "256M" - } - service.Deploy.Resources.Reservations.MemoryBytes = memory - results = append(results, FixResult{ - Service: service.Name, - Field: "deploy.resources.reservations.memory", - Action: "added", - After: after, - Reason: "missing memory reservation", - }) - } - return results -} - func fixRestart(service *composeTypes.ServiceConfig) []FixResult { restart := restartFromDeployPolicy(service) if restart == "" && isSupportedRestart(service.Restart) { @@ -176,15 +122,12 @@ func fixRestart(service *composeTypes.ServiceConfig) []FixResult { reason := "unsupported restart policy" if service.Deploy != nil && service.Deploy.RestartPolicy != nil { - reason = "deploy.restart_policy is unsupported" + reason = "deploy.restart_policy is unsupported; converted to service-level restart" + service.Deploy.RestartPolicy = nil } else if before == "" { reason = "missing restart policy" } - if service.Deploy != nil && service.Deploy.RestartPolicy != nil { - service.Deploy.RestartPolicy = nil - } - action := "changed" if before == "" { action = "added" @@ -215,23 +158,6 @@ func isSupportedRestart(restart string) bool { return restart == "always" || restart == defaultRestartPolicy } -func fixHostname(service *composeTypes.ServiceConfig) []FixResult { - if service.Hostname == "" { - return nil - } - before := service.Hostname - service.DomainName = service.Hostname - service.Hostname = "" - return []FixResult{{ - Service: service.Name, - Field: "domainname", - Action: "changed", - Before: before, - After: service.DomainName, - Reason: "hostname is unsupported", - }} -} - func fixUnsupportedDirectives(service *composeTypes.ServiceConfig) []FixResult { var results []FixResult if len(service.DNS) != 0 { @@ -266,26 +192,3 @@ func removedDirective(service, field string) FixResult { Reason: "unsupported directive", } } - -func fixIngressHealthcheck(service *composeTypes.ServiceConfig) []FixResult { - if service.HealthCheck != nil && !service.HealthCheck.Disable { - return nil - } - for _, port := range service.Ports { - if port.Mode != Mode_INGRESS { - continue - } - url := fmt.Sprintf("http://localhost:%d/", port.Target) - service.HealthCheck = &composeTypes.HealthCheckConfig{ - Test: composeTypes.HealthCheckTest{"CMD", "curl", "-f", url}, - } - return []FixResult{{ - Service: service.Name, - Field: "healthcheck", - Action: "added", - After: strings.Join(service.HealthCheck.Test, " "), - Reason: fmt.Sprintf("ingress port %d", port.Target), - }} - } - return nil -} diff --git a/src/pkg/cli/compose/fix_test.go b/src/pkg/cli/compose/fix_test.go index 1da95151f..87078a914 100644 --- a/src/pkg/cli/compose/fix_test.go +++ b/src/pkg/cli/compose/fix_test.go @@ -16,7 +16,7 @@ func TestFixProject(t *testing.T) { check func(*testing.T, *Project) }{ { - name: "web service defaults", + name: "web service port mode and restart", project: &Project{Services: Services{ "web": { Name: "web", @@ -26,25 +26,20 @@ func TestFixProject(t *testing.T) { }}, want: []FixResult{ {Service: "web", Field: "mode", Action: "added", After: Mode_INGRESS, Reason: "port 8080"}, - {Service: "web", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, {Service: "web", Field: "restart", Action: "added", After: defaultRestartPolicy, Reason: "missing restart policy"}, - {Service: "web", Field: "healthcheck", Action: "added", After: "CMD curl -f http://localhost:8080/", Reason: "ingress port 8080"}, }, check: func(t *testing.T, project *Project) { service := project.Services["web"] if service.Ports[0].Mode != Mode_INGRESS { t.Fatalf("port mode = %q, want %q", service.Ports[0].Mode, Mode_INGRESS) } - if service.HealthCheck == nil { - t.Fatal("healthcheck was not added") - } - if service.Deploy.Resources.Reservations.MemoryBytes != 512*MiB { - t.Fatalf("memory = %d, want %d", service.Deploy.Resources.Reservations.MemoryBytes, 512*MiB) + if service.Restart != defaultRestartPolicy { + t.Fatalf("restart = %q, want %q", service.Restart, defaultRestartPolicy) } }, }, { - name: "managed postgres defaults to host", + name: "managed postgres defaults to host mode", project: &Project{Services: Services{ "db": { Name: "db", @@ -54,8 +49,6 @@ func TestFixProject(t *testing.T) { }}, want: []FixResult{ {Service: "db", Field: "mode", Action: "added", After: Mode_HOST, Reason: "port 5432 (database image)"}, - {Service: "db", Field: "x-defang-postgres", Action: "added", After: "true", Reason: "postgres image detected"}, - {Service: "db", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, {Service: "db", Field: "restart", Action: "added", After: defaultRestartPolicy, Reason: "missing restart policy"}, }, check: func(t *testing.T, project *Project) { @@ -63,9 +56,6 @@ func TestFixProject(t *testing.T) { if service.Ports[0].Mode != Mode_HOST { t.Fatalf("port mode = %q, want %q", service.Ports[0].Mode, Mode_HOST) } - if service.Extensions["x-defang-postgres"] != true { - t.Fatal("x-defang-postgres was not added") - } }, }, { @@ -81,7 +71,7 @@ func TestFixProject(t *testing.T) { }, }}, want: []FixResult{ - {Service: "api", Field: "deploy.resources.reservations", Action: "added", After: "deploy.resources.limits", Reason: "limits used as reservations"}, + {Service: "api", Field: "deploy.resources.reservations", Action: "added", After: "copied from deploy.resources.limits", Reason: "Defang uses reservations for scheduling, not limits"}, }, check: func(t *testing.T, project *Project) { service := project.Services["api"] @@ -108,7 +98,6 @@ func TestFixProject(t *testing.T) { }, }}, want: []FixResult{ - {Service: "worker", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, {Service: "worker", Field: "dns", Action: "removed", Before: "present", Reason: "unsupported directive"}, {Service: "worker", Field: "dns_search", Action: "removed", Before: "present", Reason: "unsupported directive"}, {Service: "worker", Field: "devices", Action: "removed", Before: "present", Reason: "unsupported directive"}, @@ -123,26 +112,23 @@ func TestFixProject(t *testing.T) { }, }, { - name: "hostname and deploy restart policy", + name: "deploy restart policy converted to service restart", project: &Project{Services: Services{ "api": { - Name: "api", - Image: "api", - Hostname: "api.example.com", + Name: "api", + Image: "api", Deploy: &composeTypes.DeployConfig{ RestartPolicy: &composeTypes.RestartPolicy{Condition: "any"}, }, }, }}, want: []FixResult{ - {Service: "api", Field: "deploy.resources.reservations.memory", Action: "added", After: "512M", Reason: "missing memory reservation"}, - {Service: "api", Field: "restart", Action: "added", After: "always", Reason: "deploy.restart_policy is unsupported"}, - {Service: "api", Field: "domainname", Action: "changed", Before: "api.example.com", After: "api.example.com", Reason: "hostname is unsupported"}, + {Service: "api", Field: "restart", Action: "added", Before: "", After: "always", Reason: "deploy.restart_policy is unsupported; converted to service-level restart"}, }, check: func(t *testing.T, project *Project) { service := project.Services["api"] - if service.Hostname != "" || service.DomainName != "api.example.com" { - t.Fatalf("hostname/domainname = %q/%q", service.Hostname, service.DomainName) + if service.Restart != "always" { + t.Fatalf("restart = %q, want %q", service.Restart, "always") } if service.Deploy.RestartPolicy != nil { t.Fatal("deploy.restart_policy was not removed") @@ -150,25 +136,38 @@ func TestFixProject(t *testing.T) { }, }, { - name: "static files memory default", + name: "udp port defaults to host mode", project: &Project{Services: Services{ - "cdn": { - Name: "cdn", - Image: "nginx", - Restart: defaultRestartPolicy, - Extensions: composeTypes.Extensions{"x-defang-static-files": "./public"}, + "dns": { + Name: "dns", + Image: "coredns", + Restart: "always", + Ports: []composeTypes.ServicePortConfig{{Target: 53, Protocol: Protocol_UDP}}, }, }}, want: []FixResult{ - {Service: "cdn", Field: "deploy.resources.reservations.memory", Action: "added", After: "256M", Reason: "missing memory reservation"}, + {Service: "dns", Field: "mode", Action: "added", After: Mode_HOST, Reason: "port 53 (UDP port)"}, }, check: func(t *testing.T, project *Project) { - service := project.Services["cdn"] - if service.Deploy.Resources.Reservations.MemoryBytes != 256*MiB { - t.Fatalf("memory = %d, want %d", service.Deploy.Resources.Reservations.MemoryBytes, 256*MiB) + service := project.Services["dns"] + if service.Ports[0].Mode != Mode_HOST { + t.Fatalf("port mode = %q, want %q", service.Ports[0].Mode, Mode_HOST) } }, }, + { + name: "no fixes needed", + project: &Project{Services: Services{ + "app": { + Name: "app", + Image: "myapp", + Restart: "always", + Ports: []composeTypes.ServicePortConfig{{Target: 80, Mode: Mode_INGRESS}}, + }, + }}, + want: nil, + check: func(t *testing.T, project *Project) {}, + }, } for _, tt := range tests {