From 9dc927b04e1d68df67827a9b221d9707809884a9 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 2 Sep 2019 12:38:45 +0100 Subject: [PATCH 1/4] Fix-up manifests and upgrade guide for v0.10 Signed-off-by: James Munnelly --- .../cert-manager/templates/_helpers.tpl | 4 +++- .../templates/webhook-validating-webhook.yaml | 1 + docs/tasks/upgrading/upgrading-0.9-0.10.rst | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/deploy/charts/cert-manager/templates/_helpers.tpl b/deploy/charts/cert-manager/templates/_helpers.tpl index d579758f5..b116334bb 100644 --- a/deploy/charts/cert-manager/templates/_helpers.tpl +++ b/deploy/charts/cert-manager/templates/_helpers.tpl @@ -43,9 +43,11 @@ Create the name of the service account to use {{/* Expand the name of the chart. +Manually fix the 'app' and 'name' labels to 'webhook' to maintain +compatibility with the v0.9 deployment selector. */}} {{- define "webhook.name" -}} -{{- printf "%s-webhook" (default .Chart.Name .Values.nameOverride) | trunc 63 | trimSuffix "-" -}} +{{- printf "webhook" | trunc 63 | trimSuffix "-" -}} {{- end -}} {{/* diff --git a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml index 8e67165d4..523d45f5f 100644 --- a/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml +++ b/deploy/charts/cert-manager/templates/webhook-validating-webhook.yaml @@ -39,6 +39,7 @@ webhooks: - clusterissuers - certificaterequests failurePolicy: Fail + sideEffects: None clientConfig: service: name: kubernetes diff --git a/docs/tasks/upgrading/upgrading-0.9-0.10.rst b/docs/tasks/upgrading/upgrading-0.9-0.10.rst index 4c9953b85..446ada946 100644 --- a/docs/tasks/upgrading/upgrading-0.9-0.10.rst +++ b/docs/tasks/upgrading/upgrading-0.9-0.10.rst @@ -2,4 +2,21 @@ Upgrading from v0.9 to v0.10 ============================ -There are no special notes or considerations when upgrading from v0.9 to v0.10. +Due to changes in the way the webhook component's TLS is bootstrapped in v0.10, +you will need to delete your webhook's Certificate and Issuer resources. + +If you are using a deployment tool that automatically handles this (i.e. Helm), +there should be no additional action to take. + +If you are using the 'static manifests' to install, you should run the following +after upgrading: + +.. code-block:: shell + + kubectl delete -n cert-manager issuer cert-manager-webhook-ca cert-manager-webhook-selfsign + kubectl delete -n cert-manager certificate cert-manager-webhook-ca cert-manager-webhook-webhook-tls + kubectl delete apiservice v1beta1.admission.certmanager.k8s.io + +The Secret resources used to contain TLS assets for the webhook are now +automatically handled internally by cert-manager, so these resources are no +longer required. From fad10b8ca9f41b543bdf0ba456197306e3ce7bf2 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 2 Sep 2019 12:46:40 +0100 Subject: [PATCH 2/4] Fix bug in ACME certificaterequest implementation Signed-off-by: James Munnelly --- pkg/controller/certificaterequests/acme/acme.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/controller/certificaterequests/acme/acme.go b/pkg/controller/certificaterequests/acme/acme.go index 6674549cf..38beb2ef6 100644 --- a/pkg/controller/certificaterequests/acme/acme.go +++ b/pkg/controller/certificaterequests/acme/acme.go @@ -79,7 +79,6 @@ func NewACME(ctx *controllerpkg.Context) *ACME { func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer v1alpha1.GenericIssuer) (*issuerpkg.IssueResponse, error) { log := logf.FromContext(ctx, "sign") - resourceNamespace := a.issuerOptions.ResourceNamespace(issuer) // If we can't decode the CSR PEM we have to hard fail csr, err := pki.DecodeX509CertificateRequestBytes(cr.Spec.CSRPEM) @@ -107,9 +106,9 @@ func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer if k8sErrors.IsNotFound(err) { // Failing to create the order here is most likely network related. // We should backoff and keep trying. - _, err = a.cmClientV.Orders(resourceNamespace).Create(expectedOrder) + _, err = a.cmClientV.Orders(expectedOrder.Namespace).Create(expectedOrder) if err != nil { - message := fmt.Sprintf("Failed create new order resource %s/%s", resourceNamespace, expectedOrder.Name) + message := fmt.Sprintf("Failed create new order resource %s/%s", expectedOrder.Namespace, expectedOrder.Name) a.reporter.Pending(cr, err, "OrderCreatingError", message) log.Error(err, message) @@ -118,7 +117,7 @@ func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer } message := fmt.Sprintf("Created Order resource %s/%s", - resourceNamespace, expectedOrder.Name) + expectedOrder.Namespace, expectedOrder.Name) a.reporter.Pending(cr, nil, "OrderCreated", message) log.V(4).Info(message) @@ -127,7 +126,7 @@ func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer if err != nil { // We are probably in a network error here so we should backoff and retry - message := fmt.Sprintf("Failed to get order resource %s/%s", resourceNamespace, expectedOrder.Name) + message := fmt.Sprintf("Failed to get order resource %s/%s", expectedOrder.Namespace, expectedOrder.Name) a.reporter.Pending(cr, err, "OrderGetError", message) log.Error(err, message) @@ -140,7 +139,7 @@ func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer // If the acme order has failed then so too does the CertificateRequest meet the same fate. if acme.IsFailureState(order.Status.State) { message := fmt.Sprintf("Failed to wait for order resource %s/%s to become ready", - resourceNamespace, expectedOrder.Name) + expectedOrder.Namespace, expectedOrder.Name) err := fmt.Errorf("order is in %q state", order.Status.State) a.reporter.Failed(cr, err, "OrderFailed", message) @@ -160,7 +159,7 @@ func (a *ACME) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuer // We update here to just pending while we wait for the order to be resolved. a.reporter.Pending(cr, nil, "OrderPending", fmt.Sprintf("Waiting on certificate issuance from order %s/%s: %q", - resourceNamespace, order.Name, order.Status.State)) + expectedOrder.Namespace, order.Name, order.Status.State)) log.Info("acme Order resource is not in a ready state, waiting...") From cb71859badcf84604bf58944f2519737efddfa91 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Tue, 3 Sep 2019 10:52:32 +0100 Subject: [PATCH 3/4] Copy labels from Certificate to CertificateRequest Signed-off-by: James Munnelly --- pkg/controller/certificates/certificate_request.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller/certificates/certificate_request.go b/pkg/controller/certificates/certificate_request.go index c0800bb00..ef11900ec 100644 --- a/pkg/controller/certificates/certificate_request.go +++ b/pkg/controller/certificates/certificate_request.go @@ -603,7 +603,7 @@ func (c *certificateRequestManager) processCertificate(ctx context.Context, crt // If it is not Ready _OR_ Failed then we return and wait for informer // updates to re-trigger processing. default: - log.Info("CertificateRequest is in state %q, waiting until CertificateRequest is issued", reason) + log.Info("CertificateRequest is not in a final state, waiting until CertificateRequest is complete", "state", reason) return nil } } @@ -767,6 +767,7 @@ func (c *certificateRequestManager) buildCertificateRequest(log logr.Logger, crt Annotations: map[string]string{ cmapi.CRPrivateKeyAnnotationKey: crt.Spec.SecretName, }, + Labels: crt.Labels, }, Spec: cmapi.CertificateRequestSpec{ CSRPEM: csrPEM, From 9542fe50d0d1418003cc97bde679670ebfa1c623 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Tue, 3 Sep 2019 10:52:51 +0100 Subject: [PATCH 4/4] Add basic DNS zone selector unit tests Signed-off-by: James Munnelly --- .../acmeorders/selectors/BUILD.bazel | 12 ++- .../acmeorders/selectors/dns_zones_test.go | 78 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 pkg/controller/acmeorders/selectors/dns_zones_test.go diff --git a/pkg/controller/acmeorders/selectors/BUILD.bazel b/pkg/controller/acmeorders/selectors/BUILD.bazel index b72d6f571..fbb5cfa8f 100644 --- a/pkg/controller/acmeorders/selectors/BUILD.bazel +++ b/pkg/controller/acmeorders/selectors/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -30,3 +30,13 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["dns_zones_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/apis/certmanager/v1alpha1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + ], +) diff --git a/pkg/controller/acmeorders/selectors/dns_zones_test.go b/pkg/controller/acmeorders/selectors/dns_zones_test.go new file mode 100644 index 000000000..97a5c3e3c --- /dev/null +++ b/pkg/controller/acmeorders/selectors/dns_zones_test.go @@ -0,0 +1,78 @@ +/* +Copyright 2019 The Jetstack cert-manager contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package selectors + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" +) + +func TestDNSZones(t *testing.T) { + tests := []struct { + name string + selector cmapi.CertificateDNSNameSelector + meta metav1.ObjectMeta + dnsName string + matches bool + score int + }{ + { + name: "matching a domain with an empty selector", + selector: cmapi.CertificateDNSNameSelector{}, + dnsName: "www.example.com", + matches: true, + score: 0, + }, + { + name: "matching a domain in a zone", + selector: cmapi.CertificateDNSNameSelector{ + DNSZones: []string{"example.com"}, + }, + dnsName: "www.example.com", + matches: true, + score: 2, + }, + { + name: "matching a wildcard domain in a zone", + selector: cmapi.CertificateDNSNameSelector{ + DNSZones: []string{"example.com"}, + }, + dnsName: "*.example.com", + matches: true, + score: 2, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testSelector(t, DNSZones(test.selector), test.meta, test.dnsName, test.matches, test.score) + }) + } +} + +func testSelector(t *testing.T, sel Selector, meta metav1.ObjectMeta, dnsName string, expectMatch bool, expectedScore int) { + matches, score := sel.Matches(meta, dnsName) + if matches != expectMatch { + t.Errorf("expected match to be %t but it was %t", expectMatch, matches) + } + if score != expectedScore { + t.Errorf("expected score to be %d but it was %d", expectedScore, score) + } +}