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

feat(tls): generate client certificates for agents #938

Merged
merged 2 commits into from
Sep 10, 2024
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
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,9 @@ undeploy_bundle: operator-sdk ## Undeploy the controller in the bundle format wi

.PHONY: create_cryostat_cr
create_cryostat_cr: destroy_cryostat_cr ## Create a namespaced Cryostat instance.
$(CLUSTER_CLIENT) create -f config/samples/operator_v1beta2_cryostat.yaml
target_ns_json=$$(jq -nc '$$ARGS.positional' --args -- $(TARGET_NAMESPACES)) && \
$(CLUSTER_CLIENT) patch -f config/samples/operator_v1beta2_cryostat.yaml --local=true --type=merge \
-p "{\"spec\": {\"targetNamespaces\": $$target_ns_json}}" -o yaml | oc apply -f -

.PHONY: destroy_cryostat_cr
destroy_cryostat_cr: ## Delete a namespaced Cryostat instance.
Expand Down
171 changes: 136 additions & 35 deletions internal/controllers/certmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"errors"
"fmt"
"strings"

certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
"github.com/cryostatio/cryostat-operator/internal/controllers/common"
Expand Down Expand Up @@ -89,25 +90,26 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
if err != nil {
return nil, err
}
tlsConfig := &resources.TLSConfig{
CryostatSecret: cryostatCert.Spec.SecretName,
ReportsSecret: reportsCert.Spec.SecretName,
KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name,
}

// List of certificates whose secrets should be owned by this CR
certificates := []*certv1.Certificate{caCert, cryostatCert, reportsCert}

// Update owner references of TLS secrets created by cert-manager to ensure proper cleanup
err = r.setCertSecretOwner(ctx, cr.Object, certificates...)
// Get the Cryostat CA certificate bytes from certificate secret
caBytes, err := r.getCertficateBytes(ctx, caCert)
if err != nil {
return nil, err
}

secret, err := r.GetCertificateSecret(ctx, caCert)
if err != nil {
return nil, err
tlsConfig := &resources.TLSConfig{
CryostatSecret: cryostatCert.Spec.SecretName,
ReportsSecret: reportsCert.Spec.SecretName,
KeystorePassSecret: cryostatCert.Spec.Keystores.PKCS12.PasswordSecretRef.Name,
CACert: caBytes,
}
// Copy Cryostat CA secret in each target namespace

agentCertsNotReady := []string{}
for _, ns := range cr.TargetNamespaces {
// Copy Cryostat CA secret in each target namespace
if ns != cr.InstallNamespace {
namespaceSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -116,14 +118,41 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
},
Type: corev1.SecretTypeOpaque,
}
err = r.createOrUpdateCertSecret(ctx, namespaceSecret, secret.Data[corev1.TLSCertKey])
err = r.createOrUpdateCertSecret(ctx, namespaceSecret, caBytes)
if err != nil {
return nil, err
}
}

// Create a certificate for Cryostat agents in each target namespace
agentCert := resources.NewAgentCert(cr, ns, r.gvk)
err := r.reconcileAgentCertificate(ctx, agentCert, cr, ns)
if err != nil {
if err == common.ErrCertNotReady {
// Continue with other namespaces if the cert isn't ready
agentCertsNotReady = append(agentCertsNotReady, agentCert.Name)
} else {
return nil, err
}
}
certificates = append(certificates, agentCert)
}
// Delete any Cryostat CA secrets in target namespaces that are no longer requested

if len(agentCertsNotReady) > 0 {
// One or more agent certificates weren't ready, so log a message and return
r.Log.Info("Not all agent certificates were ready", "not ready", strings.Join(agentCertsNotReady, ", "))
return nil, common.ErrCertNotReady
}

// Update owner references of TLS secrets created by cert-manager to ensure proper cleanup
err = r.setCertSecretOwner(ctx, cr.Object, certificates...)
if err != nil {
return nil, err
}

// Clean up resources from target namespaces that are no longer requested
for _, ns := range toDelete(cr) {
// Delete any Cryostat CA secret copies in removed namespaces
if ns != cr.InstallNamespace {
namespaceSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -136,14 +165,31 @@ func (r *Reconciler) setupTLS(ctx context.Context, cr *model.CryostatInstance) (
return nil, err
}
}
}

// Get the Cryostat CA certificate bytes from certificate secret
caBytes, err := r.getCertficateBytes(ctx, caCert)
if err != nil {
return nil, err
// Delete any agent certificates removed target namespaces
agentCert := resources.NewAgentCert(cr, ns, r.gvk)

// Delete namespace copy
if ns != cr.InstallNamespace {
namespaceAgentSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: agentCert.Spec.SecretName,
Namespace: ns,
},
}
err = r.deleteSecret(ctx, namespaceAgentSecret)
if err != nil {
return nil, err
}
}

// Delete certificate with original secret
err := r.deleteCertWithSecret(ctx, agentCert)
if err != nil {
return nil, err
}
}
tlsConfig.CACert = caBytes

return tlsConfig, nil
}

Expand All @@ -161,8 +207,22 @@ func (r *Reconciler) finalizeTLS(ctx context.Context, cr *model.CryostatInstance
if err != nil {
return err
}

// Delete any agent certificate secrets in target namespaces
agentCert := resources.NewAgentCert(cr, ns, r.gvk)
namespaceAgentSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: agentCert.Spec.SecretName,
Namespace: ns,
},
}
err = r.deleteSecret(ctx, namespaceAgentSecret)
if err != nil {
return err
}
}
}

return nil
}

