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
31 changes: 30 additions & 1 deletion internal/config/recommendation_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,34 @@ func (r *RecommendationRecord) SuppressionVariant() (SuppressionVariant, error)
if err != nil {
return SuppressionVariant{}, err
}
return SuppressionVariant{AvailabilityZone: r.AvailabilityZone, Platform: ci.Platform, Tenancy: ci.Tenancy}, nil
v := SuppressionVariant{AvailabilityZone: r.AvailabilityZone, Platform: ci.Platform, Tenancy: ci.Tenancy}
if r.Provider == "aws" && CanonicalSuppressionService(r.Provider, r.Service) == "compute" {
v.Tenancy = canonicalSuppressionTenancy(v.Tenancy)
if v.AvailabilityZone == "" {
// Rows saved 2026-10-09..12 hold the zone only in Details (#1232).
v.AvailabilityZone = r.detailsAvailabilityZone()
}
}
return v, nil
}

// canonicalSuppressionTenancy maps the pre-2026-05-20 EC2 tenancy spelling
// "shared" to today's "default" so old and new suppressions and recs match.
func canonicalSuppressionTenancy(t string) string {
if t == "shared" {
return "default"
}
return t
}

// detailsAvailabilityZone reads the zonal AZ from Details; "" when absent or
// unreadable (ComputeIdentity has already reported a decode error).
func (r *RecommendationRecord) detailsAvailabilityZone() string {
var d struct {
AvailabilityZone string `json:"availability_zone"`
}
if json.Unmarshal(r.Details, &d) != nil {
return ""
}
return d.AvailabilityZone
}
34 changes: 34 additions & 0 deletions internal/config/suppressions_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,37 @@ func TestBuildSuppressions_CanonicalisesAWSEC2Slug(t *testing.T) {
assert.Equal(t, "compute", sups[1].Service)
assert.Equal(t, "azure", sups[1].Provider)
}

// TestBuildSuppressions_LegacyShapesCanonicalised pins the two legacy EC2
// shapes of issue #1232: Details tenancy "shared" (pre-2026-05-20) is the same
// capacity as "default", and an AZ stored only in Details is used when the AZ
// field is empty (the field wins when both are set).
func TestBuildSuppressions_LegacyShapesCanonicalised(t *testing.T) {
mk := func(service, field, details string) RecommendationRecord {
return RecommendationRecord{Provider: "aws", Service: service, Region: "us-east-1", ResourceType: "m5.large",
Count: 1, AvailabilityZone: field, Details: []byte(details)}
}
for name, tc := range map[string]struct {
rec RecommendationRecord
want SuppressionVariant
}{
"legacy shared tenancy": {mk("ec2", "us-east-1a", `{"platform":"Linux/UNIX","tenancy":"shared"}`), SuppressionVariant{"us-east-1a", "Linux/UNIX", "default"}},
"compute slug shared": {mk("compute", "", `{"platform":"Linux/UNIX","tenancy":"shared"}`), SuppressionVariant{"", "Linux/UNIX", "default"}},
"AZ only in details": {mk("ec2", "", `{"availability_zone":"us-east-1a","platform":"Linux/UNIX","tenancy":"default"}`), SuppressionVariant{"us-east-1a", "Linux/UNIX", "default"}},
"field wins over details": {mk("ec2", "us-east-1b", `{"availability_zone":"us-east-1a","platform":"Linux/UNIX","tenancy":"default"}`), SuppressionVariant{"us-east-1b", "Linux/UNIX", "default"}},
"modern rec unchanged": {mk("ec2", "us-east-1a", `{"availability_zone":"us-east-1a","platform":"Linux/UNIX","tenancy":"dedicated"}`), SuppressionVariant{"us-east-1a", "Linux/UNIX", "dedicated"}},
"non-EC2 untouched": {RecommendationRecord{Provider: "aws", Service: "rds", Region: "r", ResourceType: "db", Count: 1, Details: []byte(`{"availability_zone":"us-east-1a","tenancy":"shared"}`)}, SuppressionVariant{"", "", "shared"}},
} {
t.Run(name, func(t *testing.T) {
sups := BuildSuppressions([]RecommendationRecord{tc.rec}, "exec-1", &GlobalConfig{}, time.Now())
require.Len(t, sups, 1)
assert.Equal(t, &tc.want, sups[0].Variant)
})
}
}

