🌱 Enable optionalfields linter and fix remaining findings #12299

sbueringer · 2025-05-27T14:01:58Z

What this PR does / why we need it:
Fixes the remaining findings of the optionalfields linter.

We'll follow-up on docs.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #10852
Fixes #6416

hack/tools/.custom-gcl.yaml

.golangci-kal.yml

sbueringer · 2025-07-04T05:50:20Z

test/e2e/scale.go

-	u := &unstructured.Unstructured{}
-	Expect(scheme.Convert(cluster, u, nil)).To(Succeed())
-	modifiedClusterYAML, err := yaml.FromUnstructured([]unstructured.Unstructured{*u})
+	// Note: We have to set GVK here explicitly otherwise apiVersion + kind won't be set in the YAML.


UnstructuredConverter doesn't handle omitzero yet (it will with apimachinery v0.34). So switched to marshaling Cluster directly

sbueringer · 2025-07-04T06:24:28Z

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

sbueringer · 2025-07-04T09:46:58Z

When following the Cluster API quick-start with dualstack and ipv4 primary [IPv6] is flaky on all branches independent of this PR

JoelSpeed · 2025-07-04T08:55:49Z

.golangci-kal.yml

+      text: "optionalfields: field (Bootstrap) is optional and (should be a pointer|should have the omitempty tag|has a valid zero value)"
+      linters:
+        - kubeapilinter
+    # KAL incorrectly reports that the Taints field doesn't have to be a pointer (it has to be to preserve []).


We raised a bug on KAL for this? Can it be linked?

Should be able to pick this up from a minimum items marker set to 0

Ah. I thought the finding was intended so I didn't think about opening a bug. So MinItems=0 implies that empty array should be preserved? (I'll then open a bug for it)

Yeah, so in discussion with upstream API reviewers, I personally think that this is fine for CRDs, but we know it's not ok for built-in/server types.

So we probably want to have an option that points this out/allows it for CRDs, but then something additional that says you cannot have minItems:=0 for built-in types.

I'll open an issue in a bit

To dedup from the other thread. We have the // +kubebuilder:validation:MinItems=0 marker on the field, but we still get the following finding without the exclude:

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go:304:2: optionalfields: field Taints is optional but the underlying type does not need to be a pointer. The pointer should be removed. (kubeapilinter) Taints *[]corev1.Taint `json:"taints,omitempty"` ^

Issue: kubernetes-sigs/kube-api-linter#116

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

JoelSpeed · 2025-07-04T09:42:43Z

api/runtime/v1beta2/extensionconfig_types.go

