From 0dc9f1bf626ec92ec562c61f4dc142488072f882 Mon Sep 17 00:00:00 2001
From: Adrian Serrano <adrisr83@gmail.com>
Date: Fri, 14 Apr 2023 15:35:23 +0200
Subject: [PATCH 1/2] http: Prevent logging confusing error message

When FailIfNotSSL is set, the error message:

> Final request was not over SSL

is logged even if a previous error has prevented the request to
complete, for example when it fails due to a timeout or connection
error.

This updates the logic around the log message so it's printed when an
otherwise successful request is not performed over SSL and that causes
the probe to fail.

Signed-off-by: Adrian Serrano <adrisr83@gmail.com>
---
 prober/http.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/prober/http.go b/prober/http.go
index 441edc47..232214c3 100644
--- a/prober/http.go
+++ b/prober/http.go
@@ -647,7 +647,7 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr
 			level.Error(logger).Log("msg", "Final request was over SSL")
 			success = false
 		}
-	} else if httpConfig.FailIfNotSSL {
+	} else if httpConfig.FailIfNotSSL && success {
 		level.Error(logger).Log("msg", "Final request was not over SSL")
 		success = false
 	}

From 6637eca6f613ca5f7abbf7adcd1e9ca4c946d212 Mon Sep 17 00:00:00 2001
From: Adrian Serrano <adrisr83@gmail.com>
Date: Fri, 2 Jun 2023 12:04:11 +0200
Subject: [PATCH 2/2] Add a test to validate log message

Signed-off-by: Adrian Serrano <adrisr83@gmail.com>
---
 prober/http_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/prober/http_test.go b/prober/http_test.go
index 3d7dfdb2..05ca36d8 100644
--- a/prober/http_test.go
+++ b/prober/http_test.go
@@ -23,6 +23,7 @@ import (
 	"encoding/pem"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/textproto"
@@ -836,6 +837,95 @@ func TestFailIfNotSSL(t *testing.T) {
 	checkRegistryResults(expectedResults, mfs, t)
 }
 
+type logRecorder struct {
+	msgs map[string]bool
+}
+
+func (r *logRecorder) Log(keyvals ...interface{}) error {
+	if r.msgs == nil {
+		r.msgs = make(map[string]bool)
+	}
+	for i := 0; i < len(keyvals)-1; i += 2 {
+		if keyvals[i] == "msg" {
+			msg, ok := keyvals[i+1].(string)
+			if ok {
+				r.msgs[msg] = true
+			}
+		}
+	}
+	return nil
+}
+
+func TestFailIfNotSSLLogMsg(t *testing.T) {
+	const (
+		Msg     = "Final request was not over SSL"
+		Timeout = time.Second * 10
+	)
+
+	goodServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer goodServer.Close()
+
+	// Create a TCP server that closes the connection without an answer, to simulate failure.
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer listener.Close()
+	go func() {
+		for {
+			conn, err := listener.Accept()
+			if err != nil {
+				return
+			}
+			conn.Close()
+		}
+	}()
+	badServerURL := fmt.Sprintf("http://%s/", listener.Addr().String())
+
+	for title, tc := range map[string]struct {
+		Config          config.Module
+		URL             string
+		Success         bool
+		MessageExpected bool
+	}{
+		"SSL expected, message": {
+			Config:          config.Module{HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfNotSSL: true}},
+			URL:             goodServer.URL,
+			Success:         false,
+			MessageExpected: true,
+		},
+		"No SSL expected, no message": {
+			Config:          config.Module{HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfNotSSL: false}},
+			URL:             goodServer.URL,
+			Success:         true,
+			MessageExpected: false,
+		},
+		"SSL expected, no message": {
+			Config:          config.Module{HTTP: config.HTTPProbe{IPProtocolFallback: true, FailIfNotSSL: true}},
+			URL:             badServerURL,
+			Success:         false,
+			MessageExpected: false,
+		},
+	} {
+		t.Run(title, func(t *testing.T) {
+			recorder := logRecorder{}
+			registry := prometheus.NewRegistry()
+			testCTX, cancel := context.WithTimeout(context.Background(), Timeout)
+			defer cancel()
+
+			result := ProbeHTTP(testCTX, tc.URL, tc.Config, registry, &recorder)
+			if result != tc.Success {
+				t.Fatalf("Expected success=%v, got=%v", tc.Success, result)
+			}
+			if seen := recorder.msgs[Msg]; seen != tc.MessageExpected {
+				t.Fatalf("SSL message expected=%v, seen=%v", tc.MessageExpected, seen)
+			}
+		})
+	}
+}
+
 func TestFailIfBodyMatchesRegexp(t *testing.T) {
 	testcases := map[string]struct {
 		respBody       string