Re: [pve-devel] [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin

[Lou Lecrivain via pve-devel <pve-devel@lists.proxmox.com>]

[-- Attachment #1: Type: message/rfc822, Size: 14206 bytes --]

From: <Lou.Lecrivain@wdz.de>
To: <h.duerr@proxmox.com>, <lou.lecrivain@orange.fr>, <pve-devel@lists.proxmox.com>
Cc: <jonatan.crystall@gwdg.de>
Subject: RE: [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin
Date: Wed, 19 Mar 2025 09:50:27 +0000
Message-ID: <FR2PPF45D006270BAE05B48B2B7C1D2E18F85D92@FR2PPF45D006270.DEUP281.PROD.OUTLOOK.COM>

Re,

Ok so. Answering on that.

By default, the status "Active" already exists for fresh installs. However,
since the users are free to define whatever status they want (and delete the
pre-existing ones!), I thought it'd be best if we checked it did exist first.
There could be a case where we use a Nautobot instance where the status
"Active" does not exist. But I agree it's a bit far-fetched.

I wrote it this way since IMO it is better to have an explicit error message
rather than something like "incorrect API request" on subsequent API calls with
potentially no more details.

HTH

________________________________________
De : Hannes Dürr <h.duerr@proxmox.com>
Envoyé : mercredi 19 mars 2025 10:21
À : lou.lecrivain@orange.fr <lou.lecrivain@orange.fr>; pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>
Cc : jonatan.crystall@gwdg.de <jonatan.crystall@gwdg.de>; Lecrivain, Lou (WDZ) <Lou.Lecrivain@wdz.de>
Objet : [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin
 

On 3/6/25 12:08, lou.lecrivain@orange.fr wrote:

[...]

> +sub verify_api {
> +    my ($class, $plugin_config) = @_;
> +
> +    my $url = $plugin_config->{url};
> +    my $namespace = $plugin_config->{namespace};
> +    my $headers = default_headers($plugin_config);
> +
> +    # check that the namespace exists AND that default IP active status
> +    # exists AND that we have indeed API access
> +    eval {
> +     get_namespace_id($url, $namespace, $headers, 0)
> +         // die "namespace $namespace does not exist";
> +     get_status_id($url, default_ip_status(), $headers, 0)
> +         // die "default IP status ". default_ip_status() . " not found";
> +    };
> +    if ($@) {
> +     die "Can't use nautobot api: $@";
> +    }
> +}
> +

[...]

> +sub get_status_id {
> +    my ($url, $status, $headers, $noerr) = @_;
> +
> +    my $result = eval {
> +     return PVE::Network::SDN::api_request(
> +         "GET", "$url/extras/statuses/?q=$status", $headers);
> +    };
Unfortunately, I don't quite understand what this check actually does,
or why we need this check.
The documentation [0] says:
`Nautobot provides the ability for custom statuses to be defined
within an organisation to be used on various objects to facilitate
business workflows around object statuses.`

And as I understand the API call, it checks whether the
status object ‘Active’ exists and gets its ID. But I don't
understand why this should be checked with `verify_api`?
In my opinion, we don't really need this check, unless I'm missing
something here.

[0]
https://docs.nautobot.com/projects/core/en/stable/user-guide/platform-functionality/status/

> +    if ($@) {
> +     die "error while querying for status $status id: $@" if !$noerr;
> +    }
> +
> +    my $data = @{$result->{results}}[0];
> +    my $internalid = $data->{id};
> +    return $internalid;
> +}


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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