func TestPurchaseSuppression_AppliesTo_LegacySharedTenancy(t *testing.T) {
stored := PurchaseSuppression{Variant: &SuppressionVariant{"us-east-1a", "Linux/UNIX", "shared"}}
assert.True(t, stored.AppliesTo(&SuppressionVariant{"us-east-1a", "Linux/UNIX", "default"}), "old stored shared row must match a default rec")
assert.False(t, stored.AppliesTo(&SuppressionVariant{"us-east-1a", "Linux/UNIX", "dedicated"}))
}
7 changes: 6 additions & 1 deletion internal/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,12 @@ func CanonicalSuppressionService(provider, service string) string {
// matches every row: both fall back to the pre-#816 6-tuple match, which
// over-suppresses (under-buys) rather than over-shows.
func (s *PurchaseSuppression) AppliesTo(v *SuppressionVariant) bool {
return s.Variant == nil || v == nil || *s.Variant == *v
if s.Variant == nil || v == nil {
return true
}
a, b := *s.Variant, *v
a.Tenancy, b.Tenancy = canonicalSuppressionTenancy(a.Tenancy), canonicalSuppressionTenancy(b.Tenancy)
return a == b
}

// RecommendationFilter parameterises ListStoredRecommendations and the
Expand Down
37 changes: 37 additions & 0 deletions internal/scheduler/scheduler_suppressions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,40 @@ func TestApplySuppressions_AWSLegacySlugSpellings(t *testing.T) {
})
}
}

// TestApplySuppressions_LegacyEC2Shapes covers the two legacy shapes of issue
// #1232 end to end: a suppression stored with tenancy "shared", or built from a
// rec whose AZ lives only in Details, must still hide the modern rec.
func TestApplySuppressions_LegacyEC2Shapes(t *testing.T) {
modern := ec2VariantRec("modern", "us-east-1a", "Linux/UNIX", "default")
azOnlyInDetails := modern
azOnlyInDetails.AvailabilityZone = ""
sharedPurchase := ec2VariantRec("p", "us-east-1a", "Linux/UNIX", "shared")
exp := time.Now().Add(time.Hour)
for name, tc := range map[string]struct {
rec config.RecommendationRecord
sups []config.PurchaseSuppression
want int
}{
"stored shared row vs default rec": {modern, []config.PurchaseSuppression{{ExecutionID: "e", AccountID: "acct-1", Provider: "aws", Service: "compute", Region: "us-east-1",
ResourceType: "m5.large", SuppressedCount: 2, ExpiresAt: exp, Variant: &config.SuppressionVariant{AvailabilityZone: "us-east-1a", Platform: "Linux/UNIX", Tenancy: "shared"}}}, 3},
"built from shared rec": {modern, config.BuildSuppressions([]config.RecommendationRecord{withCount(sharedPurchase, 2)}, "e",
&config.GlobalConfig{GracePeriodDays: map[string]int{"aws": 7}}, time.Now()), 3},
"rec AZ only in details, row has AZ": {azOnlyInDetails, config.BuildSuppressions([]config.RecommendationRecord{withCount(modern, 2)}, "e",
&config.GlobalConfig{GracePeriodDays: map[string]int{"aws": 7}}, time.Now()), 3},
"purchase AZ only in details": {modern, config.BuildSuppressions([]config.RecommendationRecord{withCount(azOnlyInDetails, 2)}, "e",
&config.GlobalConfig{GracePeriodDays: map[string]int{"aws": 7}}, time.Now()), 3},
} {
t.Run(name, func(t *testing.T) {
index := aggregateSuppressions(tc.sups)
out := applySuppressionIndex([]config.RecommendationRecord{tc.rec}, index)
require.Len(t, out, 1)
assert.Equal(t, tc.want, out[0].Count)
})
}
}

func withCount(r config.RecommendationRecord, n int) config.RecommendationRecord {
r.Count = n
return r
}
Loading