Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ssh key management for softlayer #321

Closed
wants to merge 23 commits into from
Closed

Ssh key management for softlayer #321

wants to merge 23 commits into from

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Jun 22, 2014

This pull requests add ssh key management for the SoftLayer driver.

Add methods for:
list_key_pairs()
get_key_pair()
import_key_pair_from_string()
delete_key_pair()
create_key_pair()

It also adds a new property to create node "ex_keyname" which is the name of the key to be associated to the new node.

Added tests for all methods and modified the test_create_node() test to use a sshkey as well.

Itxaka and others added 18 commits June 21, 2014 06:35
…h_key as a feature for the driver, associate the key to the node on create_node if a key name is used, added list_keypairs method to get all keys on the account, add key_name_to_id method to use the id on creating the node by providing the label
…_key_pair get just one key instead of getting all of them and then check the results
…ve/get_one/get_all/import_from_string. Adds tests for all new methods.
@@ -188,6 +194,47 @@ def _get_object_mask(self, service, mask):
return {}


class KeyPair(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key pair management is now part of the base compute API so you should use the existing libcloud.compute.base.KeyPair class.

@Itxaka
Copy link
Contributor Author

Itxaka commented Jun 22, 2014

Fixed the Keypar stuff that @Kami brougth to my attention. Im guessing that the way of storing extra stuff for the Keypairs is to use the extra field, so that's where I stored the ID of the key :D

@Itxaka
Copy link
Contributor Author

Itxaka commented Jul 14, 2014

Any update if this is gonna be merged @Kami ?

@Kami
Copy link
Member

Kami commented Aug 30, 2014

@Itxaka Really sorry for the delay.

I will review it again now and try to get it merged asap.

@@ -411,6 +422,82 @@ def create_node(self, **kwargs):

return self._to_node(raw_node)

def _to_key_pairs(self, elems):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor style thing - for consistency, please move "private" methods to the end of the class after all the "public" ones.

@Kami
Copy link
Member

Kami commented Aug 30, 2014

There are some minor style things which need to be addressed, but besides that it looks good.

I will get it merged into trunk as soon as those comments are addressed.

Thanks!

@Itxaka
Copy link
Contributor Author

Itxaka commented Aug 30, 2014

@Kami thanks for the comments, will try to fix them as soon as possible.

Regarding the sshkey stuff, I still don't know if getting the local generation for ssh keys before uploading its an ok thing and could be implemented across libcloud.

Thing is, some(all?) of the cloud providers allow you to import an ssh key, but for that we need a method to generate one, upload the public part and return to the user the keypair object (public and private) so I believe it would be good if it could be used across all drivers as a new generic method. For automation purpouses it makes too much sense (we are doing it that way internally, on node creation we generate a new key from scratch for each machine, but as libcloud does not support it we are doing it outside of it)

It requires a new dependecy thougth, pycrypto.

Cheers!

@Itxaka
Copy link
Contributor Author

Itxaka commented Aug 31, 2014

@Kami Everything modified to confirm to the proper guidelines and all tests passing :)

@Kami
Copy link
Member

Kami commented Aug 31, 2014

@Itxaka Thanks.

I'm having troubles applying this patch on top of trunk (I would squash the commits myself). Can you please sync your branch with latest trunk and squash all the commits?

In the worst case, if you are also having troubles, just sync your branch with trunk, do git diff --no-prefix trunk > t.diff, create new branch, git apply t.diff and commit all the changes under a single commit.

@Itxaka
Copy link
Contributor Author

Itxaka commented Aug 31, 2014

Having issues, will recreate the branch and a new pull request.

@Itxaka Itxaka closed this Aug 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants