Skip to content

Commit

Permalink
Avoid eventual consistency reading certificates, read first from Rpaa…
Browse files Browse the repository at this point in the history 
…sinstance CRD, after that, read from the near real-time resource: nginx CRD
  • Loading branch information
@wpjunior
wpjunior committed Sep 4, 2024
1 parent ba19ffa commit 783cdfc
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 109 deletions.
10 changes: 10 additions & 0 deletions api/v1alpha1/rpaasinstance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
package v1alpha1

import (
"fmt"
"strings"

kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
nginxv1alpha1 "github.com/tsuru/nginx-operator/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -175,6 +178,13 @@ type CertManager struct {
DNSNamesDefault bool `json:"dnsNamesDefault,omitempty"`
}

func (r *CertManager) RequiredName() string {
if r.Name != "" {
return r.Name
}
return fmt.Sprintf("cert-manager-%s", strings.ToLower(strings.ReplaceAll(r.Issuer, ".", "-")))
}

type AllowedUpstream struct {
Host string `json:"host,omitempty"`
Port int `json:"port,omitempty"`
Expand Down
13 changes: 3 additions & 10 deletions internal/controllers/certificates/cert_manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func newCertificate(instance *v1alpha1.RpaasInstance, issuer *cmmeta.ObjectRefer
labels[k] = v
}

labels["rpaas.extensions.tsuru.io/certificate-name"] = cmCertificateName(req)
labels["rpaas.extensions.tsuru.io/certificate-name"] = req.RequiredName()
labels["rpaas.extensions.tsuru.io/instance-name"] = instance.Name

return &cmv1.Certificate{
Expand All @@ -223,7 +223,7 @@ func newCertificate(instance *v1alpha1.RpaasInstance, issuer *cmmeta.ObjectRefer
SecretName: CertManagerCertificateNameForInstance(instance.Name, req),
SecretTemplate: &cmv1.CertificateSecretTemplate{
Labels: map[string]string{
"rpaas.extensions.tsuru.io/certificate-name": cmCertificateName(req),
"rpaas.extensions.tsuru.io/certificate-name": req.RequiredName(),
"rpaas.extensions.tsuru.io/instance-name": instance.Name,
},
},
Expand Down Expand Up @@ -321,13 +321,6 @@ func isCertificateReady(cert *cmv1.Certificate) bool {
return false
}

func cmCertificateName(r v1alpha1.CertManager) string {
if r.Name != "" {
return r.Name
}
return fmt.Sprintf("%s-%s", CertManagerCertificateName, strings.ToLower(strings.ReplaceAll(r.Issuer, ".", "-")))
}

func CertManagerCertificateNameForInstance(instanceName string, r v1alpha1.CertManager) string {
return fmt.Sprintf("%s-%s", instanceName, cmCertificateName(r))
return fmt.Sprintf("%s-%s", instanceName, r.RequiredName())
}
6 changes: 5 additions & 1 deletion internal/controllers/certificates/cert_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,11 +715,15 @@ func Test_CertManagerCertificateName(t *testing.T) {
request: v1alpha1.CertManager{Issuer: "my-custom.ClusterIssuer.example.com"},
expected: "cert-manager-my-custom-clusterissuer-example-com",
},
{
request: v1alpha1.CertManager{Issuer: "my-custom.ClusterIssuer.example.com", Name: "cert01"},
expected: "cert01",
},
}

for _, tt := range tests {
t.Run(fmt.Sprintf("%v == %q", tt.request, tt.expected), func(t *testing.T) {
got := cmCertificateName(tt.request)
got := tt.request.RequiredName()
assert.Equal(t, tt.expected, got)
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/rpaas/fake/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var _ rpaas.RpaasManager = &RpaasManager{}

type RpaasManager struct {
FakeUpdateCertificate func(instance, name string, cert tls.Certificate) error
FakeGetCertificates func(instanceName string) ([]rpaas.CertificateData, []clientTypes.Event, error)
FakeGetCertificates func(instanceName string) ([]clientTypes.CertificateInfo, []clientTypes.Event, error)
FakeDeleteCertificate func(instance, name string) error
FakeCreateInstance func(args rpaas.CreateArgs) error
FakeDeleteInstance func(instanceName string) error
Expand Down Expand Up @@ -81,7 +81,7 @@ func (m *RpaasManager) GetInstanceInfo(ctx context.Context, instanceName string)
return nil, nil
}

func (m *RpaasManager) GetCertificates(ctx context.Context, instanceName string) ([]rpaas.CertificateData, []clientTypes.Event, error) {
func (m *RpaasManager) GetCertificates(ctx context.Context, instanceName string) ([]clientTypes.CertificateInfo, []clientTypes.Event, error) {
if m.FakeGetCertificates != nil {
return m.FakeGetCertificates(instanceName)
}
Expand Down
122 changes: 68 additions & 54 deletions internal/pkg/rpaas/k8s.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,49 +636,84 @@ func (m *k8sRpaasManager) Stop(ctx context.Context, instanceName string) error {
return m.patchInstance(ctx, originalInstance, instance)
}

func (m *k8sRpaasManager) GetCertificates(ctx context.Context, instanceName string) ([]CertificateData, []clientTypes.Event, error) {
func (m *k8sRpaasManager) GetCertificates(ctx context.Context, instanceName string) ([]clientTypes.CertificateInfo, []clientTypes.Event, error) {
instance, err := m.GetInstance(ctx, instanceName)
if err != nil {
return nil, nil, err
}

secretList := &corev1.SecretList{}
if err = m.cli.List(ctx, secretList, &client.ListOptions{
Namespace: instance.Namespace,
LabelSelector: labels.SelectorFromSet(map[string]string{
"rpaas.extensions.tsuru.io/instance-name": instance.Name,
}),
}); err != nil {
return nil, nil, err
}
secretMap := make(map[string]corev1.Secret)
for _, secret := range secretList.Items {
secretMap[secret.Name] = secret
}

nginx, err := m.getNginx(ctx, instance)
if err != nil && k8sErrors.IsNotFound(err) {
return nil, nil, nil
if err != nil {
if !k8sErrors.IsNotFound(err) {
return nil, nil, err
}
nginx = &nginxv1alpha1.Nginx{}
}

var certList []CertificateData
for _, tls := range nginx.Spec.TLS {
var s corev1.Secret
if err = m.cli.Get(ctx, types.NamespacedName{Name: tls.SecretName, Namespace: instance.Namespace}, &s); err != nil {
var certMap = make(map[string]clientTypes.CertificateInfo)
allTLS := append([]nginxv1alpha1.NginxTLS{}, instance.Spec.TLS...)
allTLS = append(allTLS, nginx.Spec.TLS...)

for _, tls := range allTLS {
s, ok := secretMap[tls.SecretName]
if !ok {
continue
}

x509Certs, err := pki.DecodeX509CertificateChainBytes(s.Data[corev1.TLSCertKey])
if err != nil {
return nil, nil, err
}

certData := CertificateData{
Name: s.Labels[certificates.CertificateNameLabel],
Certificate: string(s.Data[corev1.TLSCertKey]),
Key: string(s.Data[corev1.TLSPrivateKeyKey]),
if len(x509Certs) == 0 {
return nil, nil, fmt.Errorf("no certificates found in pem file")
}

leaf := x509Certs[0]

certInfo := clientTypes.CertificateInfo{
Name: s.Labels[certificates.CertificateNameLabel],
DNSNames: leaf.DNSNames,
ValidFrom: leaf.NotBefore,
ValidUntil: leaf.NotAfter,
PublicKeyAlgorithm: leaf.PublicKeyAlgorithm.String(),
PublicKeyBitSize: publicKeySize(leaf.PublicKey),
}

issuerGroup := s.Annotations["cert-manager.io/issuer-group"]
if issuerGroup == "cert-manager.io" {
certData.CertManagerIssuer = s.Annotations["cert-manager.io/issuer-name"]
certData.IsManagedByCertManager = true
certInfo.CertManagerIssuer = s.Annotations["cert-manager.io/issuer-name"]
certInfo.IsManagedByCertManager = true
} else if issuerGroup != "" {
certData.IsManagedByCertManager = true
certData.CertManagerIssuer = fmt.Sprintf("%s.%s.%s",
certInfo.IsManagedByCertManager = true
certInfo.CertManagerIssuer = fmt.Sprintf("%s.%s.%s",
s.Annotations["cert-manager.io/issuer-name"],
s.Annotations["cert-manager.io/issuer-kind"],
issuerGroup,
)
}

certList = append(certList, certData)
certMap[certInfo.Name] = certInfo
}

events := make([]clientTypes.Event, 0)

for _, certManagerRequest := range instance.CertManagerRequests() {
certName := certManagerRequest.RequiredName()
certManagerCertificateName := certificates.CertManagerCertificateNameForInstance(instance.Name, certManagerRequest)
certificateEvents, err := m.eventsForObjectName(ctx, instance.Namespace, "Certificate", certManagerCertificateName)
if err != nil {
Expand All @@ -692,16 +727,30 @@ func (m *k8sRpaasManager) GetCertificates(ctx context.Context, instanceName stri
Count: evt.Count,
Type: evt.Type,
Reason: evt.Reason,
Message: fmt.Sprintf("certificate %q, %s", certManagerCertificateName, evt.Message),
Message: fmt.Sprintf("certificate %q, %s", certName, evt.Message),
})
}

if _, ok := certMap[certName]; !ok {
certMap[certName] = clientTypes.CertificateInfo{
Name: certName,
DNSNames: certManagerRequest.DNSNames,
IsManagedByCertManager: true,
CertManagerIssuer: certManagerRequest.Issuer,
}
}

}

sort.SliceStable(events, func(i, j int) bool {
return events[i].Last.Before(events[j].Last) // ascending order by last event occurrence
})

var certList []clientTypes.CertificateInfo
for _, cert := range certMap {
certList = append(certList, cert)
}

sort.Slice(certList, func(i, j int) bool {
return certList[i].Name < certList[j].Name
})
Expand Down Expand Up @@ -749,7 +798,7 @@ func (m *k8sRpaasManager) UpdateCertificate(ctx context.Context, instanceName, n
return err
}

certsInfo, _, err := m.getCertificatesInfo(ctx, instance)
certsInfo, _, err := m.GetCertificates(ctx, instance.Name)
if err != nil {
return err
}
Expand Down Expand Up @@ -1673,7 +1722,7 @@ func (m *k8sRpaasManager) GetInstanceInfo(ctx context.Context, instanceName stri
}

var certificateEvents []clientTypes.Event
info.Certificates, certificateEvents, err = m.getCertificatesInfo(ctx, instance)
info.Certificates, certificateEvents, err = m.GetCertificates(ctx, instance.Name)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2106,41 +2155,6 @@ func (m *k8sRpaasManager) newPodStatus(ctx context.Context, pod *corev1.Pod) (cl
}, nil
}

func (m *k8sRpaasManager) getCertificatesInfo(ctx context.Context, instance *v1alpha1.RpaasInstance) ([]clientTypes.CertificateInfo, []clientTypes.Event, error) {
certs, events, err := m.GetCertificates(ctx, instance.Name)
if err != nil {
return nil, nil, err
}

var certsInfo []clientTypes.CertificateInfo
for _, cert := range certs {
certs, err := pki.DecodeX509CertificateChainBytes([]byte(cert.Certificate))
if err != nil {
return nil, nil, err
}

if len(certs) == 0 {
return nil, nil, fmt.Errorf("no certificates found in pem file")
}

leaf := certs[0]

certsInfo = append(certsInfo, clientTypes.CertificateInfo{
Name: cert.Name,
DNSNames: leaf.DNSNames,
ValidFrom: leaf.NotBefore,
ValidUntil: leaf.NotAfter,
PublicKeyAlgorithm: leaf.PublicKeyAlgorithm.String(),
PublicKeyBitSize: publicKeySize(leaf.PublicKey),

IsManagedByCertManager: cert.IsManagedByCertManager,
CertManagerIssuer: cert.CertManagerIssuer,
})
}

return certsInfo, events, nil
}

func getPortsForPod(pod *corev1.Pod) []clientTypes.PodPort {
var ports []clientTypes.PodPort
for _, container := range pod.Spec.Containers {
Expand Down
47 changes: 15 additions & 32 deletions internal/pkg/rpaas/k8s_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ func Test_k8sRpaasManager_GetCertificates(t *testing.T) {
Name: "my-instance-certs-abc123",
Namespace: "rpaasv2",
Labels: map[string]string{
certificates.CertificateNameLabel: "default",
certificates.CertificateNameLabel: "default",
"rpaas.extensions.tsuru.io/instance-name": "my-instance-1",
},
},
Data: map[string][]byte{
Expand Down Expand Up @@ -428,11 +429,14 @@ sM5FaDCEIJVbWjPDluxUGbVOQlFHsJs+pZv0Anf9DPwU
},
}

notBefore, _ := time.Parse("Jan 2 15:04:05 2006 MST", "Mar 26 20:21:39 2019 GMT")
notAfter, _ := time.Parse("Jan 2 15:04:05 2006 MST", "Mar 25 20:21:39 2020 GMT")

tests := map[string]struct {
name string
instanceName string
expectedError string
expected []CertificateData
expected []clientTypes.CertificateInfo
}{
"instance not found": {
instanceName: "instance-not-found",
Expand All @@ -441,37 +445,16 @@ sM5FaDCEIJVbWjPDluxUGbVOQlFHsJs+pZv0Anf9DPwU

"getting an existing certificate": {
instanceName: "my-instance-1",
expected: []CertificateData{
expected: []clientTypes.CertificateInfo{
{
Name: "default",
Certificate: `-----BEGIN CERTIFICATE-----
MIIB9TCCAV6gAwIBAgIRAIpoagB8BUn8x36iyvafmC0wDQYJKoZIhvcNAQELBQAw
EjEQMA4GA1UEChMHQWNtZSBDbzAeFw0xOTAzMjYyMDIxMzlaFw0yMDAzMjUyMDIx
MzlaMBIxEDAOBgNVBAoTB0FjbWUgQ28wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
AoGBAOIsM9LhHqI3oBhHDCGZkGKgiI72ghnLr5UpaA3I9U7np/LPzt/JpWRG4wjF
5Var2IRPGoNwLcdybFW0YTqvw1wNY88q9BcpwS5PeV7uWyZqWafdSxxveaG6VeCH
YFMqopOKri4kJ4sZB9WS3xMlGZXK6zHPwA4xPtuVEND+LI17AgMBAAGjSzBJMA4G
A1UdDwEB/wQEAwIFoDATBgNVHSUEDDAKBggrBgEFBQcDATAMBgNVHRMBAf8EAjAA
MBQGA1UdEQQNMAuCCWxvY2FsaG9zdDANBgkqhkiG9w0BAQsFAAOBgQCaF9zDYoPh
4KmqxFI3KB+cl8Z/0y0txxH4vqlnByBBiCLpPzivcCRFlT1bGPVJOLsyd/BdOset
yTcvMUPbnEPXZMR4Dsbzzjco1JxMSvZgkhm85gAlwNGjFZrMXqO8G5R/gpWN3UUc
7likRQOu7q61DlicQAZXRnOh6BbKaq1clg==
-----END CERTIFICATE-----`,
Key: `-----BEGIN RSA PRIVATE KEY-----
MIICXQIBAAKBgQDiLDPS4R6iN6AYRwwhmZBioIiO9oIZy6+VKWgNyPVO56fyz87f
yaVkRuMIxeVWq9iETxqDcC3HcmxVtGE6r8NcDWPPKvQXKcEuT3le7lsmalmn3Usc
b3mhulXgh2BTKqKTiq4uJCeLGQfVkt8TJRmVyusxz8AOMT7blRDQ/iyNewIDAQAB
AoGBAI05gJqayyALj8HZCzAnzUpoZxytvAsTbm27TyfcZaCBchNhwxFlvgphYP5n
Y468+xOSuUF9WHiDcDYLzfJxMZAqmuS+D/IREYDkcrGVT1MXfSCkNaFVqG52+hLZ
GmGsy8+KsJnDJ1HYmwfSnaTj3L8+Bf2Hg291Yb1caRH9+5vBAkEA7P5N3cSN73Fa
HwaWzqkaY75mCR4TpRi27YWGA3wdQek2G71HiSbCOxrWOymvgoNRi6M/sdrP5PTt
JAFxC+pd8QJBAPRPvS0Tm/0lMIZ0q7jxyoW/gKDzokmSszopdlvSU53lN06vaYdK
XyTvqOO95nJx0DjkdM26QojJlSueMTitJisCQDuxNfWku0dTGqrz4uo8p5v16gdj
3vjXh8O9vOqFyWy/i9Ri0XDXJVbzxH/0WPObld+BB9sJTRHTKyPFhS7GIlECQDZ8
chxTez6BxMi3zHR6uEgL5Yv/yfnOldoq1RK1XaChNix+QnLBy2ZZbLkd6P8tEtsd
WE9pct0+193ace/J7fECQQDAhwHBpJjhM+k97D92akneKXIUBo+Egr5E5qF9/g5I
sM5FaDCEIJVbWjPDluxUGbVOQlFHsJs+pZv0Anf9DPwU
-----END RSA PRIVATE KEY-----`,
Name: "default",
PublicKeyAlgorithm: "RSA",
PublicKeyBitSize: 1024,
ValidFrom: notBefore.UTC(),
ValidUntil: notAfter.UTC(),
DNSNames: []string{
"localhost",
},
},
},
},
Expand Down
11 changes: 1 addition & 10 deletions internal/pkg/rpaas/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ type RpaasManager interface {

UpdateCertificate(ctx context.Context, instance, name string, cert tls.Certificate) error
DeleteCertificate(ctx context.Context, instance, name string) error
GetCertificates(ctx context.Context, instanceName string) ([]CertificateData, []clientTypes.Event, error)
GetCertificates(ctx context.Context, instanceName string) ([]clientTypes.CertificateInfo, []clientTypes.Event, error)
CreateInstance(ctx context.Context, args CreateArgs) error
DeleteInstance(ctx context.Context, name string) error
UpdateInstance(ctx context.Context, name string, args UpdateInstanceArgs) error
Expand Down Expand Up @@ -290,15 +290,6 @@ type RpaasManager interface {
UnsetMetadata(ctx context.Context, instanceName string, metadata *clientTypes.Metadata) error
}

type CertificateData struct {
Name string `json:"name"`
Certificate string `json:"certificate"`
Key string `json:"key"`

IsManagedByCertManager bool `json:"isManagedByCertManager,omitempty"`
CertManagerIssuer string `json:"certManagerIssuer,omitempty"`
}

func getAnnotations(params map[string]interface{}) (map[string]string, error) {
p, found := params["annotations"]
if !found {
Expand Down

0 comments on commit 783cdfc

Please sign in to comment.