diff --git a/internal/commands/diagnose/authcheck.go b/internal/commands/diagnose/authcheck.go index f065e5b4a..e6c559eb8 100644 --- a/internal/commands/diagnose/authcheck.go +++ b/internal/commands/diagnose/authcheck.go @@ -69,9 +69,9 @@ func makeTestRequest(URL string, headers map[string]string) NetworkTestResult { } } -func makeAuthTestRequest() (any, error) { - collector_url := viper.GetString("observe_url") - authToken := fmt.Sprintf("Bearer %s", viper.GetString("token")) +func makeAuthTestRequest(v *viper.Viper) (any, error) { + collector_url := v.GetString("observe_url") + authToken := fmt.Sprintf("Bearer %s", v.GetString("token")) authTestResponse := makeTestRequest(collector_url, map[string]string{"Authorization": authToken}) return authTestResponse, nil } diff --git a/internal/commands/diagnose/authcheck.tmpl b/internal/commands/diagnose/authcheck.tmpl index aab442fbe..9433bc852 100644 --- a/internal/commands/diagnose/authcheck.tmpl +++ b/internal/commands/diagnose/authcheck.tmpl @@ -3,7 +3,7 @@ Running auth check against {{ .URL }} Request to test URL responded with response code {{ .ResponseCode }} {{- else }} {{- if eq .ResponseCode 401 }} -Request to test URL failed with error {{ .Error }}. +⚠️ Request to test URL failed with error {{ .Error }}. Remediation Please check that the token is present in the `observe-agent.yaml` config file and that the token is valid. diff --git a/internal/commands/diagnose/configcheck.go b/internal/commands/diagnose/configcheck.go index d9ee8d966..736cdfa35 100644 --- a/internal/commands/diagnose/configcheck.go +++ b/internal/commands/diagnose/configcheck.go @@ -5,23 +5,20 @@ import ( "fmt" "os" + "github.com/observeinc/observe-agent/internal/config" "github.com/spf13/viper" "gopkg.in/yaml.v2" ) type ConfigTestResult struct { - ConfigFile string - Passed bool - Error string + ConfigFile string + ParseSucceeded bool + IsValid bool + Error string } -func validateYaml(yamlContent []byte) error { - m := make(map[string]any) - return yaml.Unmarshal(yamlContent, &m) -} - -func checkConfig() (any, error) { - configFile := viper.ConfigFileUsed() +func checkConfig(v *viper.Viper) (any, error) { + configFile := v.ConfigFileUsed() if configFile == "" { return nil, fmt.Errorf("no config file defined") } @@ -29,16 +26,27 @@ func checkConfig() (any, error) { if err != nil { return nil, err } - if err = validateYaml(contents); err != nil { + var conf config.AgentConfig + if err = yaml.Unmarshal(contents, &conf); err != nil { + return ConfigTestResult{ + ConfigFile: configFile, + ParseSucceeded: false, + IsValid: false, + Error: err.Error(), + }, nil + } + if err = conf.Validate(); err != nil { return ConfigTestResult{ - configFile, - false, - err.Error(), + ConfigFile: configFile, + ParseSucceeded: true, + IsValid: false, + Error: err.Error(), }, nil } return ConfigTestResult{ - ConfigFile: configFile, - Passed: true, + ConfigFile: configFile, + ParseSucceeded: true, + IsValid: true, }, nil } diff --git a/internal/commands/diagnose/configcheck.tmpl b/internal/commands/diagnose/configcheck.tmpl index 8883be101..6f804f8bd 100644 --- a/internal/commands/diagnose/configcheck.tmpl +++ b/internal/commands/diagnose/configcheck.tmpl @@ -1,6 +1,9 @@ -Running check on config file {{ .ConfigFile }} -{{- if .Passed }} -Config file is valid YAML. +Running check on observe-agent config file {{ .ConfigFile }} +{{- if .IsValid }} +Config file is valid. +{{- else if .ParseSucceeded}} +⚠️ Config file validation failed with error {{ .Error }} {{- else }} -⚠️ Config file parse failed with error {{ .Error }} +⚠️ Config file could not be parsed as YAML +{{ .Error }} {{- end }} diff --git a/internal/commands/diagnose/configcheck_test.go b/internal/commands/diagnose/configcheck_test.go index c9714fc7b..01db3ccc7 100644 --- a/internal/commands/diagnose/configcheck_test.go +++ b/internal/commands/diagnose/configcheck_test.go @@ -1,17 +1,19 @@ package diagnose import ( + "os" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" ) const testConfig = ` # Observe data token -token: "some token" +token: "some:token" # Target Observe collection url -observe_url: "localhost" +observe_url: "https://collect.observeinc.com" # Debug mode - Sets agent log level to debug debug: false @@ -35,25 +37,46 @@ host_monitoring: enabled: false ` -var ( - validCases = []string{ - testConfig, - "key:\n spaceIndented: \"value\"", - } - invalidCases = []string{ - "key:\n\ttabIndented: \"value\"", - "key:\n twoSpaces: true\n threeSpaces: true", - "\tstartsWithTab: true", - } -) +var testCases = []struct { + confStr string + shouldParse bool + isValid bool +}{ + // Invalid YAML + {"key:\n\ttabIndented: \"value\"", false, false}, + {"key:\n twoSpaces: true\n threeSpaces: true", false, false}, + {"\tstartsWithTab: true", false, false}, + // Invalid Configs + {"", true, false}, + {"token: some:token\nmissing: URL", true, false}, + {"missing: token\nobserve_url: https://collect.observeinc.com", true, false}, + {"token: bad token\nobserve_url: https://collect.observeinc.com", true, false}, + {"token: some:token\nobserve_url: bad url", true, false}, + // Valid configs + {testConfig, true, true}, + {"key:\n twoSpaces: true\ntoken: some:token\nobserve_url: https://collect.observeinc.com", true, true}, +} -func Test_validateYaml(t *testing.T) { - for _, tc := range validCases { - err := validateYaml([]byte(tc)) +func Test_checkConfig(t *testing.T) { + for _, tc := range testCases { + f, err := os.CreateTemp("", "test-config-*.yaml") assert.NoError(t, err) - } - for _, tc := range invalidCases { - err := validateYaml([]byte(tc)) - assert.Error(t, err) + defer os.Remove(f.Name()) + f.Write([]byte(tc.confStr)) + + v := viper.New() + v.SetConfigFile(f.Name()) + resultAny, err := checkConfig(v) + assert.NoError(t, err) + result, ok := resultAny.(ConfigTestResult) + assert.True(t, ok) + if tc.isValid { + assert.Empty(t, result.Error) + } else { + assert.NotEmpty(t, result.Error) + } + assert.Equal(t, tc.shouldParse, result.ParseSucceeded) + assert.Equal(t, tc.isValid, result.IsValid) + assert.Equal(t, f.Name(), result.ConfigFile) } } diff --git a/internal/commands/diagnose/diagnose.go b/internal/commands/diagnose/diagnose.go index c3387d1d6..2e7a36825 100644 --- a/internal/commands/diagnose/diagnose.go +++ b/internal/commands/diagnose/diagnose.go @@ -11,18 +11,19 @@ import ( "github.com/observeinc/observe-agent/internal/root" "github.com/spf13/cobra" + "github.com/spf13/viper" ) type Diagnostic struct { - check func() (any, error) + check func(*viper.Viper) (any, error) checkName string templateName string templateFS embed.FS } var diagnostics = []Diagnostic{ - authDiagnostic(), configDiagnostic(), + authDiagnostic(), } // diagnoseCmd represents the diagnose command @@ -32,10 +33,11 @@ var diagnoseCmd = &cobra.Command{ Long: `This command runs diagnostic checks for various settings and configurations to attempt to identify issues that could cause the agent to function improperly.`, Run: func(cmd *cobra.Command, args []string) { + v := viper.GetViper() fmt.Print("Running diagnosis checks...\n") for _, diagnostic := range diagnostics { fmt.Printf("\n%s\n================\n\n", diagnostic.checkName) - data, err := diagnostic.check() + data, err := diagnostic.check(v) if err != nil { fmt.Printf("⚠️ Failed to run check: %s\n", err.Error()) continue diff --git a/internal/commands/initconfig/configschema.go b/internal/commands/initconfig/configschema.go deleted file mode 100644 index f25c33a61..000000000 --- a/internal/commands/initconfig/configschema.go +++ /dev/null @@ -1,35 +0,0 @@ -package initconfig - -type HostMonitoringLogsConfig struct { - Enabled bool `yaml:"enabled"` -} - -type HostMonitoringHostMetricsConfig struct { - Enabled bool `yaml:"enabled"` -} - -type HostMonitoringProcessMetricsConfig struct { - Enabled bool `yaml:"enabled"` -} - -type HostMonitoringMetricsConfig struct { - Host HostMonitoringHostMetricsConfig - Process HostMonitoringProcessMetricsConfig -} - -type HostMonitoringConfig struct { - Enabled bool `yaml:"enabled"` - Logs HostMonitoringLogsConfig - Metrics HostMonitoringMetricsConfig -} - -type SelfMonitoringConfig struct { - Enabled bool `yaml:"enabled"` -} - -type AgentConfig struct { - Token string `yaml:"token"` - ObserveURL string `yaml:"observe_url"` - SelfMonitoring SelfMonitoringConfig `yaml:"self_monitoring"` - HostMonitoring HostMonitoringConfig `yaml:"host_monitoring"` -} diff --git a/internal/commands/initconfig/initconfig_test.go b/internal/commands/initconfig/initconfig_test.go index c8d0cbf15..1f9c5c4b7 100644 --- a/internal/commands/initconfig/initconfig_test.go +++ b/internal/commands/initconfig/initconfig_test.go @@ -4,6 +4,7 @@ import ( "os" "testing" + "github.com/observeinc/observe-agent/internal/config" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" ) @@ -14,27 +15,27 @@ func Test_InitConfigCommand(t *testing.T) { }) testcases := []struct { args []string - expectedConfig AgentConfig + expectedConfig config.AgentConfig expectErr string }{ { args: []string{"--config_path=./test-config.yaml", "--token=test-token", "--observe_url=test-url"}, - expectedConfig: AgentConfig{ + expectedConfig: config.AgentConfig{ Token: "test-token", ObserveURL: "test-url", - SelfMonitoring: SelfMonitoringConfig{ + SelfMonitoring: config.SelfMonitoringConfig{ Enabled: true, }, - HostMonitoring: HostMonitoringConfig{ + HostMonitoring: config.HostMonitoringConfig{ Enabled: true, - Logs: HostMonitoringLogsConfig{ + Logs: config.HostMonitoringLogsConfig{ Enabled: true, }, - Metrics: HostMonitoringMetricsConfig{ - Host: HostMonitoringHostMetricsConfig{ + Metrics: config.HostMonitoringMetricsConfig{ + Host: config.HostMonitoringHostMetricsConfig{ Enabled: true, }, - Process: HostMonitoringProcessMetricsConfig{ + Process: config.HostMonitoringProcessMetricsConfig{ Enabled: false, }, }, @@ -44,19 +45,19 @@ func Test_InitConfigCommand(t *testing.T) { }, { args: []string{"--config_path=./test-config.yaml", "--token=test-token", "--observe_url=test-url", "--self_monitoring::enabled=false", "--host_monitoring::enabled=false", "--host_monitoring::logs::enabled=false", "--host_monitoring::metrics::host::enabled=false", "--host_monitoring::metrics::process::enabled=false"}, - expectedConfig: AgentConfig{ + expectedConfig: config.AgentConfig{ Token: "test-token", ObserveURL: "test-url", - HostMonitoring: HostMonitoringConfig{ + HostMonitoring: config.HostMonitoringConfig{ Enabled: false, - Logs: HostMonitoringLogsConfig{ + Logs: config.HostMonitoringLogsConfig{ Enabled: false, }, - Metrics: HostMonitoringMetricsConfig{ - Host: HostMonitoringHostMetricsConfig{ + Metrics: config.HostMonitoringMetricsConfig{ + Host: config.HostMonitoringHostMetricsConfig{ Enabled: false, }, - Process: HostMonitoringProcessMetricsConfig{ + Process: config.HostMonitoringProcessMetricsConfig{ Enabled: false, }, }, @@ -75,7 +76,7 @@ func Test_InitConfigCommand(t *testing.T) { t.Errorf("Expected no error, got %v", err) } } - var config AgentConfig + var config config.AgentConfig configFile, err := os.ReadFile("./test-config.yaml") if err != nil { t.Errorf("Expected no error, got %v", err) diff --git a/internal/config/configschema.go b/internal/config/configschema.go new file mode 100644 index 000000000..748734b77 --- /dev/null +++ b/internal/config/configschema.go @@ -0,0 +1,67 @@ +package config + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +type HostMonitoringLogsConfig struct { + Enabled bool `yaml:"enabled"` +} + +type HostMonitoringHostMetricsConfig struct { + Enabled bool `yaml:"enabled"` +} + +type HostMonitoringProcessMetricsConfig struct { + Enabled bool `yaml:"enabled"` +} + +type HostMonitoringMetricsConfig struct { + Host HostMonitoringHostMetricsConfig `yaml:"host,omitempty"` + Process HostMonitoringProcessMetricsConfig `yaml:"process,omitempty"` +} + +type HostMonitoringConfig struct { + Enabled bool `yaml:"enabled"` + Logs HostMonitoringLogsConfig `yaml:"logs,omitempty"` + Metrics HostMonitoringMetricsConfig `yaml:"metrics,omitempty"` +} + +type SelfMonitoringConfig struct { + Enabled bool `yaml:"enabled"` +} + +type AgentConfig struct { + Token string `yaml:"token"` + ObserveURL string `yaml:"observe_url"` + SelfMonitoring SelfMonitoringConfig `yaml:"self_monitoring,omitempty"` + HostMonitoring HostMonitoringConfig `yaml:"host_monitoring,omitempty"` + OtelConfigOverrides map[string]any `yaml:"otel_config_overrides,omitempty"` +} + +func (config *AgentConfig) Validate() error { + if config.ObserveURL == "" { + return errors.New("missing ObserveURL") + } + u, err := url.Parse(config.ObserveURL) + if err != nil { + return err + } + if u.Scheme == "" { + return fmt.Errorf("missing scheme for ObserveURL %s", config.ObserveURL) + } + if u.Host == "" { + return fmt.Errorf("missing host for ObserveURL %s", config.ObserveURL) + } + + if config.Token == "" { + return errors.New("missing Token") + } + if !strings.Contains(config.Token, ":") { + return errors.New("invalid Token, the provided value may be the token ID instead of the token itself") + } + return nil +} diff --git a/internal/config/configschema_test.go b/internal/config/configschema_test.go new file mode 100644 index 000000000..5cc16845e --- /dev/null +++ b/internal/config/configschema_test.go @@ -0,0 +1,45 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAgentConfigValidate(t *testing.T) { + validConfig := AgentConfig{ + Token: "some:token", + ObserveURL: "https://observeinc.com", + } + assert.NoError(t, validConfig.Validate()) + + missingURLConfig := AgentConfig{ + Token: "some:token", + ObserveURL: "", + } + assert.ErrorContains(t, missingURLConfig.Validate(), "missing ObserveURL") + + invalidURLConfig1 := AgentConfig{ + Token: "some:token", + ObserveURL: "observeinc.com", + } + assert.ErrorContains(t, invalidURLConfig1.Validate(), "missing scheme for ObserveURL") + + invalidURLConfig2 := AgentConfig{ + Token: "some:token", + ObserveURL: "http://", + } + assert.ErrorContains(t, invalidURLConfig2.Validate(), "missing host for ObserveURL") + + missingTokenConfig := AgentConfig{ + Token: "", + ObserveURL: "https://observeinc.com", + } + assert.ErrorContains(t, missingTokenConfig.Validate(), "missing Token") + + invalidTokenConfig := AgentConfig{ + Token: "1234", + ObserveURL: "https://observeinc.com", + } + assert.ErrorContains(t, invalidTokenConfig.Validate(), "invalid Token") +}