From 54f98fdc88d8040c22f2f5e0e3b75828ac350f51 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 24 Nov 2022 10:20:37 +1030
Subject: [PATCH] x-pack/filebeat/input/http_endpoint: canonicalise tls
 consistency error (#33720) (#33780)

Configs may be run in arbitrary order during tests, so make sure that
the error is invariant with respect to the address and reason. This has
no impact on the logic of the input in production.

(cherry picked from commit 4f9aadce73cf63b80fbc432ec619daa5f3efaa04)

Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
---
 x-pack/filebeat/input/http_endpoint/input.go  | 22 ++++++++++++++++---
 .../input/http_endpoint/input_test.go         | 21 +++++++++++++-----
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/x-pack/filebeat/input/http_endpoint/input.go b/x-pack/filebeat/input/http_endpoint/input.go
index 294bf5b092e9..fff5e6376bbb 100644
--- a/x-pack/filebeat/input/http_endpoint/input.go
+++ b/x-pack/filebeat/input/http_endpoint/input.go
@@ -164,16 +164,32 @@ func (p *pool) serve(ctx v2.Context, e *httpEndpoint, pub stateless.Publisher) e
 }
 
 func checkTLSConsistency(addr string, old, new *tlscommon.ServerConfig) error {
+	if old == nil && new == nil {
+		return nil
+	}
 	if (old == nil) != (new == nil) {
-		return fmt.Errorf("inconsistent TLS usage on %s: mixed TLS and unencrypted", addr)
+		return invalidTLSStateErr{addr: addr, reason: "mixed TLS and unencrypted", old: old, new: new}
 	}
 	if !reflect.DeepEqual(old, new) {
-		return fmt.Errorf("inconsistent TLS configuration on %s: configuration options do not agree: old=%s new=%s",
-			addr, renderTLSConfig(old), renderTLSConfig(new))
+		return invalidTLSStateErr{addr: addr, reason: "configuration options do not agree", old: old, new: new}
 	}
 	return nil
 }
 
+type invalidTLSStateErr struct {
+	addr     string
+	reason   string
+	old, new *tlscommon.ServerConfig
+}
+
+func (e invalidTLSStateErr) Error() string {
+	if e.old == nil || e.new == nil {
+		return fmt.Sprintf("inconsistent TLS usage on %s: %s", e.addr, e.reason)
+	}
+	return fmt.Sprintf("inconsistent TLS configuration on %s: %s: old=%s new=%s",
+		e.addr, e.reason, renderTLSConfig(e.old), renderTLSConfig(e.new))
+}
+
 func renderTLSConfig(tls *tlscommon.ServerConfig) string {
 	c, err := conf.NewConfigFrom(tls)
 	if err != nil {
diff --git a/x-pack/filebeat/input/http_endpoint/input_test.go b/x-pack/filebeat/input/http_endpoint/input_test.go
index 87ac4e2f48ea..3b38afdc0791 100644
--- a/x-pack/filebeat/input/http_endpoint/input_test.go
+++ b/x-pack/filebeat/input/http_endpoint/input_test.go
@@ -162,7 +162,7 @@ var serverPoolTests = []struct {
 				},
 			},
 		},
-		wantErr: errors.New("inconsistent TLS usage on 127.0.0.1:9001: mixed TLS and unencrypted"),
+		wantErr: invalidTLSStateErr{addr: "127.0.0.1:9001", reason: "mixed TLS and unencrypted"},
 	},
 	{
 		name: "inconsistent_tls_config",
@@ -198,10 +198,7 @@ var serverPoolTests = []struct {
 				},
 			},
 		},
-		wantErr: errors.New(`inconsistent TLS configuration on 127.0.0.1:9001: ` +
-			`configuration options do not agree: ` +
-			`old={"ca_sha256":[],"certificate":"","certificate_authorities":[],"cipher_suites":[],"client_authentication":0,"curve_types":[],"key":"","key_passphrase":"","supported_protocols":[],"verification_mode":1} ` +
-			`new={"ca_sha256":[],"certificate":"","certificate_authorities":[],"cipher_suites":[],"client_authentication":0,"curve_types":[],"key":"","key_passphrase":"","supported_protocols":[],"verification_mode":3}`),
+		wantErr: invalidTLSStateErr{addr: "127.0.0.1:9001", reason: "configuration options do not agree"},
 	},
 }
 
@@ -241,7 +238,7 @@ func TestServerPool(t *testing.T) {
 			case err := <-fails:
 				if test.wantErr == nil {
 					t.Errorf("unexpected error calling serve: %#q", err)
-				} else if test.wantErr.Error() != err.Error() {
+				} else if !errors.Is(err, test.wantErr) {
 					t.Errorf("unexpected error calling serve: got=%#q, want=%#q", err, test.wantErr)
 				}
 			default:
@@ -272,6 +269,18 @@ func TestServerPool(t *testing.T) {
 	}
 }
 
+// Is is included to simplify testing, but is not exposed to avoid unwanted error
+// matching outside tests.
+func (e invalidTLSStateErr) Is(err error) bool {
+	if err, ok := err.(invalidTLSStateErr); ok { //nolint:errorlint // "An Is method should only shallowly compare err and the target and not call Unwrap on either."
+		// However for our purposes here, we will abuse
+		// the Is convention and also consider the addr
+		// and reason fields.
+		return e.addr == err.addr && e.reason == err.reason
+	}
+	return false
+}
+
 func newCtx(log, id string) (_ v2.Context, cancel func()) {
 	ctx, cancel := context.WithCancel(context.Background())
 	return v2.Context{