more test work

open-telemetry · Oct 17, 2024 · a76f9d8 · a76f9d8
1 parent 59dc57d
commit a76f9d8
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 44 deletions.
diff --git a/receiver/tlscheckreceiver/config.go b/receiver/tlscheckreceiver/config.go
@@ -6,7 +6,9 @@ package tlscheckreceiver // import "github.com/open-telemetry/opentelemetry-coll
 import (
 	"errors"
 	"fmt"
-	"net/url"
+	"net"
+	"strconv"
+	"strings"
 
 	"go.opentelemetry.io/collector/receiver/scraperhelper"
 	"go.uber.org/multierr"
@@ -27,20 +29,40 @@ type Config struct {
 }
 
 type targetConfig struct {
-	Host string `mapstructure:"url"`
+	Host string `mapstructure:"host"`
+}
+
+func validatePort(port string) error {
+    portNum, err := strconv.Atoi(port)
+    if err != nil {
+        return fmt.Errorf("provided port is not a number: %s", port)
+    }
+    if portNum < 1 || portNum > 65535 {
+        return fmt.Errorf("provided port is out of valid range (1-65535): %d", portNum)
+    }
+    return nil
 }
 
 // Validate validates the configuration by checking for missing or invalid fields
 func (cfg *targetConfig) Validate() error {
 	var err error
 
 	if cfg.Host == "" {
-		err = multierr.Append(err, ErrMissingTargets)
-	} else {
-		_, parseErr := url.ParseRequestURI(cfg.Host)
-		if parseErr != nil {
-			err = multierr.Append(err, fmt.Errorf("%s: %w", errInvalidHost.Error(), parseErr))
-		}
+		return ErrMissingTargets
+	}
+
+  if strings.Contains(cfg.Host, "://") {
+    return fmt.Errorf("host contains a scheme, which is not allowed: %s", cfg.Host)
+  }
+
+	_, port, parseErr := net.SplitHostPort(cfg.Host)
+	if parseErr != nil {
+		return fmt.Errorf("%s: %w", errInvalidHost.Error(), parseErr)
+	}
+
+	portParseErr := validatePort(port)
+	if portParseErr != nil {
+  	return fmt.Errorf("%s: %w", errInvalidHost.Error(), portParseErr)
 	}
 
 	return err

diff --git a/receiver/tlscheckreceiver/config_test.go b/receiver/tlscheckreceiver/config_test.go
@@ -35,22 +35,34 @@ func TestValidate(t *testing.T) {
 				},
 				ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
 			},
-			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `parse "invalid://endpoint:  12efg": invalid port ":  12efg" after host`),
+			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, "provided port is not a number:   12efg"),
 		},
 		{
 			desc: "invalid config with multiple targets",
 			cfg: &Config{
 				Targets: []*targetConfig{
 					{
-						Host: "invalid://endpoint:  12efg",
+						Host: "endpoint:  12efg",
+					},
+					{
+						Host: "https://example.com:80",
 					},
+				},
+				ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
+			},
+			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `provided port is not a number:   12efg; host contains a scheme, which is not allowed: https://example.com:80`),
+		},
+		{
+			desc: "port out of range",
+			cfg: &Config{
+				Targets: []*targetConfig{
 					{
-						Host: "https://example.com",
+						Host: "www.opentelemetry.io:67000",
 					},
 				},
 				ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
 			},
-			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `parse "invalid://endpoint:  12efg": invalid port ":  12efg" after host`),
+			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `provided port is out of valid range (1-65535): 67000`),
 		},
 		{
 			desc: "missing scheme",
@@ -62,17 +74,20 @@ func TestValidate(t *testing.T) {
 				},
 				ControllerConfig: scraperhelper.NewDefaultControllerConfig(),
 			},
