Skip to content
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

When misconfigured cartridge.cfg fails with dubious error during startup #2169

Closed
RunsFor opened this issue Dec 13, 2023 · 1 comment · Fixed by #2170
Closed

When misconfigured cartridge.cfg fails with dubious error during startup #2169

RunsFor opened this issue Dec 13, 2023 · 1 comment · Fixed by #2170

Comments

@RunsFor
Copy link
Contributor

RunsFor commented Dec 13, 2023

During yet-another-cluster-startup I've ran into an issue when cartridge produced a strange error executing cartridge.cfg function:

LuajitError: ...hov/Workspace/tarantool/cartridge/cartridge/argparse.lua:357: bad argument #1 to 'pairs' (table expected, got string)
fatal error, exiting the event loop

During a brief troubleshooting, I was able to make the shortest reproducer. The problem is within cartridge.argparse module. Here is the repro:

$ echo "default: option" > cfg.yml
$ TARANTOOL_CFG=cfg.yml tarantool -e "require('cartridge.argparse').parse()"
LuajitError: ...hov/Workspace/tarantool/cartridge/cartridge/argparse.lua:357: bad argument #1 to 'pairs' (table expected, got string)
fatal error, exiting the event loop

It turned out, that argparse module can load not only a single file, but a whole directory, if specified, in order to get its arguments. But sometimes it is possible for an ops team to accidentally (or intentionally) put inside this directory a config from a different service, which accepts key-value(string) configuration.

In this scenario I expect argparse.parse() either to get only its own options from a config directory tree, or at least explicitly say that it encountered invalid configuration such as validateArgsSchemaError (better pointing at the file and section location) and not fail with "mysterious" error.

I've reproduced it on the latest cartridge version at master branch:

$ git describe
2.8.4-13-gbe208ad6
@RunsFor RunsFor changed the title Cartridge fails with dubious error during startup When misconfigured cartridge.cfg fails with dubious error during startup Dec 13, 2023
@RunsFor RunsFor changed the title When misconfigured cartridge.cfg fails with dubious error during startup When misconfigured cartridge.cfg fails with dubious error during startup Dec 13, 2023
@RunsFor
Copy link
Contributor Author

RunsFor commented Dec 13, 2023

Sorry, apparently, I did not fully describe a problem with argument parsing. In the description above I've provided an example, which highlights a problem in argparse.lua module at line 357. But it turned out, there is another similar problem on line 283, when argparse parsing a directory. The execution flow is a bit different, resulting with the same error. Here is the reproducer:

$ mkdir -p cfg
$ echo "python: option" > cfg/python.yml
$ echo "cartridge: {a: b}" > cfg/tarantool.yml
$ TARANTOOL_CFG=cfg tarantool -e "require('cartridge.argparse').parse()"
LuajitError: ...hov/Workspace/tarantool/cartridge/cartridge/argparse.lua:283: bad argument #1 to 'pairs' (table expected, got string)
fatal error, exiting the event loop

Notice there is the same error, but on the different line of code.

But here is the interesting part. If I "fix" a python section, argparse just ignores the entire section and no error produced:

$ echo "python: {c: d}" > cfg/python.yml
$ TARANTOOL_CFG=cfg tarantool -e "require('log').info(require('cartridge.argparse').parse())"
{"cfg":"cfg\/","app_name":"cartridge","a":"b"}

In the result output there is no {"c": "d"} option. So I imaging, the correct solution would be to:

  • return an error if it encounters an invalid section value within expected ones (default, app_name etc..)
  • skip/ignore any sections (valid or invalid) which are not a part of expected list of sections (default, app_name etc..)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant