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

Enhancement to API for Supporting List of Operations in Complex Configurations #1

Open
robertoberto opened this issue Mar 1, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@robertoberto
Copy link
Collaborator

A limitation has been identified in the current implementation of the API regarding the handling of complex configurations, such as those required for DHCP and PPPoE Servers, IPSec, VXLAN, and other tunnel configurations. The VyOS API documentation indicates that the API is capable of processing multiple commands by passing them as a list in the data field, which is crucial for configuring components that require a complete configuration before committing.

However, when attempting to configure a DHCP server and specifically working with VXLAN, it was observed that the _api_request function does not seem equipped to handle lists of operations (ops) that need to be sent in a single payload. For example, when configuring a VXLAN interface, a list of operations like the following needs to be sent:

[  {"op": "set", "path": ["interfaces", "vxlan", "vxlan1", "remote", "203.0.113.99"]},
  {"op": "set", "path": ["interfaces", "vxlan", "vxlan1", "vni", "1"]}
]

This format allows for complex configurations in a single request, which not only optimizes the configuration process but also meets the requirements of components that demand a complete configuration for commit. The absence of this capability in the current API could lead to incomplete configurations or the need for multiple API calls for configurations that ideally should be atomic.

It is suggested that an enhancement be implemented to allow the API to handle lists of operations within a single payload. This would not only align the implementation with the documentation and expected capabilities of the VyOS API but would also significantly improve the efficiency and robustness of network configurations performed through the API.

This issue was reported by a PyVyOS user in the VyOS forum, indicating a need for this enhancement within the community. For more details on the user's report, please visit: PyVyOS SDK Discussion.

Thank you for considering this suggestion, and looking forward to any discussions on this topic.

@robertoberto robertoberto self-assigned this Mar 1, 2024
@robertoberto robertoberto added the enhancement New feature or request label Mar 1, 2024
@xTITUSMAXIMUSX
Copy link
Contributor

This issue can be closed because of PR #3

@rico132
Copy link

rico132 commented Jul 31, 2024

@xTITUSMAXIMUSX As far as I can tell the PR did not solve the issue. I installed the package and used it like described but the API responds with 400. I am new to Python, but I append the paths to a list so it looks like this:

[
  [ 'this', 'is', 'my', 'path'],
  [ 'this', 'is', 'my', 'path2']
]

And set this list of lists as the configure_set/delete function path.
With a single list, the response is 200.

@xTITUSMAXIMUSX
Copy link
Contributor

What is your full command? Please provide more details. I have been using this without issues. My code looks something like the following.

device.configure_set(path=[["service", "dns", "forwarding", 'cache-size', cachesize],
                                            ["service", "dns", "forwarding", 'listen-address', listenaddress],
                                            ["service", "dns", "forwarding", 'allow-from', allowfrom]])

@rico132
Copy link

rico132 commented Jul 31, 2024

I am running the vyos/current Vagrant image to test pyvyos.
The installed Python version is 3.12.4.
Package installed via pip3.

My test code looks something like this:

nodes = [
    dict(Address='1.1.1.1'),
    dict(Address='2.2.2.2')
]

set_path = []

for node in nodes:
    set_path.append(['firewall', 'group', 'address-group', groupname, 'address', node['Address']])

device = VyDevice(hostname=hostname, apikey=apikey, verify=False)
print(set_path)
response = device.configure_set(path=set_path)
print(response)

set_path:

[['firewall', 'group', 'address-group', 'test-group', 'address', '1.1.1.1'], ['firewall', 'group', 'address-group', 'test-group', 'address', '2.2.2.2']]

response:

ApiResponse(status=400, request={'data': '{"op": "set", "path": [["firewall", "group", "address-group", "test-group", "address", "1.1.1.1"], ["firewall", "group", "address-group", "test-group", "address", "2.2.2.2"]], "url": "https://IP:443/configure"}'}, result={}, error='http error')

@xTITUSMAXIMUSX
Copy link
Contributor

I just ran this code below and it ran just fine.

if __name__ == '__main__':
    device = VyDevice(hostname='192.168.13.11', apikey='vyos', port=443, protocol='https', verify=False)

    groupname='testgroup'

    nodes = [
        dict(Address='1.1.1.1'),
        dict(Address='2.2.2.2')
    ]

    set_path = []

    for node in nodes:
        set_path.append(['firewall', 'group', 'address-group', groupname, 'address', node['Address']])
    print(set_path)
    response = device.configure_set(path=set_path)
    print(response)

set_path

[['firewall', 'group', 'address-group', 'testgroup', 'address', '1.1.1.1'], ['firewall', 'group', 'address-group', 'testgroup', 'address', '2.2.2.2']]

response

ApiResponse(status=200, request={'data': '[{"op": "set", "path": ["firewall", "group", "address-group", "testgroup", "address", "1.1.1.1"]}, {"op": "set", "path": ["firewall", "group", "address-group", "testgroup", "address", "2.2.2.2"]}]', 'url': 'https://192.168.13.11:443/configure'}, result=None, error=False)

vyos config

firewall {
    global-options {
        state-policy {
            established {
                action accept
            }
            invalid {
                action drop
            }
            related {
                action accept
            }
        }
    }
    group {
        address-group testgroup {
            address 1.1.1.1
            address 2.2.2.2
        }

@rico132
Copy link

rico132 commented Aug 1, 2024

Sorry, it's my fault, this explains a lot: #4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants