move ACME user registration from issuer secret to issuer status;

only delete certificate secrets if new flag --cascade-delete is set;
delay certificate reconciliation if issuer is not ready, but != error

```improvement operator
move ACME user registration from issuer secret to issuer status
```

```improvement operator
only delete certificate secrets if new flag --cascade-delete is set
```

```improvement operator
delay certificate reconciliation if issuer is not ready, but != error
```

README.md

@@ -329,6 +329,7 @@ Usage:
   cert-controller-manager [flags]
 
 Flags:
+      --cascade-delete                                     default for all controller "cascade-delete" options
       --cert-class string                                  default for all controller "cert-class" options
       --cert-target-class string                           default for all controller "cert-target-class" options
   -c, --controllers string                                 comma separated list of controllers to start (<name>,source,target,all) (default "all")
@@ -351,6 +352,7 @@ Flags:
       --ingress-cert.target-namespace string               target namespace for cross cluster generation
       --ingress-cert.targets.pool.size int                 Worker pool size for pool targets of controller ingress-cert (default: 2)
       --issuer-namespace string                            default for all controller "issuer-namespace" options
+      --issuer.cascade-delete                              If true, certificate secrets are deleted if dependent resources (certificate, ingress) are deleted
       --issuer.cert-class string                           Identifier used to differentiate responsible controllers for entries
       --issuer.default-issuer string                       name of default issuer (from default cluster)
       --issuer.default-issuer-domain-ranges string         domain range restrictions when using default issuer separated by comma

pkg/apis/cert/v1alpha1/issuer.go

@@ -19,6 +19,7 @@ package v1alpha1
 import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -58,8 +59,9 @@ type ACMESpec struct {
 }
 
 type IssuerStatus struct {
-	ObservedGeneration int64   `json:"observedGeneration,omitempty"`
-	State              string  `json:"state"`
-	Message            *string `json:"message,omitempty"`
-	Type               *string `json:"type"`
+	ObservedGeneration int64                 `json:"observedGeneration,omitempty"`
+	State              string                `json:"state"`
+	Message            *string               `json:"message,omitempty"`
+	Type               *string               `json:"type"`
+	ACME               *runtime.RawExtension `json:"acme,omitempty"`
 }

pkg/cert/legobridge/certificate.go

