Merge pull request #1420 from fluxcd/fix-tests-race-condition

Fix the race condition happening in main
fluxcd · Mar 29, 2024 · 0c81d04 · 0c81d04
2 parents 74c5f99 + 97bc896
commit 0c81d04
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 55 deletions.
diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go
@@ -25,7 +25,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"net/http"
 	"os"
 	"path"
@@ -35,7 +34,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/foxcpp/go-mockdns"
 	"github.com/notaryproject/notation-core-go/signature/cose"
 	"github.com/notaryproject/notation-core-go/testhelper"
 	"github.com/notaryproject/notation-go"
@@ -1348,14 +1346,6 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 
 	tmpDir := t.TempDir()
 
-	// Unpatch the changes we make to the default DNS resolver in `setupRegistryServer()`.
-	// This is required because the changes somehow also cause remote lookups to fail and
-	// this test tests functionality related to remote dependencies.
-	mockdns.UnpatchNet(net.DefaultResolver)
-	defer func() {
-		testRegistryServer.dnsServer.PatchNet(net.DefaultResolver)
-	}()
-
 	storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
 	g.Expect(err).ToNot(HaveOccurred())
 
@@ -2765,7 +2755,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignatureNotation(t *t
 	metadata, err := loadTestChartToOCI(chartData, server, "", "", "")
 	g.Expect(err).NotTo(HaveOccurred())
 
-	storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
+	storage, err := NewStorage(tmpDir, server.registryHost, retentionTTL, retentionRecords)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	cachedArtifact := &sourcev1.Artifact{
@@ -3089,7 +3079,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignatureCosign(t *tes
 	metadata, err := loadTestChartToOCI(chartData, server, "", "", "")
 	g.Expect(err).NotTo(HaveOccurred())
 
-	storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
+	storage, err := NewStorage(tmpDir, server.registryHost, retentionTTL, retentionRecords)
 	g.Expect(err).ToNot(HaveOccurred())
 
 	cachedArtifact := &sourcev1.Artifact{

diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go
@@ -1378,10 +1378,9 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi
 			g := NewWithT(t)
 
 			workspaceDir := t.TempDir()
-			regOpts := registryOptions{
+			server, err := setupRegistryServer(ctx, workspaceDir, registryOptions{
 				withTLS: !tt.insecure,
-			}
-			server, err := setupRegistryServer(ctx, workspaceDir, regOpts)
+			})
 			g.Expect(err).NotTo(HaveOccurred())
 			t.Cleanup(func() {
 				server.Close()
@@ -1524,7 +1523,6 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes
 	tests := []struct {
 		name                  string
 		reference             *ociv1.OCIRepositoryRef
-		insecure              bool
 		signatureVerification trustpolicy.SignatureVerification
 		trustedIdentities     []string
 		trustStores           []string
@@ -1697,27 +1695,12 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes
 
 	tmpDir := t.TempDir()
 
-	caSecret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:       "valid-trust-store",
-			Generation: 1,
-		},
-		Data: map[string][]byte{
-			"ca.crt": tlsCA,
-		},
-	}
-
-	g.Expect(r.Create(ctx, caSecret)).ToNot(HaveOccurred())
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
 			workspaceDir := t.TempDir()
-			regOpts := registryOptions{
-				withTLS: !tt.insecure,
-			}
-			server, err := setupRegistryServer(ctx, workspaceDir, regOpts)
+			server, err := setupRegistryServer(ctx, workspaceDir, registryOptions{})
 			g.Expect(err).NotTo(HaveOccurred())
 			t.Cleanup(func() {
 				server.Close()
@@ -1777,21 +1760,15 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes
 
 			g.Expect(r.Create(ctx, secret)).NotTo(HaveOccurred())
 
-			if tt.insecure {
-				obj.Spec.Insecure = true
-			} else {
-				obj.Spec.CertSecretRef = &meta.LocalObjectReference{
-					Name: "valid-trust-store",
-				}
-			}
+			obj.Spec.Insecure = true
 
 			obj.Spec.Verify.SecretRef = &meta.LocalObjectReference{Name: "notation"}
 
 			if tt.reference != nil {
 				obj.Spec.Reference = tt.reference
 			}
 
-			podinfoVersions, err := pushMultiplePodinfoImages(server.registryHost, tt.insecure, tt.reference.Tag)
+			podinfoVersions, err := pushMultiplePodinfoImages(server.registryHost, true, tt.reference.Tag)
 			g.Expect(err).ToNot(HaveOccurred())
 
 			if tt.useDigest {
@@ -1811,9 +1788,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes
 			remoteRepo, err := oras.NewRepository(artifactRef.String())
 			g.Expect(err).ToNot(HaveOccurred())
 
-			if tt.insecure {
-				remoteRepo.PlainHTTP = true
-			}
+			remoteRepo.PlainHTTP = true
 
 			repo := registry.NewRepository(remoteRepo)
 

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -156,19 +156,23 @@ func setupRegistryServer(ctx context.Context, workspaceDir string, opts registry
 	// mock DNS to map example.com to 127.0.0.1.
 	// This is required because Docker enforces HTTP if the registry
 	// is hosted on localhost/127.0.0.1.
-	server.registryHost = fmt.Sprintf("example.com:%d", port)
-	// Disable DNS server logging as it is extremely chatty.
-	dnsLog := log.Default()
-	dnsLog.SetOutput(io.Discard)
-	server.dnsServer, err = mockdns.NewServerWithLogger(map[string]mockdns.Zone{
-		"example.com.": {
-			A: []string{"127.0.0.1"},
-		},
-	}, dnsLog, false)
-	if err != nil {
-		return nil, err
+	if opts.withTLS {
+		server.registryHost = fmt.Sprintf("example.com:%d", port)
+		// Disable DNS server logging as it is extremely chatty.
+		dnsLog := log.Default()
+		dnsLog.SetOutput(io.Discard)
+		server.dnsServer, err = mockdns.NewServerWithLogger(map[string]mockdns.Zone{
+			"example.com.": {
+				A: []string{"127.0.0.1"},
+			},
+		}, dnsLog, false)
+		if err != nil {
+			return nil, err
+		}
+		server.dnsServer.PatchNet(net.DefaultResolver)
+	} else {
+		server.registryHost = fmt.Sprintf("localhost:%d", port)
 	}
-	server.dnsServer.PatchNet(net.DefaultResolver)
 
 	config.HTTP.Addr = fmt.Sprintf(":%d", port)
 	config.HTTP.DrainTimeout = time.Duration(10) * time.Second