From d19c4f709c8a4713e29ce92e30b2347fa9a17934 Mon Sep 17 00:00:00 2001 From: Brian Ojeda <9335829+sgtoj@users.noreply.github.com> Date: Sat, 6 Dec 2025 19:10:43 -0500 Subject: [PATCH] feat: improve slack messages for syncs --- internal/app/app.go | 2 +- internal/notifiers/github_slack.go | 58 +++++++++++++++++------------- internal/okta/sync.go | 17 ++++++--- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index fc974bb..6d9b1d2 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -133,7 +133,7 @@ func (a *App) handleOktaSync(ctx context.Context) error { a.Logger.Info("okta sync completed", slog.Int("report_count", len(syncResult.Reports))) if a.Notifier != nil { - if err := a.Notifier.NotifyOktaSync(ctx, syncResult.Reports); err != nil { + if err := a.Notifier.NotifyOktaSync(ctx, syncResult.Reports, a.Config.GitHubOrg); err != nil { a.Logger.Warn("failed to send slack notification", slog.String("error", err.Error())) } } diff --git a/internal/notifiers/github_slack.go b/internal/notifiers/github_slack.go index f3cd659..f9d5b9f 100644 --- a/internal/notifiers/github_slack.go +++ b/internal/notifiers/github_slack.go @@ -93,7 +93,7 @@ func (s *SlackNotifier) NotifyPRBypass(ctx context.Context, result *github.PRCom } // NotifyOktaSync sends a Slack notification with Okta sync results. -func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.SyncReport) error { +func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.SyncReport, githubOrg string) error { if len(reports) == 0 { return nil } @@ -110,12 +110,14 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if report.HasChanges() { rulesWithChanges = append(rulesWithChanges, report) - } else { + } else if !report.HasErrors() { + // only list as "no changes" if it didn't fail entirely rulesWithoutChanges = append(rulesWithoutChanges, report) } for _, err := range report.Errors { - allErrors = append(allErrors, fmt.Sprintf("%s: %s", report.GitHubTeam, err)) + // use rule name as identifier since team/group may be empty on failure + allErrors = append(allErrors, fmt.Sprintf("%s: %s", report.Rule, err)) } allSkippedExternal = append(allSkippedExternal, report.MembersSkippedExternal...) @@ -124,36 +126,42 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync blocks := []slack.Block{ slack.NewHeaderBlock( - slack.NewTextBlockObject("plain_text", "Okta Group Sync Complete", false, false), + slack.NewTextBlockObject("plain_text", "Okta GitHub Team Sync Complete", false, false), ), } - // summary stats in fields - summaryFields := []*slack.TextBlockObject{ - slack.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Rules Processed*\n%d", len(reports)), false, false), + // summary stats (slack allows max 2 columns per row) + rulesProcessedFields := []*slack.TextBlockObject{ + slack.NewTextBlockObject("mrkdwn", "*Rules Processed*", false, false), + slack.NewTextBlockObject("mrkdwn", fmt.Sprintf("%d", len(reports)), false, false), + } + memberChangesFields := []*slack.TextBlockObject{ slack.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Members Added*\n%d", totalAdded), false, false), slack.NewTextBlockObject("mrkdwn", fmt.Sprintf("*Members Removed*\n%d", totalRemoved), false, false), } - blocks = append(blocks, slack.NewSectionBlock(nil, summaryFields, nil)) + blocks = append(blocks, slack.NewSectionBlock(nil, rulesProcessedFields, nil)) + blocks = append(blocks, slack.NewSectionBlock(nil, memberChangesFields, nil)) + + // helper to build team URL + teamURL := func(teamSlug string) string { + return fmt.Sprintf("https://github.com/orgs/%s/teams/%s", githubOrg, teamSlug) + } - // table of rules with changes + // list of rules with changes if len(rulesWithChanges) > 0 { blocks = append(blocks, slack.NewDividerBlock()) - tableText := "*Rules with changes:*\n" - tableText += "```\n" - tableText += fmt.Sprintf("%-32s %8s %8s\n", "Team", "Added", "Removed") + changesText := "*Rules With Changes*\n" for _, report := range rulesWithChanges { - teamName := report.GitHubTeam - if len(teamName) > 32 { - teamName = teamName[:29] + "..." - } - tableText += fmt.Sprintf("%-32s %8d %8d\n", teamName, len(report.MembersAdded), len(report.MembersRemoved)) + changesText += fmt.Sprintf("- <%s|%s> (+%d, -%d)\n", + teamURL(report.GitHubTeam), + report.GitHubTeam, + len(report.MembersAdded), + len(report.MembersRemoved)) } - tableText += "```" blocks = append(blocks, slack.NewSectionBlock( - slack.NewTextBlockObject("mrkdwn", tableText, false, false), + slack.NewTextBlockObject("mrkdwn", changesText, false, false), nil, nil, )) } @@ -162,9 +170,9 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if len(rulesWithoutChanges) > 0 { blocks = append(blocks, slack.NewDividerBlock()) - noChangesText := "*Rules with no changes:*\n" + noChangesText := "*Rules With No Changes*\n" for _, report := range rulesWithoutChanges { - noChangesText += fmt.Sprintf("- %s\n", report.GitHubTeam) + noChangesText += fmt.Sprintf("- <%s|%s>\n", teamURL(report.GitHubTeam), report.GitHubTeam) } blocks = append(blocks, slack.NewSectionBlock( @@ -177,7 +185,7 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if len(allErrors) > 0 { blocks = append(blocks, slack.NewDividerBlock()) - errorsText := "*Errors:*\n" + errorsText := "*Errors*\n" for _, err := range allErrors { errorsText += fmt.Sprintf("- %s\n", err) } @@ -192,10 +200,10 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if len(allSkippedExternal) > 0 || len(allSkippedNoGHUsername) > 0 { blocks = append(blocks, slack.NewDividerBlock()) - skippedText := "*Skipped members:*\n" + skippedText := "*Skipped Members*\n" if len(allSkippedExternal) > 0 { - skippedText += "_External collaborators:_\n" + skippedText += "_External Collaborators_\n" for _, member := range allSkippedExternal { skippedText += fmt.Sprintf("- %s\n", member) } @@ -205,7 +213,7 @@ func (s *SlackNotifier) NotifyOktaSync(ctx context.Context, reports []*okta.Sync if len(allSkippedExternal) > 0 { skippedText += "\n" } - skippedText += "_No GitHub username in Okta:_\n" + skippedText += "_No GitHub Username In Okta:_\n" for _, member := range allSkippedNoGHUsername { skippedText += fmt.Sprintf("- %s\n", member) } diff --git a/internal/okta/sync.go b/internal/okta/sync.go index 936cbdf..1333997 100644 --- a/internal/okta/sync.go +++ b/internal/okta/sync.go @@ -106,7 +106,7 @@ type SyncResult struct { // continues processing remaining rules even if some fail. func (s *Syncer) Sync(ctx context.Context) (*SyncResult, error) { var reports []*SyncReport - var syncErrors []string + var failedRuleCount int for _, rule := range s.rules { if !rule.IsEnabled() { @@ -115,19 +115,26 @@ func (s *Syncer) Sync(ctx context.Context) (*SyncResult, error) { ruleReports, err := s.syncRule(ctx, rule) if err != nil { - errMsg := fmt.Sprintf("rule '%s' failed: %v", rule.GetName(), err) - syncErrors = append(syncErrors, errMsg) + failedRuleCount++ s.logger.Error("sync rule failed", slog.String("rule", rule.GetName()), slog.String("error", err.Error())) + + // create a report for the failed rule so error is visible + reports = append(reports, &SyncReport{ + Rule: rule.GetName(), + OktaGroup: rule.OktaGroupName, + GitHubTeam: rule.GitHubTeamName, + Errors: []string{err.Error()}, + }) continue } reports = append(reports, ruleReports...) } - if len(syncErrors) > 0 && len(reports) == 0 { - return nil, errors.Newf("all sync rules failed: %d errors", len(syncErrors)) + if failedRuleCount > 0 && failedRuleCount == len(reports) { + return nil, errors.Newf("all sync rules failed: %d errors", failedRuleCount) } return &SyncResult{