diff --git a/pkg/controller/certificaterequests/ca/BUILD.bazel b/pkg/controller/certificaterequests/ca/BUILD.bazel index f769fb67b..312bc6ca7 100644 --- a/pkg/controller/certificaterequests/ca/BUILD.bazel +++ b/pkg/controller/certificaterequests/ca/BUILD.bazel @@ -18,8 +18,6 @@ go_library( "//pkg/util/pki:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", - "//vendor/k8s.io/client-go/tools/record:go_default_library", - "//vendor/k8s.io/utils/clock:go_default_library", ], ) diff --git a/pkg/controller/certificaterequests/ca/ca.go b/pkg/controller/certificaterequests/ca/ca.go index dcbbf1a77..d89a0a789 100644 --- a/pkg/controller/certificaterequests/ca/ca.go +++ b/pkg/controller/certificaterequests/ca/ca.go @@ -22,8 +22,6 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" corelisters "k8s.io/client-go/listers/core/v1" - "k8s.io/client-go/tools/record" - "k8s.io/utils/clock" apiutil "github.com/jetstack/cert-manager/pkg/api/util" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" @@ -42,14 +40,10 @@ const ( ) type CA struct { - // used to record Events about resources to the API - recorder record.EventRecorder - issuerOptions controllerpkg.IssuerOptions secretsLister corelisters.SecretLister - // Clock used to set constant time for testing - clock clock.Clock + reporter *crutil.Reporter } func init() { @@ -70,16 +64,14 @@ func init() { func NewCA(ctx *controllerpkg.Context) *CA { return &CA{ - recorder: ctx.Recorder, issuerOptions: ctx.IssuerOptions, secretsLister: ctx.KubeSharedInformerFactory.Core().V1().Secrets().Lister(), - clock: ctx.Clock, + reporter: crutil.NewReporter(ctx.Clock, ctx.Recorder), } } func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerObj v1alpha1.GenericIssuer) (*issuerpkg.IssueResponse, error) { log := logf.FromContext(ctx, "sign") - reporter := crutil.NewReporter(cr, c.clock, c.recorder) secretName := issuerObj.GetSpec().CA.SecretName resourceNamespace := c.issuerOptions.ResourceNamespace(issuerObj) @@ -92,7 +84,7 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerOb if k8sErrors.IsNotFound(err) { message := fmt.Sprintf("Referenced secret %s/%s not found", resourceNamespace, secretName) - reporter.Pending(err, "MissingSecret", message) + c.reporter.Pending(cr, err, "MissingSecret", message) log.Error(err, message) return nil, nil @@ -101,14 +93,14 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerOb if cmerrors.IsInvalidData(err) { message := fmt.Sprintf("Failed to parse signing CA keypair from secret %s/%s", resourceNamespace, secretName) - reporter.Pending(err, "ErrorParsingSecret", message) + c.reporter.Pending(cr, err, "ErrorParsingSecret", message) log.Error(err, message) return nil, nil } // We are probably in a network error here so we should backoff and retry message := fmt.Sprintf("Failed to get certificate key pair from secret %s/%s", resourceNamespace, secretName) - reporter.Pending(err, "ErrorGettingSecret", message) + c.reporter.Pending(cr, err, "ErrorGettingSecret", message) log.Error(err, message) return nil, err } @@ -116,7 +108,7 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerOb template, err := pki.GenerateTemplateFromCertificateRequest(cr) if err != nil { message := "Error generating certificate template" - reporter.Failed(err, "ErrorSigning", message) + c.reporter.Failed(cr, err, "ErrorSigning", message) log.Error(err, message) return nil, nil } @@ -124,7 +116,7 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerOb certPEM, caPEM, err := pki.SignCSRTemplate(caCerts, caKey, template) if err != nil { message := "Error signing certificate" - reporter.Failed(err, "ErrorSigning", message) + c.reporter.Failed(cr, err, "ErrorSigning", message) log.Error(err, message) return nil, err } diff --git a/pkg/controller/certificaterequests/controller.go b/pkg/controller/certificaterequests/controller.go index f7d4f6071..0dd845d3a 100644 --- a/pkg/controller/certificaterequests/controller.go +++ b/pkg/controller/certificaterequests/controller.go @@ -30,6 +30,7 @@ import ( cmclient "github.com/jetstack/cert-manager/pkg/client/clientset/versioned" cmlisters "github.com/jetstack/cert-manager/pkg/client/listers/certmanager/v1alpha1" controllerpkg "github.com/jetstack/cert-manager/pkg/controller" + "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util" "github.com/jetstack/cert-manager/pkg/issuer" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/metrics" @@ -73,6 +74,8 @@ type Controller struct { // used for testing clock clock.Clock + + reporter *util.Reporter } func New(issuerType string, issuer Issuer) *Controller { @@ -132,6 +135,7 @@ func (c *Controller) Register(ctx *controllerpkg.Context) (workqueue.RateLimitin c.clock = ctx.Clock // recorder records events about resources to the Kubernetes api c.recorder = ctx.Recorder + c.reporter = util.NewReporter(c.clock, c.recorder) c.cmClient = ctx.CMClient c.log.Info("new certificate request controller registered", diff --git a/pkg/controller/certificaterequests/selfsigned/BUILD.bazel b/pkg/controller/certificaterequests/selfsigned/BUILD.bazel index 76922af6a..8a0e7713e 100644 --- a/pkg/controller/certificaterequests/selfsigned/BUILD.bazel +++ b/pkg/controller/certificaterequests/selfsigned/BUILD.bazel @@ -18,8 +18,6 @@ go_library( "//pkg/util/pki:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", - "//vendor/k8s.io/client-go/tools/record:go_default_library", - "//vendor/k8s.io/utils/clock:go_default_library", ], ) diff --git a/pkg/controller/certificaterequests/selfsigned/selfsigned.go b/pkg/controller/certificaterequests/selfsigned/selfsigned.go index 201cf97bd..8ed986614 100644 --- a/pkg/controller/certificaterequests/selfsigned/selfsigned.go +++ b/pkg/controller/certificaterequests/selfsigned/selfsigned.go @@ -23,8 +23,6 @@ import ( k8sErrors "k8s.io/apimachinery/pkg/api/errors" corelisters "k8s.io/client-go/listers/core/v1" - "k8s.io/client-go/tools/record" - "k8s.io/utils/clock" apiutil "github.com/jetstack/cert-manager/pkg/api/util" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" @@ -43,14 +41,10 @@ const ( ) type SelfSigned struct { - // used to record Events about resources to the API - recorder record.EventRecorder - issuerOptions controllerpkg.IssuerOptions secretsLister corelisters.SecretLister - // Clock used to set constant time for testing - clock clock.Clock + reporter *crutil.Reporter } func init() { @@ -70,16 +64,14 @@ func init() { func NewSelfSigned(ctx *controllerpkg.Context) *SelfSigned { return &SelfSigned{ - recorder: ctx.Recorder, issuerOptions: ctx.IssuerOptions, secretsLister: ctx.KubeSharedInformerFactory.Core().V1().Secrets().Lister(), - clock: ctx.Clock, + reporter: crutil.NewReporter(ctx.Clock, ctx.Recorder), } } func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, issuerObj v1alpha1.GenericIssuer) (*issuer.IssueResponse, error) { log := logf.FromContext(ctx, "sign") - reporter := crutil.NewReporter(cr, s.clock, s.recorder) resourceNamespace := s.issuerOptions.ResourceNamespace(issuerObj) @@ -89,7 +81,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, v1alpha1.CRPrivateKeyAnnotationKey) err := errors.New("secret name missing") - reporter.Failed(err, "MissingAnnotation", message) + s.reporter.Failed(cr, err, "MissingAnnotation", message) log.Error(err, message) return nil, nil @@ -100,7 +92,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, if k8sErrors.IsNotFound(err) { message := fmt.Sprintf("Referenced secret %s/%s not found", cr.Namespace, secretName) - reporter.Pending(err, "MissingSecret", message) + s.reporter.Pending(cr, err, "MissingSecret", message) log.Error(err, message) return nil, nil @@ -110,7 +102,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, message := fmt.Sprintf("Failed to get key %q referenced in annotation %q", secretName, v1alpha1.CRPrivateKeyAnnotationKey) - reporter.Pending(err, "ErrorParsingKey", message) + s.reporter.Pending(cr, err, "ErrorParsingKey", message) log.Error(err, message) return nil, nil @@ -118,7 +110,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, // We are probably in a network error here so we should backoff and retry message := fmt.Sprintf("Failed to get certificate key pair from secret %s/%s", resourceNamespace, secretName) - reporter.Pending(err, "ErrorGettingSecret", message) + s.reporter.Pending(cr, err, "ErrorGettingSecret", message) log.Error(err, message) return nil, err } @@ -126,7 +118,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, template, err := pki.GenerateTemplateFromCertificateRequest(cr) if err != nil { message := "Error generating certificate template" - reporter.Failed(err, "ErrorGenerating", message) + s.reporter.Failed(cr, err, "ErrorGenerating", message) log.Error(err, message) return nil, nil } @@ -135,7 +127,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, publickey, err := pki.PublicKeyForPrivateKey(privatekey) if err != nil { message := "Failed to get public key from private key" - reporter.Failed(err, "ErrorPublicKey", message) + s.reporter.Failed(cr, err, "ErrorPublicKey", message) log.Error(err, message) return nil, nil } @@ -148,7 +140,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, } message := "Error generating certificate template" - reporter.Failed(err, "ErrorKeyMatch", message) + s.reporter.Failed(cr, err, "ErrorKeyMatch", message) log.Error(err, message) return nil, nil @@ -158,7 +150,7 @@ func (s *SelfSigned) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest, certPem, _, err := pki.SignCertificate(template, template, publickey, privatekey) if err != nil { message := "Error signing certificate" - reporter.Failed(err, "ErrorSigning", message) + s.reporter.Failed(cr, err, "ErrorSigning", message) log.Error(err, message) return nil, nil } diff --git a/pkg/controller/certificaterequests/sync.go b/pkg/controller/certificaterequests/sync.go index 8026d906e..efed5dd5b 100644 --- a/pkg/controller/certificaterequests/sync.go +++ b/pkg/controller/certificaterequests/sync.go @@ -32,7 +32,6 @@ import ( "github.com/jetstack/cert-manager/pkg/apis/certmanager" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" "github.com/jetstack/cert-manager/pkg/apis/certmanager/validation" - "github.com/jetstack/cert-manager/pkg/controller/certificaterequests/util" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/util/pki" ) @@ -61,13 +60,12 @@ func (c *Controller) Sync(ctx context.Context, cr *v1alpha1.CertificateRequest) } }() - reporter := util.NewReporter(crCopy, c.clock, c.recorder) - dbg.Info("fetching issuer object referenced by CertificateRequest") issuerObj, err := c.helper.GetGenericIssuer(crCopy.Spec.IssuerRef, crCopy.Namespace) if k8sErrors.IsNotFound(err) { - reporter.Pending(err, "IssuerNotFound", fmt.Sprintf("referenced %q not found", apiutil.IssuerKind(crCopy.Spec.IssuerRef))) + c.reporter.Pending(crCopy, err, "IssuerNotFound", + fmt.Sprintf("referenced %q not found", apiutil.IssuerKind(crCopy.Spec.IssuerRef))) return nil } @@ -80,7 +78,8 @@ func (c *Controller) Sync(ctx context.Context, cr *v1alpha1.CertificateRequest) issuerType, err := apiutil.NameForIssuer(issuerObj) if err != nil { - reporter.Pending(err, "IssuerTypeError", "failed to obtain referenced issuer type") + c.reporter.Pending(crCopy, err, "IssuerTypeError", + "failed to obtain referenced issuer type") return nil } @@ -96,7 +95,8 @@ func (c *Controller) Sync(ctx context.Context, cr *v1alpha1.CertificateRequest) el := validation.ValidateCertificateRequest(crCopy) if len(el) > 0 { - reporter.Failed(el.ToAggregate(), "BadConfig", "resource validation failed") + c.reporter.Failed(crCopy, el.ToAggregate(), "BadConfig", + "resource validation failed") return nil } diff --git a/pkg/controller/certificaterequests/util/reporter.go b/pkg/controller/certificaterequests/util/reporter.go index c1c49e9ee..15ab0caea 100644 --- a/pkg/controller/certificaterequests/util/reporter.go +++ b/pkg/controller/certificaterequests/util/reporter.go @@ -29,35 +29,33 @@ import ( ) type Reporter struct { - cr *v1alpha1.CertificateRequest clock clock.Clock recorder record.EventRecorder } -func NewReporter(cr *v1alpha1.CertificateRequest, clock clock.Clock, recorder record.EventRecorder) *Reporter { +func NewReporter(clock clock.Clock, recorder record.EventRecorder) *Reporter { return &Reporter{ - cr: cr, clock: clock, recorder: recorder, } } -func (r *Reporter) Failed(err error, reason, message string) { +func (r *Reporter) Failed(cr *v1alpha1.CertificateRequest, err error, reason, message string) { // Set the FailureTime to c.clock.Now(), only if it has not been already set. - if r.cr.Status.FailureTime == nil { + if cr.Status.FailureTime == nil { nowTime := metav1.NewTime(r.clock.Now()) - r.cr.Status.FailureTime = &nowTime + cr.Status.FailureTime = &nowTime } message = fmt.Sprintf("%s: %v", message, err) - r.recorder.Event(r.cr, corev1.EventTypeWarning, reason, message) - apiutil.SetCertificateRequestCondition(r.cr, v1alpha1.CertificateRequestConditionReady, + r.recorder.Event(cr, corev1.EventTypeWarning, reason, message) + apiutil.SetCertificateRequestCondition(cr, v1alpha1.CertificateRequestConditionReady, v1alpha1.ConditionFalse, v1alpha1.CertificateRequestReasonFailed, message) } -func (r *Reporter) Pending(err error, reason, message string) { +func (r *Reporter) Pending(cr *v1alpha1.CertificateRequest, err error, reason, message string) { message = fmt.Sprintf("%s: %v", message, err) - r.recorder.Event(r.cr, corev1.EventTypeNormal, reason, message) - apiutil.SetCertificateRequestCondition(r.cr, v1alpha1.CertificateRequestConditionReady, + r.recorder.Event(cr, corev1.EventTypeNormal, reason, message) + apiutil.SetCertificateRequestCondition(cr, v1alpha1.CertificateRequestConditionReady, v1alpha1.ConditionFalse, v1alpha1.CertificateRequestReasonPending, message) }