✨ Webhooks for external APIs

[fischor]

This is a WIP branch. But looking for feedback already.

With this PR scaffolding for core resource types will be added. Addresses #2141.

This will currently only work with core resources, not external resources per se, since a flag to describe the external Go module is missing in this PR as of now.
Currently the changes in this PR rely of this map of groups here to get the external path set if its a core resource:

kubebuilder/pkg/plugins/golang/options.go

Line 118 in 662f78f

if domain, found := coreGroups[res.Group]; found {

Adding a flag like --module or resource-pkg-path would make this work with general external resources.

This flag could be added as part of the Golang plugin flag set or as part of the global kubebuilder flag set.
I am unsure if adding this flag to the global kubebuilder flag set might break some plugins that never assumed they will ever have to deal with external plugins.
So adding this flag to the global kubebuilder flag set would need further discussion I think.

I also had to change the SetupWithManager method that is as of now a receiver method on the resource struct type since one can not define methods on external struct types in Go.
At the moment it looks like this:

// SetupWebhookWithManager will setup the manager to manage the webhooks.
func SetupWebhookForJobWithManager(mgr ctrl.Manager) error {
	return ctrl.NewWebhookManagedBy(mgr).
		For(&batchv1.Job{}).
		WithValidator(&JobCustomValidator{}).
		WithDefaulter(&JobCustomDefaulter{}).
		Complete()
}

---

It seems webhooks are going to be moved to internal/webhook anyway: #4150
Then the new schema is similiar.

A --external-api-path flag is added also in #4171 as part of the Golang plugin flag set.

Feedback welcome.
I would espacially need to know:

• what the plan with the flag for external APIs is (✨ Add support to scaffold controllers for External Types #4171)
• should I rebase my PR on ⚠️ (go/v4) decouple webhooks from APIs (Move Webhooks from api/<version> or api/<group>/<version> to internal/webhook/<version> or internal/webhook/<group>/<version>) #4150 (probably yes)?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fischor
Once this PR has been reviewed and has the lgtm label, please assign varshaprasad96 for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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]

Welcome @fischor!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @fischor. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[fischor]

After looking at #4150, I am not sure if this PR adds any more value, since in combination with #4171 the main aspects of having an import for the API struct type and having a way to determine if a resource is external or not are already solved.

[camilamacedo86]

Those changes will not be required after : #4150

[camilamacedo86]

Here should be if the Resource.Path is not empty

[camilamacedo86]

Thank you a lot for looking on that.
But the changes here will be a little diff.

Add the support for Core and External types is blocked by:

• ⚠️ (go/v4) decouple webhooks from APIs (Move Webhooks from api/<version> or api/<group>/<version> to internal/webhook/<version> or internal/webhook/<group>/<version>) #4150
• ✨ Add support to scaffold controllers for External Types #4171

After we get this merged then we can do that in more precisely way and add tests to ensure it.

Please

/hold

[sundaram2021]

Review Feedback:
Modular Flag Addition: The flag for external resources seems essential, but clarity is needed on whether it should be added globally or to the Golang plugin. Care should be taken to ensure backward compatibility.

Webhook Setup Changes: The SetupWebhookForJobWithManager function modification looks appropriate for external types, but detailed testing and validation are required to ensure seamless integration with external APIs.

Dependency on Other PRs: The PR (#4150) decoupling webhooks from APIs needs to be merged first, as the current PR's changes depend on it.

Next Steps:
Rebase on #4150 and #4171 once they are merged.
Add tests to validate external API support fully.
Collaborate on flag placement (global vs. plugin).

[camilamacedo86]

Hi @fischor, @sundaram2021,

Thank you for looking into this.

We do plan to add support for this feature. However, I would suggest not spending time on it right now, as the approach we'll take will be slightly different from how it's done in this context. It will likely be addressed as a follow-up to the ongoing PR: #4176 (review). I want to ensure that no time or effort is wasted, as there are many other priorities in the project!

If you'd like to help us move forward, we'd really appreciate your assistance with reviewing this PR: #4150. This will help us reach our goal more efficiently.

Thanks! 🎉

[camilamacedo86]

It is not the same of what was implemented before.

[camilamacedo86]

Hi @fischor

I think the changes here would not work
Would you like to cherry-pick the PRS #4150 #4171 and then try to achieve this goal accordingly.

See that we should add the call to generate the webhook to test it out in https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/testdata/generate.sh

[fischor]

@camilamacedo86 Yes, that is the plan. I am waiting for these two PRs to get merged before continuing.

[k8s-ci-robot]

PR needs rebase.

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.

[camilamacedo86]

Hi @fischor

In order to see if we can move forward with I started the PR: #4203
Please, your help is very appreciated. Feel free to get the PR and change it as you see fit