feat(metrics): adding `certmanager_certificate_challenge_status` metric by hjoshi123 · Pull Request #7736 · cert-manager/cert-manager

hjoshi123 · 2025-05-06T16:52:31Z

Pull Request Motivation

feature

Fixes #7700. This PR adds the new challenge status metric with a controller which monitors the challenges.

/kind

Release Note

Added `certmanager_certificate_challenge_status` Prometheus metric.

cert-manager-prow · 2025-05-06T16:52:34Z

@hjoshi123: The label(s) kind/release, kind/note cannot be applied, because the repository doesn't have them.

Details

In response to this:

Pull Request Motivation

Feature

#7700. This PR adds the new challenge status metric with a controller which monitors the challenges.

/kind

Release Note
Added `certmanager_certificate_challenge_status` prometheus metric.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

cert-manager-prow · 2025-05-06T16:52:42Z

Hi @hjoshi123. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

hjoshi123 · 2025-05-06T16:53:10Z

/kind feature

pkg/metrics/acme.go

pkg/metrics/metrics.go

ThatsMrTalbot · 2025-05-15T19:31:15Z

/ok-to-test

pkg/controller/acmechallenges/metrics/controller.go

pkg/metrics/acme.go

hjoshi123 · 2025-05-20T16:48:54Z

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

ThatsMrTalbot · 2025-05-20T19:05:00Z

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

Age should not be a label, age is a constantly changing value, having it as a label would introduce high cardinality

hjoshi123 · 2025-05-20T19:48:03Z

@ThatsMrTalbot there was also talk of adding age of challenge as a label.. I was thinking of a different. metric for that since this metric is only supposed to tell the status.. what's your take on that?

Age should not be a label, age is a constantly changing value, having it as a label would introduce high cardinality

Ah yes I didnt think of that.. so how would we solve it if we wanted to? it doesnt have to be this PR I am guessing but in general.

ThatsMrTalbot · 2025-05-21T20:26:01Z

Ah yes I didnt think of that.. so how would we solve it if we wanted to? it doesnt have to be this PR I am guessing but in general.

The way I have seen time metrics work in the past is having a metric that contains a unix timestamp, for example kube_cronjob_status_last_schedule_time in kube-state-metric. These kind of metrics can be used with the time function to get relative time (time() - metric value).

ThatsMrTalbot · 2025-05-21T20:34:06Z

The code looks good, I want to run it locally, make sure it behaves as the code reads.

hjoshi123 · 2025-07-06T17:53:16Z

Code looks great now - can you squash your commits before I approve

@ThatsMrTalbot looks like I messed up while squashing commits through git rebase -i. Do you think its better to reopen a different PR?

erikgb · 2025-07-06T19:38:37Z

Should be no need to create a new PR. You just need som git-fu. 😉

This recipe usually works well for me. Assuming your fork is the origin remote and upstream is the original cert-manager remote.

git fetch --all
git checkout your-branch
git reset --hard upstream/master
git merge --squash origin/your-branch

Now check that all eventual conflicts are resolved and the changeset in your working area looks good. Then just commit and force push.

git commit -am "your-commit-message"
git push -f

Before the last step, ensure your commit looks good. Good luck! 👍

cert-manager-prow · 2025-07-07T00:12:18Z

@hjoshi123: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cert-manager-master-e2e-v1-32	`a2b366c`	link	true	`/test pull-cert-manager-master-e2e-v1-32`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

hjoshi123 · 2025-07-07T13:24:56Z

Thank you @erikgb @ThatsMrTalbot for the git-fu suggestions. The PR should be ready to merge now. Let me know if there are some final changes needed.

ThatsMrTalbot · 2025-07-07T16:04:54Z

