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 <[email protected]>
  • Loading branch information
@alecmerdler
alecmerdler committed May 5, 2021
1 parent e8b5b03 commit cb3a0c0
Show file tree
Hide file tree
Showing 13 changed files with 180 additions and 57 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
35 changes: 33 additions & 2 deletions controllers/quay/quayregistry_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ 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"
"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 Down Expand Up @@ -55,6 +57,7 @@ func newConfigBundle(name, namespace string) corev1.Secret {
},
Data: map[string][]byte{
"config.yaml": encode(config),
// FIXME(alecmerdler): Test providing TLS cert/key pair...
},
}
}
Expand Down Expand Up @@ -139,7 +142,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 +166,7 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
Name: updatedQuayRegistry.Spec.ConfigBundleSecret,
Namespace: quayRegistry.GetNamespace()},
&configBundleSecret)).
Should(Succeed())
To(Succeed())
})
})

Expand Down Expand Up @@ -262,6 +265,34 @@ var _ = Describe("Reconciling a QuayRegistry", func() {
return updatedQuayRegistry.Status.CurrentVersion
}, time.Second*30).Should(Equal(v1.QuayVersionCurrent))
})

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

// Expect(err).NotTo(HaveOccurred())
// Expect(result.Requeue).To(BeFalse())

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())
})
})

When("the current version in the `status` block is the same as the Operator", func() {
Expand Down
12 changes: 8 additions & 4 deletions kustomize/base/quay.deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@ spec:
spec:
serviceAccountName: quay-app
volumes:
- name: configvolume
secret:
secretName: quay-config-secret
- name: config
projected:
sources:
- secret:
name: quay-config-secret
- secret:
name: quay-config-tls
- name: extra-ca-certs
configMap:
name: cluster-service-ca
Expand Down Expand Up @@ -76,7 +80,7 @@ spec:
port: 8443
scheme: HTTPS
volumeMounts:
- name: configvolume
- name: config
readOnly: false
mountPath: /conf/stack
- name: extra-ca-certs
Expand Down
13 changes: 8 additions & 5 deletions kustomize/base/upgrade.deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ spec:
spec:
serviceAccountName: quay-app
volumes:
- name: configvolume
secret:
secretName: quay-config-secret
- name: config
projected:
sources:
- secret:
name: quay-config-secret
- secret:
name: quay-config-tls
- name: extra-ca-certs
configMap:
name: cluster-service-ca
Expand Down Expand Up @@ -76,10 +80,9 @@ spec:
port: 8443
scheme: HTTPS
volumeMounts:
- name: configvolume
- name: config
readOnly: false
mountPath: /conf/stack
- name: extra-ca-certs
readOnly: true
# FIXME: Cannot mount here because Quay attempts to write to this directory...
mountPath: /conf/stack/extra_ca_certs
2 changes: 1 addition & 1 deletion kustomize/components/clair/clair.deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ spec:
secretName: clair-config-secret
- name: certs
secret:
secretName: quay-config-secret
secretName: quay-config-tls
# Mount just the public certificate because we are using storage proxying.
items:
- key: ssl.cert
Expand Down
13 changes: 9 additions & 4 deletions kustomize/components/mirror/mirror.deployment.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,19 @@ spec:
spec:
serviceAccountName: quay-app
volumes:
- name: configvolume
secret:
secretName: quay-config-secret
- name: config
projected:
sources:
- secret:
name: quay-config-secret
- secret:
name: quay-config-tls
- name: extra-ca-certs
projected:
sources:
- configMap:
name: cluster-service-ca
# FIXME(alecmerdler): Have this read from `quay-config-tls` instead...
- secret:
name: quay-config-secret
items:
Expand Down Expand Up @@ -78,7 +83,7 @@ spec:
port: 8443
scheme: HTTPS
volumeMounts:
- name: configvolume
- name: config
readOnly: false
mountPath: /conf/stack
- name: extra-ca-certs
Expand Down
4 changes: 4 additions & 0 deletions pkg/context/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ type QuayRegistryContext struct {
ServerHostname string
BuildManagerHostname string

// TLS
TLSCert []byte
TLSKey []byte

// Object Storage
SupportsObjectStorage bool
ObjectStorageInitialized bool
Expand Down
Loading

0 comments on commit cb3a0c0

Please sign in to comment.