Tidy up and correct the new example
This commit is contained in:
parent
396c4f3cf2
commit
baf1d0e13d
|
@ -12,7 +12,7 @@ an ``if`` statement as root on a file server behind a bastion host:
|
||||||
.. code-block:: bash
|
.. code-block:: bash
|
||||||
|
|
||||||
ssh bastion "
|
ssh bastion "
|
||||||
if \"$PROD\";
|
if [ \"$PROD\" ];
|
||||||
then
|
then
|
||||||
ssh fileserver sudo su -c \"
|
ssh fileserver sudo su -c \"
|
||||||
if grep -qs /dev/sdb1 /proc/mounts;
|
if grep -qs /dev/sdb1 /proc/mounts;
|
||||||
|
@ -27,31 +27,28 @@ an ``if`` statement as root on a file server behind a bastion host:
|
||||||
sudo touch /var/run/start_backup;
|
sudo touch /var/run/start_backup;
|
||||||
"
|
"
|
||||||
|
|
||||||
Chances are high this is familiar territory, we've all seen it, and many of us
|
Chances are high this is familiar territory, we've all seen it, and those
|
||||||
working in infrastructure have almost certainly even written it. At first
|
working in infrastructure have almost certainly written it. At first glance,
|
||||||
glance, ignoring that annoying quoting, it looks perfectly fine: well
|
ignoring that annoying quoting, it looks perfectly fine: well structured,
|
||||||
structured, neatly indented, and the purpose of the snippet seems clear. But I
|
neatly indented, and the purpose of the snippet seems clear.
|
||||||
have some questions:
|
|
||||||
|
|
||||||
1. At first glance, can you say if ``"/media/Main Backup Volume"`` is quoted
|
1. At first glance, is ``"/media/Main Backup Volume"`` quoted correctly?
|
||||||
correctly?
|
|
||||||
2. How will the ``if`` statement behave if there is a problem with the machine,
|
2. How will the ``if`` statement behave if there is a problem with the machine,
|
||||||
and, say, the ``/bin/grep`` binary is absent?
|
and, say, the ``/bin/grep`` binary is absent?
|
||||||
3. Ignoring quoting, can you spot any other syntax problems?
|
3. Ignoring quoting, are there any other syntax problems?
|
||||||
4. If you cutpaste this snippet from its original script into an interactive
|
4. If this snippet is pasted snippet from its original script into an
|
||||||
shell, will it behave the same as before?
|
interactive shell, will it behave the same as before?
|
||||||
5. Can you think offhand of differences in how the arguments to ``sudo
|
5. Can you think offhand of differences in how the arguments to ``sudo
|
||||||
...`` and ``ssh fileserver ...`` are parsed?
|
...`` and ``ssh fileserver ...`` are parsed?
|
||||||
6. In which context will the ``*`` glob be expanded, if it is expanded at all?
|
6. In which context will the ``*`` glob be expanded, if it is expanded at all?
|
||||||
|
7. What will the exit status of ``ssh bastion`` be if ``ssh fileserver`` fails?
|
||||||
|
|
||||||
|
|
||||||
Innocent But Deadly
|
Innocent But Deadly
|
||||||
~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
And now some answers:
|
1. The quoting used is nonsense! At best, ``mount`` will receive 3 arguments.
|
||||||
|
At worst, the snippet will not parse at all.
|
||||||
1. No, the quoting used is nonsense! At best, ``mount`` will receive 3
|
|
||||||
arguments. At worst, the snippet will not parse at all.
|
|
||||||
2. The ``if`` statement will treat a missing ``grep`` binary (exit status 127)
|
2. The ``if`` statement will treat a missing ``grep`` binary (exit status 127)
|
||||||
the same as if ``/dev/sdb1`` was not mounted at all (exit status 1). Unless
|
the same as if ``/dev/sdb1`` was not mounted at all (exit status 1). Unless
|
||||||
the program executing this script is parsing ``stderr`` output, the failure
|
the program executing this script is parsing ``stderr`` output, the failure
|
||||||
|
@ -59,19 +56,23 @@ And now some answers:
|
||||||
``rm`` was executed, it got wiped.
|
``rm`` was executed, it got wiped.
|
||||||
3. There is at least one more syntax error present: a semicolon missing after
|
3. There is at least one more syntax error present: a semicolon missing after
|
||||||
the ``umount`` command.
|
the ``umount`` command.
|
||||||
4. Depending in which environment the ``PROD`` variable is set, either it will
|
4. If you paste the snippet into an interactive shell, the apparently quoted
|
||||||
|
"!" character in the ``echo`` command will be interpreted as a history
|
||||||
|
expansion.
|
||||||
|
5. ``sudo`` preserves the remainder of the argument vector as-is, while
|
||||||
|
``ssh`` **concatenates** each part into a single string that is passed to
|
||||||
|
the login shell. While quotes appearing within arguments are preserved by
|
||||||
|
``sudo``, without additional effort, pairs of quotes are effectively
|
||||||
|
stripped by ``ssh``.
|
||||||
|
6. As for where the glob is expanded, the answer is I have absolutely no idea
|
||||||
|
without running the code, which might wipe out the backups!
|
||||||
|
7. If the ``ssh fileserver`` command fails, the exit status of ``ssh bastion``
|
||||||
|
will continue to indicate success.
|
||||||
|
8. Depending in which environment the ``PROD`` variable is set, either it will
|
||||||
always evaluate to false, because it was set by the bastion host, or it
|
always evaluate to false, because it was set by the bastion host, or it
|
||||||
will do the right thing, because it was set by the script host.
|
will do the right thing, because it was set by the script host.
|
||||||
5. If you cutpaste the snippet into an interactive shell, the apparently quoted
|
|
||||||
"!" character in the ``echo`` command will be interepreted as a history
|
|
||||||
expansion.
|
|
||||||
6. Nobody knows this one! ``sudo`` preserved the remainder of the argument
|
|
||||||
vector as-is, whereas ``ssh`` **concatenates** each part into a single
|
|
||||||
string that is passed to the login shell.
|
|
||||||
7. As for where the glob is expanded, the answer is I have absolutely no idea
|
|
||||||
without running the code, which might wipe out the backups!
|
|
||||||
|
|
||||||
Golly, we've managed to hit at least 7 potentially mission-critical gotchas in
|
Golly, we've managed to hit at least 8 potentially mission-critical gotchas in
|
||||||
only 14 lines of code, and they are just those I can count! Welcome to the
|
only 14 lines of code, and they are just those I can count! Welcome to the
|
||||||
reality of "programming" in shell.
|
reality of "programming" in shell.
|
||||||
|
|
||||||
|
@ -79,63 +80,60 @@ In the end, superficial legibility counted for nothing, it's 4AM, you've been
|
||||||
paged, the network is down and your boss is angry.
|
paged, the network is down and your boss is angry.
|
||||||
|
|
||||||
|
|
||||||
The Madness That Is Shell Quoting
|
Shell Quoting Madness
|
||||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
Let's assume on first approach that we really want to handle those quoting
|
Let's assume on first approach that we really want to handle those quoting
|
||||||
issues. I wrote a little Python 3 script based around the
|
issues. I wrote a little Python script based around the :py:func:`shlex.quote`
|
||||||
:py:func:`shlex.quote` function to help construct, to the best of my knowledge,
|
function to construct, to the best of my knowledge, the quoting required for
|
||||||
the quoting required for each stage of the command:
|
each stage:
|
||||||
|
|
||||||
::
|
|
||||||
|
|
||||||
>>> c1 = q('if grep -qs /dev/sdb1 /proc/mounts ; then echo "sdb1 already mounted!"; unmount /dev/sdb1; fi; mount /dev/sdb1 "/media/Main Backup Volume"')
|
|
||||||
>>> c2 = qq('bash', '-c', c1)
|
|
||||||
>>> c3 = qq('sudo', 'su', '-c', c2)
|
|
||||||
>>> c4 = qq('bash', '-c', 'if $PROD; then ssh 'diskserver', c3)
|
|
||||||
>>> c5 = 'if "$PROD"; then %s; fi; sudo touch /var/run/start_backup' % (c4,)
|
|
||||||
>>> c6 = qq('ssh', 'bastion', qq('bash', '-c', c5))
|
|
||||||
>>> print(c6)
|
|
||||||
|
|
||||||
And now, the output:
|
|
||||||
|
|
||||||
.. code-block:: bash
|
.. code-block:: bash
|
||||||
|
|
||||||
ssh bastion 'bash -c '"'"'if "$PROD"; then ssh diskserver '"'"'"'"'"'"'"
|
ssh bastion '
|
||||||
'"'sudo su -c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'bash
|
if [ "$PROD" ];
|
||||||
-c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
then
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
ssh fileserver sudo su -c '"'"'
|
||||||
"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
if grep -qs /dev/sdb1 /proc/mounts;
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
then
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
echo "sdb1 already mounted!";
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
umount /dev/sdb1
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
fi;
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
rm -rf "/media/Main Backup Volume"/*;
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'if grep
|
mount /dev/sdb1 "/media/Main Backup Volume"
|
||||||
-qs /dev/sdb1 /proc/mounts ; then echo "sdb1 already mounted!"; unmount
|
'"'"';
|
||||||
/dev/sdb1; fi; mount /dev/sdb1 "/media/Main Backup Volume"'"'"'"'"'"'"'"'
|
fi;
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
sudo touch /var/run/start_backup
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
'
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
|
||||||
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
|
||||||
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
|
||||||
"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'; fi; sudo touch /var/run/start_backup'"'"''
|
|
||||||
|
|
||||||
Even with Python handling all the heavy lifting of correctly quoting each layer
|
Even with Python handling the heavy lifting of quoting each shell layer, and
|
||||||
of shell, and producing the monstrosity that is a syntactically correct result,
|
|
||||||
and even if we fixed the aforementioned minor disk-wiping issue, I am still not
|
and even if we fixed the aforementioned minor disk-wiping issue, I am still not
|
||||||
100% confident that I know precisely the argument handling rules for all of
|
100% confident that I know precisely the argument handling rules for all of
|
||||||
``su``, ``sudo``, ``ssh``, and ``bash``.
|
``su``, ``sudo``, ``ssh``, and ``bash``.
|
||||||
|
|
||||||
|
Finally, if any of the login shells involved may not be set to ``bash``, we
|
||||||
|
must introduce additional layers of quoting, in order to explicitly invoke
|
||||||
|
``bash`` at each stage, causing an explosion in quoting:
|
||||||
|
|
||||||
|
.. code-block:: bash
|
||||||
|
|
||||||
|
ssh bastion 'bash -c '"'"'if [ "$PROD" ]; then ssh fileserver bash -c '"'"'
|
||||||
|
"'"'"'"'"'"'sudo su -c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
||||||
|
'bash -c '"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
||||||
|
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
||||||
|
"'"'"'"'"'"'"'"'"'"'if grep -qs /dev/sdb1 /proc/mounts; then echo "sdb1 alr
|
||||||
|
eady mounted!"; umount /dev/sdb1 fi; rm -rf "/media/Main Backup Volume"/*;
|
||||||
|
mount /dev/sdb1 "/media/Main Backup Volume"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"
|
||||||
|
'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'
|
||||||
|
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'"'"'
|
||||||
|
"'"'"'"'"'"'"'"'"'"'"'"'"'"'"'"''"'"'"'"'"'"'"'"'; fi; sudo touch /var/run/
|
||||||
|
start_backup'"'"''
|
||||||
|
|
||||||
|
|
||||||
There Is Hope
|
There Is Hope
|
||||||
~~~~~~~~~~~~~
|
~~~~~~~~~~~~~
|
||||||
|
|
||||||
We could instead express the above using Mitogen directly in Python code:
|
We could instead express the above using Mitogen:
|
||||||
|
|
||||||
::
|
::
|
||||||
|
|
||||||
|
@ -163,13 +161,12 @@ We could instead express the above using Mitogen directly in Python code:
|
||||||
|
|
||||||
bastion_sudo.call(run, 'touch', '/var/run/start_backup')
|
bastion_sudo.call(run, 'touch', '/var/run/start_backup')
|
||||||
|
|
||||||
And now, a few more questions:
|
* In which context must the ``PROD`` variable be defined?
|
||||||
|
* On which machine is each step executed?
|
||||||
* Can you tell in which context the ``PROD`` variable must be defined?
|
* Are there any escaping issues?
|
||||||
* Can you tell on which machine each step executed?
|
|
||||||
* Can you see any escaping issues?
|
|
||||||
* What will happen if the ``grep`` binary is missing?
|
* What will happen if the ``grep`` binary is missing?
|
||||||
* What will happen if any step fails?
|
* What will happen if any step fails?
|
||||||
|
* What will happen if any login shell is not ``bash``?
|
||||||
|
|
||||||
|
|
||||||
Recursively Nested Bootstrap
|
Recursively Nested Bootstrap
|
||||||
|
|
Loading…
Reference in New Issue