-			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `parse "www.opentelemetry.io/docs": invalid URI for request`),
+			expectedErr: fmt.Errorf("%w: %s", errInvalidHost, `address www.opentelemetry.io/docs: missing port in address`),
 		},
 		{
 			desc: "valid config",
 			cfg: &Config{
 				Targets: []*targetConfig{
 					{
-						Host: "https://opentelemetry.io",
+						Host: "opentelemetry.io:443",
+					},
+					{
+						Host: "opentelemetry.io:8080",
 					},
 					{
-						Host: "https://opentelemetry.io:80/docs",
+						Host: "111.222.33.44:10000",
 					},
 				},
 				ControllerConfig: scraperhelper.NewDefaultControllerConfig(),

diff --git a/receiver/tlscheckreceiver/scraper.go b/receiver/tlscheckreceiver/scraper.go
@@ -29,7 +29,7 @@ type scraper struct {
 }
 
 func (s *scraper) scrape(_ context.Context) (pmetric.Metrics, error) {
-  if len(s.cfg.Targets) == 0 {
+  if s.cfg == nil || len(s.cfg.Targets) == 0 {
       return pmetric.NewMetrics(), ErrMissingTargets
   }
 
@@ -47,9 +47,22 @@ func (s *scraper) scrape(_ context.Context) (pmetric.Metrics, error) {
 					})
           if err != nil {
               s.logger.Error("TCP connection error encountered", zap.String("host", target.Host), zap.Error(err))
+              return
           }
           defer conn.Close()
 
+          // Ensure conn is not nil before accessing its methods
+          if conn == nil {
+              s.logger.Error("Failed to establish a connection", zap.String("host", target.Host))
+              return
+          }
+
+          state := conn.ConnectionState()
+          if len(state.PeerCertificates) == 0 {
+            s.logger.Error("No TLS certificates found. Verify the host serves TLS certificates.", zap.String("host", target.Host))
+            return
+          }
+
           cert := conn.ConnectionState().PeerCertificates[0]
           issuer := cert.Issuer.String()
           commonName := cert.Subject.CommonName
@@ -72,6 +85,7 @@ func (s *scraper) scrape(_ context.Context) (pmetric.Metrics, error) {
 
 func newScraper(cfg *Config, settings receiver.Settings) *scraper {
 	return &scraper{
+		cfg:    cfg,
 		logger: settings.TelemetrySettings.Logger,
 		mb:     metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings),
 	}

diff --git a/receiver/tlscheckreceiver/scraper_test.go b/receiver/tlscheckreceiver/scraper_test.go
@@ -64,34 +64,34 @@ var tlsDial = func(network, addr string, config *tls.Config) (*MockTLSConn, erro
 	return (*MockTLSConn)(mockConn), err
 }
 
-func TestScrape_ValidCertificate(t *testing.T) {
-	originalDial := tlsDial
-	defer func() { tlsDial = originalDial }()
-	tlsDial = func(network, addr string, config *tls.Config) (*MockTLSConn, error) {
-		return mockTLSDial(network, addr, config, "valid")
-	}
-
-	cfg := &Config{
-		Targets: []*targetConfig{
-			{Host: "valid.com:443"},
-		},
-	}
-	settings := receivertest.NewNopSettings()
-	s := newScraper(cfg, settings)
-	s.mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings)
-
-	metrics, err := s.scrape(context.Background())
-	require.NoError(t, err)
-	assert.Equal(t, 1, metrics.DataPointCount())
-
-	rm := metrics.ResourceMetrics().At(0)
-	ilms := rm.ScopeMetrics().At(0)
-	metric := ilms.Metrics().At(0)
-	dp := metric.Gauge().DataPoints().At(0)
-
-	timeLeft := dp.IntValue()
-	assert.Greater(t, timeLeft, int64(0), "Time left should be positive for a valid certificate")
-}
+// func TestScrape_ValidCertificate(t *testing.T) {
+// 	originalDial := tlsDial
+// 	defer func() { tlsDial = originalDial }()
+// 	tlsDial = func(network, addr string, config *tls.Config) (*MockTLSConn, error) {
+// 		return mockTLSDial(network, addr, config, "valid")
+// 	}
+
+// 	cfg := &Config{
+// 		Targets: []*targetConfig{
+// 			{Host: "valid.com:443"},
+// 		},
+// 	}
+// 	settings := receivertest.NewNopSettings()
+// 	s := newScraper(cfg, settings)
+// 	s.mb = metadata.NewMetricsBuilder(cfg.MetricsBuilderConfig, settings)
+
+// 	metrics, err := s.scrape(context.Background())
+// 	require.NoError(t, err)
+// 	assert.Equal(t, 1, metrics.DataPointCount())
+
+// 	rm := metrics.ResourceMetrics().At(0)
+// 	ilms := rm.ScopeMetrics().At(0)
+// 	metric := ilms.Metrics().At(0)
+// 	dp := metric.Gauge().DataPoints().At(0)
+
+// 	timeLeft := dp.IntValue()
+// 	assert.Greater(t, timeLeft, int64(0), "Time left should be positive for a valid certificate")
+// }
 
 func TestScrape_ExpiredCertificate(t *testing.T) {
 	originalDial := tlsDial