Merge pull request #7 from maier/master

v0.0.8

.golangci.yml

@@ -54,7 +54,7 @@ linters:
     - unparam
     - unused
     - varcheck
-    - gci
+    # - gci
     - godot
     - godox
     # - goerr113

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

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++ {

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
 	}
 

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,

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()
 	}