fix: TestToTLS to use unreferenced config attributes (#1952)

* Fix test to use unreferenced config attributes * Adjust caCertPool solution
observIQ · Nov 8, 2024 · 5ba3fa4 · 5ba3fa4
1 parent 9ac8eff
commit 5ba3fa4
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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)