Bump golang to 1.23.7 and golangci-lint to 1.64.8 #6218

seanlaii · 2025-03-20T14:24:03Z

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
To remove the dependency of hashicorp, we need to bump golang to 1.23.7.
For more details, please check the discussion at #6212.

Which issue(s) this PR fixes:
Prerequisite of #3941

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Karmada is now built with Golang v1.23.7.

codecov-commenter · 2025-03-20T14:39:02Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 70.37037% with 8 lines in your changes missing coverage. Please review.

Project coverage is 47.90%. Comparing base (56b72f1) to head (d231a87).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...ontrollers/federatedhpa/federatedhpa_controller.go	70.00%	3 Missing ⚠️
pkg/estimator/client/cache.go	0.00%	1 Missing ⚠️
pkg/estimator/client/general.go	66.66%	1 Missing ⚠️
pkg/estimator/server/estimate.go	50.00%	1 Missing ⚠️
pkg/resourceinterpreter/interpreter.go	0.00%	1 Missing ⚠️
pkg/util/overridemanager/overridemanager.go	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6218      +/-   ##
==========================================
- Coverage   47.91%   47.90%   -0.01%     
==========================================
  Files         676      676              
  Lines       55958    55962       +4     
==========================================
  Hits        26810    26810              
- Misses      27400    27404       +4     
  Partials     1748     1748

