From c305d70fae8814b65052f8030bbf5f47647dfd12 Mon Sep 17 00:00:00 2001 From: maier Date: Tue, 26 Apr 2022 07:40:52 -0400 Subject: [PATCH 1/5] add: reset flag --- trapcheck.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trapcheck.go b/trapcheck.go index 575d807..9b7d3f9 100644 --- a/trapcheck.go +++ b/trapcheck.go @@ -60,6 +60,7 @@ type TrapCheck struct { brokerMaxResponseTime time.Duration newCheckBundle bool usingPublicCA bool + resetTLSConfig bool } // New creates a new TrapCheck instance @@ -244,7 +245,7 @@ func NewFromCheckBundle(cfg *Config, bundle *apiclient.CheckBundle) (*TrapCheck, // SendMetrics submits the metrics to the broker // metrics must be valid JSON encoded data for the broker httptrap check // returns trap results in a structure or an error. -func (tc *TrapCheck) SendMetrics(ctx context.Context, metrics bytes.Buffer) (*TrapResult, error) { +func (tc *TrapCheck) SendMetrics(ctx context.Context, metrics bytes.Buffer) (*TrapResult, error) { //nolint:contextcheck if ctx == nil { ctx = context.Background() } From 8edc206d0986ca2d779b2958d02d69891772d26f Mon Sep 17 00:00:00 2001 From: maier Date: Tue, 26 Apr 2022 07:41:53 -0400 Subject: [PATCH 2/5] upd: skip conn test if check type is httptrap and using proxy env vars --- broker.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/broker.go b/broker.go index d701cca..69c4279 100644 --- a/broker.go +++ b/broker.go @@ -11,6 +11,7 @@ import ( "math/big" "net" "net/url" + "os" "reflect" "strconv" "strings" @@ -141,6 +142,9 @@ func (tc *TrapCheck) isValidBroker(broker *apiclient.Broker, checkType string) ( return false, fmt.Errorf("broker '%s' invalid, no instance details", broker.Name) } + httpProxy := os.Getenv("HTTP_PROXY") + httpsProxy := os.Getenv("HTTPS_PROXY") + for _, detail := range broker.Details { detail := detail @@ -184,6 +188,14 @@ func (tc *TrapCheck) isValidBroker(broker *apiclient.Broker, checkType string) ( brokerPort = "443" } + // do not direct connect to test broker, if a proxy env var is set and check is httptrap + if strings.Contains(strings.ToLower(checkType), "httptrap") { + if httpProxy != "" || httpsProxy != "" { + tc.Log.Debugf("skipping connection test, proxy environment var(s) set -- HTTP:'%s' HTTPS:'%s'", httpProxy, httpsProxy) + return true, nil + } + } + retries := 5 target := fmt.Sprintf("%s:%s", brokerHost, brokerPort) for attempt := 1; attempt <= retries; attempt++ { From 31f02e996db0e59837f8a3cf3eac076959bc5463 Mon Sep 17 00:00:00 2001 From: maier Date: Tue, 26 Apr 2022 07:42:42 -0400 Subject: [PATCH 3/5] upd: move cert error logging to tls config --- submit.go | 38 +++++++++++++++++++++++--------------- tls.go | 17 +++++++++++++---- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/submit.go b/submit.go index a37d5db..9162d61 100644 --- a/submit.go +++ b/submit.go @@ -9,9 +9,7 @@ import ( "bytes" "compress/gzip" "context" - "crypto/x509" "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -201,20 +199,30 @@ func (tc *TrapCheck) submit(ctx context.Context, metrics bytes.Buffer) (*TrapRes } } - retryClient.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) { - retry, err := retryablehttp.ErrorPropagatedRetryPolicy(ctx, resp, err) - if retry && err != nil { - var cie *x509.CertificateInvalidError - if errors.As(err, &cie) { - if cie.Reason == x509.NameMismatch { - tc.Log.Warnf("certificate name mismatch (refreshing TLS config) common cause, new broker added to cluster or check moved to new broker: %s", cie.Detail) - tc.clearTLSConfig() - return false, fmt.Errorf("x509 cert name mismatch: %w", err) - } - } else { - tc.Log.Warnf("request error (%s): %s", resp.Request.URL, err) - } + retryClient.CheckRetry = func(ctx context.Context, resp *http.Response, origErr error) (bool, error) { + + // if origErr != nil { + // tc.Log.Debugf("request origErr: %s", origErr.Error()) + // } + // // this gets kind of muddy - retryablehttp will eat specific x509 errors we want to log + // // see: https://github.com/hashicorp/go-retryablehttp/blob/master/client.go#L443-L494 + // // so we need to evaluate the original error not the one returned from ErrorPropagatedRetryPolicy + // var cie *x509.CertificateInvalidError + // if errors.As(origErr, &cie) { + // if cie.Reason == x509.NameMismatch { + // tc.Log.Warnf("certificate name mismatch (refreshing TLS config) common cause, new broker added to cluster or check moved to new broker: %s", cie.Detail) + // if tc.tlsConfig != nil { + // tc.clearTLSConfig() + // } + // return false, fmt.Errorf("x509 cert name mismatch: %w", origErr) + // } + // } + + retry, rhErr := retryablehttp.ErrorPropagatedRetryPolicy(ctx, resp, origErr) + if retry && rhErr != nil { + tc.Log.Warnf("request error (%s): %s (orig:%s)", resp.Request.URL, rhErr, origErr) } + return retry, nil } diff --git a/tls.go b/tls.go index 548337e..f3a2991 100644 --- a/tls.go +++ b/tls.go @@ -14,15 +14,22 @@ import ( "strings" ) +// clearTLSConfig sets the resetTLSConfig flag so that on the next setBrokerTLSConfig call +// the broker will be refreshed and a new tls configuration will be created. The most common +// reason for this to be done is a change to the configuration of a broker cluster (e.g. add/del). func (tc *TrapCheck) clearTLSConfig() { - tc.broker = nil // force refresh - tc.tlsConfig = nil // don't use, refresh and reset - tc.custTLSConfig = nil // don't use, refresh and reset + tc.resetTLSConfig = true } // setBrokerTLSConfig sets the broker tls configuration if was // not supplied by the caller in the configuration. func (tc *TrapCheck) setBrokerTLSConfig() error { + if tc.resetTLSConfig { + tc.broker = nil // force refresh + tc.tlsConfig = nil // don't use, refresh and reset + tc.resetTLSConfig = false + // tc.custTLSConfig = nil // don't use, refresh and reset + } // setBrokerTLSConfig has already initialized it if tc.tlsConfig != nil { @@ -40,6 +47,7 @@ func (tc *TrapCheck) setBrokerTLSConfig() error { // caller supplied tls config if tc.custTLSConfig != nil { + tc.Log.Debugf("using custom tls configuration") tc.tlsConfig = tc.custTLSConfig.Clone() return nil } @@ -87,8 +95,9 @@ func (tc *TrapCheck) setBrokerTLSConfig() error { InsecureSkipVerify: true, //nolint:gosec VerifyConnection: func(cs tls.ConnectionState) error { commonName := cs.PeerCertificates[0].Subject.CommonName - // if commonName != cs.ServerName { if !strings.Contains(cnList, commonName) { + tc.Log.Warnf("certificate name mismatch (refreshing TLS config) common cause, new broker added to cluster or check moved to new broker -- cn: %q, acceptable: %q", commonName, cnList) + tc.clearTLSConfig() return x509.CertificateInvalidError{ Cert: cs.PeerCertificates[0], Reason: x509.NameMismatch, From b9ca7813ae6ec5428a0524f083a5eeacd32d423c Mon Sep 17 00:00:00 2001 From: maier Date: Tue, 26 Apr 2022 07:42:49 -0400 Subject: [PATCH 4/5] upd: disable gci --- .golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 92e7bd6..3d6cdf2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -54,7 +54,7 @@ linters: - unparam - unused - varcheck - - gci + # - gci - godot - godox # - goerr113 From f2b765a7517e1e61a25956788c610a0bff369a99 Mon Sep 17 00:00:00 2001 From: maier Date: Tue, 26 Apr 2022 07:46:58 -0400 Subject: [PATCH 5/5] v0.0.8 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d4e402..e683217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# v0.0.8 + +* add: reset flag and handle rest in setBrokerTLSConfig +* upd: skip conn test if check type is httptrap and using proxy env vars HTTP_PROXY and/or HTTPS_PROXY +* upd: move cert error logging to tls config +* upd: disable gci + # v0.0.7 * add: tracking if bundle is new (created) or not