Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys

[Markus Armbruster]

Michael Roth <michael.roth@amd.com> writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add new commands to add and remove SSH public keys from
> ~/.ssh/authorized_keys.
>
> I took a different approach for testing, including the unit tests right
> with the code. I wanted to overwrite the function to get the user
> details, I couldn't easily do that over QMP. Furthermore, I prefer
> having unit tests very close to the code, and unit files that are domain
> specific (commands-posix is too crowded already). FWIW, that
> coding/testing style is Rust-style (where tests can or should even be
> part of the documentation!).
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1885332
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> *squashed in fix-ups for setting file ownership and use of QAPI
>  conditionals for CONFIG_POSIX instead of stub definitions
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++
>  qga/meson.build          |  20 +-
>  qga/qapi-schema.json     |  35 ++++
>  3 files changed, 461 insertions(+), 1 deletion(-)
>  create mode 100644 qga/commands-posix-ssh.c
>
> diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
> new file mode 100644
> index 0000000000..a7bc9a1c24
> --- /dev/null
> +++ b/qga/commands-posix-ssh.c
> @@ -0,0 +1,407 @@
> + /*
> +  * This work is licensed under the terms of the GNU GPL, version 2 or later.
> +  * See the COPYING file in the top-level directory.
> +  */
> +#include "qemu/osdep.h"
> +
> +#include <glib-unix.h>
> +#include <glib/gstdio.h>
> +#include <locale.h>
> +#include <pwd.h>
> +
> +#include "qapi/error.h"
> +#include "qga-qapi-commands.h"
> +
> +#ifdef QGA_BUILD_UNIT_TEST
> +static struct passwd *
> +test_get_passwd_entry(const gchar *user_name, GError **error)
> +{
> +    struct passwd *p;
> +    int ret;
> +
> +    if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
> +        g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
> +        return NULL;
> +    }
> +
> +    p = g_new0(struct passwd, 1);
> +    p->pw_dir = (char *)g_get_home_dir();
> +    p->pw_uid = geteuid();
> +    p->pw_gid = getegid();
> +
> +    ret = g_mkdir_with_parents(p->pw_dir, 0700);
> +    g_assert_cmpint(ret, ==, 0);

checkpatch ERROR: Use g_assert or g_assert_not_reached

See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
for rationale.

More below, and in PATCH 10+12.

[...]