diff --git a/CHANGELOG.md b/CHANGELOG.md index b377126..a3ec4a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Verbose logging (`--verbose`/`-v`) now produces output for all commands, not just those using `SCAAccessService` +- Commands `update`, `version`, `login`, `logout`, `configure`, and `favorites` now emit SDK-format verbose logs (`grant | timestamp | INFO | message`) + +### Changed + +- Migrated 4 ad-hoc `[verbose]`/`Warning:` messages in `fetchStatusData`, `buildDirectoryNameMap`, `buildWorkspaceNameMap`, and `fetchEligibility` to use the SDK logger for consistent format +- Removed `errWriter io.Writer` parameter from `fetchStatusData`, `buildDirectoryNameMap`, `buildWorkspaceNameMap`, and `fetchGroupsEligibility` (verbose output now goes through SDK logger, not injected writer) + ## [0.4.0] - 2026-02-19 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index d7d3055..c025e39 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,6 +77,8 @@ Custom `SCAAccessService` follows SDK conventions: ## Verbose / Logging - `--verbose` / `-v` global flag wired via `PersistentPreRunE` in `cmd/root.go` - Calls `config.EnableVerboseLogging("INFO")` (sets `IDSEC_LOG_LEVEL=INFO`) or `config.DisableVerboseLogging()` (sets `IDSEC_LOG_LEVEL=CRITICAL`) +- `cmdLogger` interface in `cmd/verbose.go` — `Info(msg string, v ...interface{})`, satisfied by `*common.IdsecLogger` +- `log` package-level var in `cmd/verbose.go` — all commands use `log.Info(...)` for verbose output; tests swap with `spyLogger` - `loggingClient` in `internal/sca/logging_client.go` decorates `httpClient`, logging method/route/status/duration at INFO, response headers at DEBUG with Authorization redaction - `NewSCAAccessService()` wraps ISP client with `loggingClient` using `common.GetLogger("grant", -1)` (dynamic level from env) - `NewSCAAccessServiceWithClient()` (test constructor) does not wrap — tests don't need logging diff --git a/cmd/configure.go b/cmd/configure.go index 27aba29..0f9b563 100644 --- a/cmd/configure.go +++ b/cmd/configure.go @@ -104,6 +104,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st } // Save SDK profile + log.Info("Saving profile...") if err := saver.SaveProfile(profile); err != nil { return fmt.Errorf("failed to save profile: %w", err) } @@ -124,6 +125,7 @@ func runConfigure(cmd *cobra.Command, saver profileSaver, tenantURL, username st } // Save app config + log.Info("Saving config...") cfgPath, err := config.ConfigPath() if err != nil { return err diff --git a/cmd/configure_test.go b/cmd/configure_test.go index 937a31e..c73f7e8 100644 --- a/cmd/configure_test.go +++ b/cmd/configure_test.go @@ -248,6 +248,41 @@ func TestConfigureCommand(t *testing.T) { } } +func TestConfigureCommand_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + tmpDir := t.TempDir() + cfgPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", cfgPath) + + cmd := NewConfigureCommandWithDeps( + &mockProfileSaver{}, + "https://example.cyberark.cloud", + "test.user@example.com", + ) + _, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Saving profile", "Saving config"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestConfigureCommandIntegration(t *testing.T) { // Test that configure command is properly registered rootCmd := newTestRootCommand() diff --git a/cmd/favorites.go b/cmd/favorites.go index 65ec6e8..629a482 100644 --- a/cmd/favorites.go +++ b/cmd/favorites.go @@ -188,6 +188,9 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi // Non-interactive mode requires name upfront isNonInteractive := (target != "" && role != "") || (favType == config.FavoriteTypeGroups && group != "") + if isNonInteractive { + log.Info("Non-interactive mode: target=%q role=%q provider=%q group=%q", target, role, provider, group) + } if isNonInteractive && name == "" { if favType == config.FavoriteTypeGroups { return fmt.Errorf("name is required when using --group flag\n\nUsage:\n grant favorites add --type groups --group ") @@ -248,7 +251,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi // Fetch groups eligibility (best-effort, enriched with directory names) if groupsElig != nil { - groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr()) + groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister) if gErr == nil { for i := range groups { items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) @@ -293,6 +296,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi } } + log.Info("Saving favorite %q...", name) if err := config.AddFavorite(cfg, name, fav); err != nil { return fmt.Errorf("failed to add favorite: %w", err) } @@ -322,7 +326,7 @@ func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) defer cancel() - groups, err := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr()) + groups, err := fetchGroupsEligibility(ctx, groupsElig, eligLister) if err != nil { return err } @@ -392,6 +396,7 @@ func newFavoritesRemoveCommand() *cobra.Command { } func runFavoritesList(cmd *cobra.Command, args []string) error { + log.Info("Loading config...") cfg, _, err := config.LoadDefaultWithPath() if err != nil { return err @@ -399,6 +404,7 @@ func runFavoritesList(cmd *cobra.Command, args []string) error { // List favorites favorites := config.ListFavorites(cfg) + log.Info("Found %d favorite(s)", len(favorites)) if len(favorites) == 0 { fmt.Fprintln(cmd.OutOrStdout(), "No favorites saved. Run 'grant favorites add' to create one.") return nil @@ -418,17 +424,20 @@ func runFavoritesList(cmd *cobra.Command, args []string) error { func runFavoritesRemove(cmd *cobra.Command, args []string) error { name := args[0] + log.Info("Loading config...") cfg, cfgPath, err := config.LoadDefaultWithPath() if err != nil { return err } // Remove favorite + log.Info("Removing favorite %q", name) if err := config.RemoveFavorite(cfg, name); err != nil { return err } // Save config + log.Info("Saving config...") if err := config.Save(cfg, cfgPath); err != nil { return fmt.Errorf("failed to save config: %w", err) } diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index a36d1a9..9769b76 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -89,6 +89,125 @@ func TestFavoritesListCommand(t *testing.T) { } } +func TestFavoritesList_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "dev", config.Favorite{ + Provider: "azure", + Target: "sub-123", + Role: "Contributor", + }) + _ = config.AddFavorite(cfg, "prod", config.Favorite{ + Provider: "azure", + Target: "sub-456", + Role: "Reader", + }) + _ = config.Save(cfg, configPath) + + rootCmd := newTestRootCommand() + rootCmd.AddCommand(NewFavoritesCommand()) + _, err := executeCommand(rootCmd, "favorites", "list") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Loading config", "2 favorite"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + +func TestFavoritesRemove_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "dev", config.Favorite{ + Provider: "azure", + Target: "sub-123", + Role: "Contributor", + }) + _ = config.Save(cfg, configPath) + + rootCmd := newTestRootCommand() + rootCmd.AddCommand(NewFavoritesCommand()) + _, err := executeCommand(rootCmd, "favorites", "remove", "dev") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Loading config", "Removing favorite", "Saving config"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + +func TestFavoritesAddNonInteractive_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + cfg := config.DefaultConfig() + _ = config.Save(cfg, configPath) + + rootCmd := newTestRootCommand() + rootCmd.AddCommand(NewFavoritesCommand()) + _, err := executeCommand(rootCmd, "favorites", "add", "myfav", "--target", "sub-123", "--role", "Contributor") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Non-interactive mode", "Saving"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestFavoritesRemoveCommand(t *testing.T) { tests := []struct { name string diff --git a/cmd/helpers.go b/cmd/helpers.go index 70c30c8..c3b7641 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "io" "strings" "sync" @@ -18,13 +17,12 @@ type statusData struct { // fetchStatusData fires sessions and all-CSP eligibility calls concurrently, // then joins results. A sessions error is fatal; eligibility errors are -// gracefully degraded (empty nameMap entry, verbose warning). +// gracefully degraded (empty nameMap entry, verbose warning via SDK logger). func fetchStatusData( ctx context.Context, sessionLister sessionLister, eligLister eligibilityLister, cspFilter *scamodels.CSP, - errWriter io.Writer, ) (*statusData, error) { type eligResult struct { csp scamodels.CSP @@ -76,9 +74,7 @@ func fetchStatusData( nameMap := make(map[string]string) for r := range eligResults { if r.err != nil { - if verbose { - fmt.Fprintf(errWriter, "Warning: failed to fetch names for %s: %v\n", r.csp, r.err) - } + log.Info("failed to fetch names for %s: %v", r.csp, r.err) continue } for _, t := range r.targets { @@ -99,8 +95,8 @@ func fetchStatusData( // buildDirectoryNameMap fetches Azure cloud eligibility to resolve directoryId -> name. // It looks for DIRECTORY-type workspace entries whose workspaceId matches a directory ID. // Falls back to organizationId -> first workspace name if no DIRECTORY entries exist. -// Errors are silently ignored (graceful degradation — groups display without directory context). -func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, errWriter io.Writer) map[string]string { +// Errors are logged via SDK logger (graceful degradation — groups display without directory context). +func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister) map[string]string { nameMap := make(map[string]string) if eligLister == nil { return nameMap @@ -108,8 +104,8 @@ func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, er resp, err := eligLister.ListEligibility(ctx, scamodels.CSPAzure) if err != nil || resp == nil { - if verbose && err != nil { - fmt.Fprintf(errWriter, "Warning: failed to resolve directory names: %v\n", err) + if err != nil { + log.Info("failed to resolve directory names: %v", err) } return nameMap } @@ -136,9 +132,9 @@ func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, er // buildWorkspaceNameMap fetches eligibility for each unique CSP in sessions // concurrently and builds a workspaceID -> workspaceName map. Errors are -// silently ignored (graceful degradation — the raw workspace ID is shown +// logged via SDK logger (graceful degradation — the raw workspace ID is shown // as fallback). -func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, sessions []scamodels.SessionInfo, errWriter io.Writer) map[string]string { +func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, sessions []scamodels.SessionInfo) map[string]string { nameMap := make(map[string]string) // Collect unique CSPs @@ -178,9 +174,7 @@ func buildWorkspaceNameMap(ctx context.Context, eligLister eligibilityLister, se for r := range results { if r.err != nil { - if verbose { - fmt.Fprintf(errWriter, "Warning: failed to fetch names for %s: %v\n", r.csp, r.err) - } + log.Info("failed to fetch names for %s: %v", r.csp, r.err) continue } for _, t := range r.targets { diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index 728aea8..8950578 100644 --- a/cmd/helpers_test.go +++ b/cmd/helpers_test.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "context" "errors" "strings" @@ -204,9 +203,8 @@ func TestFetchStatusData(t *testing.T) { sessionLister := tt.setupSessions() eligLister := tt.setupEligibility() - var errBuf bytes.Buffer - data, err := fetchStatusData(ctx, sessionLister, eligLister, tt.cspFilter, &errBuf) + data, err := fetchStatusData(ctx, sessionLister, eligLister, tt.cspFilter) if tt.wantErr { if err == nil { @@ -235,9 +233,10 @@ func TestFetchStatusData(t *testing.T) { } func TestFetchStatusData_VerboseWarning(t *testing.T) { - oldVerbose := verbose - verbose = true - defer func() { verbose = oldVerbose }() + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() ctx := context.Background() sessionLister := &mockSessionLister{ @@ -252,8 +251,7 @@ func TestFetchStatusData_VerboseWarning(t *testing.T) { listErr: errors.New("eligibility API unavailable"), } - var errBuf bytes.Buffer - data, err := fetchStatusData(ctx, sessionLister, eligLister, nil, &errBuf) + data, err := fetchStatusData(ctx, sessionLister, eligLister, nil) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -264,10 +262,19 @@ func TestFetchStatusData_VerboseWarning(t *testing.T) { t.Errorf("expected empty nameMap, got %v", data.nameMap) } - // Verify verbose warnings were written for both CSPs - errOutput := errBuf.String() - if !strings.Contains(errOutput, "Warning:") { - t.Errorf("expected verbose warning, got: %q", errOutput) + // Verify verbose warnings were logged via SDK logger + if len(spy.messages) == 0 { + t.Error("expected verbose log messages, got none") + } + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, "failed to fetch names") { + found = true + break + } + } + if !found { + t.Errorf("expected 'failed to fetch names' log message, got: %v", spy.messages) } } @@ -410,8 +417,7 @@ func TestBuildWorkspaceNameMap(t *testing.T) { } eligLister := tt.setupEligibility() - var errBuf bytes.Buffer - nameMap := buildWorkspaceNameMap(ctx, eligLister, tt.sessions, &errBuf) + nameMap := buildWorkspaceNameMap(ctx, eligLister, tt.sessions) for _, key := range tt.wantNameMapKeys { if _, ok := nameMap[key]; !ok { @@ -433,9 +439,10 @@ func TestBuildWorkspaceNameMap(t *testing.T) { } func TestBuildWorkspaceNameMap_VerboseWarning(t *testing.T) { - oldVerbose := verbose - verbose = true - defer func() { verbose = oldVerbose }() + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() ctx := context.Background() eligLister := &mockEligibilityLister{ @@ -446,10 +453,16 @@ func TestBuildWorkspaceNameMap_VerboseWarning(t *testing.T) { {CSP: scamodels.CSPAzure, WorkspaceID: "/subscriptions/sub-1"}, } - var buf bytes.Buffer - _ = buildWorkspaceNameMap(ctx, eligLister, sessions, &buf) + _ = buildWorkspaceNameMap(ctx, eligLister, sessions) - if !strings.Contains(buf.String(), "Warning: failed to fetch names for AZURE") { - t.Errorf("expected verbose warning, got: %q", buf.String()) + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, "failed to fetch names") && strings.Contains(msg, "AZURE") { + found = true + break + } + } + if !found { + t.Errorf("expected 'failed to fetch names for AZURE' log, got: %v", spy.messages) } } diff --git a/cmd/login.go b/cmd/login.go index cb94c6f..892b445 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -40,6 +40,7 @@ func NewLoginCommandWithAuth(auth authenticator) *cobra.Command { func runLogin(cmd *cobra.Command, auth authenticator) error { // Load the SDK profile + log.Info("Loading profile...") loader := profiles.DefaultProfilesLoader() profile, err := (*loader).LoadProfile("grant") if err != nil { @@ -68,6 +69,7 @@ func runLogin(cmd *cobra.Command, auth authenticator) error { } // Authenticate (pass empty secret for interactive auth) + log.Info("Authenticating...") token, err := auth.Authenticate(profile, nil, &authmodels.IdsecSecret{Secret: ""}, false, true) if err != nil { return fmt.Errorf("authentication failed: %w", err) diff --git a/cmd/login_test.go b/cmd/login_test.go index f2e8056..97eae9e 100644 --- a/cmd/login_test.go +++ b/cmd/login_test.go @@ -166,6 +166,61 @@ func TestLoginCommand(t *testing.T) { } } +func TestLoginCommand_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + tempDir := t.TempDir() + t.Setenv("IDSEC_PROFILES_FOLDER", tempDir) + + profile := &models.IdsecProfile{ + ProfileName: "grant", + AuthProfiles: map[string]*authmodels.IdsecAuthProfile{ + "isp": { + Username: "test.user@example.com", + AuthMethod: authmodels.Identity, + AuthMethodSettings: &authmodels.IdentityIdsecAuthMethodSettings{ + IdentityURL: "https://example.cyberark.cloud", + IdentityMFAInteractive: true, + }, + }, + }, + } + loader := &profiles.FileSystemProfilesLoader{} + if err := loader.SaveProfile(profile); err != nil { + t.Fatalf("failed to create test profile: %v", err) + } + + auth := &mockAuthenticator{ + token: &authmodels.IdsecToken{ + Token: "test-jwt", + Username: "test.user@example.com", + }, + } + + cmd := NewLoginCommandWithAuth(auth) + _, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Loading profile", "Authenticating"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestLoginCommandIntegration(t *testing.T) { // Test that login command is properly registered rootCmd := newTestRootCommand() diff --git a/cmd/logout.go b/cmd/logout.go index 99fdc11..b706106 100644 --- a/cmd/logout.go +++ b/cmd/logout.go @@ -36,10 +36,12 @@ func NewLogoutCommandWithDeps(clearer keyringClearer) *cobra.Command { } func runLogout(cmd *cobra.Command, clearer keyringClearer) error { + log.Info("Clearing keyring...") if err := clearer.ClearAllPasswords(); err != nil { return fmt.Errorf("failed to clear authentication: %w", err) } + log.Info("Keyring cleared") fmt.Fprintln(cmd.OutOrStdout(), "Logged out successfully") return nil } diff --git a/cmd/logout_test.go b/cmd/logout_test.go index e9fd797..470ea10 100644 --- a/cmd/logout_test.go +++ b/cmd/logout_test.go @@ -56,6 +56,33 @@ func TestLogoutCommand(t *testing.T) { } } +func TestLogoutCommand_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + cmd := NewLogoutCommandWithDeps(&mockKeyringClearer{}) + _, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Clearing keyring", "Keyring cleared"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestLogoutCommandIntegration(t *testing.T) { // Test that logout command is properly registered rootCmd := newTestRootCommand() diff --git a/cmd/revoke.go b/cmd/revoke.go index 380f96e..9ee1a58 100644 --- a/cmd/revoke.go +++ b/cmd/revoke.go @@ -162,7 +162,7 @@ func runRevoke( } } else { // Interactive mode - nameMap := buildWorkspaceNameMap(ctx, elig, sessions.Response, cmd.ErrOrStderr()) + nameMap := buildWorkspaceNameMap(ctx, elig, sessions.Response) selected, err := selector.SelectSessions(sessions.Response, nameMap) if err != nil { diff --git a/cmd/root.go b/cmd/root.go index 5305d87..d41ea2b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "io" "os" "slices" "strings" @@ -182,14 +181,14 @@ func runElevateProduction(cmd *cobra.Command, args []string) error { // buildCachedLister creates a CachedEligibilityLister wrapping the given services. // If the cache directory cannot be resolved, it falls back to the unwrapped services. func buildCachedLister(cfg *config.Config, refresh bool, cloudInner cache.EligibilityLister, groupsInner cache.GroupsEligibilityLister) *cache.CachedEligibilityLister { - log := common.GetLogger("grant", -1) + cacheLog := common.GetLogger("grant", -1) cacheDir, err := cache.CacheDir() if err != nil { return cache.NewCachedEligibilityLister(cloudInner, groupsInner, cache.NewStore("", 0), true, nil) } ttl := config.ParseCacheTTL(cfg) store := cache.NewStore(cacheDir, ttl) - return cache.NewCachedEligibilityLister(cloudInner, groupsInner, store, refresh, log) + return cache.NewCachedEligibilityLister(cloudInner, groupsInner, store, refresh, cacheLog) } // NewRootCommandWithDeps creates a root command with injected dependencies for testing. @@ -257,9 +256,7 @@ func fetchEligibility(ctx context.Context, eligLister eligibilityLister, provide var all []models.EligibleTarget for r := range results { if r.err != nil { - if verbose { - fmt.Fprintf(os.Stderr, "[verbose] %s eligibility query failed: %v\n", r.csp, r.err) - } + log.Info("%s eligibility query failed: %v", r.csp, r.err) continue } for _, t := range r.targets { @@ -439,7 +436,7 @@ func resolveAndElevate( } // fetchGroupsEligibility fetches groups eligibility and enriches with directory names. -func fetchGroupsEligibility(ctx context.Context, groupsEligLister groupsEligibilityLister, cloudEligLister eligibilityLister, errWriter io.Writer) ([]models.GroupsEligibleTarget, error) { +func fetchGroupsEligibility(ctx context.Context, groupsEligLister groupsEligibilityLister, cloudEligLister eligibilityLister) ([]models.GroupsEligibleTarget, error) { eligResp, err := groupsEligLister.ListGroupsEligibility(ctx, models.CSPAzure) if err != nil { return nil, fmt.Errorf("failed to fetch eligible groups: %w", err) @@ -449,7 +446,7 @@ func fetchGroupsEligibility(ctx context.Context, groupsEligLister groupsEligibil } // Resolve directory names from cloud eligibility (best-effort) - dirNameMap := buildDirectoryNameMap(ctx, cloudEligLister, errWriter) + dirNameMap := buildDirectoryNameMap(ctx, cloudEligLister) for i := range eligResp.Response { if name, ok := dirNameMap[eligResp.Response[i].DirectoryID]; ok { eligResp.Response[i].DirectoryName = name @@ -522,7 +519,7 @@ func resolveAndElevateUnified( // Group-only path (--group flag or group favorite) if flags.group != "" { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister, cmd.ErrOrStderr()) + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) if err != nil { return nil, nil, err } @@ -540,7 +537,7 @@ func resolveAndElevateUnified( // Groups-filter path (--groups flag, no --group) if flags.groups { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister, cmd.ErrOrStderr()) + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) if err != nil { return nil, nil, err } @@ -608,7 +605,7 @@ func resolveAndElevateUnified( }() go func() { - groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister, cmd.ErrOrStderr()) + groups, err := fetchGroupsEligibility(ctx, groupsEligLister, eligibilityLister) groupsCh <- groupsResult{groups: groups, err: err} }() diff --git a/cmd/status.go b/cmd/status.go index 04995a6..286379a 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -78,13 +78,13 @@ func runStatus(cmd *cobra.Command, authLoader authLoader, sessionLister sessionL // Fetch sessions and eligibility concurrently ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) defer cancel() - data, err := fetchStatusData(ctx, sessionLister, eligLister, cspFilter, cmd.ErrOrStderr()) + data, err := fetchStatusData(ctx, sessionLister, eligLister, cspFilter) if err != nil { return err } // Resolve directory names for group sessions (best-effort) - dirNameMap := buildDirectoryNameMap(ctx, eligLister, cmd.ErrOrStderr()) + dirNameMap := buildDirectoryNameMap(ctx, eligLister) for k, v := range dirNameMap { if _, exists := data.nameMap[k]; !exists { data.nameMap[k] = v diff --git a/cmd/update.go b/cmd/update.go index a108aac..f3705d3 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -34,11 +34,15 @@ func runUpdate(cmd *cobra.Command, updater selfUpdater) error { return fmt.Errorf("cannot update a dev build; install a release build or download from GitHub Releases") } + log.Info("Current version: %s", v) + current, err := semver.Parse(strings.TrimPrefix(v, "v")) if err != nil { return fmt.Errorf("failed to parse current version %q: %w", v, err) } + log.Info("Checking for updates from %s", updateSlug) + rel, err := updater.UpdateSelf(current, updateSlug) if err != nil { return fmt.Errorf("update failed: %w", err) @@ -47,6 +51,8 @@ func runUpdate(cmd *cobra.Command, updater selfUpdater) error { return fmt.Errorf("update check returned no release information") } + log.Info("Latest release: %s", rel.Version) + if current.Equals(rel.Version) { fmt.Fprintf(cmd.OutOrStdout(), "grant %s is already up to date.\n", current) return nil diff --git a/cmd/update_test.go b/cmd/update_test.go index ad30e6a..8b79b93 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -149,6 +149,48 @@ func TestUpdateCommandPassesSlug(t *testing.T) { } } +func TestUpdateCommand_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + oldVersion := version + version = "1.0.0" + defer func() { version = oldVersion }() + + updater := &mockSelfUpdater{ + release: &selfupdate.Release{ + Version: semver.MustParse("1.1.0"), + }, + } + + cmd := NewUpdateCommandWithDeps(updater) + _, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{ + "Current version: 1.0.0", + "Checking for updates", + updateSlug, + } + + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestUpdateCommandIntegration(t *testing.T) { rootCmd := newTestRootCommand() updateCmd := NewUpdateCommandWithDeps(&mockSelfUpdater{ diff --git a/cmd/verbose.go b/cmd/verbose.go new file mode 100644 index 0000000..ebe6c3d --- /dev/null +++ b/cmd/verbose.go @@ -0,0 +1,14 @@ +package cmd + +import "github.com/cyberark/idsec-sdk-golang/pkg/common" + +// cmdLogger is the verbose logging interface for command-level output. +// Satisfied by *common.IdsecLogger. +type cmdLogger interface { + Info(msg string, v ...interface{}) +} + +// log is the package-level logger used by commands for verbose output. +// Tests can swap this with a spy. PersistentPreRunE sets IDSEC_LOG_LEVEL +// which controls whether Info() calls produce output. +var log cmdLogger = common.GetLogger("grant", -1) diff --git a/cmd/verbose_test.go b/cmd/verbose_test.go new file mode 100644 index 0000000..f842aa5 --- /dev/null +++ b/cmd/verbose_test.go @@ -0,0 +1,37 @@ +package cmd + +import ( + "fmt" + "testing" +) + +// spyLogger captures Info() calls for testing verbose output. +type spyLogger struct { + messages []string +} + +func (s *spyLogger) Info(msg string, v ...interface{}) { + s.messages = append(s.messages, fmt.Sprintf(msg, v...)) +} + +func TestCmdLogger_DefaultIsNotNil(t *testing.T) { + if log == nil { + t.Fatal("package-level log should not be nil") + } +} + +func TestCmdLogger_SpyCaptures(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + log.Info("hello %s", "world") + + if len(spy.messages) != 1 { + t.Fatalf("expected 1 message, got %d", len(spy.messages)) + } + if spy.messages[0] != "hello world" { + t.Errorf("expected %q, got %q", "hello world", spy.messages[0]) + } +} diff --git a/cmd/version.go b/cmd/version.go index 6b28c96..e54e8cd 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -2,6 +2,7 @@ package cmd import ( "fmt" + "runtime" "github.com/spf13/cobra" ) @@ -40,6 +41,9 @@ func printVersion(cmd *cobra.Command) error { d = "unknown" } + log.Info("Go version: %s", runtime.Version()) + log.Info("OS/Arch: %s/%s", runtime.GOOS, runtime.GOARCH) + fmt.Fprintf(cmd.OutOrStdout(), "grant version %s\ncommit: %s\nbuilt: %s\n", v, c, d) return nil } diff --git a/cmd/version_test.go b/cmd/version_test.go index 2e7c857..24298aa 100644 --- a/cmd/version_test.go +++ b/cmd/version_test.go @@ -74,6 +74,37 @@ func TestVersionCommand(t *testing.T) { } } +func TestVersionCommand_VerboseLogs(t *testing.T) { + spy := &spyLogger{} + oldLog := log + log = spy + defer func() { log = oldLog }() + + oldVersion, oldCommit, oldDate := version, commit, buildDate + version, commit, buildDate = "1.0.0", "abc1234", "2026-02-10" + defer func() { version, commit, buildDate = oldVersion, oldCommit, oldDate }() + + cmd := NewVersionCommand() + _, err := executeCommand(cmd) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantMessages := []string{"Go version", "OS/Arch"} + for _, want := range wantMessages { + found := false + for _, msg := range spy.messages { + if strings.Contains(msg, want) { + found = true + break + } + } + if !found { + t.Errorf("expected log containing %q, got: %v", want, spy.messages) + } + } +} + func TestVersionCommandIntegration(t *testing.T) { // Test that version command is properly registered rootCmd := newTestRootCommand()