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

[Lou.Lecrivain at wdz.de]

Regarding the Accept header,

 >> sub default_headers {
 >>     my ($plugin_config) = @_;
 >>     my $token = $plugin_config->{token};
 >>     return [
 >>     'Content-Type' => "application/json", 'Authorization' => "token
$token",
 >>     'Accept' => "application/json"
 >>     ];

> Another question, why did you set Accept to application/json here, is
> that necessary?

I simply followed the documentation:
https://docs.nautobot.com/projects/core/en/stable/user-guide/platform-functionality/rest-api/overview/#creating-a-new-object

They recommend it (and it's also needed if you need to use the API versioning feature), but afaik it's not *mandatory*. Feel free to delete if not necessary.

BR

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

On 3/19/25 10:50, Lou.Lecrivain at wdz.de wrote:
> 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.

Sounds sensible, thanks for the clarification!

 > sub default_headers {
 >     my ($plugin_config) = @_;
 >     my $token = $plugin_config->{token};
 >     return [
 >     'Content-Type' => "application/json", 'Authorization' => "token
$token",
 >     'Accept' => "application/json"
 >     ];

Another question, why did you set Accept to application/json here, is
that necessary?

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