Re: [PATCH pve-network 5/9] fix #7294: sdn: register api formats for zones and vnets

[Gabriel Goller <g.goller@proxmox.com>]

Generally on this series:
What was your rationale on adding the vlan tag? IMO having
`vnet/<zone>/<vnet>/<vlan>` doesn't really make sense, as not all vnets have
a tag property, and it's also not always a vlan e.g. EVPN vnets have a vni-tag
property.

One small comment inline as well.

> [snip]
> diff --git a/src/PVE/Network/SDN/VnetPlugin.pm b/src/PVE/Network/SDN/VnetPlugin.pm
> index e04157573083..2299b46601c2 100644
> --- a/src/PVE/Network/SDN/VnetPlugin.pm
> +++ b/src/PVE/Network/SDN/VnetPlugin.pm
> @@ -16,17 +16,34 @@ PVE::Cluster::cfs_register_file(
>      sub { __PACKAGE__->write_config(@_); },
>  );
>  
> +my $sdn_vnet_id_pattern = '[a-zA-Z][a-zA-Z0-9]*[a-zA-Z0-9]';
> +my $vnet_min_length = 2;
> +my $vnet_max_length = 8;
> +
>  PVE::JSONSchema::register_standard_option(
>      'pve-sdn-vnet-id',
>      {
>          description => "The SDN vnet object identifier.",
>          type => 'string',
> -        pattern => '[a-zA-Z][a-zA-Z0-9]*[a-zA-Z0-9]',
> -        minLength => 2,
> -        maxLength => 8,
> +        pattern => $sdn_vnet_id_pattern,
> +        minLength => $vnet_min_length,
> +        maxLength => $vnet_max_length,
>      },
>  );
>  
> +sub pve_verify_sdn_vnet_id {
> +    my ($vnet, $noerr) = @_;
> +
> +    if ($vnet !~ m/^$sdn_vnet_id_pattern$/) {
> +        return undef if $noerr;
> +        die "invalid SDN VNet '$vnet' - must be $vnet_min_length-$vnet_max_length characters"
> +            . " long, start with a letter, and contain only alphanumeric characters\n";
> +    }
> +    return $vnet;
> +}

I think this is missing a min/max lenght check?

> +
> +PVE::JSONSchema::register_format('pve-sdn-vnet-id', \&pve_verify_sdn_vnet_id);
> +
>  my $defaultData = {
>  
>      propertyList => {
> diff --git a/src/PVE/Network/SDN/Zones/Plugin.pm b/src/PVE/Network/SDN/Zones/Plugin.pm
> index 74a3384cd7ae..cd761e0448c3 100644
> --- a/src/PVE/Network/SDN/Zones/Plugin.pm
> +++ b/src/PVE/Network/SDN/Zones/Plugin.pm
> @@ -19,17 +19,34 @@ PVE::Cluster::cfs_register_file(
>      sub { __PACKAGE__->write_config(@_); },
>  );
>  
> +my $sdn_zone_id_pattern = '[a-zA-Z][a-zA-Z0-9]*[a-zA-Z0-9]';
> +my $zone_min_length = 2;
> +my $zone_max_length = 8;
> +
>  PVE::JSONSchema::register_standard_option(
>      'pve-sdn-zone-id',
>      {
>          description => "The SDN zone object identifier.",
>          type => 'string',
> -        pattern => '[a-zA-Z][a-zA-Z0-9]*[a-zA-Z0-9]',
> -        minLength => 2,
> -        maxLength => 8,
> +        pattern => $sdn_zone_id_pattern,
> +        minLength => $zone_min_length,
> +        maxLength => $zone_max_length,
>      },
>  );
>  
> +sub pve_verify_sdn_zone_id {
> +    my ($zone, $noerr) = @_;
> +
> +    if ($zone !~ m/^$sdn_zone_id_pattern$/) {
> +        return undef if $noerr;
> +        die "invalid SDN zone '$zone' - must be $zone_min_length-$zone_max_length characters"
> +            . " long, start with a letter, and contain only alphanumeric characters\n";
> +    }
> +    return $zone;
> +}

Here as well.

-- 
Gabriel Goller <g.goller@proxmox.com>