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.

Sign up for GitHub

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

Jump to bottom

Expose the networking API over HTTP #1064

Merged
merged 8 commits into from
Mar 22, 2024
Merged

Expose the networking API over HTTP #1064

merged 8 commits into from
Mar 22, 2024

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Mar 1, 2024 •
edited
Loading

Implementation PBI description

In the previous PBI we documented a possible API and also created a POC but it just allowed to query some configuration, so we have extended the PR with the API implementation

Implemented Network API

Resource /network/state
  • GET: get the current network state and general information

Request example

$ curl -s localhost:3000/api/network/state -H @headers.txt | j
{
"connectivity": true,
"wireless_enabled": false,
"networking_enabled": true
}
  • PUT: Update the current network state (currently only enabling wireless is supported)

Request example

$ cat state.json 
{
"connectivity": true,
"wireless_enabled": true,
"networking_enabled": true
}

$ curl -s -X POST localhost:3000/api/network/state -H @headers.txt -H "Content-Type: application/json" -d @state.json  | jq
{
"connectivity": true,
"wireless_enabled": true,
"networking_enabled": true
}
Resource /network/devices
  • GET: list of known devices
    • Attributes
      • Name: String
      • Type: Enum

Request example

$ curl -s localhost:3000/network/devices -H @headers.txt | jq
[
  {
    "name": "enp2s0f0",
    "type": "Ethernet"
  },
  {
    "name": "wlp3s0",
    "type": "Wireless"
  },
  {
    "name": "eth0",
    "type": "Ethernet"
  }
Resource /network/connections
  • GET: list of managed / known connections
    Request example
$curl -s localhost:3000/api/network/connections -H @headers.txt | jq
[
{
  "id": "lo",
  "method4": "manual",
  "method6": "manual",
  "addresses": [
    "127.0.0.1/8",
    "::1"
  ],
  "interface": "lo"
},
{
  "id": "Wired connection 1",
  "method4": "manual",
  "gateway4": "192.168.0.1",
  "method6": "auto",
  "addresses": [
    "192.168.0.230/24"
  ],
  "nameservers": [
    "192.168.0.1"
  ],
  "interface": "eth0"
},
{
  "id": "AgamaNetwork",
  "method4": "disabled",
  "method6": "disabled",
  "wireless": {
    "password": "agama.test",
    "security": "wpa-psk",
    "ssid": "AgamaNetwork2",
    "mode": "infrastructure"
  }
}
]
  • POST: create a new connection
    ** Request example **
$ curl -X POST localhost:3000/api/network/connections -H @headers.txt -H "Content-Type: application/json" -d @data_wifi.json
"ce91e1b8-7da4-4bff-a286-91610a8cb762"

/network/connections/:id

  • GET: find a connection by its uuid or id (not implemented)
  • PATCH: update connection
  • DELETE: remove the connection

The attributes for updating a connection or creating a new one could follow the same schema used for the profile:

      "connections": {
        "description": "Network connections to be defined",
        "type": "array",
        "items": {
          "type": "object",
          "additionalProperties": false,
          "properties": {
            "id": {
              "description": "Connection ID",
              "type": "string"
            },
            "interface": {
              "description": "The name of the network interface bound to this connection",
              "type": "string"
            },
            "mac-address": {
              "description": "Custom mac-address (can also be 'preserve', 'permanent', 'random' or 'stable')",
              "type": "string"
            },
            "method4": {
              "description": "IPv4 configuration method (e.g., 'auto')",
              "type": "string",
              "enum": [
                "auto",
                "manual",
                "link-local",
                "disabled"
              ]
            },
            "method6": {
              "description": "IPv6 configuration method (e.g., 'auto')",
              "type": "string",
              "enum": [
                "auto",
                "manual",
                "link-local",
                "disabled"
              ]
            },
            "gateway4": {
              "description": "Connection gateway address (e.g., '192.168.122.1')",
              "type": "string"
            },
            "gateway6": {
              "description": "Connection gateway address (e.g., '::ffff:c0a8:7a01')",
              "type": "string"
            },
            "addresses": {
              "type": "array",
              "items": {
                "description": "Connection addresses",
                "type": "string",
                "additionalProperties": false
              }
            },
            "nameservers": {
              "type": "array",
              "items": {
                "description": "Nameservers (IPv4 and/or IPv6 are allowed)",
                "type": "string",
                "additionalProperties": false
              }
            },
            "wireless": {
              "type": "object",
              "description": "Wireless configuration",
              "additionalProperties": false,
              "properties": {
                "password": {
                  "type": "string"
                },
                "security": {
                  "type": "string"
                },
                "ssid": {
                  "type": "string"
                },
                "mode": {
                  "type": "string",
                  "enum": [
                    "infrastructure",
                    "adhoc",
                    "mesh",
                    "ap"
                  ]
                }
              }
            },
            "bond": {
              "type": "object",
              "description": "Bonding configuration",
              "additionalProperties": false,
              "properties": {
                "mode": {
                  "type": "string"
                },
                "options": {
                  "type": "string"
                },
                "ports": {
                  "type": "array",
                  "items": {
                    "description": "A list of the interfaces or connections to be bonded",
                    "type": "string",
                    "additionalProperties": false
                  }
                }
              }
            },
            "match": {
              "type": "object",
              "description": "Match settings",
              "additionalProperties": false,
              "properties": {
                "kernel": {
                  "type": "array",
                  "items": {
                    "description": "A list of kernel command line arguments to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "interface": {
                  "type": "array",
                  "items": {
                    "description": "A list of interface names to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "driver": {
                  "type": "array",
                  "items": {
                    "description": "A list of driver names to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "path": {
                  "type": "array",
                  "items": {
                    "description": "A list of paths to match against the ID_PATH udev property of devices",
                    "type": "string",
                    "additionalProperties": false
                  }
                }
              }
            }
          },
          "required": [
            "id"
          ]

Which means that for bringing up or down an specific connection / device could need an specific method or it could be handle as any other attribute.

Resource /network/wifi
  • GET: list of scanned WiFi networks
    Request example
$curl -s localhost:3000/api/network/wifi -H @headers.txt | jq
[
  "Agama",
  "Agama2"
]
Resource /network/system/apply
  • PUT: Apply the changes to the system
    Request example
curl -s -X PUT localhost:3000/api/network/system/apply -H @headers.txt | jq
null

Testing

Tested manually 
suse@vikingo-laptop:~$ ./generate_headers.sh 
Obtained token 'eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MTEwMzU4MDV9.nifdrxlq3ZKK4LoVjY-2sPb9ZzXIq5M3Xxqudy8zqvE'
suse@vikingo-laptop:~$ ./get_state.sh 
{
  "connectivity": true,
  "wireless_enabled": true,
  "networking_enabled": true
}
suse@vikingo-laptop:~$ ./get_connections.sh 
[
  {
    "id": "lo",
    "method4": "manual",
    "method6": "manual",
    "addresses": [
      "127.0.0.1/8",
      "::1"
    ],
    "interface": "lo"
  },
  {
    "id": "Wired connection 1",
    "method4": "manual",
    "gateway4": "192.168.0.1",
    "method6": "auto",
    "addresses": [
      "192.168.0.230/24"
    ],
    "nameservers": [
      "192.168.0.1"
    ],
    "interface": "eth0"
  },
  {
    "id": "AgamaNetwork",
    "method4": "disabled",
    "method6": "disabled",
    "wireless": {
      "password": "agama.test",
      "security": "wpa-psk",
      "ssid": "AgamaNetwork2",
      "mode": "infrastructure"
    }
  }
]
suse@vikingo-laptop:~$ ./get_wifis.sh 
[
  "AgamaNetwork",
  "AgamaNetwork2",
]

<details>
  <summary>Resource <b>/network/system/apply</b></summary>
  - PUT: Appl  list of scanned WiFi networks
  **Request example**
curl -s -X PUT localhost:3000/api/network/system/apply -H @headers.txt | jq

# Scripts used for obtain and modify the configuration

suse@vikingo-laptop:~$ cat generate_headers.sh 
#!/bin/bash

TOKEN=$(curl -s -X POST localhost:3000/api/auth -H 'Content-Type: application/json' -d '{"password": "agama.auth"}' | jq -r ".token")
echo "Obtained token '${TOKEN}'"

echo -n "Authorization: Bearer " >headers.txt
echo $TOKEN >>headers.txt
suse@vikingo-laptop:~$ cat get_connections.sh 
#!/bin/bash

curl -s localhost:3000/api/network/connections -H @headers.txt | jq
suse@vikingo-laptop:~$ cat get_wifis.sh 
#!/bin/bash

curl -s localhost:3000/api/network/wifi -H @headers.txt | jq
suse@vikingo-laptop:~$ cat get_state.sh 
#!/bin/bash

curl -s localhost:3000/api/network/state -H @headers.txt | jq
cat change_state.sh 
#!/bin/bash

curl -X POST localhost:3000/api/network/state -H @headers.txt -H "Content-Type: application/json" -d @state.json

Documentation PBI description

Problem

We would like to expose the Networking API over http which is currently not available.

Solution

Document an HTTP alternative as well as provide a POC exposing it.

API Design

Our Rest API could expose the general state and configuration of the network as well as the devices and connections resources below the network namespace as shown below:

Resource /network/state
  • GET: get the current network state and general information
Resource /network/config
  • GET: get the general configuration like wireless enabled or not
  • PATCH: update general configuration
Resource /network/devices
  • GET: list of known devices
    • Attributes
      • Name: String
      • Type: Enum

/network/devices/:name

  • GET: find a device by its name

Request example

$ curl -s localhost:3000/network/devices -H 'Authorization: Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDkzOTEzMDN9.RYUSttHGhGiqmSDy01Kf3-fripqohA3Li0pIneb-t_Y' | jq
[
  {
    "name": "enp2s0f0",
    "type": "Ethernet"
  },
  {
    "name": "wlp3s0",
    "type": "Wireless"
  },
  {
    "name": "eth0",
    "type": "Ethernet"
  }
Resource /network/connections
  • GET: list of managed / known connections
  • POST: create a new connection
    Request example
suse@vikingo-laptop:~$ curl -s localhost:3000/network/connections -H 'Authorization: Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDkzOTEzMDN9.RYUSttHGhGiqmSDy01Kf3-fripqohA3Li0pIneb-t_Y' | jq
[
{
  "id": "Wired connection 1",
  "uuid": "530d40c2-c580-43a7-b93e-c443702529a2",
  "mac_address": "",
  "ip_config": {
    "method4": "Manual",
    "method6": "Auto",
    "addresses": [
      "192.168.0.230/24"
    ],
    "nameservers": [
      "192.168.0.1"
    ],
    "gateway4": "192.168.0.1",
    "routes4": [
      {
        "destination": "0.0.0.0/24",
        "next_hop": "192.168.0.1"
      }
    ],
    "routes6": []
  },
  "status": "Up",
  "interface": "eth0",
  "port_config": "None",
  "match_config": {},
  "config": "Ethernet"
},
{
  "id": "AgamaTest",
  "uuid": "e7c684d5-f8e9-43a0-b303-decfe2883e53",
  "mac_address": "",
  "ip_config": {
    "method4": "Auto",
    "method6": "Auto",
    "routes4": [],
    "routes6": []
  },
  "status": "Up",
  "interface": "wlp3s0",
  "port_config": "None",
  "match_config": {},
  "config": {
    "Wireless": {
      "mode": "Infra",
      "ssid": "AgamaTest",
      "security": "WPA2",
      "wep_security": {
        "auth_alg": "Open",
        "wep_key_type": "Unknown",
        "wep_key_index": 0
      },
      "hidden": false
    }
  }
},
{
  "id": "AgamaTest2",
  "uuid": "bc323ed0-18d8-4f85-b90e-5b7907e3b711",
  "mac_address": "",
  "ip_config": {
    "method4": "Auto",
    "method6": "Auto",
    "routes4": [],
    "routes6": []
  },
  "status": "Up",
  "interface": "wlp3s0",
  "port_config": "None",
  "match_config": {},
  "config": {
    "Wireless": {
      "mode": "Infra",
      "ssid": "AgamaTest2",
      "security": "WPA2",
      "wep_security": {
        "auth_alg": "Open",
        "wep_key_type": "Unknown",
        "wep_key_index": 0
      },
      "hidden": false
    }
  }
},
{
  "id": "lo",
  "uuid": "66d7c54a-735a-42c6-b412-572a1134efec",
  "mac_address": "",
  "ip_config": {
    "method4": "Manual",
    "method6": "Manual",
    "addresses": [
      "127.0.0.1/8",
      "::1"
    ],
    "routes4": [],
    "routes6": []
  },
  "status": "Up",
  "interface": "lo",
  "port_config": "None",
  "match_config": {},
  "config": "Loopback"
}
]

/network/connections/:id

  • GET: find a connection by its uuid or id
  • PATCH: update connection
  • DELETE: remove the connection

The attributes for updating a connection or creating a new one could follow the same schema used for the profile:

      "connections": {
        "description": "Network connections to be defined",
        "type": "array",
        "items": {
          "type": "object",
          "additionalProperties": false,
          "properties": {
            "id": {
              "description": "Connection ID",
              "type": "string"
            },
            "interface": {
              "description": "The name of the network interface bound to this connection",
              "type": "string"
            },
            "mac-address": {
              "description": "Custom mac-address (can also be 'preserve', 'permanent', 'random' or 'stable')",
              "type": "string"
            },
            "method4": {
              "description": "IPv4 configuration method (e.g., 'auto')",
              "type": "string",
              "enum": [
                "auto",
                "manual",
                "link-local",
                "disabled"
              ]
            },
            "method6": {
              "description": "IPv6 configuration method (e.g., 'auto')",
              "type": "string",
              "enum": [
                "auto",
                "manual",
                "link-local",
                "disabled"
              ]
            },
            "gateway4": {
              "description": "Connection gateway address (e.g., '192.168.122.1')",
              "type": "string"
            },
            "gateway6": {
              "description": "Connection gateway address (e.g., '::ffff:c0a8:7a01')",
              "type": "string"
            },
            "addresses": {
              "type": "array",
              "items": {
                "description": "Connection addresses",
                "type": "string",
                "additionalProperties": false
              }
            },
            "nameservers": {
              "type": "array",
              "items": {
                "description": "Nameservers (IPv4 and/or IPv6 are allowed)",
                "type": "string",
                "additionalProperties": false
              }
            },
            "wireless": {
              "type": "object",
              "description": "Wireless configuration",
              "additionalProperties": false,
              "properties": {
                "password": {
                  "type": "string"
                },
                "security": {
                  "type": "string"
                },
                "ssid": {
                  "type": "string"
                },
                "mode": {
                  "type": "string",
                  "enum": [
                    "infrastructure",
                    "adhoc",
                    "mesh",
                    "ap"
                  ]
                }
              }
            },
            "bond": {
              "type": "object",
              "description": "Bonding configuration",
              "additionalProperties": false,
              "properties": {
                "mode": {
                  "type": "string"
                },
                "options": {
                  "type": "string"
                },
                "ports": {
                  "type": "array",
                  "items": {
                    "description": "A list of the interfaces or connections to be bonded",
                    "type": "string",
                    "additionalProperties": false
                  }
                }
              }
            },
            "match": {
              "type": "object",
              "description": "Match settings",
              "additionalProperties": false,
              "properties": {
                "kernel": {
                  "type": "array",
                  "items": {
                    "description": "A list of kernel command line arguments to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "interface": {
                  "type": "array",
                  "items": {
                    "description": "A list of interface names to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "driver": {
                  "type": "array",
                  "items": {
                    "description": "A list of driver names to match",
                    "type": "string",
                    "additionalProperties": false
                  }
                },
                "path": {
                  "type": "array",
                  "items": {
                    "description": "A list of paths to match against the ID_PATH udev property of devices",
                    "type": "string",
                    "additionalProperties": false
                  }
                }
              }
            }
          },
          "required": [
            "id"
          ]

Which means that for bringing up or down an specific connection / device could need an specific method or it could be handle as any other attribute.

Resource /network/wifi_networks
  • GET: list of scanned wifi networks

Note: Which attributes should be required and exposed needs to be discussed, and probably we should expose the persistent and running configuration maybe as part of the same resource or separately.

Example of the OpenAPI generated documentation based on the implemented POC code 
{
  "openapi": "3.0.3",
  "info": {
    "title": "agama-dbus-server",
    "description": "Agama web API description",
    "license": {
      "name": ""
    },
    "version": "0.1.0"
  },
  "paths": {
    "/network/connections": {
      "get": {
        "tags": [
          "crate::network::web"
        ],
        "operationId": "connections",
        "responses": {
          "200": {
            "description": "List of known connections",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Connection"
                  }
                }
              }
            }
          }
        }
      }
    },
    "/network/devices": {
      "get": {
        "tags": [
          "crate::network::web"
        ],
        "operationId": "devices",
        "responses": {
          "200": {
            "description": "List of devices",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Device"
                  }
                }
              }
            }
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Connection": {
        "type": "object",
        "description": "Represents a known network connection.",
        "required": [
          "id",
          "uuid",
          "mac_address",
          "ip_config",
          "status",
          "port_config",
          "match_config",
          "config"
        ],
        "properties": {
          "config": {
            "$ref": "#/components/schemas/ConnectionConfig"
          },
          "controller": {
            "allOf": [
              {
                "$ref": "#/components/schemas/Uuid"
              }
            ],
            "nullable": true
          },
          "id": {
            "type": "string"
          },
          "interface": {
            "type": "string",
            "nullable": true
          },
          "ip_config": {
            "$ref": "#/components/schemas/IpConfig"
          },
          "mac_address": {
            "$ref": "#/components/schemas/MacAddress"
          },
          "match_config": {
            "$ref": "#/components/schemas/MatchConfig"
          },
          "port_config": {
            "$ref": "#/components/schemas/PortConfig"
          },
          "status": {
            "$ref": "#/components/schemas/Status"
          },
          "uuid": {
            "$ref": "#/components/schemas/Uuid"
          }
        }
      },
      "Device": {
        "type": "object",
        "description": "Network device",
        "required": [
          "name",
          "type"
        ],
        "properties": {
          "name": {
            "type": "string"
          },
          "type": {
            "$ref": "#/components/schemas/DeviceType"
          }
        }
      },
      "DeviceType": {
        "type": "string",
        "enum": [
          "Loopback",
          "Ethernet",
          "Wireless",
          "Dummy",
          "Bond",
          "Vlan",
          "Bridge"
        ]
      },
      "NetworkState": {
        "type": "object",
        "required": [
          "devices",
          "connections"
        ],
        "properties": {
          "connections": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/Connection"
            }
          },
          "devices": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/Device"
            }
          }
        }
      }
    }
  }
}

Signals / Websocket notifications

With websockets we can carry more information than what is usually emitted over DBUS therefore we could subscribe to DBUS signals in the backend for the different resources submitting more information over the websocket, for example for any new device connected or any change in a connection property but have not gone over what to notify so far as we are not subscribing to signals in the backend at all and we are only sending when a connections has been added or removed.

Testing

Tested manually 
$ curl -s -X POST localhost:3000/authenticate -H 'Content-Type: application/json' -d '{"password": "your_password"}' | jq -r ".token"

eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDkzOTEzMDN9.RYUSttHGhGiqmSDy01Kf3-fripqohA3Li0pIneb-t_Y

$ curl -s localhost:3000/network/devices -H 'Authorization: Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDkzOTEzMDN9.RYUSttHGhGiqmSDy01Kf3-fripqohA3Li0pIneb-t_Y' | jq
[
  {
    "name": "enp2s0f0",
    "type": "Ethernet"
  },
  {
    "name": "wlp3s0",
    "type": "Wireless"
  },
  {
    "name": "eth0",
    "type": "Ethernet"
  }
suse@vikingo-laptop:~$ curl -s localhost:3000/network/connections -H 'Authorization: Bearer eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjE3MDkzOTEzMDN9.RYUSttHGhGiqmSDy01Kf3-fripqohA3Li0pIneb-t_Y' | jq
[
  {
    "id": "Wired connection 1",
    "uuid": "530d40c2-c580-43a7-b93e-c443702529a2",
    "mac_address": "",
    "ip_config": {
      "method4": "Manual",
      "method6": "Auto",
      "addresses": [
        "192.168.0.230/24"
      ],
      "nameservers": [
        "192.168.0.1"
      ],
      "gateway4": "192.168.0.1",
      "routes4": [
        {
          "destination": "0.0.0.0/24",
          "next_hop": "192.168.0.1"
        }
      ],
      "routes6": []
    },
    "status": "Up",
    "interface": "eth0",
    "port_config": "None",
    "match_config": {},
    "config": "Ethernet"
  },
  {
    "id": "AgamaTest",
    "uuid": "e7c684d5-f8e9-43a0-b303-decfe2883e53",
    "mac_address": "",
    "ip_config": {
      "method4": "Auto",
      "method6": "Auto",
      "routes4": [],
      "routes6": []
    },
    "status": "Up",
    "interface": "wlp3s0",
    "port_config": "None",
    "match_config": {},
    "config": {
      "Wireless": {
        "mode": "Infra",
        "ssid": "AgamaTest",
        "security": "WPA2",
        "wep_security": {
          "auth_alg": "Open",
          "wep_key_type": "Unknown",
          "wep_key_index": 0
        },
        "hidden": false
      }
    }
  },
  {
    "id": "AgamaTest2",
    "uuid": "bc323ed0-18d8-4f85-b90e-5b7907e3b711",
    "mac_address": "",
    "ip_config": {
      "method4": "Auto",
      "method6": "Auto",
      "routes4": [],
      "routes6": []
    },
    "status": "Up",
    "interface": "wlp3s0",
    "port_config": "None",
    "match_config": {},
    "config": {
      "Wireless": {
        "mode": "Infra",
        "ssid": "AgamaTest2",
        "security": "WPA2",
        "wep_security": {
          "auth_alg": "Open",
          "wep_key_type": "Unknown",
          "wep_key_index": 0
        },
        "hidden": false
      }
    }
  },
  {
    "id": "lo",
    "uuid": "66d7c54a-735a-42c6-b412-572a1134efec",
    "mac_address": "",
    "ip_config": {
      "method4": "Manual",
      "method6": "Manual",
      "addresses": [
        "127.0.0.1/8",
        "::1"
      ],
      "routes4": [],
      "routes6": []
    },
    "status": "Up",
    "interface": "lo",
    "port_config": "None",
    "match_config": {},
    "config": "Loopback"
  }
]

@coveralls
Copy link

coveralls commented Mar 1, 2024 •
edited
Loading

Coverage Status

coverage: 72.105% (-1.5%) from 73.596%
when pulling de963bd on network-service
into f02dc7d on architecture_2024.

@teclator teclator force-pushed the network-service branch 2 times, most recently from 1f47214 to 2ce4047 Compare March 5, 2024 09:03
@teclator teclator changed the base branch from master to architecture_2024 March 5, 2024 09:03
@teclator teclator marked this pull request as ready for review March 5, 2024 09:10
@teclator teclator requested a review from imobachgs March 5, 2024 09:10
@imobachgs imobachgs changed the title Expose the networking API over http Expose the networking API over HTTP Mar 5, 2024
imobachgs
imobachgs reviewed Mar 5, 2024
View reviewed changes
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

In general, it looks good. The only thing I am not convinced of is to expose the internal model directly. Perhaps we should convert the objects to a *Settings object, as we already discussed offline.

rust/agama-dbus-server/src/network/action.rs Outdated Show resolved Hide resolved
rust/agama-dbus-server/src/network/web.rs Outdated Show resolved Hide resolved
@teclator
Copy link
Contributor Author

teclator commented Mar 6, 2024

In general, it looks good. The only thing I am not convinced of is to expose the internal model directly. Perhaps we should convert the objects to a *Settings object, as we already discussed offline.

Yep, I'm also not convinced about it but as a POC it was good enough.

@teclator teclator force-pushed the network-service branch 2 times, most recently from 7d5698c to eee87a5 Compare March 7, 2024 18:40
@teclator teclator force-pushed the network-service branch 2 times, most recently from 4ea2a52 to f550483 Compare March 21, 2024 13:22
imobachgs
imobachgs requested changes Mar 22, 2024
View reviewed changes
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Great work! However, I have some comments (as usual) :-) I have not finished the review yet, but you can start looking into the suggestions.

rust/agama-server/src/network/action.rs
@@ -21,11 +21,18 @@ pub enum Action {
DeviceType,
Responder<Result<OwnedObjectPath, NetworkStateError>>,
),
/// Gets a connection
/// Add a new connection
NewConnection(
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent, it should be named AddConnection. I know it collides with the AddConnection action for the D-Bus part, but perhaps it is time to start dropping them. But we can handle that in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I planned to add this new method, but at the end used the AddConnection + UpdateConnection, but as you commented maybe it is the time to remove them and any DBUS specific action.

rust/agama-server/src/network/action.rs
GetConnectionPath(Uuid, Responder<Option<OwnedObjectPath>>),
/// Gets a connection
/// Gets a connection path by id
Copy link
Contributor

Choose a reason for hiding this comment

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

Another candidate to drop :-)

rust/agama-server/src/network/adapter.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/dbus/tree.rs
@@ -130,6 +130,10 @@ impl Tree {
self.objects.devices_paths()
}

pub fn device_path(&self, name: &str) -> Option<OwnedObjectPath> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think we want to evolve the D-Bus interface even more. We should be dropping most of it (and just keeping the signals).

rust/agama-server/src/network/model.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/nm/adapter.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs

state
.actions
.send(Action::AddConnection(
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO we should remove the need of having separate actions (AddConnection and UpdateConnection).

rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
rust/agama-server/src/network/web.rs Outdated Show resolved Hide resolved
@teclator teclator merged commit bc95d15 into architecture_2024 Mar 22, 2024
2 checks passed
@teclator teclator deleted the network-service branch March 22, 2024 16:37
@imobachgs imobachgs mentioned this pull request May 3, 2024
Merged
imobachgs added a commit that referenced this pull request May 6, 2024
@imobachgs
Merge architecture_2024 branch into master (#1177)
3353513 
After a few months of work, it is time to merge the `architecture_2024`
branch into `master`. It is still a work-in-progress, but all the
efforts should be go now against that branch.

## Pull requests

* #1061
* #1064
* #1073
* #1074
* #1080
* #1089
* #1091
* #1092
* #1094
* #1095
* #1099
* #1100
* #1102
* #1103
* #1112
* #1114
* #1116
* #1117
* #1119
* #1120
* #1123
* #1126
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1136
* #1139
* #1140
* #1143
* #1146

## Other commits

* 8efa41f
* 9e2dec0
@imobachgs imobachgs mentioned this pull request May 17, 2024
Merged
imobachgs added a commit that referenced this pull request May 17, 2024
@imobachgs
Update to release 8 (#1232)
b13e7b7 
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imobachgs imobachgs imobachgs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@teclator @coveralls @imobachgs