Skip to content

Commit

Permalink
refact: move pod auto-restart by certificate changes to operator resp…
Browse files Browse the repository at this point in the history 
…onsability
  • Loading branch information
@wpjunior
wpjunior committed Aug 19, 2024
1 parent 300c285 commit 6610bd5
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 128 deletions.
140 changes: 112 additions & 28 deletions controllers/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"strings"
"text/template"

cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
"github.com/imdario/mergo"
kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
"github.com/sirupsen/logrus"
Expand All @@ -42,6 +43,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/tsuru/rpaas-operator/api/v1alpha1"
"github.com/tsuru/rpaas-operator/internal/controllers/certificates"
controllerUtil "github.com/tsuru/rpaas-operator/internal/controllers/util"
"github.com/tsuru/rpaas-operator/internal/pkg/rpaas/nginx"
"github.com/tsuru/rpaas-operator/pkg/util"
Expand Down Expand Up @@ -1088,32 +1090,40 @@ func mergeServiceWithDNS(instance *v1alpha1.RpaasInstance) *nginxv1alpha1.NginxS
return s
}

func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1.RpaasPlan, configMap *corev1.ConfigMap) *nginxv1alpha1.Nginx {
type newNginxOptions struct {
instanceMergedWithFlavors *v1alpha1.RpaasInstance
plan *v1alpha1.RpaasPlan
configMap *corev1.ConfigMap
certManagerCertificates []cmv1.Certificate
certificateSecrets []corev1.Secret
}

func newNginx(opts newNginxOptions) *nginxv1alpha1.Nginx {
var cacheConfig nginxv1alpha1.NginxCacheSpec
if v1alpha1.BoolValue(plan.Spec.Config.CacheEnabled) {
cacheConfig.Path = plan.Spec.Config.CachePath
if v1alpha1.BoolValue(opts.plan.Spec.Config.CacheEnabled) {
cacheConfig.Path = opts.plan.Spec.Config.CachePath
cacheConfig.InMemory = true
if plan.Spec.Config.CacheSize != nil && !plan.Spec.Config.CacheSize.IsZero() {
cacheConfig.Size = plan.Spec.Config.CacheSize
if opts.plan.Spec.Config.CacheSize != nil && !opts.plan.Spec.Config.CacheSize.IsZero() {
cacheConfig.Size = opts.plan.Spec.Config.CacheSize
}
}

instanceMergedWithFlavors.Spec.Service = mergeServiceWithDNS(instanceMergedWithFlavors)
opts.instanceMergedWithFlavors.Spec.Service = mergeServiceWithDNS(opts.instanceMergedWithFlavors)

if s := instanceMergedWithFlavors.Spec.Service; s != nil {
s.Labels = instanceMergedWithFlavors.GetBaseLabels(s.Labels)
if s := opts.instanceMergedWithFlavors.Spec.Service; s != nil {
s.Labels = opts.instanceMergedWithFlavors.GetBaseLabels(s.Labels)
}

if ing := instanceMergedWithFlavors.Spec.Ingress; ing != nil {
ing.Labels = instanceMergedWithFlavors.GetBaseLabels(ing.Labels)
if ing := opts.instanceMergedWithFlavors.Spec.Ingress; ing != nil {
ing.Labels = opts.instanceMergedWithFlavors.GetBaseLabels(ing.Labels)
}

replicas := instanceMergedWithFlavors.Spec.Replicas
if shutdown := instanceMergedWithFlavors.Spec.Shutdown; shutdown {
replicas := opts.instanceMergedWithFlavors.Spec.Replicas
if shutdown := opts.instanceMergedWithFlavors.Spec.Shutdown; shutdown {
replicas = ptr.To(int32(0))
}

if isAutoscaleEnabled(&instanceMergedWithFlavors.Spec) {
if isAutoscaleEnabled(&opts.instanceMergedWithFlavors.Spec) {
// NOTE: we should avoid changing the number of replicas as it's managed by HPA.
replicas = nil
}
Expand All @@ -1124,40 +1134,39 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1.
APIVersion: "nginx.tsuru.io/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: instanceMergedWithFlavors.Name,
Namespace: instanceMergedWithFlavors.Namespace,
Name: opts.instanceMergedWithFlavors.Name,
Namespace: opts.instanceMergedWithFlavors.Namespace,
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(instanceMergedWithFlavors, schema.GroupVersionKind{
*metav1.NewControllerRef(opts.instanceMergedWithFlavors, schema.GroupVersionKind{
Group: v1alpha1.GroupVersion.Group,
Version: v1alpha1.GroupVersion.Version,
Kind: "RpaasInstance",
}),
},
Labels: instanceMergedWithFlavors.GetBaseLabels(nil),
Labels: opts.instanceMergedWithFlavors.GetBaseLabels(nil),
},
Spec: nginxv1alpha1.NginxSpec{
Image: plan.Spec.Image,
Image: opts.plan.Spec.Image,
Replicas: replicas,
Config: &nginxv1alpha1.ConfigRef{
Name: configMap.Name,
Name: opts.configMap.Name,
Kind: nginxv1alpha1.ConfigKindConfigMap,
},
Resources: plan.Spec.Resources,
Service: instanceMergedWithFlavors.Spec.Service.DeepCopy(),
Resources: opts.plan.Spec.Resources,
Service: opts.instanceMergedWithFlavors.Spec.Service.DeepCopy(),
HealthcheckPath: "/_nginx_healthcheck",
TLS: instanceMergedWithFlavors.Spec.TLS,
Cache: cacheConfig,
PodTemplate: instanceMergedWithFlavors.Spec.PodTemplate,
Lifecycle: instanceMergedWithFlavors.Spec.Lifecycle,
Ingress: instanceMergedWithFlavors.Spec.Ingress,
PodTemplate: opts.instanceMergedWithFlavors.Spec.PodTemplate,
Lifecycle: opts.instanceMergedWithFlavors.Spec.Lifecycle,
Ingress: opts.instanceMergedWithFlavors.Spec.Ingress,
},
}

if n.Spec.Service != nil && n.Spec.Service.Type == "" {
n.Spec.Service.Type = corev1.ServiceTypeLoadBalancer
}

for i, f := range instanceMergedWithFlavors.Spec.Files {
for i, f := range opts.instanceMergedWithFlavors.Spec.Files {
volumeName := fmt.Sprintf("extra-files-%d", i)

n.Spec.PodTemplate.Volumes = append(n.Spec.PodTemplate.Volumes, corev1.Volume{
Expand All @@ -1177,12 +1186,12 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1.
})
}

if isTLSSessionTicketEnabled(&instanceMergedWithFlavors.Spec) {
if isTLSSessionTicketEnabled(&opts.instanceMergedWithFlavors.Spec) {
n.Spec.PodTemplate.Volumes = append(n.Spec.PodTemplate.Volumes, corev1.Volume{
Name: sessionTicketsVolumeName,
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: secretNameForTLSSessionTickets(instanceMergedWithFlavors.Name),
SecretName: secretNameForTLSSessionTickets(opts.instanceMergedWithFlavors.Name),
},
},
})
Expand All @@ -1194,6 +1203,53 @@ func newNginx(instanceMergedWithFlavors *v1alpha1.RpaasInstance, plan *v1alpha1.
})
}

mapSecretNameToCertName := make(map[string]string)
secretsByName := make(map[string]corev1.Secret)

for _, secret := range opts.certificateSecrets {
secretsByName[secret.Name] = secret
certName := secret.Labels[certificates.CertificateNameLabel]
if certName != "" {
mapSecretNameToCertName[secret.Name] = certName
}
}

tlsByCertName := make(map[string]nginxv1alpha1.NginxTLS)
for _, tls := range opts.instanceMergedWithFlavors.Spec.TLS {
certName := mapSecretNameToCertName[tls.SecretName]
if certName == "" {
continue
}
tlsByCertName[certName] = tls

if secret, ok := secretsByName[tls.SecretName]; ok {
n.Spec.PodTemplate.Annotations[certificateHashAnnotationKey(tls.SecretName)] = util.SHA256(secret.Data[corev1.TLSCertKey])
n.Spec.PodTemplate.Annotations[keyHashAnnotationKey(tls.SecretName)] = util.SHA256(secret.Data[corev1.TLSPrivateKeyKey])
}
}

for _, cert := range opts.certManagerCertificates {
tlsByCertName[cert.Name] = nginxv1alpha1.NginxTLS{
SecretName: cert.Spec.SecretName,
Hosts: cert.Spec.DNSNames,
}

if secret, ok := secretsByName[cert.Spec.SecretName]; ok {
if n.Spec.PodTemplate.Annotations == nil {
n.Spec.PodTemplate.Annotations = make(map[string]string)
}

n.Spec.PodTemplate.Annotations[certificateHashAnnotationKey(secret.Name)] = util.SHA256(secret.Data[corev1.TLSCertKey])
n.Spec.PodTemplate.Annotations[keyHashAnnotationKey(secret.Name)] = util.SHA256(secret.Data[corev1.TLSPrivateKeyKey])
} else {

Check failure on line 1244 in controllers/controller.go

View workflow job for this annotation

GitHub Actions / test 

SA9003: empty branch (staticcheck)
// TODO: use other fields to generate the hash
}
}

for _, tls := range tlsByCertName {
n.Spec.TLS = append(n.Spec.TLS, tls)
}

return n
}

Expand All @@ -1211,6 +1267,34 @@ func generateNginxHash(nginx *nginxv1alpha1.Nginx) (string, error) {
return strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(hash[:])), nil
}

func certificateHashAnnotationKey(secretName string) string {
keyFormat := "rpaas.extensions.tsuru.io/%s-secret-cert-sha256"

// NOTE: Annotation keys must not be greater than 63 chars.
// See more: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
maxIssuer := 63 - len(fmt.Sprintf(keyFormat, ""))

if len(secretName) > maxIssuer {
secretName = secretName[:maxIssuer]
}

return fmt.Sprintf(keyFormat, secretName)
}

func keyHashAnnotationKey(secretName string) string {
keyFormat := "rpaas.extensions.tsuru.io/%s-secret-key-sha256"

// NOTE: Annotation keys must not be greater than 63 chars.
// See more: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set
maxIssuer := 63 - len(fmt.Sprintf(keyFormat, ""))

if len(secretName) > maxIssuer {
secretName = secretName[:maxIssuer]
}

return fmt.Sprintf(keyFormat, secretName)
}

func generateSpecHash(spec any) (string, error) {
if spec == nil {
return "", nil
Expand Down
6 changes: 5 additions & 1 deletion controllers/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ func Test_newNginx(t *testing.T) {
if tt.expected != nil {
nginx = tt.expected(nginx)
}
assert.Equal(t, nginx, newNginx(instance, plan, cm))
assert.Equal(t, nginx, newNginx(newNginxOptions{
instanceMergedWithFlavors: instance,
plan: plan,
configMap: cm,
}))
})
}
}
Expand Down
16 changes: 14 additions & 2 deletions controllers/rpaasinstance_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ func (r *RpaasInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}
}

if err = certificates.ReconcileDynamicCertificates(ctx, r.Client, instance, instanceMergedWithFlavors); err != nil {
certManagerCertificates, err := certificates.ReconcileCertManager(ctx, r.Client, instance, instanceMergedWithFlavors)
if err != nil {
return reconcile.Result{}, err
}

Expand Down Expand Up @@ -173,8 +174,19 @@ func (r *RpaasInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}
}

certificateSecrets, err := certificates.ListCertificateSecrets(ctx, r.Client, instanceMergedWithFlavors)
if err != nil {
return reconcile.Result{}, err
}

// Nginx CRD
nginx := newNginx(instanceMergedWithFlavors, plan, configMap)
nginx := newNginx(newNginxOptions{
instanceMergedWithFlavors: instanceMergedWithFlavors,
plan: plan,
configMap: configMap,
certManagerCertificates: certManagerCertificates,
certificateSecrets: certificateSecrets,
})
changes["nginx"], err = r.reconcileNginx(ctx, instanceMergedWithFlavors, nginx)
if err != nil {
return ctrl.Result{}, err
Expand Down
26 changes: 15 additions & 11 deletions internal/controllers/certificates/cert_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,34 @@ import (

const CertManagerCertificateName string = "cert-manager"

func reconcileCertManager(ctx context.Context, client client.Client, instance, instanceMergedWithFlavors *v1alpha1.RpaasInstance) error {
func ReconcileCertManager(ctx context.Context, client client.Client, instance, instanceMergedWithFlavors *v1alpha1.RpaasInstance) ([]cmv1.Certificate, error) {
err := removeOldCertificates(ctx, client, instance, instanceMergedWithFlavors)
if err != nil {
return err
return nil, err
}

certManagerCerts := []cmv1.Certificate{}

for _, req := range instanceMergedWithFlavors.CertManagerRequests() {
issuer, err := getCertManagerIssuer(ctx, client, req, instanceMergedWithFlavors.Namespace)
if err != nil {
return err
return nil, err
}

newCert, err := newCertificate(instanceMergedWithFlavors, issuer, req)
if err != nil {
return err
return nil, err
}

var cert cmv1.Certificate
err = client.Get(ctx, types.NamespacedName{Name: newCert.Name, Namespace: newCert.Namespace}, &cert)
if err != nil && k8serrors.IsNotFound(err) {
if err = takeOverPreviousSecret(ctx, client, instance, instanceMergedWithFlavors, newCert); err != nil {
return err
if err = takeOverPreviousSecret(ctx, client, instanceMergedWithFlavors, newCert); err != nil {
return nil, err
}

if err = client.Create(ctx, newCert); err != nil {
return err
return nil, err
}

newCert.DeepCopyInto(&cert)
Expand All @@ -61,21 +63,23 @@ func reconcileCertManager(ctx context.Context, client client.Client, instance, i
newCert.ResourceVersion = cert.ResourceVersion

if err = client.Update(ctx, newCert); err != nil {
return err
return nil, err
}
}

certManagerCerts = append(certManagerCerts, cert)

if !isCertificateReady(&cert) {
continue
}

err = UpdateCertificateFromSecret(ctx, client, instance, cmCertificateName(req), newCert.Spec.SecretName)
if err != nil {
return err
return nil, err
}
}

return nil
return certManagerCerts, nil
}

func removeOldCertificates(ctx context.Context, c client.Client, instance, instanceMergedWithFlavors *v1alpha1.RpaasInstance) error {
Expand Down Expand Up @@ -117,7 +121,7 @@ func removeOldCertificates(ctx context.Context, c client.Client, instance, insta
return nil
}

func takeOverPreviousSecret(ctx context.Context, c client.Client, instance, instanceMergedWithFlavors *v1alpha1.RpaasInstance, cert *cmv1.Certificate) error {
func takeOverPreviousSecret(ctx context.Context, c client.Client, instanceMergedWithFlavors *v1alpha1.RpaasInstance, cert *cmv1.Certificate) error {
var secret corev1.Secret

err := c.Get(ctx, types.NamespacedName{Name: cert.Spec.SecretName, Namespace: cert.Namespace}, &secret)
Expand Down
11 changes: 3 additions & 8 deletions internal/controllers/certificates/cert_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,7 @@ wg4cGbIbBPs=
},
},
assert: func(t *testing.T, cli client.Client, instance *v1alpha1.RpaasInstance) {
assert.Equal(t, []nginxv1alpha1.NginxTLS{
{SecretName: "my-instance-3-cert-manager-issuer-1", Hosts: []string{"www.example.com", "www.example.org", "www.example.test"}},
}, instance.Spec.TLS)

assert.Equal(t, "a0610da4d1958cfa7c375870e2c1bac796e84f509bbd989fa5a7c0e040965f28", instance.Spec.PodTemplate.Annotations["rpaas.extensions.tsuru.io/cert-manager-issue-certificate-sha256"])
assert.Equal(t, "e644183deec75208c5fc53b4afb98e471ee290c7e7e10c5b95caff6851346132", instance.Spec.PodTemplate.Annotations["rpaas.extensions.tsuru.io/cert-manager-issuer-1-key-sha256"])
assert.Nil(t, instance.Spec.TLS)

var cert cmv1.Certificate
err := cli.Get(context.TODO(), types.NamespacedName{
Expand All @@ -614,7 +609,7 @@ wg4cGbIbBPs=

var s corev1.Secret
err = cli.Get(context.TODO(), types.NamespacedName{
Name: instance.Spec.TLS[0].SecretName,
Name: cert.Spec.SecretName,
Namespace: instance.Namespace,
}, &s)
require.NoError(t, err)
Expand All @@ -637,7 +632,7 @@ wg4cGbIbBPs=
WithRuntimeObjects(allResources...).
Build()

err := reconcileCertManager(context.TODO(), cli, tt.instance, tt.instance)
_, err := ReconcileCertManager(context.TODO(), cli, tt.instance, tt.instance)

if tt.expectedError != "" {
assert.EqualError(t, err, tt.expectedError)
Expand Down
Loading

0 comments on commit 6610bd5

Please sign in to comment.