Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions cmd/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
Expand Down
35 changes: 35 additions & 0 deletions cmd/configure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
13 changes: 11 additions & 2 deletions cmd/favorites.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <name> --type groups --group <group>")
Expand Down Expand Up @@ -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]})
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -392,13 +396,15 @@ 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
}

// 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
Expand All @@ -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)
}
Expand Down
119 changes: 119 additions & 0 deletions cmd/favorites_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 9 additions & 15 deletions cmd/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"context"
"fmt"
"io"
"strings"
"sync"

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -99,17 +95,17 @@ 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
}

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
}
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Loading