Addressed additional pr review comments

stackrox · Oct 8, 2024 · 497d102 · 497d102
1 parent da78c7c
commit 497d102
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 20 deletions.
diff --git a/collector/lib/CollectorConfig.cpp b/collector/lib/CollectorConfig.cpp
@@ -4,7 +4,6 @@
 #include <optional>
 #include <sstream>
 
-
 #include <libsinsp/sinsp.h>
 
 #include "CollectorArgs.h"
@@ -450,8 +449,6 @@ void CollectorConfig::HandleConfig(const std::filesystem::path& filePath) {
   } catch (const std::exception& e) {
     CLOG(FATAL) << "An unexpected error occurred while trying to read: " << filePath << e.what();
   }
-
-  YamlConfigToConfig(yamlConfig);
 }
 
 bool CollectorConfig::TurnOffScrape() const {

diff --git a/collector/test/CollectorConfigTest.cpp b/collector/test/CollectorConfigTest.cpp
@@ -152,41 +152,37 @@ TEST(CollectorConfigTest, TestEnableExternalIpsRuntimeConfig) {
 }
 
 TEST(CollectorConfigTest, TestYamlConfigToConfigMultiple) {
-  std::vector<std::tuple<std::string, bool, bool>> tests = {
+  std::vector<std::pair<std::string, bool>> tests = {
       {R"(
                   networkConnectionConfig:
                     enableExternalIps: true
                )",
-       true, true},
+       true},
       {R"(
                   networkConnectionConfig:
                     enableExternalIps: false
                )",
-       false, true},
+       false},
       {R"(
                   networkConnectionConfig:
                )",
-       false, true},
+       false},
       {R"(
                   networkConnectionConfig:
-                    unknownFields: asdf
+                    unknownField: asdf
                )",
-       false, true},
-      {R"(
-                  unknownFields: asdf
-               )",
-       false, false}};
+       false}};
 
-  for (const auto& [yamlStr, expected, valid] : tests) {
+  for (const auto& [yamlStr, expected] : tests) {
     YAML::Node yamlNode = YAML::Load(yamlStr);
 
     MockCollectorConfig config;
 
     bool result = config.MockYamlConfigToConfig(yamlNode);
     auto runtime_config = config.GetRuntimeConfig();
 
-    EXPECT_EQ(result, valid);
-    EXPECT_EQ(runtime_config.has_value(), valid);
+    EXPECT_TRUE(result);
+    EXPECT_TRUE(runtime_config.has_value());
 
     if (runtime_config.has_value()) {
       const auto& cfg = runtime_config.value();
@@ -197,17 +193,29 @@ TEST(CollectorConfigTest, TestYamlConfigToConfigMultiple) {
   }
 }
 
+TEST(CollectorConfigTest, TestYamlConfigToConfigInvalid) {
+  std::string yamlStr = R"(
+                  unknownField: asdf
+               )";
+
+  YAML::Node yamlNode = YAML::Load(yamlStr);
+
+  MockCollectorConfig config;
+
+  bool result = config.MockYamlConfigToConfig(yamlNode);
+  auto runtime_config = config.GetRuntimeConfig();
+
+  EXPECT_FALSE(result);
+  EXPECT_FALSE(runtime_config.has_value());
+}
+
 TEST(CollectorConfigTest, TestYamlConfigToConfigEmpty) {
   std::string yamlStr = R"()";
   YAML::Node yamlNode = YAML::Load(yamlStr);
 
   MockCollectorConfig config;
 
   EXPECT_DEATH({ config.MockYamlConfigToConfig(yamlNode); }, ".*");
-  std::optional<sensor::CollectorConfig> runtime_config;
-  runtime_config = config.GetRuntimeConfig();
-
-  EXPECT_FALSE(runtime_config.has_value());
 }
 
 }  // namespace collector