On Fri, Nov 13, 2015 at 2:41 PM, Helga Velroyen <[email protected]> wrote:

>
>
> 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?".
>

Well, good that you pointed this out as this is actually not a string :D
Interdiff:

diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
index 941967c..fbb90d0 100644
--- a/qa/qa_cluster.py
+++ b/qa/qa_cluster.py
@@ -1198,7 +1198,7 @@ def _AssertSsconfCertFiles():
 def _TestSSHKeyChanges(master_node):
   """Tests a lot of SSH key type- and size- related functionality.

-  @type master_node: string
+  @type master_node: L{qa_config._QaNode}
   @param master_node: The cluster master.

   """


>
>
>> +
>> +  """
>> +  # 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.
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 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