fix(filters): report or refuse recommendations an account filter cannot place

CHANGELOG.md

@@ -13,6 +13,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- Recommendations with no account ID or name are now reported in the drop summary
+  when an account filter (`--include-accounts` or `--exclude-accounts`) removes
+  them (#1613).
 - A purchase result with `Success=true` and a non-nil `Error` is now treated as
   a failure in the Ctrl-C summary, the audit-stop message, the per-service
   success/failed counts and the non-zero exit. The CSV `Success` column keeps the

cmd/multi_service_filters.go

@@ -68,7 +68,10 @@ func processRecommendation(rec *common.Recommendation, cfg *Config, instanceVers
 		// operator-scoping choices, not drops worth surfacing in the summary.
 		// --min-pool-size drops are counted separately in applyFilters before
 		// this function runs, so no drop reason is surfaced here.
-		return *rec, false, ""
+		// The exception is a recommendation with no account ID or name under an
+		// account filter: it is not a scoping choice but data the filter
+		// cannot place, so it is counted.
+		return *rec, false, unidentifiedAccountDropReason(rec, cfg)
 	}
 
 	// Apply engine version filters - adjust instance count by subtracting extended support versions.
@@ -168,6 +171,42 @@ func shouldIncludeEngine(rec *common.Recommendation, cfg *Config) bool {
 	return filtersFromConfig(cfg).IncludesEngine(rec)
 }
 
+// DropSummary reasons for a recommendation that carries neither an account ID
+// nor an account name and is dropped by an account filter. With both filters
+// set the include label is used.
+const (
+	dropUnidentifiedInclude = "unidentified-account (--include-accounts)"
+	dropUnidentifiedExclude = "unidentified-account (--exclude-accounts)"
+)
+
+// unidentifiedAccountDropReason returns the drop reason when rec has no account
+// ID or name and the account filter alone removed it, else "".
+func unidentifiedAccountDropReason(rec *common.Recommendation, cfg *Config) string {
+	if !isUnidentifiedAccount(rec) || !passesWithoutAccountFilters(rec, cfg) {
+		return ""
+	}
+	if len(cfg.IncludeAccounts) > 0 {
+		return dropUnidentifiedInclude
+	}
+	if len(cfg.ExcludeAccounts) > 0 {
+		return dropUnidentifiedExclude
+	}
+	return ""
+}
+
+func isUnidentifiedAccount(rec *common.Recommendation) bool {
+	return rec.Account == "" && rec.AccountName == ""
+}
+
+// passesWithoutAccountFilters reports whether rec passes every dimension filter
+// except the account ones, so an unidentified-account drop is only counted for
+// a recommendation the account filter alone removed.
+func passesWithoutAccountFilters(rec *common.Recommendation, cfg *Config) bool {
+	c := *cfg
+	c.IncludeAccounts, c.ExcludeAccounts = nil, nil
+	return passesDimensionFilters(rec, &c)
+}
+
 // shouldIncludeAccount checks if an account should be included based on filters.
 func shouldIncludeAccount(accountName string, cfg *Config) bool {
 	return shouldIncludeAccountIDOrName("", accountName, cfg)

cmd/multi_service_filters_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"os"
@@ -382,7 +383,7 @@ func TestShouldIncludeAccount(t *testing.T) {
 		{
 			name:            "In include list",
 			accountID:       "123456789012",
-			includeAccounts: []string{"123456789012", "210987654321"},
+			includeAccounts: []string{"123456789012", "111111111111"},
 			excludeAccounts: []string{},
 			expected:        true,
 		},
@@ -945,3 +946,84 @@ func TestRequireExactExcludes(t *testing.T) {
 		})
 	}
 }
