fix(docker): support valid YAML variations in standalone mode#12949
fix(docker): support valid YAML variations in standalone mode#12949Baoyuantop merged 10 commits intoapache:masterfrom
Conversation
dosubot
bot
added
size:L
janiussyafiq
changed the title
| if ! grep -q 'config_provider: yaml' "${PREFIX}/conf/config.yaml"; then | ||
| echo "Error: ${PREFIX}/conf/config.yaml does not contain 'config_provider: yaml'. Config provider must be set to 'yaml' for standalone mode." | ||
| if ! grep -E -q '["'\'']?config_provider["'\'']?:\s*["'\'']?yaml["'\'']?' "$CONF_FILE"; then | ||
| echo "Error: $CONF_FILE does not contain 'config_provider: yaml'. Config provider must be set to 'yaml' for standalone mode." |
There was a problem hiding this comment.
please add tests that cover the changes you made in this file.
There was a problem hiding this comment.
could you provide more context on what tests are needed since i've written a few? do you mean the $CONF_FILE returning the correct file name?
nic-6443
left a comment
nic-6443
left a comment
There was a problem hiding this comment.
@janiussyafiq The toolchain used for releasing apisix docker image is in the https://github.com/apache/apisix-docker repository. Therefore, after you complete this fix, you also need to adjust https://github.com/apache/apisix-docker/blob/9b3c3d367ab5c73f95e27ee37e82d25b79dd846b/debian/Dockerfile#L67 too.
There was a problem hiding this comment.
you have changed the mode of this file, why?
There was a problem hiding this comment.
might've changed it when i did local testing, will revert it back
t/cli/test_standalone_docker.sh
Outdated
| DOCKER_IMAGE="${DOCKER_IMAGE:-apache/apisix:master-debian-dev}" | ||
| trap standalone EXIT | ||
|
|
||
| make build-on-debian-dev |
There was a problem hiding this comment.
This is going to significantly increase the duration of the CI. We can take one of the following actions to avoid this:
- use prebuilt image (image built from previous push) instead of building image while running the test.
- run this test only on master branch
- but this will require further more changes (e.g: a separate GitHub actions workflow)
- and we won't be able to detect regressions before commit
- run this test on a different GitHub actions workflow and only run them if any of the files related to this feature (ops.lua, docker-entrypoint.sh, etc) change.
There was a problem hiding this comment.
3rd option sounds good to me
There was a problem hiding this comment.
opt 1: won't work, i've discussed with @nic-6443, since the older image still rely on the script that is decided to be removed check_standalone_config.sh, so test will fail
as for opt 2 and 3, can try those solution, i'm more leaning towards opt 3 wdyt?
There was a problem hiding this comment.
will try implementing 3rd option thanks!
dosubot
bot
added
size:XL
4 participants
Description
This PR fixes a bug in the Docker standalone configuration check script (check_standalone_config.sh) where valid YAML configurations would fail validation due to rigid pattern matching. and the case where
APISIX_PROFILEdoes not respected. This PR also included necessary test cases.Which issue(s) this PR fixes:
Fixes #12427
Checklist