From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pdm-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 3F04D1FF16E
	for <inbox@lore.proxmox.com>; Mon, 14 Apr 2025 14:01:01 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id EC10E31A33;
	Mon, 14 Apr 2025 14:01:00 +0200 (CEST)
From: Gabriel Goller <g.goller@proxmox.com>
To: pdm-devel@lists.proxmox.com
Date: Mon, 14 Apr 2025 14:00:46 +0200
Message-Id: <20250414120046.486853-5-g.goller@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250414120046.486853-1-g.goller@proxmox.com>
References: <20250414120046.486853-1-g.goller@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.021 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pdm-devel] [PATCH proxmox-datacenter-manager 1/1] remotes: remove
 direct access on underlying sections HashMap
X-BeenThere: pdm-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Datacenter Manager development discussion
 <pdm-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pdm-devel/>
List-Post: <mailto:pdm-devel@lists.proxmox.com>
List-Help: <mailto:pdm-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Datacenter Manager development discussion
 <pdm-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pdm-devel-bounces@lists.proxmox.com
Sender: "pdm-devel" <pdm-devel-bounces@lists.proxmox.com>

Since accessing the underlying SectionConfigData<T> HashMap poses a lot
of risks (missing order insertion, removal) it has been made private and
the DerefMut implementation was removed. Use either Deref for read
access or use the implemented helper functions (insert, get_mut, etc.).
Instead of iterating over the HashMap, use the SectionConfigData
iterator, which will iterate in the correct order as configured in the
`order` vector.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 server/src/api/resources.rs            | 2 +-
 server/src/metric_collection/mod.rs    | 8 ++++----
 server/src/remote_tasks/mod.rs         | 6 +++---
 server/src/test_support/fake_remote.rs | 9 +++------
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 453d9e8ea6e5..6a8c8ef25b71 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -320,7 +320,7 @@ async fn get_top_entities(timeframe: Option<RrdTimeframe>) -> Result<TopEntities
     let (remotes_config, _) = pdm_config::remotes::config()?;
 
     let timeframe = timeframe.unwrap_or(RrdTimeframe::Day);
-    let res = crate::metric_collection::calculate_top(&remotes_config.sections, timeframe, 10);
+    let res = crate::metric_collection::calculate_top(&remotes_config, timeframe, 10);
     Ok(res)
 }
 
diff --git a/server/src/metric_collection/mod.rs b/server/src/metric_collection/mod.rs
index b37d678250a0..726884e5f732 100644
--- a/server/src/metric_collection/mod.rs
+++ b/server/src/metric_collection/mod.rs
@@ -49,14 +49,14 @@ async fn metric_collection_task() -> Result<(), Error> {
             }
         };
 
-        for (remote_name, remote) in &remotes.sections {
-            let start_time = *most_recent_timestamps.get(remote_name).unwrap_or(&0);
+        for (remote_name, remote) in remotes.into_iter() {
+            let start_time = *most_recent_timestamps.get(&remote_name).unwrap_or(&0);
             let remote_name_clone = remote_name.clone();
 
             let res = async {
                 let most_recent_timestamp = match remote.ty {
                     RemoteType::Pve => {
-                        let client = connection::make_pve_client(remote)?;
+                        let client = connection::make_pve_client(&remote)?;
                         let metrics = client
                             .cluster_metrics_export(Some(true), Some(false), Some(start_time))
                             .await?;
@@ -76,7 +76,7 @@ async fn metric_collection_task() -> Result<(), Error> {
                         .await
                     }
                     RemoteType::Pbs => {
-                        let client = connection::make_pbs_client(remote)?;
+                        let client = connection::make_pbs_client(&remote)?;
                         let metrics = client.metrics(Some(true), Some(start_time)).await?;
 
                         // Involves some blocking file IO
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index 7ded540845f2..234ffa76921b 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -36,10 +36,10 @@ pub async fn get_tasks(max_age: i64, filters: TaskFilters) -> Result<Vec<TaskLis
     // Room for improvements in the future.
     invalidate_cache_for_finished_tasks(&mut cache);
 
-    for (remote_name, remote) in &remotes.sections {
+    for (remote_name, remote) in remotes.iter() {
         let now = proxmox_time::epoch_i64();
 
-        if let Some(tasks) = cache.get_tasks(remote_name.as_str(), now, max_age) {
+        if let Some(tasks) = cache.get_tasks(remote_name, now, max_age) {
             // Data in cache is recent enough and has not been invalidated.
             all_tasks.extend(tasks);
         } else {
@@ -50,7 +50,7 @@ pub async fn get_tasks(max_age: i64, filters: TaskFilters) -> Result<Vec<TaskLis
                     continue;
                 }
             };
-            cache.set_tasks(remote_name.as_str(), tasks.clone(), now);
+            cache.set_tasks(remote_name, tasks.clone(), now);
 
             all_tasks.extend(tasks);
         }
diff --git a/server/src/test_support/fake_remote.rs b/server/src/test_support/fake_remote.rs
index 40f688074da0..0161d8e6e5f7 100644
--- a/server/src/test_support/fake_remote.rs
+++ b/server/src/test_support/fake_remote.rs
@@ -27,13 +27,12 @@ pub struct FakeRemoteConfig {
 
 impl RemoteConfig for FakeRemoteConfig {
     fn config(&self) -> Result<(SectionConfigData<Remote>, ConfigDigest), Error> {
-        let mut order = Vec::new();
-        let mut sections = HashMap::new();
+        let mut section_config = SectionConfigData::default();
 
         for i in 0..self.nr_of_pve_remotes {
             let name = format!("pve-{i}");
 
-            sections.insert(
+            section_config.insert(
                 name.clone(),
                 Remote {
                     ty: pdm_api_types::remotes::RemoteType::Pve,
@@ -44,13 +43,11 @@ impl RemoteConfig for FakeRemoteConfig {
                     web_url: None,
                 },
             );
-
-            order.push(name);
         }
 
         let digest = [0u8; 32].into();
 
-        Ok((SectionConfigData { sections, order }, digest))
+        Ok((section_config, digest))
     }
 
     fn lock_config(&self) -> Result<ApiLockGuard, Error> {
-- 
2.39.5



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