Copilot commented on code in PR #13078:
URL: https://github.com/apache/apisix/pull/13078#discussion_r2903063610
##########
apisix/core/config_yaml.lua:
##########
@@ -153,7 +153,7 @@ local function update_config(table, conf_version)
return
end
- local ok, err = file.resolve_conf_var(table)
+ local ok, err = file.resolve_conf_var(table, false)
Review Comment:
This disables env-var type conversion for all values in standalone config
reloads. That fixes large numeric IDs, but it also means env vars can no longer
be used to populate fields whose schema requires numbers/booleans (eg upstream
`nodes` weights, timeouts) because substituted values will remain strings and
fail schema checks. Consider narrowing the special-case (eg only skip numeric
conversion for values outside the safe integer range / long digit strings) so
standalone configs can still use env vars for numeric fields.
```suggestion
local ok, err = file.resolve_conf_var(table, true)
```
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,130 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+make stop
+sleep 0.5
+
+# 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:
+ response-rewrite:
+ body: "${{APISIX_CLIENT_ID}}"
+ 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 response body matches the exact large numeric string
+body=$(curl -s -m 5 http://127.0.0.1:9080/test-large-number)
+if [ "$body" != "356002209726529540" ]; then
+ echo "failed: large number env var was not preserved as string, got: $body"
+ exit 1
+fi
Review Comment:
For the new routes, the assertions only compare the response body. If the
request fails or returns an unexpected status (eg 404/502), the body check may
be misleading. Capture and assert the HTTP status code is 200 alongside the
body for /test-large-number and /test-quoted so failures are diagnosed
correctly.
##########
t/cli/test_standalone.sh:
##########
@@ -155,3 +155,130 @@ if [ $expected_config_reloads -ne $actual_config_reloads
]; then
exit 1
fi
echo "passed: apisix.yaml was not reloaded"
+
+make stop
+sleep 0.5
+
+# 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:
+ response-rewrite:
+ body: "${{APISIX_CLIENT_ID}}"
+ 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 response body matches the exact large numeric string
+body=$(curl -s -m 5 http://127.0.0.1:9080/test-large-number)
+if [ "$body" != "356002209726529540" ]; then
+ echo "failed: large number env var was not preserved as string, got: $body"
+ 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:
+ response-rewrite:
+ body: "${{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
+
+body=$(curl -s -m 5 http://127.0.0.1:9080/test-quoted)
+if [ "$body" != "12345" ]; then
+ echo "failed: quoted numeric env var in apisix.yaml was not preserved as
string, got: $body"
+ 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 (boolean)
+echo '
+routes: []
+#END
+' > conf/apisix.yaml
+
+echo '
+apisix:
+ enable_admin: ${{ENABLE_ADMIN}}
+deployment:
+ role: traditional
+ role_traditional:
+ config_provider: yaml
+ etcd:
+ host:
+ - "http://127.0.0.1:2379";
+' > conf/config.yaml
+
+ENABLE_ADMIN=false make init
+ENABLE_ADMIN=false make run
+sleep 0.1
+
+# If type conversion works, enable_admin is boolean false and admin API is
disabled (404)
+# If type conversion fails, enable_admin stays string "false" which is truthy,
admin API is enabled
+code=$(curl -o /dev/null -s -m 5 -w %{http_code}
http://127.0.0.1:9080/apisix/admin/routes)
+if [ "$code" -eq 200 ]; then
+ echo "failed: boolean env var in config.yaml was not converted, admin API
should be disabled"
Review Comment:
The test only fails when the admin endpoint returns 200, but it will
incorrectly pass if APISIX failed to start (curl can return 000) or if another
unexpected status code is returned. Since the intent is to assert admin is
disabled when ENABLE_ADMIN=false, check explicitly for 404 (or the expected
non-admin status) instead of only rejecting 200.
```suggestion
if [ "$code" -ne 404 ]; then
echo "failed: expected admin API to be disabled (404), but got HTTP
status: $code"
```
--
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]
