{{ message }}
Repository navigation
fix(cli): signal unknown extended-support lifecycle instead of silent false - #2138
Merged
Merged
Conversation
… false isInExtendedSupport silently returned false when the engine:majorVersion key was missing from the lifecycle map, so a partial DescribeDBMajorEngineVersions response or an unmapped engine made the extended-support exclusion a silent no-op on a money path: instances accruing extended-support fees stayed in the RI/SP purchase count. - isInExtendedSupport now returns (extended, known); known=false when the major version can't be extracted or the lookup misses - adjustRecommendationForExcludedVersions treats unknown as a hard skip: the count stays untouched and a warning naming engine, version and region is logged once per engine:version pair - tests pin the new contract for the lookup-miss path, including the warning's content and per-version deduplication Closes #1313
Contributor
cristim
added
priority/p2
Member
Author
|
@coderabbitai review |
Contributor
|
3 tasks
Member
Author
|
@coderabbitai full review |
Contributor
|
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1313.
isInExtendedSupportsilently returnedfalsewhen theengine:majorVersionkey was missing from the lifecycle map. On the money path throughadjustRecommendationForExcludedVersions, a partialDescribeDBMajorEngineVersionsresponse or an unmapped engine turned the extended-support exclusion into a silent no-op: instances accruing extended-support fees stayed in the RI/SP purchase count.Changes
isInExtendedSupportnow returns(extended, known bool);known=falsewhen the major version can't be extracted or the lookup misses, so "unknown" is distinguishable from "known: not extended".adjustRecommendationForExcludedVersionstreats unknown as a hard skip (per the issue's suggested fix): the count stays untouched, and a warning naming engine, version and region is logged once per engine:version pair.shouldExcludeForExtendedSupportto keep both functions under the pre-commit gocyclo cap (10); the duplicated inline engine-normalization closure was replaced with the existingnormalizeEngineNameForVersion.TestIsInExtendedSupportandTestIsInExtendedSupport_EdgeCasesnow assert theknownflag on every case; newTestAdjustRecommendationForExcludedVersions_UnknownLifecyclepins the lookup-miss contract (count untouched, warning content, per-version dedup).Out of scope
Audit finding A10-013 (noted on the issue): making
queryRDSInstancesInRegions/queryMajorEngineVersionsWithClienterror when every region/engine query fails, and aborting a--purchaserun when the lifecycle signal is unavailable. That's a larger change to the query/purchase flow; this PR closes the silent-fallback contract the issue's acceptance criteria cover.Verification
gofmt,go vet,go build ./cmdcleango test ./cmd/ -run 'TestIsInExtendedSupport|TestAdjustRecommendationForExcludedVersions'greengo test ./...green locally (macOS, go1.27.1)golangci-lint(2.11.4, built with go1.26.1) crashes against the installed go1.27.1 toolchain — pre-existing environment mismatch, not caused by this change; CI runs its own pinned lint.Fixture/mock-based evidence only; no live AWS calls were made.