feat(trigger): adding certificate request backoff duration to trigger controller by hjoshi123 · Pull Request #8312 · cert-manager/cert-manager

hjoshi123 · 2025-12-07T18:14:44Z

Pull Request Motivation

This PR addresses the bug where the default backoff duration was configured to be 1 hour and users had no ability to configure them whatsoever. This bug is also captured in the issue #8259.

Kind

/kind bug

Release Note

Added `certificateRequestMinimumBackoffDuration` to controller configuration, for users who want cert-manager to retry  failed certificate requests faster. The default remains 1 hour.

CyberArk tracker: VC-47963

hjoshi123 · 2025-12-13T23:23:26Z

@wallrj-cyberark Validated that the cert back off duration works with a minute backoff in a kind cluster.

/cc @wallrj-cyberark

wallrj-cyberark

Thanks @hjoshi123

It's looking good. I left a few comments and suggestions.

I also tested it locally and I think I could see it working, by manually "denying" the CertificateRequests using cmctl and observing the time taken for cert-manager to create a new CertificateRequest.

But it was quite confusing because cert-manager seems to log multiple messages about the when it will next attempt ....and it seemed to log the messages BEFORE I manually denied the CR using cmctl.

I deploye cert-manager built from this branch, as follows:

# values.yaml
config:
  logging:
    verbosity: 4
    format: text
  certificateRequestBackoffDuration: 10s
disableAutoApproval: true # So that I can simulate a certificate request denied failure

kind create cluster
$ make ko-deploy-certmanager KO_HELM_VALUES_FILES=$PWD/values.yaml
...

# test-resources.yaml
Version: cert-manager.io/v1
kind: Certificate
metadata:
  name: test1
spec:
  dnsNames:
    - example.com
  secretName: test1-tls
  revisionHistoryLimit: 10
  issuerRef:
    name: test1

kubectl apply -f test-resources.yaml

cmctl deny test1-1

I1218 18:01:20.146674       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:04:00.00000124 +0000 UTC m=+500.094002990" logger="cert-manager.controller" key="default/test1"
I1218 18:01:20.147362       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:04:00.000000781 +0000 UTC m=+500.094002529" logger="cert-manager.controller" key="default/test1"

I1218 18:04:00.142896       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:09:20.000001773 +0000 UTC m=+820.094003544" logger="cert-manager.controller" key="default/test1"
I1218 18:04:00.167329       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:09:20.000002455 +0000 UTC m=+820.094004218" logger="cert-manager.controller" key="default/test1"
I1218 18:04:00.174196       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:09:20.000002525 +0000 UTC m=+820.094004290" logger="cert-manager.controller" key="default/test1"

I1218 18:04:19.982770       1 trigger_controller.go:206] "Backing off from issuance due to previously failed issuance(s). Issuance will next be attempted at 2025-12-18 18:09:20.000003191 +0000 UTC m=+820.094004962" logger="cert-manager.controller" key="default/test1"

wallrj-cyberark · 2025-12-19T07:19:47Z

+
+	// CertificateRequestBackoffDuration configures the initial backoff duration
+	// when a certificate request fails.
+	CertificateRequestBackoffDuration *sharedv1alpha1.Duration `json:"certificateRequestBackoffDuration,omitempty"`


I was about to suggest documenting the default value here, but I see that none of the other fields document the defaults and the defaults are not rendered in the API docs:

https://cert-manager.io/docs/reference/api-docs/#controller.config.cert-manager.io%2fv1alpha1

It must be difficult for users to know what the defaults are, without running the controller with --help or by looking at the CLI reference docs:

https://cert-manager.io/docs/cli/controller/

_

Yeah I did think of adding it but based on the other fields not adding it, I thought of abiding by the convention

wallrj-cyberark · 2025-12-19T07:34:24Z

+		delay = initialDelay * time.Duration(math.Pow(2, float64(failedIssuanceAttempts-1)))
 	}

 	// Ensure that maximum returned delay is 32 hours


Will the maximum still be 32 hours, if I choose a much shorter initial delay? Or is there a maximum number of retries which will be hit first?

Update all these comments.

There's a comment below which talks about guarding agains "maths misuse". Any idea what that's about?

I think its about the integer overflow that happens at 2 ^ 99 and we are guarding against that. In the scenario where it overflows then delay's value would be unpredictable and hence we would want to return the default value of initialDelay but ofc it wont be 1 hour in this case since its configurable.

Also, I feel the maximum should still be the same? It will for sure hit the max retries first and then return the max delay.

wallrj-cyberark · 2025-12-23T09:20:27Z

