Hi On Wed, Oct 14, 2020 at 1:14 AM Philippe Mathieu-Daudé <phi...@redhat.com> wrote: > > Hi Marc-André, > > On 10/13/20 10:25 PM, marcandre.lur...@redhat.com wrote: > > From: Marc-André Lureau <marcandre.lur...@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 > > FWIW
ok > > > 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.lur...@redhat.com> > > --- > ... > > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > > index cec98c7e06..50e2854b45 100644 > > --- a/qga/qapi-schema.json > > +++ b/qga/qapi-schema.json > > @@ -1306,3 +1306,35 @@ > > ## > > { 'command': 'guest-get-devices', > > 'returns': ['GuestDeviceInfo'] } > > + > > +## > > +# @guest-ssh-add-authorized-keys: > > +# > > +# @username: the user account to add the authorized key > > +# @keys: the public keys to add (in OpenSSH format) > > You use plural but the code only seems to add (remove) one key > at a time. Uh, what makes you believe that? > > 'OpenSSH format' is confusing. From sshd(8): > > Each line of the file contains one key (empty lines and lines > starting with a ‘#’ are ignored as comments). > > Public keys consist of the following space-separated fields: > > options, keytype, base64-encoded key, comment. > > The options field is optional. > > Note that lines in this file can be several hundred bytes long > (because of the size of the public key encoding) up to a limit > of 8 kilobytes, which permits RSA keys up to 16 kilobits. > > The options (if present) consist of comma-separated option > specifications. No spaces are permitted, except within double > quotes. > > @openssh_authorized_key_line is ugly, maybe use @authorized_key > to make it clearer? Why? the name of the function already implies we are talking about authorized keys. The documentation says it's a public key in openssh format (the ones you expect in ~/.ssh/authorized_keys files) Yes the format isn't very well defined, so I did simple sanity checks. After all, people usually append keys with shell >>. I can't find a common command to do it with some checking. > > +# > > +# Append a public key to user $HOME/.ssh/authorized_keys on Unix systems > > (not > > +# implemented for other systems). > > Here "a key" singular, good. bad. it should be plural (everywhere else is plural, afaict) > > > +# > > +# Returns: Nothing on success. > > +# > > +# Since: 5.2 > > +## > > +{ 'command': 'guest-ssh-add-authorized-keys', > > Here "keys" plural :/ > > > + 'data': { 'username': 'str', 'keys': ['str'] } } > > + > > +## > > +# @guest-ssh-remove-authorized-keys: > > +# > > +# @username: the user account to add the authorized key > > +# @keys: the public keys to remove (in OpenSSH format) > > +# > > +# Remove public keys from the user $HOME/.ssh/authorized_keys on Unix > > systems > > +# (not implemented for other systems). > > +# > > +# Returns: Nothing on success. > > +# > > +# Since: 5.2 > > +## > > +{ 'command': 'guest-ssh-remove-authorized-keys', > > + 'data': { 'username': 'str', 'keys': ['str'] } } > > >