+
+const (
+	testUnidentifiedAcct = "123456789012"
+	testOtherAcct        = "999888777666"
+)
+
+func TestApplyFilters_UnidentifiedAccountUnderInclude(t *testing.T) {
+	unidentified := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1}
+	named := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1, Account: testUnidentifiedAcct, AccountName: "prod"}
+	tests := []struct {
+		name      string
+		cfg       Config
+		recs      []common.Recommendation
+		wantKept  int
+		wantDrops int
+	}{
+		{"include drops it and counts it", Config{IncludeExtendedSupport: true, IncludeAccounts: []string{"prod"}}, []common.Recommendation{unidentified, named}, 1, 1},
+		{"include with other region mismatch is not counted", Config{IncludeExtendedSupport: true, IncludeAccounts: []string{"prod"}, IncludeRegions: []string{"eu-west-1"}}, []common.Recommendation{unidentified}, 0, 0},
+		{"no account filter keeps it", Config{IncludeExtendedSupport: true}, []common.Recommendation{unidentified}, 1, 0},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			drops := common.NewDropSummary()
+			got := applyFilters(tt.recs, &tt.cfg, nil, nil, "", drops)
+			assert.Len(t, got, tt.wantKept)
+			assert.Equal(t, tt.wantDrops, drops.Total())
+			if tt.wantDrops > 0 {
+				assert.Contains(t, drops.FormatOneLine(), "unidentified-account")
+			}
+		})
+	}
+}
+
+func TestApplyFilters_UnidentifiedAccountUnderExclude(t *testing.T) {
+	unidentified := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1}
+	named := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1, Account: testUnidentifiedAcct, AccountName: "prod"}
+	other := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1, Account: testOtherAcct, AccountName: "dev"}
+	tests := []struct {
+		name       string
+		cfg        Config
+		recs       []common.Recommendation
+		wantKept   int
+		wantDrops  int
+		wantReason string
+	}{
+		{"exclude-only drops and counts it, keeps others", Config{IncludeExtendedSupport: true, ExcludeAccounts: []string{testUnidentifiedAcct}}, []common.Recommendation{unidentified, named, other}, 1, 1, "unidentified-account (--exclude-accounts)"},
+		{"exclude-only with other region mismatch is not counted", Config{IncludeExtendedSupport: true, ExcludeAccounts: []string{testUnidentifiedAcct}, IncludeRegions: []string{"eu-west-1"}}, []common.Recommendation{unidentified}, 0, 0, ""},
+		{"include and exclude count it once under the include reason", Config{IncludeExtendedSupport: true, IncludeAccounts: []string{"dev"}, ExcludeAccounts: []string{testUnidentifiedAcct}}, []common.Recommendation{unidentified, other}, 1, 1, "unidentified-account (--include-accounts)"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			drops := common.NewDropSummary()
+			got := applyFilters(tt.recs, &tt.cfg, nil, nil, "", drops)
+			assert.Len(t, got, tt.wantKept)
+			assert.Equal(t, tt.wantDrops, drops.Total())
+			if tt.wantReason != "" {
+				assert.Contains(t, drops.FormatOneLine(), tt.wantReason)
+			}
+		})
+	}
+}
+
+func TestCSVExcludeOnly_UnidentifiedRowFilteredOutWithoutError(t *testing.T) {
+	unidentified := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1}
+	other := common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro", Count: 1, Account: testOtherAcct, AccountName: "dev"}
+	cfg := Config{IncludeExtendedSupport: true, ExcludeAccounts: []string{testUnidentifiedAcct}}
+	recs := []common.Recommendation{unidentified, other}
+
+	assert.NoError(t, requireResolvedAccountNames(recs, &cfg))
+	got := applyFilters(recs, &cfg, nil, nil, "", common.NewDropSummary())
+	if assert.Len(t, got, 1) {
+		assert.Equal(t, testOtherAcct, got[0].Account)
+	}
+}
+
+func TestPopulateRecommendationAccountNames_UnidentifiedRecUnderExcludeOnly(t *testing.T) {
+	cache := NewAccountAliasCacheWithClient(&MockOrganizationsClient{})
+	recs := []common.Recommendation{{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.micro"}}
+
+	assert.NoError(t, populateRecommendationAccountNames(context.Background(), recs, cache, Config{ExcludeAccounts: []string{testUnidentifiedAcct}}))
+}

docs/cli/filtering.md

@@ -28,6 +28,8 @@ An `--exclude-accounts` value that matches no account exactly but is part of one
 
 When a filter value is a name (anything other than a 12-digit account ID) and an account's name cannot be resolved, cudly stops with an error instead of guessing: carrying on would treat an account you excluded as "not excluded". ID-only filters never need the name. The same rule applies to `--input-csv` replay, where the name comes from the CSV's `AccountName` column; a row whose name column holds the account ID counts as unresolved.
 
+A recommendation with neither an account ID nor an account name (for example a CSV row with empty `Account` and `AccountName`) cannot be placed by an account filter, so it is dropped under either `--include-accounts` or `--exclude-accounts`. On live runs the end-of-run drop summary itemises it as `unidentified-account (--include-accounts)` or `unidentified-account (--exclude-accounts)` (the include label when both filters are set), provided it passed every other filter. On `--input-csv` runs there is no drop summary, so such rows are counted only in the "filtered out N" line.
+
 ```bash
 # Only process two accounts
 cudly --services rds --include-accounts prod-account,staging-account