-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support for reading environment variables from yaml configuration files #5244 #6505
Conversation
t/cli/test_yaml_config_variables.sh
Outdated
# check supported environment variables in apisix.yaml | ||
|
||
yaml_config_variables_clean_up() { | ||
git checkout conf/config.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use the original clean up like:
apisix/t/cli/test_serverless.sh
Lines 21 to 25 in 380f762
serverless_clean_up() { | |
clean_up | |
git checkout conf/apisix.yaml | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Please review it again.
@@ -234,6 +237,20 @@ function _M.read_yaml_conf(apisix_home) | |||
end | |||
end | |||
|
|||
if default_conf.apisix.config_center == "yaml" then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in apisix/core/config_yaml.lua?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I wanted to put this in apisix/core/config_yaml.lua, but because the file loads ngx variables, it needs to be loaded in the openresty environment. Can't run in CLI mode, so we can't put this in apisix/core/config_yaml.lua
t/cli/test_yaml_config_variables.sh
Outdated
@@ -0,0 +1,68 @@ | |||
#!/usr/bin/env bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename it to test_standalone.sh, so it can be used in other places later.
@spacewander @tokers Test file renamed.Please review it again. |
Co-authored-by: tzssangglass <tzssangglass@gmail.com>
What this PR does / why we need it:
Support for reading environment variables from yaml configuration files.
resolve #5244
Pre-submission checklist: