Skip to content

Change SSH backend interface to a structure for pubkey/password authentication #62

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

marcy-terui
Copy link
Contributor

@marcy-terui marcy-terui commented Nov 7, 2017

Close #43

Change the argument of backend_ssh_new to a structure with hostname, user and key file or password.

  • Interface definition
pub struct SSHInterface {
    hostname: *const c_char,
    user: *const c_char,
    password: *const c_char,
    key_file: *const c_char,
}

If you do not give some properties, use the empty string.

Sample Code (Python binding)

import libspecinfra
import libspecinfra.backend
import ctypes

class SSHInterfaceS(ctypes.Structure):
    _fields_ = [
            ("hostname", ctypes.c_char_p),
            ("user", ctypes.c_char_p),
            ("password", ctypes.c_char_p),
            ("key_file", ctypes.c_char_p)]

    def __init__(self, **kwargs):
        self.hostname = kwargs.get('hostname', "").encode('utf-8')
        self.user = kwargs.get('user', "").encode('utf-8')
        self.password = kwargs.get('password', "").encode('utf-8')
        self.key_file = kwargs.get('key_file', "").encode('utf-8')

t = SSHInterfaceS(
    hostname='ec2-11-111-111-111.us-west-2.compute.amazonaws.com',
    user='ubuntu',
    key_file='/Users/marcy/.ssh/example.pem')

backend = libspecinfra.backend.SSH(t)
specinfra = libspecinfra.Specinfra(backend)
f = specinfra.file('/etc/passwd')

print(oct(f.mode())) # => 0o644

Note : #59 (My thought process. Sorry for writing in Japanese)

@mizzy
Copy link
Member

mizzy commented Nov 7, 2017

Thanks a lot! But your code is diffrent from my policy a little.

I'm developing libspecinfra like this:

Phase 1

In this phase, I don't think about other languages at all. I write code for using by Rust itself.

You consider about other languages from first. But it brings complexity.
For example, set_target() is not called by other languages directly, but it takes C struct as an argument. And unsafe and CStr are used in this function. It seems complicated a little. I'd like to separate code for Rust and code for FFI to reduce complexity.

I've made this PR #63 according to this policy. Please see it.

Phase 2

In this phase, I write wrapper functions for FFI. I use only primitives(char, int and so on) and a pointer as arguments and return values. I don't use C struct. From other languages, Rust struct is just only pointer. Other languages don't know the struct fields behind the pointer.

You use C struct in this PR. This also brings complexity. I'd like to avoid this complexity.

I will write code and make PR according to this policy later.

Phase 3

In this phase, I write other languages bindings. Rust way and other languages ways are different. Language bindings should absorb the difference and provide API which is easy to use for users of the language.

@marcy-terui
Copy link
Contributor Author

Thank you for your review. It's very suggestive and informative!!
And I still need to study more... 😭

But we decided the argument of backend_ssh_new to be a structure at #43
What interface do you think match to the policy?

@mizzy
Copy link
Member

mizzy commented Nov 7, 2017

But we decided the argument of backend_ssh_new to be a structure at #43

I said that without deep thinking, sorry.

What interface do you think match to the policy?

I will write code. Please wait a little.

@mizzy
Copy link
Member

mizzy commented Nov 7, 2017

Done.

This is specinfra core.
#64

This is Ruby binding.
libspecinfra/libspecinfra-ruby#1

Examples using Ruby binding.
https://github.com/libspecinfra/examples/pull/1/files

If we don't use C struct in Rust, we don't need to define the struct in other language bindings.
So it becomes simple, I think.

@marcy-terui
Copy link
Contributor Author

I got it! I think that it is very simple and nice policy 👍

  • The owner of the structures is always Rust
  • The bindings has only the pointers
  • If we want to change the structure of Rust, we use a function with pointer as argument

Thanks a lot!!

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.

2 participants