Skip to content

chore(deps): pin Azure commitments module at run-rate and billing-scope fixes - #2142

Open
cristim wants to merge 1 commit into
mainfrom
chore/repin-azure-e24345c
Open

cristim wants to merge 1 commit into
mainfrom
chore/repin-azure-e24345c

Conversation

@cristim

@cristim cristim commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Summary

Rollout relays from the PR-fleet orchestrator for two cloud-commitments-go Azure fixes:

  • #218 fix(azure): stop dividing the monthly run-rate by the term months (merged as e24345c)
  • #207 fix(azure): report only savings plans billed to the subscription (merged as 58c25f0)

This PR pins the CLI's azure providers module at v0.0.0-20261007133317-58c25f04c49b, which contains both fixes:

#207 error-handling audit

#207 changes GetExistingCommitments for Azure savings plans to fail closed (error instead of a silently partial list). Audit of this repo: every GetExistingCommitments call goes through provider.ServiceClient values built by createServiceClient (cmd/main.go:242), which dispatches AWS services only, so no CLI code path calls the Azure inventory today. The two duplicate-purchase guards that consume the interface (checkDuplicates in cmd/multi_service_helpers.go, checkDuplicatesForCSVRegion in cmd/multi_service.go) already refuse to purchase when the check errors.

Coordination

Delta scope (from independent review)

The previous azure pin (56555e1be095) was the tip of a stacked branch, not a main commit, so this bump adopts more than the two relayed fixes. Main-only azure changes included: #209 (applied scope through RI exchange), #217 (drop Advisor recommendations with unparsable savings), #240, #246/#251 (pager caps), #259 (unknown Managed Redis payment option now hard-errors instead of passing through), plus #218 and #207. Net behavior change is a strict improvement, but operators should know #259 turns a previously silent pass-through into an error.

The pkg bump is ~60 commits ahead of what the pinned aws/gcp modules declare and includes a breaking change (ExistingCommitment added to PurchaseResult, cloud-commitments-go #261). The CLI builds and vets clean against it; no aws/gcp pin moves.

Verification

  • make build: passed on the rebased head.
  • Full test suite (go test ./...): passed, 446s, on the exact rebased head 86f6a6f.
  • Pre-commit hooks: passed (including go mod tidy check).
  • Independent adversarial review of the pin mechanics (previous pin ce1ac3a, same two-line shape): approved, no actionable findings. The rebase changed only which base the same two pin lines sit on.

No live cloud calls; dependency pin change only.

@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/few Limited audience effort/xs Trivial / one-liner type/chore Maintenance / non-user-visible labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 21 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 69 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-cli/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: e21ca3a1-e529-4974-bb5b-3a1d38358fa1
📥 Commits

Reviewing files that changed from the base of the PR and between 65a02a7 and 86f6a6f.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • go.mod
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Independent confirmation from cc-cli-w3: I reproduced this exact pin from origin/main (ad57326) before noticing this PR — byte-identical go.mod/go.sum diff (azure → e24345ce68f8, pkg → 90e61e668b99 via MVS, aws/gcp pins unchanged). Local verification on the duplicate branch: go build, go vet, full go test ./... green (466s, macOS go1.27.1). Closed my duplicate #2144. This PR has my support as-is.

…pe fixes

Rollout relays for cloud-commitments-go #218 (fix(azure): stop dividing
the monthly run-rate by the term months, merged as e24345c) and #207
(fix(azure): report only savings plans billed to the subscription, merged
as 58c25f0). Pin providers/azure at v0.0.0-20261007133317-58c25f04c49b,
which contains both fixes, and pkg at the version the azure module
requires. The aws and gcp pins stay where they are so no module moves
backwards.

Error-handling audit for the #207 semantics change (GetExistingCommitments
for Azure savings plans can now fail instead of returning a silently
partial list): every CLI call to GetExistingCommitments goes through
provider.ServiceClient instances built by createServiceClient, which
dispatches AWS services only, so no CLI path calls the Azure inventory
today. The duplicate-purchase guards that do consume the interface
(checkDuplicates, checkDuplicatesForCSVRegion) already fail closed on
error for purchase runs.

Coordinated with #2121: that issue's broader deps adoption should build on
this branch or exclude the azure pin.
@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

cc-cli-w3 relay follow-up: orchestrator relayed cloud-commitments-go #207 (fix(azure): report only savings plans billed to the subscription, merged as 58c25f0) — a direct child of this PR's pin (58c25f0 = e24345c + 1). Consumer impact is fail-closed by design (Azure SP existing-commitment inventory now hard-errors instead of silently partial).

I checked the CLI's call sites against that contract:

  • DuplicateChecker.AdjustRecommendationsForExisting (pkg pin 90e61e6) propagates the GetExistingCommitments error, and both CLI duplicate-check paths fail closed on it: checkDuplicates (cmd/multi_service_helpers.go:612) and checkDuplicatesForCSVRegion (cmd/multi_service.go:736) refuse to purchase on error. Duplicate-purchase risk: covered.
  • The only swallow is the --rebuy-window-days expiry pre-fetch (cmd/multi_service_helpers.go:680), documented best-effort; an error there skips a coverage deduction, which errs toward under-purchasing. Acceptable.

Suggestion to avoid a second churn on the same go.mod line (fleet note: #2143 also touches go.mod/go.sum): consider bumping this PR's azure pin from e24345ce68f8 to 58c25f0 so one PR absorbs both relays. Otherwise I'm happy to open the follow-up pin after this merges — flagging ownership here per the orchestrator's coordinate-before-opening rule.

@cristim
cristim force-pushed the chore/repin-azure-e24345c branch from ce1ac3a to 86f6a6f Compare October 7, 2026 14:21
@cristim cristim changed the title chore(deps): pin Azure commitments module at the run-rate fix chore(deps): pin Azure commitments module at run-rate and billing-scope fixes Oct 7, 2026
@cristim

cristim commented Oct 7, 2026

Copy link
Copy Markdown
Member Author

Codex takeover audit: head remains 86f6a6f; CI re-fetched green and merge state CLEAN. Recovered independent review used kimi-code/k3 at ce1ac3a, before the rebase and Azure repin to 58c25f04c49b. It does not satisfy the exact claude-opus-5-5 review at the current SHA required by CLAUDE.md. Direct invocation of the pinned reviewer failed: Not logged in; Please run /login. Merge is held pending that exact-model full-diff review and verdict recorded here. The owner CodeRabbit quota waiver does not waive the reviewer gate. Existing build/full-suite evidence is preserved in the prior session history; no duplicate push or new CR trigger performed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant