improve error messages when resource creation fails by SgtCoDFish · Pull Request #7928 · cert-manager/cert-manager

SgtCoDFish · 2025-08-13T14:51:40Z

Pull Request Motivation

I was creating some test certs for #7642 and observed that the error messages absolutely sucked if I tried to create a CertificateRequest which was far too large:

Warning  RequestFailed  22m (x6 over 22m)  cert-manager-certificates-request-manager  Failed to create CertificateRequest: admission webhook "webhook.cert-manager.io" denied the request: spec.request: Invalid value: []byte{...}: error decoding certificate request PEM block

This has two issues:

The []byte{...} contained tens of thousands of hex-encoded characters (e.g. 0x30) from the literal certificate request, making the actual error huge
The actual error (that the CR is too large) is removed.

This PR should add the actual error message in a few places, and will also truncate the stringified CR specifically when the CR is too large.

Kind

/kind cleanup

Release Note

Improve error messages when certificates, CRLs or private keys fail admission due to malformed or missing PEM data

hjoshi123 · 2025-08-13T23:08:27Z

Looks like some of the tests are failing because the expected and actual message is different due to the change.. but otherwise lgtm

SgtCoDFish · 2025-08-14T13:15:56Z

internal/controller/certificates/policies/checks_test.go

+							{"f:data": {
+								".": {},
+								"f:tls-combined.pem": {}


note: these changes are just fixing spaces -> tabs to stop my editor complaining about mixed indentation

erikgb

This generally looks good to me. I've added a few comments/questions, but none of them are blockers. 😉

pkg/util/pki/parse.go

internal/apis/certmanager/validation/certificaterequest.go

The underlying errors were often being hidden, or else weren't clear what was wrong. Also fixes mixed indentation in checks_test.go Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish · 2025-08-15T10:43:22Z

/cherry-pick release-1.17

cert-manager-bot · 2025-08-15T10:43:26Z

@SgtCoDFish: once the present PR merges, I will cherry-pick it on top of release-1.17 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.17

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.

SgtCoDFish · 2025-08-15T10:43:28Z

/cherry-pick release-1.18

cert-manager-bot · 2025-08-15T10:43:31Z

@SgtCoDFish: once the present PR merges, I will cherry-pick it on top of release-1.18 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.18

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.

erikgb

/lgtm
/approve

🚀

cert-manager-prow · 2025-08-15T10:49:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: erikgb

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

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

cert-manager-bot · 2025-08-15T10:54:27Z

@SgtCoDFish: #7928 failed to apply on top of branch "release-1.17":

Applying: improve error messages when resource creation fails due to malformed PEM
Using index info to reconstruct a base tree...
M	internal/apis/certmanager/validation/certificaterequest.go
M	internal/controller/certificates/policies/checks_test.go
M	pkg/controller/certificaterequests/acme/acme_test.go
M	pkg/controller/certificaterequests/ca/ca_test.go
M	pkg/controller/certificaterequests/selfsigned/selfsigned_test.go
M	pkg/controller/certificaterequests/sync_test.go
M	pkg/controller/certificates/readiness/readiness_controller_test.go
M	pkg/controller/certificatesigningrequests/acme/acme_test.go
M	pkg/controller/certificatesigningrequests/ca/ca_test.go
M	pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go
M	pkg/controller/certificatesigningrequests/venafi/venafi_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/certificatesigningrequests/venafi/venafi_test.go
Auto-merging pkg/controller/certificatesigningrequests/selfsigned/selfsigned_test.go
Auto-merging pkg/controller/certificatesigningrequests/ca/ca_test.go
Auto-merging pkg/controller/certificatesigningrequests/acme/acme_test.go
Auto-merging pkg/controller/certificates/readiness/readiness_controller_test.go
Auto-merging pkg/controller/certificaterequests/sync_test.go
Auto-merging pkg/controller/certificaterequests/selfsigned/selfsigned_test.go
Auto-merging pkg/controller/certificaterequests/ca/ca_test.go
Auto-merging pkg/controller/certificaterequests/acme/acme_test.go
CONFLICT (content): Merge conflict in pkg/controller/certificaterequests/acme/acme_test.go
Auto-merging internal/controller/certificates/policies/checks_test.go
Auto-merging internal/apis/certmanager/validation/certificaterequest.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 improve error messages when resource creation fails due to malformed PEM

Details

In response to this:

/cherry-pick release-1.17

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-bot · 2025-08-15T10:55:05Z

@SgtCoDFish: new pull request created: #7964

Details

In response to this:

/cherry-pick release-1.18

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.

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>

SgtCoDFish force-pushed the better-errs branch from 9ff128c to 4287611 Compare August 14, 2025 12:46

cert-manager-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 14, 2025

SgtCoDFish commented Aug 14, 2025

View reviewed changes

erikgb reviewed Aug 14, 2025

View reviewed changes

pkg/util/pki/parse.go Show resolved Hide resolved

internal/apis/certmanager/validation/certificaterequest.go Outdated Show resolved Hide resolved

SgtCoDFish mentioned this pull request Aug 15, 2025

Issues with new PEM maximum sizes #7506

Closed

improve error messages when resource creation fails due to malformed PEM

3ef72a8

The underlying errors were often being hidden, or else weren't clear what was wrong. Also fixes mixed indentation in checks_test.go Signed-off-by: Ashley Davis <ashley.davis@cyberark.com>

SgtCoDFish force-pushed the better-errs branch from 4287611 to 3ef72a8 Compare August 15, 2025 10:42

erikgb reviewed Aug 15, 2025

View reviewed changes

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

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

cert-manager-prow bot merged commit 48bfeeb into cert-manager:master Aug 15, 2025
6 checks passed

cert-manager-bot mentioned this pull request Aug 15, 2025

[release-1.18] improve error messages when resource creation fails #7964

Merged

SgtCoDFish deleted the better-errs branch August 15, 2025 11:16

SgtCoDFish mentioned this pull request Aug 15, 2025

[release-1.17] Improve error messages when creation fails due to malformed PEM #7965

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve error messages when resource creation fails#7928

improve error messages when resource creation fails#7928
cert-manager-prow[bot] merged 1 commit intocert-manager:masterfrom
SgtCoDFish:better-errs

SgtCoDFish commented Aug 13, 2025 •

edited

Loading

Uh oh!

hjoshi123 commented Aug 13, 2025

Uh oh!

SgtCoDFish Aug 14, 2025

Uh oh!

erikgb left a comment

Uh oh!

Uh oh!

Uh oh!

SgtCoDFish commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

SgtCoDFish commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

erikgb left a comment

Uh oh!

cert-manager-prow bot commented Aug 15, 2025

Uh oh!

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

SgtCoDFish commented Aug 13, 2025 • edited 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 Aug 13, 2025

Uh oh!

SgtCoDFish Aug 14, 2025

Choose a reason for hiding this comment

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SgtCoDFish commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

SgtCoDFish commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

cert-manager-prow bot commented Aug 15, 2025

Uh oh!

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

cert-manager-bot commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

SgtCoDFish commented Aug 13, 2025 •

edited

Loading