@@ -179,7 +179,6 @@ type ExtensionHandler struct {
 	// failurePolicy defines how failures in calls to the ExtensionHandler should be handled by a client.
 	// Defaults to Fail if not set.
 	// +optional
-	// +kubebuilder:validation:Enum=Ignore;Fail
 	FailurePolicy *FailurePolicy `json:"failurePolicy,omitempty"`


Doesn't need to be a pointer because the zero value is not valid. I believe there was an exception for this in the config?

The exclude is for DeletePolicy, not FailurePolicy. But I"m going to open a follow-up PR for this independently.

I think the linter does not detect this correctly, but that is also because of the known enum bug (kubernetes-sigs/kube-api-linter#113). If I add the Enum marker here back I'm getting:

api/runtime/v1beta2/extensionconfig_types.go:183:2: optionalfields: field FailurePolicy is optional and does not allow the zero value. The field does not need to be a pointer. (kubeapilinter) FailurePolicy *FailurePolicy `json:"failurePolicy,omitempty"` ^

I'll open a PR to fix this next week

PR to fix the pointer: #12453

fabriziopandini

Great improvement!

internal/webhooks/machine.go

cmd/clusterctl/client/cluster/mover_test.go

controlplane/kubeadm/internal/controllers/helpers_test.go

api/core/v1beta2/machine_types.go

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

fabriziopandini · 2025-07-04T11:12:11Z

failures looks like unrelated (ipv6 tests)
/lgtm
/approve

/hold in case @JoelSpeed wants to make another pass

k8s-ci-robot · 2025-07-04T11:12:19Z

LGTM label has been added.

Git tree hash: da931f85fdceb93c5cbc6b57739823d2aaf99762

k8s-ci-robot · 2025-07-04T11:12:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [fabriziopandini]

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

sbueringer · 2025-07-04T11:31:52Z

api/core/v1beta2/cluster_types.go

@@ -1139,16 +1140,19 @@ func (c *ClusterStatus) GetTypedPhase() ClusterPhase {
 // ANCHOR: APIEndpoint

 // APIEndpoint represents a reachable Kubernetes API endpoint.
+// +kubebuilder:validation:MinProperties=1
 type APIEndpoint struct {


@JoelSpeed I think the changes in this PR should also address #6416

And yes we have use cases where folks set either host or port or both

I think I agree, provided we are adding omitzero at the places where this field is being used.

Yes, we have that set and we'll update the migration doc to mention it

JoelSpeed

/hold cancel

All good on my side

JoelSpeed · 2025-07-04T11:56:20Z

api/core/v1beta2/cluster_types.go

@@ -1139,16 +1140,19 @@ func (c *ClusterStatus) GetTypedPhase() ClusterPhase {
 // ANCHOR: APIEndpoint

 // APIEndpoint represents a reachable Kubernetes API endpoint.
+// +kubebuilder:validation:MinProperties=1
 type APIEndpoint struct {


I think I agree, provided we are adding omitzero at the places where this field is being used.

sbueringer · 2025-07-04T14:01:23Z

/override pull-cluster-api-e2e-latestk8s-main
/override pull-cluster-api-e2e-main

See #12299 (comment)

k8s-ci-robot · 2025-07-04T14:01:26Z

@sbueringer: Overrode contexts on behalf of sbueringer: pull-cluster-api-e2e-latestk8s-main, pull-cluster-api-e2e-main

In response to this:

/override pull-cluster-api-e2e-latestk8s-main
/override pull-cluster-api-e2e-main

See #12299 (comment)

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.

k8s-ci-robot · 2025-07-04T14:17:18Z

@sbueringer: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-apidiff-main	`6fc41b1`	link	false	`/test pull-cluster-api-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

sbueringer · 2025-07-04T14:17:42Z

/override pull-cluster-api-test-main

Known flake

k8s-ci-robot · 2025-07-04T14:17:45Z

@sbueringer: Overrode contexts on behalf of sbueringer: pull-cluster-api-test-main

In response to this:

/override pull-cluster-api-test-main

Known flake

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.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-area PR is missing an area label labels May 27, 2025

k8s-ci-robot requested review from JoelSpeed and sivchari May 27, 2025 14:02

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 27, 2025

sbueringer commented May 27, 2025

View reviewed changes

hack/tools/.custom-gcl.yaml Outdated Show resolved Hide resolved

sbueringer force-pushed the pr-optionalfields-linter branch from 1a28f2c to 1b12419 Compare May 27, 2025 14:06

JoelSpeed reviewed May 27, 2025

View reviewed changes

.golangci-kal.yml Outdated Show resolved Hide resolved

sbueringer force-pushed the pr-optionalfields-linter branch from 1b12419 to 984df7f Compare June 5, 2025 16:33

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2025

sbueringer force-pushed the pr-optionalfields-linter branch from 984df7f to 3b1fa37 Compare June 18, 2025 15:22

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2025

sbueringer force-pushed the pr-optionalfields-linter branch from 3b1fa37 to df2839e Compare June 26, 2025 15:15

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 26, 2025

sbueringer force-pushed the pr-optionalfields-linter branch from a067572 to 9311747 Compare June 27, 2025 12:37

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 27, 2025

sbueringer mentioned this pull request Jun 30, 2025

Umbrella issue: API changes #10852

Open

91 tasks

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2025

sbueringer force-pushed the pr-optionalfields-linter branch from 9311747 to c3f070b Compare July 2, 2025 14:26

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2025

sbueringer force-pushed the pr-optionalfields-linter branch 2 times, most recently from 3eefea7 to a679fc0 Compare July 2, 2025 15:36

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2025

sbueringer force-pushed the pr-optionalfields-linter branch from a679fc0 to bbb3f2d Compare July 2, 2025 15:44

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2025

sbueringer force-pushed the pr-optionalfields-linter branch 2 times, most recently from c486c9b to 031531b Compare July 2, 2025 16:40

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2025

sbueringer commented Jul 4, 2025

View reviewed changes

kubernetes-sigs deleted a comment from k8s-ci-robot Jul 4, 2025

JoelSpeed reviewed Jul 4, 2025

View reviewed changes

fabriziopandini reviewed Jul 4, 2025

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2025

k8s-ci-robot assigned fabriziopandini Jul 4, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 4, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 4, 2025

sbueringer commented Jul 4, 2025

View reviewed changes

sbueringer mentioned this pull request Jul 4, 2025

⚠️ Drop unnecessary fields from ClusterClass template references #12358

Merged

JoelSpeed reviewed Jul 4, 2025

View reviewed changes

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2025

k8s-ci-robot merged commit ea1a774 into kubernetes-sigs:main Jul 4, 2025
37 of 38 checks passed

k8s-ci-robot added this to the v1.11 milestone Jul 4, 2025

sbueringer deleted the pr-optionalfields-linter branch July 4, 2025 14:23

sbueringer mentioned this pull request Jul 14, 2025

Tracking issue for v1Beta2 #11947

Open

78 tasks

🌱 Enable optionalfields linter and fix remaining findings #12299

🌱 Enable optionalfields linter and fix remaining findings #12299

Uh oh!

Conversation

sbueringer commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer commented Jul 4, 2025

Uh oh!

sbueringer commented Jul 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabriziopandini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fabriziopandini commented Jul 4, 2025

Uh oh!

k8s-ci-robot commented Jul 4, 2025

Uh oh!

k8s-ci-robot commented Jul 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbueringer commented Jul 4, 2025

Uh oh!

k8s-ci-robot commented Jul 4, 2025

Uh oh!

k8s-ci-robot commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Jul 4, 2025

Uh oh!

k8s-ci-robot commented Jul 4, 2025

Uh oh!

Uh oh!

Uh oh!

sbueringer commented May 27, 2025 •

edited

Loading

sbueringer Jul 4, 2025 •

edited

Loading

sbueringer Jul 4, 2025 •

edited

Loading

k8s-ci-robot commented Jul 4, 2025 •

edited

Loading