Re: [pdm-devel] [PATCH pve-network v3 6/6] api: zones: update schema of endpoints

["Shannon Sterz" <s.sterz@proxmox.com>]

small comments in-line

On Tue Sep 2, 2025 at 4:09 PM CEST, Stefan Hanreich wrote:
> The possible properties returned by the zone endpoints were only
> partly documented. Add all missing properties and improve descriptions
> for existing properties.
>
> Extract all duplicate properties into a separate variable, so we
> don't have to rewrite the whole API definition for every endpoint.
>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/API2/Network/SDN/Zones.pm        | 203 +++++++++++++++++++++--
>  src/PVE/Network/SDN/Zones/EvpnPlugin.pm  |  22 ++-
>  src/PVE/Network/SDN/Zones/QinQPlugin.pm  |   6 +-
>  src/PVE/Network/SDN/Zones/VlanPlugin.pm  |   1 +
>  src/PVE/Network/SDN/Zones/VxlanPlugin.pm |  15 +-
>  5 files changed, 218 insertions(+), 29 deletions(-)
>

-->8 snip 8<--

> +    'vxlan-port' => {
> +        description =>
> +            "UDP port that should be used for the VXLAN tunnel (default 4789). VXLAN zone only.",

wouldn't it make sense to define a `default` property here then? so:

    default => 4789,

-->8 snip 8<--

>          'rt-import' => {
>              type => 'string',
> -            description => "Route-Target import",
> +            description =>
> +                'List of Route Targets that should be imported into the VRF of the zone',

nit: you added a period to all other descriptions here, so end this
sentence with one too?

>              optional => 1,
>              format => 'pve-sdn-bgp-rt-list',
>          },
> diff --git a/src/PVE/Network/SDN/Zones/QinQPlugin.pm b/src/PVE/Network/SDN/Zones/QinQPlugin.pm
> index 5806e69..3c72d35 100644
> --- a/src/PVE/Network/SDN/Zones/QinQPlugin.pm
> +++ b/src/PVE/Network/SDN/Zones/QinQPlugin.pm
> @@ -18,11 +18,11 @@ sub properties {
>          tag => {
>              type => 'integer',
>              minimum => 0,
> -            description => "Service-VLAN Tag",
> +            description => "Service-VLAN Tag (outer VLAN)",
>          },

since this is so short, i think it's probably fine to leave that without
a period here.

>          mtu => {
>              type => 'integer',
> -            description => "MTU",
> +            description => "MTU of the zone, will be used for the created VNet bridges.",
>              optional => 1,
>          },
>          'vlan-protocol' => {
> @@ -30,6 +30,8 @@ sub properties {
>              enum => ['802.1q', '802.1ad'],
>              default => '802.1q',
>              optional => 1,
> +            description =>
> +                "Which VLAN protocol should be used for the creation of the QinQ zone",

nit: add a period here too?

>          },
>      };
>  }
> diff --git a/src/PVE/Network/SDN/Zones/VlanPlugin.pm b/src/PVE/Network/SDN/Zones/VlanPlugin.pm
> index 90f16bf..9d6932f 100644
> --- a/src/PVE/Network/SDN/Zones/VlanPlugin.pm
> +++ b/src/PVE/Network/SDN/Zones/VlanPlugin.pm
> @@ -27,6 +27,7 @@ sub properties {
>      return {
>          'bridge' => {
>              type => 'string',
> +            description => 'the bridge for which VLANs should be managed',

nit: capitalize beginning of sentence and end it in a period?

>          },
>          'bridge-disable-mac-learning' => {
>              type => 'boolean',
> diff --git a/src/PVE/Network/SDN/Zones/VxlanPlugin.pm b/src/PVE/Network/SDN/Zones/VxlanPlugin.pm
> index 8f6fba0..7ab89da 100644
> --- a/src/PVE/Network/SDN/Zones/VxlanPlugin.pm
> +++ b/src/PVE/Network/SDN/Zones/VxlanPlugin.pm
> @@ -27,21 +27,22 @@ sub type {
>  sub properties {
>      return {
>          'peers' => {
> -            description => "peers address list.",
> +            description =>
> +                "Comma-separated list of peers, that are part of the VXLAN zone. Usually the IPs of the nodes.",
>              type => 'string',
>              format => 'ip-list',
>          },
> -        fabric => {
> -            description => "SDN fabric to use as underlay for this VXLAN zone.",
> -            type => 'string',
> -            format => 'pve-sdn-fabric-id',
> -        },
>          'vxlan-port' => {
> -            description => "Vxlan tunnel udp port (default 4789).",
> +            description => "UDP port that should be used for the VXLAN tunnel (default 4789).",
>              minimum => 1,
>              maximum => 65536,

could also define the default here too?

>              type => 'integer',
>          },
> +        fabric => {
> +            description => "SDN fabric to use as underlay for this VXLAN zone.",
> +            type => 'string',
> +            format => 'pve-sdn-fabric-id',
> +        },
>      };
>  }
>



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel