From b0ed01272e6162860e54e470262ee03fbf60880a Mon Sep 17 00:00:00 2001
From: Maximilian Moehl <maximilian.moehl@sap.com>
Date: Wed, 10 May 2023 13:35:47 +0200
Subject: [PATCH] fix: prevent side-effects of additional retry-able errors

This commit removes `IdempotentRequestEOF` and `IncompleteRequest` from
the fail-able and prune-able classifier group. This prevents errors that
should not affect the endpoint from being marked as failed or being
pruned. To do so all classifiers groups are split into distinct groups
and any cross references between them are removed.

The main motivation for this change is to avoid confusion and bugs due to
artificial dependencies between the groups.

Partially-Resolves: cloudfoundry/routing-release#321
Tested-By: `routing-release/scripts/run-unit-tests-in-docker gorouter`
---
 proxy/fails/classifier_group.go      | 20 ++++++++++++++++++--
 proxy/fails/classifier_group_test.go |  4 +---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/proxy/fails/classifier_group.go b/proxy/fails/classifier_group.go
index d0a1da5d..b790c014 100644
--- a/proxy/fails/classifier_group.go
+++ b/proxy/fails/classifier_group.go
@@ -24,11 +24,27 @@ var RetriableClassifiers = ClassifierGroup{
 }
 
 var FailableClassifiers = ClassifierGroup{
-	RetriableClassifiers,
+	Dial,
+	AttemptedTLSWithNonTLSBackend,
+	HostnameMismatch,
+	RemoteFailedCertCheck,
+	RemoteHandshakeFailure,
+	RemoteHandshakeTimeout,
+	UntrustedCert,
+	ExpiredOrNotYetValidCertFailure,
 	ConnectionResetOnRead,
 }
 
-var PrunableClassifiers = RetriableClassifiers
+var PrunableClassifiers = ClassifierGroup{
+	Dial,
+	AttemptedTLSWithNonTLSBackend,
+	HostnameMismatch,
+	RemoteFailedCertCheck,
+	RemoteHandshakeFailure,
+	RemoteHandshakeTimeout,
+	UntrustedCert,
+	ExpiredOrNotYetValidCertFailure,
+}
 
 // Classify returns true on errors that are retryable
 func (cg ClassifierGroup) Classify(err error) bool {
diff --git a/proxy/fails/classifier_group_test.go b/proxy/fails/classifier_group_test.go
index 8beebccb..649414b5 100644
--- a/proxy/fails/classifier_group_test.go
+++ b/proxy/fails/classifier_group_test.go
@@ -56,7 +56,7 @@ var _ = Describe("ClassifierGroup", func() {
 	})
 
 	Describe("prunable", func() {
-		It("matches hostname mismatch", func() {
+		It("matches prunable errors", func() {
 			pc := fails.PrunableClassifiers
 
 			Expect(pc.Classify(&net.OpError{Op: "dial"})).To(BeTrue())
@@ -69,8 +69,6 @@ var _ = Describe("ClassifierGroup", func() {
 			Expect(pc.Classify(x509.UnknownAuthorityError{})).To(BeTrue())
 			Expect(pc.Classify(x509.CertificateInvalidError{Reason: x509.Expired})).To(BeTrue())
 			Expect(pc.Classify(errors.New("i'm a potato"))).To(BeFalse())
-			Expect(pc.Classify(fails.IdempotentRequestEOFError)).To(BeTrue())
-			Expect(pc.Classify(fails.IncompleteRequestError)).To(BeTrue())
 		})
 	})
 })