On Fri, 13 Nov 2015 at 11:18 'Hrvoje Ribicic' via ganeti-devel <
[email protected]> wrote:

> This patch expands the testing of SSH key renewal by changing the key
> type existing on a cluster during the QA.
>
> Signed-off-by: Hrvoje Ribicic <[email protected]>
> ---
>  qa/qa_cluster.py | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index 1ad2fb9..941967c 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -1195,6 +1195,56 @@ def _AssertSsconfCertFiles():
>                        " '%s'." % (node, first_node))
>
>
> +def _TestSSHKeyChanges(master_node):
> +  """Tests a lot of SSH key type- and size- related functionality.
> +
> +  @type master_node: string
> +  @param master_node: The cluster master.
>

Nit: I personally would rephrase that to "the *name* of the cluster master"
since we have way to many places already where one has to think 'is this
now the UUID or the name?".


> +
> +  """
> +  # Helper fn to avoid specifying base params too many times
> +  def _RenewWithParams(new_params, verify=True, fail=False):
> +    AssertCommand(["gnt-cluster", "renew-crypto", "--new-ssh-keys", "-f",
> +                   "--no-ssh-key-check"] + new_params, fail=fail)
> +    if not fail and verify:
> +      AssertCommand(["gnt-cluster", "verify"])
> +
> +  # And the actual tests
> +  with qa_config.AcquireManyNodesCtx(1, exclude=[master_node]) as nodes:
> +    node_name = nodes[0].primary
> +
> +    # Another helper function for checking whether a specific key can log
> in
> +    def _CheckLoginWithKey(key_path, fail=False):
> +      AssertCommand(["ssh", "-oIdentityFile=%s" % key_path,
> "-oBatchMode=yes",
> +                     "-oStrictHostKeyChecking=no", "-oIdentitiesOnly=yes",
> +                     "-F/dev/null", node_name, "true"],
> +                    fail=fail, forward_agent=False)
> +
>

Nice to actually test that!


> +    # First test the simplest change
> +    _RenewWithParams([])
> +
> +    _RenewWithParams(["--ssh-key-type=dsa"])
> +    _CheckLoginWithKey(".ssh/id_dsa")
> +    # Stash the key for now
> +    old_key_backup = qa_utils.BackupFile(master_node.primary,
> ".ssh/id_dsa")
> +
> +    try:
> +      _RenewWithParams(["--ssh-key-type=rsa"])
> +      _CheckLoginWithKey(".ssh/id_rsa")
> +      # And check that we cannot log in with the old key
> +      _CheckLoginWithKey(old_key_backup, fail=True)
> +    finally:
> +      AssertCommand(["rm", "-f", old_key_backup])
> +
> +    _RenewWithParams(["--ssh-key-bits=4096"])
> +    _RenewWithParams(["--ssh-key-bits=521"], fail=True)
> +
> +    # Restore the cluster to its pristine state, skipping the verify as
> we did
> +    # way too many already
> +    _RenewWithParams(["--ssh-key-type=rsa", "--ssh-key-bits=2048"],
> +                     verify=False)
> +
> +
>  def TestClusterRenewCrypto():
>    """gnt-cluster renew-crypto"""
>    master = qa_config.GetMasterNode()
> @@ -1266,9 +1316,8 @@ def TestClusterRenewCrypto():
>      _AssertSsconfCertFiles()
>      AssertCommand(["gnt-cluster", "verify"])
>
> -    # Only renew SSH keys
> -    AssertCommand(["gnt-cluster", "renew-crypto", "--force",
> -                   "--new-ssh-keys", "--no-ssh-key-check"])
> +    # Comprehensively test various types of SSH key changes
> +    _TestSSHKeyChanges(master)
>
>      # Restore RAPI certificate
>      AssertCommand(["gnt-cluster", "renew-crypto", "--force",
> --
> 2.6.0.rc2.230.g3dd15c0
>
>
Nice patch, LGTM.
-- 

Helga Velroyen
Software Engineer
[email protected]

Google Germany GmbH
Dienerstraße 12
80331 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.

This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.

Reply via email to