/retest

Copilot

Pull request overview

This PR adds a configurable certificateRequestBackoffDuration field to the controller configuration, addressing issue #8259 where the backoff duration was hardcoded to 1 hour with no way for users to customize it.

Key Changes:

Added CertificateRequestMinimumBackoffDuration configuration field with a default value of 1 hour
Modified the trigger controller to use the configurable backoff duration instead of the hardcoded value
Added comprehensive test coverage for the new configurable backoff behavior

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`pkg/apis/config/controller/v1alpha1/types.go`	Adds the new `CertificateRequestMinimumBackoffDuration` field to the public API configuration struct
`internal/apis/config/controller/types.go`	Adds the internal representation of the `CertificateRequestMinimumBackoffDuration` field
`internal/apis/config/controller/v1alpha1/defaults.go`	Sets the default value for the new field to 1 hour (maintaining backward compatibility)
`internal/apis/config/controller/v1alpha1/testdata/defaults.json`	Updates test data to include the default backoff duration
`internal/apis/config/controller/v1alpha1/zz_generated.conversion.go`	Adds generated conversion code for the new field
`pkg/apis/config/controller/v1alpha1/zz_generated.deepcopy.go`	Adds generated deep copy code for the new pointer field
`pkg/controller/context.go`	Adds the `CertificateRequestMinimumBackoffDuration` field to `CertificateOptions`
`cmd/controller/app/options/options.go`	Adds CLI flag `--certificate-request-backoff-duration` for configuration
`cmd/controller/app/controller.go`	Wires the configuration value to the controller context
`pkg/controller/certificates/trigger/trigger_controller.go`	Updates the controller to use the configurable backoff duration and modifies the `shouldBackoffReissuingOnFailure` function signature
`pkg/controller/certificates/trigger/trigger_controller_test.go`	Adds test coverage for custom backoff durations and updates existing tests to use the new parameter
`test/integration/certificates/trigger_controller_test.go`	Sets the default backoff duration in integration tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T09:40:28Z

 	ACMEDNS01Config ACMEDNS01Config `json:"acmeDNS01Config,omitempty"`
+
+	// CertificateRequestMinimumBackoffDuration configures the initial backoff duration
+	// when a certificate request fails.


The comment documentation is too brief and doesn't adequately describe the behavior. It states "configures the initial backoff duration when a certificate request fails" but doesn't explain that this duration is exponentially increased based on consecutive failures or that it represents the minimum backoff. Consider revising to: "CertificateRequestMinimumBackoffDuration configures the initial backoff duration when a certificate request fails. This duration is exponentially increased (up to a maximum of 32 hours) based on the number of consecutive failures."

Suggested change

// when a certificate request fails.

// when a certificate request fails. This duration is exponentially increased

// (up to a maximum of 32 hours) based on the number of consecutive failures.

- Add controller config field for initial certificate request backoff - Add CLI flag to configure the initial backoff duration - Use configured initial delay in trigger controller backoff - Default initial delay is 1h and backoff doubles per failure up to 32h - Update defaults, conversions, controller context, and tests Signed-off-by: Hemant Joshi <mail@hjoshi.me> Co-authored-by: Richard Wall <richard.wall@cyberark.com> Signed-off-by: Richard Wall <richard.wall@cyberark.com>

hjoshi123 · 2025-12-23T15:05:05Z

/label tide/merge-method-squash

Signed-off-by: hjoshi123 <mail@hjoshi.me>

wallrj-cyberark

Thanks @hjoshi123

LGTM

wallrj · 2025-12-23T15:34:38Z

/approve
/lgtm

