Project

General

Profile

Bug #3028

Python Vici client is missing direct support of some Vici Calls

Added by Josh Marshall about 1 month ago. Updated 29 days ago.

Status:
Closed
Priority:
Normal
Category:
vici
Target version:
Start date:
Due date:
Estimated time:
Affected version:
5.7.2
Resolution:
Fixed

Description

Cross referencing the python client and Vici documentation linked below, the following functions are not directly supported my the python client, but should be:

Get-algorithms
Get-authorities
Get-counters
Get-keys
List-authorities
Load-authority
Load-token
rekey
Reset-counters
Unload-authority
Unload-key
Unload-shared

There also two calls in the Python client which are not direct Vici calls which need improved documentation:

Get-conns
List-sa
listen

These are relatively easy changes, but I'm not sure how merge requests or changes are handled here.

https://github.com/strongswan/strongswan/blob/57447015db828832e0e141dcdab7fbf61f828851/src/libcharon/plugins/vici/python/vici/session.py

https://www.strongswan.org/apidoc/md_src_libcharon_plugins_vici_README.html

Associated revisions

Revision 6b952f69
Added by Tobias Brunner 29 days ago

Merge branch 'update-vici-bindings'

Updates the command wrappers in all the bindings and simplifies calling
new commands (i.e. not yet wrapped) with the Python and Ruby bindings.

Fixes #3028.

History

#1 Updated by Tobias Brunner about 1 month ago

  • Status changed from New to Feedback

Cross referencing the python client and Vici documentation linked below, the following functions are not directly supported my the python client, but should be:

I guess we could add wrappers for these. But honestly, I think it would be way more practical to make this binding more generic and remove basically all the wrappers (they are one liners anyway). That way there also wouldn't be any confusion when using newer bindings (e.g. installed via pip) with older versions of the daemon/vici plugin (well, I guess you could install an old version of the binding too, but still). To show how commands are called we could provide some examples (e.g. one for a simple command like version, one for something like initiate with streamed log messages and one for a streamed list command e.g. list-sas - and perhaps for events). Actually, we may even extract the binding to a separate repository (the ones for Ruby and Perl too) similar to the davici library.

There also two calls in the Python client which are not direct Vici calls which need improved documentation:

Get-conns
List-sa

What do you mean? get-conns and lists-sas are definitely vici commands.

These are relatively easy changes, but I'm not sure how merge requests or changes are handled here.

See Contributions.

#2 Updated by Josh Marshall about 1 month ago

The `get-conns`, nor `list-sa` must have been excel errors, sorry. Is there any particular way you'd like to make the calls more general? If so, I can implement it and submit the patch.

#3 Updated by Tobias Brunner about 1 month ago

Is there any particular way you'd like to make the calls more general?

I was just thinking of removing the wrapper methods and merging SessionHandler into Session, resulting in what you can see in the 3028-vici-generalize branch, and then adding some examples for request(), streamed_request() and listen() as mentioned above. That would obviously not be backwards compatible, so if that's a concern, we could keep the existing wrappers (e.g. in a separate file/class that's mixed into Session) but mark them as deprecated.

#4 Updated by Josh Marshall about 1 month ago

I personally like having the training wheels around, even if they end up being one liners. It helps with immediate integration and understanding mechanics for all calls. In any case, I'd like to keep with what is more standard which means what you decide on.

#5 Updated by Tobias Brunner 29 days ago

  • Tracker changed from Issue to Bug
  • Target version set to 5.8.0

I personally like having the training wheels around, even if they end up being one liners.

My teammates agree with you. And since we have to keep the existing wrappers anyway for compatibility with existing scripts we can also just update them :) I did so in the mentioned branch (also for the other bindings).

#6 Updated by Tobias Brunner 29 days ago

  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Resolution set to Fixed

Changes are now in master.

Also available in: Atom PDF