Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BETA API] Implement conversion webhook for alpha-beta transformation #1104

Closed
SaschaSchwarze0 opened this issue Sep 2, 2022 · 2 comments · Fixed by #1302
Closed

[BETA API] Implement conversion webhook for alpha-beta transformation #1104

SaschaSchwarze0 opened this issue Sep 2, 2022 · 2 comments · Fixed by #1302
Assignees
Labels
beta BETA related issue kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature.

Comments

@SaschaSchwarze0
Copy link
Member

SaschaSchwarze0 commented Sep 2, 2022

Provide a conversion webhook for the alpha-beta transformation. https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#webhook-conversion

This should be a separate binary in this project that produces a separate container image with a separate deployment.

@SaschaSchwarze0 SaschaSchwarze0 added kind/feature Categorizes issue or PR as related to a new feature. beta BETA related issue kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Sep 2, 2022
@SaschaSchwarze0 SaschaSchwarze0 added this to the release-v0.12.0 milestone Sep 2, 2022
@qu1queee qu1queee self-assigned this Mar 10, 2023
@qu1queee qu1queee mentioned this issue Mar 10, 2023
4 tasks
@qu1queee qu1queee mentioned this issue May 17, 2023
4 tasks
@qu1queee
Copy link
Contributor

@otaviof I send you a msg in Slack, but putting it here for awareness. At the moment I see that this issue could be divided into two PRs:

  1. We need to implement a couple of functions(Hub and Spoke), this ones will basically take each of our Custom Resources(Build, BuildRun, BuildStrategy and ClusterBuildStrategy) and convert them to the desired API version(e.g. v1beta1 to v1alpha1). Where Hub is v1alpha1, and spokes is v1beta1. I can cover this via Conversion webhook #1302 .

  2. We need a webhook for the conversion. From the kubebuilder docs they place the webhook logic under each API version type, and call it at the init time. In this issue, the proposal is to have this as a single binary with a single deployment. As we could later extend it, to cover other validations/mutations. From the kubebuilder docs, it is stated: This setup doubles as setup for our conversion webhooks: as long as our types implement the [Hub](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/conversion?tab=doc#Hub) and [Convertible](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/conversion?tab=doc#Convertible) interfaces, a conversion webhook will be registered., this means to me that we do not need to specify anything on the webhook regarding conversion(I might be wrong), but we just need a webhook that can mutate our custom resources(I think 4 different endpoints).

@otaviof you can move on with 2, and then I do 1. We can exchange notes over the next days.

@qu1queee
Copy link
Contributor

We had a session on this issue today, here a list of items:

  • The conversion webhook implementation is now being done without the https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/conversion#Convertible interface. The reason is that the kubebuilder tutorial webhook setup did not work for us. Hard to say why, but the webhook was never serving the conversion. So we decided to adopt the more native approach which implies a little bit more of code.
  • Due to the above, @qu1queee to reflect that in the original SHIP.
  • Unit test are missing at the moment for the conversion.
  • We discussed on how to autogenerate the CABundle, for both the ValidatingWebhookConfiguration and the CRDS(where we need the spec.conversion block. @SaschaSchwarze0 to open a new issue on this.
  • Currently , the webhook is not doing the conversion properly, so it remains as wip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta BETA related issue kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants