Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: persist generated TLS cert/key pair (PROJQUAY-1838) #453

Merged
merged 1 commit into from
May 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion apis/quay/v1/quayregistry_types.go
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
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored all the "feature detection" functions to accept the full config bundle so that we can check ssl.cert/ssl.key, not just config.yaml.

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
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
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