From 4511f858694b7f373a7f79b8fc6d11f2543153c2 Mon Sep 17 00:00:00 2001 From: Christopher Mancini Date: Wed, 10 Dec 2025 15:52:44 -0500 Subject: [PATCH] [SDCICD-1704] add cluster id and expiration to message - add ability to use Slack API and attach log files, requires slack api token - falls back to webhook message - show all logs up to 150 lines in message payload - if greater than 150, show first 50 and last 100. Add test coverage. - add test coverage - fix lint and formatting errors --- internal/analysisengine/engine.go | 1 + internal/reporter/slack.go | 146 +++++- internal/reporter/slack_test.go | 434 ++++++++++++++++++ internal/sanitizer/sanitizer.go | 14 +- pkg/common/aws/iam.go | 3 +- .../healthchecks/clusterversionoperator.go | 4 - pkg/common/cluster/healthchecks/machines.go | 3 - pkg/common/config/config.go | 9 + pkg/common/slack/client.go | 162 +++++++ pkg/common/slack/client_test.go | 240 ++++++++++ pkg/e2e/e2e.go | 15 + 11 files changed, 1015 insertions(+), 16 deletions(-) create mode 100644 internal/reporter/slack_test.go create mode 100644 pkg/common/slack/client_test.go diff --git a/internal/analysisengine/engine.go b/internal/analysisengine/engine.go index e4c34dbd6c9..fa115db2ffc 100644 --- a/internal/analysisengine/engine.go +++ b/internal/analysisengine/engine.go @@ -30,6 +30,7 @@ type ClusterInfo struct { Region string CloudProvider string Version string + Expiration string } // Config holds configuration for the analysis engine diff --git a/internal/reporter/slack.go b/internal/reporter/slack.go index 773140cd9b9..80b2bc9dfbb 100644 --- a/internal/reporter/slack.go +++ b/internal/reporter/slack.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "fmt" + "os" + "path/filepath" "strings" commonslack "github.com/openshift/osde2e/pkg/common/slack" @@ -40,8 +42,29 @@ func (s *SlackReporter) Report(ctx context.Context, result *AnalysisResult, conf // Create simple message message := s.formatMessage(result, config) - // Send to Slack using common package - if err := s.client.SendWebhook(ctx, webhookURL, message); err != nil { + // Check if we have bot token for enhanced functionality + botToken, hasBotToken := config.Settings["bot_token"].(string) + channel, hasChannel := config.Settings["channel"].(string) + reportDir, hasReportDir := config.Settings["report_dir"].(string) + + // If we have bot token, use chat.postMessage with file attachments + if hasBotToken && botToken != "" && hasChannel && channel != "" && hasReportDir && reportDir != "" { + logFiles := s.collectLogFiles(reportDir) + if err := s.client.PostMessageWithFiles(ctx, botToken, channel, message, logFiles); err != nil { + // Fall back to webhook on error + fmt.Fprintf(os.Stderr, "Warning: Failed to post message with files, falling back to webhook: %v\n", err) + // Include stdout in fallback message since we can't attach files + messageWithStdout := s.formatMessageWithStdout(result, config) + if err := s.client.SendWebhook(ctx, webhookURL, messageWithStdout); err != nil { + return fmt.Errorf("failed to send to Slack: %w", err) + } + } + return nil + } + + // Fall back to webhook with stdout included (no files) + messageWithStdout := s.formatMessageWithStdout(result, config) + if err := s.client.SendWebhook(ctx, webhookURL, messageWithStdout); err != nil { return fmt.Errorf("failed to send to Slack: %w", err) } @@ -62,6 +85,25 @@ func (s *SlackReporter) formatMessage(result *AnalysisResult, config *ReporterCo summary := fmt.Sprintf("%s Pipeline Failed at E2E Test\n", statusEmoji) text := "" + // Add cluster information to summary + if clusterInfo, ok := config.Settings["cluster_info"].(*ClusterInfo); ok && clusterInfo != nil { + summary += "\n====== Cluster Information ======\n" + summary += fmt.Sprintf("Cluster ID: %s\n", clusterInfo.ID) + if clusterInfo.Expiration != "" { + summary += fmt.Sprintf("Expiration: %s\n", clusterInfo.Expiration) + } + if clusterInfo.Name != "" { + summary += fmt.Sprintf("Name: %s\n", clusterInfo.Name) + } + if clusterInfo.Version != "" { + summary += fmt.Sprintf("Version: %s\n", clusterInfo.Version) + } + if clusterInfo.Provider != "" { + summary += fmt.Sprintf("Provider: %s\n", clusterInfo.Provider) + } + summary += "\n" + } + if image, ok := config.Settings["image"].(string); ok && image != "" { imageInfo := strings.Split(image, ":") image := imageInfo[0] @@ -91,6 +133,21 @@ func (s *SlackReporter) formatMessage(result *AnalysisResult, config *ReporterCo return message } +// formatMessageWithStdout creates a message with stdout content included for webhook fallback +func (s *SlackReporter) formatMessageWithStdout(result *AnalysisResult, config *ReporterConfig) *Message { + message := s.formatMessage(result, config) + + // Add stdout content if report directory is available + if reportDir, ok := config.Settings["report_dir"].(string); ok && reportDir != "" { + if stdout := s.readTestOutput(reportDir); stdout != "" { + message.Analysis += "\n\n====== Test Pod Stdout ======\n" + message.Analysis += stdout + } + } + + return message +} + // formatAnalysisContent tries to parse JSON and format it nicely for Slack func (s *SlackReporter) formatAnalysisContent(content string) string { // Look for JSON content in code blocks @@ -143,6 +200,91 @@ func (s *SlackReporter) formatAnalysisContent(content string) string { return formatted.String() } +// collectLogFiles collects all log and XML files from the report directory +func (s *SlackReporter) collectLogFiles(reportDir string) []string { + var files []string + + err := filepath.Walk(reportDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil // Skip errors, continue walking + } + + if info.IsDir() { + return nil + } + + // Collect .log, .txt, and .xml files + ext := strings.ToLower(filepath.Ext(path)) + if ext == ".log" || ext == ".txt" || ext == ".xml" { + files = append(files, path) + } + + return nil + }) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: Error collecting log files: %v\n", err) + } + + return files +} + +// readTestOutput reads the test stdout from test_output.txt or test_output.log +func (s *SlackReporter) readTestOutput(reportDir string) string { + // Try test_output.txt first (main pod stdout), then test_output.log + for _, filename := range []string{"test_output.txt", "test_output.log"} { + filePath := filepath.Join(reportDir, filename) + if content, err := os.ReadFile(filepath.Clean(filePath)); err == nil { + lines := strings.Split(string(content), "\n") + totalLines := len(lines) + + // If content is small enough, return it all + if totalLines <= 150 { + return string(content) + } + + // Show first 50 lines (context) and last 100 lines (failure details) + firstN := 50 + lastN := 100 + + var result strings.Builder + + // First N lines + for i := 0; i < firstN && i < totalLines; i++ { + result.WriteString(lines[i]) + result.WriteString("\n") + } + + // Omission notice + omitted := totalLines - firstN - lastN + result.WriteString(fmt.Sprintf("\n... (%d lines omitted) ...\n\n", omitted)) + + // Last N lines + startIdx := totalLines - lastN + if startIdx < firstN { + startIdx = firstN + } + for i := startIdx; i < totalLines; i++ { + result.WriteString(lines[i]) + result.WriteString("\n") + } + + return result.String() + } + } + return "" +} + +// ClusterInfo holds cluster information for reporting (mirrored from analysisengine to avoid import cycle) +type ClusterInfo struct { + ID string + Name string + Provider string + Region string + CloudProvider string + Version string + Expiration string +} + // SlackReporterConfig creates a reporter config for Slack func SlackReporterConfig(webhookURL string, enabled bool) ReporterConfig { return ReporterConfig{ diff --git a/internal/reporter/slack_test.go b/internal/reporter/slack_test.go new file mode 100644 index 00000000000..9f214ea22c3 --- /dev/null +++ b/internal/reporter/slack_test.go @@ -0,0 +1,434 @@ +package reporter + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSlackReporter_readTestOutput(t *testing.T) { + tests := []struct { + name string + setupFunc func(t *testing.T) string + shouldContain []string + shouldNotContain []string + expectedEmpty bool + expectedTruncated bool + }{ + { + name: "returns empty string when directory does not exist", + setupFunc: func(t *testing.T) string { + return "/nonexistent/directory" + }, + expectedEmpty: true, + }, + { + name: "returns empty string when no test output files exist", + setupFunc: func(t *testing.T) string { + return t.TempDir() + }, + expectedEmpty: true, + }, + { + name: "returns full content for small file (100 lines)", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + content := generateLines(100) + err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(content), 0o644) + if err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + shouldContain: []string{"line 1", "line 50", "line 100"}, + shouldNotContain: []string{"lines omitted"}, + expectedTruncated: false, + }, + { + name: "returns full content for file with exactly 150 lines", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + content := generateLines(150) + err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(content), 0o644) + if err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + // With 150 lines + trailing newline = 151 elements after split, which is > 150, so it gets truncated + shouldContain: []string{"line 1", "line 50", "line 52", "line 150", "lines omitted"}, + expectedTruncated: true, + }, + { + name: "truncates large file with first 50 and last 100 lines", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + content := generateLines(500) + err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(content), 0o644) + if err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + shouldContain: []string{"line 1", "line 50", "line 402", "line 500", "(351 lines omitted)"}, + shouldNotContain: []string{"line 51", "line 401"}, + expectedTruncated: true, + }, + { + name: "prefers test_output.txt over test_output.log", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + txtContent := "content from txt file\n" + logContent := "content from log file\n" + if err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(txtContent), 0o644); err != nil { + t.Fatalf("failed to create txt file: %v", err) + } + if err := os.WriteFile(filepath.Join(tmpDir, "test_output.log"), []byte(logContent), 0o644); err != nil { + t.Fatalf("failed to create log file: %v", err) + } + return tmpDir + }, + shouldContain: []string{"content from txt file"}, + shouldNotContain: []string{"content from log file"}, + }, + { + name: "falls back to test_output.log when test_output.txt does not exist", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + logContent := "content from log file\n" + if err := os.WriteFile(filepath.Join(tmpDir, "test_output.log"), []byte(logContent), 0o644); err != nil { + t.Fatalf("failed to create log file: %v", err) + } + return tmpDir + }, + shouldContain: []string{"content from log file"}, + }, + { + name: "handles file with 151 lines correctly", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + content := generateLines(151) + err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(content), 0o644) + if err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + // 151 lines + trailing newline = 152 elements; first 50 + last 100 = 150, omit 2 + shouldContain: []string{"line 1", "line 50", "line 53", "line 151", "(2 lines omitted)"}, + shouldNotContain: []string{"line 51", "line 52"}, + expectedTruncated: true, + }, + { + name: "handles empty file", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + if err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(""), 0o644); err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + expectedEmpty: true, + }, + { + name: "handles file with single line", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + content := "single line\n" + if err := os.WriteFile(filepath.Join(tmpDir, "test_output.txt"), []byte(content), 0o644); err != nil { + t.Fatalf("failed to create test file: %v", err) + } + return tmpDir + }, + shouldContain: []string{"single line"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reporter := &SlackReporter{} + reportDir := tt.setupFunc(t) + result := reporter.readTestOutput(reportDir) + + if tt.expectedEmpty { + if result != "" { + t.Errorf("expected empty string, got: %q", result) + } + return + } + + for _, expected := range tt.shouldContain { + if !strings.Contains(result, expected) { + t.Errorf("expected result to contain %q, but it didn't", expected) + } + } + + for _, notExpected := range tt.shouldNotContain { + if strings.Contains(result, notExpected) { + t.Errorf("expected result to NOT contain %q, but it did", notExpected) + } + } + + if tt.expectedTruncated && !strings.Contains(result, "lines omitted") { + t.Errorf("expected result to be truncated with omission notice") + } + }) + } +} + +func TestSlackReporter_Name(t *testing.T) { + reporter := &SlackReporter{} + if got := reporter.Name(); got != "slack" { + t.Errorf("expected name to be 'slack', got %q", got) + } +} + +func TestSlackReporter_formatAnalysisContent(t *testing.T) { + tests := []struct { + name string + input string + shouldContain []string + shouldBeEmpty bool + }{ + { + name: "returns empty string for content without JSON block", + input: "This is just plain text without JSON", + shouldBeEmpty: true, + }, + { + name: "returns empty string for invalid JSON", + input: "```json\n{invalid json content\n```", + shouldBeEmpty: true, + }, + { + name: "formats valid JSON with root_cause and recommendations", + input: "Analysis result:\n```json\n{\n \"root_cause\": \"Database connection timeout\",\n \"recommendations\": [\"Check network connectivity\", \"Verify database credentials\"]\n}\n```", + shouldContain: []string{ + "====== 🔍 Possible Cause ======", + "Database connection timeout", + "====== 💡 Recommendations ======", + "1. Check network connectivity", + "2. Verify database credentials", + }, + }, + { + name: "handles JSON with only root_cause", + input: "```json\n{\"root_cause\": \"Memory exhausted\"}\n```", + shouldContain: []string{ + "====== 🔍 Possible Cause ======", + "Memory exhausted", + }, + }, + { + name: "handles JSON with only recommendations", + input: "```json\n{\"recommendations\": [\"Restart service\", \"Check logs\"]}\n```", + shouldContain: []string{ + "====== 💡 Recommendations ======", + "1. Restart service", + "2. Check logs", + }, + }, + { + name: "handles empty root_cause gracefully", + input: "```json\n{\"root_cause\": \"\", \"recommendations\": [\"Action item\"]}\n```", + shouldContain: []string{ + "====== 💡 Recommendations ======", + "1. Action item", + }, + }, + { + name: "handles empty recommendations array", + input: "```json\n{\"root_cause\": \"Error found\", \"recommendations\": []}\n```", + shouldContain: []string{ + "====== 🔍 Possible Cause ======", + "Error found", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reporter := &SlackReporter{} + result := reporter.formatAnalysisContent(tt.input) + + if tt.shouldBeEmpty { + if result != "" { + t.Errorf("expected empty result, got: %q", result) + } + return + } + + for _, expected := range tt.shouldContain { + if !strings.Contains(result, expected) { + t.Errorf("expected result to contain %q, but it didn't. Result: %s", expected, result) + } + } + }) + } +} + +func TestSlackReporter_collectLogFiles(t *testing.T) { + tests := []struct { + name string + setupFunc func(t *testing.T) string + expectedCount int + expectedExts []string + }{ + { + name: "collects all log, txt, and xml files", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + files := []string{"test.log", "output.txt", "results.xml", "ignore.json", "data.csv"} + for _, file := range files { + if err := os.WriteFile(filepath.Join(tmpDir, file), []byte("content"), 0o644); err != nil { + t.Fatalf("failed to create file: %v", err) + } + } + return tmpDir + }, + expectedCount: 3, + expectedExts: []string{".log", ".txt", ".xml"}, + }, + { + name: "collects files from subdirectories", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + subDir := filepath.Join(tmpDir, "logs") + if err := os.MkdirAll(subDir, 0o755); err != nil { + t.Fatalf("failed to create subdir: %v", err) + } + files := map[string]string{ + "test.log": "content", + "logs/nested.log": "content", + "logs/results.xml": "content", + } + for file, content := range files { + if err := os.WriteFile(filepath.Join(tmpDir, file), []byte(content), 0o644); err != nil { + t.Fatalf("failed to create file: %v", err) + } + } + return tmpDir + }, + expectedCount: 3, + expectedExts: []string{".log", ".xml"}, + }, + { + name: "returns empty slice for directory with no matching files", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + if err := os.WriteFile(filepath.Join(tmpDir, "test.json"), []byte("{}"), 0o644); err != nil { + t.Fatalf("failed to create file: %v", err) + } + return tmpDir + }, + expectedCount: 0, + }, + { + name: "returns empty slice for empty directory", + setupFunc: func(t *testing.T) string { + return t.TempDir() + }, + expectedCount: 0, + }, + { + name: "handles case-insensitive extensions", + setupFunc: func(t *testing.T) string { + tmpDir := t.TempDir() + files := []string{"test.LOG", "output.TXT", "results.XML"} + for _, file := range files { + if err := os.WriteFile(filepath.Join(tmpDir, file), []byte("content"), 0o644); err != nil { + t.Fatalf("failed to create file: %v", err) + } + } + return tmpDir + }, + expectedCount: 3, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reporter := &SlackReporter{} + reportDir := tt.setupFunc(t) + result := reporter.collectLogFiles(reportDir) + + if len(result) != tt.expectedCount { + t.Errorf("expected %d files, got %d", tt.expectedCount, len(result)) + } + + if tt.expectedExts != nil { + for _, file := range result { + ext := strings.ToLower(filepath.Ext(file)) + found := false + for _, expectedExt := range tt.expectedExts { + if ext == expectedExt { + found = true + break + } + } + if !found { + t.Errorf("unexpected file extension %q in result", ext) + } + } + } + }) + } +} + +func TestSlackReporterConfig(t *testing.T) { + tests := []struct { + name string + webhookURL string + enabled bool + }{ + { + name: "creates config with enabled reporter", + webhookURL: "https://hooks.slack.com/services/TEST/WEBHOOK/URL", + enabled: true, + }, + { + name: "creates config with disabled reporter", + webhookURL: "https://hooks.slack.com/services/TEST/WEBHOOK/URL", + enabled: false, + }, + { + name: "handles empty webhook URL", + webhookURL: "", + enabled: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := SlackReporterConfig(tt.webhookURL, tt.enabled) + + if config.Type != "slack" { + t.Errorf("expected type 'slack', got %q", config.Type) + } + + if config.Enabled != tt.enabled { + t.Errorf("expected enabled to be %v, got %v", tt.enabled, config.Enabled) + } + + webhookURL, ok := config.Settings["webhook_url"].(string) + if !ok { + t.Fatal("webhook_url setting is not a string") + } + + if webhookURL != tt.webhookURL { + t.Errorf("expected webhook_url to be %q, got %q", tt.webhookURL, webhookURL) + } + }) + } +} + +// generateLines creates N lines of test content +func generateLines(n int) string { + lines := make([]string, n) + for i := 0; i < n; i++ { + lines[i] = fmt.Sprintf("line %d", i+1) + } + return strings.Join(lines, "\n") + "\n" +} diff --git a/internal/sanitizer/sanitizer.go b/internal/sanitizer/sanitizer.go index d9d5df7efa1..1e5f9b804b2 100644 --- a/internal/sanitizer/sanitizer.go +++ b/internal/sanitizer/sanitizer.go @@ -163,12 +163,14 @@ func (s *Sanitizer) sanitizeContent(content, source string, timestamp time.Time) // Perform audit logging asynchronously to avoid blocking if s.auditLog != nil && (matchCount > 0 || !s.config.SkipAuditOnNoMatch) { - go s.auditLog.Log(AuditEntry{ - Timestamp: timestamp, - Source: source, - RulesApplied: rulesApplied, - MatchCount: matchCount, - }) + go func() { + _ = s.auditLog.Log(AuditEntry{ + Timestamp: timestamp, + Source: source, + RulesApplied: rulesApplied, + MatchCount: matchCount, + }) + }() } return result, nil diff --git a/pkg/common/aws/iam.go b/pkg/common/aws/iam.go index b4ed8530c78..fa1cd8e04f0 100644 --- a/pkg/common/aws/iam.go +++ b/pkg/common/aws/iam.go @@ -1,6 +1,7 @@ package aws import ( + "errors" "fmt" "log" "regexp" @@ -151,7 +152,7 @@ func (CcsAwsSession *ccsAwsSession) CleanupRoles(activeClusters map[string]bool, if sendSummary && errorBuilder.Len() < config.SlackMessageLength { errorBuilder.WriteString(errorMsg) } - return fmt.Errorf("%s", errorMsg) + return errors.New(errorMsg) } *deletedCounter++ fmt.Println("Removed") diff --git a/pkg/common/cluster/healthchecks/clusterversionoperator.go b/pkg/common/cluster/healthchecks/clusterversionoperator.go index 97e9c3562f1..fae4155d747 100644 --- a/pkg/common/cluster/healthchecks/clusterversionoperator.go +++ b/pkg/common/cluster/healthchecks/clusterversionoperator.go @@ -2,7 +2,6 @@ package healthchecks import ( "context" - "fmt" "log" v1 "github.com/openshift/api/config/v1" @@ -29,8 +28,6 @@ func CheckCVOReadiness(configClient configclient.ConfigV1Interface, logger *log. return false, err } - var metadataState []string - for _, v := range cvInfo.Status.Conditions { switch v.Type { case "Available": @@ -48,7 +45,6 @@ func CheckCVOReadiness(configClient configclient.ConfigV1Interface, logger *log. continue } } - metadataState = append(metadataState, fmt.Sprintf("%v", v)) logger.Printf("CVO State not complete: %v: %v %v", v.Type, v.Status, v.Message) success = false } diff --git a/pkg/common/cluster/healthchecks/machines.go b/pkg/common/cluster/healthchecks/machines.go index 6006321c0ff..ca9a9c97a9a 100644 --- a/pkg/common/cluster/healthchecks/machines.go +++ b/pkg/common/cluster/healthchecks/machines.go @@ -35,8 +35,6 @@ func CheckMachinesObjectState(dynamicClient dynamic.Interface, logger *log.Logge return false, fmt.Errorf("no machines found in the %s namespace", machinesNamespace) } - var metadataState []string - for _, item := range obj.Items { var machine machineapi.Machine err = runtime.DefaultUnstructuredConverter. @@ -46,7 +44,6 @@ func CheckMachinesObjectState(dynamicClient dynamic.Interface, logger *log.Logge } if machine.Status.Phase == nil || *machine.Status.Phase != runningPhase { - metadataState = append(metadataState, fmt.Sprintf("%v", machine)) logger.Printf("machine %s not ready", machine.Name) } } diff --git a/pkg/common/config/config.go b/pkg/common/config/config.go index bb75bd63aad..f1052218151 100644 --- a/pkg/common/config/config.go +++ b/pkg/common/config/config.go @@ -633,12 +633,17 @@ var LogAnalysis = struct { // SlackChannel is the default Slack channel for OSDE2E notifications // Env: LOG_ANALYSIS_SLACK_CHANNEL SlackChannel string + + // SlackBotToken is the Slack bot token for enhanced features (file attachments) + // Env: LOG_ANALYSIS_SLACK_BOT_TOKEN + SlackBotToken string }{ EnableAnalysis: "logAnalysis.enableAnalysis", APIKey: "logAnalysis.apiKey", Model: "logAnalysis.model", SlackWebhook: "logAnalysis.slackWebhook", SlackChannel: "logAnalysis.slackChannel", + SlackBotToken: "logAnalysis.slackBotToken", } func InitOSDe2eViper() { @@ -947,6 +952,10 @@ func InitOSDe2eViper() { viper.SetDefault(LogAnalysis.SlackChannel, defaultNotificationsChannel) _ = viper.BindEnv(LogAnalysis.SlackChannel, "LOG_ANALYSIS_SLACK_CHANNEL") + + viper.SetDefault(LogAnalysis.SlackBotToken, "") + _ = viper.BindEnv(LogAnalysis.SlackBotToken, "LOG_ANALYSIS_SLACK_BOT_TOKEN") + RegisterSecret(LogAnalysis.SlackBotToken, "slack-bot-token") } func init() { diff --git a/pkg/common/slack/client.go b/pkg/common/slack/client.go index e53b27fee9c..e04936f4d82 100644 --- a/pkg/common/slack/client.go +++ b/pkg/common/slack/client.go @@ -5,7 +5,11 @@ import ( "context" "encoding/json" "fmt" + "io" + "mime/multipart" "net/http" + "os" + "path/filepath" "time" ) @@ -83,3 +87,161 @@ func SendWebhook(ctx context.Context, webhookURL string, payload interface{}) er client := NewClient() return client.SendWebhook(ctx, webhookURL, payload) } + +// PostMessageWithFiles posts a message to Slack with file attachments using chat.postMessage + files.upload +// This provides a richer experience than webhooks by attaching files to the message +func (c *Client) PostMessageWithFiles(ctx context.Context, botToken string, channel string, message interface{}, filePaths []string) error { + // First, post the message to get a timestamp + ts, err := c.postMessage(ctx, botToken, channel, message) + if err != nil { + return fmt.Errorf("failed to post message: %w", err) + } + + // Then upload files as threaded replies to that message + if len(filePaths) > 0 { + for _, filePath := range filePaths { + if err := c.uploadFileToThread(ctx, botToken, channel, ts, filePath); err != nil { + // Log error but continue with other files + fmt.Fprintf(os.Stderr, "Warning: Failed to upload %s: %v\n", filePath, err) + } + } + } + + return nil +} + +// postMessage posts a message using chat.postMessage and returns the message timestamp +func (c *Client) postMessage(ctx context.Context, botToken string, channel string, message interface{}) (string, error) { + // Extract text content from message + var text string + if msg, ok := message.(map[string]interface{}); ok { + if summary, ok := msg["summary"].(string); ok { + text = summary + } + if analysis, ok := msg["analysis"].(string); ok { + if text != "" { + text += "\n\n" + } + text += analysis + } + } + + // Create payload for chat.postMessage + payload := map[string]interface{}{ + "channel": channel, + "text": text, + } + + jsonData, err := json.Marshal(payload) + if err != nil { + return "", fmt.Errorf("failed to marshal payload: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", "https://slack.com/api/chat.postMessage", bytes.NewBuffer(jsonData)) + if err != nil { + return "", fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Content-Type", "application/json; charset=utf-8") + req.Header.Set("Authorization", "Bearer "+botToken) + + client := &http.Client{Timeout: c.timeout} + resp, err := client.Do(req) + if err != nil { + return "", fmt.Errorf("HTTP request failed: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return "", fmt.Errorf("failed to read response: %w", err) + } + + var result map[string]interface{} + if err := json.Unmarshal(respBody, &result); err != nil { + return "", fmt.Errorf("failed to parse response: %w", err) + } + + if ok, exists := result["ok"].(bool); !exists || !ok { + if errMsg, exists := result["error"].(string); exists { + return "", fmt.Errorf("slack API error: %s", errMsg) + } + return "", fmt.Errorf("slack API returned ok=false") + } + + // Extract timestamp + ts, _ := result["ts"].(string) + return ts, nil +} + +// uploadFileToThread uploads a file to a Slack thread +func (c *Client) uploadFileToThread(ctx context.Context, botToken string, channel string, threadTs string, filePath string) error { + file, err := os.Open(filepath.Clean(filePath)) + if err != nil { + return fmt.Errorf("failed to open file: %w", err) + } + defer file.Close() + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + + // Add file field + part, err := writer.CreateFormFile("file", filepath.Base(filePath)) + if err != nil { + return fmt.Errorf("failed to create form file: %w", err) + } + + if _, err := io.Copy(part, file); err != nil { + return fmt.Errorf("failed to copy file content: %w", err) + } + + // Add channel field + if err := writer.WriteField("channels", channel); err != nil { + return fmt.Errorf("failed to write channel field: %w", err) + } + + // Add thread_ts to attach to message thread + if threadTs != "" { + if err := writer.WriteField("thread_ts", threadTs); err != nil { + return fmt.Errorf("failed to write thread_ts field: %w", err) + } + } + + if err := writer.Close(); err != nil { + return fmt.Errorf("failed to close multipart writer: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", "https://slack.com/api/files.upload", body) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Content-Type", writer.FormDataContentType()) + req.Header.Set("Authorization", "Bearer "+botToken) + + client := &http.Client{Timeout: c.timeout} + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("HTTP request failed: %w", err) + } + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("failed to read response: %w", err) + } + + var result map[string]interface{} + if err := json.Unmarshal(respBody, &result); err != nil { + return fmt.Errorf("failed to parse response: %w", err) + } + + if ok, exists := result["ok"].(bool); !exists || !ok { + if errMsg, exists := result["error"].(string); exists { + return fmt.Errorf("slack API error: %s", errMsg) + } + return fmt.Errorf("slack API returned ok=false") + } + + return nil +} diff --git a/pkg/common/slack/client_test.go b/pkg/common/slack/client_test.go new file mode 100644 index 00000000000..3e637880fe9 --- /dev/null +++ b/pkg/common/slack/client_test.go @@ -0,0 +1,240 @@ +package common + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func TestNewClient(t *testing.T) { + client := NewClient() + if client == nil { + t.Fatal("expected non-nil client") + } + if client.timeout != DefaultTimeout { + t.Errorf("expected timeout to be %v, got %v", DefaultTimeout, client.timeout) + } +} + +func TestNewClientWithTimeout(t *testing.T) { + tests := []struct { + name string + timeout time.Duration + }{ + { + name: "custom timeout 10 seconds", + timeout: 10 * time.Second, + }, + { + name: "custom timeout 1 minute", + timeout: 1 * time.Minute, + }, + { + name: "custom timeout 5 seconds", + timeout: 5 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewClientWithTimeout(tt.timeout) + if client == nil { + t.Fatal("expected non-nil client") + } + if client.timeout != tt.timeout { + t.Errorf("expected timeout to be %v, got %v", tt.timeout, client.timeout) + } + }) + } +} + +func TestClient_SendWebhook(t *testing.T) { + tests := []struct { + name string + payload interface{} + serverResponse func(w http.ResponseWriter, r *http.Request) + expectError bool + errorContains string + }{ + { + name: "successful webhook with map payload", + payload: map[string]string{ + "text": "Test message", + }, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST method, got %s", r.Method) + } + if ct := r.Header.Get("Content-Type"); ct != "application/json; charset=utf-8" { + t.Errorf("expected Content-Type application/json; charset=utf-8, got %s", ct) + } + if ua := r.Header.Get("User-Agent"); ua != "osde2e/1.0" { + t.Errorf("expected User-Agent osde2e/1.0, got %s", ua) + } + w.WriteHeader(http.StatusOK) + }, + expectError: false, + }, + { + name: "successful webhook with struct payload", + payload: struct { + Text string `json:"text"` + }{ + Text: "Test message", + }, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }, + expectError: false, + }, + { + name: "server returns error status", + payload: map[string]string{"text": "Test"}, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusBadRequest) + }, + expectError: true, + errorContains: "status 400", + }, + { + name: "server returns 500", + payload: map[string]string{"text": "Test"}, + serverResponse: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }, + expectError: true, + errorContains: "status 500", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(tt.serverResponse)) + defer server.Close() + + client := NewClient() + err := client.SendWebhook(context.Background(), server.URL, tt.payload) + + if tt.expectError && err == nil { + t.Error("expected error but got none") + } + if !tt.expectError && err != nil { + t.Errorf("unexpected error: %v", err) + } + if tt.errorContains != "" && err != nil { + if !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("expected error to contain %q, got %q", tt.errorContains, err.Error()) + } + } + }) + } +} + +func TestClient_SendMessage(t *testing.T) { + tests := []struct { + name string + message string + expectError bool + }{ + { + name: "sends simple text message", + message: "Hello, Slack!", + expectError: false, + }, + { + name: "sends empty message", + message: "", + expectError: false, + }, + { + name: "sends message with special characters", + message: "Test with emoji 🚀 and symbols @#$%", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var receivedBody map[string]interface{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewDecoder(r.Body).Decode(&receivedBody) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := NewClient() + err := client.SendMessage(context.Background(), server.URL, tt.message) + + if tt.expectError && err == nil { + t.Error("expected error but got none") + } + if !tt.expectError && err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +func TestSendWebhook_PackageLevel(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + payload := map[string]string{"text": "Test message"} + err := SendWebhook(context.Background(), server.URL, payload) + if err != nil { + t.Errorf("unexpected error from package-level SendWebhook: %v", err) + } +} + +func TestClient_SendWebhook_InvalidPayload(t *testing.T) { + client := NewClient() + + // Create an invalid payload that cannot be marshaled to JSON + invalidPayload := make(chan int) // channels cannot be marshaled to JSON + + err := client.SendWebhook(context.Background(), "http://example.com", invalidPayload) + + if err == nil { + t.Error("expected error for invalid payload, got none") + } + if !strings.Contains(err.Error(), "marshal") { + t.Errorf("expected error to contain 'marshal', got: %v", err) + } +} + +func TestClient_SendWebhook_ContextCancellation(t *testing.T) { + // Create a server that delays the response + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(100 * time.Millisecond) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + // Create a context that's already cancelled + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + client := NewClient() + err := client.SendWebhook(ctx, server.URL, map[string]string{"text": "test"}) + + if err == nil { + t.Error("expected error for cancelled context, got none") + } +} + +func TestClient_SendWebhook_InvalidURL(t *testing.T) { + client := NewClient() + + // Use an invalid URL that will cause request creation to fail + err := client.SendWebhook(context.Background(), "://invalid-url", map[string]string{"text": "test"}) + + if err == nil { + t.Error("expected error for invalid URL, got none") + } +} diff --git a/pkg/e2e/e2e.go b/pkg/e2e/e2e.go index 2c50ec70a3c..3f0a75b4872 100644 --- a/pkg/e2e/e2e.go +++ b/pkg/e2e/e2e.go @@ -46,6 +46,14 @@ func runLogAnalysis(ctx context.Context, err error) { return } + // Get cluster expiration timestamp + clusterExpiration := "" + if provider != nil { + if cluster, err := provider.GetCluster(viper.GetString(config.Cluster.ID)); err == nil { + clusterExpiration = cluster.ExpirationTimestamp().Format("2006-01-02 15:04:05 MST") + } + } + clusterInfo := &analysisengine.ClusterInfo{ ID: viper.GetString(config.Cluster.ID), Name: viper.GetString(config.Cluster.Name), @@ -53,6 +61,7 @@ func runLogAnalysis(ctx context.Context, err error) { Region: viper.GetString(config.CloudProvider.Region), CloudProvider: viper.GetString(config.CloudProvider.CloudProviderID), Version: viper.GetString(config.Cluster.Version), + Expiration: clusterExpiration, } // Setup notification config - composable approach for multiple reporters @@ -63,9 +72,15 @@ func runLogAnalysis(ctx context.Context, err error) { enableSlackNotify := viper.GetBool(config.Tests.EnableSlackNotify) slackWebhook := viper.GetString(config.LogAnalysis.SlackWebhook) defaultChannel := viper.GetString(config.LogAnalysis.SlackChannel) + slackBotToken := viper.GetString(config.LogAnalysis.SlackBotToken) if enableSlackNotify && slackWebhook != "" && defaultChannel != "" { slackConfig := reporter.SlackReporterConfig(slackWebhook, true) slackConfig.Settings["channel"] = defaultChannel + slackConfig.Settings["cluster_info"] = clusterInfo + slackConfig.Settings["report_dir"] = reportDir + if slackBotToken != "" { + slackConfig.Settings["bot_token"] = slackBotToken + } reporters = append(reporters, slackConfig) }