fix(gcp): skip a recommendation with a bad amount and cap string VCPU amounts

providers/gcp/services/computeengine/client.go

@@ -448,7 +448,7 @@ func (c *Client) GetRecommendations(ctx context.Context, p *common.Recommendatio
 			continue
 		}
 
-		converted, err := c.convertGCPRecommendation(ctx, rec, params)
+		converted, err := c.convertOrSkipBadAmount(ctx, rec, params)
 		if err != nil {
 			return nil, fmt.Errorf("computeengine: recommendation %q: %w", rec.GetName(), err)
 		}
@@ -458,6 +458,19 @@ func (c *Client) GetRecommendations(ctx context.Context, p *common.Recommendatio
 	}
 }
 
+// convertOrSkipBadAmount converts rec, returning nil with a logged warning when
+// its amount is malformed so one bad row does not discard the rest of the
+// project's recommendations (same policy as the AWS and Azure paths).
+// Structural payload errors are still returned and abort the batch.
+func (c *Client) convertOrSkipBadAmount(ctx context.Context, rec *recommenderpb.Recommendation, params common.RecommendationParams) (*common.Recommendation, error) {
+	converted, err := c.convertGCPRecommendation(ctx, rec, params)
+	if errors.Is(err, errBadAmount) {
+		log.Printf("computeengine: skipping recommendation %q: %v", rec.GetName(), err)
+		return nil, nil
+	}
+	return converted, err
+}
+
 // GetExistingCommitments retrieves existing Compute Engine CUDs.
 func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitment, error) {
 	svc, err := c.createCommitmentsService(ctx)
@@ -1417,37 +1430,45 @@ func recommendationVCPUAmount(value *structpb.Value) (int, error) {
 		return 0, err
 	}
 	if amount > math.MaxInt {
-		return 0, fmt.Errorf("VCPU amount must be positive and fit int")
+		return 0, fmt.Errorf("%w: VCPU amount must fit int", errBadAmount)
 	}
 	return int(amount), nil
 }
 
+// errBadAmount marks a malformed or out-of-range resource amount, which makes
+// GetRecommendations skip that one recommendation instead of failing the batch.
+var errBadAmount = errors.New("bad recommendation amount")
+
 // maxExactAmount is the largest integer a float64 (JSON number, or the
 // ComputeDetails.MemoryGB downstream) represents exactly.
 const maxExactAmount = 1<<53 - 1
 
 // recommendationAmount parses a resource amount that the Recommender may
-// carry as a decimal int64 string or as an exact positive JSON number (at most
-// 2^53-1, beyond which a JSON number has already lost precision).
+// carry as a decimal int64 string or as an exact positive JSON number. Both
+// forms are capped at 2^53-1: beyond it a JSON number has already lost
+// precision, and ComputeDetails.MemoryGB (float64) cannot hold a larger value.
 func recommendationAmount(value *structpb.Value, name string) (int64, error) {
 	var amount int64
 	switch v := value.GetKind().(type) {
 	case *structpb.Value_StringValue:
 		parsed, err := strconv.ParseInt(v.StringValue, 10, 64)
 		if err != nil {
-			return 0, fmt.Errorf("invalid %s amount %q: %w", name, v.StringValue, err)
+			return 0, fmt.Errorf("%w: invalid %s amount %q: %w", errBadAmount, name, v.StringValue, err)
+		}
+		if parsed > maxExactAmount {
+			return 0, fmt.Errorf("%w: %s amount must be at most 2^53-1", errBadAmount, name)
 		}
 		amount = parsed
 	case *structpb.Value_NumberValue:
 		if math.IsNaN(v.NumberValue) || v.NumberValue > maxExactAmount || math.Trunc(v.NumberValue) != v.NumberValue {
-			return 0, fmt.Errorf("%s amount must be a positive exact integer", name)
+			return 0, fmt.Errorf("%w: %s amount must be a positive exact integer", errBadAmount, name)
 		}
 		amount = int64(v.NumberValue)
 	default:
-		return 0, fmt.Errorf("%s amount must be a decimal string or number", name)
+		return 0, fmt.Errorf("%w: %s amount must be a decimal string or number", errBadAmount, name)
 	}
 	if amount <= 0 {
-		return 0, fmt.Errorf("%s amount must be positive", name)
+		return 0, fmt.Errorf("%w: %s amount must be positive", errBadAmount, name)
 	}
 	return amount, nil
 }
@@ -1477,10 +1498,6 @@ func memoryMBFromOperationGroups(content *recommenderpb.RecommendationContent) (
 			if err != nil {
 				return 0, err
 			}
-			// MemoryGB is a float64, so larger string amounts would not survive the round trip.
-			if memMB > maxExactAmount {
-				return 0, fmt.Errorf("MEMORY amount must be at most 2^53-1")
-			}
 		}
 	}
 	return memMB, nil

