diff --git a/src/pkg/cli/compose/convert.go b/src/pkg/cli/compose/convert.go index 944ff769d..dbf55b63f 100644 --- a/src/pkg/cli/compose/convert.go +++ b/src/pkg/cli/compose/convert.go @@ -16,17 +16,24 @@ import ( func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compose.Services, force BuildContext) ([]*defangv1.Service, error) { // Create a regexp to detect private service names in environment variable values var serviceNames []string + var nonReplaceServiceNames []string for _, svccfg := range serviceConfigs { if network(&svccfg) == defangv1.Network_PRIVATE && slices.ContainsFunc(svccfg.Ports, func(p compose.ServicePortConfig) bool { return p.Mode == "host" // only private services with host ports get DNS names }) { serviceNames = append(serviceNames, regexp.QuoteMeta(svccfg.Name)) + } else { + nonReplaceServiceNames = append(nonReplaceServiceNames, regexp.QuoteMeta(svccfg.Name)) } } var serviceNameRegex *regexp.Regexp if len(serviceNames) > 0 { serviceNameRegex = regexp.MustCompile(`\b(?:` + strings.Join(serviceNames, "|") + `)\b`) } + var nonReplaceServiceNameRegex *regexp.Regexp + if len(nonReplaceServiceNames) > 0 { + nonReplaceServiceNameRegex = regexp.MustCompile(`\b(?:` + strings.Join(nonReplaceServiceNames, "|") + `)\b`) + } // Preload the current config so we can detect which environment variables should be passed as "secrets" config, err := c.ListConfig(ctx) @@ -130,7 +137,11 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos // Check if the environment variable is an existing config; if so, mark it as such if _, ok := slices.BinarySearch(config.Names, key); ok { - term.Warnf("service %q: environment variable %q overridden by config", svccfg.Name, key) + if serviceNameRegex != nil && serviceNameRegex.MatchString(*value) { + term.Warnf("service %q: environment variable %q needs service name fix-up, but is overridden by config, which will not be fixed up.", svccfg.Name, key) + } else { + term.Warnf("service %q: environment variable %q overridden by config", svccfg.Name, key) + } envFromConfig = append(envFromConfig, key) continue } @@ -142,7 +153,9 @@ func ConvertServices(ctx context.Context, c client.Client, serviceConfigs compos return c.ServiceDNS(NormalizeServiceName(serviceName)) }) if val != *value { - term.Warnf("service %q: service names were replaced in environment variable %q: %q", svccfg.Name, key, val) + term.Warnf("service %q: service names were fixed up in environment variable %q: %q", svccfg.Name, key, val) + } else if nonReplaceServiceNameRegex != nil && nonReplaceServiceNameRegex.MatchString(*value) { + term.Warnf("service %q: service names in the environment variable %q were not fixed up, only services with port mode set to host will be fixed up.", svccfg.Name, key) } } envs[key] = val diff --git a/src/pkg/cli/compose/loader_test.go b/src/pkg/cli/compose/loader_test.go index b62e018ff..04fcea27e 100644 --- a/src/pkg/cli/compose/loader_test.go +++ b/src/pkg/cli/compose/loader_test.go @@ -45,8 +45,11 @@ func compare(actual []byte, goldenFile string) error { } return os.WriteFile(goldenFile, actual, 0644) } else { - return diff(string(actual), string(golden)) + if err := diff(string(actual), string(golden)); err != nil { + return fmt.Errorf("%s %w", goldenFile, err) + } } + return nil } func diff(actualRaw, goldenRaw string) error { diff --git a/src/pkg/cli/compose/validation_test.go b/src/pkg/cli/compose/validation_test.go index eeff1d9ab..38e301cc4 100644 --- a/src/pkg/cli/compose/validation_test.go +++ b/src/pkg/cli/compose/validation_test.go @@ -7,10 +7,12 @@ import ( "strings" "testing" + "github.com/DefangLabs/defang/src/pkg/cli/client" "github.com/DefangLabs/defang/src/pkg/term" + defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1" ) -func TestValidation(t *testing.T) { +func TestValidationAndConvert(t *testing.T) { oldTerm := term.DefaultTerm t.Cleanup(func() { term.DefaultTerm = oldTerm @@ -30,6 +32,14 @@ func TestValidation(t *testing.T) { logs.WriteString(err.Error() + "\n") } + mockClient := MockClient{ + configs: []string{"CONFIG1", "CONFIG2"}, + } + if _, err = ConvertServices(context.Background(), mockClient, proj.Services, BuildContextIgnore); err != nil { + t.Logf("Service conversion failed: %v", err) + logs.WriteString(err.Error() + "\n") + } + // The order of the services is not guaranteed, so we sort the logs before comparing logLines := strings.Split(strings.Trim(logs.String(), "\n"), "\n") slices.Sort(logLines) @@ -41,3 +51,19 @@ func TestValidation(t *testing.T) { } }) } + +type MockClient struct { + client.Client + configs []string +} + +func (m MockClient) ListConfig(ctx context.Context) (*defangv1.Secrets, error) { + return &defangv1.Secrets{ + Names: m.configs, + Project: "mock-project", + }, nil +} + +func (m MockClient) ServiceDNS(name string) string { + return "mock-" + name +} diff --git a/src/tests/alttestproj/compose.yaml.warnings b/src/tests/alttestproj/compose.yaml.warnings index e69de29bb..1ed9c5adc 100644 --- a/src/tests/alttestproj/compose.yaml.warnings +++ b/src/tests/alttestproj/compose.yaml.warnings @@ -0,0 +1 @@ + * Compressing build context for dfnx at . \ No newline at end of file diff --git a/src/tests/emptyenv/compose.yaml.warnings b/src/tests/emptyenv/compose.yaml.warnings index 9690cf858..3bcd08e12 100644 --- a/src/tests/emptyenv/compose.yaml.warnings +++ b/src/tests/emptyenv/compose.yaml.warnings @@ -1,2 +1,3 @@ ! service "emptyenv": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) - ! service "emptyenv": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors \ No newline at end of file + ! service "emptyenv": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + * Compressing build context for emptyenv at . \ No newline at end of file diff --git a/src/tests/fixupenv/compose.yaml b/src/tests/fixupenv/compose.yaml index fa0fa5a4e..a70e7ac53 100644 --- a/src/tests/fixupenv/compose.yaml +++ b/src/tests/fixupenv/compose.yaml @@ -9,3 +9,16 @@ services: environment: - "API_URL=http://Mistral:8000" - "SENSITIVE_DATA" + bad-service: + image: "somedb:latest" + ports: + - mode: ingress + target: 5432 + use-bad-service: + image: "service:latest" + environment: + - "DB_URL=bad-service:5432" + env-in-config: + image: "service:latest" + environment: + - "CONFIG1=http://Mistral:8000" diff --git a/src/tests/fixupenv/compose.yaml.convert b/src/tests/fixupenv/compose.yaml.convert index 53893bd0c..19a099f31 100644 --- a/src/tests/fixupenv/compose.yaml.convert +++ b/src/tests/fixupenv/compose.yaml.convert @@ -1,4 +1,27 @@ [ + { + "name": "bad-service", + "image": "somedb:latest", + "platform": 2, + "internal": true, + "ports": [ + { + "target": 5432, + "mode": 1 + } + ], + "networks": 1 + }, + { + "name": "env-in-config", + "image": "service:latest", + "platform": 2, + "internal": true, + "environment": { + "CONFIG1": "http://mistral:8000" + }, + "networks": 1 + }, { "name": "mistral", "image": "mistral:latest", @@ -25,5 +48,15 @@ } ], "networks": 1 + }, + { + "name": "use-bad-service", + "image": "service:latest", + "platform": 2, + "internal": true, + "environment": { + "DB_URL": "bad-service:5432" + }, + "networks": 1 } ] \ No newline at end of file diff --git a/src/tests/fixupenv/compose.yaml.golden b/src/tests/fixupenv/compose.yaml.golden index 51e682e5a..b3ee5d31f 100644 --- a/src/tests/fixupenv/compose.yaml.golden +++ b/src/tests/fixupenv/compose.yaml.golden @@ -7,6 +7,19 @@ services: ports: - mode: host target: 8000 + bad-service: + image: somedb:latest + networks: + default: null + ports: + - mode: ingress + target: 5432 + env-in-config: + environment: + CONFIG1: http://Mistral:8000 + image: service:latest + networks: + default: null ui: environment: API_URL: http://Mistral:8000 @@ -14,6 +27,12 @@ services: image: ui:latest networks: default: null + use-bad-service: + environment: + DB_URL: bad-service:5432 + image: service:latest + networks: + default: null networks: default: name: fixupenv_default diff --git a/src/tests/fixupenv/compose.yaml.warnings b/src/tests/fixupenv/compose.yaml.warnings index 425990c21..2916a78c8 100644 --- a/src/tests/fixupenv/compose.yaml.warnings +++ b/src/tests/fixupenv/compose.yaml.warnings @@ -1,5 +1,15 @@ ! service "Mistral": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) ! service "Mistral": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "bad-service": ingress port without healthcheck defaults to GET / HTTP/1.1 + ! service "bad-service": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) + ! service "bad-service": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "env-in-config": environment variable "CONFIG1" needs service name fix-up, but is overridden by config, which will not be fixed up. + ! service "env-in-config": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) + ! service "env-in-config": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors ! service "ui": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) ! service "ui": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "ui": service names were fixed up in environment variable "API_URL": "http://mock-mistral:8000" + ! service "use-bad-service": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) + ! service "use-bad-service": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "use-bad-service": service names in the environment variable "DB_URL" were not fixed up, only services with port mode set to host will be fixed up. ! service name "Mistral" was normalized to "mistral" \ No newline at end of file diff --git a/src/tests/redis/compose.yaml.warnings b/src/tests/redis/compose.yaml.warnings index ed747a7c7..0e12f186d 100644 --- a/src/tests/redis/compose.yaml.warnings +++ b/src/tests/redis/compose.yaml.warnings @@ -4,4 +4,5 @@ ! service "y": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) ! service "y": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors ! service "z": missing compose directive: restart; assuming 'unless-stopped' (add 'restart' to silence) - ! service "z": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors \ No newline at end of file + ! service "z": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "z": stateful service will lose data on restart; use a managed service instead \ No newline at end of file diff --git a/src/tests/secretname/compose.yaml.warnings b/src/tests/secretname/compose.yaml.warnings index f6d16217c..33762a4ce 100644 --- a/src/tests/secretname/compose.yaml.warnings +++ b/src/tests/secretname/compose.yaml.warnings @@ -1 +1,3 @@ - ! service "app": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors \ No newline at end of file + ! service "app": missing memory reservation; specify deploy.resources.reservations.memory to avoid out-of-memory errors + ! service "app": secrets will be exposed as environment variables, not files (use 'environment' instead) + * Compressing build context for app at . \ No newline at end of file diff --git a/src/tests/testproj/compose.yaml.warnings b/src/tests/testproj/compose.yaml.warnings index f6d357a7f..8a2234455 100644 --- a/src/tests/testproj/compose.yaml.warnings +++ b/src/tests/testproj/compose.yaml.warnings @@ -1 +1,4 @@ - ! unsupported compose extension: "x-unsupported" \ No newline at end of file + ! No port 'mode' was specified; defaulting to 'ingress' (add 'mode: ingress' to silence) + ! service "dfnx": secrets will be exposed as environment variables, not files (use 'environment' instead) + ! unsupported compose extension: "x-unsupported" + * Compressing build context for dfnx at . \ No newline at end of file