Skip to content

Use API defaults for issuerRef fields#7907

Merged
cert-manager-prow[bot] merged 1 commit intocert-manager:masterfrom
erikgb:use-issure-ref-defaults
Aug 11, 2025
Merged

Use API defaults for issuerRef fields#7907
cert-manager-prow[bot] merged 1 commit intocert-manager:masterfrom
erikgb:use-issure-ref-defaults

Conversation

@erikgb
Copy link
Copy Markdown
Member

@erikgb erikgb commented Aug 10, 2025

Pull Request Motivation

This is follow-up PR to #7900 and #7892, which leverage the new API defaults from issuerRef fields (group and kind) in our user-facing APIs.. The defaulter-gen setup seems a bit awkward to me, as the defaulting functions for our public APIs are generated inside the internal API package, but that's how it is. There is probably a reason for it, and I have no plans to change this. But if we had defaults for the equivalent fields in our internal APIs, this could be simplified more.

I chose to invoke the defaulting function from our unit-test constructors, as this appears to be the newer way of writing tests.

Kind

/kind cleanup

Release Note

NONE

@cert-manager-prow cert-manager-prow bot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/acme Indicates a PR directly modifies the ACME Issuer code area/acme/dns01 Indicates a PR modifies ACME DNS01 provider code area/testing Issues relating to testing labels Aug 10, 2025
ExpectedEvents: []string{
//nolint: dupword
`Normal Created Created Challenge resource "testorder-756011405" for domain "test.com"`,
`Normal Created Created Challenge resource "testorder-2580184217" for domain "test.com"`,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming this had to change since the issuer group/kind is part of the order hash name?

@erikgb erikgb requested review from SgtCoDFish and inteon August 10, 2025 13:36
Copy link
Copy Markdown
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few bits - what do you think? Nothing blocking really but I think it's worth a quick check first!

@erikgb erikgb force-pushed the use-issure-ref-defaults branch from e742cd7 to caea9f8 Compare August 11, 2025 14:35
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
@erikgb erikgb force-pushed the use-issure-ref-defaults branch from caea9f8 to f1e2d77 Compare August 11, 2025 14:37
Copy link
Copy Markdown
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

Seems fine to me! Thanks 🚀

@cert-manager-prow cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2025
@cert-manager-prow
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SgtCoDFish

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:

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

@cert-manager-prow cert-manager-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2025
@cert-manager-prow cert-manager-prow bot merged commit b00d3a2 into cert-manager:master Aug 11, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/acme/dns01 Indicates a PR modifies ACME DNS01 provider code area/acme Indicates a PR directly modifies the ACME Issuer code area/testing Issues relating to testing dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants