From 015801f896bef034cf55d665fcaa8f14cab740bd Mon Sep 17 00:00:00 2001 From: Alec Merdler Date: Thu, 18 Mar 2021 16:35:28 -0700 Subject: [PATCH] working on tests --- kustomize/base/quay.service.yaml | 2 +- pkg/kustomize/kustomize.go | 1 + pkg/kustomize/secrets.go | 2 - pkg/kustomize/secrets_test.go | 217 +++++++++++++++++++------------ 4 files changed, 138 insertions(+), 84 deletions(-) diff --git a/kustomize/base/quay.service.yaml b/kustomize/base/quay.service.yaml index c4c8d7d28..df5ba6c01 100644 --- a/kustomize/base/quay.service.yaml +++ b/kustomize/base/quay.service.yaml @@ -5,7 +5,7 @@ metadata: labels: quay-component: quay-app spec: - type: LoadBalancer + type: ClusterIP ports: - protocol: TCP name: https diff --git a/pkg/kustomize/kustomize.go b/pkg/kustomize/kustomize.go index f046ffb0c..78d50d5bc 100644 --- a/pkg/kustomize/kustomize.go +++ b/pkg/kustomize/kustomize.go @@ -462,6 +462,7 @@ func Inflate(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, baseCo } } + // FIXME(alecmerdler): Should we create certs if `route` component is unmanaged...? log.Info("Ensuring `ssl.cert` and `ssl.key` pair for Quay app TLS") tlsCert, tlsKey, err := EnsureTLSFor(ctx, quay, baseConfigBundle.Data["ssl.cert"], baseConfigBundle.Data["ssl.key"]) if err != nil { diff --git a/pkg/kustomize/secrets.go b/pkg/kustomize/secrets.go index 7b4752f99..77f1f511e 100644 --- a/pkg/kustomize/secrets.go +++ b/pkg/kustomize/secrets.go @@ -159,8 +159,6 @@ func BaseConfig() map[string]interface{} { // EnsureTLSFor checks if given TLS cert/key pair are valid for the Quay registry to use for secure communication with clients, // and generates a TLS certificate/key pair if they are invalid. -// In addition to `SERVER_HOSTNAME`, it sets certificate subject alternative names -// for the internal k8s service hostnames (i.e. `registry-quay-app.quay-enterprise.svc`). func EnsureTLSFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, tlsCert, tlsKey []byte) ([]byte, []byte, error) { fieldGroup, err := FieldGroupFor(ctx, "route", quay) if err != nil { diff --git a/pkg/kustomize/secrets_test.go b/pkg/kustomize/secrets_test.go index 7a3373de5..be2fbf217 100644 --- a/pkg/kustomize/secrets_test.go +++ b/pkg/kustomize/secrets_test.go @@ -1,9 +1,7 @@ package kustomize import ( - "net" "net/url" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -319,94 +317,151 @@ func TestContainsComponentConfig(t *testing.T) { } } -func TestEnsureTLSFor(t *testing.T) { +func certFor(hostname string) []byte { + cert, _, _ := cert.GenerateSelfSignedCertKey(hostname, nil, nil) - emptyCert := []byte{} - serverHostname := "serverhostname.com" - builderHostname := "builderhostname.com" - quayRegistry := quayRegistry("test") + return cert +} - quayContextWithoutBuilder := &quaycontext.QuayRegistryContext{ - ServerHostname: serverHostname, - } - quayContextWithBuilder := &quaycontext.QuayRegistryContext{ - BuildManagerHostname: builderHostname, - ServerHostname: serverHostname, - } +func keyFor(hostname string) []byte { + _, key, _ := cert.GenerateSelfSignedCertKey(hostname, nil, nil) - 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()) - } + return key +} - // 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()) - } +var ensureTLSForTests = []struct { + name string + routeManaged bool + serverHostname string + buildManagerHostname string + providedCert []byte + providedKey []byte + expectedMatch bool + expectedErr error +}{ + { + "ManagedRouteNoHostnameNoCerts", + true, + "", + "", + nil, + nil, + false, + nil, + }, + { + "ManagedRouteNoHostnameProvidedCerts", + true, + "", + "", + certFor("nonexistent.company.com"), + keyFor("nonexistent.company.com"), + false, + nil, + }, + { + "ManagedRouteProvidedHostnameProvidedIncorrectCerts", + true, + "registry.company.com", + "", + certFor("nonexistent.company.com"), + certFor("nonexistent.company.com"), + false, + nil, + }, + { + "ManagedRouteProvidedHostnameNoCerts", + true, + "registry.company.com", + "", + nil, + nil, + false, + nil, + }, + { + "ManagedRouteProvidedHostnameProvidedCerts", + true, + "registry.company.com", + "", + certFor("registry.company.com"), + keyFor("registry.company.com"), + true, + nil, + }, + // { + // "UnmanagedRouteNoHostnameNoCerts", + // false, + // "", + // "", + // nil, + // nil, + // nil, + // nil, + // errors.New("cannot validate TLS configuration: `SERVER_HOSTNAME` not set"), + // }, + // { + // "UnmanagedRouteNoHostnameProvidedCerts", + // false, + // "", + // "", + // nil, + // nil, + // nil, + // nil, + // errors.New("cannot validate TLS configuration: `SERVER_HOSTNAME` not set"), + // }, + // { + // "UnmanagedRouteProvidedHostnameNoCerts", + // false, + // "registry.company.com", + // "", + // nil, + // nil, + // certFor("registry.company.com"), + // keyFor("registry.company.com"), + // nil, + // }, + // { + // "UnmanagedRouteProvidedHostnameProvidedInvalidCerts", + // false, + // "registry.company.com", + // "", + // certFor("nonexistent.company.com"), + // keyFor("nonexistent.company.com"), + // certFor("registry.company.com"), + // keyFor("registry.company.com"), + // nil, + // }, + // { + // "UnmanagedRouteProvidedHostnameProvidedCerts", + // false, + // "registry.company.com", + // "", + // certFor("registry.company.com"), + // keyFor("registry.company.com"), + // certFor("registry.company.com"), + // keyFor("registry.company.com"), + // nil, + // }, +} - 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) - }) +func TestEnsureTLSFor(t *testing.T) { + assert := assert.New(t) - 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) - }) + for _, test := range ensureTLSForTests { + quayRegistry := quayRegistry("test") - 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()) + quayContext := quaycontext.QuayRegistryContext{ + ServerHostname: test.serverHostname, + BuildManagerHostname: test.buildManagerHostname, } - 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) - }) + tlsCert, tlsKey, err := EnsureTLSFor(&quayContext, quayRegistry, test.providedCert, test.providedKey) - 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) - }) + assert.Equal(test.expectedErr, err, test.name) + assert.Equal(test.expectedMatch, string(tlsCert) == string(test.providedCert), test.name) + assert.Equal(test.expectedMatch, string(tlsKey) == string(test.providedKey), test.name) + } }