Fix cert

api/annotations.go

@@ -5,6 +5,10 @@ import (
 	"net"
 	"strconv"
 	"strings"
+
+	stringz "github.com/appscode/go/strings"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	apiv1 "k8s.io/client-go/pkg/api/v1"
 )
 
 const (
@@ -139,6 +143,14 @@ const (
 	// If this annotation is not set HAProxy will connect to backend as http,
 	// This value should not be set if the backend do not support https resolution.
 	BackendTLSOptions = EngressKey + "/backend-tls"
+
+	certificateAnnotationKeyEnabled                      = "certificate.appscode.com/enabled"
+	certificateAnnotationKeyName                         = "certificate.appscode.com/name"
+	certificateAnnotationKeyProvider                     = "certificate.appscode.com/provider"
+	certificateAnnotationKeyEmail                        = "certificate.appscode.com/email"
+	certificateAnnotationKeyProviderCredentialSecretName = "certificate.appscode.com/provider-secret"
+	certificateAnnotationKeyACMEUserSecretName           = "certificate.appscode.com/user-secret"
+	certificateAnnotationKeyACMEServerURL                = "certificate.appscode.com/server-url"
 )
 
 func (r Ingress) OffshootName() string {
@@ -335,6 +347,58 @@ func (r Ingress) HAProxyOptions() map[string]bool {
 	return ret
 }
 
+func (r Ingress) CertificateSpec() (*Certificate, bool) {
+	if r.Annotations == nil {
+		return nil, false
+	}
+	if val, ok := r.Annotations[certificateAnnotationKeyEnabled]; ok && val == "true" {
+		certificateName := r.Annotations[certificateAnnotationKeyName]
+		// Check if a certificate already exists.
+		newCertificate := &Certificate{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      certificateName,
+				Namespace: r.Namespace,
+			},
+			Spec: CertificateSpec{
+				Provider: r.Annotations[certificateAnnotationKeyProvider],
+				Email:    r.Annotations[certificateAnnotationKeyEmail],
+				ProviderCredentialSecretName: r.Annotations[certificateAnnotationKeyProviderCredentialSecretName],
+				HTTPProviderIngressReference: apiv1.ObjectReference{
+					Kind:            "Ingress",
+					Name:            r.Name,
+					Namespace:       r.Namespace,
+					ResourceVersion: r.ResourceVersion,
+					UID:             r.UID,
+				},
+				ACMEUserSecretName: r.Annotations[certificateAnnotationKeyACMEUserSecretName],
+				ACMEServerURL:      r.Annotations[certificateAnnotationKeyACMEServerURL],
+			},
+		}
+
+		if v, ok := r.Annotations[APISchema]; ok {
+			if v == APISchemaIngress {
+				newCertificate.Spec.HTTPProviderIngressReference.APIVersion = APISchemaIngress
+			} else {
+				newCertificate.Spec.HTTPProviderIngressReference.APIVersion = APISchemaEngress
+			}
+		}
+
+		for _, rule := range r.Spec.Rules {
+			found := false
+			for _, tls := range r.Spec.TLS {
+				if stringz.Contains(tls.Hosts, rule.Host) {
+					found = true
+				}
+			}
+			if !found {
+				newCertificate.Spec.Domains = append(newCertificate.Spec.Domains, rule.Host)
+			}
+		}
+		return newCertificate, true
+	}
+	return nil, false
+}
+
 // ref: https://github.com/kubernetes/kubernetes/blob/078238a461a0872a8eacb887fbb3d0085714604c/staging/src/k8s.io/apiserver/pkg/apis/example/v1/types.go#L134
 // Deprecated, for newer ones use '{"k1":"v1", "k2", "v2"}' form
 // This expects the form k1=v1,k2=v2

api/validator_test.go

@@ -1,22 +1,27 @@
 package api
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 func TestIsValid(t *testing.T) {
 	for k, result := range dataTables {
-		err := k.IsValid("aws")
-		assert.Equal(t, err == nil, result, "%v", err)
+		err := k.IsValid("minikube")
+		if !assert.Equal(t, err == nil, result) {
+			fmt.Println("Failed Tests:", k.Name, "Reason\n", err)
+		}
 	}
 }
 
 var dataTables = map[*Ingress]bool{
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "No Backend Service For TCP"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -30,6 +35,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: false,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "No Listen Port for TCP"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -46,6 +52,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: false,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "TCP and HTTP in Same Port specified"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -80,6 +87,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: false,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "TCP and HTTP in Same Port not specified"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -113,6 +121,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: false,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "HTTP with host and path"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -156,6 +165,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: true,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "HTTP with hosts"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -198,6 +208,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: true,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "TCP multi in Same Port"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -226,6 +237,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: false,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "TCP with different port"},
 		Spec: IngressSpec{
 			Rules: []IngressRule{
 				{
@@ -254,6 +266,7 @@ var dataTables = map[*Ingress]bool{
 		},
 	}: true,
 	{
+		ObjectMeta: v1.ObjectMeta{Name: "Multi rule"},
 		Spec: IngressSpec{
 			Backend: &HTTPIngressBackend{
 				IngressBackend: IngressBackend{
@@ -362,10 +375,9 @@ var dataTables = map[*Ingress]bool{
 			},
 		},
 	}: true,
-	// https://github.com/appscode/voyager/issues/420
 	{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "nginx-ingress-app1",
+			Name:      "https://github.com/appscode/voyager/issues/420",
 			Namespace: "default",
 			Annotations: map[string]string{
 				"ingress.appscode.com/type": "HostPort",

pkg/certificate/acme.go

@@ -10,7 +10,6 @@ import (
 	"sync"
 
 	"github.com/appscode/errors"
-	stringutil "github.com/appscode/go/strings"
 	"github.com/appscode/log"
 	"github.com/appscode/voyager/api"
 	"github.com/appscode/voyager/pkg/certificate/providers"
@@ -30,6 +29,7 @@ import (
 	"github.com/xenolf/lego/providers/dns/route53"
 	"github.com/xenolf/lego/providers/dns/vultr"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	apiv1 "k8s.io/client-go/pkg/api/v1"
 )
 
@@ -272,9 +272,9 @@ func (c *ACMECertData) ToSecret(name, namespace string) *apiv1.Secret {
 }
 
 func (a ACMECertData) EqualDomains(c *x509.Certificate) bool {
-	certDomains := []string{
-		c.Subject.CommonName,
-	}
-	certDomains = append(certDomains, c.DNSNames...)
-	return stringutil.EqualSlice(certDomains, a.Domains.StringSlice())
+	certDomains := sets.NewString(c.Subject.CommonName)
+	certDomains.Insert(c.DNSNames...)
+
+	acmeDomains := sets.NewString(a.Domains.StringSlice()...)
+	return certDomains.Equal(acmeDomains)
 }

pkg/certificate/acme_test.go

@@ -112,3 +112,29 @@ func TestClient(t *testing.T) {
 		assert.Nil(t, err)
 	}
 }
+
+func TestEqualDomain(t *testing.T) {
+	cert := &x509.Certificate{
+		DNSNames: []string{"test1.com", "test2.com"},
+	}
+	cert.Subject.CommonName = "test.com"
+	tpr := &api.Certificate{Spec: api.CertificateSpec{Domains: []string{"test.com", "test1.com", "test2.com"}}}
+	a := ACMECertData{Domains: NewDomainCollection(tpr.Spec.Domains...)}
+	assert.Equal(t, a.EqualDomains(cert), true)
+
+	cert = &x509.Certificate{
+		DNSNames: []string{"test1.com", "test2.com"},
+	}
+	cert.Subject.CommonName = "test.com"
+	tpr = &api.Certificate{Spec: api.CertificateSpec{Domains: []string{"test.com", "test1.com", "test3.com"}}}
+	a = ACMECertData{Domains: NewDomainCollection(tpr.Spec.Domains...)}
+	assert.Equal(t, a.EqualDomains(cert), false)
+
+	cert = &x509.Certificate{
+		DNSNames: []string{"test1.com", "test2.com"},
+	}
+	cert.Subject.CommonName = "test.com"
+	tpr = &api.Certificate{Spec: api.CertificateSpec{Domains: []string{"test.com", "test1.com", "test3.com", "test2.com"}}}
+	a = ACMECertData{Domains: NewDomainCollection(tpr.Spec.Domains...)}
+	assert.Equal(t, a.EqualDomains(cert), false)
+}

pkg/certificate/controller.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/appscode/errors"
-	stringz "github.com/appscode/go/strings"
 	"github.com/appscode/log"
 	"github.com/appscode/voyager/api"
 	acs "github.com/appscode/voyager/client/clientset"
@@ -30,14 +29,6 @@ import (
 const (
 	defaultCertPrefix       = "cert-"
 	defaultUserSecretPrefix = "acme-"
-
-	certificateAnnotationKeyEnabled                      = "certificate.appscode.com/enabled"
-	certificateAnnotationKeyName                         = "certificate.appscode.com/name"
-	certificateAnnotationKeyProvider                     = "certificate.appscode.com/provider"
-	certificateAnnotationKeyEmail                        = "certificate.appscode.com/email"
-	certificateAnnotationKeyProviderCredentialSecretName = "certificate.appscode.com/provider-secret"
-	certificateAnnotationKeyACMEUserSecretName           = "certificate.appscode.com/user-secret"
-	certificateAnnotationKeyACMEServerURL                = "certificate.appscode.com/server-url"
 )
 
 type Controller struct {
@@ -69,56 +60,17 @@ func NewController(kubeClient clientset.Interface, extClient acs.ExtensionInterf
 
 func (c *Controller) HandleIngress(ingress *api.Ingress) error {
 	if ingress.Annotations != nil {
-		if val, ok := ingress.Annotations[certificateAnnotationKeyEnabled]; ok && val == "true" {
-			certificateName := ingress.Annotations[certificateAnnotationKeyName]
-			// Check if a certificate already exists.
-			certificate, err := c.ExtClient.Certificates(ingress.Namespace).Get(certificateName)
+		if cert, ok := ingress.CertificateSpec(); ok {
+			issuedCert, err := c.ExtClient.Certificates(ingress.Namespace).Get(cert.Name)
 			if err == nil {
 				// Certificate exists mount it.
 				return nil
 			}
-			if kerr.IsNotFound(err) || !certificate.Status.CertificateObtained {
-				newCertificate := &api.Certificate{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      certificateName,
-						Namespace: ingress.Namespace,
-					},
-					Spec: api.CertificateSpec{
-						Provider: ingress.Annotations[certificateAnnotationKeyProvider],
-						Email:    ingress.Annotations[certificateAnnotationKeyEmail],
-						ProviderCredentialSecretName: ingress.Annotations[certificateAnnotationKeyProviderCredentialSecretName],
-						HTTPProviderIngressReference: apiv1.ObjectReference{
-							Kind:            "Ingress",
-							Name:            ingress.Name,
-							Namespace:       ingress.Namespace,
-							ResourceVersion: ingress.ResourceVersion,
-							UID:             ingress.UID,
-						},
-						ACMEUserSecretName: ingress.Annotations[certificateAnnotationKeyACMEUserSecretName],
-						ACMEServerURL:      ingress.Annotations[certificateAnnotationKeyACMEServerURL],
-					},
-				}
-				if v, ok := ingress.Annotations[api.APISchema]; ok {
-					if v == api.APISchemaIngress {
-						newCertificate.Spec.HTTPProviderIngressReference.APIVersion = api.APISchemaIngress
-					} else {
-						newCertificate.Spec.HTTPProviderIngressReference.APIVersion = api.APISchemaEngress
-					}
-				}
-				for _, rule := range ingress.Spec.Rules {
-					found := false
-					for _, tls := range ingress.Spec.TLS {
-						if stringz.Contains(tls.Hosts, rule.Host) {
-							found = true
-						}
-					}
-					if !found {
-						newCertificate.Spec.Domains = append(newCertificate.Spec.Domains, rule.Host)
-					}
-				}
-				_, err := c.ExtClient.Certificates(newCertificate.Namespace).Create(newCertificate)
+
+			if kerr.IsNotFound(err) || !issuedCert.Status.CertificateObtained {
+				_, err := c.ExtClient.Certificates(cert.Namespace).Create(cert)
 				if err != nil {
-					errors.FromErr(err).Err()
+					return err
 				}
 			}
 		}
@@ -150,7 +102,7 @@ func (c *Controller) Process() error {
 			return errors.FromErr(err).WithMessage("Error decoding x509 encoded certificate").Err()
 		}
 		if !c.crt.NotAfter.After(time.Now().Add(time.Hour * 24 * 7)) {
-			log.Infoln("certificate is expiring in 7 days, attempting renew")
+			log.Infoln("Certificate is expiring in 7 days, attempting renew")
 			err := c.renew()
 			if err != nil {
 				c.recorder.Eventf(
@@ -170,7 +122,9 @@ func (c *Controller) Process() error {
 			)
 		}
 
+		c.acmeCert.Domains = NewDomainCollection(c.tpr.Spec.Domains...)
 		if !c.acmeCert.EqualDomains(c.crt) {
+			log.Infof("Domains not equal, tpr domains %v, cert dns %v, cert common name %v", c.tpr.Spec.Domains, c.crt.DNSNames, c.crt.Subject.CommonName)
 			err := c.renew()
 			if err != nil {
 				c.recorder.Eventf(