cert-manager-prow · 2025-12-23T15:34:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wallrj, wallrj-cyberark

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [wallrj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

… controller (cert-manager#8312) * Add configurable initial certificate request backoff - Add controller config field for initial certificate request backoff - Add CLI flag to configure the initial backoff duration - Use configured initial delay in trigger controller backoff - Default initial delay is 1h and backoff doubles per failure up to 32h - Update defaults, conversions, controller context, and tests Signed-off-by: Hemant Joshi <mail@hjoshi.me> Co-authored-by: Richard Wall <richard.wall@cyberark.com> Signed-off-by: Richard Wall <richard.wall@cyberark.com> * attempt to fix test with default backoff Signed-off-by: hjoshi123 <mail@hjoshi.me> --------- Signed-off-by: Hemant Joshi <mail@hjoshi.me> Signed-off-by: Richard Wall <richard.wall@cyberark.com> Signed-off-by: hjoshi123 <mail@hjoshi.me> Co-authored-by: Richard Wall <richard.wall@cyberark.com>

hjoshi123 force-pushed the feat/cert-backoff-duration branch from d28ee38 to e038077 Compare December 7, 2025 21:19

cert-manager-prow Bot requested a review from wallrj-cyberark December 13, 2025 23:23

wallrj-cyberark linked an issue Dec 18, 2025 that may be closed by this pull request

Request to reduce initial retry backoff for failed CertificateRequest #8259

Closed

wallrj-cyberark requested changes Dec 19, 2025

View reviewed changes

hjoshi123 force-pushed the feat/cert-backoff-duration branch from e038077 to db4bee0 Compare December 23, 2025 04:41

hjoshi123 requested a review from wallrj-cyberark December 23, 2025 04:42

hjoshi123 force-pushed the feat/cert-backoff-duration branch 2 times, most recently from 227631e to de02537 Compare December 23, 2025 05:20

cert-manager-prow Bot added the area/testing Issues relating to testing label Dec 23, 2025

wallrj requested a review from Copilot December 23, 2025 09:30

Copilot started reviewing on behalf of wallrj December 23, 2025 09:30 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

wallrj-cyberark reviewed Dec 23, 2025

View reviewed changes

Comment thread internal/apis/config/controller/v1alpha1/defaults.go Outdated

wallrj-cyberark reviewed Dec 23, 2025

View reviewed changes

Comment thread internal/apis/config/controller/v1alpha1/defaults.go Outdated

wallrj-cyberark force-pushed the feat/cert-backoff-duration branch from c207198 to 7c79b0e Compare December 23, 2025 14:28

wallrj-cyberark force-pushed the feat/cert-backoff-duration branch from 7c79b0e to a4e3f45 Compare December 23, 2025 14:49

cert-manager-prow Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 23, 2025

attempt to fix test with default backoff

5dc0e07

Signed-off-by: hjoshi123 <mail@hjoshi.me>

hjoshi123 force-pushed the feat/cert-backoff-duration branch from b810bb6 to 5dc0e07 Compare December 23, 2025 15:31

wallrj-cyberark approved these changes Dec 23, 2025

View reviewed changes

cert-manager-prow Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 23, 2025

cert-manager-prow Bot merged commit cac4f59 into cert-manager:master Dec 23, 2025
6 checks passed

wallrj-cyberark added the cybr Used by CyberArk-employed maintainers to report to line management what's being worked on. label Dec 23, 2025

bharath-b-rh mentioned this pull request Apr 6, 2026

CM-977: UPSTREAM: <carry>: Backport upstream feat to configure certificate request backoff period openshift/jetstack-cert-manager#36

Merged

phuhung273 mentioned this pull request May 8, 2026

Feature Request: allow to configure max retry backoff duration for failed CertificateRequest #8729

Closed

jasondillingham mentioned this pull request Jun 1, 2026

When performing DNS challenges, the Describe Domains interface is subject to rate limiting #8776

Open

mytreya-rh mentioned this pull request Jun 9, 2026

CM-1106: UPSTREAM: <carry>: Rebase to v1.19.5 and bump x/net to v0.55.0 for CVE fixes openshift/jetstack-cert-manager#40

Merged

lunarwhite mentioned this pull request Jun 18, 2026

feat: Add configurable CertificateRequest max backoff duration as controller option #8893

Merged

mytreya-rh mentioned this pull request Jul 22, 2026

Rebase to v1.19.6 for cert-manager-operator v1.19.1 z-stream release openshift/jetstack-cert-manager#43

Merged

mytreya-rh mentioned this pull request Jul 24, 2026

Improving Roue53 DNS Challenge Presentation to avoid repeated creation of same ChangeRecord #9066

Open

	// when a certificate request fails.
	// when a certificate request fails. This duration is exponentially increased
	// (up to a maximum of 32 hours) based on the number of consecutive failures.

Uh oh!

Conversation

hjoshi123 commented Dec 7, 2025 • edited by maelvls Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Motivation

Kind

Release Note

Uh oh!

hjoshi123 commented Dec 13, 2025

Uh oh!

wallrj-cyberark left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wallrj-cyberark Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Dec 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wallrj-cyberark Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wallrj-cyberark commented Dec 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hjoshi123 commented Dec 23, 2025

Uh oh!

wallrj-cyberark left a comment

Choose a reason for hiding this comment

Uh oh!

wallrj commented Dec 23, 2025

Uh oh!

cert-manager-prow Bot commented Dec 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hjoshi123 commented Dec 7, 2025 •

edited by maelvls

Loading

wallrj-cyberark left a comment •

edited

Loading