Re: [pve-devel] [!!ACHTUNG extern!!] - Re: [!!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: 16879 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: [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin
Date: Thu, 20 Mar 2025 09:22:12 +0000
Message-ID: <FR2PPF45D00627086D700C9AD27A7B44F2A85D82@FR2PPF45D006270.DEUP281.PROD.OUTLOOK.COM>

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@proxmox.com>
Envoyé : mercredi 19 mars 2025 17:01
À : Lecrivain, Lou (WDZ) <Lou.Lecrivain@wdz.de>; 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>
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@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@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