diff --git a/cmd/ingress-shim/controller/sync.go b/cmd/ingress-shim/controller/sync.go index cfb6939d0..4cd4e0cc7 100644 --- a/cmd/ingress-shim/controller/sync.go +++ b/cmd/ingress-shim/controller/sync.go @@ -43,12 +43,12 @@ func (c *Controller) Sync(ctx context.Context, ing *extv1beta1.Ingress) error { return nil } - crts, err := c.buildCertificates(ing) + newCrts, updateCrts, err := c.buildCertificates(ing) if err != nil { return err } - for _, crt := range crts { + for _, crt := range newCrts { _, err := c.CMClient.CertmanagerV1alpha1().Certificates(crt.Namespace).Create(crt) if err != nil { return err @@ -56,35 +56,38 @@ func (c *Controller) Sync(ctx context.Context, ing *extv1beta1.Ingress) error { c.Recorder.Eventf(ing, corev1.EventTypeNormal, "CreateCertificate", "Successfully created Certificate %q", crt.Name) } + for _, crt := range updateCrts { + _, err := c.CMClient.CertmanagerV1alpha1().Certificates(crt.Namespace).Update(crt) + if err != nil { + return err + } + c.Recorder.Eventf(ing, corev1.EventTypeNormal, "UpdateCertificate", "Successfully updated Certificate %q", crt.Name) + } + return nil } -func (c *Controller) buildCertificates(ing *extv1beta1.Ingress) ([]*v1alpha1.Certificate, error) { +func (c *Controller) buildCertificates(ing *extv1beta1.Ingress) (new, update []*v1alpha1.Certificate, _ error) { issuerName, issuerKind := c.issuerForIngress(ing) issuer, err := c.getGenericIssuer(ing.Namespace, issuerName, issuerKind) if err != nil { - return nil, err + return nil, nil, err } - var crts []*v1alpha1.Certificate + var newCrts []*v1alpha1.Certificate + var updateCrts []*v1alpha1.Certificate for i, tls := range ing.Spec.TLS { // validate the ingress TLS block if len(tls.Hosts) == 0 { - return nil, fmt.Errorf("secret %q for ingress %q has no hosts specified", tls.SecretName, ing.Name) + return nil, nil, fmt.Errorf("secret %q for ingress %q has no hosts specified", tls.SecretName, ing.Name) } if tls.SecretName == "" { - return nil, fmt.Errorf("TLS entry %d for ingress %q must specify a secretName", i, ing.Name) + return nil, nil, fmt.Errorf("TLS entry %d for ingress %q must specify a secretName", i, ing.Name) } - // check if a Certificate for this TLS entry already exists, and if it - // does then skip this entry existingCrt, err := c.certificateLister.Certificates(ing.Namespace).Get(tls.SecretName) if !apierrors.IsNotFound(err) && err != nil { - return nil, err - } - if existingCrt != nil { - glog.Infof("Certificate %q for ingress %q already exists, not re-creating", tls.SecretName, ing.Name) - continue + return nil, nil, err } crt := &v1alpha1.Certificate{ @@ -102,13 +105,65 @@ func (c *Controller) buildCertificates(ing *extv1beta1.Ingress) ([]*v1alpha1.Cer }, }, } + err = c.setIssuerSpecificConfig(crt, issuer, ing, tls) if err != nil { - return nil, err + return nil, nil, err + } + + // check if a Certificate for this TLS entry already exists, and if it + // does then skip this entry + if existingCrt != nil { + glog.Infof("Certificate %q for ingress %q already exists", tls.SecretName, ing.Name) + + if !certNeedsUpdate(existingCrt, crt) { + glog.Infof("Certificate %q for ingress %q is up to date", tls.SecretName, ing.Name) + continue + } + + updateCrt := existingCrt.DeepCopy() + + updateCrt.Spec.DNSNames = tls.Hosts + updateCrt.Spec.SecretName = tls.SecretName + updateCrt.Spec.IssuerRef.Name = issuerName + updateCrt.Spec.IssuerRef.Kind = issuerKind + updateCrts = append(updateCrts, updateCrt) + } else { + newCrts = append(newCrts, crt) } - crts = append(crts, crt) } - return crts, nil + return newCrts, updateCrts, nil +} + +// certNeedsUpdate checks and returns true if two Certificates are equal +func certNeedsUpdate(a, b *v1alpha1.Certificate) bool { + if a.Name != b.Name { + return true + } + + if len(a.Spec.DNSNames) != len(b.Spec.DNSNames) { + return true + } + + for i := range a.Spec.DNSNames { + if a.Spec.DNSNames[i] != b.Spec.DNSNames[i] { + return true + } + } + + if a.Spec.SecretName != b.Spec.SecretName { + return true + } + + if a.Spec.IssuerRef.Name != b.Spec.IssuerRef.Name { + return true + } + + if a.Spec.IssuerRef.Kind != b.Spec.IssuerRef.Kind { + return true + } + + return false } func (c *Controller) setIssuerSpecificConfig(crt *v1alpha1.Certificate, issuer v1alpha1.GenericIssuer, ing *extv1beta1.Ingress, tls extv1beta1.IngressTLS) error { diff --git a/cmd/ingress-shim/controller/sync_test.go b/cmd/ingress-shim/controller/sync_test.go index b25a7cd80..601987e63 100644 --- a/cmd/ingress-shim/controller/sync_test.go +++ b/cmd/ingress-shim/controller/sync_test.go @@ -69,7 +69,8 @@ func TestBuildCertificates(t *testing.T) { DefaultIssuerName string DefaultIssuerKind string Err bool - Expected []*v1alpha1.Certificate + ExpectedCreate []*v1alpha1.Certificate + ExpectedUpdate []*v1alpha1.Certificate } tests := []testT{ { @@ -93,7 +94,7 @@ func TestBuildCertificates(t *testing.T) { }, }, ClusterIssuerLister: []*v1alpha1.ClusterIssuer{buildACMEClusterIssuer("issuer-name")}, - Expected: []*v1alpha1.Certificate{ + ExpectedCreate: []*v1alpha1.Certificate{ { ObjectMeta: metav1.ObjectMeta{ Name: "example-com-tls", @@ -190,7 +191,7 @@ func TestBuildCertificates(t *testing.T) { }, }, ClusterIssuerLister: []*v1alpha1.ClusterIssuer{buildACMEClusterIssuer("issuer-name")}, - Expected: []*v1alpha1.Certificate{ + ExpectedCreate: []*v1alpha1.Certificate{ { ObjectMeta: metav1.ObjectMeta{ Name: "example-com-tls", @@ -259,7 +260,7 @@ func TestBuildCertificates(t *testing.T) { }, }, ClusterIssuerLister: []*v1alpha1.ClusterIssuer{buildClusterIssuer("issuer-name")}, - Expected: []*v1alpha1.Certificate{ + ExpectedCreate: []*v1alpha1.Certificate{ { ObjectMeta: metav1.ObjectMeta{ Name: "example-com-tls", @@ -333,13 +334,52 @@ func TestBuildCertificates(t *testing.T) { }, }, { - Name: "should not return any certificates if a Certificate already exists", + Name: "should not return any certificates if a correct Certificate already exists", Ingress: &extv1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "ingress-name", Namespace: "ingress-namespace", Annotations: map[string]string{ - issuerNameAnnotation: "issuer-name", + issuerNameAnnotation: "issuer-name", + acmeIssuerChallengeTypeAnnotation: "http01", + }, + }, + Spec: extv1beta1.IngressSpec{ + TLS: []extv1beta1.IngressTLS{ + { + Hosts: []string{"example.com"}, + SecretName: "existing-crt", + }, + }, + }, + }, + IssuerLister: []*v1alpha1.Issuer{buildACMEIssuer("issuer-name", "ingress-namespace")}, + CertificateLister: []*v1alpha1.Certificate{ + &v1alpha1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-crt", + Namespace: "ingress-namespace", + }, + Spec: v1alpha1.CertificateSpec{ + DNSNames: []string{"example.com"}, + SecretName: "existing-crt", + IssuerRef: v1alpha1.ObjectReference{ + Name: "issuer-name", + Kind: "Issuer", + }, + }, + }, + }, + }, + { + Name: "should update a certificate if an incorrect Certificate exists", + Ingress: &extv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-name", + Namespace: "ingress-namespace", + Annotations: map[string]string{ + issuerNameAnnotation: "issuer-name", + acmeIssuerChallengeTypeAnnotation: "http01", }, }, Spec: extv1beta1.IngressSpec{ @@ -353,6 +393,22 @@ func TestBuildCertificates(t *testing.T) { }, IssuerLister: []*v1alpha1.Issuer{buildACMEIssuer("issuer-name", "ingress-namespace")}, CertificateLister: []*v1alpha1.Certificate{buildCertificate("existing-crt", "ingress-namespace")}, + ExpectedUpdate: []*v1alpha1.Certificate{ + &v1alpha1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-crt", + Namespace: "ingress-namespace", + }, + Spec: v1alpha1.CertificateSpec{ + DNSNames: []string{"example.com"}, + SecretName: "existing-crt", + IssuerRef: v1alpha1.ObjectReference{ + Name: "issuer-name", + Kind: "Issuer", + }, + }, + }, + }, }, } testFn := func(test testT) func(t *testing.T) { @@ -380,12 +436,16 @@ func TestBuildCertificates(t *testing.T) { DefaultIssuerKind: test.DefaultIssuerKind, }, } - crts, err := c.buildCertificates(test.Ingress) + createCrts, updateCrts, err := c.buildCertificates(test.Ingress) if err != nil && !test.Err { t.Errorf("Expected no error, but got: %s", err) } - if !reflect.DeepEqual(crts, test.Expected) { - t.Errorf("Expected %+v but got %+v", test.Expected, crts) + if !reflect.DeepEqual(createCrts, test.ExpectedCreate) { + t.Errorf("Expected to create %+v but got %+v", test.ExpectedCreate, createCrts) + } + + if !reflect.DeepEqual(updateCrts, test.ExpectedUpdate) { + t.Errorf("Expected to update %+v but got %+v", test.ExpectedUpdate, updateCrts) } } }