Fix networking test problems since #2355

[frankiejol]

Fixed new network tests failed due to PR #2355

[Copilot]

Pull request overview

This PR updates the networking-related Mojolicious tests to align with the node-selection workflow introduced around PR #2355 (“choose node”), and adjusts the /v3/choose_node/:id authorization to allow operator-level users (not only admins) to select a node in session.

Changes:

• Update network tests to call /v3/choose_node/:id before creating networks via /v2/network/new.
• Adjust test URLs for the updated /v2/network/new API shape.
• Broaden /v3/choose_node/:id access control from admin-only to operator.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
t/mojo/31_networks.t	Updates network tests to select a node before creating networks and adjusts API calls accordingly.
script/rvd_front	Changes /v3/choose_node/:id authorization from is_admin to is_operator.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

script/rvd_front:1036

• The access check for /v3/choose_node was broadened from admin-only to is_operator. In this codebase, is_operator becomes true for users who can list/create their own machines (via can_list_own_machines -> can_create_machine), so this endpoint becomes reachable for many non-admin users. Since this endpoint can change session(id_node) and list_nodes_active() returns all active/enabled nodes (no per-user filtering), consider restricting access to the specific permissions that actually need node selection (e.g., admins and network managers) to avoid unintended node-context switching by regular users.

get '/v3/choose_node/(#id)' => sub($c) {

    return access_denied_json($c) if !$USER || !$USER->is_operator;
    return json_error($c,"Missing node in ".$c->req->url->to_abs->path, 400)

t/mojo/31_networks.t:200

• This test decodes the response from POST /v2/network/new immediately afterwards, but it doesn't assert the response status. If the request fails (e.g. 403/500), decode_json may still return undef or fail in a less clear way. Adding an explicit status_is(200) here (like in test_networks_access_grant) will make failures easier to diagnose and ensure the test is actually validating successful creation.

    $t->get_ok("/v3/choose_node/".$id_vm)->status_is(200);
    $t->post_ok("/v2/network/new" => json => { name => base_domain_name() });
    my $data = decode_json($t->tx->res->body);

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

script/rvd_front:4678

• In shutdown_machine, the non-force path still does $c->param('force') eq 'true' without guarding for an undefined param. This can still trigger 'uninitialized value' warnings (the force path was guarded, but the unless check wasn't). Consider normalizing the param once and branching on it so both paths are warning-free and consistent.

        my $req;
        $req = Ravada::Request->force_shutdown_domain(id_domain => $domain->id, uid => $USER->id)
            if ($c->param('force') && $c->param('force') eq 'true');
        $req = Ravada::Request->shutdown_domain(id_domain => $domain->id, uid => $USER->id)
            unless ($c->param('force') eq 'true');

t/mojo/93_choose_node.t:359

• test_choose_node now gates the storage creation path behind $create_storage, but the test suite never calls test_choose_node with a true value (the main call is test_choose_node($t) and the operator call explicitly passes 0). This leaves the test_new(...,'storage',...) branch effectively untested and potentially stale.

test_choose_node_wrong($t);
test_node_gone($t);
test_choose_node($t);
test_connect_node($t);

t/mojo/31_networks.t:93

• The access-control test URL list no longer exercises the API endpoint used to create networks (/v2/network/new). Since /v2/network/new is now the entry point (and is permission-gated), it should be included in @urls so the 403/200 expectations cover the new route as well.

    my @urls =(
        "/admin/networks", "/network/new"
        , "/v2/vm/list_networks/$id_vm");