k8s-ci-robot
commented
1 year ago @nojnhuh: This request has been marked as suitable for new contributors.
Guidelines
Please ensure that the issue body includes answers to the following questions:
- Why are we solving this issue?
- To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
- Does this issue have zero to low barrier of entry?
- How can the assignee reach out to you for help?
For more details on the requirements of such an issue, please see here and ensure that they are met.
If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.
In response to [this](https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/4204):
>/kind cleanup
>/good-first-issue
>/help
>
>**What needs cleanup:**
>
>
>There are several places in the defaulting webhooks where CAPZ defaults a static value that could be replaced with the [`// +kubebuilder:default`](https://book.kubebuilder.io/reference/markers/crd-validation.html) annotation on the API type.
>
>e.g. this code defaults a static value
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_webhook.go#L75-L78
>
>It should be removed and replaced by the following change to this block:
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_types.go#L118-L123
>
>```diff
>diff --git a/api/v1beta1/azuremanagedcontrolplane_types.go b/api/v1beta1/azuremanagedcontrolplane_types.go
>index 5d83db207..35adbf3db 100644
>--- a/api/v1beta1/azuremanagedcontrolplane_types.go
>+++ b/api/v1beta1/azuremanagedcontrolplane_types.go
>@@ -126,6 +126,7 @@ type AzureManagedControlPlaneSpec struct {
> // Allowed values are "azure", "kubenet".
> // Immutable.
> // +kubebuilder:validation:Enum=azure;kubenet
>+ // +kubebuilder:default:=azure
> // +optional
> NetworkPlugin *string `json:"networkPlugin,omitempty"`
>```
>
>Tests that verify the removed code should also be removed.
>
>After modifying the annotations, run `make generate-manifests` to encode the new defaults in the CRDs.
>
>**Describe the solution you'd like**
>Walk through the webhooks starting at the following places and replace hand-rolled code to default fields with static fields with the kubebuilder annotation:
>
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremachine_webhook.go#L221
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremachinetemplate_webhook.go#L145
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_webhook.go#L70
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedmachinepool_webhook.go#L68
>https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/exp/api/v1beta1/azuremachinepool_webhook.go#L59
>
>**Anything else you would like to add:**
>[Miscellaneous information that will assist in solving the issue.]
>
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
SubhasmitaSw
nojnhuh
kumarankit999
bryan-cox
nawazkh
/kind cleanup /good-first-issue /help
What needs cleanup:
There are several places in the defaulting webhooks where CAPZ defaults a static value that could be replaced with the
// +kubebuilder:defaultannotation on the API type.e.g. this code defaults a static value https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_webhook.go#L75-L78
It should be removed and replaced by the following change to this block: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_types.go#L118-L123
Tests that verify the removed code should also be removed.
After modifying the annotations, run
make generate-manifeststo encode the new defaults in the CRDs.Describe the solution you'd like Walk through the webhooks starting at the following places and replace hand-rolled code to default fields with static fields with the kubebuilder annotation:
https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremachine_webhook.go#L221 https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremachinetemplate_webhook.go#L145 https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedcontrolplane_webhook.go#L70 https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/api/v1beta1/azuremanagedmachinepool_webhook.go#L68 https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/e2bd59253c859b151ac6f3222ad6c749e2a1ba75/exp/api/v1beta1/azuremachinepool_webhook.go#L59
Anything else you would like to add: [Miscellaneous information that will assist in solving the issue.]