Add tests for tlsconfigutil.CABundle and all callers should use the constructor

internal/controller/tlsconfigutil/ca_bundle.go

@@ -5,6 +5,9 @@ import (
 	"crypto/x509"
 )
 
+var sHA256OfEmptyData = sha256.Sum256(nil)
+var zeroSHA256 = [32]byte{}
+
 // CABundle abstracts the internal representation of CA certificate bundles.
 type CABundle struct {
 	caBundle []byte
@@ -22,31 +25,41 @@ func NewCABundle(caBundle []byte, certPool *x509.CertPool) *CABundle {
 
 // GetCABundle returns the CA certificate bundle PEM bytes.
 func (c *CABundle) GetCABundle() []byte {
+	if c == nil {
+		return nil
+	}
 	return c.caBundle
 }
 
 // GetCABundlePemString returns the certificate bundle PEM formatted as a string.
 func (c *CABundle) GetCABundlePemString() string {
+	if c == nil {
+		return ""
+	}
 	return string(c.caBundle)
 }
 
 // GetCertPool returns a X509 cert pool with the CA certificate bundle.
 func (c *CABundle) GetCertPool() *x509.CertPool {
+	if c == nil {
+		return nil
+	}
 	return c.certPool
 }
 
 // GetCABundleHash returns a sha256 sum of the CA bundle bytes.
 func (c *CABundle) GetCABundleHash() [32]byte {
-	return sha256.Sum256(c.caBundle) // note that this will always return the same hash for nil input
+	if c == nil || len(c.caBundle) < 1 {
+		return sHA256OfEmptyData
+	}
+	// This handles improperly initialized receivers
+	if c.sha256 == zeroSHA256 {
+		c.sha256 = sha256.Sum256(c.caBundle)
+	}
+	return c.sha256 // note that this will always return the same hash for nil input
 }
 
 // IsEqual returns whether a CABundle has the same CA certificate bundle as another.
 func (c *CABundle) IsEqual(other *CABundle) bool {
-	if c == nil && other == nil {
-		return true
-	}
-	if c == nil || other == nil {
-		return false
-	}
-	return sha256.Sum256(c.caBundle) == sha256.Sum256(other.GetCABundle())
+	return c.GetCABundleHash() == other.GetCABundleHash()
 }

internal/controller/tlsconfigutil/ca_bundle_test.go

@@ -10,6 +10,84 @@ import (
 	"go.pinniped.dev/internal/certauthority"
 )
 
+func TestGetCABundle(t *testing.T) {
+	t.Run("returns the CA bundle", func(t *testing.T) {
+		caBundle := NewCABundle([]byte("here are some bytes"), nil)
+
+		require.Equal(t, []byte("here are some bytes"), caBundle.GetCABundle())
+	})
+
+	t.Run("handles nil receiver by returning nil", func(t *testing.T) {
+		var nilCABundle *CABundle
+		var expected []byte
+		require.Equal(t, expected, nilCABundle.GetCABundle())
+	})
+}
+
+func TestGetCABundlePemString(t *testing.T) {
+	t.Run("returns the CA bundle PEM string", func(t *testing.T) {
+		caBundle := NewCABundle([]byte("here is a string"), nil)
+
+		require.Equal(t, "here is a string", caBundle.GetCABundlePemString())
+	})
+
+	t.Run("handles nil receiver by returning empty sstring", func(t *testing.T) {
+		var nilCABundle *CABundle
+		require.Equal(t, "", nilCABundle.GetCABundlePemString())
+	})
+}
+
+func TestGetCertPool(t *testing.T) {
+	t.Run("returns the cert pool", func(t *testing.T) {
+		aCertPool := x509.NewCertPool()
+		caBundle := NewCABundle(nil, aCertPool)
+
+		require.Equal(t, aCertPool, caBundle.GetCertPool())
+	})
+
+	t.Run("handles nil receiver by returning nil", func(t *testing.T) {
+		var nilCABundle *CABundle
+		var expected *x509.CertPool
+		require.Equal(t, expected, nilCABundle.GetCertPool())
+	})
+}
+
+func TestGetCABundleHash(t *testing.T) {
+	sha256OfNil := [32]uint8{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}
+
+	// On the command line, `echo "test" | shasum -a 256` yields "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
+	// which is 32 bytes of data encoded as 64 characters.
+	// https://stackoverflow.com/a/70565837
+	// This is the actual binary data:
+	sha256OfTest := [32]byte{159, 134, 208, 129, 136, 76, 125, 101, 154, 47, 234, 160, 197, 90, 208, 21, 163, 191, 79, 27, 43, 11, 130, 44, 209, 93, 108, 21, 176, 240, 10, 8}
+
+	t.Run("returns the SHA256", func(t *testing.T) {
+		caBundle := NewCABundle([]byte("test"), nil)
+
+		require.Equal(t, sha256OfTest, caBundle.GetCABundleHash())
+	})
+
+	t.Run("handles nil receiver by returning the hash of nil", func(t *testing.T) {
+		var nilCABundle *CABundle
+
+		require.Equal(t, sha256OfNil, nilCABundle.GetCABundleHash())
+	})
+
+	t.Run("handles improperly initialized receiver by returning the hash of nil", func(t *testing.T) {
+		caBundle := &CABundle{}
+
+		require.Equal(t, sha256OfNil, caBundle.GetCABundleHash())
+	})
+
+	t.Run("handles improperly initialized receiver by computing the hash", func(t *testing.T) {
+		caBundle := &CABundle{
+			caBundle: []byte("test"),
+		}
+
+		require.Equal(t, sha256OfTest, caBundle.GetCABundleHash())
+	})
+}
+
 func TestCABundleIsEqual(t *testing.T) {
 	testCA, err := certauthority.New("Test CA", 1*time.Hour)
 	require.NoError(t, err)
@@ -29,39 +107,27 @@ func TestCABundleIsEqual(t *testing.T) {
 			expected: true,
 		},
 		{
-			name:     "should return not equal when left is nil and right is not",
+			name:     "should return equal when left is nil and right is empty",
 			left:     nil,
 			right:    &CABundle{},
-			expected: false,
-		},
-		{
-			name:     "should return not equal when right is nil and left is not",
-			left:     &CABundle{},
-			right:    nil,
-			expected: false,
-		},
-		{
-			name: "should return equal when both left and right have same CA certificate bytes",
-			left: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
-			right: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
 			expected: true,
 		},
 		{
-			name: "should return not equal when both left and right do not have same CA certificate bytes",
-			left: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
-			right: &CABundle{
-				caBundle: []byte("something that is not a cert"),
-				certPool: nil,
-			},
+			name:     "should return equal when right is nil and left is empty",
+			left:     &CABundle{},
+			right:    nil,
+			expected: true,
+		},
+		{
+			name:     "should return equal when both left and right have same CA certificate bytes",
+			left:     NewCABundle(testCA.Bundle(), certPool),
+			right:    NewCABundle(testCA.Bundle(), certPool),
+			expected: true,
+		},
+		{
+			name:     "should return not equal when both left and right do not have same CA certificate bytes",
+			left:     NewCABundle(testCA.Bundle(), certPool),
+			right:    NewCABundle([]byte("something that is not a cert"), certPool),
 			expected: false,
 		},
 	}
@@ -69,8 +135,8 @@ func TestCABundleIsEqual(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			actual := tt.left.IsEqual(tt.right)
-			require.Equal(t, tt.expected, actual)
+			require.Equal(t, tt.expected, tt.left.IsEqual(tt.right))
+			require.Equal(t, tt.expected, tt.right.IsEqual(tt.left))
 		})
 	}
 }

internal/controller/tlsconfigutil/tls_config_util_test.go

@@ -64,10 +64,7 @@ func TestValidateTLSConfig(t *testing.T) {
 			tlsSpec: &TLSSpec{
 				CertificateAuthorityData: base64EncodedBundle,
 			},
-			expectedCABundle: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
+			expectedCABundle: NewCABundle(testCA.Bundle(), certPool),
 			expectedCondition: &metav1.Condition{
 				Type:    typeTLSConfigurationValid,
 				Status:  metav1.ConditionTrue,
@@ -138,10 +135,7 @@ func TestValidateTLSConfig(t *testing.T) {
 					},
 				},
 			},
-			expectedCABundle: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
+			expectedCABundle: NewCABundle(testCA.Bundle(), certPool),
 			expectedCondition: &metav1.Condition{
 				Type:    typeTLSConfigurationValid,
 				Status:  metav1.ConditionTrue,
@@ -171,10 +165,7 @@ func TestValidateTLSConfig(t *testing.T) {
 					},
 				},
 			},
-			expectedCABundle: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
+			expectedCABundle: NewCABundle(testCA.Bundle(), certPool),
 			expectedCondition: &metav1.Condition{
 				Type:    typeTLSConfigurationValid,
 				Status:  metav1.ConditionTrue,
@@ -403,10 +394,7 @@ func TestValidateTLSConfig(t *testing.T) {
 					},
 				},
 			},
-			expectedCABundle: &CABundle{
-				caBundle: testCA.Bundle(),
-				certPool: certPool,
-			},
+			expectedCABundle: NewCABundle(testCA.Bundle(), certPool),
 			expectedCondition: &metav1.Condition{
 				Type:    typeTLSConfigurationValid,
 				Status:  metav1.ConditionTrue,