Expand Down Expand Up @@ -265,20 +325,7 @@ func (r *Reconciler) deleteCertChain(ctx context.Context, namespace string, caSe
for i, cert := range certs.Items {
// Is the certificate owned by this CR, and not the CA itself?
if metav1.IsControlledBy(&certs.Items[i], owner) && cert.Spec.SecretName != caSecretName {
// Clean up secret referenced by the cert
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cert.Spec.SecretName,
Namespace: cert.Namespace,
},
}

err := r.deleteSecret(ctx, secret)
if err != nil {
return err
}
// Delete the certificate
err = r.deleteCertificate(ctx, &certs.Items[i])
err := r.deleteCertWithSecret(ctx, &certs.Items[i])
if err != nil {
return err
}
Expand All @@ -288,11 +335,65 @@ func (r *Reconciler) deleteCertChain(ctx context.Context, namespace string, caSe
return nil
}

func (r *Reconciler) deleteCertWithSecret(ctx context.Context, cert *certv1.Certificate) error {
// Clean up secret referenced by the cert
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cert.Spec.SecretName,
Namespace: cert.Namespace,
},
}
err := r.deleteSecret(ctx, secret)
if err != nil {
return err
}

// Delete the certificate
err = r.deleteCertificate(ctx, cert)
if err != nil {
return err
}
return nil
}

func (r *Reconciler) reconcileAgentCertificate(ctx context.Context, cert *certv1.Certificate, cr *model.CryostatInstance, namespace string) error {
// Create the Agent certificate in the install namespace
err := r.createOrUpdateCertificate(ctx, cert, cr.Object)
if err != nil {
return err
}

// Fetch the certificate secret and create a copy in the target namespace (if not the install namespace)
if namespace != cr.InstallNamespace {
secret, err := r.GetCertificateSecret(ctx, cert)
if err != nil {
return err
}

targetSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secret.Name,
Namespace: namespace,
},
}
err = r.createOrUpdateSecret(ctx, targetSecret, nil, func() error {
targetSecret.Data = secret.Data
return nil
})
if err != nil {
return err
}
}
return nil
}

func (r *Reconciler) createOrUpdateCertificate(ctx context.Context, cert *certv1.Certificate, owner metav1.Object) error {
certSpec := cert.Spec.DeepCopy()
op, err := controllerutil.CreateOrUpdate(ctx, r.Client, cert, func() error {
if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil {
return err
if owner != nil {
if err := controllerutil.SetControllerReference(owner, cert, r.Scheme); err != nil {
return err
}
}
// Update Certificate spec
cert.Spec = *certSpec
Expand Down
18 changes: 14 additions & 4 deletions internal/controllers/common/common_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

Expand Down Expand Up @@ -69,17 +68,28 @@ func ClusterUniqueName(gvk *schema.GroupVersionKind, name string, namespace stri
return ClusterUniqueNameWithPrefix(gvk, "", name, namespace)
}

// ClusterUniqueName returns a name for cluster-scoped objects that is
// ClusterUniqueNameWithPrefix returns a name for cluster-scoped objects that is
// uniquely identified by a namespace and name. Appends the prefix to the
// provided Kind.
func ClusterUniqueNameWithPrefix(gvk *schema.GroupVersionKind, prefix string, name string, namespace string) string {
return ClusterUniqueNameWithPrefixTargetNS(gvk, prefix, name, namespace, "")
}

// ClusterUniqueNameWithPrefixTargetNS returns a name for cluster-scoped objects that is
// uniquely identified by a namespace and name, and a target namespace.
// Appends the prefix to the provided Kind.
func ClusterUniqueNameWithPrefixTargetNS(gvk *schema.GroupVersionKind, prefix string, name string, namespace string,
targetNS string) string {
prefixWithKind := strings.ToLower(gvk.Kind)
if len(prefix) > 0 {
prefixWithKind += "-" + prefix
}
toHash := namespace + "/" + name
if len(targetNS) > 0 {
toHash += "/" + targetNS
}
// Use the SHA256 checksum of the namespaced name as a suffix
nn := types.NamespacedName{Namespace: namespace, Name: name}
suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(nn.String())))
suffix := fmt.Sprintf("%x", sha256.Sum256([]byte(toHash)))
return prefixWithKind + "-" + suffix
}

Expand Down
24 changes: 24 additions & 0 deletions internal/controllers/common/resource_definitions/certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,27 @@ func NewReportsCert(cr *model.CryostatInstance) *certv1.Certificate {
},
}
}

func NewAgentCert(cr *model.CryostatInstance, namespace string, gvk *schema.GroupVersionKind) *certv1.Certificate {
name := common.ClusterUniqueNameWithPrefixTargetNS(gvk, "agent", cr.Name, cr.InstallNamespace, namespace)
return &certv1.Certificate{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: cr.InstallNamespace,
},
Spec: certv1.CertificateSpec{
CommonName: fmt.Sprintf("*.%s.pod", namespace),
DNSNames: []string{
fmt.Sprintf("*.%s.pod", namespace),
},
SecretName: name,
IssuerRef: certMeta.ObjectReference{
Name: cr.Name + "-ca",
},
Usages: append(certv1.DefaultKeyUsages(),
certv1.UsageServerAuth,
certv1.UsageClientAuth,
),
},
}
}
10 changes: 6 additions & 4 deletions internal/controllers/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,12 @@ func (r *Reconciler) reconcileCryostat(ctx context.Context, cr *model.CryostatIn
return reconcile.Result{}, err
}

// Finalizer for CA Cert secrets
err = r.finalizeTLS(ctx, cr)
if err != nil {
return reconcile.Result{}, err
// Finalizer for certificates and associated secrets
if r.IsCertManagerEnabled(cr) {
err = r.finalizeTLS(ctx, cr)
if err != nil {
return reconcile.Result{}, err
}
}

err = common.RemoveFinalizer(ctx, r.Client, cr.Object, cryostatFinalizer)
Expand Down
Loading
Loading