From b6d00d39c2cc38c8f8e19c5f6d72436c6db8255d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 15:06:56 +0000 Subject: [PATCH 1/2] Add --log-level flag with info success messages per org Agent-Logs-Url: https://github.com/CallMeGreg/gh-security-config/sessions/1892a9d2-3d8b-41c7-8d5a-effca9f37a2e Co-authored-by: CallMeGreg <110078080+CallMeGreg@users.noreply.github.com> --- README.md | 1 + cmd/apply.go | 2 +- cmd/delete.go | 2 +- cmd/modify.go | 2 +- cmd/root.go | 17 ++++++ internal/processors/concurrent.go | 2 + internal/processors/sequential.go | 2 + internal/ui/log.go | 94 +++++++++++++++++++++++++++++++ internal/ui/log_test.go | 48 ++++++++++++++++ 9 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 internal/ui/log.go create mode 100644 internal/ui/log_test.go diff --git a/README.md b/README.md index ba0af3a..afdf621 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,7 @@ These flags are available on all commands: - **`--dependabot-security-updates-available string`** (`-s`) - Whether Dependabot Security Updates are available in your GHES instance (true/false) - **`--config-name string`** (`-n`) - Name of the security configuration to operate on. Replaces the interactive configuration-name prompt for each command (the meaning is command-specific: the name to create in `generate`, the name to select in `apply`/`delete`/`modify`, or the name of the source config in `generate --copy-from-org`). - **`--skip-confirmation-message string`** - Automatically approve the final confirmation prompt for any command (`true`/`false`). +- **`--log-level string`** - Minimum log level for output (`info`, `warning`, `error`; default: `warning`). When set to `info`, a success message is printed for each organization that is processed successfully. #### `generate` Command Flags diff --git a/cmd/apply.go b/cmd/apply.go index c9c7f7c..005207a 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -345,7 +345,7 @@ func runApply(cmd *cobra.Command, args []string) error { "config-source": targetType, "scope": scope, "set-as-default": fmt.Sprintf("%t", setAsDefault), - "skip-confirmation-message": fmt.Sprintf("%t", force), + "skip-confirmation-message": fmt.Sprintf("%t", force), } // Add org targeting flags diff --git a/cmd/delete.go b/cmd/delete.go index f6dee96..941727d 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -217,7 +217,7 @@ func runDelete(cmd *cobra.Command, args []string) error { "concurrency": commonFlags.Concurrency, "delay": commonFlags.Delay, "config-name": configName, - "skip-confirmation-message": fmt.Sprintf("%t", force), + "skip-confirmation-message": fmt.Sprintf("%t", force), } // Add org targeting flags diff --git a/cmd/modify.go b/cmd/modify.go index ffe35a9..3f5f237 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -338,7 +338,7 @@ func runModify(cmd *cobra.Command, args []string) error { "secret-scanning-push-protection": fmt.Sprintf("%v", newSettings["secret_scanning_push_protection"]), "secret-scanning-non-provider-patterns": fmt.Sprintf("%v", newSettings["secret_scanning_non_provider_patterns"]), "enforcement": fmt.Sprintf("%v", newSettings["enforcement"]), - "skip-confirmation-message": fmt.Sprintf("%t", force), + "skip-confirmation-message": fmt.Sprintf("%t", force), } if v, ok := newSettings["dependabot_alerts"]; ok { replicationFlags["dependabot-alerts"] = fmt.Sprintf("%v", v) diff --git a/cmd/root.go b/cmd/root.go index 892ba06..173ca92 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,10 +1,14 @@ package cmd import ( + "fmt" "os" + "strings" "github.com/pterm/pterm" "github.com/spf13/cobra" + + "github.com/callmegreg/gh-security-config/internal/ui" ) var rootCmd = &cobra.Command{ @@ -14,6 +18,18 @@ var rootCmd = &cobra.Command{ CompletionOptions: cobra.CompletionOptions{ HiddenDefaultCmd: true, }, + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + levelStr, err := cmd.Flags().GetString("log-level") + if err != nil { + return err + } + level, err := ui.ParseLogLevel(levelStr) + if err != nil { + return err + } + ui.SetLogLevel(level) + return nil + }, } func init() { @@ -33,6 +49,7 @@ func init() { // Flags shared by all subcommands rootCmd.PersistentFlags().StringP("config-name", "n", "", "Name of the security configuration to operate on (replaces the interactive configuration-name prompt for each command)") rootCmd.PersistentFlags().String("skip-confirmation-message", "", "Automatically approve the final confirmation prompt for any command (true/false)") + rootCmd.PersistentFlags().String("log-level", ui.LogLevelDefault, fmt.Sprintf("Minimum log level for output (%s)", strings.Join(ui.LogLevelValues, ", "))) // Mark org targeting flags as mutually exclusive rootCmd.MarkFlagsMutuallyExclusive("org", "org-list", "all-orgs") diff --git a/internal/processors/concurrent.go b/internal/processors/concurrent.go index 4cbd08b..0d70bfb 100644 --- a/internal/processors/concurrent.go +++ b/internal/processors/concurrent.go @@ -8,6 +8,7 @@ import ( "github.com/pterm/pterm" "github.com/callmegreg/gh-security-config/internal/types" + "github.com/callmegreg/gh-security-config/internal/ui" ) // ConcurrentProcessor handles concurrent organization processing @@ -78,6 +79,7 @@ func (cp *ConcurrentProcessor) Process() (successCount, skippedCount, errorCount if result.Success { cp.successCount++ + ui.LogOrgSuccess(result.Organization) } else if result.Skipped { cp.skippedCount++ // Skipped message should already be printed by the processor diff --git a/internal/processors/sequential.go b/internal/processors/sequential.go index 01e1f4c..d107f56 100644 --- a/internal/processors/sequential.go +++ b/internal/processors/sequential.go @@ -8,6 +8,7 @@ import ( "github.com/pterm/pterm" "github.com/callmegreg/gh-security-config/internal/types" + "github.com/callmegreg/gh-security-config/internal/ui" ) // SequentialProcessor handles sequential organization processing with optional delay @@ -61,6 +62,7 @@ func (sp *SequentialProcessor) Process() (successCount, skippedCount, errorCount if result.Success { sp.successCount++ sp.progressBar.UpdateTitle(fmt.Sprintf("Processed %s", result.Organization)) + ui.LogOrgSuccess(result.Organization) } else if result.Skipped { sp.skippedCount++ sp.progressBar.UpdateTitle(fmt.Sprintf("Skipped %s", result.Organization)) diff --git a/internal/ui/log.go b/internal/ui/log.go new file mode 100644 index 0000000..0df4848 --- /dev/null +++ b/internal/ui/log.go @@ -0,0 +1,94 @@ +package ui + +import ( + "fmt" + "strings" + "sync" + + "github.com/pterm/pterm" +) + +// LogLevel represents the verbosity of output emitted by the extension. +type LogLevel int + +const ( + // LogLevelInfo emits informational messages in addition to warnings and errors. + LogLevelInfo LogLevel = iota + // LogLevelWarning (the default) emits warnings and errors but suppresses info messages. + LogLevelWarning + // LogLevelError emits only errors. + LogLevelError +) + +// LogLevelDefault is the default log level used when the user does not set one. +const LogLevelDefault = "warning" + +// LogLevelValues lists the accepted values for the --log-level flag. +var LogLevelValues = []string{"info", "warning", "error"} + +var ( + logLevelMu sync.RWMutex + logLevel = LogLevelWarning +) + +// ParseLogLevel converts a user-supplied string to a LogLevel. The comparison is +// case-insensitive and whitespace is trimmed. An empty string resolves to the +// default level. +func ParseLogLevel(value string) (LogLevel, error) { + normalized := strings.ToLower(strings.TrimSpace(value)) + if normalized == "" { + normalized = LogLevelDefault + } + switch normalized { + case "info": + return LogLevelInfo, nil + case "warning": + return LogLevelWarning, nil + case "error": + return LogLevelError, nil + default: + return LogLevelWarning, fmt.Errorf("invalid value for log-level flag: %q (must be one of: %s)", value, strings.Join(LogLevelValues, ", ")) + } +} + +// SetLogLevel updates the package-level log level. Safe for concurrent use. +func SetLogLevel(level LogLevel) { + logLevelMu.Lock() + defer logLevelMu.Unlock() + logLevel = level +} + +// GetLogLevel returns the current log level. Safe for concurrent use. +func GetLogLevel() LogLevel { + logLevelMu.RLock() + defer logLevelMu.RUnlock() + return logLevel +} + +// InfoEnabled reports whether informational messages should be emitted. +func InfoEnabled() bool { + return GetLogLevel() <= LogLevelInfo +} + +// LogInfof prints an informational message using pterm.Info only when the +// current log level is `info`. The format string and arguments follow the +// usual fmt.Printf conventions and a trailing newline is appended when absent. +func LogInfof(format string, args ...interface{}) { + if !InfoEnabled() { + return + } + msg := fmt.Sprintf(format, args...) + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + pterm.Info.Print(msg) +} + +// LogOrgSuccess prints a standard success message for a processed organization +// when informational logging is enabled. +func LogOrgSuccess(org string) { + if !InfoEnabled() { + return + } + pterm.Success.Printf("Successfully processed organization '%s'\n", org) +} diff --git a/internal/ui/log_test.go b/internal/ui/log_test.go new file mode 100644 index 0000000..34f9b5f --- /dev/null +++ b/internal/ui/log_test.go @@ -0,0 +1,48 @@ +package ui + +import "testing" + +func TestParseLogLevel(t *testing.T) { + tests := []struct { + name string + input string + want LogLevel + wantErr bool + }{ + {name: "empty defaults to warning", input: "", want: LogLevelWarning}, + {name: "info", input: "info", want: LogLevelInfo}, + {name: "warning", input: "warning", want: LogLevelWarning}, + {name: "error", input: "error", want: LogLevelError}, + {name: "mixed case", input: "Info", want: LogLevelInfo}, + {name: "padded", input: " warning ", want: LogLevelWarning}, + {name: "invalid", input: "verbose", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseLogLevel(tt.input) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseLogLevel(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + if !tt.wantErr && got != tt.want { + t.Errorf("ParseLogLevel(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func TestInfoEnabled(t *testing.T) { + t.Cleanup(func() { SetLogLevel(LogLevelWarning) }) + + SetLogLevel(LogLevelInfo) + if !InfoEnabled() { + t.Errorf("InfoEnabled() = false, want true for LogLevelInfo") + } + SetLogLevel(LogLevelWarning) + if InfoEnabled() { + t.Errorf("InfoEnabled() = true, want false for LogLevelWarning") + } + SetLogLevel(LogLevelError) + if InfoEnabled() { + t.Errorf("InfoEnabled() = true, want false for LogLevelError") + } +} From 7e26aa632b9146b69f703e9c35012319ce906d5d Mon Sep 17 00:00:00 2001 From: Greg Mohler Date: Fri, 24 Apr 2026 16:09:34 -0400 Subject: [PATCH 2/2] fixed the logging filter --- cmd/apply.go | 17 +++++-- cmd/delete.go | 15 ++++-- cmd/generate.go | 7 +++ cmd/modify.go | 21 +++++--- internal/api/github.go | 14 +++--- internal/loglevel/loglevel.go | 77 ++++++++++++++++++++++++++++ internal/loglevel/loglevel_test.go | 65 ++++++++++++++++++++++++ internal/processors/concurrent.go | 6 ++- internal/processors/delete.go | 5 +- internal/processors/modify.go | 5 +- internal/processors/sequential.go | 5 +- internal/types/config.go | 1 + internal/ui/display.go | 6 +-- internal/ui/log.go | 81 ++++++++++++------------------ internal/ui/log_test.go | 17 +++++++ internal/utils/csv.go | 6 ++- internal/utils/replication.go | 5 ++ 17 files changed, 269 insertions(+), 84 deletions(-) create mode 100644 internal/loglevel/loglevel.go create mode 100644 internal/loglevel/loglevel_test.go diff --git a/cmd/apply.go b/cmd/apply.go index 005207a..2ef7950 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -128,7 +128,7 @@ func runApply(cmd *cobra.Command, args []string) error { pterm.Info.Println("Detecting GitHub Enterprise Server version...") ghesVersion, err := api.GetGHESVersion() if err != nil { - pterm.Warning.Printf("Could not detect GHES version: %v\n", err) + ui.LogWarningf("Could not detect GHES version: %v", err) pterm.Info.Println("Assuming enterprise configurations are not available") ghesVersion = "" } else if ghesVersion != "" { @@ -145,7 +145,7 @@ func runApply(cmd *cobra.Command, args []string) error { pterm.Info.Println("Fetching enterprise security configurations...") enterpriseConfigs, err := api.FetchEnterpriseSecurityConfigurations(enterprise) if err != nil { - pterm.Warning.Printf("Could not fetch enterprise configurations: %v\n", err) + ui.LogWarningf("Could not fetch enterprise configurations: %v", err) } else { for _, config := range enterpriseConfigs { enterpriseConfigNames = append(enterpriseConfigNames, config.Name) @@ -203,14 +203,14 @@ func runApply(cmd *cobra.Command, args []string) error { status, err := api.CheckSingleOrganizationMembership(templateOrg) if err != nil || !status.IsMember || !status.IsOwner { if err != nil { - pterm.Warning.Printf("Could not access template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not access template organization '%s': %v", templateOrg, err) } else { - pterm.Warning.Printf("You must be an owner of template organization '%s' to fetch configurations\n", templateOrg) + ui.LogWarningf("You must be an owner of template organization '%s' to fetch configurations", templateOrg) } } else { configs, err := api.FetchSecurityConfigurations(templateOrg) if err != nil { - pterm.Warning.Printf("Could not fetch configurations from template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not fetch configurations from template organization '%s': %v", templateOrg, err) } else { for _, config := range configs { // Only add organization-level configs (not enterprise configs shown at org level) @@ -334,6 +334,12 @@ func runApply(cmd *cobra.Command, args []string) error { utils.PrintCompletionHeader("Security Configuration Application", successCount, skippedCount, errorCount) + // Extract log level flag + logLevel, err := cmd.Flags().GetString("log-level") + if err != nil { + return err + } + // Build and display replication command replicationFlags := map[string]interface{}{ "enterprise-slug": enterprise, @@ -341,6 +347,7 @@ func runApply(cmd *cobra.Command, args []string) error { "template-org": templateOrg, "concurrency": commonFlags.Concurrency, "delay": commonFlags.Delay, + "log-level": logLevel, "config-name": configName, "config-source": targetType, "scope": scope, diff --git a/cmd/delete.go b/cmd/delete.go index 941727d..168d4f1 100644 --- a/cmd/delete.go +++ b/cmd/delete.go @@ -135,15 +135,15 @@ func runDelete(cmd *cobra.Command, args []string) error { pterm.Info.Printf("Fetching security configurations from template organization '%s'...\n", templateOrg) status, err := api.CheckSingleOrganizationMembership(templateOrg) if err != nil { - pterm.Warning.Printf("Could not access template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not access template organization '%s': %v", templateOrg, err) } else if !status.IsMember { - pterm.Warning.Printf("You must be a member of template organization '%s' to fetch configurations\n", templateOrg) + ui.LogWarningf("You must be a member of template organization '%s' to fetch configurations", templateOrg) } else if !status.IsOwner { - pterm.Warning.Printf("You must be an owner of template organization '%s' to fetch configurations\n", templateOrg) + ui.LogWarningf("You must be an owner of template organization '%s' to fetch configurations", templateOrg) } else { configs, err := api.FetchSecurityConfigurations(templateOrg) if err != nil { - pterm.Warning.Printf("Could not fetch configurations from template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not fetch configurations from template organization '%s': %v", templateOrg, err) } else { for _, config := range configs { // Only add organization-level configs (not enterprise configs shown at org level) @@ -209,6 +209,12 @@ func runDelete(cmd *cobra.Command, args []string) error { utils.PrintCompletionHeader("Security Configuration Deletion", successCount, skippedCount, errorCount) + // Extract log level flag + logLevel, err := cmd.Flags().GetString("log-level") + if err != nil { + return err + } + // Build and display replication command replicationFlags := map[string]interface{}{ "enterprise-slug": enterprise, @@ -216,6 +222,7 @@ func runDelete(cmd *cobra.Command, args []string) error { "template-org": templateOrg, "concurrency": commonFlags.Concurrency, "delay": commonFlags.Delay, + "log-level": logLevel, "config-name": configName, "skip-confirmation-message": fmt.Sprintf("%t", force), } diff --git a/cmd/generate.go b/cmd/generate.go index b5fc770..2b65931 100644 --- a/cmd/generate.go +++ b/cmd/generate.go @@ -277,6 +277,12 @@ func runGenerate(cmd *cobra.Command, args []string) error { utils.PrintCompletionHeader("Security Configuration Generation", successCount, skippedCount, errorCount) + // Extract log level flag + logLevel, err := cmd.Flags().GetString("log-level") + if err != nil { + return err + } + // Build and display replication command replicationFlags := map[string]interface{}{ "enterprise-slug": enterprise, @@ -285,6 +291,7 @@ func runGenerate(cmd *cobra.Command, args []string) error { "dependabot-security-updates-available": fmt.Sprintf("%t", dependabotSecurityUpdatesAvailable), "concurrency": commonFlags.Concurrency, "delay": commonFlags.Delay, + "log-level": logLevel, "config-name": configName, "scope": scope, "set-as-default": fmt.Sprintf("%t", setAsDefault), diff --git a/cmd/modify.go b/cmd/modify.go index 3f5f237..b8513ce 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -117,7 +117,7 @@ func runModify(cmd *cobra.Command, args []string) error { ghesVersion, err := api.GetGHESVersion() var enterpriseConfigCount int if err != nil { - pterm.Warning.Printf("Could not detect GHES version: %v\n", err) + ui.LogWarningf("Could not detect GHES version: %v", err) pterm.Info.Println("Assuming enterprise configurations are not available") ghesVersion = "" } else if ghesVersion != "" { @@ -129,7 +129,7 @@ func runModify(cmd *cobra.Command, args []string) error { pterm.Info.Println("Fetching enterprise security configurations...") enterpriseConfigs, err := api.FetchEnterpriseSecurityConfigurations(enterprise) if err != nil { - pterm.Warning.Printf("Could not fetch enterprise configurations: %v\n", err) + ui.LogWarningf("Could not fetch enterprise configurations: %v", err) } else { enterpriseConfigCount = len(enterpriseConfigs) if enterpriseConfigCount > 0 { @@ -184,15 +184,15 @@ func runModify(cmd *cobra.Command, args []string) error { var orgConfigNames []string status, err := api.CheckSingleOrganizationMembership(templateOrg) if err != nil { - pterm.Warning.Printf("Could not access template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not access template organization '%s': %v", templateOrg, err) } else if !status.IsMember { - pterm.Warning.Printf("You must be a member of template organization '%s' to fetch configurations\n", templateOrg) + ui.LogWarningf("You must be a member of template organization '%s' to fetch configurations", templateOrg) } else if !status.IsOwner { - pterm.Warning.Printf("You must be an owner of template organization '%s' to fetch configurations\n", templateOrg) + ui.LogWarningf("You must be an owner of template organization '%s' to fetch configurations", templateOrg) } else { configs, err := api.FetchSecurityConfigurations(templateOrg) if err != nil { - pterm.Warning.Printf("Could not fetch configurations from template organization '%s': %v\n", templateOrg, err) + ui.LogWarningf("Could not fetch configurations from template organization '%s': %v", templateOrg, err) } else { for _, config := range configs { // Only add organization-level configs (not enterprise configs shown at org level) @@ -261,7 +261,7 @@ func runModify(cmd *cobra.Command, args []string) error { } if currentSettings == nil { - pterm.Warning.Printf("Configuration '%s' not found in template organization '%s'.\n", configName, templateOrg) + ui.LogWarningf("Configuration '%s' not found in template organization '%s'.", configName, templateOrg) return fmt.Errorf("configuration '%s' not found in template org", configName) } @@ -321,6 +321,12 @@ func runModify(cmd *cobra.Command, args []string) error { utils.PrintCompletionHeader("Security Configuration Modification", successCount, skippedCount, errorCount) + // Extract log level flag + logLevel, err := cmd.Flags().GetString("log-level") + if err != nil { + return err + } + // Build and display replication command replicationFlags := map[string]interface{}{ "enterprise-slug": enterprise, @@ -330,6 +336,7 @@ func runModify(cmd *cobra.Command, args []string) error { "dependabot-security-updates-available": fmt.Sprintf("%t", dependabotSecurityUpdatesAvailable), "concurrency": commonFlags.Concurrency, "delay": commonFlags.Delay, + "log-level": logLevel, "config-name": configName, "new-name": newName, "new-description": newDescription, diff --git a/internal/api/github.go b/internal/api/github.go index d23d4f7..2aec979 100644 --- a/internal/api/github.go +++ b/internal/api/github.go @@ -8,6 +8,7 @@ import ( "github.com/cli/go-gh/v2" "github.com/pterm/pterm" + "github.com/callmegreg/gh-security-config/internal/loglevel" "github.com/callmegreg/gh-security-config/internal/types" ) @@ -44,7 +45,9 @@ func CheckSingleOrganizationMembership(org string) (types.MembershipStatus, erro } if err := json.Unmarshal(userResponse.Bytes(), &membership); err != nil { - pterm.Warning.Printf("Failed to parse membership data for organization '%s': %v\n", org, err) + if loglevel.WarningEnabled() { + pterm.Warning.Printf("Failed to parse membership data for organization '%s': %v\n", org, err) + } return types.MembershipStatus{IsMember: false, IsOwner: false, Role: "none"}, nil } @@ -66,16 +69,13 @@ func CheckSingleOrganizationMembership(org string) (types.MembershipStatus, erro func ValidateMembershipAndSkip(org string) *types.ProcessingResult { status, err := CheckSingleOrganizationMembership(org) if err != nil { - pterm.Warning.Printf("Failed to check membership for organization '%s': %v, skipping\n", org, err) - return &types.ProcessingResult{Organization: org, Skipped: true} + return &types.ProcessingResult{Organization: org, Skipped: true, SkipReason: fmt.Sprintf("Failed to check membership for organization '%s': %v, skipping", org, err)} } if !status.IsMember { - pterm.Warning.Printf("Skipping organization '%s': You are not a member\n", org) - return &types.ProcessingResult{Organization: org, Skipped: true} + return &types.ProcessingResult{Organization: org, Skipped: true, SkipReason: fmt.Sprintf("Skipping organization '%s': You are not a member", org)} } if !status.IsOwner { - pterm.Warning.Printf("Skipping organization '%s': You are a member but not an owner\n", org) - return &types.ProcessingResult{Organization: org, Skipped: true} + return &types.ProcessingResult{Organization: org, Skipped: true, SkipReason: fmt.Sprintf("Skipping organization '%s': You are a member but not an owner", org)} } return nil // No skip needed } diff --git a/internal/loglevel/loglevel.go b/internal/loglevel/loglevel.go new file mode 100644 index 0000000..593476e --- /dev/null +++ b/internal/loglevel/loglevel.go @@ -0,0 +1,77 @@ +// Package loglevel manages the global log verbosity for the extension. +// It is intentionally free of internal dependencies so that any package +// (including api and utils) can check the current level without import cycles. +package loglevel + +import ( + "fmt" + "strings" + "sync" +) + +// LogLevel represents the verbosity of output emitted by the extension. +type LogLevel int + +const ( + // LogLevelInfo emits informational messages in addition to warnings and errors. + LogLevelInfo LogLevel = iota + // LogLevelWarning (the default) emits warnings and errors but suppresses info messages. + LogLevelWarning + // LogLevelError emits only errors. + LogLevelError +) + +// LogLevelDefault is the default log level used when the user does not set one. +const LogLevelDefault = "warning" + +// LogLevelValues lists the accepted values for the --log-level flag. +var LogLevelValues = []string{"info", "warning", "error"} + +var ( + logLevelMu sync.RWMutex + logLevel = LogLevelWarning +) + +// ParseLogLevel converts a user-supplied string to a LogLevel. The comparison is +// case-insensitive and whitespace is trimmed. An empty string resolves to the +// default level. +func ParseLogLevel(value string) (LogLevel, error) { + normalized := strings.ToLower(strings.TrimSpace(value)) + if normalized == "" { + normalized = LogLevelDefault + } + switch normalized { + case "info": + return LogLevelInfo, nil + case "warning": + return LogLevelWarning, nil + case "error": + return LogLevelError, nil + default: + return LogLevelWarning, fmt.Errorf("invalid value for log-level flag: %q (must be one of: %s)", value, strings.Join(LogLevelValues, ", ")) + } +} + +// SetLogLevel updates the package-level log level. Safe for concurrent use. +func SetLogLevel(level LogLevel) { + logLevelMu.Lock() + defer logLevelMu.Unlock() + logLevel = level +} + +// GetLogLevel returns the current log level. Safe for concurrent use. +func GetLogLevel() LogLevel { + logLevelMu.RLock() + defer logLevelMu.RUnlock() + return logLevel +} + +// WarningEnabled reports whether warning messages should be emitted. +func WarningEnabled() bool { + return GetLogLevel() <= LogLevelWarning +} + +// InfoEnabled reports whether informational messages should be emitted. +func InfoEnabled() bool { + return GetLogLevel() <= LogLevelInfo +} diff --git a/internal/loglevel/loglevel_test.go b/internal/loglevel/loglevel_test.go new file mode 100644 index 0000000..c4c0bb0 --- /dev/null +++ b/internal/loglevel/loglevel_test.go @@ -0,0 +1,65 @@ +package loglevel + +import "testing" + +func TestParseLogLevel(t *testing.T) { + tests := []struct { + name string + input string + want LogLevel + wantErr bool + }{ + {name: "empty defaults to warning", input: "", want: LogLevelWarning}, + {name: "info", input: "info", want: LogLevelInfo}, + {name: "warning", input: "warning", want: LogLevelWarning}, + {name: "error", input: "error", want: LogLevelError}, + {name: "mixed case", input: "Info", want: LogLevelInfo}, + {name: "padded", input: " warning ", want: LogLevelWarning}, + {name: "invalid", input: "verbose", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseLogLevel(tt.input) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseLogLevel(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr) + } + if !tt.wantErr && got != tt.want { + t.Errorf("ParseLogLevel(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} + +func TestWarningEnabled(t *testing.T) { + t.Cleanup(func() { SetLogLevel(LogLevelWarning) }) + + SetLogLevel(LogLevelInfo) + if !WarningEnabled() { + t.Errorf("WarningEnabled() = false, want true for LogLevelInfo") + } + SetLogLevel(LogLevelWarning) + if !WarningEnabled() { + t.Errorf("WarningEnabled() = false, want true for LogLevelWarning") + } + SetLogLevel(LogLevelError) + if WarningEnabled() { + t.Errorf("WarningEnabled() = true, want false for LogLevelError") + } +} + +func TestInfoEnabled(t *testing.T) { + t.Cleanup(func() { SetLogLevel(LogLevelWarning) }) + + SetLogLevel(LogLevelInfo) + if !InfoEnabled() { + t.Errorf("InfoEnabled() = false, want true for LogLevelInfo") + } + SetLogLevel(LogLevelWarning) + if InfoEnabled() { + t.Errorf("InfoEnabled() = true, want false for LogLevelWarning") + } + SetLogLevel(LogLevelError) + if InfoEnabled() { + t.Errorf("InfoEnabled() = true, want false for LogLevelError") + } +} diff --git a/internal/processors/concurrent.go b/internal/processors/concurrent.go index 0d70bfb..4cd6894 100644 --- a/internal/processors/concurrent.go +++ b/internal/processors/concurrent.go @@ -82,13 +82,15 @@ func (cp *ConcurrentProcessor) Process() (successCount, skippedCount, errorCount ui.LogOrgSuccess(result.Organization) } else if result.Skipped { cp.skippedCount++ - // Skipped message should already be printed by the processor + if result.SkipReason != "" { + ui.LogWarningf("%s", result.SkipReason) + } } else if result.Error != nil { cp.errorCount++ // Check if this is a "configuration exists" error var configExistsErr *types.ConfigurationExistsError if errors.As(result.Error, &configExistsErr) { - pterm.Warning.Printf("Configuration '%s' already exists in organization '%s', skipping\n", configExistsErr.ConfigName, result.Organization) + ui.LogWarningf("Configuration '%s' already exists in organization '%s', skipping", configExistsErr.ConfigName, result.Organization) cp.skippedCount++ cp.errorCount-- // Don't count this as an error } else { diff --git a/internal/processors/delete.go b/internal/processors/delete.go index f9392c8..946b52d 100644 --- a/internal/processors/delete.go +++ b/internal/processors/delete.go @@ -3,10 +3,9 @@ package processors import ( "fmt" - "github.com/pterm/pterm" - "github.com/callmegreg/gh-security-config/internal/api" "github.com/callmegreg/gh-security-config/internal/types" + "github.com/callmegreg/gh-security-config/internal/ui" ) // DeleteProcessor implements OrganizationProcessor for the delete command @@ -44,7 +43,7 @@ func (dp *DeleteProcessor) deleteConfigurationFromOrg(org string) (bool, error) // Find the configuration by name configID, found := api.FindConfigurationByName(configs, dp.ConfigName) if !found { - pterm.Warning.Printf("Configuration '%s' not found in organization '%s', skipping\n", dp.ConfigName, org) + ui.LogWarningf("Configuration '%s' not found in organization '%s', skipping", dp.ConfigName, org) return false, nil // Not an error, just skip this org } diff --git a/internal/processors/modify.go b/internal/processors/modify.go index f67d01a..fb8c6aa 100644 --- a/internal/processors/modify.go +++ b/internal/processors/modify.go @@ -3,10 +3,9 @@ package processors import ( "fmt" - "github.com/pterm/pterm" - "github.com/callmegreg/gh-security-config/internal/api" "github.com/callmegreg/gh-security-config/internal/types" + "github.com/callmegreg/gh-security-config/internal/ui" ) // ModifyProcessor implements OrganizationProcessor for the modify command @@ -47,7 +46,7 @@ func (mp *ModifyProcessor) modifyConfigurationInOrg(org string) (bool, error) { // Find the configuration by name configID, found := api.FindConfigurationByName(configs, mp.ConfigName) if !found { - pterm.Warning.Printf("Configuration '%s' not found in organization '%s', skipping\n", mp.ConfigName, org) + ui.LogWarningf("Configuration '%s' not found in organization '%s', skipping", mp.ConfigName, org) return false, nil // Not an error, just skip this org } diff --git a/internal/processors/sequential.go b/internal/processors/sequential.go index d107f56..ba73431 100644 --- a/internal/processors/sequential.go +++ b/internal/processors/sequential.go @@ -66,13 +66,16 @@ func (sp *SequentialProcessor) Process() (successCount, skippedCount, errorCount } else if result.Skipped { sp.skippedCount++ sp.progressBar.UpdateTitle(fmt.Sprintf("Skipped %s", result.Organization)) + if result.SkipReason != "" { + ui.LogWarningf("%s", result.SkipReason) + } } else if result.Error != nil { sp.errorCount++ sp.progressBar.UpdateTitle(fmt.Sprintf("Processed %s", result.Organization)) // Check if this is a "configuration exists" error var configExistsErr *types.ConfigurationExistsError if errors.As(result.Error, &configExistsErr) { - pterm.Warning.Printf("Configuration '%s' already exists in organization '%s', skipping\n", configExistsErr.ConfigName, result.Organization) + ui.LogWarningf("Configuration '%s' already exists in organization '%s', skipping", configExistsErr.ConfigName, result.Organization) sp.skippedCount++ sp.errorCount-- // Don't count this as an error } else { diff --git a/internal/types/config.go b/internal/types/config.go index 5852aec..0e6accb 100644 --- a/internal/types/config.go +++ b/internal/types/config.go @@ -22,5 +22,6 @@ type ProcessingResult struct { Organization string Success bool Skipped bool + SkipReason string Error error } diff --git a/internal/ui/display.go b/internal/ui/display.go index e8af8fd..93b8e88 100644 --- a/internal/ui/display.go +++ b/internal/ui/display.go @@ -33,11 +33,11 @@ func DisplayCurrentSettings(settings map[string]interface{}, description string) // ShowNoOrganizationsWarning displays appropriate warning based on org targeting mode func ShowNoOrganizationsWarning(flags *utils.CommonFlags) { if flags.Org != "" { - pterm.Warning.Printf("Organization '%s' was not found or is not accessible.\n", flags.Org) + LogWarningf("Organization '%s' was not found or is not accessible.", flags.Org) } else if flags.OrgListPath != "" { - pterm.Warning.Println("No valid organizations found in the CSV file.") + LogWarningf("No valid organizations found in the CSV file.") } else if flags.AllOrgs { - pterm.Warning.Println("No organizations found in the enterprise.") + LogWarningf("No organizations found in the enterprise.") } } diff --git a/internal/ui/log.go b/internal/ui/log.go index 0df4848..83c2ec8 100644 --- a/internal/ui/log.go +++ b/internal/ui/log.go @@ -3,71 +3,56 @@ package ui import ( "fmt" "strings" - "sync" "github.com/pterm/pterm" + + "github.com/callmegreg/gh-security-config/internal/loglevel" ) -// LogLevel represents the verbosity of output emitted by the extension. -type LogLevel int +// LogLevel is an alias for loglevel.LogLevel so existing callers compile unchanged. +type LogLevel = loglevel.LogLevel +// Re-export log-level constants. const ( - // LogLevelInfo emits informational messages in addition to warnings and errors. - LogLevelInfo LogLevel = iota - // LogLevelWarning (the default) emits warnings and errors but suppresses info messages. - LogLevelWarning - // LogLevelError emits only errors. - LogLevelError + LogLevelInfo = loglevel.LogLevelInfo + LogLevelWarning = loglevel.LogLevelWarning + LogLevelError = loglevel.LogLevelError ) // LogLevelDefault is the default log level used when the user does not set one. -const LogLevelDefault = "warning" +const LogLevelDefault = loglevel.LogLevelDefault // LogLevelValues lists the accepted values for the --log-level flag. -var LogLevelValues = []string{"info", "warning", "error"} +var LogLevelValues = loglevel.LogLevelValues -var ( - logLevelMu sync.RWMutex - logLevel = LogLevelWarning -) +// ParseLogLevel delegates to loglevel.ParseLogLevel. +func ParseLogLevel(value string) (LogLevel, error) { return loglevel.ParseLogLevel(value) } -// ParseLogLevel converts a user-supplied string to a LogLevel. The comparison is -// case-insensitive and whitespace is trimmed. An empty string resolves to the -// default level. -func ParseLogLevel(value string) (LogLevel, error) { - normalized := strings.ToLower(strings.TrimSpace(value)) - if normalized == "" { - normalized = LogLevelDefault - } - switch normalized { - case "info": - return LogLevelInfo, nil - case "warning": - return LogLevelWarning, nil - case "error": - return LogLevelError, nil - default: - return LogLevelWarning, fmt.Errorf("invalid value for log-level flag: %q (must be one of: %s)", value, strings.Join(LogLevelValues, ", ")) - } -} +// SetLogLevel delegates to loglevel.SetLogLevel. +func SetLogLevel(level LogLevel) { loglevel.SetLogLevel(level) } -// SetLogLevel updates the package-level log level. Safe for concurrent use. -func SetLogLevel(level LogLevel) { - logLevelMu.Lock() - defer logLevelMu.Unlock() - logLevel = level -} +// GetLogLevel delegates to loglevel.GetLogLevel. +func GetLogLevel() LogLevel { return loglevel.GetLogLevel() } -// GetLogLevel returns the current log level. Safe for concurrent use. -func GetLogLevel() LogLevel { - logLevelMu.RLock() - defer logLevelMu.RUnlock() - return logLevel -} +// WarningEnabled reports whether warning messages should be emitted. +func WarningEnabled() bool { return loglevel.WarningEnabled() } // InfoEnabled reports whether informational messages should be emitted. -func InfoEnabled() bool { - return GetLogLevel() <= LogLevelInfo +func InfoEnabled() bool { return loglevel.InfoEnabled() } + +// LogWarningf prints a warning message using pterm.Warning only when the +// current log level is `warning` or lower. The format string and arguments +// follow the usual fmt.Printf conventions and a trailing newline is appended +// when absent. +func LogWarningf(format string, args ...interface{}) { + if !WarningEnabled() { + return + } + msg := fmt.Sprintf(format, args...) + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + pterm.Warning.Print(msg) } // LogInfof prints an informational message using pterm.Info only when the diff --git a/internal/ui/log_test.go b/internal/ui/log_test.go index 34f9b5f..40df063 100644 --- a/internal/ui/log_test.go +++ b/internal/ui/log_test.go @@ -30,6 +30,23 @@ func TestParseLogLevel(t *testing.T) { } } +func TestWarningEnabled(t *testing.T) { + t.Cleanup(func() { SetLogLevel(LogLevelWarning) }) + + SetLogLevel(LogLevelInfo) + if !WarningEnabled() { + t.Errorf("WarningEnabled() = false, want true for LogLevelInfo") + } + SetLogLevel(LogLevelWarning) + if !WarningEnabled() { + t.Errorf("WarningEnabled() = false, want true for LogLevelWarning") + } + SetLogLevel(LogLevelError) + if WarningEnabled() { + t.Errorf("WarningEnabled() = true, want false for LogLevelError") + } +} + func TestInfoEnabled(t *testing.T) { t.Cleanup(func() { SetLogLevel(LogLevelWarning) }) diff --git a/internal/utils/csv.go b/internal/utils/csv.go index cd43172..92a7861 100644 --- a/internal/utils/csv.go +++ b/internal/utils/csv.go @@ -7,6 +7,8 @@ import ( "strings" "github.com/pterm/pterm" + + "github.com/callmegreg/gh-security-config/internal/loglevel" ) // ReadOrganizationsFromCSV reads organization names from a CSV file @@ -34,7 +36,9 @@ func ReadOrganizationsFromCSV(filePath string) ([]string, error) { } // Basic validation for organization name format if strings.Contains(orgName, " ") || strings.Contains(orgName, "/") { - pterm.Warning.Printf("Line %d: Invalid organization name format '%s', skipping\n", i+1, orgName) + if loglevel.WarningEnabled() { + pterm.Warning.Printf("Line %d: Invalid organization name format '%s', skipping\n", i+1, orgName) + } continue } orgs = append(orgs, orgName) diff --git a/internal/utils/replication.go b/internal/utils/replication.go index f5b208f..100fe3c 100644 --- a/internal/utils/replication.go +++ b/internal/utils/replication.go @@ -39,6 +39,7 @@ func BuildReplicationCommand(command string, flags map[string]interface{}) strin "dependabot-security-updates-available", "concurrency", "delay", + "log-level", "skip-confirmation-message", "overwrite", } @@ -48,6 +49,10 @@ func BuildReplicationCommand(command string, flags map[string]interface{}) strin switch v := value.(type) { case string: if v != "" { + // Only include log-level if it's not the default + if flagName == "log-level" && v == "warning" { + continue + } parts = append(parts, fmt.Sprintf("--%s %s", flagName, quoteIfNeeded(v))) } case bool: