improve error handling for this kind of situation like __line type #25

New Issue

ungleich-gitea · 2021-11-20T11:24:54Z

ungleich-gitea commented

2021-11-20 11:24:54 +00:00

this is a mix about I don't know what I am doing, but I am learning, and what I get is confusing and frustrating (UX). Why I do strange __line things? Well, I got inspired by the official examples. Reading them you think, why not using whitespaces (and other characters) between that words

If I do:

__line 'quit password authentication (remove)' \
  --file /etc/ssh/sshd_config \
  --regex 'PasswordAuthentication.*no' \
  --state absent

I get a strange error (no matter how much -vvvv I use, it continues to be strange:

ERROR: example.com: ssh -o User=root -o ControlPath=/tmp/tmpo2hzgvyd/s -o ControlMaster=auto -o ControlPersist=2h example.com mkdir -p /var/lib/cdist/object/__line/quit password authentication (remove)/.cdist-qzdiepox/parameter: ['ssh', '-o', 'User=root', '-o', 'ControlPath=/tmp/tmpo2hzgvyd/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', 'example.com', 'mkdir', '-p', '/var/lib/cdist/object/__line/quit password authentication (remove)/.cdist-qzdiepox/parameter']
ERROR: cdist: Failed to configure the following hosts: example.com

this way works, hence, I am suppose that this quit_password_authentication_remove is the custom name of the object id (particular instance of the __line type), I would like to see an error like "object id cannot include whitespaces, ... (or what are the characters that can be included)?"

-__line 'quit password authentication (remove)' \
+__line quit_password_authentication_remove \
  --file /etc/ssh/sshd_config \
  --regex 'PasswordAuthentication.*no' \
  --state absent

this is a mix about I don't know what I am doing, but I am learning, and what I get is confusing and frustrating (UX). Why I do strange `__line` things? Well, I got inspired by the [official examples](https://www.cdi.st/manual/latest/man7/cdist-type__line.html#examples). Reading them you think, why not using whitespaces (and other characters) between that words If I do: ``` __line 'quit password authentication (remove)' \ --file /etc/ssh/sshd_config \ --regex 'PasswordAuthentication.*no' \ --state absent ``` I get a strange error (no matter how much `-vvvv` I use, it continues to be strange: ``` ERROR: example.com: ssh -o User=root -o ControlPath=/tmp/tmpo2hzgvyd/s -o ControlMaster=auto -o ControlPersist=2h example.com mkdir -p /var/lib/cdist/object/__line/quit password authentication (remove)/.cdist-qzdiepox/parameter: ['ssh', '-o', 'User=root', '-o', 'ControlPath=/tmp/tmpo2hzgvyd/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', 'example.com', 'mkdir', '-p', '/var/lib/cdist/object/__line/quit password authentication (remove)/.cdist-qzdiepox/parameter'] ERROR: cdist: Failed to configure the following hosts: example.com ``` this way works, hence, I am suppose that this `quit_password_authentication_remove` is the custom name of the object id (particular instance of the __line type), I would like to see an error like "object id cannot include whitespaces, ... (or what are the characters that can be included)?" ```diff -__line 'quit password authentication (remove)' \ +__line quit_password_authentication_remove \ --file /etc/ssh/sshd_config \ --regex 'PasswordAuthentication.*no' \ --state absent ```

ungleich-gitea commented

2021-11-20 15:27:17 +00:00

in unix, spaces are evil

ungleich-gitea commented

2021-11-20 15:27:19 +00:00

@ander Do you know where these restrictions originate from?
One case I can think of would be the require environment variable, but this could easily solved by using shlex.split, IMO.

@ander Do you know where these restrictions originate from? One case I can think of would be the `require` environment variable, but this could easily solved by using [shlex.split](https://docs.python.org/3/library/shlex.html#shlex.split), IMO.

ungleich-gitea commented

2021-11-20 15:27:20 +00:00

__object_id must not contain spaces. it should match [a-z0-9_-]+ afaik.

`__object_id` must not contain spaces. it should match `[a-z0-9_-]+` afaik.

ungleich-gitea commented

2021-11-20 15:27:21 +00:00

I've tried a bit in cdist/exec/remote.py for the method run() by quoting all parameter, but it will fail for globing. If this should be handled, escaping spaces and parenthesis can be better. Or only problematic arguments will be quoted.

The bigger problem will be to detect which must be escaped/quoted and which not. Because the glob (chmod 0700 /var/lib/cdist/conf/explorer/*) is wanted. Maybe start escaping in an earlier stage of the path creation?

I've tried a bit in `cdist/exec/remote.py` for the method `run()` by quoting all parameter, but it will fail for globing. If this should be handled, escaping spaces and parenthesis can be better. Or only problematic arguments will be quoted. The bigger problem will be to detect which must be escaped/quoted and which not. Because the glob (`chmod 0700 /var/lib/cdist/conf/explorer/*`) is wanted. Maybe start escaping in an earlier stage of the path creation?

ungleich-gitea commented

2021-11-20 15:27:23 +00:00

Looks rather like an escaping problem. It happens not only for the __line type and looks similar to yours:

matze@matze:~$ echo "__file 'some weird name'" | cdist config localhost -i -
ERROR: localhost: scp -o User=root -q -o ControlPath=/tmp/tmplbeece9m/s -o ControlMaster=auto -o ControlPersist=2h /tmp/tmphvkceen5/421aa90e079fa326b6494f812ad13e79/data/object/__file/some weird name/.cdist-kaqk3boe/parameter/state localhost:/var/lib/cdist/object/__file/some weird name/.cdist-kaqk3boe/parameter: ['scp', '-o', 'User=root', '-q', '-o', 'ControlPath=/tmp/tmplbeece9m/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', '/tmp/tmphvkceen5/421aa90e079fa326b6494f812ad13e79/data/object/__file/some weird name/.cdist-kaqk3boe/parameter/state', 'localhost:/var/lib/cdist/object/__file/some weird name/.cdist-kaqk3boe/parameter']
ERROR: cdist: Failed to configure the following hosts: localhost
matze@matze:~$ echo "__file 'some weird name (yes)'" | cdist config localhost -i -
ERROR: localhost: ssh -o User=root -o ControlPath=/tmp/tmpiclrld_a/s -o ControlMaster=auto -o ControlPersist=2h localhost mkdir -p /var/lib/cdist/object/__file/some weird name (yes)/.cdist-d2fjfl3z/parameter: ['ssh', '-o', 'User=root', '-o', 'ControlPath=/tmp/tmpiclrld_a/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', 'localhost', 'mkdir', '-p', '/var/lib/cdist/object/__file/some weird name (yes)/.cdist-d2fjfl3z/parameter']
ERROR: cdist: Failed to configure the following hosts: localhost

I think it's a general problem not connected to the __line type. The funny thing is that there is a different point of failing with or without parenthesis.

Looks rather like an escaping problem. It happens not only for the `__line` type and looks similar to yours: ```sh matze@matze:~$ echo "__file 'some weird name'" | cdist config localhost -i - ERROR: localhost: scp -o User=root -q -o ControlPath=/tmp/tmplbeece9m/s -o ControlMaster=auto -o ControlPersist=2h /tmp/tmphvkceen5/421aa90e079fa326b6494f812ad13e79/data/object/__file/some weird name/.cdist-kaqk3boe/parameter/state localhost:/var/lib/cdist/object/__file/some weird name/.cdist-kaqk3boe/parameter: ['scp', '-o', 'User=root', '-q', '-o', 'ControlPath=/tmp/tmplbeece9m/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', '/tmp/tmphvkceen5/421aa90e079fa326b6494f812ad13e79/data/object/__file/some weird name/.cdist-kaqk3boe/parameter/state', 'localhost:/var/lib/cdist/object/__file/some weird name/.cdist-kaqk3boe/parameter'] ERROR: cdist: Failed to configure the following hosts: localhost matze@matze:~$ echo "__file 'some weird name (yes)'" | cdist config localhost -i - ERROR: localhost: ssh -o User=root -o ControlPath=/tmp/tmpiclrld_a/s -o ControlMaster=auto -o ControlPersist=2h localhost mkdir -p /var/lib/cdist/object/__file/some weird name (yes)/.cdist-d2fjfl3z/parameter: ['ssh', '-o', 'User=root', '-o', 'ControlPath=/tmp/tmpiclrld_a/s', '-o', 'ControlMaster=auto', '-o', 'ControlPersist=2h', 'localhost', 'mkdir', '-p', '/var/lib/cdist/object/__file/some weird name (yes)/.cdist-d2fjfl3z/parameter'] ERROR: cdist: Failed to configure the following hosts: localhost ``` I think it's a general problem not connected to the `__line` type. The funny thing is that there is a different point of failing with or without parenthesis.

Sign in to join this conversation.