From 8470072d8d27ced5699c8cf286cebff51e0ab56d Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick Date: Thu, 5 Oct 2023 14:23:45 +0000 Subject: [PATCH] set default SPLUNK_LISTEN_INTERFACE to 127.0.0.1 for agent configs --- CHANGELOG.md | 4 + internal/settings/settings.go | 65 +++++++++--- internal/settings/settings_test.go | 117 ++++++++++++++------- internal/settings/settings_windows_test.go | 51 +++++++++ tests/general/default_config_test.go | 26 ++--- 5 files changed, 200 insertions(+), 63 deletions(-) create mode 100644 internal/settings/settings_windows_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e89c4c66d..db78d1a07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### 🛑 Breaking changes 🛑 + +- (Splunk) Set `SPLUNK_LISTEN_INTERFACE` environment variable value to 127.0.0.1 for agent mode by default, as determined by config path. 0.0.0.0 will be set otherwise, with existing environment values respected. The installers have been updated to only set the environment variable for collector service if configured directly. + ## v0.85.0 ***ADVANCED NOTICE - SPLUNK_LISTEN_INTERFACE DEFAULTS*** diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 3b7631f420..7cd40675ca 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -18,7 +18,9 @@ import ( "fmt" "log" "os" + "path/filepath" "regexp" + "runtime" "sort" "strconv" "strings" @@ -58,8 +60,19 @@ const ( DefaultMemoryLimitPercentage = 90 DefaultMemoryTotalMiB = 512 DefaultListenInterface = "0.0.0.0" + DefaultAgentConfigLinux = "/etc/otel/collector/agent_config.yaml" ) +var DefaultAgentConfigWindows = func() string { + path := filepath.Join("Splunk", "OpenTelemetry Collector", "agent_config.yaml") + if runtime.GOOS == "windows" { + if pd, ok := os.LookupEnv("ProgramData"); ok { + path = filepath.Join(pd, path) + } + } + return filepath.Clean(path) +}() + var ( envProvider = envprovider.New() fileProvider = fileprovider.New() @@ -98,7 +111,7 @@ func New(args []string) (*Settings, error) { return nil, err } - if err = setDefaultEnvVars(); err != nil { + if err = setDefaultEnvVars(s); err != nil { return nil, err } @@ -346,17 +359,20 @@ func checkRuntimeParams(settings *Settings) error { if 2*ballastSize > memLimit { return fmt.Errorf("memory limit (%d) is less than 2x ballast (%d). Increase memory limit or decrease ballast size", memLimit, ballastSize) } - if os.Getenv(ListenInterfaceEnvVar) == "" { - os.Setenv(ListenInterfaceEnvVar, DefaultListenInterface) - } return nil } -func setDefaultEnvVars() error { - type ev struct{ e, v string } +func setDefaultEnvVars(s *Settings) error { + type ev struct { + e, v string + log bool + } - envVars := []ev{{e: ConfigServerEnabledEnvVar, v: "true"}} + envVars := []ev{ + {e: ListenInterfaceEnvVar, v: defaultListenAddr(s), log: true}, + {e: ConfigServerEnabledEnvVar, v: "true"}, + } if realm, ok := os.LookupEnv(RealmEnvVar); ok { envVars = append(envVars, @@ -364,7 +380,6 @@ func setDefaultEnvVars() error { ev{e: IngestURLEnvVar, v: fmt.Sprintf("https://ingest.%s.signalfx.com", realm)}, ev{e: TraceIngestURLEnvVar, v: fmt.Sprintf("https://ingest.%s.signalfx.com/v2/trace", realm)}, ev{e: HecLogIngestURLEnvVar, v: fmt.Sprintf("https://ingest.%s.signalfx.com/v1/log", realm)}, - ev{e: ListenInterfaceEnvVar, v: "0.0.0.0"}, ) } @@ -377,11 +392,33 @@ func setDefaultEnvVars() error { if err := os.Setenv(envVar.e, envVar.v); err != nil { return err } + if envVar.log { + log.Printf("set %q to %q", envVar.e, envVar.v) + } } } return nil } +func defaultListenAddr(s *Settings) string { + if s != nil { + for _, path := range s.configPaths.value { + scheme, location, isURI := parseURI(path) + if isURI && scheme == "file" { + path = location + } + cleaned := filepath.Clean(path) + if path == DefaultAgentConfigLinux || + cleaned == DefaultAgentConfigLinux || + path == DefaultAgentConfigWindows || + cleaned == DefaultAgentConfigWindows { + return "127.0.0.1" + } + } + } + return DefaultListenInterface +} + // Config priority (highest to lowest): // 1. '--config' flags (multiple supported), // 2. SPLUNK_CONFIG env var, @@ -517,19 +554,19 @@ func checkInputConfigs(settings *Settings) error { } func checkConfigPathEnvVar(settings *Settings) error { - configPathVar := os.Getenv(ConfigEnvVar) + configPath := os.Getenv(ConfigEnvVar) configYaml := os.Getenv(ConfigYamlEnvVar) - if _, err := os.Stat(configPathVar); err != nil { - return fmt.Errorf("unable to find the configuration file (%s), ensure %s environment variable is set properly: %w", configPathVar, ConfigEnvVar, err) + if _, err := os.Stat(configPath); err != nil { + return fmt.Errorf("unable to find the configuration file (%s), ensure %s environment variable is set properly: %w", configPath, ConfigEnvVar, err) } if configYaml != "" { - log.Printf("Both %s and %s were specified. Using %s environment variable value %s for this session", ConfigEnvVar, ConfigYamlEnvVar, ConfigEnvVar, configPathVar) + log.Printf("Both %s and %s were specified. Using %s environment variable value %s for this session", ConfigEnvVar, ConfigYamlEnvVar, ConfigEnvVar, configPath) } - if !settings.configPaths.contains(configPathVar) { - _ = settings.configPaths.Set(configPathVar) + if !settings.configPaths.contains(configPath) { + _ = settings.configPaths.Set(configPath) } return confirmRequiredEnvVarsForDefaultConfigs(settings.configPaths.value) diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index 3b39b74e53..7a39f273fd 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -24,6 +24,7 @@ import ( "testing" flag "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/confmap" @@ -225,11 +226,11 @@ func TestCheckRuntimeParams_MemTotalEnv(t *testing.T) { func TestCheckRuntimeParams_ListenInterface(t *testing.T) { t.Cleanup(setRequiredEnvVars(t)) - require.NoError(t, os.Setenv(ListenInterfaceEnvVar, "127.0.0.1")) + require.NoError(t, os.Setenv(ListenInterfaceEnvVar, "1.2.3.4")) settings, err := New([]string{}) require.NoError(t, err) require.NotNil(t, settings) - require.Equal(t, "127.0.0.1", os.Getenv(ListenInterfaceEnvVar)) + require.Equal(t, "1.2.3.4", os.Getenv(ListenInterfaceEnvVar)) } func TestCheckRuntimeParams_MemTotalAndBallastEnvs(t *testing.T) { @@ -258,55 +259,99 @@ func TestCheckRuntimeParams_LimitAndBallastEnvs(t *testing.T) { require.Equal(t, "250", os.Getenv(MemLimitMiBEnvVar)) } -func TestSetDefaultEnvVars(t *testing.T) { +func TestSetDefaultEnvVarsOnlySetsURLsWithRealmSet(t *testing.T) { t.Cleanup(clearEnv(t)) - - testArgs := []string{"SPLUNK_API_URL", "SPLUNK_INGEST_URL", "SPLUNK_TRACE_URL", "SPLUNK_HEC_URL", "SPLUNK_HEC_TOKEN", "SPLUNK_LISTEN_INTERFACE"} - for _, v := range testArgs { - setDefaultEnvVars() + envVars := []string{"SPLUNK_API_URL", "SPLUNK_INGEST_URL", "SPLUNK_TRACE_URL", "SPLUNK_HEC_URL", "SPLUNK_HEC_TOKEN"} + for _, v := range envVars { + require.NoError(t, setDefaultEnvVars(nil)) _, ok := os.LookupEnv(v) - require.False(t, ok, fmt.Sprintf("Expected %q unset given SPLUNK_ACCESS_TOKEN or SPLUNK_TOKEN is unset", v)) + require.False(t, ok, fmt.Sprintf("Expected %q unset given SPLUNK_REALM is unset", v)) } +} - realm := "us1" - token := "1234" - valTest := "test" - valEmpty := "" +func TestSetDefaultEnvVarsOnlySetsHECTokenWithTokenSet(t *testing.T) { + t.Cleanup(clearEnv(t)) + require.NoError(t, setDefaultEnvVars(nil)) + _, ok := os.LookupEnv("SPLUNK_HEC_TOKEN") + require.False(t, ok, "Expected SPLUNK_HEC_TOKEN unset given SPLUNK_ACCESS_TOKEN is unset") +} + +func TestSetDefaultEnvVarsSetsURLsFromRealm(t *testing.T) { + t.Cleanup(clearEnv(t)) + realm := "us1" os.Setenv("SPLUNK_REALM", realm) - os.Setenv("SPLUNK_ACCESS_TOKEN", token) - setDefaultEnvVars() - testArgs2 := [][]string{ + require.NoError(t, setDefaultEnvVars(nil)) + + expectedEnvVars := [][]string{ {"SPLUNK_API_URL", fmt.Sprintf("https://api.%s.signalfx.com", realm)}, {"SPLUNK_INGEST_URL", fmt.Sprintf("https://ingest.%s.signalfx.com", realm)}, {"SPLUNK_TRACE_URL", fmt.Sprintf("https://ingest.%s.signalfx.com/v2/trace", realm)}, {"SPLUNK_HEC_URL", fmt.Sprintf("https://ingest.%s.signalfx.com/v1/log", realm)}, - {"SPLUNK_HEC_TOKEN", token}, - {"SPLUNK_LISTEN_INTERFACE", "0.0.0.0"}, } - for _, v := range testArgs2 { - val, _ := os.LookupEnv(v[0]) - if val != v[1] { - t.Errorf("Expected %v got %v for %v", v[1], val, v[0]) - } + for _, v := range expectedEnvVars { + val, ok := os.LookupEnv(v[0]) + assert.True(t, ok, v[0]) + assert.Equal(t, val, v[1]) } +} - for _, v := range testArgs { - os.Setenv(v, valTest) - setDefaultEnvVars() - val, _ := os.LookupEnv(v) - if val != valTest { - t.Errorf("Expected %v got %v for %v", valTest, val, v) - } +func TestSetDefaultEnvVarsSetsHECTokenFromAccessTokenEnvVar(t *testing.T) { + t.Cleanup(clearEnv(t)) + + token := "1234" + os.Setenv("SPLUNK_ACCESS_TOKEN", token) + require.NoError(t, setDefaultEnvVars(nil)) + + val, ok := os.LookupEnv("SPLUNK_HEC_TOKEN") + assert.True(t, ok) + assert.Equal(t, token, val) +} + +func TestSetDefaultEnvVarsRespectsSetEnvVars(t *testing.T) { + t.Cleanup(clearEnv(t)) + envVars := []string{"SPLUNK_API_URL", "SPLUNK_INGEST_URL", "SPLUNK_TRACE_URL", "SPLUNK_HEC_URL", "SPLUNK_HEC_TOKEN", "SPLUNK_LISTEN_INTERFACE"} + + someValue := "some.value" + for _, v := range envVars { + os.Setenv(v, someValue) + require.NoError(t, setDefaultEnvVars(nil)) + val, ok := os.LookupEnv(v) + assert.True(t, ok, v[0]) + assert.Equal(t, someValue, val) } - for _, v := range testArgs { - os.Setenv(v, valEmpty) - setDefaultEnvVars() - val, _ := os.LookupEnv(v) - if val != valEmpty { - t.Errorf("Expected %v got %v for %v", valEmpty, val, v) - } + for _, v := range envVars { + os.Setenv(v, "") + require.NoError(t, setDefaultEnvVars(nil)) + val, ok := os.LookupEnv(v) + assert.True(t, ok, v[0]) + assert.Empty(t, val) + } +} + +func TestSetDefaultEnvVarsSetsInterfaceFromConfigOption(t *testing.T) { + for _, tc := range []struct{ config, expectedIP string }{ + {"/etc/otel/collector/agent_config.yaml", "127.0.0.1"}, + {"file:/etc/otel/collector/agent_config.yaml", "127.0.0.1"}, + {"/etc/otel/collector/gateway_config.yaml", "0.0.0.0"}, + {"file:/etc/otel/collector/gateway_config.yaml", "0.0.0.0"}, + {"some-other-config.yaml", "0.0.0.0"}, + {"file:some-other-config.yaml", "0.0.0.0"}, + } { + tc := tc + t.Run(fmt.Sprintf("%v->%v", tc.config, tc.expectedIP), func(t *testing.T) { + t.Cleanup(clearEnv(t)) + os.Setenv("SPLUNK_REALM", "noop") + os.Setenv("SPLUNK_ACCESS_TOKEN", "noop") + s, err := parseArgs([]string{"--config", tc.config}) + require.NoError(t, err) + require.NoError(t, setDefaultEnvVars(s)) + + val, ok := os.LookupEnv("SPLUNK_LISTEN_INTERFACE") + assert.True(t, ok) + assert.Equal(t, tc.expectedIP, val) + }) } } diff --git a/internal/settings/settings_windows_test.go b/internal/settings/settings_windows_test.go new file mode 100644 index 0000000000..76b2d9bdfb --- /dev/null +++ b/internal/settings/settings_windows_test.go @@ -0,0 +1,51 @@ +// Copyright Splunk, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// go:build windows + +package settings + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSetDefaultEnvVarsSetsInterfaceFromConfigOptionWithProgramData(t *testing.T) { + pd := os.Getenv("ProgramData") + for _, tc := range []struct{ config, expectedIP string }{ + {filepath.Join(pd, "Splunk", "OpenTelemetry Collector", "agent_config.yaml"), "127.0.0.1"}, + {fmt.Sprintf("file:%s", filepath.Join(pd, "Splunk", "OpenTelemetry Collector", "agent_config.yaml")), "127.0.0.1"}, + {"\\some-other-config.yaml", "0.0.0.0"}, + {"file:\\some-other-config.yaml", "0.0.0.0"}, + } { + tc := tc + t.Run(fmt.Sprintf("%v->%v", tc.config, tc.expectedIP), func(t *testing.T) { + t.Cleanup(clearEnv(t)) + os.Setenv("SPLUNK_REALM", "noop") + os.Setenv("SPLUNK_ACCESS_TOKEN", "noop") + s, err := parseArgs([]string{"--config", tc.config}) + require.NoError(t, err) + require.NoError(t, setDefaultEnvVars(s)) + + val, ok := os.LookupEnv("SPLUNK_LISTEN_INTERFACE") + assert.True(t, ok) + assert.Equal(t, tc.expectedIP, val) + }) + } +} diff --git a/tests/general/default_config_test.go b/tests/general/default_config_test.go index 9cb194179d..6c02001253 100644 --- a/tests/general/default_config_test.go +++ b/tests/general/default_config_test.go @@ -242,13 +242,13 @@ func TestDefaultAgentConfig(t *testing.T) { }, }, "extensions": map[string]any{ - "health_check": map[string]any{"endpoint": "0.0.0.0:13133"}, + "health_check": map[string]any{"endpoint": "127.0.0.1:13133"}, "http_forwarder": map[string]any{ "egress": map[string]any{ "endpoint": "https://api.not.real.signalfx.com", }, "ingress": map[string]any{ - "endpoint": "0.0.0.0:6060", + "endpoint": "127.0.0.1:6060", }, }, "memory_ballast": map[string]any{"size_mib": "168"}, @@ -285,14 +285,14 @@ func TestDefaultAgentConfig(t *testing.T) { }, "jaeger": map[string]any{ "protocols": map[string]any{ - "grpc": map[string]any{"endpoint": "0.0.0.0:14250"}, - "thrift_binary": map[string]any{"endpoint": "0.0.0.0:6832"}, - "thrift_compact": map[string]any{"endpoint": "0.0.0.0:6831"}, - "thrift_http": map[string]any{"endpoint": "0.0.0.0:14268"}}}, + "grpc": map[string]any{"endpoint": "127.0.0.1:14250"}, + "thrift_binary": map[string]any{"endpoint": "127.0.0.1:6832"}, + "thrift_compact": map[string]any{"endpoint": "127.0.0.1:6831"}, + "thrift_http": map[string]any{"endpoint": "127.0.0.1:14268"}}}, "otlp": map[string]any{ "protocols": map[string]any{ - "grpc": map[string]any{"endpoint": "0.0.0.0:4317"}, - "http": map[string]any{"endpoint": "0.0.0.0:4318"}, + "grpc": map[string]any{"endpoint": "127.0.0.1:4317"}, + "http": map[string]any{"endpoint": "127.0.0.1:4318"}, }, }, "prometheus/internal": map[string]any{ @@ -310,23 +310,23 @@ func TestDefaultAgentConfig(t *testing.T) { "scrape_interval": "10s", "static_configs": []any{ map[string]any{ - "targets": []any{"0.0.0.0:8888"}, + "targets": []any{"127.0.0.1:8888"}, }, }, }, }, }, }, - "signalfx": map[string]any{"endpoint": "0.0.0.0:9943"}, + "signalfx": map[string]any{"endpoint": "127.0.0.1:9943"}, "smartagent/processlist": map[string]any{"type": "processlist"}, "smartagent/signalfx-forwarder": map[string]any{ - "listenAddress": "0.0.0.0:9080", + "listenAddress": "127.0.0.1:9080", "type": "signalfx-forwarder", }, "zipkin": map[string]any{ - "endpoint": "0.0.0.0:9411"}}, + "endpoint": "127.0.0.1:9411"}}, "service": map[string]any{ - "telemetry": map[string]any{"metrics": map[string]any{"address": "0.0.0.0:8888"}}, + "telemetry": map[string]any{"metrics": map[string]any{"address": "127.0.0.1:8888"}}, "extensions": []any{"health_check", "http_forwarder", "zpages", "memory_ballast", "smartagent"}, "pipelines": map[string]any{ "logs": map[string]any{