Re: [pdm-devel] [PATCH datacenter-manager v2 06/12] api: resources: list: add support for view-filter parameter

["Lukas Wagner" <l.wagner@proxmox.com>]

On Wed Nov 5, 2025 at 11:08 AM CET, Dominik Csapak wrote:
> see comment inline
>
> On 11/3/25 1:35 PM, Lukas Wagner wrote:
>> A view filter allows one to get filtered subset of all resources, based
>> on filter rules defined in a config file. View filters integrate with
>> the permission system - if a user has permissions on
>> /view/{view-filter-id}, then these privileges are transitively applied
>> to all resources which are matched by the rules. All other permission
>> checks are replaced if requesting data through a view filter.
>> 
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>   server/src/api/resources.rs  | 56 ++++++++++++++++++++++++++++++------
>>   server/src/resource_cache.rs |  3 +-
>>   2 files changed, 50 insertions(+), 9 deletions(-)
>> 
>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>> index 81c9d9ae..6feda45b 100644
>> --- a/server/src/api/resources.rs
>> +++ b/server/src/api/resources.rs
>> @@ -18,7 +18,7 @@ use pdm_api_types::resource::{
>>   use pdm_api_types::subscription::{
>>       NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel,
>>   };
>> -use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT};
>> +use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_FILTER_ID_SCHEMA};
>>   use pdm_search::{Search, SearchTerm};
>>   use proxmox_access_control::CachedUserInfo;
>>   use proxmox_router::{
>> @@ -30,8 +30,8 @@ use proxmox_sortable_macro::sortable;
>>   use proxmox_subscription::SubscriptionStatus;
>>   use pve_api_types::{ClusterResource, ClusterResourceType};
>>   
>> -use crate::connection;
>>   use crate::metric_collection::top_entities;
>> +use crate::{connection, views};
>>   
>>   pub const ROUTER: Router = Router::new()
>>       .get(&list_subdirs_api_method!(SUBDIRS))
>> @@ -221,6 +221,10 @@ impl From<RemoteWithResources> for RemoteResources {
>>                   type: ResourceType,
>>                   optional: true,
>>               },
>> +            "view-filter": {
>> +                schema: VIEW_FILTER_ID_SCHEMA,
>> +                optional: true,
>> +            },
>>           }
>>       },
>>       returns: {
>> @@ -236,10 +240,17 @@ pub async fn get_resources(
>>       max_age: u64,
>>       resource_type: Option<ResourceType>,
>>       search: Option<String>,
>> +    view_filter: Option<String>,
>>       rpcenv: &mut dyn RpcEnvironment,
>>   ) -> Result<Vec<RemoteResources>, Error> {
>> -    let remotes_with_resources =
>> -        get_resources_impl(max_age, search, resource_type, Some(rpcenv)).await?;
>> +    let remotes_with_resources = get_resources_impl(
>> +        max_age,
>> +        search,
>> +        resource_type,
>> +        view_filter.as_deref(),
>> +        Some(rpcenv),
>> +    )
>> +    .await?;
>>       let resources = remotes_with_resources.into_iter().map(Into::into).collect();
>>       Ok(resources)
>>   }
>> @@ -276,6 +287,7 @@ pub(crate) async fn get_resources_impl(
>>       max_age: u64,
>>       search: Option<String>,
>>       resource_type: Option<ResourceType>,
>> +    view_filter: Option<&str>,
>>       rpcenv: Option<&mut dyn RpcEnvironment>,
>>   ) -> Result<Vec<RemoteWithResources>, Error> {
>>       let user_info = CachedUserInfo::new()?;
>> @@ -285,9 +297,15 @@ pub(crate) async fn get_resources_impl(
>>               .get_auth_id()
>>               .ok_or_else(|| format_err!("no authid available"))?
>>               .parse()?;
>> -        if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
>> +
>> +        // NOTE: Assumption is that the regular permission check is completely replaced by a check
>> +        // on the view ACL object *if* a view parameter is passed.
>> +        if let Some(view_filter) = &view_filter {
>> +            user_info.check_privs(&auth_id, &["view", view_filter], PRIV_RESOURCE_AUDIT, false)?;
>> +        } else if !user_info.any_privs_below(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT)? {
>>               http_bail!(UNAUTHORIZED, "user has no access to resources");
>>           }
>> +
>>           opt_auth_id = Some(auth_id);
>>       }
>>   
>> @@ -296,12 +314,24 @@ pub(crate) async fn get_resources_impl(
>>   
>>       let filters = search.map(Search::from).unwrap_or_default();
>>   
>> +    let view_filter = view_filter
>> +        .map(|filter_name| views::view_filter::get_view_filter(&filter_name))
>> +        .transpose()?;
>> +
>>       let remotes_only = is_remotes_only(&filters);
>>   
>>       for (remote_name, remote) in remotes_config {
>>           if let Some(ref auth_id) = opt_auth_id {
>> -            let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
>> -            if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
>> +            if view_filter.is_none() {
>> +                let remote_privs = user_info.lookup_privs(auth_id, &["resource", &remote_name]);
>> +                if remote_privs & PRIV_RESOURCE_AUDIT == 0 {
>> +                    continue;
>> +                }
>> +            }
>> +        }
>> +
>> +        if let Some(view_filter) = &view_filter {
>> +            if view_filter.can_skip_remote(&remote_name) {
>>                   continue;
>>               }
>>           }
>> @@ -374,6 +404,15 @@ pub(crate) async fn get_resources_impl(
>>           }
>>       }
>>   
>> +    if let Some(filter) = &view_filter {
>> +        remote_resources.retain_mut(|r| {
>> +            r.resources
>> +                .retain(|resource| filter.resource_matches(&r.remote_name, resource));
>> +
>> +            !r.resources.is_empty()
>
> this leads to remotes that have an error are not being returned.
>
> this should probably only return false when either
> the list was not empty before but is after
> or if r.error.is_none()
>

With the problem we discussed in the other patch, this now becomes

        remote_resources.retain_mut(|r| {
            r.resources
                .retain(|resource| view.resource_matches(&r.remote_name, resource));

            let has_any_matched_resources = !r.resources.is_empty();
            has_any_matched_resources
                || (r.error.is_some() && view.is_remote_explicitly_included(&r.remote_name))
        });

in order to avoid leaking failed remotes if they are not explicitly
included in the ruleset.


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