diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05f559f..0de2c50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,9 +73,35 @@ jobs: go tool cover -func=coverage-integration.out | grep total | awk '{print "Integration test coverage: " $3}' fi + - name: Check coverage threshold + if: matrix.go-version == '1.25' + run: | + if [ ! -f coverage.out ]; then + echo "::error::Coverage file not found" + exit 1 + fi + + COVERAGE=$(go tool cover -func=coverage.out | grep total | awk '{print $3}' | sed 's/%//') + + if [ -z "$COVERAGE" ]; then + echo "::error::Failed to extract coverage percentage" + exit 1 + fi + + THRESHOLD=80 + echo "Current coverage: ${COVERAGE}%" + echo "Minimum threshold: ${THRESHOLD}%" + + if (( $(echo "$COVERAGE < $THRESHOLD" | bc -l) )); then + echo "::error::Coverage ${COVERAGE}% is below minimum threshold of ${THRESHOLD}%" + exit 1 + fi + echo "Coverage check passed!" + - name: Upload coverage to Codecov if: matrix.go-version == '1.25' uses: codecov/codecov-action@v4 + continue-on-error: true # Codecov is informational; inline check above is the hard gate with: files: ./coverage.out,./coverage-integration.out flags: unittests diff --git a/Makefile b/Makefile index 20c09a6..d9c1ace 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,8 @@ check-prerequisites check-docker check-kubectl \ build test test-integration test-e2e test-all \ lint fmt clean install-deps \ - docker-build docker-push + docker-build docker-push \ + coverage-check # ================================================================================================ # Project Configuration @@ -232,6 +233,20 @@ test-all: @go tool cover -func=coverage/coverage.out | grep total | awk '{print "Total coverage: " $$3}' @$(call print_success,"All tests passed - coverage report: coverage/coverage.html") +# Check coverage meets minimum threshold (80%) +COVERAGE_THRESHOLD := 80 +coverage-check: + @$(call print_status,"Checking coverage threshold (minimum $(COVERAGE_THRESHOLD)%)...") + @mkdir -p coverage + @go test ./... -covermode=atomic -coverprofile=coverage/coverage.out -short > /dev/null 2>&1 + @COVERAGE=$$(go tool cover -func=coverage/coverage.out | grep total | awk '{print $$3}' | sed 's/%//'); \ + echo "Current coverage: $${COVERAGE}%"; \ + if [ $$(echo "$${COVERAGE} < $(COVERAGE_THRESHOLD)" | bc -l) -eq 1 ]; then \ + $(call print_error,"Coverage $${COVERAGE}% is below minimum threshold of $(COVERAGE_THRESHOLD)%"); \ + exit 1; \ + fi + @$(call print_success,"Coverage check passed (>= $(COVERAGE_THRESHOLD)%)") + # Lint code with golangci-lint lint: @$(call print_status,"Running golangci-lint...") @@ -546,6 +561,7 @@ help: @echo " make test-integration Run integration tests" @echo " make test-e2e Run end-to-end tests" @echo " make test-all Run all tests with coverage report" + @echo " make coverage-check Verify coverage >= 80% threshold" @echo " make lint Run golangci-lint" @echo " make fmt Format code with gofmt and goimports" @echo "" diff --git a/cmd/node-doctor/main_additional_test.go b/cmd/node-doctor/main_additional_test.go index d0b8a40..15250bc 100644 --- a/cmd/node-doctor/main_additional_test.go +++ b/cmd/node-doctor/main_additional_test.go @@ -319,6 +319,447 @@ func TestMonitorFactoryAdapter_ContextPropagation(t *testing.T) { } } +// TestCreateExporters_HTTPExporterEnabled tests HTTP exporter creation when enabled. +func TestCreateExporters_HTTPExporterEnabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // HTTP exporter uses webhooks, not bind address/port + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: false, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: true, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: "http://localhost:9999/webhook", + }, + }, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: false, + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have at least the health server and possibly HTTP exporter + if len(exporters) == 0 { + t.Error("createExporters() should create at least one exporter") + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one exporter interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_PrometheusExporterEnabled tests Prometheus exporter creation when enabled. +func TestCreateExporters_PrometheusExporterEnabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: false, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: false, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 19091, // Use high port to avoid conflicts + Path: "/metrics", + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have at least the health server and possibly Prometheus exporter + if len(exporters) == 0 { + t.Error("createExporters() should create at least one exporter") + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one exporter interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_KubernetesExporterEnabled tests Kubernetes exporter creation when enabled. +// Note: This will fail to create the exporter without valid kubeconfig, but should not error. +func TestCreateExporters_KubernetesExporterEnabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: true, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: false, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: false, + }, + }, + } + + // This should not panic even without valid kubeconfig + // It will log a warning but continue + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have at least the health server or noop exporter + if len(exporters) == 0 { + t.Error("createExporters() should create at least one exporter (health or noop)") + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one exporter interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_AllExportersEnabled tests all exporters enabled simultaneously. +func TestCreateExporters_AllExportersEnabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: true, // Will fail without kubeconfig but shouldn't crash + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: true, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook-all", + URL: "http://localhost:9998/webhook", + }, + }, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 19092, // Use high port + Path: "/metrics", + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have multiple exporters + if len(exporters) == 0 { + t.Error("createExporters() should create at least one exporter") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } + _ = interfaces +} + +// TestCreateExporters_HealthServerCreation tests that health server is always attempted. +func TestCreateExporters_HealthServerCreation(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Even with all exporters disabled, health server should be created + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: false, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: false, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: false, + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Health server should be created (or noop if it fails) + if len(exporters) == 0 { + t.Error("createExporters() should create health server or noop exporter") + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_NoopFallbackVerification verifies noop exporter is used when needed. +func TestCreateExporters_NoopFallbackVerification(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // Create config with all nil exporter configs + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + // All nil - should result in noop fallback eventually + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have at least one exporter (health server or noop) + if len(exporters) == 0 { + t.Error("createExporters() should always return at least one exporter") + } + if len(interfaces) == 0 { + t.Error("createExporters() should always return at least one interface") + } + + // Verify exporters are usable + for _, iface := range interfaces { + status := &types.Status{ + Source: "test", + Timestamp: time.Now(), + } + if err := iface.ExportStatus(ctx, status); err != nil { + t.Errorf("Exporter.ExportStatus() error = %v", err) + } + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_HTTPExporterWithValidConfig tests HTTP exporter with valid configuration. +func TestCreateExporters_HTTPExporterWithValidConfig(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + // HTTP exporter with valid configuration (requires workers > 0, queueSize > 0, timeout > 0) + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: false, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 2, + QueueSize: 100, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 3, + BaseDelayString: "1s", + BaseDelay: 1 * time.Second, + MaxDelayString: "30s", + MaxDelay: 30 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook-valid", + URL: "http://localhost:9997/webhook", + TimeoutString: "10s", + Timeout: 10 * time.Second, + }, + }, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: false, + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have health server and HTTP exporter + if len(exporters) < 2 { + t.Logf("Expected at least 2 exporters (health + HTTP), got %d", len(exporters)) + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one exporter interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_KubernetesExporterWithValidConfig tests Kubernetes exporter with valid configuration. +// Note: This test will fail to actually create a Kubernetes client without a valid kubeconfig, +// but it exercises the validation and error handling paths. +func TestCreateExporters_KubernetesExporterWithValidConfig(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateIntervalString: "30s", + UpdateInterval: 30 * time.Second, + ResyncIntervalString: "5m", + ResyncInterval: 5 * time.Minute, + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: false, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: false, + }, + }, + } + + // This will fail without kubeconfig but should exercise the validation path + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have at least one exporter (health server) + if len(exporters) == 0 { + t.Error("createExporters() should create at least one exporter") + } + if len(interfaces) == 0 { + t.Error("createExporters() should return at least one exporter interface") + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } +} + +// TestCreateExporters_MultipleExportersWithValidConfig tests multiple exporters with valid configurations. +func TestCreateExporters_MultipleExportersWithValidConfig(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + config := &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{ + NodeName: "test-node", + }, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateIntervalString: "30s", + UpdateInterval: 30 * time.Second, + ResyncIntervalString: "5m", + ResyncInterval: 5 * time.Minute, // Will still fail without kubeconfig but pass validation + }, + HTTP: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 2, + QueueSize: 100, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 3, + BaseDelayString: "1s", + BaseDelay: 1 * time.Second, + MaxDelayString: "30s", + MaxDelay: 30 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook-multi", + URL: "http://localhost:9996/webhook", + TimeoutString: "10s", + Timeout: 10 * time.Second, + }, + }, + }, + Prometheus: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 19093, + Path: "/metrics", + }, + }, + } + + exporters, interfaces, err := createExporters(ctx, config) + if err != nil { + t.Errorf("createExporters() error = %v, want nil", err) + } + + // Should have health server, HTTP exporter, and Prometheus exporter + // (Kubernetes will fail without kubeconfig) + if len(exporters) < 2 { + t.Logf("Expected at least 2 exporters, got %d", len(exporters)) + } + + // Clean up + for _, exp := range exporters { + exp.Stop() + } + _ = interfaces +} + // TestDumpConfiguration_ComplexConfig tests dumpConfiguration with a more complex configuration. func TestDumpConfiguration_ComplexConfig(t *testing.T) { config := &types.NodeDoctorConfig{ diff --git a/cmd/node-doctor/main_comprehensive_test.go b/cmd/node-doctor/main_comprehensive_test.go new file mode 100644 index 0000000..24c23a4 --- /dev/null +++ b/cmd/node-doctor/main_comprehensive_test.go @@ -0,0 +1,922 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "time" + + "github.com/supporttools/node-doctor/pkg/types" + "github.com/supporttools/node-doctor/pkg/util" +) + +// ============================================================================= +// Configuration Loading Tests +// ============================================================================= + +func TestConfigurationLoading(t *testing.T) { + tests := []struct { + name string + configFile string + expectError bool + validate func(*testing.T, *types.NodeDoctorConfig) + }{ + { + name: "valid YAML config", + configFile: "testdata/valid_config.yaml", + expectError: false, + validate: func(t *testing.T, cfg *types.NodeDoctorConfig) { + if cfg.Settings.NodeName != "test-node" { + t.Errorf("NodeName = %q, want %q", cfg.Settings.NodeName, "test-node") + } + if cfg.Settings.LogLevel != "info" { + t.Errorf("LogLevel = %q, want %q", cfg.Settings.LogLevel, "info") + } + }, + }, + { + name: "full config with all options", + configFile: "testdata/full_config.yaml", + expectError: false, + validate: func(t *testing.T, cfg *types.NodeDoctorConfig) { + if cfg.Settings.NodeName != "full-test-node" { + t.Errorf("NodeName = %q, want %q", cfg.Settings.NodeName, "full-test-node") + } + if cfg.Settings.LogLevel != "debug" { + t.Errorf("LogLevel = %q, want %q", cfg.Settings.LogLevel, "debug") + } + if len(cfg.Monitors) != 2 { + t.Errorf("Monitors count = %d, want 2", len(cfg.Monitors)) + } + }, + }, + { + name: "invalid YAML syntax", + configFile: "testdata/invalid_syntax.yaml", + expectError: true, + }, + { + name: "non-existent file", + configFile: "testdata/nonexistent.yaml", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configPath := filepath.Join(getTestdataDir(t), filepath.Base(tt.configFile)) + + cfg, err := util.LoadConfig(configPath) + + if tt.expectError { + if err == nil { + t.Error("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if tt.validate != nil { + tt.validate(t, cfg) + } + }) + } +} + +func TestConfigFileSearchOrder(t *testing.T) { + // Test that config file search order is correct + candidates := []string{ + "/etc/node-doctor/config.yaml", + "/etc/node-doctor/config.yml", + "./config.yaml", + "./config.yml", + } + + // Verify the order is as expected (from main.go) + expectedOrder := []string{ + "/etc/node-doctor/config.yaml", + "/etc/node-doctor/config.yml", + "./config.yaml", + "./config.yml", + } + + for i, c := range candidates { + if c != expectedOrder[i] { + t.Errorf("Config search order[%d] = %q, want %q", i, c, expectedOrder[i]) + } + } +} + +// ============================================================================= +// CreateMonitors Tests +// ============================================================================= + +func TestCreateMonitors_TableDriven(t *testing.T) { + ctx := context.Background() + + tests := []struct { + name string + configs []types.MonitorConfig + expectedCount int + expectedError bool + skipMonitors []string // monitors that should be skipped + }{ + { + name: "empty config list", + configs: []types.MonitorConfig{}, + expectedCount: 0, + expectedError: false, + }, + { + name: "single enabled noop monitor", + configs: []types.MonitorConfig{ + {Name: "test-noop", Type: "noop", Enabled: true, Interval: 30 * time.Second}, + }, + expectedCount: 1, + expectedError: false, + }, + { + name: "single disabled monitor", + configs: []types.MonitorConfig{ + {Name: "disabled", Type: "noop", Enabled: false}, + }, + expectedCount: 0, + expectedError: false, + skipMonitors: []string{"disabled"}, + }, + { + name: "mixed enabled and disabled", + configs: []types.MonitorConfig{ + {Name: "enabled1", Type: "noop", Enabled: true, Interval: 30 * time.Second}, + {Name: "disabled1", Type: "noop", Enabled: false}, + {Name: "enabled2", Type: "noop", Enabled: true, Interval: 30 * time.Second}, + }, + expectedCount: 2, + expectedError: false, + skipMonitors: []string{"disabled1"}, + }, + { + name: "invalid monitor type continues", + configs: []types.MonitorConfig{ + {Name: "invalid", Type: "nonexistent-type-xyz", Enabled: true}, + }, + expectedCount: 0, + expectedError: false, // createMonitors logs error but continues + }, + { + name: "mix of valid and invalid", + configs: []types.MonitorConfig{ + {Name: "valid", Type: "noop", Enabled: true, Interval: 30 * time.Second}, + {Name: "invalid", Type: "nonexistent-type", Enabled: true}, + }, + expectedCount: 1, + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + monitors, err := createMonitors(ctx, tt.configs) + + if tt.expectedError { + if err == nil { + t.Error("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if len(monitors) != tt.expectedCount { + t.Errorf("Monitor count = %d, want %d", len(monitors), tt.expectedCount) + } + }) + } +} + +func TestCreateMonitors_ContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + configs := []types.MonitorConfig{ + {Name: "test", Type: "noop", Enabled: true, Interval: 30 * time.Second}, + } + + // Should still work with cancelled context (context is stored, not used immediately) + monitors, err := createMonitors(ctx, configs) + if err != nil { + t.Errorf("createMonitors with cancelled context should not error: %v", err) + } + + // Monitor may or may not be created depending on implementation + _ = monitors +} + +// ============================================================================= +// CreateExporters Tests +// ============================================================================= + +func TestCreateExporters_TableDriven(t *testing.T) { + tests := []struct { + name string + config *types.NodeDoctorConfig + minExporterCount int + expectNoopFallback bool + }{ + { + name: "all exporters disabled", + config: &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{NodeName: "test-node"}, + Exporters: types.ExporterConfigs{ + Kubernetes: &types.KubernetesExporterConfig{Enabled: false}, + HTTP: &types.HTTPExporterConfig{Enabled: false}, + Prometheus: &types.PrometheusExporterConfig{Enabled: false}, + }, + }, + minExporterCount: 1, // At least noop or health server + expectNoopFallback: true, + }, + { + name: "nil exporter configs", + config: &types.NodeDoctorConfig{ + Settings: types.GlobalSettings{NodeName: "test-node"}, + Exporters: types.ExporterConfigs{}, + }, + minExporterCount: 1, + expectNoopFallback: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + exporters, interfaces, err := createExporters(ctx, tt.config) + if err != nil { + t.Errorf("createExporters() error = %v", err) + return + } + + if len(exporters) < tt.minExporterCount { + t.Errorf("Exporter count = %d, want at least %d", len(exporters), tt.minExporterCount) + } + + if len(interfaces) < tt.minExporterCount { + t.Errorf("Interface count = %d, want at least %d", len(interfaces), tt.minExporterCount) + } + + // Cleanup + for _, exp := range exporters { + exp.Stop() + } + }) + } +} + +// ============================================================================= +// NoopExporter Tests +// ============================================================================= + +func TestNoopExporter_ExportStatus(t *testing.T) { + exporter := &noopExporter{} + ctx := context.Background() + + tests := []struct { + name string + status *types.Status + }{ + { + name: "empty status", + status: &types.Status{ + Source: "test", + Timestamp: time.Now(), + }, + }, + { + name: "status with events", + status: &types.Status{ + Source: "test", + Events: []types.Event{ + {Severity: types.EventInfo, Reason: "test", Message: "event1"}, + {Severity: types.EventWarning, Reason: "warn", Message: "event2"}, + }, + Timestamp: time.Now(), + }, + }, + { + name: "status with conditions", + status: &types.Status{ + Source: "test", + Conditions: []types.Condition{ + {Type: "Ready", Status: types.ConditionTrue}, + {Type: "Available", Status: types.ConditionFalse}, + }, + Timestamp: time.Now(), + }, + }, + { + name: "status with events and conditions", + status: &types.Status{ + Source: "test", + Events: []types.Event{ + {Severity: types.EventInfo, Reason: "test", Message: "event"}, + }, + Conditions: []types.Condition{ + {Type: "Ready", Status: types.ConditionTrue}, + }, + Timestamp: time.Now(), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exporter.ExportStatus(ctx, tt.status) + if err != nil { + t.Errorf("ExportStatus() error = %v", err) + } + }) + } +} + +func TestNoopExporter_ExportProblem(t *testing.T) { + exporter := &noopExporter{} + ctx := context.Background() + + tests := []struct { + name string + problem *types.Problem + }{ + { + name: "info severity", + problem: &types.Problem{ + Type: "test-info", + Resource: "resource1", + Severity: types.ProblemInfo, + Message: "info message", + DetectedAt: time.Now(), + }, + }, + { + name: "warning severity", + problem: &types.Problem{ + Type: "test-warning", + Resource: "resource2", + Severity: types.ProblemWarning, + Message: "warning message", + DetectedAt: time.Now(), + }, + }, + { + name: "critical severity", + problem: &types.Problem{ + Type: "test-critical", + Resource: "resource3", + Severity: types.ProblemCritical, + Message: "critical message", + DetectedAt: time.Now(), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exporter.ExportProblem(ctx, tt.problem) + if err != nil { + t.Errorf("ExportProblem() error = %v", err) + } + }) + } +} + +func TestNoopExporter_Stop(t *testing.T) { + exporter := &noopExporter{} + + // Stop should be idempotent + for i := 0; i < 3; i++ { + err := exporter.Stop() + if err != nil { + t.Errorf("Stop() call %d error = %v", i+1, err) + } + } +} + +func TestNoopExporter_ConcurrentAccess(t *testing.T) { + exporter := &noopExporter{} + ctx := context.Background() + + var wg sync.WaitGroup + errChan := make(chan error, 100) + + // Concurrent ExportStatus calls + for i := 0; i < 50; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + status := &types.Status{ + Source: "test", + Timestamp: time.Now(), + } + if err := exporter.ExportStatus(ctx, status); err != nil { + errChan <- err + } + }(i) + } + + // Concurrent ExportProblem calls + for i := 0; i < 50; i++ { + wg.Add(1) + go func(id int) { + defer wg.Done() + problem := &types.Problem{ + Type: "test", + Resource: "resource", + Severity: types.ProblemInfo, + Message: "test", + DetectedAt: time.Now(), + } + if err := exporter.ExportProblem(ctx, problem); err != nil { + errChan <- err + } + }(i) + } + + wg.Wait() + close(errChan) + + for err := range errChan { + t.Errorf("Concurrent access error: %v", err) + } +} + +// ============================================================================= +// MonitorFactoryAdapter Tests +// ============================================================================= + +func TestMonitorFactoryAdapter_CreateMonitor(t *testing.T) { + ctx := context.Background() + adapter := &monitorFactoryAdapter{ctx: ctx} + + tests := []struct { + name string + config types.MonitorConfig + expectError bool + }{ + { + name: "valid noop monitor", + config: types.MonitorConfig{ + Name: "test", + Type: "noop", + Enabled: true, + Interval: 30 * time.Second, + }, + expectError: false, + }, + { + name: "invalid monitor type", + config: types.MonitorConfig{ + Name: "test", + Type: "nonexistent-monitor-type", + Enabled: true, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + monitor, err := adapter.CreateMonitor(tt.config) + + if tt.expectError { + if err == nil { + t.Error("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if monitor == nil { + t.Error("Monitor is nil") + } + }) + } +} + +// ============================================================================= +// DumpConfiguration Tests +// ============================================================================= + +func TestDumpConfiguration_OutputFormat(t *testing.T) { + config := &types.NodeDoctorConfig{ + APIVersion: "v1", + Kind: "NodeDoctorConfig", + Metadata: types.ConfigMetadata{Name: "test"}, + Settings: types.GlobalSettings{ + NodeName: "test-node", + LogLevel: "info", + LogFormat: "json", + }, + } + + // Capture stdout + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + dumpConfiguration(config) + + w.Close() + os.Stdout = old + + var buf bytes.Buffer + io.Copy(&buf, r) + output := buf.String() + + // Verify it's valid JSON + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(output), &parsed); err != nil { + t.Errorf("Output is not valid JSON: %v", err) + } + + // Verify indentation (pretty printed) + if !strings.Contains(output, "\n") { + t.Error("Output should be pretty printed with newlines") + } +} + +func TestDumpConfiguration_PreservesAllFields(t *testing.T) { + config := &types.NodeDoctorConfig{ + APIVersion: "v1", + Kind: "NodeDoctorConfig", + Metadata: types.ConfigMetadata{ + Name: "test-config", + Namespace: "test-namespace", + }, + Settings: types.GlobalSettings{ + NodeName: "test-node", + LogLevel: "debug", + LogFormat: "text", + DryRunMode: true, + }, + Monitors: []types.MonitorConfig{ + {Name: "mon1", Type: "noop", Enabled: true}, + {Name: "mon2", Type: "noop", Enabled: false}, + }, + } + + // Capture stdout + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + dumpConfiguration(config) + + w.Close() + os.Stdout = old + + var buf bytes.Buffer + io.Copy(&buf, r) + output := buf.String() + + // Parse and verify + var parsed types.NodeDoctorConfig + if err := json.Unmarshal([]byte(output), &parsed); err != nil { + t.Fatalf("Failed to parse output: %v", err) + } + + if parsed.Metadata.Name != "test-config" { + t.Errorf("Metadata.Name = %q, want %q", parsed.Metadata.Name, "test-config") + } + if parsed.Settings.DryRunMode != true { + t.Error("DryRunMode not preserved") + } + if len(parsed.Monitors) != 2 { + t.Errorf("Monitor count = %d, want 2", len(parsed.Monitors)) + } +} + +// ============================================================================= +// Version Information Tests +// ============================================================================= + +func TestVersionVariables(t *testing.T) { + // Test that version variables are defined + if Version == "" { + t.Error("Version should not be empty") + } + + // In test context, these should have default values + if Version != "dev" && Version == "" { + t.Error("Version should be 'dev' or a valid version string") + } +} + +func TestVersionOutputFormat(t *testing.T) { + // Test the expected format of version output + // This tests the format used in main() for -version flag + + expectedFields := []string{ + "Node Doctor", + "Git Commit:", + "Build Time:", + "Go Version:", + "OS/Arch:", + } + + // Build expected output format + output := "Node Doctor " + Version + "\n" + output += "Git Commit: " + GitCommit + "\n" + output += "Build Time: " + BuildTime + "\n" + output += "Go Version: " + runtime.Version() + "\n" + output += "OS/Arch: " + runtime.GOOS + "/" + runtime.GOARCH + "\n" + + for _, field := range expectedFields { + if !strings.Contains(output, field) { + t.Errorf("Version output should contain %q", field) + } + } +} + +// ============================================================================= +// Interface Compliance Tests +// ============================================================================= + +func TestInterfaceCompliance(t *testing.T) { + // Verify noopExporter implements required interfaces + var _ types.Exporter = (*noopExporter)(nil) + var _ ExporterLifecycle = (*noopExporter)(nil) + + // Verify monitorFactoryAdapter can be used + ctx := context.Background() + _ = &monitorFactoryAdapter{ctx: ctx} +} + +// ============================================================================= +// Integration-Style Tests (using subprocess) +// ============================================================================= + +func TestMainBinary_VersionFlag(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with -version flag + cmd = exec.Command(binaryPath, "-version") + output, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("Running with -version flag failed: %v\nOutput: %s", err, output) + return + } + + outputStr := string(output) + if !strings.Contains(outputStr, "Node Doctor") { + t.Error("Version output should contain 'Node Doctor'") + } + if !strings.Contains(outputStr, "Go Version:") { + t.Error("Version output should contain Go version") + } +} + +func TestMainBinary_ListMonitors(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with -list-monitors flag + cmd = exec.Command(binaryPath, "-list-monitors") + output, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("Running with -list-monitors flag failed: %v\nOutput: %s", err, output) + return + } + + outputStr := string(output) + if !strings.Contains(outputStr, "Available monitor types") { + t.Error("Output should contain 'Available monitor types'") + } +} + +func TestMainBinary_ValidateConfig(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with valid config + configPath := filepath.Join(getTestdataDir(t), "valid_config.yaml") + cmd = exec.Command(binaryPath, "-config", configPath, "-validate-config") + output, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("Validation of valid config failed: %v\nOutput: %s", err, output) + } +} + +func TestMainBinary_DumpConfig(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with valid config and dump-config + configPath := filepath.Join(getTestdataDir(t), "valid_config.yaml") + cmd = exec.Command(binaryPath, "-config", configPath, "-dump-config") + output, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("Dump config failed: %v\nOutput: %s", err, output) + return + } + + // Output should be valid JSON + var parsed map[string]interface{} + // Skip log lines and find JSON + lines := strings.Split(string(output), "\n") + var jsonLines []string + inJSON := false + for _, line := range lines { + if strings.HasPrefix(line, "{") { + inJSON = true + } + if inJSON { + jsonLines = append(jsonLines, line) + } + } + jsonOutput := strings.Join(jsonLines, "\n") + + if err := json.Unmarshal([]byte(jsonOutput), &parsed); err != nil { + t.Errorf("Dump config output is not valid JSON: %v\nOutput: %s", err, jsonOutput) + } +} + +func TestMainBinary_MissingConfig(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with non-existent config + cmd = exec.Command(binaryPath, "-config", "/nonexistent/config.yaml") + output, err := cmd.CombinedOutput() + + // Should fail with error + if err == nil { + t.Error("Expected error for missing config file") + } + + outputStr := string(output) + if !strings.Contains(outputStr, "Failed to load configuration") && !strings.Contains(outputStr, "No configuration file") { + t.Errorf("Output should indicate config loading failure, got: %s", outputStr) + } +} + +func TestMainBinary_InvalidConfig(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with invalid config + configPath := filepath.Join(getTestdataDir(t), "invalid_syntax.yaml") + cmd = exec.Command(binaryPath, "-config", configPath) + output, err := cmd.CombinedOutput() + + // Should fail + if err == nil { + t.Error("Expected error for invalid config file") + } + + _ = output // Error output may vary +} + +func TestMainBinary_FlagOverrides(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + + // Build the binary first + binaryPath := filepath.Join(t.TempDir(), "node-doctor-test") + cmd := exec.Command("go", "build", "-o", binaryPath, ".") + cmd.Dir = getPackageDir(t) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("Failed to build binary: %v\nOutput: %s", err, output) + } + + // Run with config and flags + configPath := filepath.Join(getTestdataDir(t), "valid_config.yaml") + cmd = exec.Command(binaryPath, + "-config", configPath, + "-debug", + "-log-level", "warn", + "-dump-config", + ) + output, err := cmd.CombinedOutput() + if err != nil { + t.Errorf("Running with flag overrides failed: %v\nOutput: %s", err, output) + return + } + + // Parse JSON output (skip log lines) + lines := strings.Split(string(output), "\n") + var jsonLines []string + inJSON := false + for _, line := range lines { + if strings.HasPrefix(line, "{") { + inJSON = true + } + if inJSON { + jsonLines = append(jsonLines, line) + } + } + jsonOutput := strings.Join(jsonLines, "\n") + + var parsed types.NodeDoctorConfig + if err := json.Unmarshal([]byte(jsonOutput), &parsed); err != nil { + t.Fatalf("Failed to parse config output: %v", err) + } + + // -log-level should override to "warn" + if parsed.Settings.LogLevel != "warn" { + t.Errorf("LogLevel = %q, want %q (flag override)", parsed.Settings.LogLevel, "warn") + } +} + +// ============================================================================= +// Helper Functions +// ============================================================================= + +func getPackageDir(t *testing.T) string { + t.Helper() + _, filename, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("Failed to get caller info") + } + return filepath.Dir(filename) +} + +func getTestdataDir(t *testing.T) string { + t.Helper() + return filepath.Join(getPackageDir(t), "testdata") +} diff --git a/cmd/node-doctor/testdata/empty_monitors.yaml b/cmd/node-doctor/testdata/empty_monitors.yaml new file mode 100644 index 0000000..07474ec --- /dev/null +++ b/cmd/node-doctor/testdata/empty_monitors.yaml @@ -0,0 +1,13 @@ +apiVersion: node-doctor.io/v1alpha1 +kind: NodeDoctorConfig +metadata: + name: empty-monitors-config +settings: + nodeName: test-node + logLevel: info +monitors: [] +exporters: + kubernetes: + enabled: false +remediation: + enabled: false diff --git a/cmd/node-doctor/testdata/full_config.yaml b/cmd/node-doctor/testdata/full_config.yaml new file mode 100644 index 0000000..9b18840 --- /dev/null +++ b/cmd/node-doctor/testdata/full_config.yaml @@ -0,0 +1,37 @@ +apiVersion: node-doctor.io/v1alpha1 +kind: NodeDoctorConfig +metadata: + name: full-config + namespace: kube-system +settings: + nodeName: full-test-node + logLevel: debug + logFormat: text + dryRunMode: false +monitors: + - name: noop-monitor + type: noop + enabled: true + interval: 30s + - name: disabled-monitor + type: noop + enabled: false + interval: 60s +exporters: + kubernetes: + enabled: false + http: + enabled: false + port: 8081 + prometheus: + enabled: false + port: 9090 + path: /metrics +remediation: + enabled: true + dryRun: true + globalCooldown: 5m + maxConcurrentRemediations: 1 +features: + enableProfiling: false + profilingPort: 6060 diff --git a/cmd/node-doctor/testdata/invalid_syntax.yaml b/cmd/node-doctor/testdata/invalid_syntax.yaml new file mode 100644 index 0000000..58efcb5 --- /dev/null +++ b/cmd/node-doctor/testdata/invalid_syntax.yaml @@ -0,0 +1,9 @@ +apiVersion: node-doctor.io/v1alpha1 +kind: NodeDoctorConfig +metadata: + name: "invalid config + # Missing closing quote - invalid YAML syntax +settings: + nodeName: test-node +monitors: + - name: test-monitor diff --git a/cmd/node-doctor/testdata/valid_config.yaml b/cmd/node-doctor/testdata/valid_config.yaml new file mode 100644 index 0000000..e4a3070 --- /dev/null +++ b/cmd/node-doctor/testdata/valid_config.yaml @@ -0,0 +1,22 @@ +apiVersion: node-doctor.io/v1alpha1 +kind: NodeDoctorConfig +metadata: + name: test-config +settings: + nodeName: test-node + logLevel: info + logFormat: json +monitors: + - name: test-noop + type: noop + enabled: true + interval: 30s +exporters: + kubernetes: + enabled: false + http: + enabled: false + prometheus: + enabled: false +remediation: + enabled: false diff --git a/codecov.yml b/codecov.yml new file mode 100644 index 0000000..f96b36e --- /dev/null +++ b/codecov.yml @@ -0,0 +1,48 @@ +# Codecov Configuration +# https://docs.codecov.com/docs/codecov-yaml + +coverage: + # Global coverage thresholds + precision: 2 + round: down + range: "70...100" + + status: + # Project-level coverage (overall repository) + project: + default: + target: 80% + threshold: 2% + if_ci_failed: error + + # Patch-level coverage (only changed lines in PR) + patch: + default: + target: 80% + threshold: 5% + if_ci_failed: error + +# Comment configuration for PRs +comment: + layout: "reach,diff,flags,tree,betaprofiling" + behavior: default + require_changes: true + require_base: false + require_head: true + +# Ignore certain paths from coverage +ignore: + - "**/*_test.go" + - "**/test/**" + - "**/testdata/**" + - "docs/**" + - "deployment/**" + - "helm/**" + +# Flag management +flags: + unittests: + paths: + - pkg/ + - cmd/ + carryforward: true diff --git a/pkg/detector/monitor_handle_test.go b/pkg/detector/monitor_handle_test.go new file mode 100644 index 0000000..d4e30a7 --- /dev/null +++ b/pkg/detector/monitor_handle_test.go @@ -0,0 +1,304 @@ +package detector + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/supporttools/node-doctor/pkg/types" +) + +// TestMonitorHandle_GetConfig tests the GetConfig method of MonitorHandle. +func TestMonitorHandle_GetConfig(t *testing.T) { + config := types.MonitorConfig{ + Name: "test-monitor", + Type: "test", + Enabled: true, + Interval: 10 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "key": "value", + }, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mock := NewMockMonitor(config.Name) + statusCh := make(chan *types.Status) + + mh := &MonitorHandle{ + monitor: mock, + config: config, + statusCh: statusCh, + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: false, + } + + // Test GetConfig returns the correct config + got := mh.GetConfig() + + if got.Name != config.Name { + t.Errorf("GetConfig().Name = %s, want %s", got.Name, config.Name) + } + if got.Type != config.Type { + t.Errorf("GetConfig().Type = %s, want %s", got.Type, config.Type) + } + if got.Enabled != config.Enabled { + t.Errorf("GetConfig().Enabled = %v, want %v", got.Enabled, config.Enabled) + } + if got.Interval != config.Interval { + t.Errorf("GetConfig().Interval = %v, want %v", got.Interval, config.Interval) + } + if got.Timeout != config.Timeout { + t.Errorf("GetConfig().Timeout = %v, want %v", got.Timeout, config.Timeout) + } +} + +// TestMonitorHandle_IsRunning tests the IsRunning method of MonitorHandle. +func TestMonitorHandle_IsRunning(t *testing.T) { + tests := []struct { + name string + stopped bool + want bool + }{ + { + name: "running when not stopped", + stopped: false, + want: true, + }, + { + name: "not running when stopped", + stopped: true, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mock := NewMockMonitor("test") + statusCh := make(chan *types.Status) + + mh := &MonitorHandle{ + monitor: mock, + config: types.MonitorConfig{Name: "test"}, + statusCh: statusCh, + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: tt.stopped, + } + + got := mh.IsRunning() + if got != tt.want { + t.Errorf("IsRunning() = %v, want %v", got, tt.want) + } + }) + } +} + +// TestMonitorHandle_IsRunning_ConcurrentAccess tests thread safety of IsRunning. +func TestMonitorHandle_IsRunning_ConcurrentAccess(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mock := NewMockMonitor("test") + statusCh := make(chan *types.Status) + + mh := &MonitorHandle{ + monitor: mock, + config: types.MonitorConfig{Name: "test"}, + statusCh: statusCh, + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: false, + } + + var wg sync.WaitGroup + const goroutines = 10 + const iterations = 100 + + // Start multiple goroutines calling IsRunning + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for j := 0; j < iterations; j++ { + _ = mh.IsRunning() + } + }() + } + + // Stop the monitor midway through + go func() { + time.Sleep(5 * time.Millisecond) + mh.mu.Lock() + mh.stopped = true + mh.mu.Unlock() + }() + + wg.Wait() + // Test passes if no race conditions or panics occurred +} + +// TestMonitorHandle_GetName tests the GetName method of MonitorHandle. +func TestMonitorHandle_GetName(t *testing.T) { + tests := []struct { + name string + monitorName string + expectedName string + }{ + { + name: "simple name", + monitorName: "test-monitor", + expectedName: "test-monitor", + }, + { + name: "name with spaces", + monitorName: "my test monitor", + expectedName: "my test monitor", + }, + { + name: "empty name", + monitorName: "", + expectedName: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + mh := &MonitorHandle{ + monitor: NewMockMonitor(tt.monitorName), + config: types.MonitorConfig{Name: tt.monitorName}, + statusCh: make(chan *types.Status), + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: false, + } + + got := mh.GetName() + if got != tt.expectedName { + t.Errorf("GetName() = %s, want %s", got, tt.expectedName) + } + }) + } +} + +// TestProblemDetector_Run tests the Run method which is an alias for Start. +func TestProblemDetector_Run(t *testing.T) { + helper := NewTestHelper() + config := helper.CreateTestConfig() + factory := NewMockMonitorFactory() + + detector, err := NewProblemDetector( + config, + []types.Monitor{NewMockMonitor("test")}, + []types.Exporter{NewMockExporter("test")}, + "/tmp/test-config.yaml", + factory, + ) + if err != nil { + t.Fatalf("NewProblemDetector() error = %v", err) + } + defer detector.Stop() + + // Test that Run() starts the detector (same as Start()) + err = detector.Run() + if err != nil { + t.Errorf("Run() error = %v", err) + } + + // Verify detector is running + if !detector.IsRunning() { + t.Error("Run() should have started the detector") + } + + // Test that calling Run() again returns an error (same as Start()) + err = detector.Run() + if err == nil { + t.Error("Run() should error when detector is already running") + } +} + +// TestMonitorHandle_Stop tests the Stop method of MonitorHandle. +func TestMonitorHandle_Stop(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + mock := NewMockMonitor("test") + statusCh := make(chan *types.Status) + + mh := &MonitorHandle{ + monitor: mock, + config: types.MonitorConfig{Name: "test"}, + statusCh: statusCh, + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: false, + } + + // Stop should succeed the first time + err := mh.Stop() + if err != nil { + t.Errorf("Stop() error = %v", err) + } + + // Verify IsRunning returns false after stop + if mh.IsRunning() { + t.Error("IsRunning() should return false after Stop()") + } + + // Stop should be idempotent + err = mh.Stop() + if err != nil { + t.Errorf("Stop() should be idempotent, got error = %v", err) + } +} + +// TestMonitorHandle_StopConcurrent tests concurrent Stop calls. +func TestMonitorHandle_StopConcurrent(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + mock := NewMockMonitor("test") + statusCh := make(chan *types.Status) + + mh := &MonitorHandle{ + monitor: mock, + config: types.MonitorConfig{Name: "test"}, + statusCh: statusCh, + cancelFunc: cancel, + wg: &sync.WaitGroup{}, + ctx: ctx, + stopped: false, + } + + var wg sync.WaitGroup + const goroutines = 10 + + // Start multiple goroutines calling Stop concurrently + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _ = mh.Stop() + }() + } + + wg.Wait() + + // Verify monitor is stopped + if mh.IsRunning() { + t.Error("Monitor should be stopped after concurrent Stop calls") + } +} diff --git a/pkg/detector/reload_integration_test.go b/pkg/detector/reload_integration_test.go index 00af8c8..1bf86f2 100644 --- a/pkg/detector/reload_integration_test.go +++ b/pkg/detector/reload_integration_test.go @@ -3,7 +3,6 @@ package detector import ( "context" "fmt" - "io/ioutil" "os" "path/filepath" "sync" @@ -28,7 +27,7 @@ type ReloadTestHelper struct { // NewReloadTestHelper creates a new test helper for reload integration tests func NewReloadTestHelper(t *testing.T) *ReloadTestHelper { - tempDir, err := ioutil.TempDir("", "reload-test-*") + tempDir, err := os.MkdirTemp("", "reload-test-*") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } @@ -85,7 +84,7 @@ func (h *ReloadTestHelper) UpdateConfig(t *testing.T, newConfig *types.NodeDocto // Write atomically like Kubernetes ConfigMap tmpFile := h.configFile + ".tmp" - if err := ioutil.WriteFile(tmpFile, data, 0644); err != nil { + if err := os.WriteFile(tmpFile, data, 0644); err != nil { t.Fatalf("Failed to write temp config: %v", err) } @@ -106,11 +105,16 @@ func (h *ReloadTestHelper) WaitForReload(t *testing.T, timeout time.Duration) er for time.Now().Before(deadline) { h.eventsMu.Lock() for _, event := range h.events { - // Match the actual event reasons emitted by the detector - if event.Reason == "ReloadSuccess" || event.Reason == "NoChanges" || - event.Reason == "ReloadFailed" || event.Reason == "ReloadPartialFailure" { + // Match event reasons from both ReloadCoordinator (ConfigReload*) and detector (Reload*) + // ReloadCoordinator emits: ConfigReloadSucceeded, ConfigReloadFailed, ConfigReloadNoChanges + // Detector emits: ReloadSuccess, ReloadFailed, ReloadPartialFailure, NoChanges + isSuccess := event.Reason == "ReloadSuccess" || event.Reason == "ConfigReloadSucceeded" || + event.Reason == "NoChanges" || event.Reason == "ConfigReloadNoChanges" + isFailure := event.Reason == "ReloadFailed" || event.Reason == "ConfigReloadFailed" || event.Reason == "ReloadPartialFailure" + + if isSuccess || isFailure { h.eventsMu.Unlock() - if event.Reason == "ReloadFailed" || event.Reason == "ReloadPartialFailure" { + if isFailure { return fmt.Errorf("reload failed: %s", event.Message) } return nil @@ -378,7 +382,7 @@ exporters: ` // Write invalid config directly - if err := ioutil.WriteFile(helper.configFile, []byte(invalidYAML), 0644); err != nil { + if err := os.WriteFile(helper.configFile, []byte(invalidYAML), 0644); err != nil { t.Fatalf("Failed to write invalid config: %v", err) } @@ -507,7 +511,7 @@ func TestConfigReloadExporterUpdates(t *testing.T) { initialConfig.Exporters.HTTP.Webhooks[0].URL = "http://localhost:8080/webhook1" // Create reloadable mock exporter - mockExporter := NewMockReloadableExporter("http-exporter") + mockExporter := NewMockHTTPReloadableExporter("http-exporter") helper.Setup(t, initialConfig) // Replace exporter with reloadable one @@ -735,8 +739,9 @@ func TestConfigReloadPartialFailureRecovery(t *testing.T) { } } -// MockReloadableExporter is a mock exporter that implements ReloadableExporter interface -type MockReloadableExporter struct { +// MockHTTPReloadableExporter is a mock HTTP exporter that implements ReloadableExporter interface. +// The name contains "HTTP" so getExporterType recognizes it as an HTTP exporter. +type MockHTTPReloadableExporter struct { *MockExporter reloadable bool reloadCount int @@ -744,16 +749,16 @@ type MockReloadableExporter struct { mu sync.RWMutex } -// NewMockReloadableExporter creates a new mock reloadable exporter -func NewMockReloadableExporter(name string) *MockReloadableExporter { - return &MockReloadableExporter{ +// NewMockHTTPReloadableExporter creates a new mock reloadable HTTP exporter +func NewMockHTTPReloadableExporter(name string) *MockHTTPReloadableExporter { + return &MockHTTPReloadableExporter{ MockExporter: NewMockExporter(name), reloadable: true, } } // Reload implements ReloadableExporter interface -func (m *MockReloadableExporter) Reload(config interface{}) error { +func (m *MockHTTPReloadableExporter) Reload(config interface{}) error { m.mu.Lock() defer m.mu.Unlock() @@ -763,28 +768,28 @@ func (m *MockReloadableExporter) Reload(config interface{}) error { } // IsReloadable implements ReloadableExporter interface -func (m *MockReloadableExporter) IsReloadable() bool { +func (m *MockHTTPReloadableExporter) IsReloadable() bool { m.mu.RLock() defer m.mu.RUnlock() return m.reloadable } // SetReloadable sets whether this exporter is reloadable -func (m *MockReloadableExporter) SetReloadable(reloadable bool) { +func (m *MockHTTPReloadableExporter) SetReloadable(reloadable bool) { m.mu.Lock() defer m.mu.Unlock() m.reloadable = reloadable } // GetReloadCount returns the number of times Reload was called -func (m *MockReloadableExporter) GetReloadCount() int { +func (m *MockHTTPReloadableExporter) GetReloadCount() int { m.mu.RLock() defer m.mu.RUnlock() return m.reloadCount } // GetLastConfig returns the last config passed to Reload -func (m *MockReloadableExporter) GetLastConfig() interface{} { +func (m *MockHTTPReloadableExporter) GetLastConfig() interface{} { m.mu.RLock() defer m.mu.RUnlock() return m.lastConfig diff --git a/pkg/detector/statistics_test.go b/pkg/detector/statistics_test.go index b800270..a9ec4f6 100644 --- a/pkg/detector/statistics_test.go +++ b/pkg/detector/statistics_test.go @@ -212,3 +212,265 @@ func TestStatistics_Copy(t *testing.T) { t.Errorf("After modifying original, Copy().GetProblemsDeduplicated() = %d, want 42 (unchanged)", got) } } + +// TestStatistics_GetTotalExports tests the GetTotalExports method. +func TestStatistics_GetTotalExports(t *testing.T) { + tests := []struct { + name string + succeeded int + failed int + expected int64 + }{ + { + name: "no exports", + succeeded: 0, + failed: 0, + expected: 0, + }, + { + name: "only succeeded", + succeeded: 10, + failed: 0, + expected: 10, + }, + { + name: "only failed", + succeeded: 0, + failed: 5, + expected: 5, + }, + { + name: "both succeeded and failed", + succeeded: 10, + failed: 3, + expected: 13, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStatistics() + for i := 0; i < tt.succeeded; i++ { + stats.IncrementExportsSucceeded() + } + for i := 0; i < tt.failed; i++ { + stats.IncrementExportsFailed() + } + + got := stats.GetTotalExports() + if got != tt.expected { + t.Errorf("GetTotalExports() = %d, want %d", got, tt.expected) + } + }) + } +} + +// TestStatistics_GetExportSuccessRate tests the GetExportSuccessRate method. +func TestStatistics_GetExportSuccessRate(t *testing.T) { + tests := []struct { + name string + succeeded int + failed int + expected float64 + }{ + { + name: "no exports returns zero", + succeeded: 0, + failed: 0, + expected: 0.0, + }, + { + name: "100% success rate", + succeeded: 10, + failed: 0, + expected: 100.0, + }, + { + name: "0% success rate", + succeeded: 0, + failed: 10, + expected: 0.0, + }, + { + name: "50% success rate", + succeeded: 5, + failed: 5, + expected: 50.0, + }, + { + name: "75% success rate", + succeeded: 75, + failed: 25, + expected: 75.0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStatistics() + for i := 0; i < tt.succeeded; i++ { + stats.IncrementExportsSucceeded() + } + for i := 0; i < tt.failed; i++ { + stats.IncrementExportsFailed() + } + + got := stats.GetExportSuccessRate() + if got != tt.expected { + t.Errorf("GetExportSuccessRate() = %f, want %f", got, tt.expected) + } + }) + } +} + +// TestStatistics_GetDeduplicationRate tests the GetDeduplicationRate method. +func TestStatistics_GetDeduplicationRate(t *testing.T) { + tests := []struct { + name string + detected int + deduplicated int + expected float64 + }{ + { + name: "no problems returns zero", + detected: 0, + deduplicated: 0, + expected: 0.0, + }, + { + name: "100% unique (no duplicates)", + detected: 10, + deduplicated: 10, + expected: 100.0, + }, + { + name: "50% unique", + detected: 10, + deduplicated: 5, + expected: 50.0, + }, + { + name: "all duplicates", + detected: 10, + deduplicated: 0, + expected: 0.0, + }, + { + name: "80% unique", + detected: 100, + deduplicated: 80, + expected: 80.0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stats := NewStatistics() + stats.AddProblemsDetected(tt.detected) + stats.AddProblemsDeduplicated(tt.deduplicated) + + got := stats.GetDeduplicationRate() + if got != tt.expected { + t.Errorf("GetDeduplicationRate() = %f, want %f", got, tt.expected) + } + }) + } +} + +// TestStatistics_Summary tests the Summary method. +func TestStatistics_Summary(t *testing.T) { + stats := NewStatistics() + + // Add various data + stats.IncrementMonitorsStarted() + stats.IncrementMonitorsStarted() + stats.IncrementMonitorsFailed() + stats.IncrementStatusesReceived() + stats.AddProblemsDetected(10) + stats.AddProblemsDeduplicated(8) + stats.IncrementExportsSucceeded() + stats.IncrementExportsSucceeded() + stats.IncrementExportsFailed() + + summary := stats.Summary() + + // Verify summary contains expected keys + expectedKeys := []string{ + "uptime", + "monitors_started", + "monitors_failed", + "statuses_received", + "problems_detected", + "problems_deduplicated", + "exports_succeeded", + "exports_failed", + "total_exports", + "export_success_rate_pct", + "deduplication_rate_pct", + } + + for _, key := range expectedKeys { + if _, ok := summary[key]; !ok { + t.Errorf("Summary() missing key %q", key) + } + } + + // Verify specific values + if summary["monitors_started"] != int64(2) { + t.Errorf("Summary()[monitors_started] = %v, want 2", summary["monitors_started"]) + } + if summary["monitors_failed"] != int64(1) { + t.Errorf("Summary()[monitors_failed] = %v, want 1", summary["monitors_failed"]) + } + if summary["total_exports"] != int64(3) { + t.Errorf("Summary()[total_exports] = %v, want 3", summary["total_exports"]) + } + + // Verify export success rate (2/3 = 66.67%) + rate := summary["export_success_rate_pct"].(float64) + expectedRate := float64(2) / float64(3) * 100.0 + if rate != expectedRate { + t.Errorf("Summary()[export_success_rate_pct] = %v, want %v", rate, expectedRate) + } + + // Verify deduplication rate (8/10 = 80%) + dedupRate := summary["deduplication_rate_pct"].(float64) + if dedupRate != 80.0 { + t.Errorf("Summary()[deduplication_rate_pct] = %v, want 80.0", dedupRate) + } +} + +// TestStatistics_ExportMetrics tests export-related metrics together. +func TestStatistics_ExportMetrics(t *testing.T) { + stats := NewStatistics() + + // Verify initial state + if got := stats.GetExportsSucceeded(); got != 0 { + t.Errorf("Initial GetExportsSucceeded() = %d, want 0", got) + } + if got := stats.GetExportsFailed(); got != 0 { + t.Errorf("Initial GetExportsFailed() = %d, want 0", got) + } + + // Add exports + for i := 0; i < 7; i++ { + stats.IncrementExportsSucceeded() + } + for i := 0; i < 3; i++ { + stats.IncrementExportsFailed() + } + + // Verify all export metrics + if got := stats.GetExportsSucceeded(); got != 7 { + t.Errorf("GetExportsSucceeded() = %d, want 7", got) + } + if got := stats.GetExportsFailed(); got != 3 { + t.Errorf("GetExportsFailed() = %d, want 3", got) + } + if got := stats.GetTotalExports(); got != 10 { + t.Errorf("GetTotalExports() = %d, want 10", got) + } + if got := stats.GetExportSuccessRate(); got != 70.0 { + t.Errorf("GetExportSuccessRate() = %f, want 70.0", got) + } +} diff --git a/pkg/exporters/http/exporter_test.go b/pkg/exporters/http/exporter_test.go index c773c4f..4c2660a 100644 --- a/pkg/exporters/http/exporter_test.go +++ b/pkg/exporters/http/exporter_test.go @@ -785,3 +785,456 @@ func TestHTTPExporterMultipleWebhooks(t *testing.T) { // Good - timeout means no request was received } } + +// TestHTTPExporter_IsReloadable tests the IsReloadable method. +func TestHTTPExporter_IsReloadable(t *testing.T) { + config := &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 5, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: "https://example.com/webhook", + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + } + + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewHTTPExporter(config, settings) + if err != nil { + t.Fatalf("Failed to create HTTP exporter: %v", err) + } + + // HTTP exporter should always be reloadable + if !exporter.IsReloadable() { + t.Error("Expected IsReloadable() to return true") + } +} + +// TestHTTPExporter_Reload tests the Reload method with various scenarios. +func TestHTTPExporter_Reload(t *testing.T) { + // Create test server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(WebhookResponse{Success: true}) + })) + defer server.Close() + + baseConfig := &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 5, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + } + + settings := &types.GlobalSettings{NodeName: "test-node"} + + tests := []struct { + name string + newConfig interface{} + expectError bool + startFirst bool + }{ + { + name: "wrong config type", + newConfig: "invalid-config", + expectError: true, + startFirst: false, + }, + { + name: "nil config", + newConfig: (*types.HTTPExporterConfig)(nil), + expectError: true, + startFirst: false, + }, + { + name: "empty webhooks", + newConfig: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 5, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{}, + }, + expectError: true, + startFirst: false, + }, + { + name: "valid config without worker pool recreation", + newConfig: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, // Same as initial + QueueSize: 5, // Same as initial + Timeout: 30 * time.Second, // Same as initial + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "updated-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + }, + expectError: false, + startFirst: true, + }, + { + name: "valid config with worker count change", + newConfig: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 2, // Changed from 1 to 2 + QueueSize: 5, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + }, + expectError: false, + startFirst: true, + }, + { + name: "valid config with queue size change", + newConfig: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 10, // Changed from 5 to 10 + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + }, + expectError: false, + startFirst: true, + }, + { + name: "valid config with timeout change", + newConfig: &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 5, + Timeout: 60 * time.Second, // Changed from 30s to 60s + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + }, + expectError: false, + startFirst: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create new exporter for each test + exporter, err := NewHTTPExporter(baseConfig, settings) + if err != nil { + t.Fatalf("Failed to create HTTP exporter: %v", err) + } + defer exporter.Stop() + + if tt.startFirst { + ctx := context.Background() + if err := exporter.Start(ctx); err != nil { + t.Fatalf("Failed to start exporter: %v", err) + } + } + + err = exporter.Reload(tt.newConfig) + if (err != nil) != tt.expectError { + t.Errorf("Reload() error = %v, expectError = %v", err, tt.expectError) + } + }) + } +} + +// TestHTTPExporter_needsWorkerPoolRecreation tests the needsWorkerPoolRecreation method. +func TestHTTPExporter_needsWorkerPoolRecreation(t *testing.T) { + config := &types.HTTPExporterConfig{ + Enabled: true, + Workers: 2, + QueueSize: 10, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "test-webhook", + URL: "https://example.com/webhook", + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + } + + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewHTTPExporter(config, settings) + if err != nil { + t.Fatalf("Failed to create HTTP exporter: %v", err) + } + + tests := []struct { + name string + oldConfig *types.HTTPExporterConfig + newConfig *types.HTTPExporterConfig + expected bool + }{ + { + name: "nil old config", + oldConfig: nil, + newConfig: config, + expected: true, + }, + { + name: "same config", + oldConfig: config, + newConfig: config, + expected: false, + }, + { + name: "worker count changed", + oldConfig: config, + newConfig: &types.HTTPExporterConfig{ + Workers: 4, + QueueSize: 10, + Timeout: 30 * time.Second, + }, + expected: true, + }, + { + name: "queue size changed", + oldConfig: config, + newConfig: &types.HTTPExporterConfig{ + Workers: 2, + QueueSize: 20, + Timeout: 30 * time.Second, + }, + expected: true, + }, + { + name: "timeout changed", + oldConfig: config, + newConfig: &types.HTTPExporterConfig{ + Workers: 2, + QueueSize: 10, + Timeout: 60 * time.Second, + }, + expected: true, + }, + { + name: "no relevant changes", + oldConfig: config, + newConfig: &types.HTTPExporterConfig{ + Workers: 2, + QueueSize: 10, + Timeout: 30 * time.Second, + Enabled: false, // This change doesn't affect worker pool + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := exporter.needsWorkerPoolRecreation(tt.oldConfig, tt.newConfig) + if got != tt.expected { + t.Errorf("needsWorkerPoolRecreation() = %v, expected %v", got, tt.expected) + } + }) + } +} + +// TestHTTPExporter_performHealthCheck tests the performHealthCheck method. +func TestHTTPExporter_performHealthCheck(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + json.NewEncoder(w).Encode(WebhookResponse{Success: true}) + })) + defer server.Close() + + config := &types.HTTPExporterConfig{ + Enabled: true, + Workers: 1, + QueueSize: 5, + Timeout: 30 * time.Second, + Retry: types.RetryConfig{ + MaxAttempts: 2, + BaseDelay: 100 * time.Millisecond, + MaxDelay: 1 * time.Second, + }, + Webhooks: []types.WebhookEndpoint{ + { + Name: "healthy-webhook", + URL: server.URL, + Timeout: 30 * time.Second, + Auth: types.AuthConfig{Type: "none"}, + Retry: &types.RetryConfig{ + MaxAttempts: 1, + BaseDelay: 1 * time.Second, + MaxDelay: 30 * time.Second, + }, + SendStatus: true, + }, + }, + } + + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewHTTPExporter(config, settings) + if err != nil { + t.Fatalf("Failed to create HTTP exporter: %v", err) + } + + ctx := context.Background() + if err := exporter.Start(ctx); err != nil { + t.Fatalf("Failed to start exporter: %v", err) + } + defer exporter.Stop() + + // Export a status to create webhook stats + status := &types.Status{ + Source: "test", + Timestamp: time.Now(), + } + _ = exporter.ExportStatus(ctx, status) + + // Wait for export to complete + time.Sleep(200 * time.Millisecond) + + // Perform health check - should not panic and should work with stats + exporter.performHealthCheck() + + // Verify health status + health := exporter.GetHealthStatus() + if health["started"] != true { + t.Error("Expected exporter to be started after health check") + } +} + +// TestHTTPExporter_min tests the min helper function. +func TestHTTPExporter_min(t *testing.T) { + tests := []struct { + a, b, expected int + }{ + {1, 2, 1}, + {2, 1, 1}, + {5, 5, 5}, + {0, 10, 0}, + {-1, 1, -1}, + {-5, -2, -5}, + } + + for _, tt := range tests { + got := min(tt.a, tt.b) + if got != tt.expected { + t.Errorf("min(%d, %d) = %d, expected %d", tt.a, tt.b, got, tt.expected) + } + } +} diff --git a/pkg/exporters/http/stats_test.go b/pkg/exporters/http/stats_test.go new file mode 100644 index 0000000..c12a1ea --- /dev/null +++ b/pkg/exporters/http/stats_test.go @@ -0,0 +1,526 @@ +package http + +import ( + "errors" + "testing" + "time" +) + +// TestStats_RecordDroppedRequest tests the RecordDroppedRequest method. +func TestStats_RecordDroppedRequest(t *testing.T) { + stats := NewStats() + + // Initially, dropped count should be 0 + snapshot := stats.GetSnapshot() + if snapshot.RequestsDropped != 0 { + t.Errorf("Initial RequestsDropped = %d, want 0", snapshot.RequestsDropped) + } + + // Record dropped requests + stats.RecordDroppedRequest() + stats.RecordDroppedRequest() + stats.RecordDroppedRequest() + + snapshot = stats.GetSnapshot() + if snapshot.RequestsDropped != 3 { + t.Errorf("After 3 drops, RequestsDropped = %d, want 3", snapshot.RequestsDropped) + } +} + +// TestStats_RecordRetryAttempt tests the RecordRetryAttempt method. +func TestStats_RecordRetryAttempt(t *testing.T) { + stats := NewStats() + + // Initialize webhook stats + stats.GetWebhookStats("test-webhook") + stats.GetWebhookStats("other-webhook") + + // Record retry attempts + stats.RecordRetryAttempt("test-webhook") + stats.RecordRetryAttempt("test-webhook") + stats.RecordRetryAttempt("other-webhook") + + // Verify retry counts + snapshot := stats.GetSnapshot() + + testWebhook, ok := snapshot.WebhookStats["test-webhook"] + if !ok { + t.Fatal("Expected test-webhook stats to exist") + } + if testWebhook.RetryAttempts != 2 { + t.Errorf("test-webhook RetryAttempts = %d, want 2", testWebhook.RetryAttempts) + } + + otherWebhook, ok := snapshot.WebhookStats["other-webhook"] + if !ok { + t.Fatal("Expected other-webhook stats to exist") + } + if otherWebhook.RetryAttempts != 1 { + t.Errorf("other-webhook RetryAttempts = %d, want 1", otherWebhook.RetryAttempts) + } + + // Record retry for non-existent webhook (should not panic) + stats.RecordRetryAttempt("non-existent") +} + +// TestStatsSnapshot_GetTotalFailures tests the GetTotalFailures method. +func TestStatsSnapshot_GetTotalFailures(t *testing.T) { + tests := []struct { + name string + statusFailed int64 + problemFailed int64 + expected int64 + }{ + { + name: "no failures", + statusFailed: 0, + problemFailed: 0, + expected: 0, + }, + { + name: "only status failures", + statusFailed: 5, + problemFailed: 0, + expected: 5, + }, + { + name: "only problem failures", + statusFailed: 0, + problemFailed: 3, + expected: 3, + }, + { + name: "both failures", + statusFailed: 7, + problemFailed: 4, + expected: 11, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshot := StatsSnapshot{ + StatusExportsFailed: tt.statusFailed, + ProblemExportsFailed: tt.problemFailed, + } + + got := snapshot.GetTotalFailures() + if got != tt.expected { + t.Errorf("GetTotalFailures() = %d, want %d", got, tt.expected) + } + }) + } +} + +// TestStatsSnapshot_GetStatusSuccessRate tests the GetStatusSuccessRate method. +func TestStatsSnapshot_GetStatusSuccessRate(t *testing.T) { + tests := []struct { + name string + total int64 + success int64 + expected float64 + }{ + { + name: "no exports", + total: 0, + success: 0, + expected: 0.0, + }, + { + name: "100% success", + total: 10, + success: 10, + expected: 100.0, + }, + { + name: "50% success", + total: 10, + success: 5, + expected: 50.0, + }, + { + name: "0% success", + total: 10, + success: 0, + expected: 0.0, + }, + { + name: "75% success", + total: 100, + success: 75, + expected: 75.0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshot := StatsSnapshot{ + StatusExportsTotal: tt.total, + StatusExportsSuccess: tt.success, + } + + got := snapshot.GetStatusSuccessRate() + if got != tt.expected { + t.Errorf("GetStatusSuccessRate() = %f, want %f", got, tt.expected) + } + }) + } +} + +// TestStatsSnapshot_GetProblemSuccessRate tests the GetProblemSuccessRate method. +func TestStatsSnapshot_GetProblemSuccessRate(t *testing.T) { + tests := []struct { + name string + total int64 + success int64 + expected float64 + }{ + { + name: "no exports", + total: 0, + success: 0, + expected: 0.0, + }, + { + name: "100% success", + total: 20, + success: 20, + expected: 100.0, + }, + { + name: "60% success", + total: 10, + success: 6, + expected: 60.0, + }, + { + name: "0% success", + total: 5, + success: 0, + expected: 0.0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshot := StatsSnapshot{ + ProblemExportsTotal: tt.total, + ProblemExportsSuccess: tt.success, + } + + got := snapshot.GetProblemSuccessRate() + if got != tt.expected { + t.Errorf("GetProblemSuccessRate() = %f, want %f", got, tt.expected) + } + }) + } +} + +// TestWebhookStatsSnapshot_GetSuccessRate tests the WebhookStatsSnapshot GetSuccessRate method. +func TestWebhookStatsSnapshot_GetSuccessRate(t *testing.T) { + tests := []struct { + name string + total int64 + success int64 + expected float64 + }{ + { + name: "no requests", + total: 0, + success: 0, + expected: 0.0, + }, + { + name: "100% success", + total: 10, + success: 10, + expected: 100.0, + }, + { + name: "80% success", + total: 100, + success: 80, + expected: 80.0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snapshot := WebhookStatsSnapshot{ + RequestsTotal: tt.total, + RequestsSuccess: tt.success, + } + + got := snapshot.GetSuccessRate() + if got != tt.expected { + t.Errorf("GetSuccessRate() = %f, want %f", got, tt.expected) + } + }) + } +} + +// TestWebhookStatsSnapshot_IsHealthy tests the WebhookStatsSnapshot IsHealthy method. +func TestWebhookStatsSnapshot_IsHealthy(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + snapshot WebhookStatsSnapshot + expected bool + }{ + { + name: "no requests - healthy by default", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 0, + RequestsSuccess: 0, + }, + expected: true, + }, + { + name: "high success rate with recent success", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 95, + LastSuccessTime: now, + }, + expected: true, + }, + { + name: "exactly 90% success rate with recent success", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 90, + LastSuccessTime: now, + }, + expected: true, + }, + { + name: "success rate below 90%", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 89, + LastSuccessTime: now, + }, + expected: false, + }, + { + name: "high success rate but no recent success", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 95, + LastSuccessTime: now.Add(-10 * time.Minute), // More than 5 minutes ago + }, + expected: false, + }, + { + name: "high success rate with success just under 5 minutes ago", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 95, + LastSuccessTime: now.Add(-4*time.Minute - 59*time.Second), + }, + expected: true, + }, + { + name: "low success rate with no recent success", + snapshot: WebhookStatsSnapshot{ + RequestsTotal: 100, + RequestsSuccess: 50, + LastSuccessTime: now.Add(-10 * time.Minute), + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.snapshot.IsHealthy() + if got != tt.expected { + t.Errorf("IsHealthy() = %v, want %v", got, tt.expected) + } + }) + } +} + +// TestWebhookStats_recordRequest tests the recordRequest method. +func TestWebhookStats_recordRequest(t *testing.T) { + tests := []struct { + name string + success bool + responseTime time.Duration + err error + }{ + { + name: "successful request with response time", + success: true, + responseTime: 100 * time.Millisecond, + err: nil, + }, + { + name: "failed request with error", + success: false, + responseTime: 50 * time.Millisecond, + err: errors.New("connection refused"), + }, + { + name: "successful request with zero response time", + success: true, + responseTime: 0, + err: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ws := NewWebhookStats("test-webhook") + + // Record the request + ws.recordRequest(tt.success, tt.responseTime, tt.err) + + snapshot := ws.getSnapshot() + + // Verify request was counted + if snapshot.RequestsTotal != 1 { + t.Errorf("RequestsTotal = %d, want 1", snapshot.RequestsTotal) + } + + if tt.success { + if snapshot.RequestsSuccess != 1 { + t.Errorf("RequestsSuccess = %d, want 1", snapshot.RequestsSuccess) + } + if snapshot.RequestsFailed != 0 { + t.Errorf("RequestsFailed = %d, want 0", snapshot.RequestsFailed) + } + } else { + if snapshot.RequestsFailed != 1 { + t.Errorf("RequestsFailed = %d, want 1", snapshot.RequestsFailed) + } + if snapshot.RequestsSuccess != 0 { + t.Errorf("RequestsSuccess = %d, want 0", snapshot.RequestsSuccess) + } + if snapshot.LastError == nil { + t.Error("Expected LastError to be set") + } + } + }) + } +} + +// TestWebhookStats_recordRequest_ResponseTimeAverage tests response time averaging. +func TestWebhookStats_recordRequest_ResponseTimeAverage(t *testing.T) { + ws := NewWebhookStats("test-webhook") + + // Record multiple requests with different response times + ws.recordRequest(true, 100*time.Millisecond, nil) + ws.recordRequest(true, 200*time.Millisecond, nil) + ws.recordRequest(true, 300*time.Millisecond, nil) + + snapshot := ws.getSnapshot() + + // Average should be (100+200+300)/3 = 200ms + expectedAvg := 200 * time.Millisecond + if snapshot.AvgResponseTime != expectedAvg { + t.Errorf("AvgResponseTime = %v, want %v", snapshot.AvgResponseTime, expectedAvg) + } +} + +// TestStats_RecordStatusExport tests the RecordStatusExport method with failures. +func TestStats_RecordStatusExport_Failures(t *testing.T) { + stats := NewStats() + stats.GetWebhookStats("test-webhook") + + // Record a failed status export + stats.RecordStatusExport(false, "test-webhook", 50*time.Millisecond, errors.New("timeout")) + + snapshot := stats.GetSnapshot() + + if snapshot.StatusExportsTotal != 1 { + t.Errorf("StatusExportsTotal = %d, want 1", snapshot.StatusExportsTotal) + } + if snapshot.StatusExportsFailed != 1 { + t.Errorf("StatusExportsFailed = %d, want 1", snapshot.StatusExportsFailed) + } + if snapshot.StatusExportsSuccess != 0 { + t.Errorf("StatusExportsSuccess = %d, want 0", snapshot.StatusExportsSuccess) + } + if snapshot.LastError == nil { + t.Error("Expected LastError to be set") + } +} + +// TestStats_RecordProblemExport tests the RecordProblemExport method with failures. +func TestStats_RecordProblemExport_Failures(t *testing.T) { + stats := NewStats() + stats.GetWebhookStats("test-webhook") + + // Record a failed problem export + stats.RecordProblemExport(false, "test-webhook", 50*time.Millisecond, errors.New("connection refused")) + + snapshot := stats.GetSnapshot() + + if snapshot.ProblemExportsTotal != 1 { + t.Errorf("ProblemExportsTotal = %d, want 1", snapshot.ProblemExportsTotal) + } + if snapshot.ProblemExportsFailed != 1 { + t.Errorf("ProblemExportsFailed = %d, want 1", snapshot.ProblemExportsFailed) + } + if snapshot.ProblemExportsSuccess != 0 { + t.Errorf("ProblemExportsSuccess = %d, want 0", snapshot.ProblemExportsSuccess) + } +} + +// TestStats_ConcurrentAccess tests thread-safety of Stats methods. +func TestStats_ConcurrentAccess(t *testing.T) { + stats := NewStats() + stats.GetWebhookStats("webhook-1") + stats.GetWebhookStats("webhook-2") + + done := make(chan bool) + const iterations = 100 + + // Writer goroutine for queued requests + go func() { + for i := 0; i < iterations; i++ { + stats.RecordQueuedRequest() + } + done <- true + }() + + // Writer goroutine for dropped requests + go func() { + for i := 0; i < iterations; i++ { + stats.RecordDroppedRequest() + } + done <- true + }() + + // Writer goroutine for retries + go func() { + for i := 0; i < iterations; i++ { + stats.RecordRetryAttempt("webhook-1") + } + done <- true + }() + + // Reader goroutine + go func() { + for i := 0; i < iterations; i++ { + _ = stats.GetSnapshot() + } + done <- true + }() + + // Wait for all goroutines + for i := 0; i < 4; i++ { + <-done + } + + // Verify final state + snapshot := stats.GetSnapshot() + if snapshot.RequestsQueued != iterations { + t.Errorf("RequestsQueued = %d, want %d", snapshot.RequestsQueued, iterations) + } + if snapshot.RequestsDropped != iterations { + t.Errorf("RequestsDropped = %d, want %d", snapshot.RequestsDropped, iterations) + } +} diff --git a/pkg/exporters/kubernetes/client_test.go b/pkg/exporters/kubernetes/client_test.go index fad8e8c..069862f 100644 --- a/pkg/exporters/kubernetes/client_test.go +++ b/pkg/exporters/kubernetes/client_test.go @@ -2,6 +2,7 @@ package kubernetes import ( "context" + "strings" "testing" "time" @@ -452,3 +453,278 @@ func TestJsonMarshal(t *testing.T) { }) } } + +// TestConditionUpdateString tests the String method of ConditionUpdate +func TestConditionUpdateString(t *testing.T) { + tests := []struct { + name string + update ConditionUpdate + expected string + }{ + { + name: "true condition", + update: ConditionUpdate{ + Type: "TestCondition", + Status: corev1.ConditionTrue, + Reason: "TestReason", + Message: "Test message", + }, + expected: "TestCondition=True: Test message", + }, + { + name: "false condition", + update: ConditionUpdate{ + Type: "FailedCondition", + Status: corev1.ConditionFalse, + Reason: "Failed", + Message: "Something failed", + }, + expected: "FailedCondition=False: Something failed", + }, + { + name: "unknown condition", + update: ConditionUpdate{ + Type: "UnknownCondition", + Status: corev1.ConditionUnknown, + Reason: "Unknown", + Message: "Status is unknown", + }, + expected: "UnknownCondition=Unknown: Status is unknown", + }, + { + name: "empty message", + update: ConditionUpdate{ + Type: "EmptyMessage", + Status: corev1.ConditionTrue, + Reason: "EmptyReason", + Message: "", + }, + expected: "EmptyMessage=True: ", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.update.String() + if got != tt.expected { + t.Errorf("ConditionUpdate.String() = %q, want %q", got, tt.expected) + } + }) + } +} + +// TestConvertConditionsToNodeConditions tests the ConvertConditionsToNodeConditions function +func TestConvertConditionsToNodeConditions(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + conditions []types.Condition + wantCount int + }{ + { + name: "empty conditions", + conditions: []types.Condition{}, + wantCount: 0, + }, + { + name: "nil conditions", + conditions: nil, + wantCount: 0, + }, + { + name: "single condition", + conditions: []types.Condition{ + { + Type: "TestCondition", + Status: types.ConditionTrue, + Transition: now, + Reason: "TestReason", + Message: "Test message", + }, + }, + wantCount: 1, + }, + { + name: "multiple conditions", + conditions: []types.Condition{ + { + Type: "Condition1", + Status: types.ConditionTrue, + Transition: now, + Reason: "Reason1", + Message: "Message 1", + }, + { + Type: "Condition2", + Status: types.ConditionFalse, + Transition: now, + Reason: "Reason2", + Message: "Message 2", + }, + { + Type: "Condition3", + Status: types.ConditionUnknown, + Transition: now, + Reason: "Reason3", + Message: "Message 3", + }, + }, + wantCount: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ConvertConditionsToNodeConditions(tt.conditions) + if len(result) != tt.wantCount { + t.Errorf("ConvertConditionsToNodeConditions() returned %d conditions, want %d", len(result), tt.wantCount) + } + + // Verify the conversion was correct for non-empty results + for i, cond := range result { + if tt.wantCount > 0 { + // Check that the condition type was set + if string(cond.Type) == "" { + t.Errorf("Condition %d has empty type", i) + } + // Check that status was mapped correctly + if cond.Status == "" { + t.Errorf("Condition %d has empty status", i) + } + } + } + }) + } +} + +// TestConvertConditionsToNodeConditionsStatusMapping tests status mapping in ConvertConditionsToNodeConditions +func TestConvertConditionsToNodeConditionsStatusMapping(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + inputStatus types.ConditionStatus + expectStatus corev1.ConditionStatus + }{ + { + name: "True status", + inputStatus: types.ConditionTrue, + expectStatus: corev1.ConditionTrue, + }, + { + name: "False status", + inputStatus: types.ConditionFalse, + expectStatus: corev1.ConditionFalse, + }, + { + name: "Unknown status", + inputStatus: types.ConditionUnknown, + expectStatus: corev1.ConditionUnknown, + }, + { + name: "Invalid status defaults to Unknown", + inputStatus: types.ConditionStatus("Invalid"), + expectStatus: corev1.ConditionUnknown, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conditions := []types.Condition{ + { + Type: "TestCondition", + Status: tt.inputStatus, + Transition: now, + Reason: "TestReason", + Message: "Test message", + }, + } + + result := ConvertConditionsToNodeConditions(conditions) + if len(result) != 1 { + t.Fatalf("Expected 1 condition, got %d", len(result)) + } + + if result[0].Status != tt.expectStatus { + t.Errorf("ConvertConditionsToNodeConditions() status = %v, want %v", result[0].Status, tt.expectStatus) + } + }) + } +} + +// TestConvertConditionsToNodeConditionsTypePrefixing tests that non-standard conditions get prefixed +func TestConvertConditionsToNodeConditionsTypePrefixing(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + inputType string + shouldBePrefixed bool + }{ + { + name: "Ready is not prefixed (standard)", + inputType: "Ready", + shouldBePrefixed: false, + }, + { + name: "MemoryPressure is not prefixed (standard)", + inputType: "MemoryPressure", + shouldBePrefixed: false, + }, + { + name: "DiskPressure is not prefixed (standard)", + inputType: "DiskPressure", + shouldBePrefixed: false, + }, + { + name: "PIDPressure is not prefixed (standard)", + inputType: "PIDPressure", + shouldBePrefixed: false, + }, + { + name: "NetworkUnavailable is not prefixed (standard)", + inputType: "NetworkUnavailable", + shouldBePrefixed: false, + }, + { + name: "CustomCondition is prefixed", + inputType: "CustomCondition", + shouldBePrefixed: true, + }, + { + name: "ServiceFailed is prefixed", + inputType: "ServiceFailed", + shouldBePrefixed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + conditions := []types.Condition{ + { + Type: tt.inputType, + Status: types.ConditionTrue, + Transition: now, + Reason: "TestReason", + Message: "Test message", + }, + } + + result := ConvertConditionsToNodeConditions(conditions) + if len(result) != 1 { + t.Fatalf("Expected 1 condition, got %d", len(result)) + } + + condType := string(result[0].Type) + hasPrefix := strings.HasPrefix(condType, "NodeDoctor") + + if tt.shouldBePrefixed && !hasPrefix { + t.Errorf("Expected condition type %q to be prefixed with NodeDoctor, got %q", tt.inputType, condType) + } + if !tt.shouldBePrefixed && hasPrefix { + t.Errorf("Expected condition type %q to NOT be prefixed, got %q", tt.inputType, condType) + } + }) + } +} diff --git a/pkg/exporters/kubernetes/exporter_test.go b/pkg/exporters/kubernetes/exporter_test.go index cc0810a..a495f82 100644 --- a/pkg/exporters/kubernetes/exporter_test.go +++ b/pkg/exporters/kubernetes/exporter_test.go @@ -2,6 +2,7 @@ package kubernetes import ( "context" + "fmt" "testing" "time" @@ -604,3 +605,479 @@ func TestExporterBackgroundLoops(t *testing.T) { t.Error("Uptime should be set if background loops are running") } } + +// TestIsReloadable tests that the exporter is reloadable +func TestIsReloadable(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + if !exporter.IsReloadable() { + t.Error("KubernetesExporter.IsReloadable() should return true") + } +} + +// TestReload tests the Reload functionality +func TestReload(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the exporter + exporter.Start(ctx) + defer exporter.Stop() + + tests := []struct { + name string + config interface{} + expectError bool + }{ + { + name: "nil config", + config: nil, + expectError: true, + }, + { + name: "wrong type config", + config: "invalid config type", + expectError: true, + }, + { + name: "valid config without client recreation", + config: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, // Same as initial + ResyncInterval: 200 * time.Millisecond, // Same as initial + HeartbeatInterval: 300 * time.Millisecond, // Same as initial + Namespace: "test-namespace", // Same as initial - no client recreation + Conditions: []types.ConditionConfig{}, + Annotations: []types.AnnotationConfig{ + {Key: "test-key", Value: "test-value"}, // Different annotations don't require client recreation + }, + Events: types.EventConfig{ + MaxEventsPerMinute: 10, // Same as initial + DeduplicationWindow: time.Minute, // Same as initial + }, + }, + expectError: false, + }, + { + name: "valid config with namespace change (requires client recreation - expected to fail outside cluster)", + config: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 200 * time.Millisecond, + ResyncInterval: 400 * time.Millisecond, + HeartbeatInterval: 600 * time.Millisecond, + Namespace: "new-namespace", // Different namespace triggers client recreation + Conditions: []types.ConditionConfig{}, + Annotations: []types.AnnotationConfig{}, + Events: types.EventConfig{ + MaxEventsPerMinute: 20, + DeduplicationWindow: 2 * time.Minute, + }, + }, + expectError: true, // Client recreation fails outside of cluster + }, + { + name: "invalid config with negative interval", + config: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: -1 * time.Second, + ResyncInterval: time.Minute, + HeartbeatInterval: time.Minute, + Namespace: "test-namespace", + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exporter.Reload(tt.config) + if (err != nil) != tt.expectError { + t.Errorf("Reload() error = %v, expectError %v", err, tt.expectError) + } + }) + } +} + +// TestNeedsClientRecreation tests the needsClientRecreation method +func TestNeedsClientRecreation(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + baseConfig := &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + } + + tests := []struct { + name string + newConfig *types.KubernetesExporterConfig + expected bool + }{ + { + name: "namespace changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "different-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "update interval changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 500 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "resync interval changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 500 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "heartbeat interval changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 500 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "max events per minute changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 50, + DeduplicationWindow: time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "event TTL changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + EventTTL: 2 * time.Hour, + }, + }, + expected: true, + }, + { + name: "deduplication window changed", + newConfig: &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "test-namespace", + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: 5 * time.Minute, + EventTTL: time.Hour, + }, + }, + expected: true, + }, + { + name: "no changes", + newConfig: baseConfig, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reset config for each test + exporter.config = baseConfig + got := exporter.needsClientRecreation(tt.newConfig) + if got != tt.expected { + t.Errorf("needsClientRecreation() = %v, want %v", got, tt.expected) + } + }) + } +} + +// TestNeedsClientRecreationWithNilConfig tests needsClientRecreation with nil current config +func TestNeedsClientRecreationWithNilConfig(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + // Set config to nil + exporter.config = nil + + newConfig := &types.KubernetesExporterConfig{ + Enabled: true, + Namespace: "test-namespace", + } + + // With nil config, should return true (first time configuration) + if !exporter.needsClientRecreation(newConfig) { + t.Error("needsClientRecreation() should return true when current config is nil") + } +} + +// TestAnnotationsEqual tests the annotationsEqual method +func TestAnnotationsEqual(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + tests := []struct { + name string + old []types.AnnotationConfig + new []types.AnnotationConfig + expected bool + }{ + { + name: "both empty", + old: []types.AnnotationConfig{}, + new: []types.AnnotationConfig{}, + expected: true, + }, + { + name: "both nil", + old: nil, + new: nil, + expected: true, + }, + { + name: "equal annotations", + old: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + {Key: "key2", Value: "value2"}, + }, + new: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + {Key: "key2", Value: "value2"}, + }, + expected: true, + }, + { + name: "different lengths", + old: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + }, + new: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + {Key: "key2", Value: "value2"}, + }, + expected: false, + }, + { + name: "different values", + old: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + }, + new: []types.AnnotationConfig{ + {Key: "key1", Value: "different-value"}, + }, + expected: false, + }, + { + name: "different keys", + old: []types.AnnotationConfig{ + {Key: "key1", Value: "value1"}, + }, + new: []types.AnnotationConfig{ + {Key: "different-key", Value: "value1"}, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := exporter.annotationsEqual(tt.old, tt.new) + if got != tt.expected { + t.Errorf("annotationsEqual() = %v, want %v", got, tt.expected) + } + }) + } +} + +// TestPerformHealthCheck tests the performHealthCheck method +func TestPerformHealthCheck(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the exporter + exporter.Start(ctx) + defer exporter.Stop() + + // Give the exporter time to initialize + time.Sleep(100 * time.Millisecond) + + // Test performHealthCheck - should not panic + exporter.performHealthCheck(ctx) + + // Verify condition manager has the health condition + stats := exporter.conditionManager.GetStats() + if stats == nil { + t.Error("Condition manager stats should not be nil after health check") + } +} + +// TestLogMetrics tests the logMetrics method +func TestLogMetrics(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the exporter + exporter.Start(ctx) + defer exporter.Stop() + + // Test logMetrics - should not panic + exporter.logMetrics() + + // Verify stats are still accessible + stats := exporter.GetStats() + if stats == nil { + t.Error("GetStats() should not return nil after logMetrics") + } +} + +// TestStatsWithLastError tests stats output when there's a last error +func TestStatsWithLastError(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the exporter + exporter.Start(ctx) + defer exporter.Stop() + + // Set a last error + exporter.updateStats(func(s *ExporterStats) { + s.LastError = fmt.Errorf("test error") + s.LastErrorTime = time.Now() + }) + + stats := exporter.GetStats() + + // Verify last error is included in stats + if _, exists := stats["last_error"]; !exists { + t.Error("GetStats() should include last_error when there's a last error") + } + if _, exists := stats["last_error_time"]; !exists { + t.Error("GetStats() should include last_error_time when there's a last error") + } +} + +// TestReloadWithClientRecreation tests Reload when client recreation is needed +func TestReloadWithClientRecreation(t *testing.T) { + exporter, err := createTestKubernetesExporter() + if err != nil { + t.Fatalf("Failed to create test exporter: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Start the exporter + exporter.Start(ctx) + defer exporter.Stop() + + // Reload with a new namespace (triggers client recreation) + newConfig := &types.KubernetesExporterConfig{ + Enabled: true, + UpdateInterval: 100 * time.Millisecond, + ResyncInterval: 200 * time.Millisecond, + HeartbeatInterval: 300 * time.Millisecond, + Namespace: "new-namespace-recreation", + Conditions: []types.ConditionConfig{}, + Annotations: []types.AnnotationConfig{}, + Events: types.EventConfig{ + MaxEventsPerMinute: 10, + DeduplicationWindow: time.Minute, + }, + } + + // This will fail because we can't create a real K8s client, but we're testing the code path + err = exporter.Reload(newConfig) + // We expect an error because NewClient will fail without real K8s + if err == nil { + t.Log("Reload succeeded (may be running in a real K8s environment)") + } +} diff --git a/pkg/exporters/prometheus/exporter_test.go b/pkg/exporters/prometheus/exporter_test.go index b509d3c..058d3b9 100644 --- a/pkg/exporters/prometheus/exporter_test.go +++ b/pkg/exporters/prometheus/exporter_test.go @@ -3,6 +3,7 @@ package prometheus import ( "context" "fmt" + "net" "net/http" "testing" "time" @@ -10,6 +11,28 @@ import ( "github.com/supporttools/node-doctor/pkg/types" ) +// waitForServerReady polls until the server is accepting connections +func waitForServerReady(addr string, timeout time.Duration) error { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + ticker := time.NewTicker(50 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + return ctx.Err() + case <-ticker.C: + conn, err := net.DialTimeout("tcp", addr, 100*time.Millisecond) + if err == nil { + conn.Close() + return nil + } + } + } +} + func TestNewPrometheusExporter(t *testing.T) { tests := []struct { name string @@ -399,3 +422,731 @@ func contains(s, substr string) bool { return false }() } + +func TestIsReloadable(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9106, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + // Test IsReloadable returns true + if !exporter.IsReloadable() { + t.Errorf("expected IsReloadable to return true") + } +} + +func TestReload(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9107, + Path: "/metrics", + Namespace: "test", + Subsystem: "sub1", + Labels: map[string]string{"env": "test"}, + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + // Use t.Cleanup for reliable resource cleanup + t.Cleanup(func() { + if exporter != nil { + exporter.Stop() + } + }) + + ctx := context.Background() + err = exporter.Start(ctx) + if err != nil { + t.Fatalf("failed to start exporter: %v", err) + } + + tests := []struct { + name string + newConfig interface{} + expectedError bool + errorContains string + }{ + { + name: "invalid config type", + newConfig: "invalid", + expectedError: true, + errorContains: "invalid config type", + }, + { + name: "nil config", + newConfig: (*types.PrometheusExporterConfig)(nil), + expectedError: true, + errorContains: "cannot be nil", + }, + { + name: "invalid port in new config", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 70000, + }, + expectedError: true, + errorContains: "invalid port", + }, + { + name: "same config - no restart needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9107, + Path: "/metrics", + Namespace: "test", + Subsystem: "sub1", + Labels: map[string]string{"env": "test"}, + }, + expectedError: false, + }, + { + name: "different port - restart needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9108, + Path: "/metrics", + Namespace: "test", + Subsystem: "sub1", + }, + expectedError: false, + }, + { + name: "different path - restart needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9108, + Path: "/custom-metrics", + Namespace: "test", + }, + expectedError: false, + }, + { + name: "different namespace - metrics recreation needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9108, + Path: "/custom-metrics", + Namespace: "new_namespace", + }, + expectedError: false, + }, + { + name: "different subsystem - metrics recreation needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9108, + Path: "/custom-metrics", + Namespace: "new_namespace", + Subsystem: "new_subsystem", + }, + expectedError: false, + }, + { + name: "different labels - metrics recreation needed", + newConfig: &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9108, + Path: "/custom-metrics", + Namespace: "new_namespace", + Labels: map[string]string{"env": "prod", "region": "us-east"}, + }, + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := exporter.Reload(tt.newConfig) + + if tt.expectedError { + if err == nil { + t.Errorf("expected error but got none") + } else if tt.errorContains != "" && !contains(err.Error(), tt.errorContains) { + t.Errorf("expected error to contain '%s', got: %v", tt.errorContains, err) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +func TestReloadNotStarted(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9109, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + // Reload without starting - should work but not restart server + newConfig := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9110, + Path: "/new-metrics", + Namespace: "new_test", + } + + err = exporter.Reload(newConfig) + if err != nil { + t.Errorf("unexpected error reloading not-started exporter: %v", err) + } +} + +func TestNeedsServerRestart(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9111, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + tests := []struct { + name string + oldConfig *types.PrometheusExporterConfig + newConfig *types.PrometheusExporterConfig + needRestart bool + }{ + { + name: "nil old config", + oldConfig: nil, + newConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + }, + needRestart: true, + }, + { + name: "same config", + oldConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "test", + Subsystem: "sub", + }, + newConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "test", + Subsystem: "sub", + }, + needRestart: false, + }, + { + name: "different port", + oldConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + }, + newConfig: &types.PrometheusExporterConfig{ + Port: 9200, + Path: "/metrics", + }, + needRestart: true, + }, + { + name: "different path", + oldConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + }, + newConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/custom", + }, + needRestart: true, + }, + { + name: "different namespace triggers restart via metrics recreation", + oldConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "old", + }, + newConfig: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "new", + }, + needRestart: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := exporter.needsServerRestart(tt.oldConfig, tt.newConfig) + if result != tt.needRestart { + t.Errorf("expected needsServerRestart=%v, got %v", tt.needRestart, result) + } + }) + } +} + +func TestNeedsMetricsRecreation(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9112, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + tests := []struct { + name string + oldConfig *types.PrometheusExporterConfig + newConfig *types.PrometheusExporterConfig + needRecreate bool + }{ + { + name: "nil old config", + oldConfig: nil, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + }, + needRecreate: true, + }, + { + name: "same namespace and subsystem", + oldConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Subsystem: "sub", + Labels: map[string]string{"env": "test"}, + }, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Subsystem: "sub", + Labels: map[string]string{"env": "test"}, + }, + needRecreate: false, + }, + { + name: "different namespace", + oldConfig: &types.PrometheusExporterConfig{ + Namespace: "old", + }, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "new", + }, + needRecreate: true, + }, + { + name: "different subsystem", + oldConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Subsystem: "old", + }, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Subsystem: "new", + }, + needRecreate: true, + }, + { + name: "different labels", + oldConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Labels: map[string]string{"env": "dev"}, + }, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Labels: map[string]string{"env": "prod"}, + }, + needRecreate: true, + }, + { + name: "different label count", + oldConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Labels: map[string]string{"env": "dev"}, + }, + newConfig: &types.PrometheusExporterConfig{ + Namespace: "test", + Labels: map[string]string{"env": "dev", "region": "us"}, + }, + needRecreate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := exporter.needsMetricsRecreation(tt.oldConfig, tt.newConfig) + if result != tt.needRecreate { + t.Errorf("expected needsMetricsRecreation=%v, got %v", tt.needRecreate, result) + } + }) + } +} + +func TestLabelsEqual(t *testing.T) { + tests := []struct { + name string + old map[string]string + new map[string]string + expect bool + }{ + { + name: "both nil", + old: nil, + new: nil, + expect: true, + }, + { + name: "both empty", + old: map[string]string{}, + new: map[string]string{}, + expect: true, + }, + { + name: "same labels", + old: map[string]string{"env": "test", "region": "us"}, + new: map[string]string{"env": "test", "region": "us"}, + expect: true, + }, + { + name: "different values", + old: map[string]string{"env": "test"}, + new: map[string]string{"env": "prod"}, + expect: false, + }, + { + name: "different keys", + old: map[string]string{"env": "test"}, + new: map[string]string{"region": "test"}, + expect: false, + }, + { + name: "different lengths - old longer", + old: map[string]string{"env": "test", "region": "us"}, + new: map[string]string{"env": "test"}, + expect: false, + }, + { + name: "different lengths - new longer", + old: map[string]string{"env": "test"}, + new: map[string]string{"env": "test", "region": "us"}, + expect: false, + }, + { + name: "one nil one empty", + old: nil, + new: map[string]string{}, + expect: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := labelsEqual(tt.old, tt.new) + if result != tt.expect { + t.Errorf("expected labelsEqual=%v, got %v", tt.expect, result) + } + }) + } +} + +func TestIsValidMetricName(t *testing.T) { + tests := []struct { + name string + input string + expect bool + }{ + { + name: "empty string", + input: "", + expect: false, + }, + { + name: "valid lowercase", + input: "node_doctor", + expect: true, + }, + { + name: "valid uppercase", + input: "NODE_DOCTOR", + expect: true, + }, + { + name: "valid mixed case", + input: "Node_Doctor", + expect: true, + }, + { + name: "valid with colon", + input: "node:doctor", + expect: true, + }, + { + name: "valid with numbers", + input: "node_doctor_v2", + expect: true, + }, + { + name: "starts with underscore", + input: "_node_doctor", + expect: true, + }, + { + name: "starts with colon", + input: ":node_doctor", + expect: true, + }, + { + name: "starts with number - invalid", + input: "1node_doctor", + expect: false, + }, + { + name: "contains hyphen - invalid", + input: "node-doctor", + expect: false, + }, + { + name: "contains space - invalid", + input: "node doctor", + expect: false, + }, + { + name: "contains special char - invalid", + input: "node@doctor", + expect: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isValidMetricName(tt.input) + if result != tt.expect { + t.Errorf("isValidMetricName(%q) = %v, want %v", tt.input, result, tt.expect) + } + }) + } +} + +func TestValidateConfig(t *testing.T) { + tests := []struct { + name string + config *types.PrometheusExporterConfig + expectedError bool + errorContains string + }{ + { + name: "valid config", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "node_doctor", + Subsystem: "exporter", + }, + expectedError: false, + }, + { + name: "port zero - valid default", + config: &types.PrometheusExporterConfig{ + Port: 0, + }, + expectedError: false, + }, + { + name: "negative port - invalid", + config: &types.PrometheusExporterConfig{ + Port: -1, + }, + expectedError: true, + errorContains: "invalid port", + }, + { + name: "port too high - invalid", + config: &types.PrometheusExporterConfig{ + Port: 65536, + }, + expectedError: true, + errorContains: "invalid port", + }, + { + name: "path without leading slash - invalid", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "metrics", + }, + expectedError: true, + errorContains: "path must start with", + }, + { + name: "invalid namespace", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "node-doctor", // hyphen invalid + }, + expectedError: true, + errorContains: "invalid namespace", + }, + { + name: "invalid subsystem", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "/metrics", + Namespace: "node_doctor", + Subsystem: "my-sub", // hyphen invalid + }, + expectedError: true, + errorContains: "invalid subsystem", + }, + { + name: "empty path - valid", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Path: "", + }, + expectedError: false, + }, + { + name: "empty namespace - valid", + config: &types.PrometheusExporterConfig{ + Port: 9100, + Namespace: "", + }, + expectedError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateConfig(tt.config) + + if tt.expectedError { + if err == nil { + t.Errorf("expected error but got none") + } else if tt.errorContains != "" && !contains(err.Error(), tt.errorContains) { + t.Errorf("expected error to contain '%s', got: %v", tt.errorContains, err) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +func TestShutdownServer(t *testing.T) { + // Test nil server + err := shutdownServer(nil, 5*time.Second) + if err != nil { + t.Errorf("expected nil error for nil server, got: %v", err) + } + + // Test shutdown of valid server + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9113, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + ctx := context.Background() + err = exporter.Start(ctx) + if err != nil { + t.Fatalf("failed to start exporter: %v", err) + } + + // Wait for server to be ready using proper readiness check + addr := fmt.Sprintf("localhost:%d", config.Port) + if err := waitForServerReady(addr, 5*time.Second); err != nil { + t.Fatalf("server never became ready: %v", err) + } + + // Verify server is running + resp, err := http.Get(fmt.Sprintf("http://localhost:%d/health", config.Port)) + if err != nil { + t.Fatalf("failed to connect to server: %v", err) + } + resp.Body.Close() + + // Test normal shutdown + err = shutdownServer(exporter.server, 5*time.Second) + if err != nil { + t.Errorf("unexpected error during shutdown: %v", err) + } + + // Verify server is stopped - use short timeout with retry + time.Sleep(100 * time.Millisecond) + client := &http.Client{Timeout: 500 * time.Millisecond} + _, err = client.Get(fmt.Sprintf("http://localhost:%d/health", config.Port)) + if err == nil { + t.Errorf("expected error connecting to stopped server") + } +} + +func TestExportProblemBeforeStart(t *testing.T) { + config := &types.PrometheusExporterConfig{ + Enabled: true, + Port: 9114, + Path: "/metrics", + Namespace: "test", + } + settings := &types.GlobalSettings{NodeName: "test-node"} + + exporter, err := NewPrometheusExporter(config, settings) + if err != nil { + t.Fatalf("failed to create exporter: %v", err) + } + + ctx := context.Background() + + // Test export before starting (should fail) + problem := &types.Problem{ + Type: "DiskPressure", + Severity: types.ProblemWarning, + Resource: "/dev/sda1", + DetectedAt: time.Now(), + Message: "Disk usage high", + } + + err = exporter.ExportProblem(ctx, problem) + if err == nil { + t.Errorf("expected error when exporting to stopped exporter") + } + if !contains(err.Error(), "not started") { + t.Errorf("expected error to contain 'not started', got: %v", err) + } +} diff --git a/pkg/monitors/network/connectivity_test.go b/pkg/monitors/network/connectivity_test.go index ed56e16..05cb75c 100644 --- a/pkg/monitors/network/connectivity_test.go +++ b/pkg/monitors/network/connectivity_test.go @@ -3,6 +3,7 @@ package network import ( "context" "fmt" + "strings" "testing" "time" @@ -693,3 +694,356 @@ type mockHTTPClientFunc struct { func (m *mockHTTPClientFunc) CheckEndpoint(ctx context.Context, endpoint EndpointConfig) (*EndpointResult, error) { return m.checkFunc(ctx, endpoint) } + +// TestParseEndpointConfig_EdgeCases tests edge cases in endpoint configuration parsing. +func TestParseEndpointConfig_EdgeCases(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + wantErr bool + errMsg string + }{ + { + name: "url is not a string", + config: map[string]interface{}{ + "url": 12345, + }, + wantErr: true, + errMsg: "url must be a string", + }, + { + name: "name is not a string", + config: map[string]interface{}{ + "url": "https://example.com", + "name": 12345, + }, + wantErr: true, + errMsg: "name must be a string", + }, + { + name: "method is not a string", + config: map[string]interface{}{ + "url": "https://example.com", + "method": 12345, + }, + wantErr: true, + errMsg: "method must be a string", + }, + { + name: "invalid method", + config: map[string]interface{}{ + "url": "https://example.com", + "method": "POST", + }, + wantErr: true, + errMsg: "invalid HTTP method", + }, + { + name: "expectedStatusCode is not a number", + config: map[string]interface{}{ + "url": "https://example.com", + "expectedStatusCode": "200", + }, + wantErr: true, + errMsg: "expectedStatusCode must be an integer", + }, + { + name: "expectedStatusCode as float64", + config: map[string]interface{}{ + "url": "https://example.com", + "expectedStatusCode": float64(201), + }, + wantErr: false, + }, + { + name: "followRedirects is not a bool", + config: map[string]interface{}{ + "url": "https://example.com", + "followRedirects": "true", + }, + wantErr: true, + errMsg: "followRedirects must be a boolean", + }, + { + name: "valid minimal config", + config: map[string]interface{}{ + "url": "https://example.com", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseEndpointConfig(tt.config) + + if tt.wantErr { + if err == nil { + t.Error("expected error, got nil") + return + } + if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("error = %v, want error containing %q", err, tt.errMsg) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +// TestNewConnectivityMonitor_EdgeCases tests edge cases in monitor creation. +func TestNewConnectivityMonitor_EdgeCases(t *testing.T) { + tests := []struct { + name string + config types.MonitorConfig + wantErr bool + errMsg string + }{ + { + name: "too many endpoints", + config: types.MonitorConfig{ + Name: "test-monitor", + Type: "network-connectivity-check", + Interval: 30 * time.Second, + Timeout: 10 * time.Second, + Enabled: true, + Config: map[string]interface{}{ + "endpoints": func() []interface{} { + endpoints := make([]interface{}, 51) // exceeds maxEndpoints (50) + for i := 0; i < 51; i++ { + endpoints[i] = map[string]interface{}{ + "url": fmt.Sprintf("https://example%d.com", i), + } + } + return endpoints + }(), + }, + }, + wantErr: true, + errMsg: "too many endpoints", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + _, err := NewConnectivityMonitor(ctx, tt.config) + + if tt.wantErr { + if err == nil { + t.Error("expected error, got nil") + return + } + if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("error = %v, want error containing %q", err, tt.errMsg) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +// TestIsValidHTTPMethod tests HTTP method validation. +func TestIsValidHTTPMethod(t *testing.T) { + tests := []struct { + method string + valid bool + }{ + {"GET", true}, + {"HEAD", true}, + {"OPTIONS", true}, + {"get", true}, // lowercase + {"head", true}, // lowercase + {"options", true}, // lowercase + {"Get", true}, // mixed case + {"POST", false}, + {"PUT", false}, + {"DELETE", false}, + {"PATCH", false}, + {"", false}, + {"INVALID", false}, + } + + for _, tt := range tests { + t.Run(tt.method, func(t *testing.T) { + got := isValidHTTPMethod(tt.method) + if got != tt.valid { + t.Errorf("isValidHTTPMethod(%q) = %v, want %v", tt.method, got, tt.valid) + } + }) + } +} + +// TestValidateURL tests URL validation for connectivity endpoints. +func TestValidateURL(t *testing.T) { + tests := []struct { + name string + url string + wantErr bool + errMsg string + }{ + { + name: "valid https URL", + url: "https://example.com", + wantErr: false, + }, + { + name: "valid http URL", + url: "http://example.com", + wantErr: false, + }, + { + name: "valid URL with path", + url: "https://example.com/api/v1/health", + wantErr: false, + }, + { + name: "valid URL with port", + url: "https://example.com:8443", + wantErr: false, + }, + { + name: "missing scheme", + url: "example.com", + wantErr: true, + errMsg: "scheme", + }, + { + name: "unsupported scheme - ftp", + url: "ftp://example.com", + wantErr: true, + errMsg: "unsupported URL scheme", + }, + { + name: "unsupported scheme - file", + url: "file:///etc/passwd", + wantErr: true, + errMsg: "unsupported URL scheme", + }, + { + name: "missing host", + url: "https://", + wantErr: true, + errMsg: "host", + }, + { + name: "empty string", + url: "", + wantErr: true, + errMsg: "scheme", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateURL(tt.url) + + if tt.wantErr { + if err == nil { + t.Error("expected error, got nil") + return + } + if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("error = %v, want error containing %q", err, tt.errMsg) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +// TestParseConnectivityConfig_EdgeCases tests edge cases in connectivity config parsing. +func TestParseConnectivityConfig_EdgeCases(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + wantErr bool + errMsg string + }{ + { + name: "nil config", + config: nil, + wantErr: true, + errMsg: "connectivity monitor requires configuration", + }, + { + name: "failureThreshold invalid type", + config: map[string]interface{}{ + "failureThreshold": "not-a-number", + "endpoints": []interface{}{ + map[string]interface{}{"url": "https://example.com"}, + }, + }, + wantErr: true, + errMsg: "failureThreshold must be an integer", + }, + { + name: "failureThreshold as float64", + config: map[string]interface{}{ + "failureThreshold": float64(3), + "endpoints": []interface{}{ + map[string]interface{}{"url": "https://example.com"}, + }, + }, + wantErr: false, + }, + { + name: "failureThreshold too low", + config: map[string]interface{}{ + "failureThreshold": 0, + "endpoints": []interface{}{ + map[string]interface{}{"url": "https://example.com"}, + }, + }, + wantErr: true, + errMsg: "failureThreshold must be at least 1", + }, + { + name: "endpoints not a list", + config: map[string]interface{}{ + "endpoints": "not-a-list", + }, + wantErr: true, + errMsg: "endpoints must be a list", + }, + { + name: "endpoint not a map", + config: map[string]interface{}{ + "endpoints": []interface{}{"not-a-map"}, + }, + wantErr: true, + errMsg: "endpoint 0 must be a map", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := parseConnectivityConfig(tt.config) + + if tt.wantErr { + if err == nil { + t.Error("expected error but got none") + } else if tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("error = %v, want error containing %q", err, tt.errMsg) + } + return + } + + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} diff --git a/pkg/monitors/network/dns_test.go b/pkg/monitors/network/dns_test.go index 5746d20..47218a1 100644 --- a/pkg/monitors/network/dns_test.go +++ b/pkg/monitors/network/dns_test.go @@ -904,3 +904,227 @@ func BenchmarkDNSResolution(b *testing.B) { _, _ = monitor.checkDNS(ctx) } } + +// TestValidateDNSConfig_EdgeCases tests additional edge cases for DNS config validation. +func TestValidateDNSConfig_EdgeCases(t *testing.T) { + tests := []struct { + name string + config types.MonitorConfig + wantError bool + errorMsg string + }{ + { + name: "invalid latency threshold - negative", + config: types.MonitorConfig{ + Name: "test-dns", + Type: "network-dns-check", + Interval: 60 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "clusterDomains": []interface{}{"kubernetes.default.svc.cluster.local"}, + "latencyThreshold": "-1s", + }, + }, + wantError: true, + errorMsg: "latencyThreshold must be positive", + }, + { + name: "valid config with customQueries only", + config: types.MonitorConfig{ + Name: "test-dns", + Type: "network-dns-check", + Interval: 60 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "customQueries": []interface{}{ + map[string]interface{}{ + "domain": "example.com", + "recordType": "A", + }, + }, + }, + }, + wantError: false, + }, + { + name: "invalid config parsing - malformed customQueries", + config: types.MonitorConfig{ + Name: "test-dns", + Type: "network-dns-check", + Interval: 60 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "customQueries": "not-an-array", + }, + }, + wantError: true, + errorMsg: "customQueries must be an array", + }, + { + name: "invalid config parsing - customQueries entry not object", + config: types.MonitorConfig{ + Name: "test-dns", + Type: "network-dns-check", + Interval: 60 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "customQueries": []interface{}{"not-an-object"}, + }, + }, + wantError: true, + errorMsg: "customQueries[0] must be an object", + }, + { + name: "invalid config parsing - customQueries domain not string", + config: types.MonitorConfig{ + Name: "test-dns", + Type: "network-dns-check", + Interval: 60 * time.Second, + Timeout: 5 * time.Second, + Config: map[string]interface{}{ + "customQueries": []interface{}{ + map[string]interface{}{ + "domain": 12345, + }, + }, + }, + }, + wantError: true, + errorMsg: "customQueries[0].domain must be a string", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateDNSConfig(tt.config) + + if tt.wantError { + if err == nil { + t.Errorf("expected error but got none") + } else if tt.errorMsg != "" && !strings.Contains(err.Error(), tt.errorMsg) { + t.Errorf("expected error containing %q, got %q", tt.errorMsg, err.Error()) + } + } else { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + }) + } +} + +// TestCheckCustomQueries tests the checkCustomQueries method. +func TestCheckCustomQueries(t *testing.T) { + tests := []struct { + name string + queries []DNSQuery + setupMock func(*mockResolver) + wantEventTypes []string + }{ + { + name: "unsupported record type - MX", + queries: []DNSQuery{ + {Domain: "example.com", RecordType: "MX"}, + }, + setupMock: func(m *mockResolver) {}, + wantEventTypes: []string{"UnsupportedQueryType"}, + }, + { + name: "unsupported record type - AAAA", + queries: []DNSQuery{ + {Domain: "example.com", RecordType: "AAAA"}, + }, + setupMock: func(m *mockResolver) {}, + wantEventTypes: []string{"UnsupportedQueryType"}, + }, + { + name: "successful A record query", + queries: []DNSQuery{ + {Domain: "example.com", RecordType: "A"}, + }, + setupMock: func(m *mockResolver) { + m.responses["example.com"] = []string{"93.184.216.34"} + }, + wantEventTypes: []string{}, + }, + { + name: "A record query with empty record type defaults to A", + queries: []DNSQuery{ + {Domain: "example.com", RecordType: ""}, + }, + setupMock: func(m *mockResolver) { + m.responses["example.com"] = []string{"93.184.216.34"} + }, + wantEventTypes: []string{}, + }, + { + name: "DNS query failure", + queries: []DNSQuery{ + {Domain: "nonexistent.invalid", RecordType: "A"}, + }, + setupMock: func(m *mockResolver) { + m.errors["nonexistent.invalid"] = fmt.Errorf("no such host") + }, + wantEventTypes: []string{"CustomDNSQueryFailed"}, + }, + { + name: "no records found", + queries: []DNSQuery{ + {Domain: "norecords.example.com", RecordType: "A"}, + }, + setupMock: func(m *mockResolver) { + m.responses["norecords.example.com"] = []string{} + }, + wantEventTypes: []string{"CustomDNSNoRecords"}, + }, + { + name: "high latency warning", + queries: []DNSQuery{ + {Domain: "slow.example.com", RecordType: "A"}, + }, + setupMock: func(m *mockResolver) { + m.responses["slow.example.com"] = []string{"1.2.3.4"} + m.latencies["slow.example.com"] = 2 * time.Second + }, + wantEventTypes: []string{"HighCustomDNSLatency"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := newMockResolver() + tt.setupMock(mock) + + monitor := &DNSMonitor{ + config: &DNSMonitorConfig{ + CustomQueries: tt.queries, + LatencyThreshold: 500 * time.Millisecond, + }, + resolver: mock, + } + + status := types.NewStatus("test-dns") + ctx := context.Background() + monitor.checkCustomQueries(ctx, status) + + // Check for expected event types + events := status.Events + eventTypes := make([]string, len(events)) + for i, e := range events { + eventTypes[i] = e.Reason + } + + if len(eventTypes) != len(tt.wantEventTypes) { + t.Errorf("got %d events, want %d. Got types: %v, want: %v", + len(eventTypes), len(tt.wantEventTypes), eventTypes, tt.wantEventTypes) + return + } + + for i, wantType := range tt.wantEventTypes { + if eventTypes[i] != wantType { + t.Errorf("event[%d] type = %s, want %s", i, eventTypes[i], wantType) + } + } + }) + } +} diff --git a/pkg/monitors/network/gateway_test.go b/pkg/monitors/network/gateway_test.go index 09c612b..7592a44 100644 --- a/pkg/monitors/network/gateway_test.go +++ b/pkg/monitors/network/gateway_test.go @@ -722,3 +722,73 @@ func TestNewGatewayMonitor(t *testing.T) { }) } } + +// TestGatewayMonitor_getGatewayIP tests the getGatewayIP method. +func TestGatewayMonitor_getGatewayIP(t *testing.T) { + tests := []struct { + name string + config *GatewayMonitorConfig + wantIP string + wantErr bool + errContain string + }{ + { + name: "manual gateway configured", + config: &GatewayMonitorConfig{ + ManualGateway: "192.168.1.1", + AutoDetectGateway: false, + }, + wantIP: "192.168.1.1", + wantErr: false, + }, + { + name: "manual gateway takes precedence over auto-detect", + config: &GatewayMonitorConfig{ + ManualGateway: "10.0.0.1", + AutoDetectGateway: true, // should be ignored when manual is set + }, + wantIP: "10.0.0.1", + wantErr: false, + }, + { + name: "no gateway and auto-detect disabled", + config: &GatewayMonitorConfig{ + ManualGateway: "", + AutoDetectGateway: false, + }, + wantErr: true, + errContain: "no gateway configured", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + monitor := &GatewayMonitor{ + name: "test-gateway", + config: tt.config, + } + + ip, err := monitor.getGatewayIP() + + if tt.wantErr { + if err == nil { + t.Errorf("getGatewayIP() expected error, got nil") + return + } + if tt.errContain != "" && !strings.Contains(err.Error(), tt.errContain) { + t.Errorf("getGatewayIP() error = %v, want error containing %q", err, tt.errContain) + } + return + } + + if err != nil { + t.Errorf("getGatewayIP() unexpected error: %v", err) + return + } + + if ip != tt.wantIP { + t.Errorf("getGatewayIP() = %q, want %q", ip, tt.wantIP) + } + }) + } +} diff --git a/pkg/monitors/network/http_client_test.go b/pkg/monitors/network/http_client_test.go index 6666fd0..8c80a57 100644 --- a/pkg/monitors/network/http_client_test.go +++ b/pkg/monitors/network/http_client_test.go @@ -2,6 +2,7 @@ package network import ( "context" + "net" "net/http" "net/http/httptest" "testing" @@ -377,6 +378,49 @@ func TestClassifyHTTPError(t *testing.T) { } } +// TestClassifyHTTPError_NetworkErrors tests network-related error classification. +func TestClassifyHTTPError_NetworkErrors(t *testing.T) { + // Test DNS error + dnsErr := &net.DNSError{ + Err: "no such host", + Name: "invalid.host", + } + if got := classifyHTTPError(dnsErr); got != "DNS" { + t.Errorf("classifyHTTPError(DNSError) = %s, want DNS", got) + } + + // Test OpError with dial + opErr := &net.OpError{ + Op: "dial", + Net: "tcp", + Err: &net.DNSError{Err: "lookup failed"}, + } + result := classifyHTTPError(opErr) + if result != "DNS" && result != "Connection" { + t.Errorf("classifyHTTPError(OpError with DNS) = %s, want DNS or Connection", result) + } + + // Test OpError with dial operation + dialErr := &net.OpError{ + Op: "dial", + Net: "tcp", + Err: &net.AddrError{Err: "connection refused"}, + } + if got := classifyHTTPError(dialErr); got != "Connection" { + t.Errorf("classifyHTTPError(dial OpError) = %s, want Connection", got) + } + + // Test generic error falls back to Network + genericErr := &net.OpError{ + Op: "read", + Net: "tcp", + Err: &net.AddrError{Err: "connection reset"}, + } + if got := classifyHTTPError(genericErr); got != "Network" { + t.Errorf("classifyHTTPError(generic network error) = %s, want Network", got) + } +} + // mockHTTPClient implements HTTPClient for testing. type mockHTTPClient struct { result *EndpointResult diff --git a/pkg/reload/diff.go b/pkg/reload/diff.go index b0bd5d2..156c2c5 100644 --- a/pkg/reload/diff.go +++ b/pkg/reload/diff.go @@ -167,11 +167,25 @@ func exportersEqual(a, b *types.ExporterConfigs) bool { len(a.HTTP.Webhooks) != len(b.HTTP.Webhooks) { return false } + // Compare webhook contents + for i := range a.HTTP.Webhooks { + if !webhookEqual(&a.HTTP.Webhooks[i], &b.HTTP.Webhooks[i]) { + return false + } + } } return true } +// webhookEqual checks if two webhook endpoints are equal. +func webhookEqual(a, b *types.WebhookEndpoint) bool { + return a.Name == b.Name && + a.URL == b.URL && + a.SendStatus == b.SendStatus && + a.SendProblems == b.SendProblems +} + // remediationEqual checks if remediation configurations are equal. func remediationEqual(a, b *types.RemediationConfig) bool { // Handle nil pointers diff --git a/pkg/reload/validator_test.go b/pkg/reload/validator_test.go index 9cae50d..c5ecc13 100644 --- a/pkg/reload/validator_test.go +++ b/pkg/reload/validator_test.go @@ -821,3 +821,1063 @@ func createValidConfig() *types.NodeDoctorConfig { return config } + +// TestValidate_KubeletMonitorConfig tests kubelet monitor validation +func TestValidate_KubeletMonitorConfig(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + expectedField string + expectedMsg string + }{ + { + name: "valid kubelet config", + config: map[string]interface{}{ + "port": 10250, + "endpoint": "/healthz", + "timeout": "10s", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "invalid port type", + config: map[string]interface{}{ + "port": "invalid", + }, + expectedField: "monitors[0].config.port", + expectedMsg: "port must be a number", + }, + { + name: "port out of range", + config: map[string]interface{}{ + "port": 70000, + }, + expectedField: "monitors[0].config.port", + expectedMsg: "port must be between 1 and 65535", + }, + { + name: "invalid endpoint type", + config: map[string]interface{}{ + "endpoint": 123, + }, + expectedField: "monitors[0].config.endpoint", + expectedMsg: "endpoint must be a string", + }, + { + name: "empty endpoint", + config: map[string]interface{}{ + "endpoint": "", + }, + expectedField: "monitors[0].config.endpoint", + expectedMsg: "endpoint cannot be empty", + }, + { + name: "invalid timeout type", + config: map[string]interface{}{ + "timeout": 123, + }, + expectedField: "monitors[0].config.timeout", + expectedMsg: "timeout must be a string", + }, + { + name: "invalid timeout format", + config: map[string]interface{}{ + "timeout": "invalid-duration", + }, + expectedField: "monitors[0].config.timeout", + expectedMsg: "invalid timeout duration format", + }, + { + name: "port as string", + config: map[string]interface{}{ + "port": "8080", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "port zero", + config: map[string]interface{}{ + "port": 0, + }, + expectedField: "monitors[0].config.port", + expectedMsg: "port must be between 1 and 65535", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "kubelet" + cfg.Monitors[0].Config = tt.config + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_CapacityMonitorConfig tests capacity monitor validation +func TestValidate_CapacityMonitorConfig(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + expectedField string + expectedMsg string + }{ + { + name: "valid capacity config", + config: map[string]interface{}{ + "cpu": 80.0, + "memory": 85.0, + "disk": 90.0, + "warning": 80.0, + "critical": 95.0, + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "cpu threshold over 100", + config: map[string]interface{}{ + "cpu": 150.0, + }, + expectedField: "monitors[0].config.cpu", + expectedMsg: "threshold must be between 0 and 100", + }, + { + name: "memory threshold negative", + config: map[string]interface{}{ + "memory": -10.0, + }, + expectedField: "monitors[0].config.memory", + expectedMsg: "threshold must be between 0 and 100", + }, + { + name: "disk threshold invalid type", + config: map[string]interface{}{ + "disk": "high", + }, + expectedField: "monitors[0].config.disk", + expectedMsg: "threshold must be a number", + }, + { + name: "warning threshold as int", + config: map[string]interface{}{ + "warning": 80, + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "critical as string", + config: map[string]interface{}{ + "critical": "90", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "invalid string threshold", + config: map[string]interface{}{ + "warning": "high", + }, + expectedField: "monitors[0].config.warning", + expectedMsg: "threshold must be a number", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "capacity" + cfg.Monitors[0].Config = tt.config + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_LogPatternMonitorConfig tests log-pattern monitor validation +func TestValidate_LogPatternMonitorConfig(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + expectedField string + expectedMsg string + }{ + { + name: "valid log-pattern config", + config: map[string]interface{}{ + "logFile": "/var/log/messages", + "pattern": "ERROR|FATAL", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "nil config", + config: nil, + expectedField: "monitors[0].config", + expectedMsg: "log pattern monitor requires configuration", + }, + { + name: "missing logFile", + config: map[string]interface{}{ + "pattern": "ERROR", + }, + expectedField: "monitors[0].config.logFile", + expectedMsg: "logFile is required", + }, + { + name: "missing pattern", + config: map[string]interface{}{ + "logFile": "/var/log/messages", + }, + expectedField: "monitors[0].config.pattern", + expectedMsg: "pattern is required", + }, + { + name: "invalid logFile type", + config: map[string]interface{}{ + "logFile": 123, + "pattern": "ERROR", + }, + expectedField: "monitors[0].config.logFile", + expectedMsg: "logFile must be a string", + }, + { + name: "invalid pattern type", + config: map[string]interface{}{ + "logFile": "/var/log/messages", + "pattern": 123, + }, + expectedField: "monitors[0].config.pattern", + expectedMsg: "pattern must be a string", + }, + { + name: "relative logFile path", + config: map[string]interface{}{ + "logFile": "var/log/messages", + "pattern": "ERROR", + }, + expectedField: "monitors[0].config.logFile", + expectedMsg: "file path must be absolute", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "log-pattern" + cfg.Monitors[0].Config = tt.config + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_ScriptMonitorConfig tests script monitor validation +func TestValidate_ScriptMonitorConfig(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + expectedField string + expectedMsg string + }{ + { + name: "valid script config", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh", + "timeout": "30s", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "nil config", + config: nil, + expectedField: "monitors[0].config", + expectedMsg: "script monitor requires configuration", + }, + { + name: "missing script", + config: map[string]interface{}{ + "timeout": "30s", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "script path is required", + }, + { + name: "invalid script type", + config: map[string]interface{}{ + "script": 123, + }, + expectedField: "monitors[0].config.script", + expectedMsg: "script must be a string", + }, + { + name: "invalid timeout type", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh", + "timeout": 123, + }, + expectedField: "monitors[0].config.timeout", + expectedMsg: "timeout must be a string", + }, + { + name: "invalid timeout format", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh", + "timeout": "invalid", + }, + expectedField: "monitors[0].config.timeout", + expectedMsg: "invalid timeout duration format", + }, + { + name: "relative script path", + config: map[string]interface{}{ + "script": "scripts/check.sh", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "file path must be absolute", + }, + { + name: "script with shell metacharacter semicolon", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh; rm -rf /", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "shell metacharacters", + }, + { + name: "script with shell metacharacter pipe", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh | cat", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "shell metacharacters", + }, + { + name: "script with shell metacharacter ampersand", + config: map[string]interface{}{ + "script": "/usr/local/bin/check.sh & sleep 1", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "shell metacharacters", + }, + { + name: "script with shell metacharacter backtick", + config: map[string]interface{}{ + "script": "/usr/local/bin/`whoami`.sh", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "shell metacharacters", + }, + { + name: "script with shell metacharacter dollar", + config: map[string]interface{}{ + "script": "/usr/local/bin/$USER.sh", + }, + expectedField: "monitors[0].config.script", + expectedMsg: "shell metacharacters", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "script" + cfg.Monitors[0].Config = tt.config + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_PrometheusMonitorConfig tests prometheus monitor validation +func TestValidate_PrometheusMonitorConfig(t *testing.T) { + tests := []struct { + name string + config map[string]interface{} + expectedField string + expectedMsg string + }{ + { + name: "valid prometheus config", + config: map[string]interface{}{ + "url": "http://prometheus:9090", + "query": "up{job=\"kubernetes\"}", + }, + expectedField: "", + expectedMsg: "", + }, + { + name: "nil config", + config: nil, + expectedField: "monitors[0].config", + expectedMsg: "prometheus monitor requires configuration", + }, + { + name: "missing url", + config: map[string]interface{}{ + "query": "up{job=\"kubernetes\"}", + }, + expectedField: "monitors[0].config.url", + expectedMsg: "url is required", + }, + { + name: "missing query", + config: map[string]interface{}{ + "url": "http://prometheus:9090", + }, + expectedField: "monitors[0].config.query", + expectedMsg: "query is required", + }, + { + name: "invalid url type", + config: map[string]interface{}{ + "url": 123, + "query": "up", + }, + expectedField: "monitors[0].config.url", + expectedMsg: "url must be a string", + }, + { + name: "invalid query type", + config: map[string]interface{}{ + "url": "http://prometheus:9090", + "query": 123, + }, + expectedField: "monitors[0].config.query", + expectedMsg: "query must be a string", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "prometheus" + cfg.Monitors[0].Config = tt.config + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_PrometheusExporter tests prometheus exporter validation +func TestValidate_PrometheusExporter(t *testing.T) { + tests := []struct { + name string + port int + path string + expectedField string + expectedMsg string + }{ + { + name: "valid config", + port: 9100, + path: "/metrics", + expectedField: "", + expectedMsg: "", + }, + { + name: "port zero", + port: 0, + path: "/metrics", + expectedField: "exporters.prometheus.port", + expectedMsg: "port must be positive", + }, + { + name: "port negative", + port: -1, + path: "/metrics", + expectedField: "exporters.prometheus.port", + expectedMsg: "port must be positive", + }, + { + name: "port below 1024", + port: 80, + path: "/metrics", + expectedField: "exporters.prometheus.port", + expectedMsg: "port must be between 1024 and 65535", + }, + { + name: "port above 65535", + port: 70000, + path: "/metrics", + expectedField: "exporters.prometheus.port", + expectedMsg: "port must be between 1024 and 65535", + }, + { + name: "empty path", + port: 9100, + path: "", + expectedField: "exporters.prometheus.path", + expectedMsg: "metrics path is required", + }, + { + name: "path without leading slash", + port: 9100, + path: "metrics", + expectedField: "exporters.prometheus.path", + expectedMsg: "path must start with '/'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Exporters.Prometheus.Port = tt.port + cfg.Exporters.Prometheus.Path = tt.path + // Disable other exporters + cfg.Exporters.Kubernetes.Enabled = false + cfg.Exporters.HTTP.Enabled = false + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_KubernetesExporter tests kubernetes exporter validation +func TestValidate_KubernetesExporter(t *testing.T) { + tests := []struct { + name string + namespace string + expectedField string + expectedMsg string + }{ + { + name: "valid namespace", + namespace: "kube-system", + expectedField: "", + expectedMsg: "", + }, + { + name: "empty namespace", + namespace: "", + expectedField: "exporters.kubernetes.namespace", + expectedMsg: "namespace is required", + }, + { + name: "invalid namespace with uppercase", + namespace: "Kube-System", + expectedField: "exporters.kubernetes.namespace", + expectedMsg: "namespace must be a valid Kubernetes resource name", + }, + { + name: "namespace starting with dash", + namespace: "-kube-system", + expectedField: "exporters.kubernetes.namespace", + expectedMsg: "namespace must be a valid Kubernetes resource name", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Exporters.Kubernetes.Namespace = tt.namespace + // Disable other exporters + cfg.Exporters.HTTP.Enabled = false + cfg.Exporters.Prometheus.Enabled = false + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_Remediation tests remediation config validation +func TestValidate_Remediation(t *testing.T) { + tests := []struct { + name string + modifyFunc func(*types.RemediationConfig) + expectedField string + expectedMsg string + }{ + { + name: "valid remediation", + modifyFunc: func(r *types.RemediationConfig) {}, + expectedField: "", + expectedMsg: "", + }, + { + name: "negative cooldown", + modifyFunc: func(r *types.RemediationConfig) { + r.CooldownPeriod = -5 * time.Minute + }, + expectedField: "remediation.cooldownPeriod", + expectedMsg: "cooldown period cannot be negative", + }, + { + name: "negative max attempts", + modifyFunc: func(r *types.RemediationConfig) { + r.MaxAttemptsGlobal = -1 + }, + expectedField: "remediation.maxAttemptsGlobal", + expectedMsg: "max attempts cannot be negative", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + tt.modifyFunc(&cfg.Remediation) + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if tt.expectedField == "" { + if !result.Valid { + t.Errorf("Expected valid config, got errors: %v", result.Errors) + } + } else { + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + } + }) + } +} + +// TestValidate_HTTPWebhookConfig tests webhook config validation +func TestValidate_HTTPWebhookConfig(t *testing.T) { + tests := []struct { + name string + webhooks []types.WebhookEndpoint + expectedField string + expectedMsg string + }{ + { + name: "webhook with zero timeout", + webhooks: []types.WebhookEndpoint{ + { + Name: "test", + URL: "https://example.com/webhook", + Timeout: 0, + }, + }, + expectedField: "exporters.http.webhooks[0].timeout", + expectedMsg: "timeout must be positive", + }, + { + name: "webhook with negative timeout", + webhooks: []types.WebhookEndpoint{ + { + Name: "test", + URL: "https://example.com/webhook", + Timeout: -5 * time.Second, + }, + }, + expectedField: "exporters.http.webhooks[0].timeout", + expectedMsg: "timeout must be positive", + }, + { + name: "webhook with empty URL", + webhooks: []types.WebhookEndpoint{ + { + Name: "test", + URL: "", + Timeout: 30 * time.Second, + }, + }, + expectedField: "exporters.http.webhooks[0].url", + expectedMsg: "webhook URL is required", + }, + { + name: "too many webhooks", + webhooks: func() []types.WebhookEndpoint { + webhooks := make([]types.WebhookEndpoint, 60) + for i := range webhooks { + webhooks[i] = types.WebhookEndpoint{ + Name: "webhook-" + string(rune('a'+i%26)), + URL: "https://example.com/webhook", + Timeout: 30 * time.Second, + } + } + return webhooks + }(), + expectedField: "exporters.http.webhooks", + expectedMsg: "too many webhooks", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := createValidConfig() + cfg.Exporters.HTTP.Webhooks = tt.webhooks + // Disable other exporters + cfg.Exporters.Kubernetes.Enabled = false + cfg.Exporters.Prometheus.Enabled = false + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail") + } + hasExpectedError := false + for _, err := range result.Errors { + if err.Field == tt.expectedField && strings.Contains(err.Message, tt.expectedMsg) { + hasExpectedError = true + break + } + } + if !hasExpectedError { + t.Errorf("Expected error field=%s containing %q, got: %v", tt.expectedField, tt.expectedMsg, result.Errors) + } + }) + } +} + +// TestValidate_MonitorTypeEmpty tests validation for empty monitor type +func TestValidate_MonitorTypeEmpty(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "" + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for empty monitor type") + } + + hasTypeError := false + for _, err := range result.Errors { + if err.Field == "monitors[0].type" && strings.Contains(err.Message, "type is required") { + hasTypeError = true + break + } + } + if !hasTypeError { + t.Errorf("Expected type required error, got: %v", result.Errors) + } +} + +// TestValidate_InvalidMonitorType tests validation for unknown monitor type +func TestValidate_InvalidMonitorType(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Type = "unknown-type" + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for unknown monitor type") + } + + hasTypeError := false + for _, err := range result.Errors { + if err.Field == "monitors[0].type" && strings.Contains(err.Message, "unsupported monitor type") { + hasTypeError = true + break + } + } + if !hasTypeError { + t.Errorf("Expected unsupported type error, got: %v", result.Errors) + } +} + +// TestValidate_MonitorDependenciesExceedMax tests dependency limit validation +func TestValidate_MonitorDependenciesExceedMax(t *testing.T) { + cfg := createValidConfig() + // Create many dependencies + deps := make([]string, 25) + for i := range deps { + deps[i] = "monitor-" + string(rune('a'+i%26)) + } + cfg.Monitors[0].DependsOn = deps + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for too many dependencies") + } + + hasDepsError := false + for _, err := range result.Errors { + if strings.Contains(err.Field, "dependsOn") && strings.Contains(err.Message, "too many dependencies") { + hasDepsError = true + break + } + } + if !hasDepsError { + t.Errorf("Expected too many dependencies error, got: %v", result.Errors) + } +} + +// TestValidate_InvalidKubernetesName tests various invalid kubernetes names +func TestValidate_InvalidKubernetesName(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + {"valid simple", "test", true}, + {"valid with dash", "test-name", true}, + {"valid with dot", "test.name", true}, + {"valid with numbers", "test123", true}, + {"empty", "", false}, + {"too long", strings.Repeat("a", 64), false}, + {"uppercase", "Test", false}, + {"underscore", "test_name", false}, + {"starts with dash", "-test", false}, + {"ends with dash", "test-", false}, + {"starts with dot", ".test", false}, + {"ends with dot", "test.", false}, + {"special char", "test@name", false}, + } + + validator := NewConfigValidator() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := validator.isValidKubernetesName(tt.input) + if result != tt.expected { + t.Errorf("isValidKubernetesName(%q) = %v, want %v", tt.input, result, tt.expected) + } + }) + } +} + +// TestFormatValidationErrors_SingleError tests formatting with single error +func TestFormatValidationErrors_SingleError(t *testing.T) { + errors := []ValidationError{ + {Field: "field1", Message: "error message 1"}, + } + + result := FormatValidationErrors(errors) + expected := "field1: error message 1" + + if result != expected { + t.Errorf("Expected %q, got %q", expected, result) + } +} + +// TestValidate_NoMonitors tests validation when no monitors are configured +func TestValidate_NoMonitors(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors = []types.MonitorConfig{} + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for empty monitors") + } + + hasMonitorsError := false + for _, err := range result.Errors { + if err.Field == "monitors" && strings.Contains(err.Message, "at least one monitor") { + hasMonitorsError = true + break + } + } + if !hasMonitorsError { + t.Errorf("Expected monitors required error, got: %v", result.Errors) + } +} + +// TestValidate_EmptyMonitorName tests validation when monitor name is empty +func TestValidate_EmptyMonitorName(t *testing.T) { + cfg := createValidConfig() + cfg.Monitors[0].Name = "" + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for empty monitor name") + } + + hasNameError := false + for _, err := range result.Errors { + if err.Field == "monitors[0].name" && strings.Contains(err.Message, "name is required") { + hasNameError = true + break + } + } + if !hasNameError { + t.Errorf("Expected name required error, got: %v", result.Errors) + } +} + +// TestValidate_InvalidMetadataName tests validation for invalid metadata name +func TestValidate_InvalidMetadataName(t *testing.T) { + cfg := createValidConfig() + cfg.Metadata.Name = "Invalid-Name" + + validator := NewConfigValidator() + result := validator.Validate(cfg) + + if result.Valid { + t.Error("Expected validation to fail for invalid metadata name") + } + + hasNameError := false + for _, err := range result.Errors { + if err.Field == "metadata.name" && strings.Contains(err.Message, "valid Kubernetes resource name") { + hasNameError = true + break + } + } + if !hasNameError { + t.Errorf("Expected invalid name error, got: %v", result.Errors) + } +} diff --git a/pkg/reload/watcher_test.go b/pkg/reload/watcher_test.go index f863f85..e5dd2f7 100644 --- a/pkg/reload/watcher_test.go +++ b/pkg/reload/watcher_test.go @@ -420,3 +420,61 @@ func TestIsConfigFileEvent(t *testing.T) { }) } } + +// TestNewConfigWatcher_EmptyPath tests that empty path returns error +func TestNewConfigWatcher_EmptyPath(t *testing.T) { + watcher, err := NewConfigWatcher("", 100*time.Millisecond) + if err == nil { + t.Error("Expected error for empty config path") + if watcher != nil { + watcher.Stop() + } + return + } + if watcher != nil { + t.Error("Expected nil watcher for empty config path") + watcher.Stop() + } +} + +// TestNewConfigWatcher_DefaultDebounce tests default debounce interval +func TestNewConfigWatcher_DefaultDebounce(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + + if err := os.WriteFile(configPath, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test config: %v", err) + } + + // Use zero debounce interval - should default to 500ms + watcher, err := NewConfigWatcher(configPath, 0) + if err != nil { + t.Fatalf("Failed to create watcher: %v", err) + } + defer watcher.Stop() + + if watcher.debounceInterval != 500*time.Millisecond { + t.Errorf("Expected default debounce of 500ms, got %v", watcher.debounceInterval) + } +} + +// TestNewConfigWatcher_NegativeDebounce tests negative debounce interval +func TestNewConfigWatcher_NegativeDebounce(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + + if err := os.WriteFile(configPath, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test config: %v", err) + } + + // Use negative debounce interval - should default to 500ms + watcher, err := NewConfigWatcher(configPath, -100*time.Millisecond) + if err != nil { + t.Fatalf("Failed to create watcher: %v", err) + } + defer watcher.Stop() + + if watcher.debounceInterval != 500*time.Millisecond { + t.Errorf("Expected default debounce of 500ms, got %v", watcher.debounceInterval) + } +} diff --git a/pkg/remediators/base_test.go b/pkg/remediators/base_test.go index 04d312f..c210b10 100644 --- a/pkg/remediators/base_test.go +++ b/pkg/remediators/base_test.go @@ -626,6 +626,62 @@ func TestConcurrentStateQueries(t *testing.T) { // If we get here without deadlock or race, test passes } +// TestBaseRemediator_LogWarnf_NilLogger verifies logWarnf handles nil logger. +func TestBaseRemediator_LogWarnf_NilLogger(t *testing.T) { + remediator, err := NewBaseRemediator("test", 5*time.Minute) + if err != nil { + t.Fatalf("failed to create remediator: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + remediator.logWarnf("test warning message: %s", "value") +} + +// TestBaseRemediator_LogErrorf_NilLogger verifies logErrorf handles nil logger. +func TestBaseRemediator_LogErrorf_NilLogger(t *testing.T) { + remediator, err := NewBaseRemediator("test", 5*time.Minute) + if err != nil { + t.Fatalf("failed to create remediator: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + remediator.logErrorf("test error message: %s", "value") +} + +// TestBaseRemediator_LogWarnf_WithLogger verifies logWarnf logs with logger set. +func TestBaseRemediator_LogWarnf_WithLogger(t *testing.T) { + remediator, err := NewBaseRemediator("test", 5*time.Minute) + if err != nil { + t.Fatalf("failed to create remediator: %v", err) + } + logger := &mockLogger{} + _ = remediator.SetLogger(logger) + + remediator.logWarnf("test warning: %s", "value") + + if len(logger.warnMessages) != 1 { + t.Errorf("expected 1 warn message, got %d", len(logger.warnMessages)) + } +} + +// TestBaseRemediator_LogErrorf_WithLogger verifies logErrorf logs with logger set. +func TestBaseRemediator_LogErrorf_WithLogger(t *testing.T) { + remediator, err := NewBaseRemediator("test", 5*time.Minute) + if err != nil { + t.Fatalf("failed to create remediator: %v", err) + } + logger := &mockLogger{} + _ = remediator.SetLogger(logger) + + remediator.logErrorf("test error: %s", "value") + + if len(logger.errorMessages) != 1 { + t.Errorf("expected 1 error message, got %d", len(logger.errorMessages)) + } +} + // TestEmbeddingPattern demonstrates embedding BaseRemediator func TestEmbeddingPattern(t *testing.T) { // Define a concrete remediator that embeds BaseRemediator diff --git a/pkg/remediators/custom_test.go b/pkg/remediators/custom_test.go index ccfddd6..4180b72 100644 --- a/pkg/remediators/custom_test.go +++ b/pkg/remediators/custom_test.go @@ -685,3 +685,78 @@ func TestCustomRemediator_WorkingDirectory(t *testing.T) { t.Errorf("Working directory = %s, want %s", m.executedWorkDir[0], config.WorkingDir) } } + +// TestCustomRemediator_GetScriptPath tests the GetScriptPath method. +func TestCustomRemediator_GetScriptPath(t *testing.T) { + expectedPath := "/usr/local/bin/test-script.sh" + config := CustomConfig{ + ScriptPath: expectedPath, + } + + r, err := NewCustomRemediator(config) + if err != nil { + t.Fatalf("NewCustomRemediator() error: %v", err) + } + + actualPath := r.GetScriptPath() + if actualPath != expectedPath { + t.Errorf("GetScriptPath() = %s, want %s", actualPath, expectedPath) + } +} + +// TestCustomRemediator_LogInfof_NilLogger verifies logInfof handles nil logger. +func TestCustomRemediator_LogInfof_NilLogger(t *testing.T) { + config := CustomConfig{ + ScriptPath: "/usr/local/bin/remediate.sh", + } + + r, err := NewCustomRemediator(config) + if err != nil { + t.Fatalf("NewCustomRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logInfof("test info message: %s", "value") +} + +// TestCustomRemediator_LogWarnf_NilLogger verifies logWarnf handles nil logger. +func TestCustomRemediator_LogWarnf_NilLogger(t *testing.T) { + config := CustomConfig{ + ScriptPath: "/usr/local/bin/remediate.sh", + } + + r, err := NewCustomRemediator(config) + if err != nil { + t.Fatalf("NewCustomRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logWarnf("test warning message: %s", "value") +} + +// TestCustomRemediator_LogWithLogger verifies log methods work with logger set. +func TestCustomRemediator_LogWithLogger(t *testing.T) { + config := CustomConfig{ + ScriptPath: "/usr/local/bin/remediate.sh", + } + + r, err := NewCustomRemediator(config) + if err != nil { + t.Fatalf("NewCustomRemediator() error: %v", err) + } + + logger := &mockLogger{} + r.SetLogger(logger) + + r.logInfof("test info: %s", "value") + r.logWarnf("test warn: %s", "value") + + if len(logger.infoMessages) != 1 { + t.Errorf("expected 1 info message, got %d", len(logger.infoMessages)) + } + if len(logger.warnMessages) != 1 { + t.Errorf("expected 1 warn message, got %d", len(logger.warnMessages)) + } +} diff --git a/pkg/remediators/disk_test.go b/pkg/remediators/disk_test.go index ba088f6..7ff90c0 100644 --- a/pkg/remediators/disk_test.go +++ b/pkg/remediators/disk_test.go @@ -827,3 +827,63 @@ func TestDiskRemediator_DefaultConfig(t *testing.T) { }) } } + +// TestDiskRemediator_LogInfof_NilLogger verifies logInfof handles nil logger. +func TestDiskRemediator_LogInfof_NilLogger(t *testing.T) { + config := DiskConfig{ + Operation: DiskCleanJournalLogs, + TargetPath: "/var/log", + } + + r, err := NewDiskRemediator(config) + if err != nil { + t.Fatalf("NewDiskRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logInfof("test info message: %s", "value") +} + +// TestDiskRemediator_LogWarnf_NilLogger verifies logWarnf handles nil logger. +func TestDiskRemediator_LogWarnf_NilLogger(t *testing.T) { + config := DiskConfig{ + Operation: DiskCleanJournalLogs, + TargetPath: "/var/log", + } + + r, err := NewDiskRemediator(config) + if err != nil { + t.Fatalf("NewDiskRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logWarnf("test warning message: %s", "value") +} + +// TestDiskRemediator_LogWithLogger verifies log methods work with logger set. +func TestDiskRemediator_LogWithLogger(t *testing.T) { + config := DiskConfig{ + Operation: DiskCleanJournalLogs, + TargetPath: "/var/log", + } + + r, err := NewDiskRemediator(config) + if err != nil { + t.Fatalf("NewDiskRemediator() error: %v", err) + } + + logger := &mockLogger{} + r.SetLogger(logger) + + r.logInfof("test info: %s", "value") + r.logWarnf("test warn: %s", "value") + + if len(logger.infoMessages) != 1 { + t.Errorf("expected 1 info message, got %d", len(logger.infoMessages)) + } + if len(logger.warnMessages) != 1 { + t.Errorf("expected 1 warn message, got %d", len(logger.warnMessages)) + } +} diff --git a/pkg/remediators/network_test.go b/pkg/remediators/network_test.go index 323fbe8..de943f7 100644 --- a/pkg/remediators/network_test.go +++ b/pkg/remediators/network_test.go @@ -755,3 +755,132 @@ func TestNetworkRemediator_InterfaceCheckFailure(t *testing.T) { t.Errorf("Remediate() expected error for interface check failure, got nil") } } + +// TestNetworkRemediator_LogWarnf_NilLogger verifies logWarnf handles nil logger. +func TestNetworkRemediator_LogWarnf_NilLogger(t *testing.T) { + config := NetworkConfig{ + Operation: NetworkRestartInterface, + InterfaceName: "eth0", + } + + r, err := NewNetworkRemediator(config) + if err != nil { + t.Fatalf("NewNetworkRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logWarnf("test warning message: %s", "value") +} + +// TestNetworkRemediator_LogInfof_NilLogger verifies logInfof handles nil logger. +func TestNetworkRemediator_LogInfof_NilLogger(t *testing.T) { + config := NetworkConfig{ + Operation: NetworkRestartInterface, + InterfaceName: "eth0", + } + + r, err := NewNetworkRemediator(config) + if err != nil { + t.Fatalf("NewNetworkRemediator() error: %v", err) + } + // Don't set a logger - logger is nil + + // This should not panic + r.logInfof("test info message: %s", "value") +} + +// TestNetworkRemediator_VerifyOperation_ResetRouting tests verifyOperation for reset routing. +func TestNetworkRemediator_VerifyOperation_ResetRouting(t *testing.T) { + config := NetworkConfig{ + Operation: NetworkResetRouting, + } + + r, err := NewNetworkRemediator(config) + if err != nil { + t.Fatalf("NewNetworkRemediator() error: %v", err) + } + + mockExec := &mockNetworkExecutor{ + routingTable: "default via 192.168.1.1", + } + r.SetNetworkExecutor(mockExec) + + problem := types.Problem{ + Type: "network-routing", + Resource: "routing-table", + Severity: types.ProblemWarning, + } + + ctx := context.Background() + err = r.Remediate(ctx, problem) + + // Should succeed with routing verification + if err != nil { + t.Errorf("Remediate() unexpected error: %v", err) + } +} + +// TestNetworkRemediator_VerifyRoutingTable_Direct tests verifyRoutingTable directly. +func TestNetworkRemediator_VerifyRoutingTable_Direct(t *testing.T) { + config := NetworkConfig{ + Operation: NetworkResetRouting, + } + + r, err := NewNetworkRemediator(config) + if err != nil { + t.Fatalf("NewNetworkRemediator() error: %v", err) + } + + t.Run("success case", func(t *testing.T) { + mockExec := &mockNetworkExecutor{ + routingTable: "default via 192.168.1.1", + } + r.SetNetworkExecutor(mockExec) + + ctx := context.Background() + err := r.verifyRoutingTable(ctx) + if err != nil { + t.Errorf("verifyRoutingTable() unexpected error: %v", err) + } + }) + + t.Run("failure case", func(t *testing.T) { + mockExec := &mockNetworkExecutor{ + shouldFailCommand: true, // This will cause routing table fetch to fail + } + r.SetNetworkExecutor(mockExec) + + ctx := context.Background() + err := r.verifyRoutingTable(ctx) + if err == nil { + t.Error("verifyRoutingTable() expected error for routing table failure, got nil") + } + }) +} + +// TestNetworkRemediator_LogWithLogger tests log methods with logger set. +func TestNetworkRemediator_LogWithLogger(t *testing.T) { + config := NetworkConfig{ + Operation: NetworkRestartInterface, + InterfaceName: "eth0", + } + + r, err := NewNetworkRemediator(config) + if err != nil { + t.Fatalf("NewNetworkRemediator() error: %v", err) + } + + logger := &mockLogger{} + r.SetLogger(logger) + + r.logInfof("test info: %s", "value") + r.logWarnf("test warn: %s", "value") + + if len(logger.infoMessages) != 1 { + t.Errorf("expected 1 info message, got %d", len(logger.infoMessages)) + } + if len(logger.warnMessages) != 1 { + t.Errorf("expected 1 warn message, got %d", len(logger.warnMessages)) + } +} diff --git a/pkg/remediators/registry_test.go b/pkg/remediators/registry_test.go index a0e433c..75607d9 100644 --- a/pkg/remediators/registry_test.go +++ b/pkg/remediators/registry_test.go @@ -1458,3 +1458,39 @@ func TestSanitizeEventName(t *testing.T) { }) } } + +// TestRemediatorRegistry_LogWarnf_NilLogger verifies logWarnf handles nil logger. +func TestRemediatorRegistry_LogWarnf_NilLogger(t *testing.T) { + registry := NewRegistry(100, 1000) + // Don't set a logger - logger is nil by default + + // This should not panic + registry.logWarnf("test warning message: %s", "value") +} + +// TestRemediatorRegistry_LogErrorf_NilLogger verifies logErrorf handles nil logger. +func TestRemediatorRegistry_LogErrorf_NilLogger(t *testing.T) { + registry := NewRegistry(100, 1000) + // Don't set a logger - logger is nil by default + + // This should not panic + registry.logErrorf("test error message: %s", "value") +} + +// TestRemediatorRegistry_LogWithLogger verifies log methods work with logger set. +func TestRemediatorRegistry_LogWithLogger(t *testing.T) { + registry := NewRegistry(100, 1000) + + logger := &mockLogger{} + registry.SetLogger(logger) + + registry.logWarnf("test warn: %s", "value") + registry.logErrorf("test error: %s", "value") + + if len(logger.warnMessages) != 1 { + t.Errorf("expected 1 warn message, got %d", len(logger.warnMessages)) + } + if len(logger.errorMessages) != 1 { + t.Errorf("expected 1 error message, got %d", len(logger.errorMessages)) + } +} diff --git a/pkg/types/exporter_test.go b/pkg/types/exporter_test.go new file mode 100644 index 0000000..f808b18 --- /dev/null +++ b/pkg/types/exporter_test.go @@ -0,0 +1,151 @@ +package types + +import ( + "errors" + "testing" +) + +// TestExporterReloadSummary_AddResult verifies AddResult method functionality. +func TestExporterReloadSummary_AddResult(t *testing.T) { + tests := []struct { + name string + results []ExporterReloadResult + expectedTotal int + expectedSuccessful int + expectedFailed int + expectedReloadableCount int + }{ + { + name: "empty summary", + results: []ExporterReloadResult{}, + expectedTotal: 0, + expectedSuccessful: 0, + expectedFailed: 0, + expectedReloadableCount: 0, + }, + { + name: "single successful reload", + results: []ExporterReloadResult{ + {ExporterType: "prometheus", Success: true, Message: "reloaded successfully"}, + }, + expectedTotal: 1, + expectedSuccessful: 1, + expectedFailed: 0, + expectedReloadableCount: 0, + }, + { + name: "single failed reload", + results: []ExporterReloadResult{ + {ExporterType: "kubernetes", Success: false, Error: errors.New("config invalid"), Message: "reload failed"}, + }, + expectedTotal: 1, + expectedSuccessful: 0, + expectedFailed: 1, + expectedReloadableCount: 0, + }, + { + name: "mixed results", + results: []ExporterReloadResult{ + {ExporterType: "prometheus", Success: true, Message: "reloaded successfully"}, + {ExporterType: "kubernetes", Success: false, Error: errors.New("config invalid")}, + {ExporterType: "http", Success: true, Message: "reloaded successfully"}, + {ExporterType: "logs", Success: false, Error: errors.New("not reloadable")}, + }, + expectedTotal: 4, + expectedSuccessful: 2, + expectedFailed: 2, + expectedReloadableCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + summary := &ExporterReloadSummary{ + ReloadableCount: tt.expectedReloadableCount, + } + + for _, result := range tt.results { + summary.AddResult(result) + } + + if summary.TotalExporters != tt.expectedTotal { + t.Errorf("TotalExporters = %d, want %d", summary.TotalExporters, tt.expectedTotal) + } + if summary.SuccessfulReloads != tt.expectedSuccessful { + t.Errorf("SuccessfulReloads = %d, want %d", summary.SuccessfulReloads, tt.expectedSuccessful) + } + if summary.FailedReloads != tt.expectedFailed { + t.Errorf("FailedReloads = %d, want %d", summary.FailedReloads, tt.expectedFailed) + } + if len(summary.Results) != len(tt.results) { + t.Errorf("Results count = %d, want %d", len(summary.Results), len(tt.results)) + } + }) + } +} + +// TestExporterReloadResult_Fields verifies ExporterReloadResult field assignments. +func TestExporterReloadResult_Fields(t *testing.T) { + err := errors.New("test error") + result := ExporterReloadResult{ + ExporterType: "prometheus", + Success: false, + Error: err, + Message: "config validation failed", + } + + if result.ExporterType != "prometheus" { + t.Errorf("ExporterType = %s, want prometheus", result.ExporterType) + } + if result.Success { + t.Error("Success should be false") + } + if result.Error != err { + t.Errorf("Error = %v, want %v", result.Error, err) + } + if result.Message != "config validation failed" { + t.Errorf("Message = %s, want 'config validation failed'", result.Message) + } +} + +// TestExporterReloadSummary_InitialState verifies initial state of ExporterReloadSummary. +func TestExporterReloadSummary_InitialState(t *testing.T) { + summary := ExporterReloadSummary{} + + if summary.TotalExporters != 0 { + t.Errorf("TotalExporters = %d, want 0", summary.TotalExporters) + } + if summary.ReloadableCount != 0 { + t.Errorf("ReloadableCount = %d, want 0", summary.ReloadableCount) + } + if summary.SuccessfulReloads != 0 { + t.Errorf("SuccessfulReloads = %d, want 0", summary.SuccessfulReloads) + } + if summary.FailedReloads != 0 { + t.Errorf("FailedReloads = %d, want 0", summary.FailedReloads) + } + if summary.Results != nil && len(summary.Results) != 0 { + t.Errorf("Results should be nil or empty, got %d items", len(summary.Results)) + } +} + +// TestExporterReloadSummary_ResultsPreserveOrder verifies results maintain insertion order. +func TestExporterReloadSummary_ResultsPreserveOrder(t *testing.T) { + summary := &ExporterReloadSummary{} + + results := []ExporterReloadResult{ + {ExporterType: "first"}, + {ExporterType: "second"}, + {ExporterType: "third"}, + } + + for _, r := range results { + summary.AddResult(r) + } + + for i, r := range results { + if summary.Results[i].ExporterType != r.ExporterType { + t.Errorf("Results[%d].ExporterType = %s, want %s", i, summary.Results[i].ExporterType, r.ExporterType) + } + } +} diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go index a3618ea..1824264 100644 --- a/pkg/types/types_test.go +++ b/pkg/types/types_test.go @@ -618,3 +618,210 @@ func TestNilPointerHandling(t *testing.T) { t.Error("expected GetMetadata on nil pointer to return empty string and false") } } + +// TestProblemWithMetadata_NilMetadataMap verifies WithMetadata handles nil Metadata map. +func TestProblemWithMetadata_NilMetadataMap(t *testing.T) { + // Create problem manually without initializing Metadata + problem := &Problem{ + Type: "test-type", + Resource: "test-resource", + Severity: ProblemWarning, + Message: "Test message", + DetectedAt: time.Now(), + Metadata: nil, // Explicitly nil + } + + // WithMetadata should initialize the map + problem.WithMetadata("key1", "value1") + + if problem.Metadata == nil { + t.Error("expected Metadata to be initialized") + } + if val, ok := problem.Metadata["key1"]; !ok || val != "value1" { + t.Errorf("expected Metadata[key1]=value1, got %v", problem.Metadata) + } +} + +// TestConditionValidation_EdgeCases verifies Condition validation edge cases. +func TestConditionValidation_EdgeCases(t *testing.T) { + tests := []struct { + name string + condition Condition + wantErr bool + errSubstr string + }{ + { + name: "missing reason", + condition: Condition{ + Type: "TestType", + Status: ConditionTrue, + Transition: time.Now(), + Message: "Test message", + }, + wantErr: true, + errSubstr: "reason", + }, + { + name: "missing message", + condition: Condition{ + Type: "TestType", + Status: ConditionTrue, + Transition: time.Now(), + Reason: "TestReason", + }, + wantErr: true, + errSubstr: "message", + }, + { + name: "missing transition time", + condition: Condition{ + Type: "TestType", + Status: ConditionTrue, + Reason: "TestReason", + Message: "Test message", + }, + wantErr: true, + errSubstr: "transition", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.condition.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil && tt.errSubstr != "" { + if !stringContains(err.Error(), tt.errSubstr) { + t.Errorf("Error message %q should contain %q", err.Error(), tt.errSubstr) + } + } + }) + } +} + +// TestStatusValidation_EdgeCases verifies Status validation edge cases. +func TestStatusValidation_EdgeCases(t *testing.T) { + tests := []struct { + name string + status *Status + wantErr bool + errSubstr string + }{ + { + name: "missing timestamp", + status: &Status{ + Source: "test-monitor", + Events: []Event{}, + Conditions: []Condition{}, + }, + wantErr: true, + errSubstr: "timestamp", + }, + { + name: "invalid condition in status", + status: &Status{ + Source: "test-monitor", + Timestamp: time.Now(), + Events: []Event{}, + Conditions: []Condition{ + {Type: "TestType"}, // missing required fields + }, + }, + wantErr: true, + errSubstr: "condition", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.status.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil && tt.errSubstr != "" { + if !stringContains(err.Error(), tt.errSubstr) { + t.Errorf("Error message %q should contain %q", err.Error(), tt.errSubstr) + } + } + }) + } +} + +// TestProblemValidation_EdgeCases verifies Problem validation edge cases. +func TestProblemValidation_EdgeCases(t *testing.T) { + tests := []struct { + name string + problem *Problem + wantErr bool + errSubstr string + }{ + { + name: "missing severity", + problem: &Problem{ + Type: "test-type", + Resource: "test-resource", + Message: "Test message", + DetectedAt: time.Now(), + }, + wantErr: true, + errSubstr: "severity", + }, + { + name: "missing message", + problem: &Problem{ + Type: "test-type", + Resource: "test-resource", + Severity: ProblemWarning, + DetectedAt: time.Now(), + }, + wantErr: true, + errSubstr: "message", + }, + { + name: "missing detected time", + problem: &Problem{ + Type: "test-type", + Resource: "test-resource", + Severity: ProblemWarning, + Message: "Test message", + }, + wantErr: true, + errSubstr: "detected", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.problem.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil && tt.errSubstr != "" { + if !stringContains(err.Error(), tt.errSubstr) { + t.Errorf("Error message %q should contain %q", err.Error(), tt.errSubstr) + } + } + }) + } +} + +// TestGetMetadata_NilMetadataMap verifies GetMetadata handles nil Metadata map. +func TestGetMetadata_NilMetadataMap(t *testing.T) { + problem := &Problem{ + Type: "test-type", + Resource: "test-resource", + Severity: ProblemWarning, + Message: "Test message", + DetectedAt: time.Now(), + Metadata: nil, // Explicitly nil + } + + val, ok := problem.GetMetadata("nonexistent") + if ok { + t.Error("expected ok=false for nil Metadata map") + } + if val != "" { + t.Errorf("expected empty string, got %q", val) + } +}