fix: no 'overridden by config' warning for X=${X} (#1316)

Author: lionello

src/pkg/cli/client/mock.go

@@ -18,11 +18,11 @@ func (m MockProvider) CreateUploadURL(ctx context.Context, req *defangv1.UploadU
 }
 
 func (m MockProvider) ListConfig(ctx context.Context, req *defangv1.ListConfigsRequest) (*defangv1.Secrets, error) {
-	return &defangv1.Secrets{Names: []string{"VAR1"}}, nil
+	return &defangv1.Secrets{Names: []string{"CONFIG1", "CONFIG2", "dummy", "ENV1", "SENSITIVE_DATA", "VAR1"}}, nil
 }
 
 func (m MockProvider) ServiceDNS(service string) string {
-	return service
+	return "mock-" + service
 }
 
 // MockServerStream mocks a ServerStream.

src/pkg/cli/compose/fixup.go

@@ -97,11 +97,13 @@ func FixupServices(ctx context.Context, provider client.Provider, project *compo
 			}
 
 			// Pack the build context into a Archive and upload
-			url, err := getRemoteBuildContext(ctx, provider, project.Name, svccfg.Name, svccfg.Build, upload)
-			if err != nil {
-				return err
+			if !strings.Contains(svccfg.Build.Context, "://") {
+				url, err := getRemoteBuildContext(ctx, provider, project.Name, svccfg.Name, svccfg.Build, upload)
+				if err != nil {
+					return err
+				}
+				svccfg.Build.Context = url
 			}
-			svccfg.Build.Context = url
 
 			var removedArgs []string
 			for key, value := range svccfg.Build.Args {
@@ -143,8 +145,8 @@ func FixupServices(ctx context.Context, provider client.Provider, project *compo
 				delete(svccfg.Environment, key) // remove the empty key; this is safe
 				continue
 			}
-			if value == nil {
-				continue // will be from config
+			if value == nil || *value == "${"+key+"}" {
+				continue // actual value will be from config
 			}
 
 			// Check if the environment variable is an existing config; if so, mark it as such

src/pkg/cli/compose/loader.go

@@ -156,14 +156,14 @@ func (c *Loader) NewProjectOptions() (*cli.ProjectOptions, error) {
 					if hasSubstitution(templ, key) {
 						// We don't (yet) support substitution patterns during deployment
 						if inEnv {
-							term.Warnf("Environment variable %q is not used; add it to `.env` if needed", key)
+							term.Warnf("Environment variable %q is ignored; add it to `.env` if needed", key)
 						} else {
 							term.Debugf("Unresolved environment variable %q", key)
 						}
 						return "", false
 					}
 					if inEnv {
-						term.Warnf("Environment variable %q is not used; add it to `.env` or it may be resolved from config during deployment", key)
+						term.Warnf("Environment variable %q is ignored; add it to `.env` or it may be resolved from config during deployment", key)
 					} else {
 						term.Debugf("Environment variable %q was not resolved locally. It may be resolved from config during deployment", key)
 					}

src/pkg/cli/compose/validation_test.go

@@ -16,22 +16,6 @@ import (
 	composeTypes "github.com/compose-spec/compose-go/v2/types"
 )
 
-type validationMockProvider struct {
-	client.Provider
-	configs []string
-}
-
-func (m validationMockProvider) ListConfig(ctx context.Context, req *defangv1.ListConfigsRequest) (*defangv1.Secrets, error) {
-	return &defangv1.Secrets{
-		Names:   m.configs,
-		Project: "mock-project",
-	}, nil
-}
-
-func (m validationMockProvider) ServiceDNS(name string) string {
-	return "mock-" + name
-}
-
 func TestValidationAndConvert(t *testing.T) {
 	oldTerm := term.DefaultTerm
 	t.Cleanup(func() {
@@ -40,9 +24,7 @@ func TestValidationAndConvert(t *testing.T) {
 
 	t.Setenv("NODE_ENV", "if-you-see-this-env-was-used") // for interpolate/compose.yaml; should be ignored
 
-	mockClient := validationMockProvider{
-		configs: []string{"CONFIG1", "CONFIG2", "dummy", "ENV1", "SENSITIVE_DATA"},
-	}
+	mockClient := client.MockProvider{}
 	listConfigNamesFunc := func(ctx context.Context) ([]string, error) {
 		configs, err := mockClient.ListConfig(ctx, &defangv1.ListConfigsRequest{})
 		if err != nil {

src/testdata/configoverride/compose.yaml.warnings

@@ -1 +1,2 @@
+ ! service "service1": environment variable(s) ["VAR1"] overridden by config
  ! service "service1": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors

src/testdata/fixupenv/compose.yaml.fixup

@@ -18,7 +18,7 @@
     "command": null,
     "entrypoint": null,
     "environment": {
-      "CONFIG1": "http://mistral:8000"
+      "CONFIG1": null
     },
     "image": "service:latest",
     "networks": {
@@ -30,7 +30,7 @@
       "context": ".",
       "dockerfile": "Dockerfile",
       "args": {
-        "API_URL": "http://mistral:8000"
+        "API_URL": "http://mock-mistral:8000"
       }
     },
     "command": null,
@@ -59,7 +59,7 @@
     "command": null,
     "entrypoint": null,
     "environment": {
-      "API_URL": "http://mistral:8000",
+      "API_URL": "http://mock-mistral:8000",
       "SENSITIVE_DATA": null
     },
     "image": "ui:latest",

src/testdata/interpolate/compose.yaml

@@ -11,3 +11,4 @@ services:
       - NOP_BRACED=abc$${def} # FIXME: this should not get resolved in CD
       - NODE_ENV=${NODE_ENV}
       - PORT=${PORT:-8080}
+      - VAR1=${VAR1} # from config

src/testdata/interpolate/compose.yaml.fixup

@@ -10,6 +10,7 @@
       "NOP": "abc$def",
       "NOP_BRACED": "abc${def}",
       "PORT": "8080",
+      "VAR1": "${VAR1}",
       "interpolate": "value"
     },
     "image": "alpine",

src/testdata/interpolate/compose.yaml.golden

@@ -9,6 +9,7 @@ services:
       NOP: abc$def
       NOP_BRACED: abc${def}
       PORT: "8080"
+      VAR1: ${VAR1}
       interpolate: value
     image: alpine
     networks:

src/testdata/interpolate/compose.yaml.warnings

@@ -1,4 +1,4 @@
- ! Environment variable "NODE_ENV" is not used; add it to `.env` if needed
- ! Environment variable "NODE_ENV" is not used; add it to `.env` or it may be resolved from config during deployment
+ ! Environment variable "NODE_ENV" is ignored; add it to `.env` if needed
+ ! Environment variable "NODE_ENV" is ignored; add it to `.env` or it may be resolved from config during deployment
  ! service "interpolate": missing memory reservation; using provider-specific defaults. Specify deploy.resources.reservations.memory to avoid out-of-memory errors
 missing configs ["NODE_ENV" "POSTGRES_PASSWORD" "def"] (https://docs.defang.io/docs/concepts/configuration)