From bc9181a9250482fff5717ce9806ad72aa5aa6078 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Wed, 27 Jun 2018 10:52:00 +0100 Subject: [PATCH 1/3] Update spec.acme.config field when ingress changes Fixes #619. --- pkg/controller/ingress-shim/sync.go | 13 ++++ pkg/controller/ingress-shim/sync_test.go | 92 ++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/pkg/controller/ingress-shim/sync.go b/pkg/controller/ingress-shim/sync.go index 1a7791290..dc7075b86 100644 --- a/pkg/controller/ingress-shim/sync.go +++ b/pkg/controller/ingress-shim/sync.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "reflect" "strconv" "github.com/golang/glog" @@ -133,6 +134,12 @@ func (c *Controller) buildCertificates(ing *extv1beta1.Ingress) (new, update []* updateCrt.Spec.SecretName = tls.SecretName updateCrt.Spec.IssuerRef.Name = issuerName updateCrt.Spec.IssuerRef.Kind = issuerKind + updateCrt.Spec.IssuerRef.Kind = issuerKind + updateCrt.Spec.IssuerRef.Kind = issuerKind + err = c.setIssuerSpecificConfig(updateCrt, issuer, ing, tls) + if err != nil { + return nil, nil, err + } updateCrts = append(updateCrts, updateCrt) } else { newCrts = append(newCrts, crt) @@ -169,6 +176,12 @@ func certNeedsUpdate(a, b *v1alpha1.Certificate) bool { return true } + if a.Spec.ACME != nil && b.Spec.ACME != nil { + if !reflect.DeepEqual(a.Spec.ACME.Config, b.Spec.ACME.Config) { + return true + } + } + return false } diff --git a/pkg/controller/ingress-shim/sync_test.go b/pkg/controller/ingress-shim/sync_test.go index 1524c7460..085081544 100644 --- a/pkg/controller/ingress-shim/sync_test.go +++ b/pkg/controller/ingress-shim/sync_test.go @@ -568,6 +568,98 @@ func TestBuildCertificates(t *testing.T) { Name: "issuer-name", Kind: "Issuer", }, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.ACMECertificateDomainConfig{ + { + Domains: []string{"example.com"}, + ACMESolverConfig: v1alpha1.ACMESolverConfig{ + HTTP01: &v1alpha1.ACMECertificateHTTP01Config{ + Ingress: "", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + Name: "should update a certificate's config 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", + ingressClassAnnotation: "toot-ing", + }, + }, + Spec: extv1beta1.IngressSpec{ + TLS: []extv1beta1.IngressTLS{ + { + Hosts: []string{"example.com"}, + SecretName: "existing-crt", + }, + }, + }, + }, + IssuerLister: []*v1alpha1.Issuer{buildACMEIssuer("issuer-name", "ingress-namespace")}, + CertificateLister: []*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", + }, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.ACMECertificateDomainConfig{ + { + Domains: []string{"wrong-example.com"}, + ACMESolverConfig: v1alpha1.ACMESolverConfig{ + HTTP01: &v1alpha1.ACMECertificateHTTP01Config{ + Ingress: "wrong-ingress", + }, + }, + }, + }, + }, + }, + }, + }, + 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", + }, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.ACMECertificateDomainConfig{ + { + Domains: []string{"example.com"}, + ACMESolverConfig: v1alpha1.ACMESolverConfig{ + HTTP01: &v1alpha1.ACMECertificateHTTP01Config{ + Ingress: "", + IngressClass: strPtr("toot-ing"), + }, + }, + }, + }, + }, }, }, }, From 25311a57c5cff248ec611dd8cb08d1b2ab8b2fc7 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Wed, 27 Jun 2018 14:37:20 +0100 Subject: [PATCH 2/3] Add better check for nil spec.acme --- pkg/controller/ingress-shim/sync.go | 18 ++++++++++++------ pkg/controller/ingress-shim/sync_test.go | 12 ++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/pkg/controller/ingress-shim/sync.go b/pkg/controller/ingress-shim/sync.go index dc7075b86..47f4add67 100644 --- a/pkg/controller/ingress-shim/sync.go +++ b/pkg/controller/ingress-shim/sync.go @@ -134,8 +134,6 @@ func (c *Controller) buildCertificates(ing *extv1beta1.Ingress) (new, update []* updateCrt.Spec.SecretName = tls.SecretName updateCrt.Spec.IssuerRef.Name = issuerName updateCrt.Spec.IssuerRef.Kind = issuerKind - updateCrt.Spec.IssuerRef.Kind = issuerKind - updateCrt.Spec.IssuerRef.Kind = issuerKind err = c.setIssuerSpecificConfig(updateCrt, issuer, ing, tls) if err != nil { return nil, nil, err @@ -176,10 +174,18 @@ func certNeedsUpdate(a, b *v1alpha1.Certificate) bool { return true } - if a.Spec.ACME != nil && b.Spec.ACME != nil { - if !reflect.DeepEqual(a.Spec.ACME.Config, b.Spec.ACME.Config) { - return true - } + var configA, configB []v1alpha1.ACMECertificateDomainConfig + + if a.Spec.ACME != nil { + configA = a.Spec.ACME.Config + } + + if b.Spec.ACME != nil { + configB = b.Spec.ACME.Config + } + + if !reflect.DeepEqual(configA, configB) { + return true } return false diff --git a/pkg/controller/ingress-shim/sync_test.go b/pkg/controller/ingress-shim/sync_test.go index 085081544..f02572e15 100644 --- a/pkg/controller/ingress-shim/sync_test.go +++ b/pkg/controller/ingress-shim/sync_test.go @@ -529,6 +529,18 @@ func TestBuildCertificates(t *testing.T) { Name: "issuer-name", Kind: "Issuer", }, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.ACMECertificateDomainConfig{ + { + Domains: []string{"example.com"}, + ACMESolverConfig: v1alpha1.ACMESolverConfig{ + HTTP01: &v1alpha1.ACMECertificateHTTP01Config{ + Ingress: "", + }, + }, + }, + }, + }, }, }, }, From 86685369aa53c33ba9737dac040c2c25c9739143 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Fri, 29 Jun 2018 09:46:04 +0100 Subject: [PATCH 3/3] Add test for a non-acme certificate being appropriately updated --- pkg/controller/ingress-shim/sync_test.go | 68 ++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/pkg/controller/ingress-shim/sync_test.go b/pkg/controller/ingress-shim/sync_test.go index f02572e15..13091635a 100644 --- a/pkg/controller/ingress-shim/sync_test.go +++ b/pkg/controller/ingress-shim/sync_test.go @@ -676,6 +676,74 @@ func TestBuildCertificates(t *testing.T) { }, }, }, + { + Name: "should update a Certificate correctly if an existing one of a different type exists", + Ingress: &extv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ingress-name", + Namespace: "ingress-namespace", + Annotations: map[string]string{ + issuerNameAnnotation: "issuer-name", + acmeIssuerChallengeTypeAnnotation: "http01", + ingressClassAnnotation: "toot-ing", + }, + }, + Spec: extv1beta1.IngressSpec{ + TLS: []extv1beta1.IngressTLS{ + { + Hosts: []string{"example.com"}, + SecretName: "existing-crt", + }, + }, + }, + }, + IssuerLister: []*v1alpha1.Issuer{buildACMEIssuer("issuer-name", "ingress-namespace")}, + CertificateLister: []*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", + }, + }, + }, + }, + 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", + }, + ACME: &v1alpha1.ACMECertificateConfig{ + Config: []v1alpha1.ACMECertificateDomainConfig{ + { + Domains: []string{"example.com"}, + ACMESolverConfig: v1alpha1.ACMESolverConfig{ + HTTP01: &v1alpha1.ACMECertificateHTTP01Config{ + Ingress: "", + IngressClass: strPtr("toot-ing"), + }, + }, + }, + }, + }, + }, + }, + }, + }, } testFn := func(test testT) func(t *testing.T) { return func(t *testing.T) {