@@ -105,7 +105,7 @@ func Obtain(input ObtainInput) error {
 	if err != nil {
 		return err
 	}
-	nameservers := []string{"8.8.8.8", "8.8.4.4", "1.1.1.1"}
+	nameservers := []string{"8.8.8.8", "8.8.4.4"}
 	err = client.Challenge.SetDNS01Provider(provider, dns01.AddRecursiveNameservers(dns01.ParseNameservers(nameservers)))
 	if err != nil {
 		return err

pkg/cert/legobridge/reguser.go

@@ -32,9 +32,8 @@ import (
 )
 
 const (
-	KEY_EMAIL        = "email"
-	KEY_REGISTRATION = "registration"
-	KEY_PRIVATE_KEY  = "privateKey"
+	KEY_EMAIL       = "email"
+	KEY_PRIVATE_KEY = "privateKey"
 )
 
 type RegistrationUser struct {
@@ -143,19 +142,18 @@ func (u *RegistrationUser) ToSecretData() (map[string][]byte, error) {
 	if err != nil {
 		return nil, fmt.Errorf("encoding private key failed: %s", err.Error())
 	}
+	return map[string][]byte{KEY_PRIVATE_KEY: privkey, KEY_EMAIL: []byte(u.Email)}, nil
+}
+
+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 map[string][]byte{KEY_PRIVATE_KEY: privkey, KEY_REGISTRATION: reg, KEY_EMAIL: []byte(u.Email)}, nil
-}
-
-func SecretDataHasRegistration(data map[string][]byte) bool {
-	_, ok := data[KEY_REGISTRATION]
-	return ok
+	return reg, nil
 }
 
-func RegistrationUserFromSecretData(data map[string][]byte) (*RegistrationUser, error) {
+func RegistrationUserFromSecretData(registrationRaw []byte, data map[string][]byte) (*RegistrationUser, error) {
 	privkeyBytes, ok := data[KEY_PRIVATE_KEY]
 	if !ok {
 		return nil, fmt.Errorf("`%s` data not found in secret", KEY_PRIVATE_KEY)
@@ -165,12 +163,8 @@ func RegistrationUserFromSecretData(data map[string][]byte) (*RegistrationUser,
 		return nil, err
 	}
 
-	regBytes, ok := data[KEY_REGISTRATION]
-	if !ok {
-		return nil, fmt.Errorf("`%s` data not found in secret", KEY_REGISTRATION)
-	}
 	reg := &registration.Resource{}
-	err = json.Unmarshal(regBytes, reg)
+	err = json.Unmarshal(registrationRaw, reg)
 	if err != nil {
 		return nil, fmt.Errorf("unmarshalling registration json failed with %s", err.Error())
 	}

pkg/controller/issuer/acme/handler.go

@@ -83,15 +83,15 @@ func (r *acmeIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Ob
 		hash := r.support.CalcSecretHash(secret)
 		r.support.RememberIssuerSecret(obj.ObjectName(), issuer.Spec.ACME.PrivateKeySecretRef, hash)
 	}
-	if secret != nil && legobridge.SecretDataHasRegistration(secret.Data) {
-		user, err := legobridge.RegistrationUserFromSecretData(secret.Data)
+	if secret != nil && issuer.Status.ACME != nil && issuer.Status.ACME.Raw != nil {
+		user, err := legobridge.RegistrationUserFromSecretData(issuer.Status.ACME.Raw, secret.Data)
 		if err != nil {
 			return r.failedAcme(logger, obj, api.STATE_ERROR, fmt.Errorf("extracting registration user from secret failed with %s", err.Error()))
 		}
 		if user.Email != acme.Email {
 			return r.failedAcme(logger, obj, api.STATE_ERROR, fmt.Errorf("email of registration user from secret does not match %s != %s", user.Email, acme.Email))
 		}
-		return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType)
+		return r.support.SucceededAndTriggerCertificates(logger, obj, &acmeType, issuer.Status.ACME.Raw)
 	} else if secret != nil || acme.AutoRegistration {
 		var secretData map[string][]byte
 		if secret != nil {
@@ -117,14 +117,16 @@ func (r *acmeIssuerHandler) Reconcile(logger logger.LogContext, obj resources.Ob
 			r.support.RememberIssuerSecret(obj.ObjectName(), issuer.Spec.ACME.PrivateKeySecretRef, hash)
 		}
 
-		issuer.Status.State = api.STATE_READY
-		issuer.Status.Message = nil
+		regRaw, err := user.RawRegistration()
+		if err != nil {
+			return r.failedAcme(logger, obj, api.STATE_ERROR, fmt.Errorf("registration marshalling failed with %s", err.Error()))
+		}
 		newObj, err := r.support.GetIssuerResources().Update(issuer)
 		if err != nil {
 			return r.failedAcme(logger, obj, api.STATE_ERROR, fmt.Errorf("updating resource failed with %s", err.Error()))
 		}
 
-		return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType)
+		return r.support.SucceededAndTriggerCertificates(logger, newObj, &acmeType, regRaw)
 	} else {
 		return r.failedAcme(logger, obj, api.STATE_ERROR, fmt.Errorf("neither `SecretRef` or `AutoRegistration: true` provided"))
 	}

pkg/controller/issuer/certificate/reconciler.go

@@ -86,6 +86,7 @@ func CertReconciler(c controller.Interface, support *core.Support) (reconcile.In
 	if dnsOwnerId != "" {
 		reconciler.dnsOwnerId = &dnsOwnerId
 	}
+	reconciler.cascadeDelete, _ = c.GetBoolOption(core.OptCascadeDelete)
 
 	renewalWindow, err := c.GetDurationOption(core.OptRenewalWindow)
 	if err != nil {
@@ -96,6 +97,19 @@ func CertReconciler(c controller.Interface, support *core.Support) (reconcile.In
 	return reconciler, nil
 }
 
+type recoverableError struct {
+	Msg string
+}
+
+func (err *recoverableError) Error() string {
+	return err.Msg
+}
+
+func isRecoverableError(err error) bool {
+	_, ok := err.(*recoverableError)
+	return ok
+}
+
 type certReconciler struct {
 	reconcile.DefaultReconciler
 	support             *core.Support
@@ -111,6 +125,7 @@ type certReconciler struct {
 	renewalWindow       time.Duration
 	renewalCheckPeriod  time.Duration
 	classes             *controller.Classes
+	cascadeDelete       bool
 }
 
 func (r *certReconciler) Start() {
@@ -286,16 +301,22 @@ func (r *certReconciler) restoreRegUser(crt *api.Certificate) (*legobridge.Regis
 		return nil, "", fmt.Errorf("missing secret ref in issuer")
 	}
 	if issuer.Status.State != api.STATE_READY {
+		if issuer.Status.State != api.STATE_ERROR {
+			return nil, "", &recoverableError{fmt.Sprintf("referenced issuer not ready: state=%s", issuer.Status.State)}
+		}
 		return nil, "", fmt.Errorf("referenced issuer not ready: state=%s", issuer.Status.State)
 	}
+	if issuer.Status.ACME == nil || issuer.Status.ACME.Raw == nil {
+		return nil, "", fmt.Errorf("ACME registration missing in status")
+	}
 	issuerSecretObjectName := resources.NewObjectName(secretRef.Namespace, secretRef.Name)
 	issuerSecret := &corev1.Secret{}
 	_, err = r.support.GetIssuerSecretResources().GetInto(issuerSecretObjectName, issuerSecret)
 	if err != nil {
 		return nil, "", fmt.Errorf("fetching issuer secret failed with %s", err.Error())
 	}
 
-	reguser, err := legobridge.RegistrationUserFromSecretData(issuerSecret.Data)
+	reguser, err := legobridge.RegistrationUserFromSecretData(issuer.Status.ACME.Raw, issuerSecret.Data)
 	if err != nil {
 		return nil, "", fmt.Errorf("restoring registration issuer from issuer secret failed with %s", err.Error())
 	}
@@ -489,13 +510,15 @@ func (r *certReconciler) writeCertificateSecret(objectMeta metav1.ObjectMeta, ce
 	}
 	secret.Labels = map[string]string{LabelCertificateHashKey: specHash, LabelCertificateKey: "true"}
 	secret.Data = legobridge.CertificatesToSecretData(certificates)
-	ownerReferences := []metav1.OwnerReference{{APIVersion: api.Version, Kind: api.CertificateKind, Name: objectMeta.GetName(), UID: objectMeta.GetUID()}}
-	if objectMeta.GetAnnotations() != nil && objectMeta.GetAnnotations()[source.ANNOT_FORWARD_OWNER_REFS] == "true" {
-		if objectMeta.OwnerReferences != nil {
-			ownerReferences = append(ownerReferences, objectMeta.OwnerReferences...)
+	if r.cascadeDelete {
+		ownerReferences := []metav1.OwnerReference{{APIVersion: api.Version, Kind: api.CertificateKind, Name: objectMeta.GetName(), UID: objectMeta.GetUID()}}
+		if objectMeta.GetAnnotations() != nil && objectMeta.GetAnnotations()[source.ANNOT_FORWARD_OWNER_REFS] == "true" {
+			if objectMeta.OwnerReferences != nil {
+				ownerReferences = append(ownerReferences, objectMeta.OwnerReferences...)
+			}
 		}
+		secret.SetOwnerReferences(ownerReferences)
 	}
-	secret.SetOwnerReferences(ownerReferences)
 
 	obj, err := r.targetCluster.Resources().CreateOrUpdateObject(secret)
 	if err != nil {
@@ -585,6 +608,9 @@ func (r *certReconciler) failed(logger logger.LogContext, obj resources.Object,
 	mod, _ := r.prepareUpdateStatus(obj, state, &msg)
 	r.updateStatus(mod)
 
+	if isRecoverableError(err) {
+		return reconcile.Delay(logger, err)
+	}
 	return reconcile.Failed(logger, err)
 }
 

pkg/controller/issuer/controller.go

@@ -38,6 +38,7 @@ func init() {
 		StringOption(core.OptDefaultIssuerDomainRanges, "domain range restrictions when using default issuer separated by comma").
 		StringOption(core.OptDNSNamespace, "namespace for creating challenge DNSEntries (in DNS cluster)").
 		StringOption(core.OptDNSOwnerId, "ownerId for creating challenge DNSEntries").
+		BoolOption(core.OptCascadeDelete, "If true, certificate secrets are deleted if dependent resources (certificate, ingress) are deleted").
 		StringOption(source.OPT_CLASS, "Identifier used to differentiate responsible controllers for entries").
 		DefaultedDurationOption(core.OptRenewalWindow, 30*24*time.Hour, "certificate is renewed if its validity period is shorter").
 		FinalizerDomain(cert.GroupName).

pkg/controller/issuer/core/const.go

@@ -23,4 +23,5 @@ const (
 	OptDNSOwnerId                = "dns-owner-id"
 	OptDefaultIssuerDomainRanges = "default-issuer-domain-ranges"
 	OptRenewalWindow             = "renewal-window"
+	OptCascadeDelete             = "cascade-delete"
 )

pkg/controller/issuer/core/support.go

@@ -17,13 +17,14 @@
 package core
 
 import (
+	"bytes"
 	"crypto/sha256"
 	"fmt"
-	"sort"
-	"strings"
-
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sort"
+	"strings"
 
 	api "github.com/gardener/cert-management/pkg/apis/cert/v1alpha1"
 	"github.com/gardener/cert-management/pkg/cert/legobridge"
@@ -274,13 +275,20 @@ func (s *Support) Failed(logger logger.LogContext, obj resources.Object, state s
 	return reconcile.Failed(logger, err)
 }
 
-func (s *Support) SucceededAndTriggerCertificates(logger logger.LogContext, obj resources.Object, itype *string) reconcile.Status {
+func (s *Support) SucceededAndTriggerCertificates(logger logger.LogContext, obj resources.Object, itype *string, regRaw []byte) reconcile.Status {
 	s.triggerCertificates(logger, obj.ObjectName())
-	return s.Succeeded(logger, obj, itype, nil)
-}
 
-func (s *Support) Succeeded(logger logger.LogContext, obj resources.Object, itype *string, msg *string) reconcile.Status {
-	mod, _ := s.prepareUpdateStatus(obj, api.STATE_READY, itype, msg)
+	mod, status := s.prepareUpdateStatus(obj, api.STATE_READY, itype, nil)
+	changedRegistration := false
+	if status.ACME == nil || status.ACME.Raw == nil {
+		changedRegistration = regRaw != nil
+	} else {
+		changedRegistration = !bytes.Equal(status.ACME.Raw, regRaw)
+	}
+	if changedRegistration {
+		status.ACME = &runtime.RawExtension{Raw: regRaw}
+		mod.Modify(true)
+	}
 	s.updateStatus(mod)
 
 	return reconcile.Succeeded(logger)