Allow multiple instances of cert-manager certificates (#155)

* cmd: add name field using cert-manager to allow multiples instances

* Display info about cert-manager on "rpaas info"

* Add support to delete and update by name

* operator: create multiple cm.Certificate based on name

* Add support to take over classic certificate secret

* Add some business validation on issuers

* refact: move pod auto-restart by certificate changes to operator responsability

* Use SecretTemplate instead of manual label propagation

* Add tests to validate construction of nginx.Spec.TLS

* propagate labels to cmv1.Certificate

* fix order of certificates

* Update CRD

* expose cert-manager events

* Drop unused route

* Avoid eventual consistency reading certificates, read first from Rpaasinstance CRD, after that, read from the near real-time resource: nginx CRD

* Fix lint

* use $nginxTLS to fill the certificates

* Set commonName for each certificate

* Expose an event when restart nginx for certificate reasons

* api: add test to cover multiple certificates with same issuer

* Add issuer option to be strict on certificate names

* plugin: remove cert-manager request by name

* Add support to validation using multiple cert-manager requests

Dockerfile.operator

@@ -1,4 +1,4 @@
-FROM golang:1.21-alpine3.18 AS builder
+FROM golang:1.22-alpine3.18 AS builder
 COPY . /go/src/github.com/tsuru/rpaas-operator
 WORKDIR /go/src/github.com/tsuru/rpaas-operator
 RUN apk add --update gcc git make musl-dev && \

Makefile

@@ -29,7 +29,7 @@ build/api: build-dirs
 
 .PHONY: build/manager
 build/manager: manager
-	
+
 .PHONY: build/plugin/rpaasv2
 build/plugin/rpaasv2: build-dirs
 	go build -o $(GO_BUILD_DIR)/ ./cmd/plugin/rpaasv2
@@ -102,7 +102,7 @@ controller-gen:
 ifeq (, $(shell which controller-gen))
 	@{ \
 	set -e ;\
-	go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.7.0 ;\
+	go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.15.0 ;\
 	}
 CONTROLLER_GEN=$(GOBIN)/controller-gen
 else

api/v1alpha1/rpaasinstance.go

@@ -64,46 +64,75 @@ func (i *RpaasInstance) BelongsToCluster(clusterName string) bool {
 	return clusterName == instanceCluster
 }
 
-func (i *RpaasInstance) CertManagerRequests() (reqs []CertManager) {
-	if i == nil || i.Spec.DynamicCertificates == nil {
+func (s *RpaasInstanceSpec) CertManagerRequests(name string) (reqs []CertManager) {
+	if s == nil || s.DynamicCertificates == nil {
 		return
 	}
 
-	uniqueCerts := make(map[string]*CertManager)
-	if req := i.Spec.DynamicCertificates.CertManager; req != nil {
+	uniqueCertsByIssuer := make(map[string]*CertManager)
+	uniqueCertsByName := make(map[string]*CertManager)
+
+	if req := s.DynamicCertificates.CertManager; req != nil {
 		r := req.DeepCopy()
-		r.DNSNames = r.dnsNames(i)
-		uniqueCerts[r.Issuer] = r
+		r.DNSNames = r.dnsNames(name, s)
+
+		if req.Name != "" {
+			uniqueCertsByName[req.Name] = r
+		} else {
+			uniqueCertsByIssuer[r.Issuer] = r
+		}
 	}
 
-	for _, req := range i.Spec.DynamicCertificates.CertManagerRequests {
-		r, found := uniqueCerts[req.Issuer]
+	for _, req := range s.DynamicCertificates.CertManagerRequests {
+
+		if req.Name != "" {
+			r, found := uniqueCertsByName[req.Name]
+			if found {
+				r.DNSNames = append(r.DNSNames, req.dnsNames(name, s)...)
+				r.IPAddresses = append(r.IPAddresses, req.IPAddresses...)
+			} else {
+				uniqueCertsByName[req.Name] = req.DeepCopy()
+			}
+
+			continue
+		}
+
+		r, found := uniqueCertsByIssuer[req.Issuer]
 		if !found {
-			uniqueCerts[req.Issuer] = req.DeepCopy()
+			uniqueCertsByIssuer[req.Issuer] = req.DeepCopy()
 			continue
 		}
 
-		r.DNSNames = append(r.DNSNames, req.dnsNames(i)...)
+		r.DNSNames = append(r.DNSNames, req.dnsNames(name, s)...)
 		r.IPAddresses = append(r.IPAddresses, req.IPAddresses...)
 	}
 
-	for _, v := range uniqueCerts {
+	for _, v := range uniqueCertsByName {
+		reqs = append(reqs, *v)
+	}
+	for _, v := range uniqueCertsByIssuer {
 		reqs = append(reqs, *v)
 	}
 
-	sort.Slice(reqs, func(i, j int) bool { return reqs[i].Issuer < reqs[j].Issuer })
+	sort.Slice(reqs, func(i, j int) bool {
+		if reqs[i].Name != reqs[j].Name {
+			return reqs[i].Name < reqs[j].Name
+		}
+
+		return reqs[i].Issuer < reqs[j].Issuer
+	})
 
 	return
 }
 
-func (c *CertManager) dnsNames(i *RpaasInstance) (names []string) {
+func (c *CertManager) dnsNames(name string, spec *RpaasInstanceSpec) (names []string) {
 	if c == nil {
 		return
 	}
 
 	names = append(names, c.DNSNames...)
-	if c.DNSNamesDefault && i.Spec.DNS != nil && i.Spec.DNS.Zone != "" {
-		names = append(names, fmt.Sprintf("%s.%s", i.Name, i.Spec.DNS.Zone))
+	if c.DNSNamesDefault && spec.DNS != nil && spec.DNS.Zone != "" {
+		names = append(names, fmt.Sprintf("%s.%s", name, spec.DNS.Zone))
 	}
 
 	return

api/v1alpha1/rpaasinstance_test.go

@@ -105,5 +105,5 @@ func TestCertManagerRequests(t *testing.T) {
 			IPAddresses:     []string{"10.1.1.1", "10.1.1.2"},
 			DNSNamesDefault: true,
 		},
-	}, instance.CertManagerRequests())
+	}, instance.Spec.CertManagerRequests(instance.Name))
 }

api/v1alpha1/rpaasinstance_types.go

@@ -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"
@@ -153,6 +156,10 @@ type DynamicCertificates struct {
 }
 
 type CertManager struct {
+	// Name is recently introduced to allow multiple certificates in the same instance.
+	// +optional
+	Name string `json:"name,omitempty"`
+
 	// Issuer refers either to Issuer or ClusterIssuer resource.
 	//
 	// NOTE: when there's no Issuer on this name, it tries using ClusterIssuer instead.
@@ -171,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"`

cmd/plugin/rpaasv2/cmd/certificates.go

@@ -49,7 +49,6 @@ func NewCmdUpdateCertitifcate() *cli.Command {
 			&cli.StringFlag{
 				Name:  "name",
 				Usage: "an identifier for the current certificate and key",
-				Value: "default",
 			},
 			&cli.PathFlag{
 				Name:    "certificate",
@@ -104,9 +103,14 @@ func runUpdateCertificate(c *cli.Context) error {
 		return err
 	}
 
+	name := c.String("name")
+	if name == "" {
+		name = "default"
+	}
+
 	args := rpaasclient.UpdateCertificateArgs{
 		Instance:    c.String("instance"),
-		Name:        c.String("name"),
+		Name:        name,
 		Certificate: string(certificate),
 		Key:         string(key),
 	}
@@ -131,6 +135,7 @@ func updateCertManagerCertificate(c *cli.Context, client rpaasclient.Client) (bo
 	err := client.UpdateCertManager(c.Context, rpaasclient.UpdateCertManagerArgs{
 		Instance: c.String("instance"),
 		CertManager: clientTypes.CertManager{
+			Name:        c.String("name"),
 			Issuer:      c.String("issuer"),
 			DNSNames:    c.StringSlice("dns"),
 			IPAddresses: c.StringSlice("ip"),
@@ -187,7 +192,13 @@ func runDeleteCertificate(c *cli.Context) error {
 	}
 
 	if c.Bool("cert-manager") {
-		if err = client.DeleteCertManager(c.Context, c.String("instance"), c.String("issuer")); err != nil {
+		if c.String("name") != "" {
+			err = client.DeleteCertManagerByName(c.Context, c.String("instance"), c.String("name"))
+		} else {
+			err = client.DeleteCertManagerByIssuer(c.Context, c.String("instance"), c.String("issuer"))
+		}
+
+		if err != nil {
 			return err
 		}
 
@@ -211,7 +222,12 @@ func runDeleteCertificate(c *cli.Context) error {
 func writeCertificatesInfoOnTableFormat(w io.Writer, certs []clientTypes.CertificateInfo) {
 	var data [][]string
 	for _, c := range certs {
-		data = append(data, []string{c.Name, formatPublicKeyInfo(c), formatCertificateValidity(c), strings.Join(c.DNSNames, "\n")})
+		extraInfo := ""
+		if c.IsManagedByCertManager {
+			extraInfo = "\n  managed by: cert-manager\n  issuer: " + c.CertManagerIssuer
+		}
+
+		data = append(data, []string{c.Name + extraInfo, formatPublicKeyInfo(c), formatCertificateValidity(c), strings.Join(c.DNSNames, "\n")})
 	}
 
 	table := tablewriter.NewWriter(w)

cmd/plugin/rpaasv2/cmd/certificates_test.go

@@ -93,6 +93,24 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
 			expected: "certificate \"my-instance.example.com\" updated in my-instance\n",
 		},
 
+		{
+			name: "when UpdateCertificate with default name",
+			args: []string{"./rpaasv2", "certificates", "update", "-i", "my-instance", "--cert", certFile.Name(), "--key", keyFile.Name()},
+			client: &fake.FakeClient{
+				FakeUpdateCertificate: func(args rpaasclient.UpdateCertificateArgs) error {
+					expected := rpaasclient.UpdateCertificateArgs{
+						Instance:    "my-instance",
+						Name:        "default",
+						Certificate: certPem,
+						Key:         keyPem,
+					}
+					assert.Equal(t, expected, args)
+					return nil
+				},
+			},
+			expected: "certificate \"default\" updated in my-instance\n",
+		},
+
 		{
 			name: "enabling cert-manager integration",
 			args: []string{"./rpaasv2", "certificates", "add", "-i", "my-instance", "--cert-manager", "--issuer", "lets-encrypt", "--dns", "my-instance.example.com", "--dns", "foo.example.com", "--ip", "169.196.100.100", "--ip", "2001:db8:dead:beef::"},
@@ -112,6 +130,25 @@ EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA==
 			expected: "cert manager certificate was updated\n",
 		},
 
+		{
+			name: "enabling cert-manager integration with name definition",
+			args: []string{"./rpaasv2", "certificates", "add", "-i", "my-instance", "--cert-manager", "--name", "cert01", "--issuer", "lets-encrypt", "--dns", "my-instance.example.com"},
+			client: &fake.FakeClient{
+				FakeUpdateCertManager: func(args rpaasclient.UpdateCertManagerArgs) error {
+					assert.Equal(t, rpaasclient.UpdateCertManagerArgs{
+						Instance: "my-instance",
+						CertManager: types.CertManager{
+							Name:     "cert01",
+							Issuer:   "lets-encrypt",
+							DNSNames: []string{"my-instance.example.com"},
+						},
+					}, args)
+					return nil
+				},
+			},
+			expected: "cert manager certificate was updated\n",
+		},
+
 		{
 			name: "passing DNS names without cert manager flag",
 			args: []string{"./rpaasv2", "certificates", "add", "-i", "my-instance", "--dns", "my-instance.example.com"},
@@ -182,17 +219,30 @@ func TestDeleteCertificate(t *testing.T) {
 			expected: "certificate \"my-instance.example.com\" successfully deleted on my-instance\n",
 		},
 		{
-			name: "removing a certificate request for Cert Manager",
+			name: "removing a certificate request for Cert Manager by issuer",
 			args: []string{"./rpaasv2", "certificates", "delete", "-i", "my-instance", "--cert-manager", "--issuer", "some-issuer"},
 			client: &fake.FakeClient{
-				FakeDeleteCertManager: func(instance, issuer string) error {
+				FakeDeleteCertManagerByIssuer: func(instance, issuer string) error {
 					assert.Equal(t, "my-instance", instance)
 					assert.Equal(t, "some-issuer", issuer)
 					return nil
 				},
 			},
 			expected: "cert manager integration was disabled\n",
 		},
+
+		{
+			name: "removing a certificate request for Cert Manager by issuer",
+			args: []string{"./rpaasv2", "certificates", "delete", "-i", "my-instance", "--cert-manager", "--name", "some-name"},
+			client: &fake.FakeClient{
+				FakeDeleteCertManagerByName: func(instance, name string) error {
+					assert.Equal(t, "my-instance", instance)
+					assert.Equal(t, "some-name", name)
+					return nil
+				},
+			},
+			expected: "cert manager integration was disabled\n",
+		},
 	}
 
 	for _, tt := range tests {

cmd/plugin/rpaasv2/cmd/info_test.go

@@ -288,6 +288,9 @@ func TestInfo(t *testing.T) {
 								ValidUntil:         time.Date(2050, time.August, 00, 00, 0, 0, 0, time.UTC),
 								PublicKeyAlgorithm: "ECDSA",
 								PublicKeyBitSize:   384,
+
+								IsManagedByCertManager: true,
+								CertManagerIssuer:      "lets-encrypt",
 							},
 						},
 						Events: []clientTypes.Event{
@@ -434,21 +437,21 @@ Addresses:
 +------------------+---------------------------------------+-----------------+--------+
 
 Certificates:
-+---------------+--------------------+----------------------+----------------------------+
-| Name          | Public Key Info    | Validity             | DNS names                  |
-+---------------+--------------------+----------------------+----------------------------+
-| default       |     Algorithm      |      Not before      |      my-instance.test      |
-|               |        RSA         | 2020-08-11T19:00:00Z |  my-instance.example.com   |
-|               |                    |                      |  .my-instance.example.com  |
-|               | Key size (in bits) |      Not after       | *.my-instance.example.com  |
-|               |        4096        | 2020-08-11T19:00:00Z |                            |
-+---------------+--------------------+----------------------+----------------------------+
-| default.ecdsa |     Algorithm      |      Not before      | another-domain.example.com |
-|               |       ECDSA        | 2000-07-31T00:00:00Z |                            |
-|               |                    |                      |                            |
-|               | Key size (in bits) |      Not after       |                            |
-|               |        384         | 2050-07-31T00:00:00Z |                            |
-+---------------+--------------------+----------------------+----------------------------+
++----------------------------+--------------------+----------------------+----------------------------+
+| Name                       | Public Key Info    | Validity             | DNS names                  |
++----------------------------+--------------------+----------------------+----------------------------+
+| default                    |     Algorithm      |      Not before      |      my-instance.test      |
+|                            |        RSA         | 2020-08-11T19:00:00Z |  my-instance.example.com   |
+|                            |                    |                      |  .my-instance.example.com  |
+|                            | Key size (in bits) |      Not after       | *.my-instance.example.com  |
+|                            |        4096        | 2020-08-11T19:00:00Z |                            |
++----------------------------+--------------------+----------------------+----------------------------+
+| default.ecdsa              |     Algorithm      |      Not before      | another-domain.example.com |
+|   managed by: cert-manager |       ECDSA        | 2000-07-31T00:00:00Z |                            |
+|   issuer: lets-encrypt     |                    |                      |                            |
+|                            | Key size (in bits) |      Not after       |                            |
+|                            |        384         | 2050-07-31T00:00:00Z |                            |
++----------------------------+--------------------+----------------------+----------------------------+
 
 Extra files:
 +-----------------+---------------------------------------------------------+