Flag	Coverage Δ
unittests	`47.90% <70.37%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

seanlaii · 2025-03-21T00:05:05Z

Hi @RainbowMango , please help take a look when you get a change! Thank you!

There are six lint errors from a new gosec rule that warns converting an int or int64 to int32 may cause an overflow on 64-bit systems.
I think in these cases we could add a nolint to ignore the errors, but I'd like to confirm with the community if a type change is preferable. Thank you!

Error: pkg/webhook/interpreter/response.go:56:19: G115: integer overflow conversion int -> int32 (gosec)
				Code:    int32(code),
				              ^
Error: pkg/scheduler/core/spreadconstraint/select_clusters_by_cluster.go:43:85: G115: integer overflow conversion int -> int32 (gosec)
		clusterInfos = selectClustersByAvailableResource(groupClustersInfo.Clusters, int32(needCnt), needReplicas)
		                                                                                  ^
Error: pkg/estimator/client/cache.go:113:86: G115: integer overflow conversion int -> int32 (gosec)
		names.GenerateEstimatorServiceName(serviceInfo.NamePrefix, serviceInfo.Name), int32(grpcConfig.TargetPort))
		                                                                                   ^
Error: pkg/estimator/client/general.go:6[9](https://github.com/karmada-io/karmada/actions/runs/13971691479/job/39114901333?pr=6218#step:6:10):15: G115: integer overflow conversion int64 -> int32 (gosec)
		return int32(maximumReplicas)
		            ^
Error: pkg/estimator/client/general.go:83:16: G[11](https://github.com/karmada-io/karmada/actions/runs/13971691479/job/39114901333?pr=6218#step:6:12)5: integer overflow conversion int64 -> int32 (gosec)
			return int32(maximumReplicas)
			            ^
Error: pkg/estimator/client/general.go:93:14: G115: integer overflow conversion int64 -> int32 (gosec)
	return int32(maximumReplicas)

seanlaii · 2025-03-21T00:07:45Z

pkg/controllers/federatedhpa/federatedhpa_controller.go

@@ -1415,14 +1415,14 @@ func setConditionInList(inputList []autoscalingv2.HorizontalPodAutoscalerConditi
 	return resList
 }

-func max(a, b int32) int32 {
+func maxInt32(a, b int32) int32 {


Change the name of the function to avoid overlapping with the name of the built-in max function.

+1
on change the name.

But the naming is misleading with the math.MaxInt32.

How about maxOfInt32 or computeMaxInt32?

RainbowMango

/assign
I will take a look soon.

Copilot

Pull Request Overview

This PR bumps the Golang version to v1.23.7 and golangci-lint to v1.64.8 while cleaning up some redundant code patterns and improving function naming for clarity in various modules.

Updated random number generation variable naming and removed redundant nil checks in tests
Refactored error creation to use errors.New and updated helper function names for consistency
Adjusted error handling in test cases and production code for a more streamlined error message format

Reviewed Changes

Copilot reviewed 6 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/controllers/workloadrebalancer/workloadrebalancer_controller_test.go	Renamed variable from "max" to "maxInt" for clarity in generating a random suffix
pkg/util/helper/workstatus_test.go	Removed unnecessary nil slice checks in test functions
pkg/resourceinterpreter/customized/declarative/luavm/lua_test.go	Changed t.Errorf calls to t.Error for error logging in tests
pkg/estimator/server/estimate.go	Simplified error creation; note potential spelling issue in error message
pkg/controllers/federatedhpa/federatedhpa_controller.go	Updated error handling and function naming (minInt32/maxInt32) for improved clarity and consistency
pkg/util/overridemanager/overridemanager.go	Replaced fmt.Errorf with errors.New for consistent error handling in the override manager

Files not reviewed (3)

.go-version: Language not supported
go.mod: Language not supported
hack/verify-staticcheck.sh: Language not supported

Comments suppressed due to low confidence (1)

pkg/controllers/federatedhpa/federatedhpa_controller.go:1289

The renaming to 'minInt32' (and similarly 'maxInt32') improves clarity; please ensure that all affected references and related documentation are updated accordingly.

selectPolicyFn = minInt32

pkg/estimator/server/estimate.go

RainbowMango · 2025-03-21T03:00:36Z

pkg/controllers/federatedhpa/federatedhpa_controller.go

@@ -1415,14 +1415,14 @@ func setConditionInList(inputList []autoscalingv2.HorizontalPodAutoscalerConditi
 	return resList
 }

-func max(a, b int32) int32 {
+func maxInt32(a, b int32) int32 {


+1
on change the name.

But the naming is misleading with the math.MaxInt32.

RainbowMango · 2025-03-21T03:07:51Z

pkg/controllers/federatedhpa/federatedhpa_controller.go

@@ -1415,14 +1415,14 @@ func setConditionInList(inputList []autoscalingv2.HorizontalPodAutoscalerConditi
 	return resList
 }

-func max(a, b int32) int32 {
+func maxInt32(a, b int32) int32 {


How about maxOfInt32 or computeMaxInt32?

pkg/controllers/federatedhpa/federatedhpa_controller.go

pkg/controllers/workloadrebalancer/workloadrebalancer_controller_test.go

pkg/estimator/server/estimate.go

RainbowMango · 2025-03-21T03:32:40Z

I think in these cases we could add a nolint to ignore the errors, but I'd like to confirm with the community if a type change is preferable. Thank you!

I agree to ignore these false-positive warnings so do not let that block this PR.
But I'd like to invite @zhzhuang-zju to take another look if we can change the type to avoid the conversion.

zhzhuang-zju · 2025-03-21T07:15:30Z

@RainbowMango @seanlaii Through investigation, only the third item is more suitable for avoiding the conversion by modifying the type.

Error: pkg/estimator/client/cache.go:113:86: G115: integer overflow conversion int -> int32 (gosec)
		names.GenerateEstimatorServiceName(serviceInfo.NamePrefix, serviceInfo.Name), int32(grpcConfig.TargetPort))

For the others, it is necessary to modify the field definitions in the API or make other changes with relatively high costs, so it is not recommended.

RainbowMango · 2025-03-21T07:27:31Z

Through investigation, only the third item is more suitable for avoiding the conversion by modifying the type.

By how? Give an example.

zhzhuang-zju · 2025-03-21T07:35:25Z

By how? Give an example.

func resolveCluster(kubeClient kubernetes.Interface, namespace, id string, port int) ([]string, error)

// findServicePort finds the service port by name or numerically.
func findServicePort(svc *corev1.Service, port int) (*corev1.ServicePort, error) {
	for _, svcPort := range svc.Spec.Ports {
		if int(svcPort.Port) == port {
			return &svcPort, nil
		}
	}
	return nil, apierrors.NewServiceUnavailable(fmt.Sprintf("no service port %d found for service %q", port, svc.Name))
}

Change the conversion from int -> int32 to int32 -> int.

seanlaii · 2025-03-21T21:41:26Z

By how? Give an example.

func resolveCluster(kubeClient kubernetes.Interface, namespace, id string, port int) ([]string, error)

// findServicePort finds the service port by name or numerically.
func findServicePort(svc *corev1.Service, port int) (*corev1.ServicePort, error) {
	for _, svcPort := range svc.Spec.Ports {
		if int(svcPort.Port) == port {
			return &svcPort, nil
		}
	}
	return nil, apierrors.NewServiceUnavailable(fmt.Sprintf("no service port %d found for service %q", port, svc.Name))
}

Change the conversion from int -> int32 to int32 -> int.

Thanks for the suggestion and review!
I have tried it but found that the type of the Port of corev1.Service is int32. It means we will still need to convert from int --> int32.
The possible way to avoid the conversion is to change the type of targetPort in the grpc config.

Do you suggest to do this?

In my opinion, since this config is used by multiple components, maybe we could address this in another PR?

RainbowMango · 2025-03-22T02:30:57Z

In my opinion, since this config is used by multiple components, maybe we could address this in another PR?

+1
Just ignore this waring in this PR would be fine.

zhzhuang-zju · 2025-03-22T03:06:14Z

In my opinion, since this config is used by multiple components, maybe we could address this in another PR?

agree

I have tried it but found that the type of the Port of corev1.Service is int32. It means we will still need to convert from int --> int32.

@seanlaii Which specific part of the code is causing the issue? Is it the following section?

karmada/pkg/estimator/client/service.go

Line 74 in 56b72f1

if svcPort.Port == port {

seanlaii · 2025-03-22T03:15:33Z

@seanlaii Which specific part of the code is causing the issue? Is it the following section?

Yes, that's the primary reason. The gRPC configuration's port field (int) requires conversion to match the servicePort (int32).

zhzhuang-zju · 2025-03-22T03:24:14Z

Yes, that's the primary reason. The gRPC configuration's port field (int) requires conversion to match the servicePort (int32).

How about converting servicePort (of type int32) to int to match the port field (of type int)? Converting int32 to int should not result in an integer overflow.

		if int(svcPort.Port) == port {
			return &svcPort, nil
		}

Of course, this is just a technical discussion, and making such a modification is a bit of a clever workaround. It would be more reasonable to try to change the type of targetPort in the gRPC config.

seanlaii · 2025-03-22T03:33:40Z

Ah, sorry, you're absolutely right! I misread that as an assignment instead of an equality check. Should I change it?

zhzhuang-zju · 2025-03-22T03:36:10Z

Ah, sorry, you're absolutely right! I misread that as an assignment instead of an equality check. Should I change it?

No need, changing the type of targetPort in the gRPC config in another PR would fix this issue too~

seanlaii · 2025-03-22T03:59:15Z

Gotcha! Please help review again. Fixed all the lint errors. Thanks!

pkg/controllers/binding/common.go

Signed-off-by: wei-chenglai <qazwsx0939059006@gmail.com>

RainbowMango

/lgtm
/approve

karmada-bot · 2025-03-24T01:25:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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~~ [RainbowMango]

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

RainbowMango · 2025-03-26T01:21:53Z

Hi @seanlaii , I noticed that golangci-lint has been upgraded to v2, which is a major change. I don't know if you are interested in upgrading it. After a quick look at the changelog, it appears that the golangci-lint configuration should be tweaked a little bit.

seanlaii · 2025-03-26T02:00:21Z

Hi @RainbowMango ,

I am interested in working on it. Let me take a look. Thank you!

RainbowMango · 2025-03-26T02:17:33Z

Thank you, no rush, take your time.

karmada-bot added the kind/cleanup label Mar 20, 2025

karmada-bot requested review from chaunceyjiang, Garrybest, ikaven1024, Poor12, whitewindmills and XiShanYongYe-Chang March 20, 2025 14:24

karmada-bot added the size/M label Mar 20, 2025

seanlaii commented Mar 21, 2025

View reviewed changes

RainbowMango reviewed Mar 21, 2025

View reviewed changes

karmada-bot assigned RainbowMango Mar 21, 2025

RainbowMango requested a review from Copilot March 21, 2025 02:52

Copilot AI reviewed Mar 21, 2025

View reviewed changes

pkg/estimator/server/estimate.go Outdated Show resolved Hide resolved

RainbowMango reviewed Mar 21, 2025

View reviewed changes

seanlaii force-pushed the bump-go-ci-lint branch from 76f4d99 to d18d17a Compare March 21, 2025 03:53

seanlaii force-pushed the bump-go-ci-lint branch 3 times, most recently from 2c4eecc to 1126123 Compare March 21, 2025 21:26

seanlaii force-pushed the bump-go-ci-lint branch from 1126123 to 01808c5 Compare March 22, 2025 02:50

RainbowMango reviewed Mar 22, 2025

View reviewed changes

pkg/controllers/binding/common.go Outdated Show resolved Hide resolved

seanlaii force-pushed the bump-go-ci-lint branch from 01808c5 to d231a87 Compare March 22, 2025 17:03

RainbowMango approved these changes Mar 24, 2025

View reviewed changes

karmada-bot added the lgtm label Mar 24, 2025

karmada-bot added the approved label Mar 24, 2025

RainbowMango added this to the v1.14 milestone Mar 24, 2025

karmada-bot merged commit 0235dc6 into karmada-io:master Mar 24, 2025
24 checks passed

seanlaii deleted the bump-go-ci-lint branch March 26, 2025 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump golang to 1.23.7 and golangci-lint to 1.64.8 #6218

Bump golang to 1.23.7 and golangci-lint to 1.64.8 #6218

seanlaii commented Mar 20, 2025 •

edited by RainbowMango

Loading

codecov-commenter commented Mar 20, 2025 •

edited

Loading

seanlaii commented Mar 21, 2025 •

edited

Loading

seanlaii Mar 21, 2025

RainbowMango Mar 21, 2025

RainbowMango Mar 21, 2025

RainbowMango left a comment

Copilot AI left a comment

RainbowMango Mar 21, 2025

RainbowMango Mar 21, 2025

RainbowMango commented Mar 21, 2025

zhzhuang-zju commented Mar 21, 2025

RainbowMango commented Mar 21, 2025

zhzhuang-zju commented Mar 21, 2025

seanlaii commented Mar 21, 2025

RainbowMango commented Mar 22, 2025

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025 •

edited

Loading

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025

RainbowMango left a comment

karmada-bot commented Mar 24, 2025

RainbowMango commented Mar 26, 2025

seanlaii commented Mar 26, 2025

RainbowMango commented Mar 26, 2025

Bump golang to 1.23.7 and golangci-lint to 1.64.8 #6218

Bump golang to 1.23.7 and golangci-lint to 1.64.8 #6218

Conversation

seanlaii commented Mar 20, 2025 • edited by RainbowMango Loading

codecov-commenter commented Mar 20, 2025 • edited Loading

Codecov Report

seanlaii commented Mar 21, 2025 • edited Loading

seanlaii Mar 21, 2025

Choose a reason for hiding this comment

RainbowMango Mar 21, 2025

Choose a reason for hiding this comment

RainbowMango Mar 21, 2025

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

RainbowMango Mar 21, 2025

Choose a reason for hiding this comment

RainbowMango Mar 21, 2025

Choose a reason for hiding this comment

RainbowMango commented Mar 21, 2025

zhzhuang-zju commented Mar 21, 2025

RainbowMango commented Mar 21, 2025

zhzhuang-zju commented Mar 21, 2025

seanlaii commented Mar 21, 2025

RainbowMango commented Mar 22, 2025

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025 • edited Loading

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025

zhzhuang-zju commented Mar 22, 2025

seanlaii commented Mar 22, 2025

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Mar 24, 2025

RainbowMango commented Mar 26, 2025

seanlaii commented Mar 26, 2025

RainbowMango commented Mar 26, 2025

seanlaii commented Mar 20, 2025 •

edited by RainbowMango

Loading

codecov-commenter commented Mar 20, 2025 •

edited

Loading

seanlaii commented Mar 21, 2025 •

edited

Loading

seanlaii commented Mar 22, 2025 •

edited

Loading