diff --git a/collector/lib/CollectorConfig.h b/collector/lib/CollectorConfig.h index 51d6602f08..3ebfdc8beb 100644 --- a/collector/lib/CollectorConfig.h +++ b/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,37 @@ 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(); + // Converts from max connections per minute to connections per scrape interval. + return int64_t(std::round(float(max_connections_per_minute) * float(scrape_interval_) / 60.0)); } std::string GetRuntimeConfigStr() { @@ -207,7 +224,7 @@ class CollectorConfig { std::vector 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 grpc_server_; diff --git a/collector/lib/CollectorRuntimeConfigInspector.cpp b/collector/lib/CollectorRuntimeConfigInspector.cpp index 95e1b1faac..26a2645659 100644 --- a/collector/lib/CollectorRuntimeConfigInspector.cpp +++ b/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; diff --git a/collector/lib/ConfigLoader.cpp b/collector/lib/ConfigLoader.cpp index 14d7c3c26f..d94b308f2c 100644 --- a/collector/lib/ConfigLoader.cpp +++ b/collector/lib/ConfigLoader.cpp @@ -72,16 +72,27 @@ bool ConfigLoader::LoadConfiguration(CollectorConfig& config, const YAML::Node& return true; } - bool enable_external_ips = external_ips_node["enable"].as(false); - int64_t per_container_rate_limit = networking_node["perContainerRateLimit"].as(1024); + sensor::ExternalIpsEnabled enable_external_ips; + std::string enabled_value = external_ips_node["enabled"] ? external_ips_node["enabled"].as() : ""; + std::transform(enabled_value.begin(), enabled_value.end(), enabled_value.begin(), ::tolower); + + if (enabled_value == "enabled") { + enable_external_ips = sensor::ExternalIpsEnabled::ENABLED; + } else if (enabled_value == "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(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)); diff --git a/collector/proto/third_party/stackrox b/collector/proto/third_party/stackrox index ef2e2cc853..0e536c9e74 160000 --- a/collector/proto/third_party/stackrox +++ b/collector/proto/third_party/stackrox @@ -1 +1 @@ -Subproject commit ef2e2cc85370988de7f0d01c56338c0d14804e79 +Subproject commit 0e536c9e7423aac43e1f79d8245ba52b311eef91 diff --git a/collector/test/CollectorConfigTest.cpp b/collector/test/CollectorConfigTest.cpp index df92d50182..8b03bb9af2 100644 --- a/collector/test/CollectorConfigTest.cpp +++ b/collector/test/CollectorConfigTest.cpp @@ -134,7 +134,7 @@ 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); @@ -142,7 +142,7 @@ TEST(CollectorConfigTest, TestEnableExternalIpsRuntimeConfig) { 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()); diff --git a/collector/test/ConfigLoaderTest.cpp b/collector/test/ConfigLoaderTest.cpp index 8a86f00992..66af0df56f 100644 --- a/collector/test/ConfigLoaderTest.cpp +++ b/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> 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 diff --git a/collector/test/NetworkStatusNotifierTest.cpp b/collector/test/NetworkStatusNotifierTest.cpp index dc246f2962..988509efdb 100644 --- a/collector/test/NetworkStatusNotifierTest.cpp +++ b/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); diff --git a/docs/references.md b/docs/references.md index 996e8fd623..0ea575b0fd 100644 --- a/docs/references.md +++ b/docs/references.md @@ -114,15 +114,14 @@ seconds. The default value is 30 seconds. ### File mount or ConfigMap The external IPs feature can be enabled or disabled using a file or ConfigMap. This is an optional -method and does not have to be used. This file overwites the ENABLE_EXTERNAL_IPS feature flag. -When using collector by itself a file can be mounted to it at /etc/stackrox/runtime_config.yaml. The -following is an example of the contents +method and does not have to be used. When using collector by itself a file can be mounted to it at +/etc/stackrox/runtime_config.yaml. The following is an example of the contents ``` networking: externalIps: - enable: true - perContainerRateLimit: 1234 + enabled: ENABLED + maxConnectionsPerMinute: 1234 ``` Alternatively, if collector is used as a part of Stackrox, the configuration can be set @@ -138,8 +137,8 @@ data: runtime_config.yaml: | networking: externalIps: - enable: true - perContainerRateLimit: 1234 + enabled: ENABLED + maxConnectionsPerMinute: 1234 ``` The file path can be set using the `ROX_COLLECTOR_CONFIG_PATH` environment variable. diff --git a/integration-tests/pkg/assert/assert.go b/integration-tests/pkg/assert/assert.go index 5f14fc0827..ff8c869860 100644 --- a/integration-tests/pkg/assert/assert.go +++ b/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 }) } diff --git a/integration-tests/pkg/types/runtime_config.go b/integration-tests/pkg/types/runtime_config.go index 8cb26a964c..c0b29e54d4 100644 --- a/integration-tests/pkg/types/runtime_config.go +++ b/integration-tests/pkg/types/runtime_config.go @@ -4,16 +4,29 @@ import ( "gopkg.in/yaml.v3" ) +type ExternalIpsConfig struct { + Enabled string `yaml:"enabled"` +} + +type NetworkConfig struct { + ExternalIps ExternalIpsConfig `yaml:"externalIps"` +} + type RuntimeConfig struct { - Networking struct { - ExternalIps struct { - Enable bool `yaml:"enable"` - } `yaml:"externalIps"` - } `yaml:"networking"` + Networking NetworkConfig `yaml:"networking"` } +// e.g. +// runtimeConfig := types.RuntimeConfig { +// Networking: types.NetworkConfig { +// ExternalIps: types.ExternalIpsConfig { +// Enabled: "ENABLED" +// }, +// }, +// } + 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) { diff --git a/integration-tests/suites/k8s/config_reload.go b/integration-tests/suites/k8s/config_reload.go index 0be89c5b09..7a9b98ec93 100644 --- a/integration-tests/suites/k8s/config_reload.go +++ b/integration-tests/suites/k8s/config_reload.go @@ -4,27 +4,36 @@ 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" ) +func init() { + var err error + var runtimeConfig types.RuntimeConfig + runtimeConfig.Networking.ExternalIps.Enabled = "ENABLED" + + EXT_IP_ENABLE, err = runtimeConfig.GetRuntimeConfigStr() + if err != nil { + panic(err) + } + + runtimeConfig.Networking.ExternalIps.Enabled = "DISABLED" + EXT_IP_DISABLE, err = runtimeConfig.GetRuntimeConfigStr() + if err != nil { + panic(err) + } +} + type K8sConfigReloadTestSuite struct { K8sTestSuiteBase } @@ -64,12 +73,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 +87,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 +108,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()) } diff --git a/integration-tests/suites/runtime_config_file.go b/integration-tests/suites/runtime_config_file.go index e5bd252849..c2e700381a 100644 --- a/integration-tests/suites/runtime_config_file.go +++ b/integration-tests/suites/runtime_config_file.go @@ -66,18 +66,11 @@ func (s *RuntimeConfigFileTestSuite) setRuntimeConfig(runtimeConfigFile string, s.Require().NoError(err) } -func (s *RuntimeConfigFileTestSuite) getRuntimeConfigEnabledStr(enabled bool) string { +func (s *RuntimeConfigFileTestSuite) setExternalIpsEnabled(runtimeConfigFile string, enabled string) { var runtimeConfig types.RuntimeConfig - runtimeConfig.Networking.ExternalIps.Enable = enabled - - configStr, err := runtimeConfig.GetRuntimeConfigStr() + runtimeConfig.Networking.ExternalIps.Enabled = enabled + runtimeConfigStr, err := runtimeConfig.GetRuntimeConfigStr() s.Require().NoError(err) - - return configStr -} - -func (s *RuntimeConfigFileTestSuite) setExternalIpsEnabled(runtimeConfigFile string, enabled bool) { - runtimeConfigStr := s.getRuntimeConfigEnabledStr(enabled) s.setRuntimeConfig(runtimeConfigFile, runtimeConfigStr) } @@ -130,8 +123,8 @@ func (s *RuntimeConfigFileTestSuite) TestRuntimeConfigFileEnable() { // External IPs enabled. // Normalized connection must be reported as inactive // Unnormalized connection will now be reported. - s.setExternalIpsEnabled(runtimeConfigFile, true) - assert.AssertExternalIps(s.T(), true, collectorIP) + s.setExternalIpsEnabled(runtimeConfigFile, "ENABLED") + assert.AssertExternalIps(s.T(), "ENABLED", collectorIP) expectedConnections = append(expectedConnections, activeUnnormalizedConnection, inactiveNormalizedConnection) connectionSuccess = s.Sensor().ExpectSameElementsConnections(s.T(), s.ClientContainer, 10*time.Second, expectedConnections...) s.Require().True(connectionSuccess) @@ -145,8 +138,8 @@ func (s *RuntimeConfigFileTestSuite) TestRuntimeConfigFileEnable() { s.Require().True(connectionSuccess) // Back to having external IPs enabled. - s.setExternalIpsEnabled(runtimeConfigFile, true) - assert.AssertExternalIps(s.T(), true, collectorIP) + s.setExternalIpsEnabled(runtimeConfigFile, "ENABLED") + assert.AssertExternalIps(s.T(), "ENABLED", collectorIP) expectedConnections = append(expectedConnections, activeUnnormalizedConnection, inactiveNormalizedConnection) connectionSuccess = s.Sensor().ExpectSameElementsConnections(s.T(), s.ClientContainer, 10*time.Second, expectedConnections...) s.Require().True(connectionSuccess) @@ -163,8 +156,8 @@ func (s *RuntimeConfigFileTestSuite) TestRuntimeConfigFileDisable() { // The runtime config file is created, but external IPs is disables. // There is no change in the state, so there are no changes to the connections - s.setExternalIpsEnabled(runtimeConfigFile, false) - assert.AssertExternalIps(s.T(), false, collectorIP) + s.setExternalIpsEnabled(runtimeConfigFile, "DISABLED") + assert.AssertExternalIps(s.T(), "DISABLED", collectorIP) common.Sleep(3 * time.Second) // Sleep so that collector has a chance to report connections connectionSuccess = s.Sensor().ExpectSameElementsConnections(s.T(), s.ClientContainer, 10*time.Second, expectedConnections...) s.Require().True(connectionSuccess) @@ -189,7 +182,7 @@ func (s *RuntimeConfigFileTestSuite) TestRuntimeConfigFileInvalid() { // Testing an invalid configuration. There should not be a change in the configuration or reported connections invalidConfig := "asdf" s.setRuntimeConfig(runtimeConfigFile, invalidConfig) - assert.AssertExternalIps(s.T(), false, collectorIP) + assert.AssertNoRuntimeConfig(s.T(), collectorIP) common.Sleep(3 * time.Second) // Sleep so that collector has a chance to report connections connectionSuccess = s.Sensor().ExpectSameElementsConnections(s.T(), s.ClientContainer, 10*time.Second, expectedConnections...) s.Require().True(connectionSuccess)