diff --git a/Makefile b/Makefile index 5425d73..6a7c7dd 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ build: build-release build-dev: go build -o build/hawkling-dev ./cmd/hawkling -# リリースビルド(サイズ最適化) +# Release build (size optimization) build-release: CGO_ENABLED=0 go build -ldflags="-s -w" -o build/hawkling ./cmd/hawkling diff --git a/README.md b/README.md index 1329211..4bbe278 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Hawkling is a command-line tool for managing AWS IAM roles, with a focus on iden ## Installation ```bash -curl -fsSL https://raw.githubusercontent.com/watany-dev/hawkling/main/install.sh | sh +curl -fsSL https://raw.githubusercontent.com/watany-dev/hawkling/main/script/install.sh |sh hawkling -h ``` diff --git a/cmd/hawkling/commands/common.go b/cmd/hawkling/commands/common.go index cc9eca2..96613d6 100644 --- a/cmd/hawkling/commands/common.go +++ b/cmd/hawkling/commands/common.go @@ -1,9 +1,37 @@ package commands import ( + "bufio" + "fmt" + "os" + "strings" + "github.com/spf13/cobra" + + "hawkling/pkg/errors" ) +// FilterOptions contains common filtering options used across commands +type FilterOptions struct { + Days int + OnlyUsed bool + OnlyUnused bool +} + +// ConfirmAction prompts the user for confirmation and returns their response +func ConfirmAction(prompt string) (bool, error) { + fmt.Print(prompt) + + reader := bufio.NewReader(os.Stdin) + response, err := reader.ReadString('\n') + if err != nil { + return false, errors.Wrap(err, "failed to read confirmation") + } + + response = strings.TrimSpace(strings.ToLower(response)) + return response == "y" || response == "yes", nil +} + // AddCommonFlags adds common flags to a command func AddCommonFlags(cmd *cobra.Command, profile *string, region *string) { cmd.PersistentFlags().StringVarP(profile, "profile", "p", "", "AWS profile to use") @@ -12,7 +40,7 @@ func AddCommonFlags(cmd *cobra.Command, profile *string, region *string) { // AddFilterFlags adds filtering flags to a command func AddFilterFlags(cmd *cobra.Command, days *int, onlyUsed *bool, onlyUnused *bool) { - cmd.Flags().IntVarP(days, "days", "d", 90, "Number of days to consider for usage") + cmd.Flags().IntVarP(days, "days", "d", 0, "Number of days to consider for usage") cmd.Flags().BoolVar(onlyUsed, "used", false, "Show only used roles") cmd.Flags().BoolVar(onlyUnused, "unused", false, "Show only unused roles") } diff --git a/cmd/hawkling/commands/delete.go b/cmd/hawkling/commands/delete.go index cecd61f..c4209ac 100644 --- a/cmd/hawkling/commands/delete.go +++ b/cmd/hawkling/commands/delete.go @@ -3,6 +3,9 @@ package commands import ( "context" "fmt" + + "hawkling/pkg/aws" + "hawkling/pkg/errors" ) // DeleteOptions contains options for the delete command @@ -31,7 +34,54 @@ func NewDeleteCommand(profile, region, roleName string, options DeleteOptions) * // Execute runs the delete command func (c *DeleteCommand) Execute(ctx context.Context) error { - // Implementation would go here - fmt.Printf("Deleting IAM role: %s\n", c.roleName) + client, err := aws.NewAWSClient(ctx, c.profile, c.region) + if err != nil { + return errors.Wrap(err, "failed to create AWS client") + } + + // Get the role to ensure it exists + roles, err := client.ListRoles(ctx) + if err != nil { + return errors.Wrap(err, "failed to list roles") + } + + var targetRole *aws.Role + for i, role := range roles { + if role.Name == c.roleName { + targetRole = &roles[i] + break + } + } + + if targetRole == nil { + return errors.Errorf("role '%s' not found", c.roleName) + } + + // If dry run, just show what would be deleted + if c.options.DryRun { + fmt.Printf("DRY RUN: Would delete IAM role: %s\n", c.roleName) + return nil + } + + // Confirm deletion if force flag is not set + if !c.options.Force { + prompt := fmt.Sprintf("Are you sure you want to delete role '%s'? This cannot be undone. [y/N]: ", c.roleName) + confirmed, err := ConfirmAction(prompt) + if err != nil { + return errors.Wrap(err, "failed to read confirmation") + } + + if !confirmed { + fmt.Println("Deletion cancelled") + return nil + } + } + + // Delete the role + if err := client.DeleteRole(ctx, c.roleName); err != nil { + return errors.Wrap(err, "failed to delete role") + } + + fmt.Printf("Successfully deleted IAM role: %s\n", c.roleName) return nil } diff --git a/cmd/hawkling/commands/list.go b/cmd/hawkling/commands/list.go index 310b8bc..0053721 100644 --- a/cmd/hawkling/commands/list.go +++ b/cmd/hawkling/commands/list.go @@ -2,16 +2,18 @@ package commands import ( "context" - "fmt" + "strings" + + "hawkling/pkg/aws" + "hawkling/pkg/errors" + "hawkling/pkg/formatter" ) // ListOptions contains options for the list command type ListOptions struct { - Days int - Output string - ShowAll bool - OnlyUsed bool - OnlyUnused bool + FilterOptions + Output string + ShowAll bool } // ListCommand represents the list command @@ -32,7 +34,31 @@ func NewListCommand(profile, region string, options ListOptions) *ListCommand { // Execute runs the list command func (c *ListCommand) Execute(ctx context.Context) error { - // Implementation would go here - fmt.Println("Listing IAM roles") + client, err := aws.NewAWSClient(ctx, c.profile, c.region) + if err != nil { + return errors.Wrap(err, "failed to create AWS client") + } + + roles, err := client.ListRoles(ctx) + if err != nil { + return errors.Wrap(err, "failed to list roles") + } + + // Filter roles if needed + filterOptions := aws.FilterOptions{ + Days: c.options.FilterOptions.Days, + OnlyUsed: c.options.FilterOptions.OnlyUsed, + OnlyUnused: c.options.FilterOptions.OnlyUnused, + } + + // Use unified filter implementation + roles = aws.FilterRoles(roles, filterOptions) + + // Format output + format := formatter.Format(strings.ToLower(c.options.Output)) + if err := formatter.FormatRoles(roles, format, c.options.ShowAll); err != nil { + return errors.Wrap(err, "failed to format output") + } + return nil } diff --git a/cmd/hawkling/commands/prune.go b/cmd/hawkling/commands/prune.go index d5e7eb0..df192e5 100644 --- a/cmd/hawkling/commands/prune.go +++ b/cmd/hawkling/commands/prune.go @@ -3,11 +3,15 @@ package commands import ( "context" "fmt" + "strings" + + "hawkling/pkg/aws" + "hawkling/pkg/errors" ) // PruneOptions contains options for the prune command type PruneOptions struct { - Days int + FilterOptions DryRun bool Force bool } @@ -30,7 +34,89 @@ func NewPruneCommand(profile, region string, options PruneOptions) *PruneCommand // Execute runs the prune command func (c *PruneCommand) Execute(ctx context.Context) error { - // Implementation would go here - fmt.Println("Pruning unused IAM roles") + client, err := aws.NewAWSClient(ctx, c.profile, c.region) + if err != nil { + return errors.Wrap(err, "failed to create AWS client") + } + + // Get all roles + roles, err := client.ListRoles(ctx) + if err != nil { + return errors.Wrap(err, "failed to list roles") + } + + // Find roles based on the specified options + filterOptions := aws.FilterOptions{ + Days: c.options.FilterOptions.Days, + OnlyUnused: c.options.FilterOptions.OnlyUnused, + OnlyUsed: c.options.FilterOptions.OnlyUsed, + } + + filteredRoles := aws.FilterRoles(roles, filterOptions) + + if len(filteredRoles) == 0 { + fmt.Println("No IAM roles found matching criteria") + return nil + } + + // Show filtered roles + message := "Found %d IAM roles" + if c.options.FilterOptions.OnlyUnused { + message = "Found %d unused IAM roles (not used in the last %d days)" + } else if c.options.FilterOptions.OnlyUsed { + message = "Found %d used IAM roles" + if c.options.FilterOptions.Days > 0 { + message += " (not used in the last %d days)" + } + } else if c.options.FilterOptions.Days > 0 { + message = "Found %d IAM roles (not used in the last %d days)" + } + + if strings.Contains(message, "%d days") { + fmt.Printf(message+":\n", len(filteredRoles), c.options.FilterOptions.Days) + } else { + fmt.Printf(message+":\n", len(filteredRoles)) + } + for i, role := range filteredRoles { + fmt.Printf("%d. %s\n", i+1, role.Name) + } + + // If dry run, stop here + if c.options.DryRun { + fmt.Println("\nDRY RUN: No roles were deleted") + return nil + } + + // Confirm deletion if force flag is not set + if !c.options.Force { + prompt := fmt.Sprintf("\nAre you sure you want to delete %d roles? This cannot be undone. [y/N]: ", len(filteredRoles)) + confirmed, err := ConfirmAction(prompt) + if err != nil { + return errors.Wrap(err, "failed to read confirmation") + } + + if !confirmed { + fmt.Println("Deletion cancelled") + return nil + } + } + + // Delete the filtered roles + var failedRoles []string + for _, role := range filteredRoles { + if err := client.DeleteRole(ctx, role.Name); err != nil { + failedRoles = append(failedRoles, role.Name) + fmt.Printf("Failed to delete role %s: %v\n", role.Name, err) + } else { + fmt.Printf("Deleted role: %s\n", role.Name) + } + } + + if len(failedRoles) > 0 { + fmt.Printf("\nFailed to delete %d roles: %s\n", len(failedRoles), strings.Join(failedRoles, ", ")) + return errors.Errorf("failed to delete %d roles", len(failedRoles)) + } + + fmt.Printf("\nSuccessfully deleted %d IAM roles\n", len(filteredRoles)) return nil } diff --git a/cmd/hawkling/main.go b/cmd/hawkling/main.go index e012d38..0876dc1 100644 --- a/cmd/hawkling/main.go +++ b/cmd/hawkling/main.go @@ -50,11 +50,13 @@ Complete documentation is available at https://github.com/watany-dev/hawkling`, Short: "List IAM roles, optionally filtering for unused roles", RunE: func(cmd *cobra.Command, args []string) error { listOptions := commands.ListOptions{ - Days: listDays, - Output: output, - ShowAll: showAllInfo, - OnlyUsed: onlyUsed, - OnlyUnused: onlyUnused, + FilterOptions: commands.FilterOptions{ + Days: listDays, + OnlyUsed: onlyUsed, + OnlyUnused: onlyUnused, + }, + Output: output, + ShowAll: showAllInfo, } listCmd := commands.NewListCommand(profile, region, listOptions) @@ -84,12 +86,18 @@ Complete documentation is available at https://github.com/watany-dev/hawkling`, // Prune command var pruneDays int + var pruneOnlyUnused bool + var pruneOnlyUsed bool pruneCmd := &cobra.Command{ Use: "prune", - Short: "Delete all unused IAM roles", + Short: "Delete IAM roles based on specified criteria", RunE: func(cmd *cobra.Command, args []string) error { pruneOptions := commands.PruneOptions{ - Days: pruneDays, + FilterOptions: commands.FilterOptions{ + Days: pruneDays, + OnlyUnused: pruneOnlyUnused, + OnlyUsed: pruneOnlyUsed, + }, DryRun: dryRun, Force: force, } @@ -99,6 +107,11 @@ Complete documentation is available at https://github.com/watany-dev/hawkling`, }, } commands.AddPruneFlags(pruneCmd, &pruneDays, &dryRun, &force) + pruneCmd.Flags().BoolVar(&pruneOnlyUnused, "unused", false, "Delete only unused roles") + pruneCmd.Flags().BoolVar(&pruneOnlyUsed, "used", false, "Delete only used roles") + + // Add commands to root command + rootCmd.AddCommand(listCmd, deleteCmd, pruneCmd) return rootCmd } diff --git a/pkg/aws/awsclient.go b/pkg/aws/awsclient.go index 82787fa..57d944d 100644 --- a/pkg/aws/awsclient.go +++ b/pkg/aws/awsclient.go @@ -13,13 +13,31 @@ import ( "github.com/schollz/progressbar/v3" ) +// For testing +var testClient IAMClient + +// SetTestClient sets a test client for unit testing +func SetTestClient(client IAMClient) { + testClient = client +} + +// ClearTestClient clears the test client after tests +func ClearTestClient() { + testClient = nil +} + // AWSclient implements the IAMClient interface type AWSClient struct { iamClient *iam.Client } // NewAWSClient creates a new AWS client with the specified profile and region -func NewAWSClient(ctx context.Context, profile, region string) (*AWSClient, error) { +func NewAWSClient(ctx context.Context, profile, region string) (IAMClient, error) { + // If we're in test mode, return the test client + if testClient != nil { + return testClient, nil + } + var cfg aws.Config var err error diff --git a/pkg/aws/filter.go b/pkg/aws/filter.go new file mode 100644 index 0000000..4c8f12f --- /dev/null +++ b/pkg/aws/filter.go @@ -0,0 +1,46 @@ +package aws + +// FilterOptions contains various filtering criteria +type FilterOptions struct { + Days int + OnlyUsed bool + OnlyUnused bool +} + +// FilterRoles filters roles based on specified options +// This unified implementation follows these rules: +// - Days=0: No days-based filtering +// - OnlyUsed=true: Show only roles that have been used at least once +// - OnlyUnused=true: Show only roles that have never been used +// - Days>0: Show roles not used in the specified days +// - Days>0 + OnlyUsed: Show roles that have been used at least once but not in the specified days +// - Days>0 + OnlyUnused: Show roles that have never been used (days parameter doesn't affect these) +func FilterRoles(roles []Role, options FilterOptions) []Role { + // If both filters are enabled, return empty list (logical conflict) + if options.OnlyUsed && options.OnlyUnused { + return []Role{} + } + + filteredRoles := make([]Role, 0, len(roles)) + + for _, role := range roles { + // OnlyUnused: Include only roles that have never been used (LastUsed == nil) + if options.OnlyUnused && role.LastUsed != nil { + continue + } + + // OnlyUsed: Include only roles that have been used at least once (LastUsed != nil) + if options.OnlyUsed && role.LastUsed == nil { + continue + } + + // Days filter: For roles not matching OnlyUnused, apply the days filter + if options.Days > 0 && !options.OnlyUnused && !role.IsUnused(options.Days) { + continue + } + + filteredRoles = append(filteredRoles, role) + } + + return filteredRoles +} diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 55ff574..7fa708e 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -2,6 +2,16 @@ package errors import "fmt" +// Wrap wraps an error with a message +func Wrap(err error, message string) error { + return fmt.Errorf("%s: %w", message, err) +} + +// Errorf formats an error message +func Errorf(format string, args ...interface{}) error { + return fmt.Errorf(format, args...) +} + // WrapRoleError wraps an error with a role operation context func WrapRoleError(op string, roleName string, err error) error { return fmt.Errorf("failed to %s role %s: %w", op, roleName, err) diff --git a/review.md b/review.md index 95e1a0e..cd271b1 100644 --- a/review.md +++ b/review.md @@ -1,25 +1,26 @@ -# パフォーマンス改善レビュー +# Performance Improvement Review -## 現状の問題点 +## Current Issues -1. `main.go` における不必要なフィルタリングの重複 -2. AWS APIリクエストの非効率な使用 -3. スライスの初期化方法が非効率 -4. フォーマッタにおけるメモリ使用の最適化不足 +1. Unnecessary duplicate filtering in `main.go` +2. Inefficient use of AWS API requests +3. Inefficient slice initialization methods +4. Insufficient memory usage optimization in formatter +5. Filtering flags (--used, --unused, --days) not functioning correctly -## 改善提案 +## Improvement Suggestions -### 1. フィルタリングロジックの最適化 (main.go) +### 1. Optimize Filtering Logic (main.go) -現在、`listRoles`関数では複数のフィルタ条件(days, onlyUsed, onlyUnused)がそれぞれ独立して適用されており、複数回のスライス走査が発生しています。これを1回の走査で済むように最適化できます。 +Currently, in the `listRoles` function, multiple filter conditions (days, onlyUsed, onlyUnused) are applied independently, resulting in multiple slice traversals. This can be optimized to complete with a single traversal. ```go -// 改善前: 複数回のフィルタリングループ -if days > 0 { /* フィルタリング処理 */ } -if onlyUsed { /* フィルタリング処理 */ } -if onlyUnused { /* フィルタリング処理 */ } +// Before improvement: Multiple filtering loops +if days > 0 { /* filtering process */ } +if onlyUsed { /* filtering process */ } +if onlyUnused { /* filtering process */ } -// 改善後: 1回のループで全条件を評価 +// After improvement: Evaluate all conditions in one loop var filteredRoles []aws.Role for _, role := range roles { if (days > 0 && !role.IsUnused(days)) || @@ -31,44 +32,103 @@ for _, role := range roles { } ``` -### 2. AWS APIリクエストの最適化 (awsclient.go) +### 2. Fix Filtering Flags (list.go) -`ListRoles`メソッドでは現在、ロールの取得とロールの最終使用日時の取得が分離されています。AWS APIが提供する情報を最大限活用することで、APIコールを減らせる可能性があります。 +There are issues with the filtering logic in the current implementation: -また、ゴルーチンの起動方法を改善することでオーバーヘッドを減らせます。 +```go +// Issue: Filtering logic is reversed +if (c.options.OnlyUsed && !isUnused) || (c.options.OnlyUnused && isUnused) { + filteredRoles = append(filteredRoles, role) +} +``` +In this implementation: +1. When the `OnlyUsed` flag is set, it filters roles that are `!isUnused` (i.e., in use), but `isUnused` is incorrectly determined. +2. When the `OnlyUnused` flag is set, it filters roles that are `isUnused` (unused), but the consideration of days is insufficient. + +Fix proposal: ```go -// 改善前: ゴルーチン起動のオーバーヘッド +// Fix proposal: Clarify filtering logic +for _, role := range roles { + // Properly handle the days > 0 condition + isUnusedForDays := role.IsUnused(c.options.Days) + + // OnlyUsed: Show only roles that have been used (LastUsed != nil) + if c.options.OnlyUsed && (role.LastUsed == nil) { + continue + } + + // OnlyUnused: Show only roles that have not been used within the specified days + if c.options.OnlyUnused && !isUnusedForDays { + continue + } + + filteredRoles = append(filteredRoles, role) +} +``` + +### 3. Optimize AWS API Requests (awsclient.go) + +In the `ListRoles` method, currently the retrieval of roles and the retrieval of the last used date for roles are separated. There may be potential to reduce API calls by maximizing the use of information provided by the AWS API. + +Also, improving how goroutines are launched can reduce overhead. + +```go +// Before improvement: Overhead of launching goroutines for i := range roles { go func(i int) { // ... }(i) } -// 改善後: ワーカープールパターンの採用 +// After improvement: Adopt worker pool pattern ``` -### 3. スライス初期化の最適化 +### 4. Fix IsUnused Logic (iam.go) + +The logic in the current `IsUnused` method also needs to be checked: + +```go +// IsUnused checks if a role is unused for the specified number of days +func (r *Role) IsUnused(days int) bool { + if r.LastUsed == nil { + return true // If LastUsed is nil, the role is unused + } + + threshold := time.Now().AddDate(0, 0, -days) + return r.LastUsed.Before(threshold) // If it was last used before 'days' days ago, consider it unused +} +``` + +In this logic, if `LastUsed` is `nil` (never used), it is always treated as "unused". Also, the comparison with the threshold determines that a role is "unused" if the last used date is before (older than) the threshold. This is logically correct. + +### 5. Optimize Slice Initialization -多くの場所でスライスが事前容量指定なしで初期化されています。容量を事前に指定することで、動的な拡張によるメモリ割り当てを減らせます。 +In many places, slices are initialized without specifying a pre-capacity. By specifying capacity in advance, memory allocations due to dynamic expansion can be reduced. ```go -// 改善前 +// Before improvement var unusedRoles []aws.Role -// 改善後 +// After improvement unusedRoles := make([]aws.Role, 0, len(roles)) ``` -### 4. formatter.goのメモリ使用最適化 +### 6. Optimize Memory Usage in formatter.go -`FormatRolesAsJSON`関数では、すべてのロール情報がJSONとしてメモリに格納されてから出力されます。大量のロールがある場合、ストリーミング方式の出力に変更することで、メモリ使用量を削減できます。 +In the `FormatRolesAsJSON` function, all role information is stored in memory as JSON before being output. For cases with many roles, changing to a streaming output method can reduce memory usage. -## 実装優先度 +## Implementation Priorities -1. **高**: スライス初期化の最適化(実装が容易かつ効果が明確) -2. **中**: フィルタリングロジックの統合(コード可読性を維持しつつ改善) -3. **中**: ゴルーチン管理の最適化(現状でもセマフォがあり基本的には問題ない) -4. **低**: JSONフォーマッタのストリーミング出力(大量データ時のみ効果あり) +1. **Highest**: Fix filtering flags (resolve functional issues) + - ✅ Implementation complete: Modified filtering logic so that --used, --unused, --days flags function correctly + - ✅ Tests added: Added tests for filtering logic +2. **High**: Optimize slice initialization (easy to implement and clear effect) + - ✅ Partially implemented: Optimized initialization of filteredRoles slice +3. **Medium**: Integrate filtering logic (improve while maintaining code readability) + - ✅ Implementation complete: Optimized filtering logic to complete in a single loop +4. **Medium**: Optimize goroutine management (currently has semaphores, so basically no problem) +5. **Low**: Stream output in JSON formatter (only effective with large amounts of data) -これらの改善により、メモリ使用量の削減とCPU使用効率の向上が期待できます。特に大量のロールを処理する場合に効果が顕著になるでしょう。 \ No newline at end of file +These improvements are expected to reduce memory usage and improve CPU usage efficiency. The effects will be particularly noticeable when processing large numbers of roles. \ No newline at end of file diff --git a/test/list_command_test.go b/test/list_command_test.go new file mode 100644 index 0000000..fe660fb --- /dev/null +++ b/test/list_command_test.go @@ -0,0 +1,188 @@ +package test + +import ( + "bytes" + "context" + "io" + "os" + "strings" + "testing" + + "hawkling/cmd/hawkling/commands" + "hawkling/pkg/aws" + // "hawkling/pkg/formatter" +) + +// simulatedError is already defined in mock_iamclient.go + +// mockFormatter satisfies the formatter interface for testing +// type mockFormatter struct { +// output string +// } + +// func (m *mockFormatter) FormatRoles(roles []aws.Role, format formatter.Format, showAllInfo bool) error { +// var output strings.Builder +// for _, role := range roles { +// output.WriteString(role.Name + "\n") +// } +// m.output = output.String() +// return nil +// } + +func TestListCommandFilterFlags(t *testing.T) { + // Set up mock AWS client + mockClient := NewMockIAMClient() + aws.SetTestClient(mockClient) + // Clean up test client after test + defer aws.ClearTestClient() + + tests := []struct { + name string + options commands.ListOptions + expectedRoles int // Number of roles expected after filtering + shouldContain []string // Role names that should be in the output + shouldNotContain []string // Role names that should not be in the output + }{ + { + name: "No filters - should return all roles", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: false, + OnlyUnused: false, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 3, // All 3 roles from the mock + shouldContain: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + shouldNotContain: []string{}, + }, + { + name: "Used filter - should return only roles that were used at least once", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: true, + OnlyUnused: false, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 2, // ActiveRole and InactiveRole have been used + shouldContain: []string{"ActiveRole", "InactiveRole"}, + shouldNotContain: []string{"NeverUsedRole"}, + }, + { + name: "Unused filter - should return only roles that were never used", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: false, + OnlyUnused: true, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 1, // Only NeverUsedRole was never used + shouldContain: []string{"NeverUsedRole"}, + shouldNotContain: []string{"ActiveRole", "InactiveRole"}, + }, + { + name: "Both filters - should return no roles (conflicting filters)", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: true, + OnlyUnused: true, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 0, // No roles match both used and unused + shouldContain: []string{}, + shouldNotContain: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with 90 days - should return roles not used in 90 days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: false, + OnlyUnused: false, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 2, // InactiveRole was used 100 days ago, NeverUsedRole was never used + shouldContain: []string{"InactiveRole", "NeverUsedRole"}, + shouldNotContain: []string{"ActiveRole"}, + }, + { + name: "Days filter with Used filter - should return used roles not used in 90 days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: true, + OnlyUnused: false, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 1, // Only InactiveRole was used but not in last 90 days + shouldContain: []string{"InactiveRole"}, + shouldNotContain: []string{"ActiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with Unused filter - should return never used roles", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: false, + OnlyUnused: true, + }, + Output: "table", + ShowAll: false, + }, + expectedRoles: 1, // Only NeverUsedRole was never used + shouldContain: []string{"NeverUsedRole"}, + shouldNotContain: []string{"ActiveRole", "InactiveRole"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Capture stdout + originalStdout := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + // Run the command + cmd := commands.NewListCommand("test-profile", "us-west-2", test.options) + err := cmd.Execute(context.Background()) + if err != nil { + t.Fatalf("Command execution failed: %v", err) + } + + // Restore stdout and get output + w.Close() + os.Stdout = originalStdout + var buf bytes.Buffer + _, _ = io.Copy(&buf, r) + output := buf.String() + + // Check expectations + for _, expectedStr := range test.shouldContain { + if !strings.Contains(output, expectedStr) { + t.Errorf("Expected output to contain %q, but it didn't.\nOutput: %s", expectedStr, output) + } + } + + for _, unexpectedStr := range test.shouldNotContain { + if strings.Contains(output, unexpectedStr) { + t.Errorf("Expected output to NOT contain %q, but it did.\nOutput: %s", unexpectedStr, output) + } + } + }) + } +} diff --git a/test/list_filter_test.go b/test/list_filter_test.go new file mode 100644 index 0000000..a21ca0f --- /dev/null +++ b/test/list_filter_test.go @@ -0,0 +1,149 @@ +package test + +import ( + "testing" + "time" + + "hawkling/pkg/aws" +) + +// Local timePtr for tests +func localTimePtr(t time.Time) *time.Time { + return &t +} + +func TestFilterRoles(t *testing.T) { + now := time.Now() + + // Create test roles with different last used times + roles := []aws.Role{ + { + Name: "ActiveRole", + Arn: "arn:aws:iam::123456789012:role/ActiveRole", + CreateDate: now.AddDate(-1, 0, 0), + LastUsed: localTimePtr(now.AddDate(0, 0, -5)), // Used 5 days ago + Description: "Recently used role", + }, + { + Name: "InactiveRole", + Arn: "arn:aws:iam::123456789012:role/InactiveRole", + CreateDate: now.AddDate(-2, 0, 0), + LastUsed: localTimePtr(now.AddDate(0, 0, -100)), // Used 100 days ago + Description: "Role unused for a long time", + }, + { + Name: "NeverUsedRole", + Arn: "arn:aws:iam::123456789012:role/NeverUsedRole", + CreateDate: now.AddDate(0, -6, 0), + LastUsed: nil, // Never used + Description: "Role that was never used", + }, + } + + tests := []struct { + name string + days int + onlyUsed bool + onlyUnused bool + expectedRoles []string // Names of roles that should be in the result + }{ + { + name: "No filters - should return all roles", + days: 0, + onlyUsed: false, + onlyUnused: false, + expectedRoles: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + }, + { + name: "Used filter - should return only roles that were used at least once", + days: 0, + onlyUsed: true, + onlyUnused: false, + expectedRoles: []string{"ActiveRole", "InactiveRole"}, + }, + { + name: "Unused filter - should return only roles that were never used", + days: 0, + onlyUsed: false, + onlyUnused: true, + expectedRoles: []string{"NeverUsedRole"}, + }, + { + name: "Both filters - should return no roles (conflicting filters)", + days: 0, + onlyUsed: true, + onlyUnused: true, + expectedRoles: []string{}, + }, + { + name: "Days filter with 90 days - should return roles not used in 90 days", + days: 90, + onlyUsed: false, + onlyUnused: false, + expectedRoles: []string{"InactiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with 3 days - should return roles not used in 3 days", + days: 3, + onlyUsed: false, + onlyUnused: false, + expectedRoles: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with Used filter - should return used roles not used in 90 days", + days: 90, + onlyUsed: true, + onlyUnused: false, + expectedRoles: []string{"InactiveRole"}, + }, + { + name: "Days filter with Unused filter - should return never used roles", + days: 90, + onlyUsed: false, + onlyUnused: true, + expectedRoles: []string{"NeverUsedRole"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Apply filter using unified filtering logic + filterOptions := aws.FilterOptions{ + Days: test.days, + OnlyUsed: test.onlyUsed, + OnlyUnused: test.onlyUnused, + } + + filteredRoles := aws.FilterRoles(roles, filterOptions) + + // Check if filtered roles match expected + if len(filteredRoles) != len(test.expectedRoles) { + t.Errorf("Expected %d roles, got %d", len(test.expectedRoles), len(filteredRoles)) + t.Logf("Expected: %v", test.expectedRoles) + t.Logf("Got: %v", getRoleNames(filteredRoles)) + } + + // Check if each expected role is in the filtered roles + for _, expectedName := range test.expectedRoles { + found := false + for _, role := range filteredRoles { + if role.Name == expectedName { + found = true + break + } + } + if !found { + t.Errorf("Expected role %q not found in filtered roles", expectedName) + } + } + }) + } +} + +func getRoleNames(roles []aws.Role) []string { + names := make([]string, len(roles)) + for i, role := range roles { + names[i] = role.Name + } + return names +} diff --git a/test/test_unused_roles.go b/test/test_unused_roles.go new file mode 100644 index 0000000..09bc644 --- /dev/null +++ b/test/test_unused_roles.go @@ -0,0 +1,66 @@ +package test + +import ( + "testing" + "time" + + "hawkling/pkg/aws" +) + +// TestIsUnused tests the IsUnused method of Role +func TestIsUnused(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + role aws.Role + days int + expected bool + }{ + { + name: "Role used recently - should not be considered unused", + role: aws.Role{ + Name: "RecentlyUsedRole", + LastUsed: timePtr(now.AddDate(0, 0, -5)), // Used 5 days ago + }, + days: 90, + expected: false, // Not unused for 90 days + }, + { + name: "Role not used for a long time - should be considered unused", + role: aws.Role{ + Name: "OldRole", + LastUsed: timePtr(now.AddDate(0, 0, -100)), // Used 100 days ago + }, + days: 90, + expected: true, // Unused for 90 days + }, + { + name: "Role never used - should be considered unused", + role: aws.Role{ + Name: "NeverUsedRole", + LastUsed: nil, // Never used + }, + days: 90, + expected: true, // Never used, so unused for any days + }, + { + name: "Edge case - Role used exactly at threshold", + role: aws.Role{ + Name: "ThresholdRole", + LastUsed: timePtr(now.AddDate(0, 0, -90)), // Used 90 days ago + }, + days: 90, + expected: true, // At threshold, should be considered unused + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := test.role.IsUnused(test.days) + if result != test.expected { + t.Errorf("IsUnused(%d) = %v; want %v", test.days, result, test.expected) + } + }) + } +} diff --git a/test/unused_roles_test.go b/test/unused_roles_test.go new file mode 100644 index 0000000..2ca3b84 --- /dev/null +++ b/test/unused_roles_test.go @@ -0,0 +1,156 @@ +package test + +import ( + "testing" + + "hawkling/cmd/hawkling/commands" + "hawkling/pkg/aws" +) + +func TestFilteringLogic(t *testing.T) { + // Create test roles - reusing the MockIAMClient setup + mockClient := NewMockIAMClient() + roles := mockClient.Roles + + tests := []struct { + name string + options commands.ListOptions + expectedCount int + expectedRoles []string + }{ + { + name: "No filtering", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, // Changed from 90 to 0 - no days filter + OnlyUsed: false, + OnlyUnused: false, + }, + }, + expectedCount: 3, + expectedRoles: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + }, + { + name: "Only used roles - no days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: true, + OnlyUnused: false, + }, + }, + expectedCount: 2, + expectedRoles: []string{"ActiveRole", "InactiveRole"}, + }, + { + name: "Only unused roles - no days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 0, + OnlyUsed: false, + OnlyUnused: true, + }, + }, + expectedCount: 1, + expectedRoles: []string{"NeverUsedRole"}, + }, + { + name: "Days filter with 90 days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: false, + OnlyUnused: false, + }, + }, + expectedCount: 2, + expectedRoles: []string{"InactiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with 3 days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 3, + OnlyUsed: false, + OnlyUnused: false, + }, + }, + expectedCount: 3, + expectedRoles: []string{"ActiveRole", "InactiveRole", "NeverUsedRole"}, + }, + { + name: "Days filter with Used filter - 90 days", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: true, + OnlyUnused: false, + }, + }, + expectedCount: 1, + expectedRoles: []string{"InactiveRole"}, + }, + { + name: "Days filter with Unused filter", + options: commands.ListOptions{ + FilterOptions: commands.FilterOptions{ + Days: 90, + OnlyUsed: false, + OnlyUnused: true, + }, + }, + expectedCount: 1, + expectedRoles: []string{"NeverUsedRole"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Apply filter using common filtering logic + filterOptions := aws.FilterOptions{ + Days: test.options.FilterOptions.Days, + OnlyUsed: test.options.FilterOptions.OnlyUsed, + OnlyUnused: test.options.FilterOptions.OnlyUnused, + } + + filteredRoles := aws.FilterRoles(roles, filterOptions) + + // Check count + if len(filteredRoles) != test.expectedCount { + t.Errorf("Expected %d roles, got %d", test.expectedCount, len(filteredRoles)) + t.Logf("Expected: %v", test.expectedRoles) + t.Logf("Got: %v", getRoleNames(filteredRoles)) + } + + // Check if expected roles are in the result + for _, expectedName := range test.expectedRoles { + found := false + for _, role := range filteredRoles { + if role.Name == expectedName { + found = true + break + } + } + if !found { + t.Errorf("Expected role %q not found in filtered roles", expectedName) + } + } + + // Check if there are no unexpected roles + if len(filteredRoles) > 0 { + for _, role := range filteredRoles { + found := false + for _, expectedName := range test.expectedRoles { + if role.Name == expectedName { + found = true + break + } + } + if !found { + t.Errorf("Unexpected role %q found in filtered roles", role.Name) + } + } + } + }) + } +}