From 59ee19916d45b446006a5fc1c01e82fd5b5bcb67 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 12 Oct 2026 15:37:40 +0200 Subject: [PATCH] chore(deps): bump cloud-commitments-go pins to 9dae64b and adapt archera key Bumps pkg and providers/aws|azure|gcp to 9dae64b216d4. The Archera key is now an insurance.Secret and is trimmed, since the library rejects non-printable key bytes such as a trailing newline. Adds tests for the zonal EC2 AZ path (kept AZ reaches the service client; AZ-less stale row fails closed) and the key trim. Refs #791 Co-Authored-By: Claude Sonnet 5.5 --- go.mod | 8 +- go.sum | 16 ++-- internal/api/purchase_pricing_test.go | 10 +++ internal/archera/provider.go | 3 +- internal/archera/provider_test.go | 15 ++++ internal/purchase/zonal_az_test.go | 89 +++++++++++++++++++ .../scheduler/recommendation_identity_test.go | 16 ++++ 7 files changed, 144 insertions(+), 13 deletions(-) create mode 100644 internal/purchase/zonal_az_test.go diff --git a/go.mod b/go.mod index 4e98e90a..3b8fff1b 100644 --- a/go.mod +++ b/go.mod @@ -84,10 +84,10 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261008151559-73d3366903bc - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009122502-bcc0dfc49227 - github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007130138-e24345ce68f8 - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009093408-2c18f303abfc + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012130811-9dae64b216d4 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261012130811-9dae64b216d4 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261012130811-9dae64b216d4 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261012130811-9dae64b216d4 github.com/aws/aws-lambda-go v1.47.0 github.com/aws/aws-sdk-go-v2/service/kms v1.50.4 github.com/aws/aws-sdk-go-v2/service/lambda v1.89.0 diff --git a/go.sum b/go.sum index fac5a9c3..a21fc96e 100644 --- a/go.sum +++ b/go.sum @@ -92,14 +92,14 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261008151559-73d3366903bc h1:GTjjeD1cGMn+LfTSRB7CfO6kJBrfQyHu3kZAtHx1TJk= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261008151559-73d3366903bc/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009122502-bcc0dfc49227 h1:S9pAwtTnwt02+Fo1uVWYgtmGyB1koQAHmBSficHOZ2E= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261009122502-bcc0dfc49227/go.mod h1:yJoe/zCikjjlGsy47BZTTuKUcLkLVDwqgLEYklWjlm4= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007130138-e24345ce68f8 h1:4qx5bagtW25H5X+whaZ/3tWXP7VGyG/qcypTh8yK7Hc= -github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007130138-e24345ce68f8/go.mod h1:iIvPuWLWd+SXqLHAQNdodYMUAAcq7PnwfScXWgARt5A= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009093408-2c18f303abfc h1:hCLUvpI5U42BRh4M9EvjKaBg70P2ijbKxO9eNn7wMK0= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261009093408-2c18f303abfc/go.mod h1:D2ZwYqlDm/304JWfOw8YX/e+/YTc51Q9kiIaHJN7nxY= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012130811-9dae64b216d4 h1:4C/4ZLemkgC45z47LtCjICglnfcTBzP2KUAq/gZEzZQ= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261012130811-9dae64b216d4/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261012130811-9dae64b216d4 h1:fnEv+2eCL0ObXn1nAFlBa2kZy1A0/nhc7LxYeDFCCkQ= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261012130811-9dae64b216d4/go.mod h1:C0ildZeRds0aRvDZp49MLY0kFWh/4FiM4lPWWWd3HFA= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261012130811-9dae64b216d4 h1:HljKKZOmfam4C3T5109DMeUQuyxVHRohroA4j/OU5oY= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261012130811-9dae64b216d4/go.mod h1:nM3Zqd5N2mMIsNoTtnKF6RFyM1E3HVjAGIsXVx9b3bY= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261012130811-9dae64b216d4 h1:o9hnEg+Ro/RynW8ZWDVMC0E2yWt08izWeFbrCsZZx/c= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261012130811-9dae64b216d4/go.mod h1:uJ7UpLaUqu1fpfCEECuqjLXV+As2tibtPCapPF3AZys= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/aws/aws-lambda-go v1.47.0 h1:0H8s0vumYx/YKs4sE7YM0ktwL2eWse+kfopsRI1sXVI= diff --git a/internal/api/purchase_pricing_test.go b/internal/api/purchase_pricing_test.go index f19dcb67..dc857384 100644 --- a/internal/api/purchase_pricing_test.go +++ b/internal/api/purchase_pricing_test.go @@ -1202,3 +1202,13 @@ func TestStaleAzurePricingErrorMapping(t *testing.T) { assert.Equal(t, 500, ce.code) assert.ErrorIs(t, tokenActionError(audit), config.ErrAuditLoss) } + +// Fixture-based: the go EC2 client's AZ-less zonal refusal reaches the user as +// a 502 whose message keeps the "re-fetch" instruction. +func TestExecuteFailureError_ZonalRefusalIs502WithRefetchMessage(t *testing.T) { + refusal := fmt.Errorf("EC2 recommendation for m5.large has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation") + ce, ok := IsClientError(executeFailureError("id", "approved", &config.PurchaseExecution{Status: "failed"}, refusal)) + require.True(t, ok) + assert.Equal(t, 502, ce.code) + assert.Contains(t, ce.message, "re-fetch") +} diff --git a/internal/archera/provider.go b/internal/archera/provider.go index b2c9d3ee..81545e92 100644 --- a/internal/archera/provider.go +++ b/internal/archera/provider.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "os" + "strings" "sync" "sync/atomic" @@ -135,7 +136,7 @@ func (p *Provider) Client(ctx context.Context) (insurance.QuoteClient, error) { logging.Warnf("archera: resolving %s failed: %v", EnvKeySecret, err) return nil, ErrKeyUnavailable } - c, err := insurance.NewClient(insurance.Config{APIKey: key, OrgID: p.settings.OrgID}, p.hc) + c, err := insurance.NewClient(insurance.Config{APIKey: insurance.NewSecret(strings.TrimSpace(key)), OrgID: p.settings.OrgID}, p.hc) if err != nil { logging.Warnf("archera: building client failed: %v", err) return nil, ErrKeyUnavailable diff --git a/internal/archera/provider_test.go b/internal/archera/provider_test.go index 3fa3fa2d..608815b9 100644 --- a/internal/archera/provider_test.go +++ b/internal/archera/provider_test.go @@ -175,3 +175,18 @@ func TestComposition_EnvToDecodedComparison_AndRedaction(t *testing.T) { assertNoKey("after a successful comparison (client now built)") assert.NotContains(t, fmt.Sprintf("%+v %v %#v", c, c, c), testKey) } + +// A secret stored with a trailing newline (common with files and CLI-created +// secrets) must still build a client: the library rejects non-printable key +// bytes, which would otherwise turn every comparison into ErrKeyUnavailable. +func TestClient_KeyWithTrailingNewlineIsTrimmed(t *testing.T) { + t.Setenv(testSecret, testKey+"\n") + rt := &hostAssertingTransport{t: t, body: comparison} + p := NewProvider(fullSettings(), secrets.NewEnvResolver(), &http.Client{Transport: rt}) + + c, err := p.Client(context.Background()) + require.NoError(t, err) + _, err = c.Comparison(context.Background(), insurance.ComparisonRequest{PlanID: p.PlanID()}) + require.NoError(t, err) + assert.Equal(t, 1, int(rt.calls.Load())) +} diff --git a/internal/purchase/zonal_az_test.go b/internal/purchase/zonal_az_test.go new file mode 100644 index 00000000..86f6e480 --- /dev/null +++ b/internal/purchase/zonal_az_test.go @@ -0,0 +1,89 @@ +package purchase + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + awsec2sdk "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" + ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +func zonalRecRecord(t *testing.T, az string) config.RecommendationRecord { + t.Helper() + blob, err := common.MarshalServiceDetails(&common.ComputeDetails{ + InstanceType: "m5.large", Platform: "Linux/UNIX", Tenancy: "default", + Scope: "availability-zone", AvailabilityZone: az, + }) + require.NoError(t, err) + return config.RecommendationRecord{ + Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "m5.large", + Count: 1, Term: 1, Payment: "no-upfront", Details: blob, + } +} + +func managerServing(sc provider.ServiceClient) *Manager { + prov := new(MockProvider) + prov.On("GetServiceClient", mock.Anything, mock.Anything, "us-east-1").Return(sc, nil) + factory := new(MockProviderFactory) + factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(prov, nil) + return &Manager{providerFactory: factory} +} + +// A zonal rec stored WITH its AZ reaches the service client with the AZ +// (persisted blob -> decode -> PurchaseCommitment). +func TestExecuteSinglePurchase_ZonalRecKeepsAvailabilityZone(t *testing.T) { + sc := new(MockServiceClient) + var got common.Recommendation + sc.On("PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { got = args.Get(1).(common.Recommendation) }). + Return(common.PurchaseResult{Success: true, CommitmentID: "ri-1"}, nil) + + _, err := managerServing(sc).executeSinglePurchase(context.Background(), zonalRecRecord(t, "us-east-1b"), nil, common.PurchaseOptions{ExecutionID: "exec-az"}) + require.NoError(t, err) + + d, ok := got.Details.(*common.ComputeDetails) + require.True(t, ok) + assert.Equal(t, "us-east-1b", d.AvailabilityZone) +} + +// EC2 API stub that fails the test on any call that could spend money. The +// embedded nil interface makes any other call panic. +type noPurchaseEC2API struct { + ec2svc.API + purchases, lookups int +} + +func (a *noPurchaseEC2API) PurchaseReservedInstancesOffering(context.Context, *awsec2sdk.PurchaseReservedInstancesOfferingInput, ...func(*awsec2sdk.Options)) (*awsec2sdk.PurchaseReservedInstancesOfferingOutput, error) { + a.purchases++ + return &awsec2sdk.PurchaseReservedInstancesOfferingOutput{}, nil +} + +func (a *noPurchaseEC2API) DescribeReservedInstancesOfferings(context.Context, *awsec2sdk.DescribeReservedInstancesOfferingsInput, ...func(*awsec2sdk.Options)) (*awsec2sdk.DescribeReservedInstancesOfferingsOutput, error) { + a.lookups++ + return &awsec2sdk.DescribeReservedInstancesOfferingsOutput{}, nil +} + +// A zonal row stored under an older pin has no AZ. The real EC2 client must +// refuse it before any offering lookup or purchase call. +func TestExecuteSinglePurchase_StaleZonalRowWithoutAZFailsClosed(t *testing.T) { + api := &noPurchaseEC2API{} + c := ec2svc.NewClient(awsConfigForTest()) + c.SetEC2API(api) + + _, err := managerServing(c).executeSinglePurchase(context.Background(), zonalRecRecord(t, ""), nil, common.PurchaseOptions{ExecutionID: "exec-stale"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "re-fetch") + assert.Zero(t, api.purchases, "no purchase call") + assert.Zero(t, api.lookups, "no offering lookup") +} + +func awsConfigForTest() aws.Config { return aws.Config{Region: "us-east-1"} } diff --git a/internal/scheduler/recommendation_identity_test.go b/internal/scheduler/recommendation_identity_test.go index ecdec875..0ce15622 100644 --- a/internal/scheduler/recommendation_identity_test.go +++ b/internal/scheduler/recommendation_identity_test.go @@ -115,3 +115,19 @@ func TestRecommendationIdentity_NaturalKeyDimensions(t *testing.T) { }) } } + +// A zonal EC2 rec keeps its AZ in the persisted details blob, so the purchase +// path can buy in that AZ (go#305). +func TestConvertRecommendations_ZonalEC2KeepsAvailabilityZoneInDetails(t *testing.T) { + s := &Scheduler{} + rec := common.Recommendation{ + Provider: common.ProviderAWS, Service: common.ServiceEC2, Region: "us-east-1", ResourceType: "m5.large", Count: 1, Term: "1yr", PaymentOption: "all-upfront", + Details: &common.ComputeDetails{ + InstanceType: "m5.large", Platform: "Linux/UNIX", Tenancy: "default", + Scope: "availability-zone", AvailabilityZone: "us-east-1b", + }, + } + records := s.convertRecommendations([]common.Recommendation{rec}, "aws") + require.Len(t, records, 1) + assert.Contains(t, string(records[0].Details), `"availability_zone":"us-east-1b"`) +}