From 1fc8b050a9369d9e7e499830dde5dbaf14da9b09 Mon Sep 17 00:00:00 2001 From: Martin Weindel Date: Fri, 23 Jul 2021 17:49:23 +0200 Subject: [PATCH] recognize change of issuer secret, no renewal on issuer secret change, migrate certificate hash, stop using pkg/errors --- build/Dockerfile | 2 +- go.mod | 1 - pkg/cert/legobridge/cakeypair.go | 3 +- pkg/cert/legobridge/certificate.go | 6 +- pkg/cert/legobridge/dnscontrollerprovider.go | 10 +- pkg/cert/legobridge/pki.go | 23 ++-- pkg/cert/legobridge/reguser.go | 9 +- pkg/cert/utils/utils_certificate.go | 2 +- pkg/controller/issuer/acme/handler.go | 120 +++++++----------- pkg/controller/issuer/ca/handler.go | 8 +- .../issuer/certificate/backupsecret.go | 20 ++- .../issuer/certificate/reconciler.go | 110 ++++++++++++---- pkg/controller/issuer/core/support.go | 36 ++++-- .../issuer/core/wrappedregistration.go | 43 +++++++ .../issuer/revocation/reconciler.go | 50 ++++---- test/functional/basics.go | 4 +- test/functional/config/config.go | 4 +- test/functional/config/utils.go | 7 +- vendor/modules.txt | 1 - 19 files changed, 274 insertions(+), 185 deletions(-) create mode 100644 pkg/controller/issuer/core/wrappedregistration.go diff --git a/build/Dockerfile b/build/Dockerfile index 2e181a9b..88bf523e 100644 --- a/build/Dockerfile +++ b/build/Dockerfile @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 ############# builder ############# -FROM eu.gcr.io/gardener-project/3rd/golang:1.16.2 AS builder +FROM eu.gcr.io/gardener-project/3rd/golang:1.16.6 AS builder WORKDIR /build COPY . . diff --git a/go.mod b/go.mod index de7a8170..3cb34040 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,6 @@ require ( github.com/miekg/dns v1.1.31 github.com/onsi/ginkgo v1.14.1 github.com/onsi/gomega v1.10.2 - github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.7.1 golang.org/x/lint v0.0.0-20200302205851-738671d3881b k8s.io/api v0.20.6 diff --git a/pkg/cert/legobridge/cakeypair.go b/pkg/cert/legobridge/cakeypair.go index 089ade7f..73f5cda2 100644 --- a/pkg/cert/legobridge/cakeypair.go +++ b/pkg/cert/legobridge/cakeypair.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" ) @@ -79,7 +78,7 @@ func (c *TLSKeyPair) RawCertInfo() ([]byte, error) { certInfo, err := json.Marshal(raw) if err != nil { - return nil, errors.Wrap(err, "encoding certificate info failed") + return nil, fmt.Errorf("encoding certificate info failed: %w", err) } return certInfo, nil } diff --git a/pkg/cert/legobridge/certificate.go b/pkg/cert/legobridge/certificate.go index f07876e6..72d82032 100644 --- a/pkg/cert/legobridge/certificate.go +++ b/pkg/cert/legobridge/certificate.go @@ -14,8 +14,6 @@ import ( "sync" "time" - "github.com/pkg/errors" - "github.com/gardener/cert-management/pkg/cert/metrics" "github.com/gardener/cert-management/pkg/cert/utils" @@ -368,7 +366,7 @@ func DecodeCertificate(tlsCrt []byte) (*x509.Certificate, error) { } cert, err := x509.ParseCertificate(block.Bytes) if err != nil { - return nil, fmt.Errorf("parsing certificate failed with %s", err.Error()) + return nil, fmt.Errorf("parsing certificate failed: %w", err) } return cert, nil } @@ -415,7 +413,7 @@ func RevokeCertificate(user *RegistrationUser, cert []byte) error { // A client facilitates communication with the CA server. client, err := lego.NewClient(config) if err != nil { - return errors.Wrap(err, "client creation failed") + return fmt.Errorf("client creation failed: %w", err) } return client.Certificate.Revoke(cert) diff --git a/pkg/cert/legobridge/dnscontrollerprovider.go b/pkg/cert/legobridge/dnscontrollerprovider.go index f2f510f6..07ba7d60 100644 --- a/pkg/cert/legobridge/dnscontrollerprovider.go +++ b/pkg/cert/legobridge/dnscontrollerprovider.go @@ -38,7 +38,7 @@ func newDNSControllerProvider(settings DNSControllerSettings, certificateName resources.ObjectName, targetClass string, issuerKey utils.IssuerKey) (ProviderWithCount, error) { itf, err := settings.Cluster.Resources().GetByExample(&dnsapi.DNSEntry{}) if err != nil { - return nil, fmt.Errorf("cannot get DNSEntry resources: %s", err.Error()) + return nil, fmt.Errorf("cannot get DNSEntry resources: %w", err) } n := atomic.AddUint32(&index, 1) return &dnsControllerProvider{ @@ -136,7 +136,7 @@ func (p *dnsControllerProvider) Present(domain, token, keyAuth string) error { } _, err := p.entryResources.Create(entry) if err != nil { - return fmt.Errorf("creating DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error()) + return fmt.Errorf("creating DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err) } return nil } @@ -145,14 +145,14 @@ func (p *dnsControllerProvider) Present(domain, token, keyAuth string) error { err := retryOnUpdateError(func() error { obj, err := p.entryResources.Get_(entry) if err != nil { - return fmt.Errorf("getting DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error()) + return fmt.Errorf("getting DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err) } entry = obj.Data().(*dnsapi.DNSEntry) setSpec(entry) p.logger.Infof("presenting DNSEntry %s/%s for certificate resource %s with %d values", entry.Namespace, entry.Name, p.certificateName, len(values)) _, err = p.entryResources.Update(entry) if err != nil { - return &updateError{msg: fmt.Sprintf("updating DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error())} + return &updateError{msg: fmt.Sprintf("updating DNSEntry %s/%s failed: %s", entry.Namespace, entry.Name, err)} } return nil }) @@ -199,7 +199,7 @@ func (p *dnsControllerProvider) CleanUp(domain, token, keyAuth string) error { p.logger.Infof("cleanup DNSEntry %s/%s for request %s", entry.Namespace, entry.Name, p.certificateName) err := p.entryResources.Delete(entry) if err != nil { - return fmt.Errorf("deleting DNSEntry %s/%s failed with %s", entry.Namespace, entry.Name, err.Error()) + return fmt.Errorf("deleting DNSEntry %s/%s failed: %w", entry.Namespace, entry.Name, err) } return nil } diff --git a/pkg/cert/legobridge/pki.go b/pkg/cert/legobridge/pki.go index 5bcd050d..5a2d7033 100644 --- a/pkg/cert/legobridge/pki.go +++ b/pkg/cert/legobridge/pki.go @@ -22,7 +22,6 @@ import ( "time" "github.com/go-acme/lego/v4/certificate" - "github.com/pkg/errors" ) var ( @@ -107,15 +106,15 @@ func generateKey(algo x509.PublicKeyAlgorithm, size int) (crypto.Signer, []byte, switch algo { case x509.RSA: if size < RSAMinSize { - return nil, nil, errors.New("RSA key is too weak") + return nil, nil, fmt.Errorf("RSA key is too weak") } if size > RSAMaxSize { - return nil, nil, errors.New("RSA key size too large") + return nil, nil, fmt.Errorf("RSA key size too large") } key, err = rsa.GenerateKey(rand.Reader, size) if err != nil { - return nil, nil, errors.Wrap(err, "unable to generate RSA private key") + return nil, nil, fmt.Errorf("unable to generate RSA private key: %w", err) } case x509.ECDSA: var curve elliptic.Curve @@ -127,20 +126,20 @@ func generateKey(algo x509.PublicKeyAlgorithm, size int) (crypto.Signer, []byte, case ECCurve256: curve = elliptic.P256() default: - return nil, nil, errors.New("invalid elliptic curve") + return nil, nil, fmt.Errorf("invalid elliptic curve") } key, err = ecdsa.GenerateKey(curve, rand.Reader) if err != nil { - return nil, nil, errors.Wrap(err, "unable to generate RSA private key") + return nil, nil, fmt.Errorf("unable to generate RSA private key: %w", err) } default: - return nil, nil, errors.New("algorithm not supported") + return nil, nil, fmt.Errorf("algorithm not supported") } pem, err := privateKeyToBytes(key) if err != nil { - return nil, nil, errors.Wrap(err, "encoding private key failed") + return nil, nil, fmt.Errorf("encoding private key failed: %w", err) } return key, pem, nil } @@ -185,7 +184,7 @@ func createCertReq(input ObtainInput) (*x509.CertificateRequest, error) { func generateCSRPEM(csr *x509.CertificateRequest, privateKey crypto.Signer) ([]byte, error) { derBytes, err := x509.CreateCertificateRequest(rand.Reader, csr, privateKey) if err != nil { - return nil, errors.Wrap(err, "failed to create certificate request") + return nil, fmt.Errorf("failed to create certificate request: %w", err) } pemBytes := bytes.NewBuffer([]byte{}) @@ -220,7 +219,7 @@ func generateCertFromCSR(csrPEM []byte, duration time.Duration, isCA bool) (*x50 serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) if err != nil { - return nil, errors.Wrap(err, "failed to generate serial number") + return nil, fmt.Errorf("failed to generate serial number: %w", err) } return &x509.Certificate{ @@ -244,7 +243,7 @@ func generateCertFromCSR(csrPEM []byte, duration time.Duration, isCA bool) (*x50 func signCert(cert, issuerCert *x509.Certificate, publicKey crypto.PublicKey, signerKey crypto.PrivateKey) ([]byte, error) { derBytes, err := x509.CreateCertificate(rand.Reader, cert, issuerCert, publicKey, signerKey) if err != nil { - return nil, fmt.Errorf("error creating x509 certificate: %s", err.Error()) + return nil, fmt.Errorf("error creating x509 certificate: %w", err) } pemBytes := bytes.NewBuffer([]byte{}) @@ -365,7 +364,7 @@ func encodeCertPEM(out io.Writer, derBytes []byte) error { func encodePEM(out io.Writer, b *pem.Block) error { err := pem.Encode(out, b) if err != nil { - return errors.Wrap(err, "error encoding certificate PEM") + return fmt.Errorf("error encoding certificate PEM: %w", err) } return nil } diff --git a/pkg/cert/legobridge/reguser.go b/pkg/cert/legobridge/reguser.go index 5dbd26ab..8eb758fa 100644 --- a/pkg/cert/legobridge/reguser.go +++ b/pkg/cert/legobridge/reguser.go @@ -140,7 +140,7 @@ func NewRegistrationUserFromEmailAndPrivateKey(issuerKey utils.IssuerKey, func (u *RegistrationUser) ToSecretData() (map[string][]byte, error) { privkey, err := privateKeyToBytes(u.key) if err != nil { - return nil, fmt.Errorf("encoding private key failed: %s", err.Error()) + return nil, fmt.Errorf("encoding private key failed: %w", err) } return map[string][]byte{KeyPrivateKey: privkey}, nil } @@ -149,7 +149,7 @@ func (u *RegistrationUser) ToSecretData() (map[string][]byte, error) { func (u *RegistrationUser) RawRegistration() ([]byte, error) { reg, err := json.Marshal(u.registration) if err != nil { - return nil, fmt.Errorf("encoding registration failed: %s", err.Error()) + return nil, fmt.Errorf("encoding registration failed: %w", err) } return reg, nil } @@ -169,7 +169,10 @@ func RegistrationUserFromSecretData(issuerKey utils.IssuerKey, reg := ®istration.Resource{} err = json.Unmarshal(registrationRaw, reg) if err != nil { - return nil, fmt.Errorf("unmarshalling registration json failed with %s", err.Error()) + return nil, fmt.Errorf("unmarshalling registration json failed: %w", err) + } + if reg.URI == "" { + return nil, fmt.Errorf("unmarshalling registration with unexpected empty URI") } metrics.AddACMEAccountRegistration(issuerKey, reg.URI, email) return &RegistrationUser{email: email, registration: reg, caDirURL: caDirURL, key: privateKey, diff --git a/pkg/cert/utils/utils_certificate.go b/pkg/cert/utils/utils_certificate.go index 5a5f7adf..720398ce 100644 --- a/pkg/cert/utils/utils_certificate.go +++ b/pkg/cert/utils/utils_certificate.go @@ -99,7 +99,7 @@ func ExtractDomains(spec *api.CertificateSpec) ([]string, error) { func ExtractCommonNameAnDNSNames(csr []byte) (cn *string, san []string, err error) { certificateRequest, err := extractCertificateRequest(csr) if err != nil { - err = fmt.Errorf("parsing CSR failed with: %s", err) + err = fmt.Errorf("parsing CSR failed: %w", err) return } cnvalue := certificateRequest.Subject.CommonName diff --git a/pkg/controller/issuer/acme/handler.go b/pkg/controller/issuer/acme/handler.go index bb9ccf23..dc9e92b9 100644 --- a/pkg/controller/issuer/acme/handler.go +++ b/pkg/controller/issuer/acme/handler.go @@ -56,12 +56,13 @@ func (r *acmeIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Ob return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("missing server in ACME spec")) } - r.support.AddIssuerDomains(obj.ClusterKey(), issuer.Spec.ACME.Domains) + r.support.AddIssuerDomains(obj.ClusterKey(), acme.Domains) - r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, "") + r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, "") issuerKey := r.support.ToIssuerKey(obj.ClusterKey()) var secret *corev1.Secret + var secretHash string var err error if acme.PrivateKeySecretRef != nil { secret, err = r.support.ReadIssuerSecret(issuerKey, acme.PrivateKeySecretRef) @@ -69,110 +70,77 @@ func (r *acmeIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Ob if acme.AutoRegistration { logger.Info("spec.acme.privateKeySecretRef not existing, creating new account") } else { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed with %s", err.Error())) + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed: %w", err)) } } - hash := r.support.CalcSecretHash(secret) - r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, hash) + secretHash = r.support.CalcSecretHash(secret) + r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, secretHash) } - if secret != nil && issuer.Status.ACME != nil && issuer.Status.ACME.Raw != nil { - eabKeyID, eabHmacKey, err := r.support.LoadEABHmacKey(issuerKey, acme) + if secret != nil { + objKey := obj.ClusterKey() + eabKeyID, eabHmacKey, err := r.support.LoadEABHmacKey(&objKey, issuerKey, acme) if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading EAB secret failed: %s", err)) + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("loading EAB secret failed: %w", err)) } - user, err := legobridge.RegistrationUserFromSecretData(issuerKey, acme.Email, acme.Server, issuer.Status.ACME.Raw, + var raw []byte + if core.IsSameExistingRegistration(issuer.Status.ACME, secretHash) { + raw = issuer.Status.ACME.Raw + } else { + user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, secret.Data, eabKeyID, eabHmacKey) + if err != nil { + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed: %w", err)) + } + raw, err = user.RawRegistration() + if err != nil { + return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed: %w", err)) + } + } + user, err := legobridge.RegistrationUserFromSecretData(issuerKey, acme.Email, acme.Server, raw, secret.Data, eabKeyID, eabHmacKey) if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("extracting registration user from secret failed with %s", err.Error())) + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("extracting registration user from secret failed: %w", err)) } if user.GetEmail() != acme.Email { return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("email of registration user from secret does not match %s != %s", user.GetEmail(), acme.Email)) } - return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType, issuer.Status.ACME.Raw) - } else if secret != nil || acme.AutoRegistration { - eabKid, eabHmacKey, err := r.prepareEAB(obj, issuer) + wrapped, err := core.WrapRegistration(raw, secretHash) if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, err) + return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("wrapped registration marshalling failed: %w", err)) } - var secretData map[string][]byte - if secret != nil { - secretData = secret.Data - } - user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, secretData, eabKid, eabHmacKey) + return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType, wrapped) + } else if acme.AutoRegistration { + user, err := legobridge.NewRegistrationUserFromEmail(issuerKey, acme.Email, acme.Server, nil, "", "") if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed with %s", err.Error())) + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("creating registration user failed: %w", err)) } - if secret != nil { - err = r.support.UpdateIssuerSecret(issuerKey, user, secret) - if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating issuer secret failed with %s", err.Error())) - } - } else { - secretRef, secret, err := r.support.WriteIssuerSecretFromRegistrationUser(issuerKey, issuer.UID, user, acme.PrivateKeySecretRef) - if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("writing issuer secret failed with %s", err.Error())) - } - issuer.Spec.ACME.PrivateKeySecretRef = secretRef - hash := r.support.CalcSecretHash(secret) - r.support.RememberIssuerSecret(obj.ClusterKey(), issuer.Spec.ACME.PrivateKeySecretRef, hash) + secretRef, secret, err := r.support.WriteIssuerSecretFromRegistrationUser(issuerKey, issuer.UID, user, acme.PrivateKeySecretRef) + if err != nil { + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("writing issuer secret failed: %w", err)) } + acme.PrivateKeySecretRef = secretRef + secretHash = r.support.CalcSecretHash(secret) + r.support.RememberIssuerSecret(obj.ClusterKey(), acme.PrivateKeySecretRef, secretHash) regRaw, err := user.RawRegistration() if err != nil { - return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed with %s", err.Error())) + return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("registration marshalling failed: %w", err)) } newObj, err := r.support.GetIssuerResources(issuerKey).Update(issuer) if err != nil { - return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating resource failed with %s", err.Error())) + return r.failedAcmeRetry(logger, obj, api.StateError, fmt.Errorf("updating resource failed: %w", err)) } - return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType, regRaw) + raw, err := core.WrapRegistration(regRaw, secretHash) + if err != nil { + return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("wrapped registration marshalling failed: %w", err)) + } + return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType, raw) } else { return r.failedAcme(logger, obj, api.StateError, fmt.Errorf("neither `SecretRef` or `AutoRegistration: true` provided")) } } -func (r *acmeIssuerHandler) prepareEAB(obj resources.Object, issuer *api.Issuer) (eabKid, eabHmacKey string, err error) { - acme := issuer.Spec.ACME - eab := acme.ExternalAccountBinding - - if eab == nil { - return - } - - r.support.RememberIssuerEABSecret(obj.ClusterKey(), eab.KeySecretRef, "") - - if eab.KeyID == "" { - err = fmt.Errorf("missing keyID for external account binding in ACME spec") - return - } - - if eab.KeySecretRef == nil { - err = fmt.Errorf("missing keySecretRef for external account binding in ACME spec") - return - } - - issuerKey := r.support.ToIssuerKey(obj.ClusterKey()) - secret, err := r.support.ReadIssuerSecret(issuerKey, eab.KeySecretRef) - if err != nil { - err = fmt.Errorf("loading issuer secret for external account binding failed with %s", err.Error()) - return - } - hash := r.support.CalcSecretHash(secret) - r.support.RememberIssuerEABSecret(obj.ClusterKey(), eab.KeySecretRef, hash) - - hmacEncoded, ok := secret.Data[legobridge.KeyHmacKey] - if !ok { - err = fmt.Errorf("key %s not found in secret data", legobridge.KeyHmacKey) - return - } - - eabKid = eab.KeyID - eabHmacKey = string(hmacEncoded) - return -} - func (r *acmeIssuerHandler) failedAcme(logger logger.LogContext, obj resources.Object, state string, err error) reconcile.Status { return r.support.Failed(logger, obj, state, &acmeType, err, false) } diff --git a/pkg/controller/issuer/ca/handler.go b/pkg/controller/issuer/ca/handler.go index b39c4471..b7c8755c 100644 --- a/pkg/controller/issuer/ca/handler.go +++ b/pkg/controller/issuer/ca/handler.go @@ -57,7 +57,7 @@ func (r *caIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Obje issuerKey := r.support.ToIssuerKey(obj.ClusterKey()) secret, err = r.support.ReadIssuerSecret(issuerKey, ca.PrivateKeySecretRef) if err != nil { - return r.failedCARetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed with %s", err.Error())) + return r.failedCARetry(logger, obj, api.StateError, fmt.Errorf("loading issuer secret failed: %w", err)) } hash := r.support.CalcSecretHash(secret) r.support.RememberIssuerSecret(obj.ClusterKey(), ca.PrivateKeySecretRef, hash) @@ -88,13 +88,13 @@ func validateSecretCA(secret *corev1.Secret) ([]byte, error) { // Validate it can be used as a CAKeyPair CAKeyPair, err := legobridge.CAKeyPairFromSecretData(secret.Data) if err != nil { - return nil, fmt.Errorf("extracting CA Keypair from secret failed with %s", err.Error()) + return nil, fmt.Errorf("extracting CA Keypair from secret failed: %w", err) } // Validate cert and key are valid and that they match together ok, err := legobridge.ValidatePublicKeyWithPrivateKey(CAKeyPair.Cert.PublicKey, CAKeyPair.Key) if err != nil { - return nil, fmt.Errorf("check private key failed with %s", err.Error()) + return nil, fmt.Errorf("check private key failed: %w", err) } if !ok { return nil, fmt.Errorf("private key does not match certificate") @@ -112,7 +112,7 @@ func validateSecretCA(secret *corev1.Secret) ([]byte, error) { CAInfoRaw, err := CAKeyPair.RawCertInfo() if err != nil { - return nil, fmt.Errorf("cert info marshalling failed with %s", err.Error()) + return nil, fmt.Errorf("cert info marshalling failed: %w", err) } return CAInfoRaw, nil diff --git a/pkg/controller/issuer/certificate/backupsecret.go b/pkg/controller/issuer/certificate/backupsecret.go index cf17cf74..ccd6d865 100644 --- a/pkg/controller/issuer/certificate/backupsecret.go +++ b/pkg/controller/issuer/certificate/backupsecret.go @@ -40,7 +40,7 @@ func BackupSecret(res resources.Interface, secret *corev1.Secret, hashKey string } sn := SerialNumberToString(cert.SerialNumber, true) list, err := res.Namespace(metav1.NamespaceSystem).List(metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s,%s=%s", LabelCertificateSerialNumber, sn, LabelCertificateHashKey, hashKey), + LabelSelector: fmt.Sprintf("%s=%s,%s=%s", LabelCertificateSerialNumber, sn, LabelCertificateNewHashKey, hashKey), }) if err != nil { return @@ -61,7 +61,7 @@ func BackupSecret(res resources.Interface, secret *corev1.Secret, hashKey string backupSecret.GenerateName = fmt.Sprintf("cert-backup-%s-%s-", issuerInfo.Key().Name(), sn[len(sn)-8:]) backupSecret.Namespace = metav1.NamespaceSystem backupSecret.Data = secret.Data - resources.SetLabel(backupSecret, LabelCertificateHashKey, hashKey) + resources.SetLabel(backupSecret, LabelCertificateNewHashKey, hashKey) resources.SetLabel(backupSecret, LabelCertificateKey, "true") resources.SetLabel(backupSecret, LabelCertificateSerialNumber, sn) resources.SetLabel(backupSecret, LabelCertificateBackup, "true") @@ -85,10 +85,18 @@ func BackupSecret(res resources.Interface, secret *corev1.Secret, hashKey string }, true, nil } -// FindAllCertificateSecretsByHashLabel get all certificate secrets by the certificate hash -func FindAllCertificateSecretsByHashLabel(res resources.Interface, hashKey string) ([]resources.Object, error) { +// FindAllCertificateSecretsByOldHashLabel get all certificate secrets by the old certificate hash +func FindAllCertificateSecretsByOldHashLabel(res resources.Interface, hashKey string) ([]resources.Object, error) { opts := metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", LabelCertificateHashKey, hashKey), + LabelSelector: fmt.Sprintf("%s=%s", LabelCertificateOldHashKey, hashKey), + } + return res.List(opts) +} + +// FindAllCertificateSecretsByNewHashLabel get all certificate secrets by the certificate hash +func FindAllCertificateSecretsByNewHashLabel(res resources.Interface, hashKey string) ([]resources.Object, error) { + opts := metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", LabelCertificateNewHashKey, hashKey), } return res.List(opts) } @@ -96,7 +104,7 @@ func FindAllCertificateSecretsByHashLabel(res resources.Interface, hashKey strin // FindAllOldBackupSecrets finds all certificate secret backups which have not been requested after the given timestamp. func FindAllOldBackupSecrets(res resources.Interface, hashKey string, timestamp time.Time) ([]api.CertificateSecretRef, error) { list, err := res.Namespace(metav1.NamespaceSystem).List(metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", LabelCertificateHashKey, hashKey), + LabelSelector: fmt.Sprintf("%s=%s", LabelCertificateNewHashKey, hashKey), }) if err != nil { return nil, err diff --git a/pkg/controller/issuer/certificate/reconciler.go b/pkg/controller/issuer/certificate/reconciler.go index a5a85cab..91097d06 100644 --- a/pkg/controller/issuer/certificate/reconciler.go +++ b/pkg/controller/issuer/certificate/reconciler.go @@ -15,7 +15,6 @@ import ( "time" "github.com/go-acme/lego/v4/certificate" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -24,6 +23,7 @@ import ( "github.com/gardener/controller-manager-library/pkg/controllermanager/cluster" "github.com/gardener/controller-manager-library/pkg/controllermanager/controller" "github.com/gardener/controller-manager-library/pkg/controllermanager/controller/reconcile" + "github.com/gardener/controller-manager-library/pkg/errors" "github.com/gardener/controller-manager-library/pkg/logger" "github.com/gardener/controller-manager-library/pkg/resources" cmlutils "github.com/gardener/controller-manager-library/pkg/utils" @@ -39,8 +39,10 @@ import ( ) const ( - // LabelCertificateHashKey is the label for the certificate hash - LabelCertificateHashKey = api.GroupName + "/certificate-hash" + // LabelCertificateOldHashKey is the old label for the certificate hash + LabelCertificateOldHashKey = api.GroupName + "/certificate-hash" + // LabelCertificateNewHashKey is the new label for the certificate hash + LabelCertificateNewHashKey = api.GroupName + "/hash" // LabelCertificateKey is the label for marking secrets created for a certificate LabelCertificateKey = api.GroupName + "/certificate" // LabelCertificateBackup is the label for marking backup secrets @@ -234,12 +236,19 @@ func (r *certReconciler) reconcileCert(logctx logger.LogContext, obj resources.O // ignore if SecretRef is specified but not existing // will later be used to store the secret } else { - if storedHash := cert.Labels[LabelCertificateHashKey]; storedHash != "" { - specHash := r.buildSpecHash(&cert.Spec, issuerKey) + if storedHash := cert.Labels[LabelCertificateNewHashKey]; storedHash != "" { + specHash := r.buildSpecNewHash(&cert.Spec, issuerKey) if specHash != storedHash { return r.removeStoredHashKeyAndRepeat(logctx, obj) } return r.checkForRenewAndSucceeded(logctx, obj, secret) + } else if storedOldHash := cert.Labels[LabelCertificateOldHashKey]; storedOldHash != "" { + specOldHash := r.buildSpecOldHash(&cert.Spec, issuerKey) + if specOldHash != storedOldHash { + return r.removeStoredHashKeyAndRepeat(logctx, obj) + } + specNewHash := r.buildSpecNewHash(&cert.Spec, issuerKey) + return r.migrateSpecHash(logctx, obj, specOldHash, specNewHash) } // corner case: existing secret but no stored hash, check if renewal is overdue @@ -269,7 +278,7 @@ func (r *certReconciler) addEvent(obj resources.Object, status reconcile.Status, func (r *certReconciler) handleObtainOutput(logctx logger.LogContext, obj resources.Object, result *legobridge.ObtainOutput) (reconcile.Status, bool) { if result.Err != nil { - return r.failed(logctx, obj, api.StateError, errors.Wrapf(result.Err, "obtaining certificate failed")), true + return r.failed(logctx, obj, api.StateError, fmt.Errorf("obtaining certificate failed: %w", result.Err)), true } cert, _ := obj.Data().(*api.Certificate) @@ -285,13 +294,13 @@ func (r *certReconciler) handleObtainOutput(logctx logger.LogContext, obj resour IssuerRef: &api.IssuerRef{Name: result.IssuerInfo.Key().Name(), Namespace: result.IssuerInfo.Key().Namespace()}, } issuerKey := r.support.IssuerClusterObjectKey(cert.Namespace, spec) - specHash := r.buildSpecHash(spec, issuerKey) + specHash := r.buildSpecNewHash(spec, issuerKey) now := time.Now() secretRef, err := r.writeCertificateSecret(logctx, result.IssuerInfo, cert.ObjectMeta, result.Certificates, specHash, specSecretRef, &now) if err != nil { - return r.failed(logctx, obj, api.StateError, errors.Wrapf(err, "writing certificate secret failed")), false + return r.failed(logctx, obj, api.StateError, fmt.Errorf("writing certificate secret failed: %w", err)), false } logctx.Infof("certificate written in secret %s/%s", secretRef.Namespace, secretRef.Name) @@ -444,7 +453,7 @@ func (r *certReconciler) obtainCertificateAndPendingACME(logctx logger.LogContex case *legobridge.ConcurrentObtainError: return r.delay(logctx, obj, api.StatePending, err) default: - return r.failed(logctx, obj, api.StateError, errors.Wrapf(err, "preparing obtaining certificates failed")) + return r.failed(logctx, obj, api.StateError, fmt.Errorf("preparing obtaining certificates failed: %w", err)) } } r.pendingRequests.Add(objectName) @@ -471,12 +480,12 @@ func (r *certReconciler) restoreCA(issuerKey utils.IssuerKey, issuer *api.Issuer issuerSecret := &corev1.Secret{} _, err := r.support.GetIssuerSecretResources(issuerKey).GetInto(issuerSecretObjectName, issuerSecret) if err != nil { - return nil, errors.Wrap(err, "fetching issuer secret failed") + return nil, fmt.Errorf("fetching issuer secret failed: %w", err) } CAKeyPair, err := legobridge.CAKeyPairFromSecretData(issuerSecret.Data) if err != nil { - return nil, errors.Wrap(err, "restoring CA issuer from issuer secret failed") + return nil, fmt.Errorf("restoring CA issuer from issuer secret failed: %w", err) } return CAKeyPair, nil @@ -531,7 +540,7 @@ func (r *certReconciler) obtainCertificateCA(logctx logger.LogContext, obj resou case *legobridge.ConcurrentObtainError: return r.delay(logctx, obj, api.StatePending, err) default: - return r.failed(logctx, obj, api.StateError, errors.Wrap(err, "preparing obtaining certificates failed")) + return r.failed(logctx, obj, api.StateError, fmt.Errorf("preparing obtaining certificates failed: %w", err)) } } r.pendingRequests.Add(objectName) @@ -586,7 +595,7 @@ func (r *certReconciler) loadSecret(secretRef *corev1.SecretReference) (*corev1. secret := &corev1.Secret{} _, err := r.certSecretResources.GetInto(secretObjectName, secret) if err != nil { - return nil, errors.Wrap(err, "fetching certificate secret failed") + return nil, fmt.Errorf("fetching certificate secret failed: %w", err) } return secret, nil @@ -664,7 +673,7 @@ func (r *certReconciler) updateForRenewalAndRepeat(logctx logger.LogContext, obj crt.Spec.EnsureRenewedAfter = renewalTime err := obj.Update() if err != nil { - status := r.failed(logctx, obj, crt.Status.State, errors.Wrap(err, "requesting renewal")) + status := r.failed(logctx, obj, crt.Status.State, fmt.Errorf("requesting renewal: %w", err)) return &status } status := reconcile.RescheduleAfter(logctx, 1*time.Second) @@ -673,7 +682,7 @@ func (r *certReconciler) updateForRenewalAndRepeat(logctx logger.LogContext, obj return nil } -func (r *certReconciler) buildSpecHash(spec *api.CertificateSpec, issuerKey utils.IssuerKey) string { +func (r *certReconciler) buildSpecOldHash(spec *api.CertificateSpec, issuerKey utils.IssuerKey) string { h := sha256.New224() if spec.CommonName != nil { h.Write([]byte(*spec.CommonName)) @@ -694,6 +703,26 @@ func (r *certReconciler) buildSpecHash(spec *api.CertificateSpec, issuerKey util return fmt.Sprintf("%x", h.Sum(nil)) } +func (r *certReconciler) buildSpecNewHash(spec *api.CertificateSpec, issuerKey utils.IssuerKey) string { + h := sha256.New224() + if spec.CommonName != nil { + h.Write([]byte(*spec.CommonName)) + h.Write([]byte{0}) + for _, domain := range spec.DNSNames { + h.Write([]byte(domain)) + h.Write([]byte{0}) + } + } + if spec.CSR != nil { + h.Write([]byte{0}) + h.Write(spec.CSR) + h.Write([]byte{0}) + } + h.Write([]byte(issuerKey.String())) + h.Write([]byte{0}) + return fmt.Sprintf("%x", h.Sum(nil)) +} + func (r *certReconciler) explictRenewalRequested(cert *x509.Certificate, requestedAt *time.Time, ensureRenewalAfter *metav1.Time) bool { return ensureRenewalAfter != nil && WasRequestedBefore(cert, requestedAt, ensureRenewalAfter.Time) } @@ -735,8 +764,8 @@ func (r *certReconciler) determineSecretRef(namespace string, spec *api.Certific func (r *certReconciler) findSecretByHashLabel(namespace string, spec *api.CertificateSpec) (*corev1.SecretReference, string, *time.Time) { issuerKey := r.support.IssuerClusterObjectKey(namespace, spec) - specHash := r.buildSpecHash(spec, issuerKey) - objs, err := FindAllCertificateSecretsByHashLabel(r.certSecretResources, specHash) + specHash := r.buildSpecNewHash(spec, issuerKey) + objs, err := FindAllCertificateSecretsByNewHashLabel(r.certSecretResources, specHash) if err != nil { return nil, "", nil } @@ -807,7 +836,7 @@ func (r *certReconciler) writeCertificateSecret(logctx logger.LogContext, issuer } else { secret.SetGenerateName(objectMeta.GetName() + "-") } - resources.SetLabel(secret, LabelCertificateHashKey, specHash) + resources.SetLabel(secret, LabelCertificateNewHashKey, specHash) resources.SetLabel(secret, LabelCertificateKey, "true") resources.RemoveAnnotation(secret, AnnotationRevoked) setRequestedAtAnnotation(secret, requestedAt) @@ -824,7 +853,7 @@ func (r *certReconciler) writeCertificateSecret(logctx logger.LogContext, issuer obj, err := r.certSecretResources.CreateOrUpdate(secret) if err != nil { - return nil, errors.Wrap(err, "creating/updating certificate secret failed") + return nil, fmt.Errorf("creating/updating certificate secret failed: %w", err) } ref, created, err := BackupSecret(r.certSecretResources, secret, specHash, issuerInfo) @@ -844,7 +873,7 @@ func (r *certReconciler) updateSecretRefAndSucceeded(logctx logger.LogContext, o if crt.Labels == nil { crt.Labels = map[string]string{} } - crt.Labels[LabelCertificateHashKey] = specHash + crt.Labels[LabelCertificateNewHashKey] = specHash if notAfter != nil { resources.SetAnnotation(crt, AnnotationNotAfter, notAfter.Format(time.RFC3339)) } else { @@ -852,21 +881,56 @@ func (r *certReconciler) updateSecretRefAndSucceeded(logctx logger.LogContext, o } obj2, err := r.certResources.Update(crt) if err != nil { - return r.failed(logctx, obj, api.StateError, errors.Wrap(err, "updating certificate resource failed")) + return r.failed(logctx, obj, api.StateError, fmt.Errorf("updating certificate resource failed: %w", err)) } return r.succeeded(logctx, obj2, nil) } func (r *certReconciler) removeStoredHashKeyAndRepeat(logctx logger.LogContext, obj resources.Object) reconcile.Status { c := obj.Data().(*api.Certificate) - delete(c.Labels, LabelCertificateHashKey) + delete(c.Labels, LabelCertificateOldHashKey) + delete(c.Labels, LabelCertificateNewHashKey) obj2, err := r.certResources.Update(c) if err != nil { - return r.failed(logctx, obj, api.StateError, errors.Wrap(err, "updating certificate resource failed")) + return r.failed(logctx, obj, api.StateError, fmt.Errorf("updating certificate resource failed: %w", err)) } return r.repeat(logctx, obj2) } +func (r *certReconciler) migrateSpecHash(logctx logger.LogContext, obj resources.Object, specOldHash, specNewHash string) reconcile.Status { + logctx.Infof("migrating spec hash") + + crt := obj.Data().(*api.Certificate) + + objs, err := FindAllCertificateSecretsByOldHashLabel(r.certSecretResources, specOldHash) + if err != nil { + return r.failed(logctx, obj, api.StateError, fmt.Errorf("find all certificates by old hash failed: %w", err)) + } + + for _, obj := range objs { + if _, ok := resources.GetLabel(obj.Data(), LabelCertificateNewHashKey); !ok { + obj.SetLabels(resources.AddLabel(obj.GetLabels(), LabelCertificateNewHashKey, specNewHash)) + err = obj.Update() + if err != nil { + return r.failed(logctx, obj, api.StateError, fmt.Errorf("updating label for certificate secret %s failed: %w", obj.ObjectName(), err)) + } + } + } + + if crt.Labels == nil { + crt.Labels = map[string]string{} + } + crt.Labels[LabelCertificateNewHashKey] = specNewHash + _, err = r.certResources.Update(crt) + if err != nil { + return r.failed(logctx, obj, api.StateError, fmt.Errorf("updating certificate resource failed: %w", err)) + } + + time.Sleep(500 * time.Millisecond) + + return reconcile.Succeeded(logctx) +} + func (r *certReconciler) prepareUpdateStatus(obj resources.Object, state string, msg *string, mode backoffMode) (*resources.ModificationState, *api.CertificateStatus) { crt := obj.Data().(*api.Certificate) status := &crt.Status diff --git a/pkg/controller/issuer/core/support.go b/pkg/controller/issuer/core/support.go index 2c7fd438..89a39ba1 100644 --- a/pkg/controller/issuer/core/support.go +++ b/pkg/controller/issuer/core/support.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -267,7 +266,7 @@ func (s *Support) WriteIssuerSecretFromRegistrationUser(issuerKey utils.IssuerKe obj, err := s.GetIssuerSecretResources(issuerKey).CreateOrUpdate(secret) if err != nil { - return nil, nil, fmt.Errorf("creating/updating issuer secret failed with %s", err.Error()) + return nil, nil, fmt.Errorf("creating/updating issuer secret failed: %w", err) } return &corev1.SecretReference{Name: obj.GetName(), Namespace: secret.GetNamespace()}, secret, nil @@ -283,11 +282,11 @@ func (s *Support) UpdateIssuerSecret(issuerKey utils.IssuerKey, reguser *legobri } obj, err := s.GetIssuerSecretResources(issuerKey).Wrap(secret) if err != nil { - return fmt.Errorf("wrapping issuer secret failed with %s", err.Error()) + return fmt.Errorf("wrapping issuer secret failed: %w", err) } err = obj.Update() if err != nil { - return fmt.Errorf("updating issuer secret failed with %s", err.Error()) + return fmt.Errorf("updating issuer secret failed: %w", err) } return nil @@ -544,9 +543,9 @@ func (s *Support) CertificateNamesForIssuer(issuer resources.ClusterObjectKey) [ // IssuerNamesForSecretOrEABSecret returns issuer names for a secret name func (s *Support) IssuerNamesForSecretOrEABSecret(secretKey resources.ClusterObjectKey) resources.ObjectNameSet { - cluster := utils.ClusterDefault - if secretKey.Cluster() == s.targetCluster.GetId() { - cluster = utils.ClusterTarget + cluster := utils.ClusterTarget + if secretKey.Cluster() == s.defaultCluster.GetId() { + cluster = utils.ClusterDefault } key := utils.NewIssuerSecretKey(cluster, secretKey.Namespace(), secretKey.Name()) list1 := s.toObjectNameSet(s.state.IssuerNamesForSecret(key)) @@ -569,7 +568,7 @@ func (s *Support) RememberIssuerSecret(issuer resources.ClusterObjectKey, secret // RememberIssuerEABSecret stores issuer EAB secret ref pair. func (s *Support) RememberIssuerEABSecret(issuer resources.ClusterObjectKey, secretRef *corev1.SecretReference, hash string) { issuerKey := s.ToIssuerKey(issuer) - s.state.RememberIssuerSecret(issuerKey, secretRef, hash) + s.state.RememberIssuerEABSecret(issuerKey, secretRef, hash) } // RememberIssuerQuotas stores the issuer quotas. @@ -630,6 +629,7 @@ func (s *Support) GetIssuerSecretResources(issuerKey utils.IssuerKey) resources. } // CalcSecretHash calculates the secret hash +// If real is true, precalculated hash value of `IssuerSecretHashKey` is ignored func (s *Support) CalcSecretHash(secret *corev1.Secret) string { if secret == nil { return "" @@ -702,7 +702,7 @@ func (s *Support) LoadIssuer(issuerKey utils.IssuerKey) (*api.Issuer, error) { issuer := &api.Issuer{} _, err := res.GetInto(issuerKey.ObjectName(s.IssuerNamespace()), issuer) if err != nil { - return nil, errors.Wrap(err, "fetching issuer failed") + return nil, fmt.Errorf("fetching issuer failed: %w", err) } return issuer, nil } @@ -731,10 +731,10 @@ func (s *Support) RestoreRegUser(issuerKey utils.IssuerKey, issuer *api.Issuer) } issuerSecret, err := s.ReadIssuerSecret(issuerKey, secretRef) if err != nil { - return nil, errors.Wrap(err, "fetching issuer secret failed") + return nil, fmt.Errorf("fetching issuer secret failed: %w", err) } - eabKeyID, eabHmacKey, err := s.LoadEABHmacKey(issuerKey, acme) + eabKeyID, eabHmacKey, err := s.LoadEABHmacKey(nil, issuerKey, acme) if err != nil { return nil, err } @@ -742,26 +742,34 @@ func (s *Support) RestoreRegUser(issuerKey utils.IssuerKey, issuer *api.Issuer) reguser, err := legobridge.RegistrationUserFromSecretData(issuerKey, issuer.Spec.ACME.Email, issuer.Spec.ACME.Server, issuer.Status.ACME.Raw, issuerSecret.Data, eabKeyID, eabHmacKey) if err != nil { - return nil, errors.Wrap(err, "restoring registration issuer from issuer secret failed") + return nil, fmt.Errorf("restoring registration issuer from issuer secret failed: %w", err) } return reguser, nil } // LoadEABHmacKey reads the external account binding MAC key from the referenced secret -func (s *Support) LoadEABHmacKey(issuerKey utils.IssuerKey, acme *api.ACMESpec) (string, string, error) { +func (s *Support) LoadEABHmacKey(objKey *resources.ClusterObjectKey, issuerKey utils.IssuerKey, acme *api.ACMESpec) (string, string, error) { eab := acme.ExternalAccountBinding if eab == nil { return "", "", nil } + if eab.KeyID == "" { + return "", "", fmt.Errorf("missing keyID for external account binding in ACME spec") + } + if eab.KeySecretRef == nil { return "", "", fmt.Errorf("missing secret ref in issuer for external account binding") } secret, err := s.ReadIssuerSecret(issuerKey, eab.KeySecretRef) if err != nil { - return "", "", errors.Wrap(err, "fetching issuer EAB secret failed") + return "", "", fmt.Errorf("fetching issuer EAB secret failed: %w", err) + } + if objKey != nil { + hash := s.CalcSecretHash(secret) + s.RememberIssuerEABSecret(*objKey, eab.KeySecretRef, hash) } hmacEncoded, ok := secret.Data[legobridge.KeyHmacKey] diff --git a/pkg/controller/issuer/core/wrappedregistration.go b/pkg/controller/issuer/core/wrappedregistration.go new file mode 100644 index 00000000..08874aa8 --- /dev/null +++ b/pkg/controller/issuer/core/wrappedregistration.go @@ -0,0 +1,43 @@ +/* + * SPDX-FileCopyrightText: 2019 SAP SE or an SAP affiliate company and Gardener contributors + * + * SPDX-License-Identifier: Apache-2.0 + */ + +package core + +import ( + "encoding/json" + + "github.com/go-acme/lego/v4/registration" + "k8s.io/apimachinery/pkg/runtime" +) + +type wrappedRegistration struct { + registration.Resource `json:",inline"` + SecretHash *string `json:"secretHash,omitempty"` +} + +// WrapRegistration wraps registration +func WrapRegistration(raw []byte, secretHash string) ([]byte, error) { + reg := &wrappedRegistration{} + err := json.Unmarshal(raw, reg) + if err != nil { + return nil, err + } + reg.SecretHash = &secretHash + return json.Marshal(®) +} + +// IsSameExistingRegistration returns true if status ACME has same secret hash +// or if it has in the old format without secret hash (for migration) +func IsSameExistingRegistration(raw *runtime.RawExtension, realSecretHash string) bool { + if raw == nil || raw.Raw == nil { + return false + } + reg := &wrappedRegistration{} + if err := json.Unmarshal(raw.Raw, reg); err == nil && reg.SecretHash != nil { + return *reg.SecretHash == realSecretHash + } + return true +} diff --git a/pkg/controller/issuer/revocation/reconciler.go b/pkg/controller/issuer/revocation/reconciler.go index e9c9748b..b317d90b 100644 --- a/pkg/controller/issuer/revocation/reconciler.go +++ b/pkg/controller/issuer/revocation/reconciler.go @@ -12,7 +12,6 @@ import ( "strings" "time" - "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" apierrrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -22,6 +21,7 @@ import ( "github.com/gardener/controller-manager-library/pkg/controllermanager/cluster" "github.com/gardener/controller-manager-library/pkg/controllermanager/controller" "github.com/gardener/controller-manager-library/pkg/controllermanager/controller/reconcile" + "github.com/gardener/controller-manager-library/pkg/errors" "github.com/gardener/controller-manager-library/pkg/logger" "github.com/gardener/controller-manager-library/pkg/resources" @@ -110,7 +110,7 @@ func (r *revokeReconciler) Reconcile(logctx logger.LogContext, obj resources.Obj revocation.Spec.QualifyingDate = &metav1.Time{Time: time.Now()} _, err := r.certRevocationResources.Update(revocation) if err != nil && !apierrrors.IsConflict(err) { - return r.failed(logctx, obj, api.StateError, errors.Wrap(err, "updating certificate revocation resource failed")) + return r.failed(logctx, obj, api.StateError, fmt.Errorf("updating certificate revocation resource failed: %w", err)) } return reconcile.Recheck(logctx, nil, 500*time.Millisecond) } @@ -127,9 +127,9 @@ func (r *revokeReconciler) Reconcile(logctx logger.LogContext, obj resources.Obj if cert.Status.State == api.StateRevoked && !isInvolved(revocation, cert) { return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certficate already revoked")) } - hashKey, ok := cert.Labels[certificate.LabelCertificateHashKey] + hashKey, ok := cert.Labels[certificate.LabelCertificateNewHashKey] if !ok { - return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate has no %s label", certificate.LabelCertificateHashKey)) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate has no %s label", certificate.LabelCertificateNewHashKey)) } issuerKey := r.support.IssuerClusterObjectKey(cert.Namespace, &cert.Spec) @@ -185,24 +185,24 @@ func (r *revokeReconciler) collectSecretsRefsAndRepeat(logctx logger.LogContext, secret, err := r.loadSecret(certSecretRef) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "certificate secret")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate secret: %w", err)) } - hashKey, ok := secret.Labels[certificate.LabelCertificateHashKey] + hashKey, ok := secret.Labels[certificate.LabelCertificateNewHashKey] if !ok { - return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("secret has no %s label", certificate.LabelCertificateHashKey)) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("secret has no %s label", certificate.LabelCertificateNewHashKey)) } // secret is already backed up on certificate creation, only needed for backwards compatibility issuerInfo := utils.NewACMEIssuerInfo(issuerKey) _, _, err = certificate.BackupSecret(r.certSecretResources, secret, hashKey, issuerInfo) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "secret backup failed")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("secret backup failed: %w", err)) } secretRefs, err := certificate.FindAllOldBackupSecrets(r.certSecretResources, hashKey, revocation.Spec.QualifyingDate.Time) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "find all old secrets failed")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("find all old secrets failed: %w", err)) } return r.updateStatusAndDelay(logctx, obj, 1*time.Second, func(logctx logger.LogContext, obj resources.Object) (*resources.ModificationState, error) { @@ -218,11 +218,11 @@ func (r *revokeReconciler) collectCertificateRefsAndRepeat(logctx logger.LogCont revocation := obj.Data().(*api.CertificateRevocation) selector, err := createLabelCertificateHashKeySelector(hashKey) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "collectCertificateRefsAndRepeat")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("collectCertificateRefsAndRepeat: %w", err)) } list, err := r.certResources.ListCached(selector) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "list certificates with same hash")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("list certificates with same hash: %w", err)) } qualifyingDate := *revocation.Spec.QualifyingDate @@ -240,11 +240,11 @@ func (r *revokeReconciler) collectCertificateRefsAndRepeat(logctx logger.LogCont if apierrrors.IsNotFound(errors.Cause(err)) { continue } - return r.failedStop(logctx, obj, api.StateError, errors.Wrapf(err, "certificate %s/%s", cert.Namespace, cert.Name)) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate %s/%s: %w", cert.Namespace, cert.Name, err)) } x509cert, err := legobridge.DecodeCertificateFromSecretData(secret.Data) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrapf(err, "certificate %s/%s", cert.Namespace, cert.Name)) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate %s/%s: %w", cert.Namespace, cert.Name, err)) } requestedAt := certificate.ExtractRequestedAtFromAnnotation(secret) if !certificate.WasRequestedBefore(x509cert, requestedAt, qualifyingDate.Time) { @@ -257,7 +257,7 @@ func (r *revokeReconciler) collectCertificateRefsAndRepeat(logctx logger.LogCont if apierrrors.IsGone(err) { continue } - return r.failedStop(logctx, obj, api.StateError, errors.Wrapf(err, "certificate %s/%s cannot be updated", cert.Namespace, cert.Name)) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("certificate %s/%s cannot be updated: %w", cert.Namespace, cert.Name, err)) } } @@ -435,13 +435,13 @@ func (r *revokeReconciler) revokeOldCertificateSecrets(logctx logger.LogContext, if apierrrors.IsGone(err) { continue } - errs = append(errs, errors.Wrapf(err, "cannot load backup certificate secret: %s", secret.Name)) + errs = append(errs, fmt.Errorf("cannot load backup certificate secret: %s: %w", secret.Name, err)) failedSecrets = append(failedSecrets, ref) continue } cert, err := legobridge.DecodeCertificateFromSecretData(secret.Data) if err != nil { - errs = append(errs, errors.Wrapf(err, "cannot decode backup certificate secret %s", secret.Name)) + errs = append(errs, fmt.Errorf("cannot decode backup certificate secret %s: %w", secret.Name, err)) failedSecrets = append(failedSecrets, ref) continue } @@ -450,7 +450,7 @@ func (r *revokeReconciler) revokeOldCertificateSecrets(logctx logger.LogContext, if _, ok := resources.GetAnnotation(secret, certificate.AnnotationRevoked); !ok { err = legobridge.RevokeCertificate(user, tlscrt) if err != nil { - errs = append(errs, errors.Wrapf(err, "certificate revocation failed for backup certificate secret %s", secret.Name)) + errs = append(errs, fmt.Errorf("certificate revocation failed for backup certificate secret %s: %w", secret.Name, err)) failedSecrets = append(failedSecrets, ref) continue } @@ -461,7 +461,7 @@ func (r *revokeReconciler) revokeOldCertificateSecrets(logctx logger.LogContext, revokedSecrets = append(revokedSecrets, ref) err = r.setCertificateSecretRevoked(secret) if err != nil { - err = errors.Wrapf(err, "updating backup certificate secret %s failed", secret.Name) + err = fmt.Errorf("updating backup certificate secret %s failed: %w", secret.Name, err) logctx.Warn(err.Error()) errs = append(errs, err) } @@ -469,7 +469,7 @@ func (r *revokeReconciler) revokeOldCertificateSecrets(logctx logger.LogContext, err = r.setCertificateSecretsRevokedBySerialNumbers(hashKey, revokedSerialNumbers) if err != nil { - err = errors.Wrap(err, "marking certificate secret as revoked failed") + err = fmt.Errorf("marking certificate secret as revoked failed: %w", err) logctx.Warn(err.Error()) errs = append(errs, err) } @@ -487,7 +487,7 @@ func (r *revokeReconciler) revokeOldCertificateSecrets(logctx logger.LogContext, // trigger all certificate objects for _, ref := range revocation.Status.Objects.Processing { key := resources.NewClusterKey(r.certResources.GetCluster().GetId(), api.Kind(api.CertificateKind), ref.Namespace, ref.Name) - r.support.EnqueueKey(key) + _ = r.support.EnqueueKey(key) } } @@ -535,7 +535,7 @@ func (r *revokeReconciler) setCertificateSecretRevoked(secret *corev1.Secret) er } func (r *revokeReconciler) setCertificateSecretsRevokedBySerialNumbers(hashKey string, serialNumbers []*big.Int) error { - list, err := certificate.FindAllCertificateSecretsByHashLabel(r.certSecretResources, hashKey) + list, err := certificate.FindAllCertificateSecretsByNewHashLabel(r.certSecretResources, hashKey) if err != nil { return err } @@ -570,7 +570,7 @@ func (r *revokeReconciler) loadSecret(secretRef *corev1.SecretReference) (*corev secret := &corev1.Secret{} _, err := r.certSecretResources.GetInto(secretObjectName, secret) if err != nil { - return nil, errors.Wrap(err, "fetching secret failed") + return nil, fmt.Errorf("fetching secret failed: %w", err) } return secret, nil @@ -580,12 +580,12 @@ func (r *revokeReconciler) updateStatusAndDelay(logctx logger.LogContext, obj re statusUpdater func(logctx logger.LogContext, obj resources.Object) (*resources.ModificationState, error)) reconcile.Status { mod, err := statusUpdater(logctx, obj) if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "statusUpdater")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("statusUpdater: %w", err)) } if mod.Modified { err := mod.UpdateStatus() if err != nil { - return r.failedStop(logctx, obj, api.StateError, errors.Wrap(err, "UpdateStatus failed")) + return r.failedStop(logctx, obj, api.StateError, fmt.Errorf("UpdateStatus failed: %w", err)) } } @@ -635,7 +635,7 @@ func (r *revokeReconciler) recheck(logger logger.LogContext, obj resources.Objec } func createLabelCertificateHashKeySelector(hash string) (labels.Selector, error) { - requirement, err := labels.NewRequirement(certificate.LabelCertificateHashKey, selection.Equals, []string{hash}) + requirement, err := labels.NewRequirement(certificate.LabelCertificateNewHashKey, selection.Equals, []string{hash}) if err != nil { return nil, err } diff --git a/test/functional/basics.go b/test/functional/basics.go index b6808f10..938cdc54 100644 --- a/test/functional/basics.go +++ b/test/functional/basics.go @@ -14,8 +14,8 @@ import ( . "github.com/onsi/gomega/gstruct" "github.com/gardener/cert-management/pkg/apis/cert/v1alpha1" - "github.com/gardener/controller-manager-library/pkg/resources" "github.com/gardener/cert-management/test/functional/config" + "github.com/gardener/controller-manager-library/pkg/resources" ) var basicTemplate = ` @@ -144,7 +144,7 @@ func functestbasics(cfg *config.Config, iss *config.IssuerConfig) { entryName(iss, "1"): MatchKeys(IgnoreExtras, Keys{ "metadata": MatchKeys(IgnoreExtras, Keys{ "labels": MatchKeys(IgnoreExtras, Keys{ - "cert.gardener.cloud/certificate-hash": HavePrefix(""), + "cert.gardener.cloud/hash": HavePrefix(""), }), }), "spec": MatchKeys(IgnoreExtras, Keys{ diff --git a/test/functional/config/config.go b/test/functional/config/config.go index 10b1563e..fb6c3e15 100644 --- a/test/functional/config/config.go +++ b/test/functional/config/config.go @@ -98,12 +98,12 @@ func LoadConfig(filename string) (*Config, error) { config := &Config{} err = decoder.Decode(config) if err != nil { - return nil, fmt.Errorf("Parsing config file %s failed with %s", filename, err) + return nil, fmt.Errorf("Parsing config file %s failed: %w", filename, err) } err = config.postProcess() if err != nil { - return nil, fmt.Errorf("Post processing config file %s failed with %s", filename, err) + return nil, fmt.Errorf("Post processing config file %s failed: %w", filename, err) } config.Utils = CreateDefaultTestUtils() diff --git a/test/functional/config/utils.go b/test/functional/config/utils.go index a2672c1a..67941b9f 100644 --- a/test/functional/config/utils.go +++ b/test/functional/config/utils.go @@ -9,10 +9,11 @@ package config import ( "encoding/json" "fmt" - "github.com/onsi/gomega" "os/exec" "strings" "time" + + "github.com/onsi/gomega" ) const STATE_DELETED = "~DELETED~" @@ -89,7 +90,7 @@ func (u *TestUtils) runCmd(cmdline string) (string, error) { out, err := cmd.Output() if err != nil { println(string(err.(*exec.ExitError).Stderr)) - return string(out), fmt.Errorf("command `%s` failed with %s", cmdline, err) + return string(out), fmt.Errorf("command `%s` failed: %w", cmdline, err) } return string(out), nil } @@ -170,7 +171,7 @@ func (u *TestUtils) AwaitWithTimeout(msg string, check CheckFunc, timeout time.D time.Sleep(u.PollingPeriod) } if err != nil { - return fmt.Errorf("Timeout during check %s with error %s", msg, err.Error()) + return fmt.Errorf("Timeout during check %s with error: %w", msg, err) } return fmt.Errorf("Timeout during check %s", msg) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 780726bd..428a9437 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -216,7 +216,6 @@ github.com/onsi/gomega/types # github.com/pelletier/go-toml v1.8.1 github.com/pelletier/go-toml # github.com/pkg/errors v0.9.1 -## explicit github.com/pkg/errors # github.com/prometheus/client_golang v1.7.1 ## explicit