PROJQUAY-1577: Fixed certs being overwritten when BUILDMAN_HOSTNAME not present

pkg/kustomize/secrets.go

@@ -173,11 +173,16 @@ func EnsureTLSFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, t
 		svc,
 		strings.Join([]string{svc, quay.GetNamespace(), "svc"}, "."),
 		strings.Join([]string{svc, quay.GetNamespace(), "svc", "cluster", "local"}, "."),
-		strings.Split(ctx.BuildManagerHostname, ":")[0],
+	}
+
+	// Only add BUILDMAN_HOSTNAME as host if provided.
+	if ctx.BuildManagerHostname != "" {
+		hosts = append(hosts, strings.Split(ctx.BuildManagerHostname, ":")[0])
 	}
 
 	for _, host := range hosts {
 		if valid, _ := shared.ValidateCertPairWithHostname(tlsCert, tlsKey, host, fieldGroupNameFor("route")); !valid {
+			fmt.Printf("Host %s not valid for certificates provided. Generating self-signed certs", host) // change to logger?
 			return cert.GenerateSelfSignedCertKey(routeFieldGroup.ServerHostname, []net.IP{}, hosts)
 		}
 	}

pkg/kustomize/secrets_test.go

@@ -1,11 +1,14 @@
 package kustomize
 
 import (
+	"net"
 	"net/url"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/util/cert"
 	"sigs.k8s.io/yaml"
 
 	"github.com/quay/config-tool/pkg/lib/fieldgroups/database"
@@ -315,3 +318,95 @@ func TestContainsComponentConfig(t *testing.T) {
 		}
 	}
 }
+
+func TestEnsureTLSFor(t *testing.T) {
+
+	emptyCert := []byte{}
+	serverHostname := "serverhostname.com"
+	builderHostname := "builderhostname.com"
+	quayRegistry := quayRegistry("test")
+
+	quayContextWithoutBuilder := &quaycontext.QuayRegistryContext{
+		ServerHostname: serverHostname,
+	}
+	quayContextWithBuilder := &quaycontext.QuayRegistryContext{
+		BuildManagerHostname: builderHostname,
+		ServerHostname:       serverHostname,
+	}
+
+	svc := quayRegistry.GetName() + "-quay-app"
+	hostsWithoutBuilder := []string{
+		serverHostname,
+		svc,
+		strings.Join([]string{svc, quayRegistry.GetNamespace(), "svc"}, "."),
+		strings.Join([]string{svc, quayRegistry.GetNamespace(), "svc", "cluster", "local"}, "."),
+	}
+	// Generate certs without builder hostname in SAN
+	pubWithoutBuilder, privWithoutBuilder, err := cert.GenerateSelfSignedCertKey(serverHostname, []net.IP{}, hostsWithoutBuilder)
+	if err != nil {
+		t.Errorf(err.Error())
+	}
+
+	// Generate certs with builder hostname in SAN
+	hostsWithBuilder := append(hostsWithoutBuilder, builderHostname)
+	pubWithBuilder, privWithBuilder, err := cert.GenerateSelfSignedCertKey(serverHostname, []net.IP{}, hostsWithBuilder)
+	if err != nil {
+		t.Errorf(err.Error())
+	}
+
+	t.Run("Quay context has buildman hostname. Certs are empty, Operator should generate own.", func(t *testing.T) {
+		assert := assert.New(t)
+		// Empty certs, should generate own
+		recPublicKey, recPrivateKey, err := EnsureTLSFor(quayContextWithBuilder, quayRegistry, emptyCert, emptyCert)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		assert.NotEqual(recPublicKey, emptyCert)
+		assert.NotEqual(recPrivateKey, emptyCert)
+	})
+
+	t.Run("Quay context has buildman hostname. Certs do not contain buildman hostname. Operator should generate own.", func(t *testing.T) {
+		assert := assert.New(t)
+		// Buildman missing from certs, should generate own
+		recPublicKey, recPrivateKey, err := EnsureTLSFor(quayContextWithBuilder, quayRegistry, pubWithoutBuilder, privWithoutBuilder)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		assert.NotEqual(recPublicKey, pubWithoutBuilder)
+		assert.NotEqual(recPrivateKey, privWithoutBuilder)
+	})
+
+	t.Run("Quay context has buildman hostname. Certs contain buildman hostname. Operator should not generate own.", func(t *testing.T) {
+		assert := assert.New(t)
+		// Buildman present in certs, should not generate
+		recPublicKey, recPrivateKey, err := EnsureTLSFor(quayContextWithBuilder, quayRegistry, pubWithBuilder, privWithBuilder)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		assert.Equal(recPublicKey, pubWithBuilder)
+		assert.Equal(recPrivateKey, privWithBuilder)
+	})
+
+	t.Run("Quay context does not have buildman hostname. Certs do not contain buildman hostname. Operator should not generate own.", func(t *testing.T) {
+		assert := assert.New(t)
+		// Buildman not in certs, not in context, should not generate
+		recPublicKey, recPrivateKey, err := EnsureTLSFor(quayContextWithoutBuilder, quayRegistry, pubWithoutBuilder, privWithoutBuilder)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		assert.Equal(recPublicKey, pubWithoutBuilder)
+		assert.Equal(recPrivateKey, privWithoutBuilder)
+	})
+
+	t.Run("Quay context does not have buildman hostname. Certs contain buildman hostname. Operator should not generate own.", func(t *testing.T) {
+		assert := assert.New(t)
+		// Buildman not in certs, not in context, should not generate
+		recPublicKey, recPrivateKey, err := EnsureTLSFor(quayContextWithoutBuilder, quayRegistry, pubWithBuilder, privWithBuilder)
+		if err != nil {
+			t.Errorf(err.Error())
+		}
+		assert.Equal(recPublicKey, pubWithBuilder)
+		assert.Equal(recPrivateKey, privWithBuilder)
+	})
+
+}