ROX-27634: Improvements to CollectorConfig protobuf definitions

collector/lib/CollectorConfig.h

@@ -29,6 +29,7 @@ class CollectorConfig {
  public:
   static constexpr bool kTurnOffScrape = false;
   static constexpr int kScrapeInterval = 30;
+  static constexpr int64_t kMaxConnectionsPerMinute = 2048;
   static constexpr CollectionMethod kCollectionMethod = CollectionMethod::CORE_BPF;
   static constexpr const char* kSyscalls[] = {
       "accept",
@@ -101,21 +102,36 @@ class CollectorConfig {
     auto lock = ReadLock();
     if (runtime_config_.has_value()) {
       return runtime_config_.value()
-          .networking()
-          .external_ips()
-          .enable();
+                 .networking()
+                 .external_ips()
+                 .enabled() == sensor::ExternalIpsEnabled::ENABLED;
     }
     return enable_external_ips_;
   }
 
-  int64_t PerContainerRateLimit() const {
+  void RuntimeConfigHeuristics() {
+    auto lock = WriteLock();
+    if (runtime_config_.has_value()) {
+      auto* networking = runtime_config_.value().mutable_networking();
+      if (networking->max_connections_per_minute() == 0) {
+        networking->set_max_connections_per_minute(kMaxConnectionsPerMinute);
+      }
+    }
+  }
+
+  int64_t MaxConnectionsPerMinute() const {
     auto lock = ReadLock();
     if (runtime_config_.has_value()) {
       return runtime_config_.value()
           .networking()
-          .per_container_rate_limit();
+          .max_connections_per_minute();
     }
-    return per_container_rate_limit_;
+    return max_connections_per_minute_;
+  }
+
+  int64_t PerContainerRateLimit() const {
+    int64_t max_connections_per_minute = MaxConnectionsPerMinute();
+    return int64_t(float(max_connections_per_minute) * float(scrape_interval_) / 60.0 + 0.5);
   }
 
   std::string GetRuntimeConfigStr() {
@@ -207,7 +223,7 @@ class CollectorConfig {
   std::vector<double> connection_stats_quantiles_;
   double connection_stats_error_;
   unsigned int connection_stats_window_;
-  int64_t per_container_rate_limit_ = 1024;
+  int64_t max_connections_per_minute_ = kMaxConnectionsPerMinute;
 
   // URL to the GRPC server
   std::optional<std::string> grpc_server_;

collector/lib/CollectorRuntimeConfigInspector.cpp

@@ -22,7 +22,9 @@ std::string CollectorConfigInspector::configToJson(bool& isError) {
 
   std::string jsonString;
   const auto& config = runtime_config.value();
-  absl::Status status = google::protobuf::util::MessageToJsonString(config, &jsonString);
+  google::protobuf::util::JsonPrintOptions options;
+  options.always_print_fields_with_no_presence = true;
+  absl::Status status = google::protobuf::util::MessageToJsonString(config, &jsonString, options);
 
   if (!status.ok()) {
     isError = true;

collector/lib/ConfigLoader.cpp

@@ -72,16 +72,28 @@ bool ConfigLoader::LoadConfiguration(CollectorConfig& config, const YAML::Node&
     return true;
   }
 
-  bool enable_external_ips = external_ips_node["enable"].as<bool>(false);
-  int64_t per_container_rate_limit = networking_node["perContainerRateLimit"].as<int64_t>(1024);
+  sensor::ExternalIpsEnabled enable_external_ips;
+  const std::string enabled_value = external_ips_node["enabled"] ? external_ips_node["enabled"].as<std::string>() : "";
+  std::string enabled_value_lower = enabled_value;
+  std::transform(enabled_value_lower.begin(), enabled_value_lower.end(), enabled_value_lower.begin(), ::tolower);
+
+  if (enabled_value_lower == "enabled") {
+    enable_external_ips = sensor::ExternalIpsEnabled::ENABLED;
+  } else if (enabled_value_lower == "disabled") {
+    enable_external_ips = sensor::ExternalIpsEnabled::DISABLED;
+  } else {
+    CLOG(WARNING) << "Unknown value for for networking.externalIps.enabled. Setting it to DISABLED";
+    enable_external_ips = sensor::ExternalIpsEnabled::DISABLED;
+  }
+  int64_t max_connections_per_minute = networking_node["maxConnectionsPerMinute"].as<int64_t>(CollectorConfig::kMaxConnectionsPerMinute);
 
   sensor::CollectorConfig runtime_config;
   auto* networking = runtime_config.mutable_networking();
   networking
       ->mutable_external_ips()
-      ->set_enable(enable_external_ips);
+      ->set_enabled(enable_external_ips);
   networking
-      ->set_per_container_rate_limit(per_container_rate_limit);
+      ->set_max_connections_per_minute(max_connections_per_minute);
 
   config.SetRuntimeConfig(std::move(runtime_config));
 

collector/test/CollectorConfigTest.cpp

@@ -134,15 +134,15 @@ TEST(CollectorConfigTest, TestEnableExternalIpsRuntimeConfig) {
   auto* networking_config = runtime_config.mutable_networking();
   auto* external_ips_config = networking_config->mutable_external_ips();
 
-  external_ips_config->set_enable(false);
+  external_ips_config->set_enabled(sensor::ExternalIpsEnabled::DISABLED);
 
   config.SetRuntimeConfig(runtime_config);
 
   EXPECT_FALSE(config.EnableExternalIPs());
 
   config.MockSetEnableExternalIPs(false);
 
-  external_ips_config->set_enable(true);
+  external_ips_config->set_enabled(sensor::ExternalIpsEnabled::ENABLED);
   config.SetRuntimeConfig(runtime_config);
 
   EXPECT_TRUE(config.EnableExternalIPs());

collector/test/ConfigLoaderTest.cpp

@@ -8,13 +8,13 @@ TEST(CollectorConfigTest, TestYamlConfigToConfigMultiple) {
       {R"(
                   networking:
                     externalIps:
-                      enable: true
+                      enabled: ENABLED
                )",
        true},
       {R"(
                   networking:
                     externalIps:
-                      enable: false
+                      enabled: DISABLED
                )",
        false},
       {R"(
@@ -36,7 +36,7 @@ TEST(CollectorConfigTest, TestYamlConfigToConfigMultiple) {
     bool enabled = runtime_config.value()
                        .networking()
                        .external_ips()
-                       .enable();
+                       .enabled() == sensor::ExternalIpsEnabled::ENABLED;
     EXPECT_EQ(enabled, expected);
     EXPECT_EQ(config.EnableExternalIPs(), expected);
   }
@@ -84,29 +84,29 @@ TEST(CollectorConfigTest, TestYamlConfigToConfigEmptyOrMalformed) {
   }
 }
 
-TEST(CollectorConfigTest, TestPerContainerRateLimit) {
+TEST(CollectorConfigTest, TestMaxConnectionsPerMinute) {
   std::vector<std::pair<std::string, int>> tests = {
       {R"(
                   networking:
                     externalIps:
-                      enabled: false
-                    perContainerRateLimit: 1234
+                      enabled: DISABLED
+                    maxConnectionsPerMinute: 1234
                )",
        1234},
       {R"(
                   networking:
                     externalIps:
-                      enabled: false
-                    perContainerRateLimit: 1337
+                      enabled: DISABLED
+                    maxConnectionsPerMinute: 1337
                )",
        1337},
       {R"(
                   networking:
                     externalIps:
-                      enabled: false
-                    perContainerRateLimit: invalid
+                      enabled: DISABLED
+                    maxConnectionsPerMinute: invalid
                )",
-       1024},
+       2048},
   };
 
   for (const auto& [yamlStr, expected] : tests) {
@@ -120,9 +120,9 @@ TEST(CollectorConfigTest, TestPerContainerRateLimit) {
 
     int rate = runtime_config.value()
                    .networking()
-                   .per_container_rate_limit();
+                   .max_connections_per_minute();
     EXPECT_EQ(rate, expected);
-    EXPECT_EQ(config.PerContainerRateLimit(), expected);
+    EXPECT_EQ(config.MaxConnectionsPerMinute(), expected);
   }
 }
 }  // namespace collector

collector/test/NetworkStatusNotifierTest.cpp

@@ -77,8 +77,8 @@ class MockCollectorConfig : public collector::CollectorConfig {
     enable_afterglow_ = false;
   }
 
-  void SetPerContainerRateLimit(int64_t limit) {
-    per_container_rate_limit_ = limit;
+  void SetMaxConnectionsPerMinute(int64_t limit) {
+    max_connections_per_minute_ = limit;
   }
 };
 
@@ -353,7 +353,9 @@ TEST(NetworkStatusNotifier, RateLimitedConnections) {
   // maximum of 2 connections per scrape interval
   // if we throw four connections from the same container into the conn
   // tracker delta, we expect only two to be returned
-  config.SetPerContainerRateLimit(2);
+  // The scrape interval is 30 seconds so max_connections_per_minute_
+  // should be 4 to make per_container_rate_limit 2
+  config.SetMaxConnectionsPerMinute(4);
 
   NetworkStatusNotifier netStatusNotifier(conn_scraper, conn_tracker, comm, config);
 

integration-tests/pkg/assert/assert.go

@@ -19,7 +19,7 @@ var (
 	runtimeConfigErrorMsg = "Runtime configuration was not updated"
 )
 
-func AssertExternalIps(t *testing.T, enabled bool, collectorIP string) {
+func AssertExternalIps(t *testing.T, enabled string, collectorIP string) {
 	tickTime := 1 * time.Second
 	timeout := 3 * time.Minute
 	AssertRepeated(t, tickTime, timeout, runtimeConfigErrorMsg, func() bool {
@@ -29,7 +29,7 @@ func AssertExternalIps(t *testing.T, enabled bool, collectorIP string) {
 		err = json.Unmarshal(body, &response)
 		assert.NoError(t, err)
 
-		return response.Networking.ExternalIps.Enable == enabled
+		return response.Networking.ExternalIps.Enabled == enabled
 	})
 }
 

integration-tests/pkg/types/runtime_config.go

@@ -7,13 +7,13 @@ import (
 type RuntimeConfig struct {
 	Networking struct {
 		ExternalIps struct {
-			Enable bool `yaml:"enable"`
+			Enabled string `yaml:"enabled"`
 		} `yaml:"externalIps"`
 	} `yaml:"networking"`
 }
 
 func (n *RuntimeConfig) Equal(other RuntimeConfig) bool {
-	return n.Networking.ExternalIps.Enable == other.Networking.ExternalIps.Enable
+	return n.Networking.ExternalIps.Enabled == other.Networking.ExternalIps.Enabled
 }
 
 func (n *RuntimeConfig) GetRuntimeConfigStr() (string, error) {
@@ -25,3 +25,10 @@ func (n *RuntimeConfig) GetRuntimeConfigStr() (string, error) {
 
 	return string(yamlBytes), err
 }
+
+func GetRuntimeConfigEnabledStr(enabled string) (string, error) {
+	var runtimeConfig RuntimeConfig
+	runtimeConfig.Networking.ExternalIps.Enabled = enabled
+
+	return runtimeConfig.GetRuntimeConfigStr()
+}

integration-tests/suites/k8s/config_reload.go

@@ -4,23 +4,15 @@ import (
 	"github.com/stackrox/collector/integration-tests/pkg/assert"
 	"github.com/stackrox/collector/integration-tests/pkg/collector"
 	"github.com/stackrox/collector/integration-tests/pkg/log"
+	"github.com/stackrox/collector/integration-tests/pkg/types"
 
 	coreV1 "k8s.io/api/core/v1"
 	metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	EXT_IP_ENABLE = `
-networking:
-  externalIps:
-    enable: true
-`
-
-	EXT_IP_DISABLE = `
-networking:
-  externalIps:
-    enable: false
-`
+var (
+	EXT_IP_ENABLE  string
+	EXT_IP_DISABLE string
 
 	CONFIG_MAP_NAME = "collector-config"
 )
@@ -30,6 +22,14 @@ type K8sConfigReloadTestSuite struct {
 }
 
 func (k *K8sConfigReloadTestSuite) SetupSuite() {
+	var err error
+
+	EXT_IP_ENABLE, err = types.GetRuntimeConfigEnabledStr("ENABLED")
+	k.Require().NoError(err, "Failed to get runtime config for ENABLED")
+
+	EXT_IP_DISABLE, err = types.GetRuntimeConfigEnabledStr("DISABLED")
+	k.Require().NoError(err, "Failed to get runtime config for DISABLED")
+
 	k.T().Cleanup(func() {
 		k.Sensor().Stop()
 		k.teardownTargetNamespace()
@@ -64,12 +64,12 @@ func (k *K8sConfigReloadTestSuite) TestCreateConfigurationAfterStart() {
 		},
 	}
 	k.createConfigMap(&configMap)
-	assert.AssertExternalIps(k.T(), true, k.Collector().IP())
+	assert.AssertExternalIps(k.T(), "ENABLED", k.Collector().IP())
 
 	log.Info("Checking external IPs is disabled")
 	configMap.Data["runtime_config.yaml"] = EXT_IP_DISABLE
 	k.updateConfigMap(&configMap)
-	assert.AssertExternalIps(k.T(), false, k.Collector().IP())
+	assert.AssertExternalIps(k.T(), "DISABLED", k.Collector().IP())
 
 	log.Info("Checking runtime configuration is not in use")
 	k.deleteConfigMap(CONFIG_MAP_NAME)
@@ -78,7 +78,7 @@ func (k *K8sConfigReloadTestSuite) TestCreateConfigurationAfterStart() {
 	log.Info("Checking external IPs is enabled again")
 	configMap.Data["runtime_config.yaml"] = EXT_IP_ENABLE
 	k.createConfigMap(&configMap)
-	assert.AssertExternalIps(k.T(), true, k.Collector().IP())
+	assert.AssertExternalIps(k.T(), "ENABLED", k.Collector().IP())
 }
 
 func (k *K8sConfigReloadTestSuite) TestConfigurationReload() {
@@ -99,10 +99,10 @@ func (k *K8sConfigReloadTestSuite) TestConfigurationReload() {
 			"ROX_COLLECTOR_INTROSPECTION_ENABLE": "true",
 		},
 	})
-	assert.AssertExternalIps(k.T(), true, k.Collector().IP())
+	assert.AssertExternalIps(k.T(), "ENABLED", k.Collector().IP())
 
 	log.Info("Checking external IPs is disabled")
 	configMap.Data["runtime_config.yaml"] = EXT_IP_DISABLE
 	k.updateConfigMap(&configMap)
-	assert.AssertExternalIps(k.T(), false, k.Collector().IP())
+	assert.AssertExternalIps(k.T(), "DISABLED", k.Collector().IP())
 }