Skip to content

Commit

Permalink
tls: persist generated TLS cert/key pair (PROJQUAY-1838)
Browse files Browse the repository at this point in the history 
Move the 'ssl.cert' and 'ssl.key' to a separate, persistent
Secret to ensure that the cert/key pair is not re-generated
on every reconcile. Use k8s projected volumes to mount the
config and TLS Secrets to the same directory in the Quay
container.

Signed-off-by: Alec Merdler <alecmerdler@gmail.com>
  • Loading branch information
@alecmerdler
alecmerdler committed May 14, 2021
1 parent 8903b99 commit 9507d22
Show file tree
Hide file tree
Showing 13 changed files with 226 additions and 65 deletions.
9 changes: 8 additions & 1 deletion apis/quay/v1/quayregistry_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ var requiredComponents = []ComponentKind{
ComponentRoute,
}

const ManagedKeysName = "quay-registry-managed-secret-keys"
const (
ManagedKeysName = "quay-registry-managed-secret-keys"
QuayConfigTLSSecretName = "quay-config-tls"
)

// QuayRegistrySpec defines the desired state of QuayRegistry.
type QuayRegistrySpec struct {
Expand Down Expand Up @@ -387,6 +390,10 @@ func IsManagedKeysSecretFor(quay *QuayRegistry, secret *corev1.Secret) bool {
return strings.Contains(secret.GetName(), quay.GetName()+"-"+ManagedKeysName)
}

func IsManagedTLSSecretFor(quay *QuayRegistry, secret *corev1.Secret) bool {
return strings.Contains(secret.GetName(), quay.GetName()+"-"+QuayConfigTLSSecretName)
}

func init() {
SchemeBuilder.Register(&QuayRegistry{}, &QuayRegistryList{})
}
87 changes: 66 additions & 21 deletions controllers/quay/features.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ const (
GrafanaDashboardConfigNamespace = "openshift-config-managed"
)

func (r *QuayRegistryReconciler) checkManagedKeys(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, rawConfig []byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
// FeatureDetection is a method which should return an updated `QuayRegistryContext` after performing a feature detection task.
// TODO(alecmerdler): Refactor all "feature detection" functions to use a common function interface...
type FeatureDetection func(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error)

func (r *QuayRegistryReconciler) checkManagedKeys(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
var secrets corev1.SecretList
listOptions := &client.ListOptions{
Namespace: quay.GetNamespace(),
Expand All @@ -64,7 +68,62 @@ func (r *QuayRegistryReconciler) checkManagedKeys(ctx *quaycontext.QuayRegistryC
return ctx, quay, nil
}

func (r *QuayRegistryReconciler) checkRoutesAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, rawConfig []byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
func (r *QuayRegistryReconciler) checkManagedTLS(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
providedTLSCert := configBundle["ssl.cert"]
providedTLSKey := configBundle["ssl.key"]

if providedTLSCert != nil && providedTLSKey != nil {
r.Log.Info("provided TLS cert/key pair in `configBundleSecret` will be stored in persistent `Secret`")
ctx.TLSCert = providedTLSCert
ctx.TLSKey = providedTLSKey

return ctx, quay, nil
}

var secrets corev1.SecretList
listOptions := &client.ListOptions{
Namespace: quay.GetNamespace(),
LabelSelector: labels.SelectorFromSet(map[string]string{
kustomize.QuayRegistryNameLabel: quay.GetName(),
}),
}

if err := r.List(context.Background(), &secrets, listOptions); err != nil {
return ctx, quay, err
}

for _, secret := range secrets.Items {
if v1.IsManagedTLSSecretFor(quay, &secret) {
ctx.TLSCert = secret.Data["ssl.cert"]
ctx.TLSKey = secret.Data["ssl.key"]
break
}
}

if ctx.TLSCert == nil || ctx.TLSKey == nil {
r.Log.Info("existing TLS cert/key pair not found, one will be generated")
}

return ctx, quay, nil
}

func (r *QuayRegistryReconciler) checkRoutesAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
// NOTE: The `route` component is unique because we allow users to set the `SERVER_HOSTNAME` field instead of controlling the entire fieldgroup.
// This value is then passed to the created `Route` using a Kustomize variable.
var config map[string]interface{}
if err := yaml.Unmarshal(configBundle["config.yaml"], &config); err != nil {
return ctx, quay, err
}

fieldGroup, err := hostsettings.NewHostSettingsFieldGroup(config)
if err != nil {
return ctx, quay, err
}

if fieldGroup.ServerHostname != "" {
ctx.ServerHostname = fieldGroup.ServerHostname
}

fakeRoute, err := v1.EnsureOwnerReference(quay, &routev1.Route{
ObjectMeta: metav1.ObjectMeta{
Name: quay.GetName() + "-test-route",
Expand Down Expand Up @@ -101,21 +160,7 @@ func (r *QuayRegistryReconciler) checkRoutesAvailable(ctx *quaycontext.QuayRegis
return ctx, quay, err
}

// NOTE: The `route` component is unique because we allow users to set the `SERVER_HOSTNAME` field instead of controlling the entire fieldgroup.
// This value is then passed to the created `Route` using a Kustomize variable.
var config map[string]interface{}
if err := yaml.Unmarshal(rawConfig, &config); err != nil {
return ctx, quay, err
}

fieldGroup, err := hostsettings.NewHostSettingsFieldGroup(config)
if err != nil {
return ctx, quay, err
}

if fieldGroup.ServerHostname != "" {
ctx.ServerHostname = fieldGroup.ServerHostname
} else {
if ctx.ServerHostname == "" {
ctx.ServerHostname = strings.Join([]string{
strings.Join([]string{quay.GetName(), "quay", quay.GetNamespace()}, "-"),
ctx.ClusterHostname},
Expand All @@ -136,7 +181,7 @@ func (r *QuayRegistryReconciler) checkRoutesAvailable(ctx *quaycontext.QuayRegis
return ctx, quay, nil
}

func (r *QuayRegistryReconciler) checkObjectBucketClaimsAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, rawConfig []byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
func (r *QuayRegistryReconciler) checkObjectBucketClaimsAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
datastoreName := types.NamespacedName{Namespace: quay.GetNamespace(), Name: quay.GetName() + "-quay-datastore"}
var objectBucketClaims objectbucket.ObjectBucketClaimList
if err := r.Client.List(context.Background(), &objectBucketClaims); err == nil {
Expand Down Expand Up @@ -192,9 +237,9 @@ func (r *QuayRegistryReconciler) checkObjectBucketClaimsAvailable(ctx *quayconte
}

// TODO: Improve this once `builds` is a managed component.
func (r *QuayRegistryReconciler) checkBuildManagerAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, rawConfig []byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
func (r *QuayRegistryReconciler) checkBuildManagerAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
var config map[string]interface{}
if err := yaml.Unmarshal(rawConfig, &config); err != nil {
if err := yaml.Unmarshal(configBundle["config.yaml"], &config); err != nil {
return ctx, quay, err
}

Expand All @@ -208,7 +253,7 @@ func (r *QuayRegistryReconciler) checkBuildManagerAvailable(ctx *quaycontext.Qua
// Validates if the monitoring component can be run. We assume that we are
// running in an Openshift environment with cluster monitoring enabled for our
// monitoring component to work
func (r *QuayRegistryReconciler) checkMonitoringAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, rawConfig []byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
func (r *QuayRegistryReconciler) checkMonitoringAvailable(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, configBundle map[string][]byte) (*quaycontext.QuayRegistryContext, *v1.QuayRegistry, error) {
if len(r.WatchNamespace) > 0 {
msg := "monitoring is only supported in AllNamespaces mode. Disabling component monitoring"
r.Log.Info(msg)
Expand Down
19 changes: 14 additions & 5 deletions controllers/quay/quayregistry_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ type QuayRegistryReconciler struct {
Scheme *runtime.Scheme
EventRecorder record.EventRecorder
WatchNamespace string

// TODO(alecmerdler): Somehow generalize feature detection functions so that they can match a type signature and be iterated over...
}

// +kubebuilder:rbac:groups=quay.redhat.com,resources=quayregistries,verbs=get;list;watch;create;update;patch;delete
Expand Down Expand Up @@ -163,21 +165,28 @@ func (r *QuayRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request

log.Info("successfully retrieved referenced `configBundleSecret`", "configBundleSecret", configBundle.GetName(), "resourceVersion", configBundle.GetResourceVersion())

quayContext, updatedQuay, err := r.checkManagedKeys(quayContext, updatedQuay.DeepCopy(), configBundle.Data["config.yaml"])
quayContext, updatedQuay, err := r.checkManagedKeys(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil {
msg := fmt.Sprintf("unable to retrieve managed keys `Secret`: %s", err)

return r.reconcileWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonConfigInvalid, msg)
}

quayContext, updatedQuay, err = r.checkRoutesAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data["config.yaml"])
quayContext, updatedQuay, err = r.checkManagedTLS(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil {
msg := fmt.Sprintf("unable to retrieve managed TLS `Secret`: %s", err)

return r.reconcileWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonConfigInvalid, msg)
}

quayContext, updatedQuay, err = r.checkRoutesAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil && v1.ComponentIsManaged(updatedQuay.Spec.Components, v1.ComponentRoute) {
msg := fmt.Sprintf("could not check for `Routes` API: %s", err)

return r.reconcileWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonRouteComponentDependencyError, msg)
}

quayContext, updatedQuay, err = r.checkObjectBucketClaimsAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data["config.yaml"])
quayContext, updatedQuay, err = r.checkObjectBucketClaimsAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil && v1.ComponentIsManaged(updatedQuay.Spec.Components, v1.ComponentObjectStorage) {
msg := fmt.Sprintf("could not check for `ObjectBucketClaims` API: %s", err)
if _, err = r.updateWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonObjectStorageComponentDependencyError, msg); err != nil {
Expand All @@ -187,14 +196,14 @@ func (r *QuayRegistryReconciler) Reconcile(ctx context.Context, req ctrl.Request
return ctrl.Result{RequeueAfter: time.Millisecond * 1000}, nil
}

quayContext, updatedQuay, err = r.checkBuildManagerAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data["config.yaml"])
quayContext, updatedQuay, err = r.checkBuildManagerAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil {
msg := fmt.Sprintf("could not check for build manager support: %s", err)

return r.reconcileWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonObjectStorageComponentDependencyError, msg)
}

quayContext, updatedQuay, err = r.checkMonitoringAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data["config.yaml"])
quayContext, updatedQuay, err = r.checkMonitoringAvailable(quayContext, updatedQuay.DeepCopy(), configBundle.Data)
if err != nil && v1.ComponentIsManaged(updatedQuay.Spec.Components, v1.ComponentMonitoring) {
msg := fmt.Sprintf("could not check for monitoring support: %s", err)

Expand Down
90 changes: 80 additions & 10 deletions controllers/quay/quayregistry_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/util/cert"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

v1 "github.com/quay/quay-operator/apis/quay/v1"
quaycontext "github.com/quay/quay-operator/pkg/context"
"github.com/quay/quay-operator/pkg/kustomize"
)

func newQuayRegistry(name, namespace string) *v1.QuayRegistry {
Expand All @@ -41,21 +44,37 @@ func newQuayRegistry(name, namespace string) *v1.QuayRegistry {
return quay
}

func newConfigBundle(name, namespace string) corev1.Secret {
func newConfigBundle(name, namespace string, withCerts bool) corev1.Secret {
config := map[string]interface{}{
"ENTERPRISE_LOGO_URL": "/static/img/quay-horizontal-color.svg",
"FEATURE_SUPER_USERS": true,
"SERVER_HOSTNAME": "quay-app.quay-enterprise",
}

data := map[string][]byte{
"config.yaml": encode(config),
}

if withCerts {
cert, key, err := cert.GenerateSelfSignedCertKey(
config["SERVER_HOSTNAME"].(string),
nil,
[]string{config["SERVER_HOSTNAME"].(string)})

if err != nil {
panic(err)
}

data["ssl.cert"] = cert
data["ssl.key"] = key
}

return corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Data: map[string][]byte{
"config.yaml": encode(config),
},
Data: data,
}
}

Expand Down Expand Up @@ -139,7 +158,7 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
Name: updatedQuayRegistry.Spec.ConfigBundleSecret,
Namespace: quayRegistry.GetNamespace()},
&configBundleSecret)).
Should(Succeed())
To(Succeed())
})

It("will reference the same `configBundleSecret` when reconciled again", func() {
Expand All @@ -163,7 +182,33 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
Name: updatedQuayRegistry.Spec.ConfigBundleSecret,
Namespace: quayRegistry.GetNamespace()},
&configBundleSecret)).
Should(Succeed())
To(Succeed())
})

It("should generate a self-signed TLS cert/key pair in a new `Secret`", func() {
// Reconcile again to get past defaulting step
result, err = controller.Reconcile(context.Background(), reconcile.Request{NamespacedName: quayRegistryName})

var secrets corev1.SecretList
listOptions := client.ListOptions{
Namespace: namespace,
LabelSelector: labels.SelectorFromSet(map[string]string{
kustomize.QuayRegistryNameLabel: quayRegistryName.Name,
})}

Expect(k8sClient.List(context.Background(), &secrets, &listOptions)).To(Succeed())

found := false
for _, secret := range secrets.Items {
if v1.IsManagedTLSSecretFor(quayRegistry, &secret) {
found = true

Expect(secret.Data).To(HaveKey("ssl.cert"))
Expect(secret.Data).To(HaveKey("ssl.key"))
}
}

Expect(found).To(BeTrue())
})
})

Expand Down Expand Up @@ -211,7 +256,7 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
When("it references a `configBundleSecret` that does exist", func() {
BeforeEach(func() {
quayRegistry = newQuayRegistry("test-registry", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace, true)
quayRegistry.Spec.ConfigBundleSecret = configBundle.GetName()
quayRegistryName = types.NamespacedName{
Name: quayRegistry.Name,
Expand Down Expand Up @@ -262,12 +307,37 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
return updatedQuayRegistry.Status.CurrentVersion
}, time.Second*30).Should(Equal(v1.QuayVersionCurrent))
})

It("should copy the provided TLS cert/key pair into a new `Secret`", func() {
var secrets corev1.SecretList
listOptions := client.ListOptions{
Namespace: namespace,
LabelSelector: labels.SelectorFromSet(map[string]string{
kustomize.QuayRegistryNameLabel: quayRegistryName.Name,
})}

Expect(k8sClient.List(context.Background(), &secrets, &listOptions)).To(Succeed())

found := false
for _, secret := range secrets.Items {
if v1.IsManagedTLSSecretFor(quayRegistry, &secret) {
found = true

Expect(secret.Data).To(HaveKey("ssl.cert"))
Expect(secret.Data["ssl.cert"]).To(Equal(configBundle.Data["ssl.cert"]))
Expect(secret.Data).To(HaveKey("ssl.key"))
Expect(secret.Data["ssl.key"]).To(Equal(configBundle.Data["ssl.key"]))
}
}

Expect(found).To(BeTrue())
})
})

When("the current version in the `status` block is the same as the Operator", func() {
BeforeEach(func() {
quayRegistry = newQuayRegistry("test-registry", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace, true)
quayRegistry.Spec.ConfigBundleSecret = configBundle.GetName()
quayRegistryName = types.NamespacedName{
Name: quayRegistry.Name,
Expand Down Expand Up @@ -297,7 +367,7 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
When("the current version in the `status` block is upgradable", func() {
BeforeEach(func() {
quayRegistry = newQuayRegistry("test-registry", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace, true)
quayRegistry.Spec.ConfigBundleSecret = configBundle.GetName()
quayRegistryName = types.NamespacedName{
Name: quayRegistry.Name,
Expand Down Expand Up @@ -361,7 +431,7 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
Name: quayRegistry.Name,
Namespace: quayRegistry.Namespace,
}
configBundle = newConfigBundle("quay-config-secret-abc123", namespace)
configBundle = newConfigBundle("quay-config-secret-abc123", namespace, true)
quayRegistry.Spec.ConfigBundleSecret = configBundle.GetName()
quayRegistry.Spec.Components = nil

Expand Down
Loading

0 comments on commit 9507d22

Please sign in to comment.