telemetry: improve test cases

remove an unnecessary nil check in tlsutils added as workaround
to make telemetry test cases work

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

go.mod

@@ -157,7 +157,7 @@ require (
 	golang.org/x/tools v0.6.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
-	google.golang.org/genproto v0.0.0-20230222225845-10f96fb3dbec // indirect
+	google.golang.org/genproto v0.0.0-20230223222841-637eb2293923 // indirect
 	google.golang.org/grpc v1.53.0 // indirect
 	google.golang.org/protobuf v1.28.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

go.sum

@@ -2715,8 +2715,8 @@ google.golang.org/genproto v0.0.0-20221109142239-94d6d90a7d66/go.mod h1:rZS5c/ZV
 google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
 google.golang.org/genproto v0.0.0-20221201164419-0e50fba7f41c/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
 google.golang.org/genproto v0.0.0-20221201204527-e3fa12d562f3/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg=
-google.golang.org/genproto v0.0.0-20230222225845-10f96fb3dbec h1:6rwgChOSUfpzJF2/KnLgo+gMaxGpujStSkPWrbhXArU=
-google.golang.org/genproto v0.0.0-20230222225845-10f96fb3dbec/go.mod h1:3Dl5ZL0q0isWJt+FVcfpQyirqemEuLAK/iFvg1UP1Hw=
+google.golang.org/genproto v0.0.0-20230223222841-637eb2293923 h1:znp6mq/drrY+6khTAlJUDNFFcDGV2ENLYKpMq8SyCds=
+google.golang.org/genproto v0.0.0-20230223222841-637eb2293923/go.mod h1:3Dl5ZL0q0isWJt+FVcfpQyirqemEuLAK/iFvg1UP1Hw=
 google.golang.org/grpc v0.0.0-20160317175043-d3ddb4469d5a/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
 google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=

internal/common/tlsutils.go

@@ -86,11 +86,12 @@ func (m *CertManager) loadCertificates() error {
 		}
 		newCert, err := tls.LoadX509KeyPair(keyPair.Cert, keyPair.Key)
 		if err != nil {
-			logger.Warn(m.logSender, "", "unable to load X509 key pair, cert file %q key file %q error: %v",
+			logger.Error(m.logSender, "", "unable to load X509 key pair, cert file %q key file %q error: %v",
 				keyPair.Cert, keyPair.Key, err)
 			return err
 		}
 		if _, ok := certs[keyPair.ID]; ok {
+			logger.Error(m.logSender, "", "TLS certificate with id %q is duplicated", keyPair.ID)
 			return fmt.Errorf("TLS certificate with id %q is duplicated", keyPair.ID)
 		}
 		logger.Debug(m.logSender, "", "TLS certificate %q successfully loaded, id %v", keyPair.Cert, keyPair.ID)
@@ -115,7 +116,8 @@ func (m *CertManager) GetCertificateFunc(certID string) func(*tls.ClientHelloInf
 
 		val, ok := m.certs[certID]
 		if !ok {
-			return nil, fmt.Errorf("no certificate for id %v", certID)
+			logger.Error(m.logSender, "", "no certificate for id %s", certID)
+			return nil, fmt.Errorf("no certificate for id %s", certID)
 		}
 
 		return val, nil
@@ -128,7 +130,7 @@ func (m *CertManager) IsRevoked(crt *x509.Certificate, caCrt *x509.Certificate)
 	defer m.RUnlock()
 
 	if crt == nil || caCrt == nil {
-		logger.Warn(m.logSender, "", "unable to verify crt %v, ca crt %v", crt, caCrt)
+		logger.Error(m.logSender, "", "unable to verify crt %v, ca crt %v", crt, caCrt)
 		return len(m.crls) > 0
 	}
 
@@ -162,7 +164,7 @@ func (m *CertManager) LoadCRLs() error {
 		}
 		crlBytes, err := os.ReadFile(revocationList)
 		if err != nil {
-			logger.Warn(m.logSender, "", "unable to read revocation list %q", revocationList)
+			logger.Error(m.logSender, "", "unable to read revocation list %q", revocationList)
 			return err
 		}
 		if bytes.HasPrefix(crlBytes, pemCRLPrefix) {
@@ -173,7 +175,7 @@ func (m *CertManager) LoadCRLs() error {
 		}
 		crl, err := x509.ParseRevocationList(crlBytes)
 		if err != nil {
-			logger.Warn(m.logSender, "", "unable to parse revocation list %q", revocationList)
+			logger.Error(m.logSender, "", "unable to parse revocation list %q", revocationList)
 			return err
 		}
 
@@ -218,13 +220,14 @@ func (m *CertManager) LoadRootCAs() error {
 		}
 		crt, err := os.ReadFile(rootCA)
 		if err != nil {
+			logger.Error(m.logSender, "", "unable to read root CA from file %q: %v", rootCA, err)
 			return err
 		}
 		if rootCAs.AppendCertsFromPEM(crt) {
 			logger.Debug(m.logSender, "", "TLS certificate authority %q successfully loaded", rootCA)
 		} else {
 			err := fmt.Errorf("unable to load TLS certificate authority %q", rootCA)
-			logger.Warn(m.logSender, "", "%v", err)
+			logger.Error(m.logSender, "", "%v", err)
 			return err
 		}
 	}
@@ -285,10 +288,10 @@ func (m *CertManager) monitor() {
 func NewCertManager(keyPairs []TLSKeyPair, configDir, logSender string) (*CertManager, error) {
 	manager := &CertManager{
 		keyPairs:  keyPairs,
-		certs:     make(map[string]*tls.Certificate),
-		certsInfo: make(map[string]fs.FileInfo),
 		configDir: configDir,
 		logSender: logSender,
+		certs:     make(map[string]*tls.Certificate),
+		certsInfo: make(map[string]fs.FileInfo),
 	}
 	err := manager.loadCertificates()
 	if err != nil {
@@ -296,9 +299,6 @@ func NewCertManager(keyPairs []TLSKeyPair, configDir, logSender string) (*CertMa
 	}
 	randSecs := rand.Intn(59)
 	manager.monitor()
-	if eventScheduler != nil {
-		logger.Debug(manager.logSender, "", "starting certificates monitoring tasks")
-		_, err = eventScheduler.AddFunc(fmt.Sprintf("@every 8h0m%ds", randSecs), manager.monitor)
-	}
+	_, err = eventScheduler.AddFunc(fmt.Sprintf("@every 8h0m%ds", randSecs), manager.monitor)
 	return manager, err
 }

internal/telemetry/telemetry_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/drakkan/sftpgo/v2/internal/common"
+	"github.com/drakkan/sftpgo/v2/internal/dataprovider"
 )
 
 const (
@@ -54,12 +55,22 @@ CzgWkxiz7XE4lgUwX44FCXZM3+JeUbI=
 )
 
 func TestInitialization(t *testing.T) {
+	configDir := filepath.Join(".", "..", "..")
+	providerConf := dataprovider.Config{
+		Driver:      dataprovider.MemoryDataProviderName,
+		BackupsPath: "backups",
+	}
+	err := dataprovider.Initialize(providerConf, configDir, false)
+	require.NoError(t, err)
+	commonConfig := common.Configuration{}
+	err = common.Initialize(commonConfig, 0)
+	require.NoError(t, err)
 	c := Conf{
 		BindPort:       10000,
 		BindAddress:    "invalid address",
 		EnableProfiler: false,
 	}
-	err := c.Initialize(".")
+	err = c.Initialize(configDir)
 	require.Error(t, err)
 
 	c.AuthUserFile = "missing"