From 5ba3fa4e139902acb00af9744b19b28f67b608a1 Mon Sep 17 00:00:00 2001
From: Ian Adams <ian.sam.adams@gmail.com>
Date: Fri, 8 Nov 2024 15:54:12 -0500
Subject: [PATCH] fix: TestToTLS to use unreferenced config attributes (#1952)

* Fix test to use unreferenced config attributes

* Adjust caCertPool solution
---
 opamp/config.go                   |  6 ++++--
 opamp/config_test.go              | 19 ++++++++++---------
 opamp/observiq/observiq_client.go |  4 ++--
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/opamp/config.go b/opamp/config.go
index 72b91c4ed..7b9f5bc81 100644
--- a/opamp/config.go
+++ b/opamp/config.go
@@ -212,7 +212,7 @@ type TLSConfig struct {
 }
 
 // ToTLS converts the config to a tls.Config
-func (c Config) ToTLS() (*tls.Config, error) {
+func (c Config) ToTLS(caCertPool *x509.CertPool) (*tls.Config, error) {
 	if c.TLS == nil {
 		return nil, nil
 	}
@@ -232,7 +232,9 @@ func (c Config) ToTLS() (*tls.Config, error) {
 		if err != nil {
 			return nil, fmt.Errorf("failed to read CA file: %w", err)
 		}
-		caCertPool := x509.NewCertPool()
+		if caCertPool == nil {
+			caCertPool = x509.NewCertPool()
+		}
 		caCertPool.AppendCertsFromPEM(caCert)
 
 		tlsConfig.RootCAs = caCertPool
diff --git a/opamp/config_test.go b/opamp/config_test.go
index e67c02f33..58c547efe 100644
--- a/opamp/config_test.go
+++ b/opamp/config_test.go
@@ -58,7 +58,7 @@ func TestToTLS(t *testing.T) {
 					TLS: nil,
 				}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.NoError(t, err)
 				assert.Nil(t, actual)
 			},
@@ -77,7 +77,7 @@ func TestToTLS(t *testing.T) {
 					MinVersion:         tls.VersionTLS12,
 				}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.NoError(t, err)
 				assert.Equal(t, &expectedConfig, actual)
 			},
@@ -95,7 +95,7 @@ func TestToTLS(t *testing.T) {
 					MinVersion: tls.VersionTLS12,
 				}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.NoError(t, err)
 				assert.Equal(t, &expectedConfig, actual)
 			},
@@ -110,7 +110,7 @@ func TestToTLS(t *testing.T) {
 					},
 				}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.ErrorContains(t, err, "failed to read CA file")
 				assert.Nil(t, actual)
 			},
@@ -125,7 +125,7 @@ func TestToTLS(t *testing.T) {
 					},
 				}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.NoError(t, err)
 				assert.NotNil(t, actual)
 				assert.False(t, actual.InsecureSkipVerify)
@@ -147,7 +147,7 @@ func TestToTLS(t *testing.T) {
 				_, err := tls.LoadX509KeyPair(invalidCertFile, invalidKeyFile)
 				errinvalidKeyorCertFile := fmt.Sprintf("failed to read Key and Cert file: %s", err)
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.ErrorContains(t, err, errinvalidKeyorCertFile)
 				assert.Nil(t, actual)
 			},
@@ -171,7 +171,7 @@ func TestToTLS(t *testing.T) {
 				require.NoError(t, err)
 				expectedConfig.Certificates = []tls.Certificate{cert}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(nil)
 				assert.NoError(t, err)
 				assert.Equal(t, &expectedConfig, actual)
 			},
@@ -202,9 +202,10 @@ func TestToTLS(t *testing.T) {
 				require.NoError(t, err)
 				expectedConfig.Certificates = []tls.Certificate{cert}
 
-				actual, err := cfg.ToTLS()
+				actual, err := cfg.ToTLS(caCertPool)
 				assert.NoError(t, err)
-				assert.Equal(t, expectedConfig.Certificates, actual.Certificates)
+
+				assert.Equal(t, &expectedConfig, actual)
 			},
 		},
 	}
diff --git a/opamp/observiq/observiq_client.go b/opamp/observiq/observiq_client.go
index f14fb12a2..e77db1915 100644
--- a/opamp/observiq/observiq_client.go
+++ b/opamp/observiq/observiq_client.go
@@ -103,7 +103,7 @@ func NewClient(args *NewClientArgs) (opamp.Client, error) {
 	}
 
 	// Propagate TLS config to reportManager agent
-	tlsCfg, err := args.Config.ToTLS()
+	tlsCfg, err := args.Config.ToTLS(nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed creating TLS config: %w", err)
 	}
@@ -208,7 +208,7 @@ func (c *Client) Connect(ctx context.Context) error {
 		return err
 	}
 
-	tlsCfg, err := c.currentConfig.ToTLS()
+	tlsCfg, err := c.currentConfig.ToTLS(nil)
 	if err != nil {
 		// Set package status file for error (for Updater to pick up), but do not force send to Server
 		c.tryToFailPackageInstall(fmt.Sprintf("Failed creating TLS config: %s", err.Error()), false)