Skip to content

Conversation

@moko-poi
Copy link

@moko-poi moko-poi commented Nov 18, 2025

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This PR enables the nofloats linter as part of the KAL (Kube-API-Linter) configuration to enforce Kubernetes API best practices by preventing the use of float types in API definitions.

Float types are discouraged in Kubernetes APIs due to:

  • Precision issues across different platforms
  • Unreliable round-tripping in serialization/deserialization
  • Difficulties in comparison operations

The internal rate limiter implementation (pkg/internal/rate/rate.go) legitimately uses float64 for token bucket calculations and has been exempted from this linter rule using //nolint:kubeapilinter comments, as it is not part of the Kubernetes API surface.

Which issue(s) this PR fixes:
Fixes #5490

Special notes for your reviewer:

The nofloats linter only flags float usage in type definitions and struct fields, not in type conversions or temporary calculations. This is why exp/api/v1beta2/validation.go (which uses float64() conversions) doesn't trigger errors.

The existing path-except: "api//*" rule in .golangci-kal.yml doesn't effectively limit KAL linting to API folders only - KAL runs on all Go code similar to how optionalorrequired linter was previously enabled. Therefore, internal packages that legitimately use floats need explicit //nolint comments.

Checklist:

  • squashed commits
  • includes documentation
  • includes emoji in title
  • adds unit tests
  • adds or updates e2e tests

Release note:

NONE

@k8s-ci-robot
Copy link
Contributor

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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 k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority labels Nov 18, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabriziopandini for approval. For more information see the 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:

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 added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 18, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @moko-poi. Thanks for your PR.

I'm waiting for a github.com 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.

@moko-poi
Copy link
Author

The nofloats linter detected 4 violations in pkg/internal/rate/rate.go related to float usage in the token bucket rate limiter implementation.
https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/main/pkg/internal/rate/rate.go

Rather than refactoring the rate limiting logic (which would be out of scope for this PR), I've added //nolint:kubeapilinter comments to exempt this internal code.

This is justified because:

  • The rate limiter is internal infrastructure, not part of the Kubernetes API surface
  • The nofloats linter's purpose is to prevent floats in API definitions
  • Modifying the rate limiter implementation would introduce unnecessary risk

The nofloats linter only flags type/field definitions, not conversions, which is why exp/api/v1beta2/validation.go doesn't trigger errors despite using float64().

@moko-poi moko-poi force-pushed the enable_nofloats_linter branch from 23d5d5c to 9c4f75b Compare November 22, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[KAL] Enable the nofloats linter

2 participants