From 987ade38cd3003c04b8aff5b6b4f6ab73affde8a Mon Sep 17 00:00:00 2001
From: Anna <anna.yafi@datadoghq.com>
Date: Wed, 10 Apr 2024 14:23:34 +0200
Subject: [PATCH] Document important method

---
 .../AppSec/Rcm/ConfigurationStatus.cs         | 21 ++++++++++++++-----
 tracer/src/Datadog.Trace/AppSec/Security.cs   |  2 +-
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs
index 4f9e05998a3e..dee90d181051 100644
--- a/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs
+++ b/tracer/src/Datadog.Trace/AppSec/Rcm/ConfigurationStatus.cs
@@ -8,21 +8,24 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using Datadog.Trace.AppSec.Coordinator;
 using Datadog.Trace.AppSec.Rcm.Models.Asm;
 using Datadog.Trace.AppSec.Rcm.Models.AsmData;
 using Datadog.Trace.AppSec.Rcm.Models.AsmDd;
 using Datadog.Trace.AppSec.Rcm.Models.AsmFeatures;
 using Datadog.Trace.AppSec.Waf.Initialization;
 using Datadog.Trace.ExtensionMethods;
-using Datadog.Trace.Logging;
 using Datadog.Trace.RemoteConfigurationManagement;
 using Datadog.Trace.Vendors.Newtonsoft.Json.Linq;
-using Datadog.Trace.Vendors.Serilog;
 using Action = Datadog.Trace.AppSec.Rcm.Models.Asm.Action;
 
 namespace Datadog.Trace.AppSec.Rcm;
 
+/// <summary>
+/// This class represents the state of RCM for ASM.
+/// It has 2 possible status:
+/// - ASM is not activated, and _fileUpdates/_fileRemoves contain some pending non-deserialized changes to apply when ASM_FEATURES activate ASM. Every time an RC payload is received here, pending changes are reset to the last ones
+/// - ASM is activated, stored configs in _fileUpdates/_fileRemoves are applied every time.
+/// </summary>
 internal record ConfigurationStatus
 {
     internal const string WafRulesKey = "rules";
@@ -149,6 +152,7 @@ internal Dictionary<string, object> BuildDictionaryForWafAccordingToIncomingUpda
     /// </summary>
     public void ApplyStoredFiles()
     {
+        // no need to clear _fileUpdates / _fileRemoves after they've been applied, as when we receive a new config, `StoreLastConfigState` method will clear anything remaining anyway.
         foreach (var updater in _productConfigUpdaters)
         {
             var fileUpdates = _fileUpdates.TryGetValue(updater.Key, out var value);
@@ -165,9 +169,16 @@ public void ApplyStoredFiles()
         }
     }
 
-    public bool StoreConfigs(Dictionary<string, List<RemoteConfiguration>> configsByProduct, Dictionary<string, List<RemoteConfigurationPath>>? removedConfigs)
+    /// <summary>
+    /// This method just stores the config state without deserializing anything, this state will be ready to use and deserialized if ASM is enabled later on.
+    /// This method considers that RC sends us everything again, the whole state together. That's why it's clearing all unapplied updates / removals before processing the last ones received.
+    /// In case ASM remained disabled, we discard previous updates and removals stored here that were never applied.
+    /// </summary>
+    /// <param name="configsByProduct">configsByProduct</param>
+    /// <param name="removedConfigs">removedConfigs</param>
+    /// <returns>whether or not there is any change, i.e any update/removal</returns>
+    public bool StoreLastConfigState(Dictionary<string, List<RemoteConfiguration>> configsByProduct, Dictionary<string, List<RemoteConfigurationPath>>? removedConfigs)
     {
-        // todo make sure remote config sends everything again, normally yes
         _fileUpdates.Clear();
         _fileRemoves.Clear();
         List<RemoteConfiguration> asmFeaturesToUpdate = new();
diff --git a/tracer/src/Datadog.Trace/AppSec/Security.cs b/tracer/src/Datadog.Trace/AppSec/Security.cs
index 937c8afde125..37f4f97a3edd 100644
--- a/tracer/src/Datadog.Trace/AppSec/Security.cs
+++ b/tracer/src/Datadog.Trace/AppSec/Security.cs
@@ -171,7 +171,7 @@ private ApplyDetails[] UpdateFromRcm(Dictionary<string, List<RemoteConfiguration
 
             try
             {
-                var anyChange = _configurationStatus.StoreConfigs(configsByProduct, removedConfigs);
+                var anyChange = _configurationStatus.StoreLastConfigState(configsByProduct, removedConfigs);
                 var securityStateChange = Enabled != _configurationStatus.EnableAsm;
 
                 // normally CanBeToggled should not need a check as asm_features capacity is only sent if AppSec env var is null, but still guards it in case