Address comments in PR

internal/collector/otel_collector_plugin.go

@@ -82,7 +82,10 @@ func (oc *Collector) Init(ctx context.Context, mp bus.MessagePipeInterface) erro
 	if oc.config.Collector.Receivers.OtlpReceivers != nil {
 		for _, receiver := range oc.config.Collector.Receivers.OtlpReceivers {
 			if receiver.OtlpTLSConfig != nil && receiver.OtlpTLSConfig.GenerateSelfSignedCert {
-				slog.WarnContext(ctx, "Self-signed certificate for OTEL receiver requested")
+				slog.WarnContext(ctx,
+					"Self-signed certificate for OTEL receiver requested, "+
+						"this is not recommended for production environments.",
+				)
 				if receiver.OtlpTLSConfig.ExistingCert {
 					slog.WarnContext(
 						ctx,

internal/config/config.go

@@ -422,7 +422,7 @@ func handleSelfSignedCertificates(col *Collector) error {
 }
 
 func processOtlpReceivers(tlsConfig *OtlpTLSConfig) error {
-	sanNames := []string{"127.0.0.1", "::1", "localhost"}
+	sanNames := strings.Split(DefCollectorTLSSANNames, ",")
 
 	if tlsConfig.Ca == "" {
 		tlsConfig.Ca = DefCollectorTLSCAPath
@@ -438,7 +438,7 @@ func processOtlpReceivers(tlsConfig *OtlpTLSConfig) error {
 		sanNames = append(sanNames, tlsConfig.ServerName)
 	}
 	if len(sanNames) > 0 {
-		existingCert, err := selfsignedcerts.GenerateServerCert(
+		existingCert, err := selfsignedcerts.GenerateServerCerts(
 			sanNames,
 			tlsConfig.Ca,
 			tlsConfig.Cert,

internal/config/defaults.go

@@ -22,6 +22,7 @@ const (
 	DefCollectorTLSCertPath          = "/var/lib/nginx-agent/cert.pem"
 	DefCollectorTLSKeyPath           = "/var/lib/nginx-agent/key.pem"
 	DefCollectorTLSCAPath            = "/var/lib/nginx-agent/ca.pem"
+	DefCollectorTLSSANNames          = "127.0.0.1, ::1, localhost"
 
 	DefCommandServerHostKey    = ""
 	DefCommandServerPortKey    = 0

pkg/tls/self_signed_cert.go

@@ -15,7 +15,6 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"log/slog"
 	"math/big"
 	"os"
 	"time"
@@ -35,7 +34,7 @@ type certReq struct {
 	privateKey *ecdsa.PrivateKey
 }
 
-func genCert(req *certReq) (*x509.Certificate, []byte) {
+func genCert(req *certReq) (*x509.Certificate, []byte, error) {
 	certBytes, createCertErr := x509.CreateCertificate(
 		rand.Reader,
 		req.template,
@@ -45,20 +44,18 @@ func genCert(req *certReq) (*x509.Certificate, []byte) {
 	)
 
 	if createCertErr != nil {
-		slog.Error("Failed to generate certificate", "error", createCertErr)
-		return &x509.Certificate{}, []byte{}
+		return &x509.Certificate{}, []byte{}, fmt.Errorf("error generating certificate: %w", createCertErr)
 	}
 
 	cert, parseCertErr := x509.ParseCertificate(certBytes)
 	if parseCertErr != nil {
-		slog.Error("Failed to parse certificate")
-		return &x509.Certificate{}, []byte{}
+		return &x509.Certificate{}, []byte{}, fmt.Errorf("error parsing certificate: %w", parseCertErr)
 	}
 
 	b := pem.Block{Type: "CERTIFICATE", Bytes: certBytes}
 	certPEM := pem.EncodeToMemory(&b)
 
-	return cert, certPEM
+	return cert, certPEM, nil
 }
 
 func GenerateCA(now time.Time, caCertPath string) (*x509.Certificate, *ecdsa.PrivateKey, error) {
@@ -89,13 +86,14 @@ func GenerateCA(now time.Time, caCertPath string) (*x509.Certificate, *ecdsa.Pri
 		privateKey: caKeyPair,
 	}
 
-	caCert, caCertPEM := genCert(&caRequest)
-	if len(caCertPEM) == 0 {
-		slog.Error("Error generating certificate authority")
+	caCert, caCertPEM, caErr := genCert(&caRequest)
+	if caErr != nil {
+		return &x509.Certificate{}, &ecdsa.PrivateKey{}, fmt.Errorf(
+			"error generating certificate authority: %w",
+			caErr)
 	}
 
 	// Write the CA certificate to a file
-	slog.Debug("About to write CA file", "path", caCertPath)
 	writeCAErr := os.WriteFile(caCertPath, caCertPEM, certFilePermissions)
 	if writeCAErr != nil {
 		return &x509.Certificate{}, &ecdsa.PrivateKey{}, fmt.Errorf(
@@ -107,10 +105,11 @@ func GenerateCA(now time.Time, caCertPath string) (*x509.Certificate, *ecdsa.Pri
 	return caCert, caKeyPair, nil
 }
 
+// Writes CA, Cert, Key to specified destinations. If cert files are already present, does nothing, returns true
 // nolint: revive
-func GenerateServerCert(hostnames []string, caPath, certPath, keyPath string) (existingCert bool, err error) {
+func GenerateServerCerts(hostnames []string, caPath, certPath, keyPath string) (existingCert bool, err error) {
 	// Check for and return existing cert if it already exists
-	existingCert, existingCertErr := ReturnExistingCert(certPath)
+	existingCert, existingCertErr := DoesCertAlreadyExist(certPath)
 	if existingCertErr != nil {
 		return false, fmt.Errorf("error reading existing certificate data: %w", existingCertErr)
 	}
@@ -157,9 +156,9 @@ func GenerateServerCert(hostnames []string, caPath, certPath, keyPath string) (e
 	}
 
 	// Generate server certficated signed by the CA
-	_, servCertPEM := genCert(&servRequest)
-	if len(servCertPEM) == 0 {
-		return false, errors.New("error generating server certificate")
+	_, servCertPEM, servCertErr := genCert(&servRequest)
+	if servCertErr != nil {
+		return false, fmt.Errorf("error generating server certificate: %w", servCertErr)
 	}
 
 	// Write the certificate to a file
@@ -183,7 +182,7 @@ func GenerateServerCert(hostnames []string, caPath, certPath, keyPath string) (e
 	return false, nil
 }
 
-func ReturnExistingCert(certPath string) (bool, error) {
+func DoesCertAlreadyExist(certPath string) (bool, error) {
 	if _, certErr := os.Stat(certPath); certErr == nil {
 		certBytes, certReadErr := os.ReadFile(certPath)
 		if certReadErr != nil {

pkg/tls/self_signed_cert_test.go

@@ -219,7 +219,7 @@ func TestGenerateSelfSignedCert(t *testing.T) {
 			err := tc.setup()
 			require.NoError(t, err)
 
-			existingCert, genCertErr := GenerateServerCert(tc.hostNames, tc.caPath, tc.certPath, tc.keyPath)
+			existingCert, genCertErr := GenerateServerCerts(tc.hostNames, tc.caPath, tc.certPath, tc.keyPath)
 
 			// Check the results
 			if tc.expectedError != "" {