Update module github.com/cert-manager/cert-manager to v1.20.1#445
Conversation
|
/cc @hjoshi123 @inteon |
There was a problem hiding this comment.
Pull request overview
This PR updates cmctl’s cert-manager dependency to v1.20.0 and extends the internal ACME API/conversion layer so the new Azure DNS zoneType field round-trips through older ACME API versions.
Changes:
- Bump
github.com/cert-manager/cert-managertov1.20.0(and associated indirect deps). - Add
zoneTypeto ACME Issuer AzureDNS structs inv1alpha2,v1alpha3, andv1beta1internal APIs. - Update generated ACME conversion functions to copy
zoneTypebetween versioned types and the internal hub type.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/convert/internal/apis/acme/v1beta1/zz_generated.conversion.go | Copy ZoneType during v1beta1 ↔ internal ACME conversions. |
| pkg/convert/internal/apis/acme/v1beta1/types_issuer.go | Add AzureZoneType and ZoneType field to v1beta1 AzureDNS provider type. |
| pkg/convert/internal/apis/acme/v1alpha3/zz_generated.conversion.go | Copy ZoneType during v1alpha3 ↔ internal ACME conversions. |
| pkg/convert/internal/apis/acme/v1alpha3/types_issuer.go | Add AzureZoneType and ZoneType field to v1alpha3 AzureDNS provider type. |
| pkg/convert/internal/apis/acme/v1alpha2/zz_generated.conversion.go | Copy ZoneType during v1alpha2 ↔ internal ACME conversions. |
| pkg/convert/internal/apis/acme/v1alpha2/types_issuer.go | Add AzureZoneType and ZoneType field to v1alpha2 AzureDNS provider type. |
| pkg/convert/internal/apis/acme/v1/zz_generated.conversion.go | Copy ZoneType during cert-manager v1 ↔ internal ACME conversions. |
| pkg/convert/internal/apis/acme/types_issuer.go | Add AzureZoneType and ZoneType to the internal ACME hub type. |
| go.mod | Bump cert-manager to v1.20.0 and refresh some indirect requirements. |
| go.sum | Update module checksums consistent with the dependency bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Mostly LGTM but had a minor comment |
No, I don't think it's required to add more testing. The fuzz-tests really cover a lot. 😅 |
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
|
LGTM but probably Tim should approve it too since he requested changes |
|
Thanks @hjoshi123! I have fixed the (valid) remark from @inteon now, and if you agree to the simple approach taken here, we should be good to merge. We need to upgrade the cert-manager dependency and should probably cut a new release soonish. 🤠 /approve |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Yes, given the change requested was simple enough, I feel approval is valid :) |
This PR extends #434 by adding the new Azure DNS
zoneTypefield to older API versions. This appears to be the correct/simplest thing to do, but please let me know if another solution is preferred.Closes #434