Skip to content

Commit

Permalink
Merge pull request #404 from jonathankingfc/PROJQUAY-1577
Browse files Browse the repository at this point in the history
PROJQUAY-1577: Fixed certs being overwritten when BUILDMAN_HOSTNAME not present
  • Loading branch information
jonathankingfc authored Mar 15, 2021
2 parents 6ee3490 + 08aa698 commit 76718ab
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 1 deletion.
7 changes: 6 additions & 1 deletion pkg/kustomize/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
95 changes: 95 additions & 0 deletions pkg/kustomize/secrets_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
})

}

0 comments on commit 76718ab

Please sign in to comment.