Repository navigation
Conversation
|
Hi @chennaji9. Thanks for your PR. I'm waiting for a OpenAtom-Linyaps member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
loadRuntimeConfigFromDir ignored the error_code from the config.json exists() probe and the config.d is_directory() probe, silently reporting "no configuration" when the check itself failed (e.g. symlink loop or missing search permission). Check the error codes and return an error, keeping the legitimate no-config case as success with nullopt, and add unit tests. Signed-off-by: 陈那几 <2078634206@qq.com>
exists()/is_directory() report an error_code even for regular ENOENT on POSIX, so checking ec directly turned every absent config.json/config.d into a load error. Probe with status() instead: file_type::none means the status could not be determined (ELOOP, EACCES, ...) and is reported as an error, while not_found keeps meaning absent. Signed-off-by: 陈那几 <2078634206@qq.com>
ae0b0cd to
e062d08
Compare
What does this PR do?
loadRuntimeConfigFromDirprobes forconfig.jsonand theconfig.ddrop-in directory but ignores the error codes of those probes:When the query itself fails — e.g.
config.jsonis a symlink loop (ELOOP) or a parent lost search permission (EACCES) — the probe returns false and the failure is silently treated as "no configuration": the caller gets success with an empty optional instead of an error.Probe with
status()and distinguish real failures from absence:file_type::none(status could not be determined) is reported as aLINGLONG_ERRwith the path andec.message();not_foundkeeps meaning legitimately absent (POSIX reports an error_code for ENOENT as well, so checkingecdirectly is not a valid absence test — this is why the fix keys off the status type instead).Validation
Three new unit tests (using the public
loadRuntimeConfig(configDirs, appId, instance)overload):LoadRuntimeConfigWithSymlinkLoop—config.jsonas a self-referential symlink returns an error;LoadRuntimeConfigWithConfigDSymlinkLoop—config.das a self-referential symlink returns an error;LoadRuntimeConfigWithoutConfig— empty config dir still returns success with an empty optional (regression guard).ELOOP is privilege-independent, so the tests behave the same for any user. Full result:
ctest -R RuntimeConfig→ 29/29 Passed (26 pre-existing + 3 new);ll-testsbuilds cleanly.🤖 Generated with Claude Code