Baoyuantop commented on code in PR #13686:
URL: https://github.com/apache/apisix/pull/13686#discussion_r4284600625
##########
apisix/cli/file.lua:
##########
@@ -92,12 +93,32 @@ end
-- Substitute env vars in raw text before parsing. YAML parser then infers
-- types naturally: `"${{VAR}}"` stays string, `${{VAR}}` infers from value.
+-- Substitution runs line by line so that full-line comments stay untouched:
+-- a commented-out `# ${{VAR}}` must not fail startup when VAR is unset.
+-- Known limits of this text-level heuristic: an inline trailing comment is
+-- still substituted (`#` may legally appear inside YAML strings), and a
+-- block-scalar content line starting with `#` is skipped like a comment.
local function resolve_conf_var_in_text(text)
- local new_text, _, err = var_sub(text)
- if err then
- return nil, err
+ local parts = {}
+ -- a UTF-8 BOM would hide the `#` of a first-line comment, so detach it
+ if str_sub(text, 1, 3) == "\239\187\191" then
+ parts[1] = str_sub(text, 1, 3)
+ text = str_sub(text, 4)
+ end
+ for line, eol in str_gmatch(text, "([^\n]*)(\n?)") do
Review Comment:
When ${{VAR and }} span lines, var_sub never sees the complete reference.
YAML folds the remaining text into an unresolved value, breaking configuration
that previously loaded.
##########
t/cli/test_standalone.sh:
##########
@@ -660,5 +660,55 @@ fi
echo "passed: missing env var produces clear startup error"
+# test: unset env vars referenced only in commented-out lines must be ignored
+echo '
+apisix:
+ enable_admin: false
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+' > conf/config.yaml
+
+echo '
+# ${{COMMENTED_UNSET_VAR}}
+routes:
+ -
+ uri: /test-commented-var
+ # indented comment: ${{ANOTHER_COMMENTED_UNSET_VAR}}
+ plugins:
+ response-rewrite:
+ body: "commented-ok"
+ status_code: 200
+ upstream:
+ nodes:
+ "127.0.0.1:9091": 1
+ type: roundrobin
+#END
+' > conf/apisix.yaml
+
+unset COMMENTED_UNSET_VAR
+unset ANOTHER_COMMENTED_UNSET_VAR
+make init
+
+if ! make run > output.log 2>&1; then
+ cat output.log
+ echo "failed: make run should ignore env vars in commented-out lines"
+ exit 1
+fi
+sleep 0.1
+
+code=$(curl -o /tmp/response_body -s -m 5 -w %{http_code}
http://127.0.0.1:9080/test-commented-var)
Review Comment:
The test writes the YAML before make run and makes one request. It covers
initial loading, not a file change during runtime as the PR description claims.
##########
apisix/cli/file.lua:
##########
@@ -92,12 +93,32 @@ end
-- Substitute env vars in raw text before parsing. YAML parser then infers
-- types naturally: `"${{VAR}}"` stays string, `${{VAR}}` infers from value.
+-- Substitution runs line by line so that full-line comments stay untouched:
+-- a commented-out `# ${{VAR}}` must not fail startup when VAR is unset.
+-- Known limits of this text-level heuristic: an inline trailing comment is
+-- still substituted (`#` may legally appear inside YAML strings), and a
+-- block-scalar content line starting with `#` is skipped like a comment.
local function resolve_conf_var_in_text(text)
- local new_text, _, err = var_sub(text)
- if err then
- return nil, err
+ local parts = {}
+ -- a UTF-8 BOM would hide the `#` of a first-line comment, so detach it
+ if str_sub(text, 1, 3) == "\239\187\191" then
+ parts[1] = str_sub(text, 1, 3)
+ text = str_sub(text, 4)
+ end
+ for line, eol in str_gmatch(text, "([^\n]*)(\n?)") do
+ if is_empty_yaml_line(line) or not str_find(line, "${{", 1, true) then
Review Comment:
When a block scalar contains a line starting with #, is_empty_yaml_line
skips its environment reference. The loaded value retains ${{VAR}} instead of
the variable's value.
--
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]
