Re: [PATCH pve-network v3 9/9] api: add dry-run endpoint for sdn apply to preview changes

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

On 06.03.2026 10:24, Stefan Hanreich wrote:
> On 3/5/26 11:04 AM, Gabriel Goller wrote:
> > Allows users to see the diff of frr and interfaces configuration files
> > before applying SDN changes. Previously this was not possible and the
> > user had to apply and then see what changed. For the ifupdown2 dry-run
> > to work, pull out the running_config generation to the parent functions.
> > 
> > Also add an option to the compile_running_cfg function so that we can
> > skip the /etc/network/interfaces.d/sdn file version bump. This means
> > when nothing has been changed and dry-run is pressed, nothing will be
> > shown.
> > 
> > Rename a few constants so that they don't clash with local variables.
> > 
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> > ---
> >  src/PVE/API2/Network/SDN.pm      | 88 ++++++++++++++++++++++++++++++++
> >  src/PVE/Network/SDN.pm           | 36 ++++++++-----
> >  src/PVE/Network/SDN/Fabrics.pm   | 10 +++-
> >  src/PVE/Network/SDN/Zones.pm     |  3 +-
> >  src/test/debug/generateconfig.pl |  3 +-
> >  src/test/run_test_zones.pl       |  2 +-
> >  6 files changed, 124 insertions(+), 18 deletions(-)
> > [snip]
> > +__PACKAGE__->register_method({
> > +    name => 'dry-run',
> > +    path => 'dry-run',
> > +    method => 'PUT',
> 
> Should this really be a PUT method? GET would imo be better suited,
> since it doesn't have any side-effects. The node param could just be
> part of the path instead of sending it in the body.
> 

Done, thanks for the review!

> > +    permissions => {
> > +        check => ['perm', '/nodes/{node}', ['Sys.Modify']],
> > +    },
> > +    description =>
> > +        "Dry-run the SDN apply action and return the difference between the current configuration and the pending configuration",
> > +    protected => 1,
> > +    proxyto => 'node',
> > +    parameters => {
> > +        additionalProperties => 0,
> > +        properties => {
> > +            node => get_standard_option('pve-node'),
> > +        },
> > +    },
> > +
> > +    returns => {
> > +        type => 'object',
> > +        properties => {
> > +            "frr-diff" => {
> > +                type => 'string',
> > +                description =>
> > +                    'The difference between the current and pending FRR configuration.',
> > +            },
> > +            "interfaces-diff" => {
> > +                type => 'string',
> > +                description =>
> > +                    'The difference between the current and pending /etc/network/interfaces.d/sdn configuration.',
> > +            },
> > +        },
> > [snip]