Merge pull request #101926 from bhcleek/errors/code-check

check APIStatus.Code in Is* family of functions

staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go

@@ -44,6 +44,28 @@ type APIStatus interface {
 
 var _ error = &StatusError{}
 
+var knownReasons = map[metav1.StatusReason]struct{}{
+	// metav1.StatusReasonUnknown : {}
+	metav1.StatusReasonUnauthorized:          {},
+	metav1.StatusReasonForbidden:             {},
+	metav1.StatusReasonNotFound:              {},
+	metav1.StatusReasonAlreadyExists:         {},
+	metav1.StatusReasonConflict:              {},
+	metav1.StatusReasonGone:                  {},
+	metav1.StatusReasonInvalid:               {},
+	metav1.StatusReasonServerTimeout:         {},
+	metav1.StatusReasonTimeout:               {},
+	metav1.StatusReasonTooManyRequests:       {},
+	metav1.StatusReasonBadRequest:            {},
+	metav1.StatusReasonMethodNotAllowed:      {},
+	metav1.StatusReasonNotAcceptable:         {},
+	metav1.StatusReasonRequestEntityTooLarge: {},
+	metav1.StatusReasonUnsupportedMediaType:  {},
+	metav1.StatusReasonInternalError:         {},
+	metav1.StatusReasonExpired:               {},
+	metav1.StatusReasonServiceUnavailable:    {},
+}
+
 // Error implements the Error interface.
 func (e *StatusError) Error() string {
 	return e.ErrStatus.Message
@@ -478,7 +500,14 @@ func NewGenericServerResponse(code int, verb string, qualifiedResource schema.Gr
 // IsNotFound returns true if the specified error was created by NewNotFound.
 // It supports wrapped errors.
 func IsNotFound(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonNotFound
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonNotFound {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusNotFound {
+		return true
+	}
+	return false
 }
 
 // IsAlreadyExists determines if the err is an error which indicates that a specified resource already exists.
@@ -490,19 +519,40 @@ func IsAlreadyExists(err error) bool {
 // IsConflict determines if the err is an error which indicates the provided update conflicts.
 // It supports wrapped errors.
 func IsConflict(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonConflict
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonConflict {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusConflict {
+		return true
+	}
+	return false
 }
 
 // IsInvalid determines if the err is an error which indicates the provided resource is not valid.
 // It supports wrapped errors.
 func IsInvalid(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonInvalid
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonInvalid {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusUnprocessableEntity {
+		return true
+	}
+	return false
 }
 
 // IsGone is true if the error indicates the requested resource is no longer available.
 // It supports wrapped errors.
 func IsGone(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonGone
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonGone {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusGone {
+		return true
+	}
+	return false
 }
 
 // IsResourceExpired is true if the error indicates the resource has expired and the current action is
@@ -515,77 +565,147 @@ func IsResourceExpired(err error) bool {
 // IsNotAcceptable determines if err is an error which indicates that the request failed due to an invalid Accept header
 // It supports wrapped errors.
 func IsNotAcceptable(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonNotAcceptable
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonNotAcceptable {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusNotAcceptable {
+		return true
+	}
+	return false
 }
 
 // IsUnsupportedMediaType determines if err is an error which indicates that the request failed due to an invalid Content-Type header
 // It supports wrapped errors.
 func IsUnsupportedMediaType(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonUnsupportedMediaType
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonUnsupportedMediaType {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusUnsupportedMediaType {
+		return true
+	}
+	return false
 }
 
 // IsMethodNotSupported determines if the err is an error which indicates the provided action could not
 // be performed because it is not supported by the server.
 // It supports wrapped errors.
 func IsMethodNotSupported(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonMethodNotAllowed
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonMethodNotAllowed {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusMethodNotAllowed {
+		return true
+	}
+	return false
 }
 
 // IsServiceUnavailable is true if the error indicates the underlying service is no longer available.
 // It supports wrapped errors.
 func IsServiceUnavailable(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonServiceUnavailable
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonServiceUnavailable {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusServiceUnavailable {
+		return true
+	}
+	return false
 }
 
 // IsBadRequest determines if err is an error which indicates that the request is invalid.
 // It supports wrapped errors.
 func IsBadRequest(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonBadRequest
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonBadRequest {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusBadRequest {
+		return true
+	}
+	return false
 }
 
 // IsUnauthorized determines if err is an error which indicates that the request is unauthorized and
 // requires authentication by the user.
 // It supports wrapped errors.
 func IsUnauthorized(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonUnauthorized
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonUnauthorized {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusUnauthorized {
+		return true
+	}
+	return false
 }
 
 // IsForbidden determines if err is an error which indicates that the request is forbidden and cannot
 // be completed as requested.
 // It supports wrapped errors.
 func IsForbidden(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonForbidden
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonForbidden {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusForbidden {
+		return true
+	}
+	return false
 }
 
 // IsTimeout determines if err is an error which indicates that request times out due to long
 // processing.
 // It supports wrapped errors.
 func IsTimeout(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonTimeout
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonTimeout {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusGatewayTimeout {
+		return true
+	}
+	return false
 }
 
 // IsServerTimeout determines if err is an error which indicates that the request needs to be retried
 // by the client.
 // It supports wrapped errors.
 func IsServerTimeout(err error) bool {
+	// do not check the status code, because no https status code exists that can
+	// be scoped to retryable timeouts.
 	return ReasonForError(err) == metav1.StatusReasonServerTimeout
 }
 
 // IsInternalError determines if err is an error which indicates an internal server error.
 // It supports wrapped errors.
 func IsInternalError(err error) bool {
-	return ReasonForError(err) == metav1.StatusReasonInternalError
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonInternalError {
+		return true
+	}
+	if _, ok := knownReasons[reason]; !ok && code == http.StatusInternalServerError {
+		return true
+	}
+	return false
 }
 
 // IsTooManyRequests determines if err is an error which indicates that there are too many requests
 // that the server cannot handle.
 // It supports wrapped errors.
 func IsTooManyRequests(err error) bool {
-	if ReasonForError(err) == metav1.StatusReasonTooManyRequests {
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonTooManyRequests {
 		return true
 	}
-	if status := APIStatus(nil); errors.As(err, &status) {
-		return status.Status().Code == http.StatusTooManyRequests
+
+	// IsTooManyRequests' checking of code predates the checking of the code in
+	// the other Is* functions. In order to maintain backward compatibility, this
+	// does not check that the reason is unknown.
+	if code == http.StatusTooManyRequests {
+		return true
 	}
 	return false
 }
@@ -594,11 +714,16 @@ func IsTooManyRequests(err error) bool {
 // the request entity is too large.
 // It supports wrapped errors.
 func IsRequestEntityTooLargeError(err error) bool {
-	if ReasonForError(err) == metav1.StatusReasonRequestEntityTooLarge {
+	reason, code := reasonAndCodeForError(err)
+	if reason == metav1.StatusReasonRequestEntityTooLarge {
 		return true
 	}
-	if status := APIStatus(nil); errors.As(err, &status) {
-		return status.Status().Code == http.StatusRequestEntityTooLarge
+
+	// IsRequestEntityTooLargeError's checking of code predates the checking of
+	// the code in the other Is* functions. In order to maintain backward
+	// compatibility, this does not check that the reason is unknown.
+	if code == http.StatusRequestEntityTooLarge {
+		return true
 	}
 	return false
 }
@@ -653,6 +778,13 @@ func ReasonForError(err error) metav1.StatusReason {
 	return metav1.StatusReasonUnknown
 }
 
+func reasonAndCodeForError(err error) (metav1.StatusReason, int32) {
+	if status := APIStatus(nil); errors.As(err, &status) {
+		return status.Status().Reason, status.Status().Code
+	}
+	return metav1.StatusReasonUnknown, 0
+}
+
 // ErrorReporter converts generic errors into runtime.Object errors without
 // requiring the caller to take a dependency on meta/v1 (where Status lives).
 // This prevents circular dependencies in core watch code.

staging/src/k8s.io/apimachinery/pkg/api/errors/errors_test.go

@@ -478,3 +478,95 @@ func TestSuggestsClientDelaySupportsWrapping(t *testing.T) {
 		})
 	}
 }
+
+func TestIsErrorTypesByReasonAndCode(t *testing.T) {
+	testCases := []struct {
+		name                  string
+		knownReason           metav1.StatusReason
+		otherReason           metav1.StatusReason
+		otherReasonConsidered bool
+		code                  int32
+		fn                    func(error) bool
+	}{
+		{
+			name:                  "IsRequestEntityTooLarge",
+			knownReason:           metav1.StatusReasonRequestEntityTooLarge,
+			otherReason:           metav1.StatusReasonForbidden,
+			otherReasonConsidered: false,
+			code:                  http.StatusRequestEntityTooLarge,
+			fn:                    IsRequestEntityTooLargeError,
+		}, {
+			name:                  "TooManyRequests",
+			knownReason:           metav1.StatusReasonTooManyRequests,
+			otherReason:           metav1.StatusReasonForbidden,
+			otherReasonConsidered: false,
+			code:                  http.StatusTooManyRequests,
+			fn:                    IsTooManyRequests,
+		}, {
+			name:                  "Forbidden",
+			knownReason:           metav1.StatusReasonForbidden,
+			otherReason:           metav1.StatusReasonNotFound,
+			otherReasonConsidered: true,
+			code:                  http.StatusForbidden,
+			fn:                    IsForbidden,
+		}, {
+			name:                  "NotFound",
+			knownReason:           metav1.StatusReasonNotFound,
+			otherReason:           metav1.StatusReasonForbidden,
+			otherReasonConsidered: true,
+			code:                  http.StatusNotFound,
+			fn:                    IsNotFound,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Run("by known reason", func(t *testing.T) {
+				err := &StatusError{
+					metav1.Status{
+						Reason: tc.knownReason,
+					},
+				}
+
+				got := tc.fn(err)
+				if !got {
+					t.Errorf("expected reason %s to match", tc.knownReason)
+				}
+			})
+
+			t.Run("by code and unknown reason", func(t *testing.T) {
+				err := &StatusError{
+					metav1.Status{
+						Reason: metav1.StatusReasonUnknown, // this could be _any_ reason that isn't in knownReasons.
+						Code:   tc.code,
+					},
+				}
+
+				got := tc.fn(err)
+				if !got {
+					t.Errorf("expected code %d with reason %s to match", tc.code, tc.otherReason)
+				}
+			})
+
+			if !tc.otherReasonConsidered {
+				return
+			}
+
+			t.Run("by code and other known reason", func(t *testing.T) {
+				err := &StatusError{
+					metav1.Status{
+						Reason: tc.otherReason,
+						Code:   tc.code,
+					},
+				}
+
+				got := tc.fn(err)
+				if got {
+					t.Errorf("expected code %d with reason %s to not match", tc.code, tc.otherReason)
+				}
+			})
+
+		})
+
+	}
+}