ROX-26578: Improvements to CollectorConfig protobuf definitions (#1887)

stackrox · Oct 11, 2024 · afdbcd1 · afdbcd1
1 parent 23339ca
commit afdbcd1
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 48 deletions.
diff --git a/collector/lib/CollectorConfig.cpp b/collector/lib/CollectorConfig.cpp
@@ -397,31 +397,35 @@ void CollectorConfig::HandleSinspEnvVars() {
   }
 }
 
-bool CollectorConfig::YamlConfigToConfig(YAML::Node& yamlConfig) {
+void CollectorConfig::YamlConfigToConfig(YAML::Node& yamlConfig) {
   if (yamlConfig.IsNull() || !yamlConfig.IsDefined()) {
     CLOG(FATAL) << "Unable to read config from config file";
-    return false;
+    return;
   }
-  YAML::Node networkConnectionConfig = yamlConfig["networkConnectionConfig"];
-  if (!networkConnectionConfig) {
-    CLOG(WARNING) << "No networkConnectionConfig in config file";
-    return false;
+  YAML::Node networking = yamlConfig["networking"];
+  if (!networking) {
+    CLOG(INFO) << "No networking in config file";
+    return;
   }
 
   bool enableExternalIps = false;
-  if (networkConnectionConfig["enableExternalIps"]) {
-    enableExternalIps = networkConnectionConfig["enableExternalIps"].as<bool>(false);
+  YAML::Node externalIpsNode = networking["externalIps"];
+  if (!externalIpsNode) {
+    CLOG(INFO) << "No external IPs in config file";
+    return;
   }
+  enableExternalIps = externalIpsNode["enable"].as<bool>(false);
 
   sensor::CollectorConfig runtime_config;
-  auto* networkConfig = runtime_config.mutable_network_connection_config();
-  networkConfig->set_enable_external_ips(enableExternalIps);
+  auto* networkingConfig = runtime_config.mutable_networking();
+  auto* externalIpsConfig = networkingConfig->mutable_external_ips();
+  externalIpsConfig->set_enable(enableExternalIps);
 
   SetRuntimeConfig(runtime_config);
   CLOG(INFO) << "Runtime configuration:";
   CLOG(INFO) << GetRuntimeConfigStr();
 
-  return true;
+  return;
 }
 
 void CollectorConfig::HandleConfig(const std::filesystem::path& filePath) {

diff --git a/collector/lib/CollectorConfig.h b/collector/lib/CollectorConfig.h
@@ -93,9 +93,10 @@ class CollectorConfig {
   // otherwise, we rely on the feature flag (env var)
   bool EnableExternalIPs() const {
     if (runtime_config_.has_value()) {
-      const auto& cfg = runtime_config_.value();
-      const auto& network_cfg = cfg.network_connection_config();
-      return network_cfg.enable_external_ips();
+      return runtime_config_.value()
+          .networking()
+          .external_ips()
+          .enable();
     }
     return enable_external_ips_;
   }
@@ -197,7 +198,7 @@ class CollectorConfig {
   void HandleAfterglowEnvVars();
   void HandleConnectionStatsEnvVars();
   void HandleSinspEnvVars();
-  bool YamlConfigToConfig(YAML::Node& yamlConfig);
+  void YamlConfigToConfig(YAML::Node& yamlConfig);
   void HandleConfig(const std::filesystem::path& filePath);
 
   // Protected, used for testing purposes

diff --git a/collector/proto/third_party/stackrox b/collector/proto/third_party/stackrox
diff --git a/collector/test/CollectorConfigTest.cpp b/collector/test/CollectorConfigTest.cpp
@@ -35,8 +35,8 @@ class MockCollectorConfig : public CollectorConfig {
     SetEnableExternalIPs(value);
   }
 
-  bool MockYamlConfigToConfig(YAML::Node& yamlConfig) {
-    return YamlConfigToConfig(yamlConfig);
+  void MockYamlConfigToConfig(YAML::Node& yamlConfig) {
+    YamlConfigToConfig(yamlConfig);
   }
 };
 
@@ -135,17 +135,18 @@ TEST(CollectorConfigTest, TestEnableExternalIpsRuntimeConfig) {
   config.MockSetEnableExternalIPs(true);
 
   sensor::CollectorConfig runtime_config;
-  sensor::NetworkConnectionConfig* network_config = runtime_config.mutable_network_connection_config();
+  auto* networking_config = runtime_config.mutable_networking();
+  auto* external_ips_config = networking_config->mutable_external_ips();
 
-  network_config->set_enable_external_ips(false);
+  external_ips_config->set_enable(false);
 
   config.SetRuntimeConfig(runtime_config);
 
   EXPECT_FALSE(config.EnableExternalIPs());
 
   config.MockSetEnableExternalIPs(false);
 
-  network_config->set_enable_external_ips(true);
+  external_ips_config->set_enable(true);
   config.SetRuntimeConfig(runtime_config);
 
   EXPECT_TRUE(config.EnableExternalIPs());
@@ -154,57 +155,66 @@ TEST(CollectorConfigTest, TestEnableExternalIpsRuntimeConfig) {
 TEST(CollectorConfigTest, TestYamlConfigToConfigMultiple) {
   std::vector<std::pair<std::string, bool>> tests = {
       {R"(
-                  networkConnectionConfig:
-                    enableExternalIps: true
+                  networking:
+                    externalIps:
+                      enable: true
                )",
        true},
       {R"(
-                  networkConnectionConfig:
-                    enableExternalIps: false
+                  networking:
+                    externalIps:
+                      enable: false
                )",
        false},
       {R"(
-                  networkConnectionConfig:
+                  networking:
+                    externalIps:
                )",
        false},
-      {R"(
-                  networkConnectionConfig:
-                    unknownField: asdf
-               )",
-       false}};
+  };
 
   for (const auto& [yamlStr, expected] : tests) {
     YAML::Node yamlNode = YAML::Load(yamlStr);
 
     MockCollectorConfig config;
 
-    bool result = config.MockYamlConfigToConfig(yamlNode);
+    config.MockYamlConfigToConfig(yamlNode);
     auto runtime_config = config.GetRuntimeConfig();
 
-    EXPECT_TRUE(result);
     EXPECT_TRUE(runtime_config.has_value());
 
-    const auto& cfg = runtime_config.value();
-    const auto& network_cfg = cfg.network_connection_config();
-    EXPECT_EQ(network_cfg.enable_external_ips(), expected);
+    bool enabled = runtime_config.value()
+                       .networking()
+                       .external_ips()
+                       .enable();
+    EXPECT_EQ(enabled, expected);
     EXPECT_EQ(config.EnableExternalIPs(), expected);
   }
 }
 
 TEST(CollectorConfigTest, TestYamlConfigToConfigInvalid) {
-  std::string yamlStr = R"(
+  std::vector<std::string> tests = {
+      R"(
+                  networking:
+               )",
+      R"(
+                  networking:
+                    unknownFiled: asdf
+               )",
+      R"(
                   unknownField: asdf
-               )";
+               )"};
 
-  YAML::Node yamlNode = YAML::Load(yamlStr);
+  for (const auto& yamlStr : tests) {
+    YAML::Node yamlNode = YAML::Load(yamlStr);
 
-  MockCollectorConfig config;
+    MockCollectorConfig config;
 
-  bool result = config.MockYamlConfigToConfig(yamlNode);
-  auto runtime_config = config.GetRuntimeConfig();
+    config.MockYamlConfigToConfig(yamlNode);
+    auto runtime_config = config.GetRuntimeConfig();
 
-  EXPECT_FALSE(result);
-  EXPECT_FALSE(runtime_config.has_value());
+    EXPECT_FALSE(runtime_config.has_value());
+  }
 }
 
 TEST(CollectorConfigTest, TestYamlConfigToConfigEmpty) {

diff --git a/docs/references.md b/docs/references.md
@@ -114,8 +114,9 @@ When using collector by itself a file can be mounted to it at /etc/stackrox/runt
 following is an example of the contents
 
 ```
-networkConnectionConfig:
-    enableExternalIps: true
+networking:
+  externalIps:
+    enable: true
 ```
 
 Alternatively, if collector is used as a part of Stackrox, the configuration can be set
@@ -129,8 +130,9 @@ metadata:
   namespace: stackrox
 data:
   runtime_config.yaml: |
-    networkConnectionConfig:
-        enableExternalIps: true
+    networking:
+      externalIps:
+        enable: true
 ```
 
 The file path can be set using the `ROX_COLLECTOR_CONFIG_PATH` environment variable.