diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index b3b3d612cba..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,11 +202,16 @@ 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) { + if errors.As(err, &gerr) && gerr.Code < 500 && gerr.Code != http.StatusForbidden { 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..7b31a7a78da 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", @@ -1509,21 +1510,33 @@ 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: "Non-API error returns internal error", - diskType: "pd-ssd", - mockErr: fmt.Errorf("network timeout"), - expectedError: true, - expectedErrMsg: `network timeout`, + 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", + 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 +1552,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) + } }) } }