Skip to content

Add openapi-gen to client packages#7872

Merged
cert-manager-prow[bot] merged 2 commits intocert-manager:masterfrom
erikgb:openapi-gen
Jul 28, 2025
Merged

Add openapi-gen to client packages#7872
cert-manager-prow[bot] merged 2 commits intocert-manager:masterfrom
erikgb:openapi-gen

Conversation

@erikgb
Copy link
Copy Markdown
Member

@erikgb erikgb commented Jul 23, 2025

Pull Request Motivation

In this "small" PR, I propose to enable openapi-gen for all our client packages. The purpose of this is to get something that knows how generate the OpenAPI YAMLs fed to applyconfiguration-gen. This will generate new fake clientsets that actually work, and unblock #7869. I have tested this PR on top of the changes here, and most tests are passing. Failures seem to be caused by SSA support in the new fake client, and test updates are needed.

In addition to fix the fake clientsets, adding openapi generation makes applyconfiguration-gen generate useful Extract functions that is useful to create new apply configuration structs from the standard resource API struct.

The PR consists of two commits, where the second commit isn't strictly required. But running the generated OpenAPI v2 (Swagger) through https://editor.swagger.io/, I discovered that adding a few more packages to the openapi-gen input eliminates a lot of warnings. But it doesn't change the generated code we actually care for, so could get dropped.

The solution to generate the OpenAPI YAMLs is quite hacky and was copied from kubernetes-sigs/kueue#5940. There is an upstream issue for this, but the issue seems to be stuck: kubernetes/kubernetes#126850.

Apart from adding/updating a lot of generated code and a helper command, I had to make some minor adjustments:

  • Move the +k8s:openapi-gen=true markers to the package level in user-facing client packages. Adding them to each and every type doesn't scale IMO.
  • Add new patch-makers to the acme imagePullSecrets. It already had JSON tags for it, and the tooling forced me to do this minor change to make the build succeed. We should review all these markers in a follow-up PR, especially for array fields. The generated openapi report added in this PR gives us a list of our current violations.

Blocks #7869.

Kind

/kind cleanup

Release Note

NONE

@cert-manager-prow
Copy link
Copy Markdown
Contributor

@erikgb: 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

Kind

/kind

Release Note

NONE

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 cert-manager-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 23, 2025
@cert-manager-prow cert-manager-prow bot added the area/api Indicates a PR directly modifies the 'pkg/apis' directory label Jul 23, 2025
@erikgb erikgb force-pushed the openapi-gen branch 5 times, most recently from 5c22ecc to 2508af6 Compare July 23, 2025 21:13
@erikgb
Copy link
Copy Markdown
Member Author

erikgb commented Jul 24, 2025

/kind bug

@cert-manager-prow cert-manager-prow bot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 24, 2025
@erikgb erikgb force-pushed the openapi-gen branch 2 times, most recently from 4a55abf to 5d46a60 Compare July 24, 2025 18:32
@erikgb erikgb changed the title WIP: Add openapi-gen to client packages Add openapi-gen to client packages Jul 24, 2025
@erikgb erikgb requested a review from inteon July 24, 2025 18:32
@cert-manager-prow cert-manager-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2025
@erikgb erikgb requested a review from ThatsMrTalbot July 24, 2025 18:32
@erikgb erikgb force-pushed the openapi-gen branch 2 times, most recently from fdeeb3f to 6fd8842 Compare July 24, 2025 18:55
@erikgb erikgb changed the title Add openapi-gen to client packages WIP: Add openapi-gen to client packages Jul 25, 2025
@cert-manager-prow cert-manager-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2025
@cert-manager-prow cert-manager-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2025
@cert-manager-prow cert-manager-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 27, 2025
@erikgb erikgb force-pushed the openapi-gen branch 2 times, most recently from a6d541e to 049110a Compare July 28, 2025 11:09
@erikgb erikgb changed the title WIP: Add openapi-gen to client packages Add openapi-gen to client packages Jul 28, 2025
@cert-manager-prow cert-manager-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 28, 2025
erikgb added 2 commits July 28, 2025 13:13
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once we deal with the violations here we should add it to gitignore and create a validation target to run openapigen and validate no new violations crop up

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.

Yes, we should fix most of these violations. But I didn't want to do it in the same PR.

@ThatsMrTalbot
Copy link
Copy Markdown
Contributor

Thanks for moving the package internal - I can see us dropping this in the future if we can generate the structured merge config that powers the fake client without it.
/lgtm

@cert-manager-prow cert-manager-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2025
@erikgb
Copy link
Copy Markdown
Member Author

erikgb commented Jul 28, 2025

/approve

To allow me to continue on the fake clientsets migration.

@cert-manager-prow
Copy link
Copy Markdown
Contributor

[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:

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 Jul 28, 2025
@cert-manager-prow cert-manager-prow bot merged commit 6da1111 into cert-manager:master Jul 28, 2025
7 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/api Indicates a PR directly modifies the 'pkg/apis' directory dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants