improve certificate deduplication operation

Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com>

improvement for TestBundlesDeduplication and name convention

Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com>

pkg/bundle/sync.go

@@ -130,7 +130,7 @@ func (b *bundle) buildSourceBundle(ctx context.Context, bundle *trustapi.Bundle)
 		return bundleData{}, fmt.Errorf("couldn't find any valid certificates in bundle")
 	}
 
-	if err := resolvedBundle.populateData(bundles, bundle.Spec.Target); err != nil {
+	if err := resolvedBundle.populateData(deduplicateBundles(bundles), bundle.Spec.Target); err != nil {
 		return bundleData{}, err
 	}
 
@@ -777,3 +777,21 @@ func (b *bundle) migrateConfigMapToApply(ctx context.Context, obj client.Object,
 	cm.SetManagedFields(managedFields)
 	return true, b.directClient.Update(ctx, &cm)
 }
+
+// remove duplicate certificates from bundles
+func deduplicateBundles(bundles []string) []string {
+	var certificatesHashes = make(map[[32]byte]struct{})
+	var dedupCerts []string
+
+	for _, cert := range bundles {
+		// calculate hash sum of the given certificate
+		hash := sha256.Sum256([]byte(cert))
+		// check existence of the hash
+		if _, ok := certificatesHashes[hash]; !ok {
+			dedupCerts = append(dedupCerts, cert)
+			certificatesHashes[hash] = struct{}{}
+		}
+	}
+
+	return dedupCerts
+}

pkg/bundle/sync_test.go

@@ -1729,3 +1729,30 @@ func Test_certAlias(t *testing.T) {
 		t.Fatalf("expected alias to be %q but got %q", expectedAlias, alias)
 	}
 }
+
+func TestBundlesDeduplication(t *testing.T) {
+	// list of certificates
+	bundle := []string{
+		dummy.TestCertificate3Duplicate,
+		dummy.TestCertificate1,
+		dummy.TestCertificate2,
+		dummy.TestCertificate3,
+		dummy.TestCertificate5Duplicate,
+		dummy.TestCertificate4,
+		dummy.TestCertificate5,
+	}
+
+	// test bundle
+	testBundle := []string{
+		dummy.TestCertificate3Duplicate,
+		dummy.TestCertificate1,
+		dummy.TestCertificate2,
+		dummy.TestCertificate5Duplicate,
+		dummy.TestCertificate4,
+	}
+
+	resultBundle := deduplicateBundles(bundle)
+
+	// check certificates bundle for duplicated certificates
+	assert.ElementsMatch(t, testBundle, resultBundle)
+}

pkg/util/cert_pool.go

@@ -17,7 +17,6 @@ limitations under the License.
 package util
 
 import (
-	"crypto/sha256"
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
@@ -26,36 +25,18 @@ import (
 
 // CertPool is a set of certificates.
 type certPool struct {
-	certificatesHashes map[[32]byte]struct{}
-	certificates       []*x509.Certificate
-	filterExpired      bool
+	certificates  []*x509.Certificate
+	filterExpired bool
 }
 
 // newCertPool returns a new, empty CertPool.
 func newCertPool(filterExpired bool) *certPool {
 	return &certPool{
-		certificatesHashes: make(map[[32]byte]struct{}),
-		certificates:       make([]*x509.Certificate, 0),
-		filterExpired:      filterExpired,
+		certificates:  make([]*x509.Certificate, 0),
+		filterExpired: filterExpired,
 	}
 }
 
-// Check if the given certificate was added to certificates bundle already
-func (cp *certPool) isCertificateDuplicate(certData []byte) bool {
-	// calculate hash sum of the given certificate
-	hash := sha256.Sum256(certData)
-
-	// check a hash existence
-	if _, ok := cp.certificatesHashes[hash]; ok {
-		return ok
-	}
-
-	// put certificate hash into a set of hashes
-	cp.certificatesHashes[hash] = struct{}{}
-
-	return false
-}
-
 // Append certificate to a pool
 func (cp *certPool) appendCertFromPEM(PEMdata []byte) error {
 	if PEMdata == nil {
@@ -94,9 +75,7 @@ func (cp *certPool) appendCertFromPEM(PEMdata []byte) error {
 			continue
 		}
 
-		if !cp.isCertificateDuplicate(block.Bytes) {
-			cp.certificates = append(cp.certificates, certificate)
-		}
+		cp.certificates = append(cp.certificates, certificate)
 	}
 
 	return nil

pkg/util/cert_pool_test.go

@@ -30,69 +30,6 @@ func TestNewCertPool(t *testing.T) {
 	}
 }
 
-func TestCertificatesDeduplication(t *testing.T) {
-	// create a pool
-	certPool := newCertPool(false)
-
-	// list of certificates
-	certificateList := [...]struct {
-		certificateName string
-		certificate     string
-	}{
-		{
-			"TestCertificate3Duplicate", // this certificate is duplicate of TestCertificate3
-			dummy.TestCertificate3Duplicate,
-		},
-		{
-			"TestCertificate1",
-			dummy.TestCertificate1,
-		},
-		{
-			"TestCertificate2",
-			dummy.TestCertificate2,
-		},
-		{
-			"TestCertificate3",
-			dummy.TestCertificate3,
-		},
-		{
-			"TestCertificate5Duplicate", // this certificate is duplicate of TestCertificate5
-			dummy.TestCertificate5Duplicate,
-		},
-		{
-			"TestCertificate4",
-			dummy.TestCertificate4,
-		},
-		{
-			"TestCertificate5",
-			dummy.TestCertificate5,
-		},
-	}
-
-	// certificates bundle structure
-	certificateBundle := []struct {
-		certificateName string
-		certificate     string
-	}{}
-
-	// populate certificates bundle
-	for _, crt := range certificateList {
-		if !certPool.isCertificateDuplicate([]byte(crt.certificate)) {
-			certificateBundle = append(certificateBundle, crt)
-		}
-	}
-
-	// create a new pool
-	newCertPool := newCertPool(false)
-
-	// check certificates bundle for duplicated certificates
-	for _, crt := range certificateBundle {
-		if newCertPool.isCertificateDuplicate([]byte(crt.certificate)) {
-			t.Errorf("duplicate certificate found %s\n", crt.certificateName)
-		}
-	}
-}
-
 func TestAppendCertFromPEM(t *testing.T) {
 	// list of certificates
 	certificateList := [...]struct {