feat(rfc2136): new protocol field for dns update by hjoshi123 · Pull Request #7881 · cert-manager/cert-manager

hjoshi123 · 2025-07-26T04:45:14Z

Pull Request Motivation

Kind

/kind feature

Release Note

added `protocol` field for `rfc2136` DNS01 provider

hjoshi123 · 2025-07-26T04:46:19Z

/ok-to-test

pkg/apis/acme/v1/types_issuer.go

internal/apis/acme/types_issuer.go

pkg/issuer/acme/dns/rfc2136/rfc2136.go

test/integration/rfc2136_dns01/rfc2136_test.go

pkg/issuer/acme/dns/rfc2136/rfc2136.go

erikgb

Thanks for following up on this, @hjoshi123! Looks good, and I think this is close to LGTM from me. I just did a more detailed review and added a few nit comments.

/approve

internal/apis/acme/types_issuer.go

pkg/apis/acme/v1/types_issuer.go

erikgb · 2025-08-06T16:48:39Z

pkg/issuer/acme/dns/rfc2136/rfc2136.go

+func WithNetwork(protocol string) ProviderOption {
+	return func(d *DNSProvider) {
+		if protocol == "" {
+			protocol = strings.ToLower(string(cmacme.RFC2136UpdateProtocolUDP))
+		}
+		d.network = strings.ToLower(protocol)
+	}
+}


Suggested change

func WithNetwork(protocol string) ProviderOption {

return func(d *DNSProvider) {

if protocol == "" {

protocol = strings.ToLower(string(cmacme.RFC2136UpdateProtocolUDP))

}

d.network = strings.ToLower(protocol)

}

}

func WithNetwork(network string) ProviderOption {

return func(d *DNSProvider) {

if network == "" {

network = "udp"

}

d.network = strings.ToLower(network)

}

}

We could also move the strings.ToLower conversion outside this function, to avoid having 3 representations of the same thing. 😉

pkg/issuer/acme/dns/rfc2136/rfc2136.go

test/acme/server/server.go

test/integration/rfc2136_dns01/rfc2136_test.go

cert-manager-prow · 2025-08-06T16:55:56Z

[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

test/integration/rfc2136_dns01/rfc2136_test.go

hjoshi123 · 2025-08-06T22:03:14Z

@erikgb thank you for those nits.. changed it and we should be good now.. Would really love to shadow you for a code review so that I can help out in those too

Signed-off-by: hjoshi123 <mail@hjoshi.me> Simplify options pattern use Signed-off-by: Erik Godding Boye <egboye@gmail.com> Signed-off-by: hjoshi123 <mail@hjoshi.me>

erikgb

Thanks @hjoshi123! 🚀 I think this looks good now! Hopefully some of the requesters of this feature can test it when available in an alpha release of cert-manager 1.19.

/lgtm

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:54:28Z

@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 ok-to-test label Jul 26, 2025

hjoshi123 force-pushed the feat/protocol-dns-update branch from 051171b to 2579ad7 Compare July 28, 2025 13:33

erikgb reviewed Aug 1, 2025

View reviewed changes

pkg/apis/acme/v1/types_issuer.go Outdated Show resolved Hide resolved

erikgb reviewed Aug 1, 2025

View reviewed changes

internal/apis/acme/types_issuer.go Outdated Show resolved Hide resolved

erikgb reviewed Aug 1, 2025

View reviewed changes

pkg/issuer/acme/dns/rfc2136/rfc2136.go Outdated Show resolved Hide resolved

hjoshi123 force-pushed the feat/protocol-dns-update branch 2 times, most recently from ce8ed40 to ee33e41 Compare August 2, 2025 23:01

hjoshi123 requested a review from erikgb August 2, 2025 23:27

erikgb reviewed Aug 3, 2025

View reviewed changes

test/integration/rfc2136_dns01/rfc2136_test.go Show resolved Hide resolved

erikgb reviewed Aug 3, 2025

View reviewed changes

pkg/issuer/acme/dns/rfc2136/rfc2136.go Outdated Show resolved Hide resolved

hjoshi123 force-pushed the feat/protocol-dns-update branch 3 times, most recently from be42f14 to 3dbfc6b Compare August 5, 2025 17:52

hjoshi123 requested a review from erikgb August 6, 2025 13:35

erikgb reviewed Aug 6, 2025

View reviewed changes

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

erikgb reviewed Aug 6, 2025

View reviewed changes

test/integration/rfc2136_dns01/rfc2136_test.go Outdated Show resolved Hide resolved

hjoshi123 force-pushed the feat/protocol-dns-update branch from 3dbfc6b to 8c29cb6 Compare August 6, 2025 22:02

added protocol field and codegen

945f656

Signed-off-by: hjoshi123 <mail@hjoshi.me> Simplify options pattern use Signed-off-by: Erik Godding Boye <egboye@gmail.com> Signed-off-by: hjoshi123 <mail@hjoshi.me>

hjoshi123 force-pushed the feat/protocol-dns-update branch from 8c29cb6 to 945f656 Compare August 6, 2025 22:09

erikgb reviewed Aug 7, 2025

View reviewed changes

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

cert-manager-prow bot merged commit 466a31e into cert-manager:master Aug 7, 2025
6 checks passed

Conversation

hjoshi123 commented Jul 26, 2025

Pull Request Motivation

Kind

Release Note

Uh oh!

hjoshi123 commented Jul 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erikgb Aug 6, 2025 • 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!

Uh oh!

Uh oh!

cert-manager-prow bot commented Aug 6, 2025

Uh oh!

Uh oh!

hjoshi123 commented Aug 6, 2025

Uh oh!

erikgb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wallrj-cyberark commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

erikgb Aug 6, 2025 •

edited

Loading