From e603bf49faff1fa8df5faf3f20ce7864976b0183 Mon Sep 17 00:00:00 2001 From: John Eikenberry <jae@zhar.net> Date: Tue, 11 Oct 2022 16:18:21 -0700 Subject: [PATCH] fix header comparison causing spurious http checks Consul modifies (adds headers) the header map on the cached version which makes it always fail the comparison (DeepEqual) test. This fixes the comparison to ignore the fields Consul adds. --- check.go | 18 +++++++++++++++++- check_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/check.go b/check.go index b1db21a..39de677 100644 --- a/check.go +++ b/check.go @@ -136,7 +136,7 @@ func (c *CheckRunner) updateCheckHTTP( httpCheck, httpCheckExists := c.checksHTTP.Load(checkHash) if httpCheckExists && httpCheck.HTTP == http.HTTP && - reflect.DeepEqual(httpCheck.Header, http.Header) && + headersAlmostEqual(httpCheck.Header, http.Header) && httpCheck.Method == http.Method && httpCheck.TLSClientConfig.InsecureSkipVerify == http.TLSClientConfig.InsecureSkipVerify && httpCheck.TLSClientConfig.ServerName == http.TLSClientConfig.ServerName && @@ -172,6 +172,22 @@ func (c *CheckRunner) updateCheckHTTP( return true } +// Compares headers, skipping ones automatically added by Consul +// in consul/agent/checks/check.go +func headersAlmostEqual(h1, h2 map[string][]string) bool { + skip := map[string]bool{"User-Agent": true, "Accept": true} + for k1, v1 := range h1 { + if skip[k1] { + continue + } + v2 := h2[k1] + if !reflect.DeepEqual(v1, v2) { + return false + } + } + return true +} + func (c *CheckRunner) updateCheckTCP( latestCheck *api.HealthCheck, checkHash types.CheckID, definition *api.HealthCheckDefinition, updated, added checkIDSet, diff --git a/check_test.go b/check_test.go index 1ba1cfe..687138c 100644 --- a/check_test.go +++ b/check_test.go @@ -423,3 +423,50 @@ func TestCheck_NoFlapping(t *testing.T) { assert.Equal(t, 1, currentCheck.successCounter) assert.Equal(t, api.HealthCritical, currentCheck.Status) } + +func TestHeadersAlmostEqual(t *testing.T) { + type headers map[string][]string + type testCase struct { + h1, h2 headers + equal bool + } + testCases := []testCase{ + { + h1: headers{}, + h2: headers{}, + equal: true, + }, + { + h1: headers{"foo": {"foo"}}, + h2: headers{"bar": {"bar"}}, + equal: false, + }, + { + h1: headers{"User-Agent": {"foo"}}, + h2: headers{"Accept": {"bar"}}, + equal: true, + }, + { + h1: headers{"foo": {"foo"}, "User-Agent": {"foo"}}, + h2: headers{"Accept": {"bar"}}, + equal: false, + }, + { + h1: headers{"foo": {"foo"}, "User-Agent": {"foo"}}, + h2: headers{"foo": {"foo"}, "Accept": {"bar"}}, + equal: true, + }, + } + for _, tc := range testCases { + switch eq := headersAlmostEqual(tc.h1, tc.h2); tc.equal { + case true: + if !eq { + t.Error("headers should be equal", tc.h1, tc.h2) + } + case false: + if eq { + t.Error("headers should NOT be equal", tc.h1, tc.h2) + } + } + } +}