Skip to content

🌱 Fix AC generation so it builds and test all submodules#1226

Merged
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
alvaroaleman:fix
Jun 28, 2025
Merged

🌱 Fix AC generation so it builds and test all submodules#1226
k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
alvaroaleman:fix

Conversation

@alvaroaleman
Copy link
Member

Currently, we do not test if the generated applyconfiguration builds because it is in a submodule. As a result, we didn't notice that it doesn't.

This change:

  • Fixes the AC to build by commenting out the problematic fields until the generation gets fixed
  • Updated our test script to run the go test command for all folders with a go.mod file to prevent this class of issue in the future

FIxes #1219

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 27, 2025
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 27, 2025
@alvaroaleman alvaroaleman force-pushed the fix branch 4 times, most recently from a720c54 to bab0a54 Compare June 27, 2025 21:12
Currently, we do not test if the generated applyconfiguration builds
because it is in a submodule. As a result, we didn't notice that it
doesn't.

This change:
* Fixes the AC to build by commenting out the problematic fields until
  the generation gets fixed
* Updated our test script to run the `go test` command for all folders
  with a `go.mod` file to prevent this class of issue in the future
@alvaroaleman alvaroaleman changed the title 🌱 Fix AC generation to so it builds and test all submodules 🌱 Fix AC generation so it builds and test all submodules Jun 27, 2025
ExportedStruct `json:",inline"`
//
// This currently does not work
//unexportedStruct `json:",inline"`
Copy link
Member

Choose a reason for hiding this comment

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

Are we tracking this limitation and the one below somewhere? (e.g. in an ApplyConfiguration umbrella issue?)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but I will have a look into the upstream generator to see if I can fix it easily and create issues if not

@sbueringer
Copy link
Member

/lgtm
/approve

Thx!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 28, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

DetailsGit tree hash: 40e417f5e47434c4062f19a71ae05df8674ead04

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, sbueringer

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 [alvaroaleman,sbueringer]

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

@k8s-ci-robot k8s-ci-robot merged commit 0db89df into kubernetes-sigs:main Jun 28, 2025
10 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Applyconfiguration testdata doesn't build

3 participants