From b820e884e3d11755e306ca2e14e83c207855091a 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 d0a1da5d2..b790c0149 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 8beebccba..649414b57 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())
 		})
 	})
 })