Copilot commented on code in PR #13078:
URL: https://github.com/apache/apisix/pull/13078#discussion_r2895101022
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,120 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+
+# test: environment variable with large number should be preserved as string
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-large-number
+ plugins:
+ openid-connect:
+ client_id: "${{APISIX_CLIENT_ID}}"
+ client_secret: "secret"
+ discovery: "http://example.com/.well-known/openid-configuration";
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+# Test with large number that exceeds Lua double precision
+APISIX_CLIENT_ID="356002209726529540" make init
+
+if ! APISIX_CLIENT_ID="356002209726529540" make run > output.log 2>&1; then
+ cat output.log
+ echo "failed: large number in env var should not cause type conversion
error"
+ exit 1
+fi
+
+sleep 0.1
+
+# Verify the service is running (should not have validation errors)
+code=$(curl -o /dev/null -s -m 5 -w %{http_code}
http://127.0.0.1:9080/test-large-number)
+if [ $code -eq 500 ]; then
+ echo "failed: large number env var was converted to scientific notation"
+ exit 1
+fi
+
+make stop
+sleep 0.5
+
+echo "passed: large number in env var preserved as string in apisix.yaml"
+
+# test: quoted numeric env vars in apisix.yaml should remain strings
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-quoted
+ plugins:
+ proxy-rewrite:
+ headers:
+ X-Custom-ID: "${{NUMERIC_ID}}"
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+NUMERIC_ID="12345" make init
+NUMERIC_ID="12345" make run
+sleep 0.1
+
+code=$(curl -s -H "Host: test.com" http://127.0.0.1:9080/test-quoted -o
/dev/null -w %{http_code})
+if [ ! $code -eq 404 ] && [ ! $code -eq 200 ]; then
+ echo "failed: quoted numeric env var in apisix.yaml should work"
+ exit 1
+fi
+
+make stop
+sleep 0.5
+
+echo "passed: quoted numeric env var preserved as string in apisix.yaml"
+
+# test: config.yaml should still support type conversion
+echo '
+routes: []
+#END
+' > conf/apisix.yaml
+
+echo '
+apisix:
+ node_listen: ${{NODE_PORT}}
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+NODE_PORT=9080 make init
+
+if ! grep "listen 0.0.0.0:9080" conf/nginx.conf > /dev/null; then
+ echo "failed: numeric env var in config.yaml should be converted to number"
+ exit 1
+fi
+
+echo "passed: config.yaml still converts numeric env vars correctly"
+
Review Comment:
Grepping for `listen 0.0.0.0:9080` doesn't prove type conversion is still
happening, since both a string and a number can render identically in
`nginx.conf`. To validate conversion is preserved for `config.yaml`, assert a
type-sensitive field (e.g., `apisix.enable_admin: ${{ENABLE_ADMIN}}` with
`ENABLE_ADMIN=false` and verify the admin API is disabled) or otherwise check a
value that would fail schema validation if left as a string.
```suggestion
enable_admin: ${{ENABLE_ADMIN}}
deployment:
role: data_plane
role_data_plane:
config_provider: yaml
' > conf/config.yaml
ENABLE_ADMIN=false make init
ENABLE_ADMIN=false make run
sleep 1
if curl -s -o /dev/null http://127.0.0.1:9180/apisix/admin/routes; then
echo "failed: admin API should be disabled when ENABLE_ADMIN=false in
config.yaml"
make stop
exit 1
fi
make stop
echo "passed: config.yaml still converts boolean env vars correctly"
```
##########
apisix/cli/file.lua:
##########
@@ -90,7 +90,10 @@ local function var_sub(val)
end
-local function resolve_conf_var(conf)
+local function resolve_conf_var(conf, enable_type_conversion)
+ if enable_type_conversion == nil then
+ enable_type_conversion = true
Review Comment:
`resolve_conf_var` now defaults `enable_type_conversion` to `true`, but
`apisix/core/config_yaml.lua` calls `file.resolve_conf_var(table)` when
reloading `apisix.yaml` in standalone mode. That means type conversion is still
enabled at runtime for `apisix.yaml`, so the large-number/scientific-notation
issue will persist unless that call site is updated to pass `false` (or
otherwise disable conversion for `apisix.yaml`).
```suggestion
enable_type_conversion = false
```
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,120 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+
+# test: environment variable with large number should be preserved as string
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-large-number
+ plugins:
+ openid-connect:
+ client_id: "${{APISIX_CLIENT_ID}}"
+ client_secret: "secret"
+ discovery: "http://example.com/.well-known/openid-configuration";
Review Comment:
This test uses `openid-connect` with a placeholder `discovery` URL and then
treats an HTTP 500 as evidence of type conversion. That can be flaky because
500s may come from plugin runtime behavior (failed discovery fetch, etc.)
rather than config type conversion. Consider switching to a plugin with no
external dependencies (e.g., `response-rewrite` with `body:
${{APISIX_CLIENT_ID}}`) and assert the response body matches the exact large
numeric string (not scientific notation).
```suggestion
response-rewrite:
body: "${{APISIX_CLIENT_ID}}"
```
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,120 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+
+# test: environment variable with large number should be preserved as string
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-large-number
+ plugins:
+ openid-connect:
+ client_id: "${{APISIX_CLIENT_ID}}"
+ client_secret: "secret"
+ discovery: "http://example.com/.well-known/openid-configuration";
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+# Test with large number that exceeds Lua double precision
+APISIX_CLIENT_ID="356002209726529540" make init
+
+if ! APISIX_CLIENT_ID="356002209726529540" make run > output.log 2>&1; then
+ cat output.log
+ echo "failed: large number in env var should not cause type conversion
error"
+ exit 1
+fi
+
+sleep 0.1
+
+# Verify the service is running (should not have validation errors)
+code=$(curl -o /dev/null -s -m 5 -w %{http_code}
http://127.0.0.1:9080/test-large-number)
+if [ $code -eq 500 ]; then
+ echo "failed: large number env var was converted to scientific notation"
Review Comment:
The curl assertion only fails on status `500`, so it will still pass if
APISIX isn't reachable (`000`) or if the route wasn't loaded due to schema
validation (`404`). To make the test reliable, assert a known-success status
(typically `200`) and/or explicitly fail if curl returns `000`/`404`.
```suggestion
if [ "$code" != "200" ]; then
echo "failed: expected 200 from /test-large-number, got $code (large
number env var may have caused an error)"
```
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,120 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+
+# test: environment variable with large number should be preserved as string
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-large-number
+ plugins:
+ openid-connect:
+ client_id: "${{APISIX_CLIENT_ID}}"
+ client_secret: "secret"
+ discovery: "http://example.com/.well-known/openid-configuration";
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+# Test with large number that exceeds Lua double precision
+APISIX_CLIENT_ID="356002209726529540" make init
+
+if ! APISIX_CLIENT_ID="356002209726529540" make run > output.log 2>&1; then
+ cat output.log
+ echo "failed: large number in env var should not cause type conversion
error"
+ exit 1
+fi
+
+sleep 0.1
+
+# Verify the service is running (should not have validation errors)
+code=$(curl -o /dev/null -s -m 5 -w %{http_code}
http://127.0.0.1:9080/test-large-number)
+if [ $code -eq 500 ]; then
+ echo "failed: large number env var was converted to scientific notation"
+ exit 1
+fi
+
+make stop
+sleep 0.5
+
+echo "passed: large number in env var preserved as string in apisix.yaml"
+
+# test: quoted numeric env vars in apisix.yaml should remain strings
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+routes:
+ -
+ uri: /test-quoted
+ plugins:
+ proxy-rewrite:
+ headers:
+ X-Custom-ID: "${{NUMERIC_ID}}"
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+NUMERIC_ID="12345" make init
+NUMERIC_ID="12345" make run
+sleep 0.1
+
+code=$(curl -s -H "Host: test.com" http://127.0.0.1:9080/test-quoted -o
/dev/null -w %{http_code})
+if [ ! $code -eq 404 ] && [ ! $code -eq 200 ]; then
Review Comment:
This check allows `404`, which can mask a failure to load the route (e.g.,
schema validation failing because the env var was converted to a number). To
actually validate behavior, assert `200` for a known-working upstream, or check
the error log for the absence of schema/type-validation errors.
```suggestion
if [ "$code" -ne 200 ]; then
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
