From a48f8d8efb2b6f26e36a511067c3b0529113f142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Mon, 5 Jan 2026 16:53:00 -0800 Subject: [PATCH 1/5] /{go,integration-tests}: support environment variable interpolation --- go/cmd/dolt/commands/sqlserver/sqlserver.go | 12 ++ .../doltcore/servercfg/env_interpolate.go | 124 +++++++++++++ .../doltcore/servercfg/yaml_config.go | 5 + .../doltcore/servercfg/yaml_config_test.go | 78 ++++++++ .../sql-server-config-env-interpolation.bats | 169 ++++++++++++++++++ 5 files changed, 388 insertions(+) create mode 100644 go/libraries/doltcore/servercfg/env_interpolate.go create mode 100644 integration-tests/bats/sql-server-config-env-interpolation.bats diff --git a/go/cmd/dolt/commands/sqlserver/sqlserver.go b/go/cmd/dolt/commands/sqlserver/sqlserver.go index 1e21163af66..10864228970 100644 --- a/go/cmd/dolt/commands/sqlserver/sqlserver.go +++ b/go/cmd/dolt/commands/sqlserver/sqlserver.go @@ -83,6 +83,18 @@ var sqlServerDocs = cli.CommandDocumentationContent{ " other command line arguments are ignored.\n\nThis is an example yaml configuration file showing all supported" + " items and their default values:\n\n" + indentLines(servercfg.ServerConfigAsYAMLConfig(DefaultCommandLineServerConfig()).String()) + "\n\n" + ` +ENVIRONMENT VARIABLE INTERPOLATION: + +SQL server yaml configs support environment variable interpolation: + + ${VAR} Expands to the value of VAR (error if VAR is unset or empty) + ${VAR:-default} Expands to VAR if set and non-empty; otherwise expands to default + $$ Escapes to a literal '$' + +Notes: + - Interpolation happens before YAML parsing. + - Quote values for string fields when needed (e.g. values containing ':'), but do not quote placeholders intended for numeric/bool fields. + SUPPORTED CONFIG FILE FIELDS: {{.EmphasisLeft}}data_dir{{.EmphasisRight}}: A directory where the server will load dolt databases to serve, and create new ones. Defaults to the current directory. diff --git a/go/libraries/doltcore/servercfg/env_interpolate.go b/go/libraries/doltcore/servercfg/env_interpolate.go new file mode 100644 index 00000000000..65a43e19a53 --- /dev/null +++ b/go/libraries/doltcore/servercfg/env_interpolate.go @@ -0,0 +1,124 @@ +package servercfg + +import ( + "fmt" + "os" +) + +// envLookupFunc returns (value, true) if the env var exists. +// Note that an env var may exist but be empty. +type envLookupFunc func(string) (string, bool) + +// interpolateEnv expands environment variable placeholders in |data|. +// +// Supported syntax: +// - ${VAR} : expands to VAR's value; error if unset or empty +// - ${VAR:-default} : expands to VAR's value if set and non-empty; otherwise to default +// - $$ : escapes to a literal '$' +// +// Notes: +// - Expansion is applied to the input text only (env var values are inserted literally). +// - Default expressions are expanded (i.e. nested placeholders inside defaults are supported). +func interpolateEnv(data []byte, lookup envLookupFunc) ([]byte, error) { + if lookup == nil { + lookup = os.LookupEnv + } + + out := make([]byte, 0, len(data)) + for i := 0; i < len(data); i++ { + if data[i] != '$' { + out = append(out, data[i]) + continue + } + + // Escape sequence: $$ -> $ + if i+1 < len(data) && data[i+1] == '$' { + out = append(out, '$') + i++ + continue + } + + // Placeholder: ${...} + if i+1 >= len(data) || data[i+1] != '{' { + // Leave stray '$' untouched + out = append(out, '$') + continue + } + + // Find closing brace. + start := i + 2 // after ${ + j := start + for ; j < len(data) && data[j] != '}'; j++ { + } + if j >= len(data) { + return nil, fmt.Errorf("unterminated environment placeholder starting at byte %d", i) + } + + expr := data[start:j] + varName, def, hasDefault, err := parseEnvExpr(expr) + if err != nil { + return nil, err + } + + val, ok := lookup(varName) + if ok && val != "" { + out = append(out, []byte(val)...) + } else if hasDefault { + expandedDef, err := interpolateEnv(def, lookup) + if err != nil { + return nil, err + } + out = append(out, expandedDef...) + } else { + return nil, fmt.Errorf("environment variable %q is not set", varName) + } + + // Skip to closing brace. + i = j + } + + return out, nil +} + +func parseEnvExpr(expr []byte) (varName string, def []byte, hasDefault bool, err error) { + // Split on the first occurrence of ":-" + varPart := expr + var defPart []byte + for k := 0; k+1 < len(expr); k++ { + if expr[k] == ':' && expr[k+1] == '-' { + varPart = expr[:k] + defPart = expr[k+2:] + hasDefault = true + break + } + } + + if len(varPart) == 0 { + return "", nil, false, fmt.Errorf("invalid environment placeholder: empty variable name") + } + if !isValidEnvVarName(varPart) { + return "", nil, false, fmt.Errorf("invalid environment variable name %q", string(varPart)) + } + + return string(varPart), defPart, hasDefault, nil +} + +func isValidEnvVarName(b []byte) bool { + if len(b) == 0 { + return false + } + for i := range b { + c := b[i] + if i == 0 { + if !((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_') { + return false + } + } else { + if !((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_') { + return false + } + } + } + return true +} + diff --git a/go/libraries/doltcore/servercfg/yaml_config.go b/go/libraries/doltcore/servercfg/yaml_config.go index d71fcb6b78a..0d5b74f3ced 100644 --- a/go/libraries/doltcore/servercfg/yaml_config.go +++ b/go/libraries/doltcore/servercfg/yaml_config.go @@ -194,6 +194,11 @@ func YamlConfigFromFile(fs filesys.Filesys, path string) (ServerConfig, error) { return nil, fmt.Errorf("Failed to read file '%s'. Error: %s", path, err.Error()) } + data, err = interpolateEnv(data, nil) + if err != nil { + return nil, fmt.Errorf("Failed to interpolate environment variables in yaml file '%s'. Error: %s", path, err.Error()) + } + cfg, err := NewYamlConfig(data) if err != nil { return nil, fmt.Errorf("Failed to parse yaml file '%s'. Error: %s", path, err.Error()) diff --git a/go/libraries/doltcore/servercfg/yaml_config_test.go b/go/libraries/doltcore/servercfg/yaml_config_test.go index 42678dc997f..e8b8c717580 100644 --- a/go/libraries/doltcore/servercfg/yaml_config_test.go +++ b/go/libraries/doltcore/servercfg/yaml_config_test.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" + + "github.com/dolthub/dolt/go/libraries/utils/filesys" ) var trueValue = true @@ -206,6 +208,82 @@ cluster: require.Equal(t, "http://doltdb-1.doltdb:50051/{database}", config.ClusterConfig().StandbyRemotes()[0].RemoteURLTemplate()) } +func TestYamlConfigFromFileEnvInterpolation_String(t *testing.T) { + t.Setenv("DOLT_TEST_SQLSERVER_HOST", "127.0.0.1") + + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + host: ${DOLT_TEST_SQLSERVER_HOST} +`), 0o644) + require.NoError(t, err) + + cfg, err := YamlConfigFromFile(fs, "config.yaml") + require.NoError(t, err) + require.Equal(t, "127.0.0.1", cfg.Host()) +} + +func TestYamlConfigFromFileEnvInterpolation_Int(t *testing.T) { + t.Setenv("DOLT_TEST_SQLSERVER_PORT", "15200") + + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + port: ${DOLT_TEST_SQLSERVER_PORT} +`), 0o644) + require.NoError(t, err) + + cfg, err := YamlConfigFromFile(fs, "config.yaml") + require.NoError(t, err) + require.Equal(t, 15200, cfg.Port()) +} + +func TestYamlConfigFromFileEnvInterpolation_Default(t *testing.T) { + // Empty env vars are treated as missing for interpolation purposes. + t.Setenv("DOLT_TEST_SQLSERVER_PORT", "") + + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + port: ${DOLT_TEST_SQLSERVER_PORT:-15200} +`), 0o644) + require.NoError(t, err) + + cfg, err := YamlConfigFromFile(fs, "config.yaml") + require.NoError(t, err) + require.Equal(t, 15200, cfg.Port()) +} + +func TestYamlConfigFromFileEnvInterpolation_MissingVarErrors(t *testing.T) { + // Empty env vars are treated as missing for interpolation purposes. + t.Setenv("DOLT_TEST_SQLSERVER_MISSING", "") + + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + port: ${DOLT_TEST_SQLSERVER_MISSING} +`), 0o644) + require.NoError(t, err) + + _, err = YamlConfigFromFile(fs, "config.yaml") + require.Error(t, err) + require.Contains(t, err.Error(), "DOLT_TEST_SQLSERVER_MISSING") +} + +func TestYamlConfigFromFileEnvInterpolation_EscapeDollar(t *testing.T) { + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +golden_mysql_conn: "$$dollar" +`), 0o644) + require.NoError(t, err) + + cfg, err := YamlConfigFromFile(fs, "config.yaml") + require.NoError(t, err) + yc, ok := cfg.(*YAMLConfig) + require.True(t, ok) + require.Equal(t, "$dollar", yc.GoldenMysqlConnectionString()) +} + func TestValidateClusterConfig(t *testing.T) { cases := []struct { Name string diff --git a/integration-tests/bats/sql-server-config-env-interpolation.bats b/integration-tests/bats/sql-server-config-env-interpolation.bats new file mode 100644 index 00000000000..94788587b95 --- /dev/null +++ b/integration-tests/bats/sql-server-config-env-interpolation.bats @@ -0,0 +1,169 @@ +#!/usr/bin/env bats +load $BATS_TEST_DIRNAME/helper/common.bash +load $BATS_TEST_DIRNAME/helper/query-server-common.bash + +make_repo() { + mkdir "$1" + cd "$1" + dolt init + cd .. +} + +# wait_for_http_ok +wait_for_http_ok() { + url="$1" + timeout_ms="$2" + end_time=$((SECONDS+($timeout_ms/1000))) + while [ $SECONDS -lt $end_time ]; do + if curl -fsS "$url" >/dev/null 2>&1; then + return 0 + fi + sleep 1 + done + return 1 +} + +setup() { + skiponwindows "tests are flaky on Windows" + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "This test tests remote connections directly, SQL_ENGINE is not needed." + fi + setup_no_dolt_init + make_repo repo1 +} + +teardown() { + stop_sql_server 1 && sleep 0.5 + rm -rf "$BATS_TMPDIR/sql-server-config-env-interpolation-test$$" + teardown_common +} + +# bats test_tags=no_lambda +@test "sql-server-config-env-interpolation: required env var expands listener.port" { + cd repo1 + + SQL_PORT=$(definePORT) + + # Use a single-quoted heredoc so bash does not expand ${...} + cat > config.yml <<'EOF' +listener: + host: "0.0.0.0" + port: ${DOLT_TEST_SQLSERVER_PORT} +EOF + + if [ "$IS_WINDOWS" == true ]; then + DOLT_TEST_SQLSERVER_PORT=$SQL_PORT PORT=$SQL_PORT dolt sql-server --config ./config.yml & + else + DOLT_TEST_SQLSERVER_PORT=$SQL_PORT PORT=$SQL_PORT dolt sql-server --config ./config.yml --socket "dolt.$SQL_PORT.sock" & + fi + SERVER_PID=$! + + run wait_for_connection "$SQL_PORT" 8500 + [ $status -eq 0 ] +} + +# bats test_tags=no_lambda +@test "sql-server-config-env-interpolation: default expression uses default when unset/empty" { + cd repo1 + + SQL_PORT=$(definePORT) + unset DOLT_TEST_SQLSERVER_PORT + + # Leave ${DOLT_TEST_SQLSERVER_PORT:-} unexpanded for Dolt, but expand $SQL_PORT here. + cat > config.yml < config.yml <<'EOF' +listener: + host: "0.0.0.0" + port: ${DOLT_TEST_SQLSERVER_MISSING} +EOF + + run dolt sql-server --config ./config.yml + [ $status -ne 0 ] + log_output_has "Failed to interpolate environment variables in yaml file" + log_output_has "DOLT_TEST_SQLSERVER_MISSING" +} + +# bats test_tags=no_lambda +@test "sql-server-config-env-interpolation: dollar escaping and env composition works (metrics labels)" { + cd repo1 + + SQL_PORT=$(definePORT) + METRICS_PORT=$(definePORT) + export DOLT_TEST_LABEL="foo" + + # We want the YAML to contain $$${DOLT_TEST_LABEL} literally: + # - $$ becomes a literal '$' + # - ${DOLT_TEST_LABEL} expands to "foo" + # => final label value should be "$foo" + cat > config.yml < "$BATS_TMPDIR/metrics_env_interp_$$.out" 2>/dev/null || true + if grep -F "$expectedLabel" "$BATS_TMPDIR/metrics_env_interp_$$.out" >/dev/null 2>&1; then + found=1 + break + fi + sleep 1 + done + + [ "$found" -eq 1 ] +} + +# Tests to add: +# - ${VAR} expands into typed fields (listener.port) and server starts +# - ${VAR:-default} uses default when unset/empty +# - missing ${VAR} fails with clear error +# - $$ escaping and $$${VAR} composition (verify via metrics labels and /metrics scrape) + From 1fa04b5f9a11f54d7bb85a836c80977ee4f7dc2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Tue, 6 Jan 2026 09:41:57 -0800 Subject: [PATCH 2/5] /go/libraries/doltcore/servercfg/env_interpolate.go: refactor --- .../doltcore/servercfg/env_interpolate.go | 95 ++++++++++++++----- 1 file changed, 70 insertions(+), 25 deletions(-) diff --git a/go/libraries/doltcore/servercfg/env_interpolate.go b/go/libraries/doltcore/servercfg/env_interpolate.go index 65a43e19a53..d6c6848f12c 100644 --- a/go/libraries/doltcore/servercfg/env_interpolate.go +++ b/go/libraries/doltcore/servercfg/env_interpolate.go @@ -1,3 +1,17 @@ +// Copyright 2025 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package servercfg import ( @@ -9,6 +23,13 @@ import ( // Note that an env var may exist but be empty. type envLookupFunc func(string) (string, bool) +type envPlaceholder struct { + varName string + def []byte + hasDefault bool + closingBrace int +} + // interpolateEnv expands environment variable placeholders in |data|. // // Supported syntax: @@ -26,7 +47,8 @@ func interpolateEnv(data []byte, lookup envLookupFunc) ([]byte, error) { out := make([]byte, 0, len(data)) for i := 0; i < len(data); i++ { - if data[i] != '$' { + b := data[i] + if b != '$' { out = append(out, data[i]) continue } @@ -45,41 +67,65 @@ func interpolateEnv(data []byte, lookup envLookupFunc) ([]byte, error) { continue } - // Find closing brace. - start := i + 2 // after ${ - j := start - for ; j < len(data) && data[j] != '}'; j++ { - } - if j >= len(data) { - return nil, fmt.Errorf("unterminated environment placeholder starting at byte %d", i) - } - - expr := data[start:j] - varName, def, hasDefault, err := parseEnvExpr(expr) + ph, err := parseEnvPlaceholder(data, i) if err != nil { return nil, err } - val, ok := lookup(varName) - if ok && val != "" { - out = append(out, []byte(val)...) - } else if hasDefault { - expandedDef, err := interpolateEnv(def, lookup) - if err != nil { - return nil, err - } - out = append(out, expandedDef...) - } else { - return nil, fmt.Errorf("environment variable %q is not set", varName) + out, err = appendEnvExpansion(out, lookup, ph) + if err != nil { + return nil, err } // Skip to closing brace. - i = j + i = ph.closingBrace } return out, nil } +func parseEnvPlaceholder(data []byte, dollarIdx int) (envPlaceholder, error) { + // data[dollarIdx] == '$' and data[dollarIdx+1] == '{' expected. + start := dollarIdx + 2 // after ${ + + closingBrace := start + for ; closingBrace < len(data) && data[closingBrace] != '}'; closingBrace++ { + } + if closingBrace >= len(data) { + return envPlaceholder{}, fmt.Errorf("unterminated environment placeholder starting at byte %d", dollarIdx) + } + + expr := data[start:closingBrace] + varName, def, hasDefault, err := parseEnvExpr(expr) + if err != nil { + return envPlaceholder{}, err + } + + return envPlaceholder{ + varName: varName, + def: def, + hasDefault: hasDefault, + closingBrace: closingBrace, + }, nil +} + +func appendEnvExpansion(out []byte, lookup envLookupFunc, ph envPlaceholder) ([]byte, error) { + val, ok := lookup(ph.varName) + if ok && val != "" { + return append(out, []byte(val)...), nil + } + + if ph.hasDefault { + expandedDef, err := interpolateEnv(ph.def, lookup) + if err != nil { + return nil, err + } + return append(out, expandedDef...), nil + } + + return nil, fmt.Errorf("environment variable %q is not set", ph.varName) +} + func parseEnvExpr(expr []byte) (varName string, def []byte, hasDefault bool, err error) { // Split on the first occurrence of ":-" varPart := expr @@ -121,4 +167,3 @@ func isValidEnvVarName(b []byte) bool { } return true } - From f88ac3dc1af13272e62cadfa93ac17f4fc5eab00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Tue, 6 Jan 2026 09:49:19 -0800 Subject: [PATCH 3/5] /integration-tests/bats/sql-server-config-env-interpolation.bats: remove comments --- .../bats/sql-server-config-env-interpolation.bats | 6 ------ 1 file changed, 6 deletions(-) diff --git a/integration-tests/bats/sql-server-config-env-interpolation.bats b/integration-tests/bats/sql-server-config-env-interpolation.bats index 94788587b95..b4d7e814f1c 100644 --- a/integration-tests/bats/sql-server-config-env-interpolation.bats +++ b/integration-tests/bats/sql-server-config-env-interpolation.bats @@ -161,9 +161,3 @@ EOF [ "$found" -eq 1 ] } -# Tests to add: -# - ${VAR} expands into typed fields (listener.port) and server starts -# - ${VAR:-default} uses default when unset/empty -# - missing ${VAR} fails with clear error -# - $$ escaping and $$${VAR} composition (verify via metrics labels and /metrics scrape) - From e16c81c8d405c05e779d8ea636334beaceb10d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Tue, 6 Jan 2026 12:17:15 -0800 Subject: [PATCH 4/5] /go/libraries/doltcore/servercfg/env_interpolate.go: remove env lookup func --- .../doltcore/servercfg/env_interpolate.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/go/libraries/doltcore/servercfg/env_interpolate.go b/go/libraries/doltcore/servercfg/env_interpolate.go index d6c6848f12c..5d30746d483 100644 --- a/go/libraries/doltcore/servercfg/env_interpolate.go +++ b/go/libraries/doltcore/servercfg/env_interpolate.go @@ -19,10 +19,6 @@ import ( "os" ) -// envLookupFunc returns (value, true) if the env var exists. -// Note that an env var may exist but be empty. -type envLookupFunc func(string) (string, bool) - type envPlaceholder struct { varName string def []byte @@ -40,11 +36,7 @@ type envPlaceholder struct { // Notes: // - Expansion is applied to the input text only (env var values are inserted literally). // - Default expressions are expanded (i.e. nested placeholders inside defaults are supported). -func interpolateEnv(data []byte, lookup envLookupFunc) ([]byte, error) { - if lookup == nil { - lookup = os.LookupEnv - } - +func interpolateEnv(data []byte) ([]byte, error) { out := make([]byte, 0, len(data)) for i := 0; i < len(data); i++ { b := data[i] @@ -72,7 +64,7 @@ func interpolateEnv(data []byte, lookup envLookupFunc) ([]byte, error) { return nil, err } - out, err = appendEnvExpansion(out, lookup, ph) + out, err = appendEnvExpansion(out, ph) if err != nil { return nil, err } @@ -109,14 +101,14 @@ func parseEnvPlaceholder(data []byte, dollarIdx int) (envPlaceholder, error) { }, nil } -func appendEnvExpansion(out []byte, lookup envLookupFunc, ph envPlaceholder) ([]byte, error) { - val, ok := lookup(ph.varName) +func appendEnvExpansion(out []byte, ph envPlaceholder) ([]byte, error) { + val, ok := os.LookupEnv(ph.varName) if ok && val != "" { return append(out, []byte(val)...), nil } if ph.hasDefault { - expandedDef, err := interpolateEnv(ph.def, lookup) + expandedDef, err := interpolateEnv(ph.def) if err != nil { return nil, err } From d9376a3caeb6233515779433f2e846418f307884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?coffeegoddd=E2=98=95=EF=B8=8F=E2=9C=A8?= Date: Tue, 6 Jan 2026 12:53:00 -0800 Subject: [PATCH 5/5] /{go,integration-tests}: pr feedback --- go/cmd/dolt/commands/sqlserver/sqlserver.go | 1 - .../doltcore/servercfg/env_interpolate.go | 178 ++++++++++-------- .../doltcore/servercfg/yaml_config.go | 2 +- .../doltcore/servercfg/yaml_config_test.go | 52 +++-- .../sql-server-config-env-interpolation.bats | 22 +-- 5 files changed, 153 insertions(+), 102 deletions(-) diff --git a/go/cmd/dolt/commands/sqlserver/sqlserver.go b/go/cmd/dolt/commands/sqlserver/sqlserver.go index 10864228970..0315b042ac1 100644 --- a/go/cmd/dolt/commands/sqlserver/sqlserver.go +++ b/go/cmd/dolt/commands/sqlserver/sqlserver.go @@ -88,7 +88,6 @@ ENVIRONMENT VARIABLE INTERPOLATION: SQL server yaml configs support environment variable interpolation: ${VAR} Expands to the value of VAR (error if VAR is unset or empty) - ${VAR:-default} Expands to VAR if set and non-empty; otherwise expands to default $$ Escapes to a literal '$' Notes: diff --git a/go/libraries/doltcore/servercfg/env_interpolate.go b/go/libraries/doltcore/servercfg/env_interpolate.go index 5d30746d483..18a9a7b9b45 100644 --- a/go/libraries/doltcore/servercfg/env_interpolate.go +++ b/go/libraries/doltcore/servercfg/env_interpolate.go @@ -15,62 +15,64 @@ package servercfg import ( + "bytes" "fmt" "os" + "regexp" ) type envPlaceholder struct { varName string - def []byte - hasDefault bool closingBrace int } // interpolateEnv expands environment variable placeholders in |data|. // // Supported syntax: -// - ${VAR} : expands to VAR's value; error if unset or empty -// - ${VAR:-default} : expands to VAR's value if set and non-empty; otherwise to default +// - ${VAR} : expands to VAR's value; error if VAR is unset or empty // - $$ : escapes to a literal '$' // // Notes: // - Expansion is applied to the input text only (env var values are inserted literally). -// - Default expressions are expanded (i.e. nested placeholders inside defaults are supported). func interpolateEnv(data []byte) ([]byte, error) { out := make([]byte, 0, len(data)) - for i := 0; i < len(data); i++ { - b := data[i] - if b != '$' { - out = append(out, data[i]) - continue + for i := 0; i < len(data); { + relDollar := bytes.IndexByte(data[i:], '$') + if relDollar == -1 { + out = append(out, data[i:]...) + break } + dollarIdx := i + relDollar + out = append(out, data[i:dollarIdx]...) + // Escape sequence: $$ -> $ - if i+1 < len(data) && data[i+1] == '$' { + if dollarIdx+1 < len(data) && data[dollarIdx+1] == '$' { out = append(out, '$') - i++ + i = dollarIdx + 2 continue } // Placeholder: ${...} - if i+1 >= len(data) || data[i+1] != '{' { - // Leave stray '$' untouched - out = append(out, '$') - continue - } + if dollarIdx+1 < len(data) && data[dollarIdx+1] == '{' { + ph, err := parseEnvPlaceholder(data, dollarIdx) + if err != nil { + return nil, err + } - ph, err := parseEnvPlaceholder(data, i) - if err != nil { - return nil, err - } + out, err = appendEnvExpansion(out, ph) + if err != nil { + return nil, err + } - out, err = appendEnvExpansion(out, ph) - if err != nil { - return nil, err + // Continue after the closing brace. + i = ph.closingBrace + 1 + continue } - // Skip to closing brace. - i = ph.closingBrace + // Leave stray '$' untouched + out = append(out, '$') + i = dollarIdx + 1 } return out, nil @@ -80,82 +82,110 @@ func parseEnvPlaceholder(data []byte, dollarIdx int) (envPlaceholder, error) { // data[dollarIdx] == '$' and data[dollarIdx+1] == '{' expected. start := dollarIdx + 2 // after ${ - closingBrace := start - for ; closingBrace < len(data) && data[closingBrace] != '}'; closingBrace++ { + rest := data[start:] + + relBrace := bytes.IndexByte(rest, '}') + relNL := bytes.IndexByte(rest, '\n') + relCR := bytes.IndexByte(rest, '\r') + + relLineEnd := minPositive(relNL, relCR) + if relLineEnd != -1 && (relBrace == -1 || relLineEnd < relBrace) { + // Placeholders must be on a single line. This prevents bad/missing braces from + // consuming the rest of the file and producing giant error messages. + return envPlaceholder{}, envErrorAt(data, dollarIdx, "unterminated environment placeholder") } - if closingBrace >= len(data) { - return envPlaceholder{}, fmt.Errorf("unterminated environment placeholder starting at byte %d", dollarIdx) + + if relBrace == -1 { + return envPlaceholder{}, envErrorAt(data, dollarIdx, "unterminated environment placeholder") } + closingBrace := start + relBrace + expr := data[start:closingBrace] - varName, def, hasDefault, err := parseEnvExpr(expr) + varName, err := parseEnvExpr(expr) if err != nil { - return envPlaceholder{}, err + return envPlaceholder{}, envErrorAt(data, dollarIdx, err.Error()) } return envPlaceholder{ varName: varName, - def: def, - hasDefault: hasDefault, closingBrace: closingBrace, }, nil } -func appendEnvExpansion(out []byte, ph envPlaceholder) ([]byte, error) { - val, ok := os.LookupEnv(ph.varName) - if ok && val != "" { - return append(out, []byte(val)...), nil +func minPositive(a, b int) int { + if a == -1 { + return b } - - if ph.hasDefault { - expandedDef, err := interpolateEnv(ph.def) - if err != nil { - return nil, err - } - return append(out, expandedDef...), nil + if b == -1 { + return a + } + if a < b { + return a } + return b +} - return nil, fmt.Errorf("environment variable %q is not set", ph.varName) +func appendEnvExpansion(out []byte, ph envPlaceholder) ([]byte, error) { + val, ok := os.LookupEnv(ph.varName) + if !ok || val == "" { + return nil, fmt.Errorf("environment variable %q is not set or empty", ph.varName) + } + return append(out, []byte(val)...), nil } -func parseEnvExpr(expr []byte) (varName string, def []byte, hasDefault bool, err error) { - // Split on the first occurrence of ":-" - varPart := expr - var defPart []byte - for k := 0; k+1 < len(expr); k++ { - if expr[k] == ':' && expr[k+1] == '-' { - varPart = expr[:k] - defPart = expr[k+2:] - hasDefault = true - break - } +func parseEnvExpr(expr []byte) (varName string, err error) { + // Default expressions are intentionally unsupported to avoid silently masking misconfiguration. + if bytes.Contains(expr, []byte(":-")) { + return "", fmt.Errorf("environment variable default expressions are not supported (found %q)", string(expr)) } - if len(varPart) == 0 { - return "", nil, false, fmt.Errorf("invalid environment placeholder: empty variable name") + if len(expr) == 0 { + return "", fmt.Errorf("invalid environment placeholder: empty variable name") } - if !isValidEnvVarName(varPart) { - return "", nil, false, fmt.Errorf("invalid environment variable name %q", string(varPart)) + if !isValidEnvVarName(expr) { + return "", fmt.Errorf("invalid environment variable name %q", string(expr)) } - return string(varPart), defPart, hasDefault, nil + return string(expr), nil } +var envVarRe = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + func isValidEnvVarName(b []byte) bool { - if len(b) == 0 { - return false + return envVarRe.Match(b) +} + +func envErrorAt(data []byte, idx int, msg string) error { + line, col := lineAndColAt(data, idx) + return fmt.Errorf("%s (line %d, column %d)", msg, line, col) +} + +func lineAndColAt(data []byte, idx int) (line int, col int) { + if idx < 0 { + idx = 0 } - for i := range b { - c := b[i] - if i == 0 { - if !((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_') { - return false - } - } else { - if !((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '_') { - return false + if idx > len(data) { + idx = len(data) + } + + line = 1 + col = 1 + for i := 0; i < idx; i++ { + switch data[i] { + case '\n': + line++ + col = 1 + case '\r': + line++ + col = 1 + // Handle CRLF as a single newline. + if i+1 < idx && data[i+1] == '\n' { + i++ } + default: + col++ } } - return true + return line, col } diff --git a/go/libraries/doltcore/servercfg/yaml_config.go b/go/libraries/doltcore/servercfg/yaml_config.go index 0d5b74f3ced..9f5aa5f2493 100644 --- a/go/libraries/doltcore/servercfg/yaml_config.go +++ b/go/libraries/doltcore/servercfg/yaml_config.go @@ -194,7 +194,7 @@ func YamlConfigFromFile(fs filesys.Filesys, path string) (ServerConfig, error) { return nil, fmt.Errorf("Failed to read file '%s'. Error: %s", path, err.Error()) } - data, err = interpolateEnv(data, nil) + data, err = interpolateEnv(data) if err != nil { return nil, fmt.Errorf("Failed to interpolate environment variables in yaml file '%s'. Error: %s", path, err.Error()) } diff --git a/go/libraries/doltcore/servercfg/yaml_config_test.go b/go/libraries/doltcore/servercfg/yaml_config_test.go index e8b8c717580..87ff3b90276 100644 --- a/go/libraries/doltcore/servercfg/yaml_config_test.go +++ b/go/libraries/doltcore/servercfg/yaml_config_test.go @@ -238,36 +238,46 @@ listener: require.Equal(t, 15200, cfg.Port()) } -func TestYamlConfigFromFileEnvInterpolation_Default(t *testing.T) { - // Empty env vars are treated as missing for interpolation purposes. - t.Setenv("DOLT_TEST_SQLSERVER_PORT", "") +func TestYamlConfigFromFileEnvInterpolation_MissingVarErrors(t *testing.T) { + // Empty env vars result in an interpolation error. + t.Setenv("DOLT_TEST_SQLSERVER_MISSING", "") fs := filesys.EmptyInMemFS("/") err := fs.WriteFile("config.yaml", []byte(` listener: - port: ${DOLT_TEST_SQLSERVER_PORT:-15200} + port: ${DOLT_TEST_SQLSERVER_MISSING} `), 0o644) require.NoError(t, err) - cfg, err := YamlConfigFromFile(fs, "config.yaml") - require.NoError(t, err) - require.Equal(t, 15200, cfg.Port()) + _, err = YamlConfigFromFile(fs, "config.yaml") + require.Error(t, err) + require.Contains(t, err.Error(), "DOLT_TEST_SQLSERVER_MISSING") } -func TestYamlConfigFromFileEnvInterpolation_MissingVarErrors(t *testing.T) { - // Empty env vars are treated as missing for interpolation purposes. - t.Setenv("DOLT_TEST_SQLSERVER_MISSING", "") +func TestYamlConfigFromFileEnvInterpolation_UnsetVarErrors(t *testing.T) { + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + port: ${DOLT_TEST_SQLSERVER_UNSET} +`), 0o644) + require.NoError(t, err) + _, err = YamlConfigFromFile(fs, "config.yaml") + require.Error(t, err) + require.Contains(t, err.Error(), "DOLT_TEST_SQLSERVER_UNSET") +} + +func TestYamlConfigFromFileEnvInterpolation_DefaultSyntaxErrors(t *testing.T) { fs := filesys.EmptyInMemFS("/") err := fs.WriteFile("config.yaml", []byte(` listener: - port: ${DOLT_TEST_SQLSERVER_MISSING} + port: ${DOLT_TEST_SQLSERVER_PORT:-15200} `), 0o644) require.NoError(t, err) _, err = YamlConfigFromFile(fs, "config.yaml") require.Error(t, err) - require.Contains(t, err.Error(), "DOLT_TEST_SQLSERVER_MISSING") + require.Contains(t, err.Error(), "default expressions") } func TestYamlConfigFromFileEnvInterpolation_EscapeDollar(t *testing.T) { @@ -284,6 +294,24 @@ golden_mysql_conn: "$$dollar" require.Equal(t, "$dollar", yc.GoldenMysqlConnectionString()) } +func TestYamlConfigFromFileEnvInterpolation_UnterminatedStopsAtNewline(t *testing.T) { + fs := filesys.EmptyInMemFS("/") + err := fs.WriteFile("config.yaml", []byte(` +listener: + port: ${DOLT_TEST_SQLSERVER_PORT +behavior: + read_only: true +junk: "}" +`), 0o644) + require.NoError(t, err) + + _, err = YamlConfigFromFile(fs, "config.yaml") + require.Error(t, err) + require.Contains(t, err.Error(), "unterminated environment placeholder") + require.Contains(t, err.Error(), "line 3") + require.Contains(t, err.Error(), "column") +} + func TestValidateClusterConfig(t *testing.T) { cases := []struct { Name string diff --git a/integration-tests/bats/sql-server-config-env-interpolation.bats b/integration-tests/bats/sql-server-config-env-interpolation.bats index b4d7e814f1c..d0209709ade 100644 --- a/integration-tests/bats/sql-server-config-env-interpolation.bats +++ b/integration-tests/bats/sql-server-config-env-interpolation.bats @@ -63,28 +63,22 @@ EOF } # bats test_tags=no_lambda -@test "sql-server-config-env-interpolation: default expression uses default when unset/empty" { +@test "sql-server-config-env-interpolation: default expression syntax fails clearly" { cd repo1 SQL_PORT=$(definePORT) - unset DOLT_TEST_SQLSERVER_PORT - # Leave ${DOLT_TEST_SQLSERVER_PORT:-} unexpanded for Dolt, but expand $SQL_PORT here. - cat > config.yml < config.yml <<'EOF' listener: host: "0.0.0.0" - port: \${DOLT_TEST_SQLSERVER_PORT:-$SQL_PORT} + port: ${DOLT_TEST_SQLSERVER_PORT:-15200} EOF - if [ "$IS_WINDOWS" == true ]; then - PORT=$SQL_PORT dolt sql-server --config ./config.yml & - else - PORT=$SQL_PORT dolt sql-server --config ./config.yml --socket "dolt.$SQL_PORT.sock" & - fi - SERVER_PID=$! - - run wait_for_connection "$SQL_PORT" 8500 - [ $status -eq 0 ] + run dolt sql-server --config ./config.yml + [ $status -ne 0 ] + log_output_has "Failed to interpolate environment variables in yaml file" + log_output_has "default expressions" } # bats test_tags=no_lambda