test/integration/certificates/metrics_controller_test.go

 func TestMetricsController(t *testing.T) {
 	config, stopFn := framework.RunControlPlane(t)
-	t.Cleanup(stopFn)
+	defer stopFn()


Why has t.Cleanup been changed to defer?

Oh good point.. I think this was one of the merge errors I had and forgot to clean that up (because my base branch was before the master had this code I guess). Will quickly amend it now

ThatsMrTalbot · 2025-07-07T20:52:23Z

test/integration/certificates/metrics_controller_test.go

 		}
 	}()
 	defer func() {
-		shutdownCtx, cancel := context.WithTimeout(context.WithoutCancel(t.Context()), time.Second*5)


Another thing that looks to have changed - was this intentional or part of the merge fun?

oh missed this.. yup seems to be part of the merge fun.. fixing it.. sorry for the minor mistakes 😅

Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>

ThatsMrTalbot · 2025-07-08T09:49:39Z

Thanks for your persistence on this!

/lgtm
/approve

cert-manager-prow · 2025-07-08T09:49:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ThatsMrTalbot

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~~ [ThatsMrTalbot]

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

hjoshi123 · 2025-07-08T11:38:09Z

Thank you @ThatsMrTalbot for helping me out and briefing me on things. Also thank you @erikgb

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [cert-manager](https://cert-manager.io) ([source](https://github.com/cert-manager/cert-manager)) | minor | `v1.18.2` -> `v1.19.0` | --- ### Release Notes <details> <summary>cert-manager/cert-manager (cert-manager)</summary> ### [`v1.19.0`](https://github.com/cert-manager/cert-manager/releases/tag/v1.19.0) [Compare Source](cert-manager/cert-manager@v1.18.2...v1.19.0) cert-manager is the easiest way to automatically manage certificates in Kubernetes and OpenShift clusters. This release focuses on expanding platform compatibility, improving deployment flexibility, enhancing observability, and addressing key reliability issues. > 📖 Read the full release notes at cert-manager.io: <https://cert-manager.io/docs/releases/release-notes/release-notes-1.19> Changes since `v1.18.0`: #### Feature - Add IPv6 rules to the default network policy ([#7726](cert-manager/cert-manager#7726), [@jcpunk](https://github.com/jcpunk)) - Add `global.nodeSelector` to helm chart to allow for a single `nodeSelector` to be set across all services. ([#7818](cert-manager/cert-manager#7818), [@StingRayZA](https://github.com/StingRayZA)) - Add a feature gate to default to Ingress `pathType` `Exact` in ACME HTTP01 Ingress challenge solvers. ([#7795](cert-manager/cert-manager#7795), [@sspreitzer](https://github.com/sspreitzer)) - Add generated `applyconfigurations` allowing clients to make type-safe server-side apply requests for cert-manager resources. ([#7866](cert-manager/cert-manager#7866), [@erikgb](https://github.com/erikgb)) - Added API defaults to issuer references group (cert-manager.io) and kind (Issuer). ([#7414](cert-manager/cert-manager#7414), [@erikgb](https://github.com/erikgb)) - Added `certmanager_certificate_challenge_status` Prometheus metric. ([#7736](cert-manager/cert-manager#7736), [@hjoshi123](https://github.com/hjoshi123)) - Added `protocol` field for `rfc2136` DNS01 provider ([#7881](cert-manager/cert-manager#7881), [@hjoshi123](https://github.com/hjoshi123)) - Added experimental field `hostUsers` flag to all pods. Not set by default. ([#7973](cert-manager/cert-manager#7973), [@hjoshi123](https://github.com/hjoshi123)) - Support configurable resource requests and limits for ACME HTTP01 solver pods through ClusterIssuer and Issuer specifications, allowing granular resource management that overrides global `--acme-http01-solver-resource-*` settings. ([#7972](cert-manager/cert-manager#7972), [@lunarwhite](https://github.com/lunarwhite)) - The `CAInjectorMerging` feature has been promoted to BETA and is now enabled by default ([#8017](cert-manager/cert-manager#8017), [@ThatsMrTalbot](https://github.com/ThatsMrTalbot)) - The controller, webhook and ca-injector now log their version and git commit on startup for easier debugging and support. ([#8072](cert-manager/cert-manager#8072), [@prasad89](https://github.com/prasad89)) - Updated `certificate` metrics to the collector approach. ([#7856](cert-manager/cert-manager#7856), [@hjoshi123](https://github.com/hjoshi123)) #### Bug or Regression - ACME: Increased challenge authorization timeout to 2 minutes to fix `error waiting for authorization` ([#7796](cert-manager/cert-manager#7796), [@hjoshi123](https://github.com/hjoshi123)) - BUGFIX: permitted URI domains were incorrectly used to set the excluded URI domains in the CSR's name constraints ([#7816](cert-manager/cert-manager#7816), [@kinolaev](https://github.com/kinolaev)) - Enforced ACME HTTP-01 solver validation to properly reject configurations when multiple ingress options (`class`, `ingressClassName`, `name`) are specified simultaneously ([#8021](cert-manager/cert-manager#8021), [@lunarwhite](https://github.com/lunarwhite)) - Increase maximum sizes of PEM certificates and chains which can be parsed in cert-manager, to handle leaf certificates with large numbers of DNS names or other identities ([#7961](cert-manager/cert-manager#7961), [@SgtCoDFish](https://github.com/SgtCoDFish)) - Reverted adding the `global.rbac.disableHTTPChallengesRole` Helm option. ([#7836](cert-manager/cert-manager#7836), [@inteon](https://github.com/inteon)) - This change removes the `path` label of core ACME client metrics and will require users to update their monitoring dashboards and alerting rules if using those metrics. ([#8109](cert-manager/cert-manager#8109), [@mladen-rusev-cyberark](https://github.com/mladen-rusev-cyberark)) - Use the latest version of `ingress-nginx` in E2E tests to ensure compatibility ([#7792](cert-manager/cert-manager#7792), [@wallrj](https://github.com/wallrj)) #### Other (Cleanup or Flake) - Helm: Fix naming template of `tokenrequest` RoleBinding resource to improve consistency ([#7761](cert-manager/cert-manager#7761), [@lunarwhite](https://github.com/lunarwhite)) - Improve error messages when certificates, CRLs or private keys fail admission due to malformed or missing PEM data ([#7928](cert-manager/cert-manager#7928), [@SgtCoDFish](https://github.com/SgtCoDFish)) - Major upgrade of Akamai SDK. NOTE: The new version has not been fully tested end-to-end due to the lack of cloud infrastructure. ([#8003](cert-manager/cert-manager#8003), [@hjoshi123](https://github.com/hjoshi123)) - Update kind images to include the Kubernetes 1.33 node image ([#7786](cert-manager/cert-manager#7786), [@wallrj](https://github.com/wallrj)) - Use `maps.Copy` for cleaner map handling ([#8092](cert-manager/cert-manager#8092), [@quantpoet](https://github.com/quantpoet)) - Vault: Migrate Vault E2E add-on tests from deprecated `vault-client-go` to the new `vault/api` client. ([#8059](cert-manager/cert-manager#8059), [@armagankaratosun](https://github.com/armagankaratosun)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1711 Co-authored-by: Renovate Bot <renovate-bot@alexlebens.net> Co-committed-by: Renovate Bot <renovate-bot@alexlebens.net>

wallrj-cyberark · 2025-10-15T15:53:25Z

@hjoshi123 We have released this. Please test and feedback: https://github.com/cert-manager/cert-manager/releases/tag/v1.19.1

cert-manager-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 6, 2025

cert-manager-prow bot added kind/feature Categorizes issue or PR as related to a new feature. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels May 6, 2025

ThatsMrTalbot reviewed May 15, 2025

View reviewed changes

pkg/metrics/acme.go Outdated Show resolved Hide resolved

erikgb reviewed May 15, 2025

View reviewed changes

pkg/metrics/metrics.go Outdated Show resolved Hide resolved

cert-manager-prow bot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 15, 2025

ThatsMrTalbot reviewed May 20, 2025

View reviewed changes

pkg/controller/acmechallenges/metrics/controller.go Outdated Show resolved Hide resolved

ThatsMrTalbot reviewed May 20, 2025

View reviewed changes

pkg/metrics/acme.go Outdated Show resolved Hide resolved

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from 2812dfc to 501a401 Compare May 20, 2025 14:47

cert-manager-prow bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels May 20, 2025

cert-manager-prow bot added kind/design Categorizes issue or PR as related to design. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 6, 2025

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from d159660 to bc2bbe3 Compare July 6, 2025 22:05

cert-manager-prow bot added area/ca Indicates a PR directly modifies the CA Issuer code area/vault Indicates a PR directly modifies the Vault Issuer code and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 6, 2025

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch 2 times, most recently from 381b889 to 35841d5 Compare July 6, 2025 22:17

cert-manager-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 6, 2025

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from 35841d5 to f467393 Compare July 6, 2025 23:43

ThatsMrTalbot reviewed Jul 7, 2025

View reviewed changes

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from f467393 to 002e2bd Compare July 7, 2025 16:52

ThatsMrTalbot reviewed Jul 7, 2025

View reviewed changes

added collector for cert challenge and unit, integrationt test

2558e46

Signed-off-by: hjoshi123 <hemant.joshi@vizio.com>

hjoshi123 force-pushed the feat/cert-challenge-status-metrics branch from 002e2bd to 2558e46 Compare July 7, 2025 21:15

cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 8, 2025

cert-manager-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2025

cert-manager-prow bot merged commit cc4b6f3 into cert-manager:master Jul 8, 2025
6 checks passed

hjoshi123 mentioned this pull request Jul 24, 2025

REQUEST: New membership for hjoshi123 cert-manager/community#52

Closed

7 tasks

Conversation

hjoshi123 commented May 6, 2025 • edited by wallrj-cyberark Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Motivation

feature

Release Note

Uh oh!

cert-manager-prow bot commented May 6, 2025

Pull Request Motivation

Feature

Release Note

Uh oh!

cert-manager-prow bot commented May 6, 2025

Uh oh!

hjoshi123 commented May 6, 2025

Uh oh!

Uh oh!

Uh oh!

ThatsMrTalbot commented May 15, 2025

Uh oh!

Uh oh!

Uh oh!

hjoshi123 commented May 20, 2025

Uh oh!

ThatsMrTalbot commented May 20, 2025

Uh oh!

hjoshi123 commented May 20, 2025

Uh oh!

ThatsMrTalbot commented May 21, 2025

Uh oh!

ThatsMrTalbot commented May 21, 2025

Uh oh!

hjoshi123 commented Jul 6, 2025

Uh oh!

erikgb commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cert-manager-prow bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hjoshi123 commented Jul 7, 2025

Uh oh!

ThatsMrTalbot Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

ThatsMrTalbot Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

hjoshi123 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

ThatsMrTalbot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cert-manager-prow bot commented Jul 8, 2025

Uh oh!

Uh oh!

hjoshi123 commented Jul 8, 2025

Uh oh!

wallrj-cyberark commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hjoshi123 commented May 6, 2025 •

edited by wallrj-cyberark

Loading

erikgb commented Jul 6, 2025 •

edited

Loading

cert-manager-prow bot commented Jul 7, 2025 •

edited

Loading

ThatsMrTalbot commented Jul 8, 2025 •

edited

Loading