Registration crate

[jreidinger]

Problem

With the drop of ruby software service we will also loose suse connect ruby bindings.

Solution

Write crate that acts like suseconnect bindings. We also have to write headers ourself from suseconnect-ng sources. This can be maybe in future send to their git.

Crate implementation is done for now as it export all methods that ruby agama needed, but it can be extended for future if need arise.

Testing

• Tested manually

To ensure correctness I tested using examples. How to use it:

cd rust/suseconnect-agama
cargo test
../target/debug/examples/announce <regcode>

Feel free to test both correct and invalid SLES16 registration code.

Another example is for listing addons. I hardcoded sles 16 there and it needs to be run on already registered system.

[imobachgs]

Great job! Just a few notes, nothing critical.

Thanks!

[imobachgs]

I wonder if we should use a permalink, because this one might get outdated at some point (yes, I know, permalinks are long and ugly). It would be https://github.com/SUSE/connect-ng/blob/5a200487c50d9c955708b34d0d35ebef47dc5a3e/internal/connect/config.go#L45.

[jreidinger]

that is questionable as on ther hand we are mainly interested in the latest code to see if it changes in a way that can affect us.

[lslezak]

It's easy to get the latest version just by clicking in the GitHub page or by manually rewriting the commit hash in the URL by "master".

But the opposite way, getting the old original status at the time when wrote the code, is more difficult. So I'd also prefer using permalinks.

Moreover having a git commit allows to easily see a diff from master and check the changes.

[imobachgs]

I think you can do (just a suggestion):

Suggested change

	let Some(credentials) = value.get("credentials").and_then(|c| c.as_array()) else {
	let Some(credentials) = value.get("credentials").and_then(Value::as_array) else {

You can do the same with others as_* (e.g., as_str).

[jreidinger]

It is a bit shorter, so why not.

[imobachgs]

Should we document why calling unwrap is OK here? Just forget about it if it is the common approach when writing bindings.

[jreidinger]

well it is tricky question. Error can happen only if string contain '\0' inside it ( which basically kills C string as C uses \0 for end of string for basic char *). Here when we construct it from json I hope it cannot happen. But maybe we can introduce new error type for malformed strings passed to bindings.

[imobachgs]

Why is not the e-mail part of the params struct?

[jreidinger]

it is already there...but do not ask me why this call have it as separate param. It is even part of connect config https://github.com/SUSE/connect-ng/blob/main/internal/connect/config.go#L55 .. so my guess is that it is some backward compatibility relict. But checking code it really uses this param and we also use it in ruby part, so for safety I would also use it here.

[imobachgs]

If we are ignoring the result, why is this call needed (just asking)?

[jreidinger]

this is done at multiple parts. Basically here the issue is that SUSEConnect headers use "char " and not "const char" like we used in libzypp. Result is that we need to transform CString into_raw and basically give away memory ownership ( for const char* we do not need to do it )....but SUSEConnect won't clear passed string, so we need to take back ownership after the call to ensure that CString will free the memory when it gets out of scope. And that is exactly what happens there.
If you are interested here it is explained https://doc.rust-lang.org/stable/std/ffi/struct.CString.html#method.from_raw

[lslezak]

Some minor comments.

[lslezak]

It's easy to get the latest version just by clicking in the GitHub page or by manually rewriting the commit hash in the URL by "master".

But the opposite way, getting the old original status at the time when wrote the code, is more difficult. So I'd also prefer using permalinks.

Moreover having a git commit allows to easily see a diff from master and check the changes.

[lslezak]

As a I can see it now uses url:Url so the comment can be removed.

[lslezak]

I know this is just a testing example, but how difficult would be to detect the real current architecture? We need it in the real code anyway...

[jreidinger]

Well, in original code we use rpm arch...sadly using zypper system-architecture returns different string, so it would probably need a bit of processing. I kind of hope that we can get rpm architecture from libzypp. ( which I do not want to use here )

[lslezak]

I guess a link to the SUSEconnect code would be useful here...

[jreidinger]

something like this mapping? https://github.com/SUSE/connect-ng/blob/5a200487c50d9c955708b34d0d35ebef47dc5a3e/third_party/libsuseconnect/libsuseconnect.go#L292

[lslezak]

Is that really name or the alias?

[jreidinger]

probably alias, but they call it in suse connect "name". That is always a bit challenge for me in such bindings if I should change names or keep original API names.

[lslezak]

Ah, I see...

[lslezak]

I think u32 should be more than enough for HTTP error codes...

[jreidinger]

well, even u16 will be enough, but it is more related to json interger sizes. Maybe even more correct would be i64 as suseconnect also use signed integer https://github.com/SUSE/connect-ng/blob/5a200487c50d9c955708b34d0d35ebef47dc5a3e/pkg/connection/api_error.go#L12

[lslezak]

Maybe rather use unused_argument name? As more important is "why" and not "what", as is obviously visible.

[jreidinger]

Well, I was a bit scaried to send there anything else, so I replicate explicit empty string as in ruby bindings in connect - https://github.com/SUSE/connect-ng/blob/main/third_party/yast/lib/suse/connect/yast.rb#L169