providers/gcp/services/computeengine/client_test.go

@@ -1704,13 +1704,14 @@ func TestRecommendationVCPUAmountRejectsInvalidQuantities(t *testing.T) {
 		structpb.NewNumberValue(math.Inf(-1)), structpb.NewNumberValue(1 << 53), structpb.NewNumberValue(float64(math.MaxInt)),
 		structpb.NewStringValue(""), structpb.NewStringValue("0"), structpb.NewStringValue("-1"),
 		structpb.NewStringValue("1.5"), structpb.NewStringValue("1e3"), structpb.NewStringValue("9223372036854775808"),
+		structpb.NewStringValue("9007199254740992"), structpb.NewStringValue(strconv.FormatInt(int64(math.MaxInt), 10)),
 	} {
 		_, err := recommendationVCPUAmount(value)
 		require.Error(t, err, "value %v", value)
 	}
-	count, err := recommendationVCPUAmount(structpb.NewStringValue(strconv.FormatInt(int64(math.MaxInt), 10)))
+	count, err := recommendationVCPUAmount(structpb.NewStringValue("9007199254740991"))
 	require.NoError(t, err)
-	assert.Equal(t, math.MaxInt, count)
+	assert.Equal(t, 1<<53-1, count)
 }
 
 func TestConvertGCPRecommendationMemoryAmountRepresentations(t *testing.T) {
@@ -2417,3 +2418,71 @@ func TestGetRecommendationsStringMemoryAmountEndToEnd(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, int64(6144), amount)
 }
+
+func badAmountRecommendation(name, vcpu, memory string) *recommenderpb.Recommendation {
+	rec := commitmentOnlyCUDRecommendation()
+	rec.Name = name
+	rec.StateInfo = &recommenderpb.RecommendationStateInfo{State: recommenderpb.RecommendationStateInfo_ACTIVE}
+	ops := rec.Content.OperationGroups[0].Operations
+	ops[0].PathValue = &recommenderpb.Operation_Value{Value: structpb.NewStringValue(vcpu)}
+	ops[1].PathValue = &recommenderpb.Operation_Value{Value: structpb.NewStringValue(memory)}
+	return rec
+}
+
+func TestGetRecommendations_SkipsBadAmountRowKeepsOthers(t *testing.T) {
+	cases := []struct{ name, vcpu, memory string }{
+		{"malformed vcpu", "abc", "6144"},
+		{"malformed memory", "4", "6Gi"},
+		{"oversized vcpu string", "9007199254740992", "6144"},
+		{"oversized memory string", "4", "9223372036854775807"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			client, _ := NewClient(ctx, "test-project", "us-central1")
+			client.SetRecommenderClient(&MockRecommenderClient{iterator: &MockRecommenderIterator{
+				recommendations: []*recommenderpb.Recommendation{
+					badAmountRecommendation("good-before", "4", "6144"),
+					badAmountRecommendation("bad", tc.vcpu, tc.memory),
+					badAmountRecommendation("good-after", "8", "12288"),
+				},
+			}})
+
+			recs, err := client.GetRecommendations(ctx, &common.RecommendationParams{})
+			require.NoError(t, err)
+			require.Len(t, recs, 2)
+			assert.Equal(t, 4, recs[0].Count)
+			assert.Equal(t, 8, recs[1].Count)
+		})
+	}
+}
+
+func TestGetRecommendations_StructuralErrorsStillAbort(t *testing.T) {
+	cases := map[string]func(*recommenderpb.Recommendation){
+		"duplicate vcpu": func(r *recommenderpb.Recommendation) {
+			g := r.Content.OperationGroups[0]
+			g.Operations = append(g.Operations, g.Operations[0])
+		},
+		"unknown resource type": func(r *recommenderpb.Recommendation) {
+			r.Content.OperationGroups[0].Operations[1].PathFilters["/resources/1/type"] = structpb.NewStringValue("BOGUS")
+		},
+		"missing vcpu": func(r *recommenderpb.Recommendation) {
+			r.Content.OperationGroups[0].Operations = r.Content.OperationGroups[0].Operations[1:]
+		},
+	}
+	for name, mutate := range cases {
+		t.Run(name, func(t *testing.T) {
+			ctx := context.Background()
+			client, _ := NewClient(ctx, "test-project", "us-central1")
+			bad := badAmountRecommendation("bad", "4", "6144")
+			mutate(bad)
+			client.SetRecommenderClient(&MockRecommenderClient{iterator: &MockRecommenderIterator{
+				recommendations: []*recommenderpb.Recommendation{badAmountRecommendation("good", "4", "6144"), bad},
+			}})
+
+			recs, err := client.GetRecommendations(ctx, &common.RecommendationParams{})
+			require.Error(t, err)
+			assert.Nil(t, recs)
+		})
+	}
+}