From 0fb5cd20aa9b8cc721b89a39cfae1c1462df31f8 Mon Sep 17 00:00:00 2001 From: Patrick Dillon Date: Wed, 29 Jul 2026 18:46:55 -0400 Subject: [PATCH 1/2] GCP: gracefully handle 503 in disk type check Add 500-type errors, such as 503 Service Unavailable to the graceful handling of the disk type validation. The disk type validation is supposed to prevent known failures, but if for some reason the API call fails we do not want that in and of itself to be fatal. That was the original design of the validation, this commit just adds 503 to that handling. (cherry picked from commit 9fb3564b853d75837f5bca6c126b6b9f79c48758) --- pkg/asset/installconfig/gcp/validation.go | 5 +++-- .../installconfig/gcp/validation_test.go | 21 ++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index b3b3d612cba..9dc86066b13 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -202,10 +202,11 @@ func validateDiskTypeAvailability(client API, fieldPath *field.Path, project, re dt, dtZones, err := client.GetDiskTypeWithZones(context.TODO(), project, region, diskType) if err != nil { var gerr *googleapi.Error - if errors.As(err, &gerr) { + if errors.As(err, &gerr) && gerr.Code < 500 { return append(allErrs, field.Invalid(fieldPath.Child("diskType"), diskType, err.Error())) } - return append(allErrs, field.InternalError(fieldPath.Child("diskType"), err)) + logrus.Warnf("could not verify disk type %s availability in %s, skipping API check: %v", diskType, region, err) + return allErrs } if dt == nil { diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 601b232f3fb..2e0330562f7 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -1465,6 +1465,7 @@ func TestValidateDiskTypeAvailability(t *testing.T) { mockErr error expectedError bool expectedErrMsg string + expectedWarn string }{ { name: "Empty disk type is a no-op", @@ -1514,16 +1515,22 @@ func TestValidateDiskTypeAvailability(t *testing.T) { expectedErrMsg: `forbidden`, }, { - name: "Non-API error returns internal error", - diskType: "pd-ssd", - mockErr: fmt.Errorf("network timeout"), - expectedError: true, - expectedErrMsg: `network timeout`, + name: "GCP 503 server error degrades gracefully", + diskType: "pd-ssd", + mockErr: &googleapi.Error{Code: http.StatusServiceUnavailable, Message: "backend error"}, + expectedWarn: `could not verify disk type pd-ssd availability`, + }, + { + name: "Non-API error degrades gracefully", + diskType: "pd-ssd", + mockErr: fmt.Errorf("network timeout"), + expectedWarn: `could not verify disk type pd-ssd availability`, }, } for _, test := range cases { t.Run(test.name, func(t *testing.T) { + hook := logrusTest.NewGlobal() mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() gcpClient := mock.NewMockAPI(mockCtrl) @@ -1539,6 +1546,10 @@ func TestValidateDiskTypeAvailability(t *testing.T) { } else { assert.Empty(t, errs) } + if test.expectedWarn != "" { + assert.NotEmpty(t, hook.Entries) + assert.Regexp(t, test.expectedWarn, hook.LastEntry().Message) + } }) } } From 5eab36319e61390c2fd817ce0c2bc3e5904e52c4 Mon Sep 17 00:00:00 2001 From: barbacbd Date: Wed, 9 Sep 2026 12:49:16 -0400 Subject: [PATCH 2/2] OCPBUGS-122133: gcp: skip disk type check when the permission is denied The disk type availability check queries compute.diskTypes.get once per zone. Service accounts holding only the documented minimum install permissions do not have that permission, so the lookup returns 403 and the install fails before any resource is created: platform.gcp.defaultMachinePlatform.diskType: Invalid value: "pd-ssd": googleapi: Error 403: Required 'compute.diskTypes.get' permission The disk type is valid; only the check is unavailable. Reading a disk type is not needed to install, so treat a denial the same way as a backend error and skip the check rather than report a bad value. Co-Authored-By: Claude Opus 5 --- pkg/asset/installconfig/gcp/validation.go | 7 ++++++- pkg/asset/installconfig/gcp/validation_test.go | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 9dc86066b13..b5569d6a337 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net" + "net/http" "slices" "strings" @@ -201,8 +202,12 @@ func validateDiskTypeAvailability(client API, fieldPath *field.Path, project, re dt, dtZones, err := client.GetDiskTypeWithZones(context.TODO(), project, region, diskType) if err != nil { + // Reading a disk type is not required to install, so a service account + // holding only the minimum set of install permissions is denied here. That + // says nothing about the configured disk type, so skip the check rather + // than report a bad value. var gerr *googleapi.Error - if errors.As(err, &gerr) && gerr.Code < 500 { + if errors.As(err, &gerr) && gerr.Code < 500 && gerr.Code != http.StatusForbidden { return append(allErrs, field.Invalid(fieldPath.Child("diskType"), diskType, err.Error())) } logrus.Warnf("could not verify disk type %s availability in %s, skipping API check: %v", diskType, region, err) diff --git a/pkg/asset/installconfig/gcp/validation_test.go b/pkg/asset/installconfig/gcp/validation_test.go index 2e0330562f7..7b31a7a78da 100644 --- a/pkg/asset/installconfig/gcp/validation_test.go +++ b/pkg/asset/installconfig/gcp/validation_test.go @@ -1510,9 +1510,15 @@ func TestValidateDiskTypeAvailability(t *testing.T) { { name: "GCP API error returns field error", diskType: "pd-ssd", - mockErr: &googleapi.Error{Code: http.StatusForbidden, Message: "forbidden"}, + mockErr: &googleapi.Error{Code: http.StatusBadRequest, Message: "bad request"}, expectedError: true, - expectedErrMsg: `forbidden`, + expectedErrMsg: `bad request`, + }, + { + name: "GCP 403 permission denied degrades gracefully", + diskType: "pd-ssd", + mockErr: &googleapi.Error{Code: http.StatusForbidden, Message: "Required 'compute.diskTypes.get' permission"}, + expectedWarn: `could not verify disk type pd-ssd availability`, }, { name: "GCP 503 server error degrades gracefully",