chore: Use 'go fix ./...' to replace utils with primitives - #2048
Conversation
fc09406 to
641acd2
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The change set is broad and toolchain-driven across many packages/tests, so it needs confirmation via CI (compile + unit/e2e tests) before it can be safely approved.
Pull request overview
This PR applies go fix ./...-driven updates across Fluent Operator’s Go codebase, modernizing syntax and standard-library usage in controllers, APIs, and tests to align with the Go 1.26.x toolchain.
Changes:
- Replaces older
interface{}usage withany, and updates various string helpers tostrings.CutPrefix/CutSuffix. - Modernizes map handling in the Fluent Bit controller with
maps.Copy. - Refactors multiple code paths/tests to newer iteration and reflection patterns (e.g., integer
range,strings.SplitSeq, updated reflect field iteration).
File summaries
| File | Description |
|---|---|
| tests/e2e/fluentd/deployment_test.go | Updates pointer construction style used in Fluentd E2E CR creation. |
| pkg/utils/utils.go | Updates generic pointer helper implementation and adds a //go:fix directive. |
| controllers/fluentbitconfig_controller_test.go | Uses map[string]any in YAML parse validation. |
| controllers/fluentbit_controller.go | Replaces manual map merging with maps.Copy. |
| controllers/collector_config_test.go | Updates pointer creation helper and usages; adds //go:fix directive. |
| cmd/doc-gen/main.go | Adopts strings.CutPrefix/CutSuffix and any type usage. |
| apis/fluentd/v1alpha1/tests/tools.go | Replaces interface{} with any in YAML parsing helpers. |
| apis/fluentd/v1alpha1/tests/helper_test.go | Uses integer range loops where the index is unused. |
| apis/fluentd/v1alpha1/plugins/params/model.go | Replaces string concatenation with strings.Builder for body rendering. |
| apis/fluentd/v1alpha1/plugins/params/model_test.go | Uses strings.SplitSeq iteration for line scanning. |
| apis/fluentd/v1alpha1/plugins/filter/types.go | Adjusts embedded JSON tag for FilterCommon. |
| apis/fluentbit/v1alpha2/plugins/output/syslog_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/splunk_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/s3_types_test.go | Updates pointer creation approach in output plugin tests (including nested structs). |
| apis/fluentbit/v1alpha2/plugins/output/open_telemetry_types_test.go | Updates pointer creation approach in output plugin tests (including TLS/networking). |
| apis/fluentbit/v1alpha2/plugins/output/kinesis_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/influxdb_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/http_types_test.go | Updates pointer creation approach in output plugin tests (including TLS/networking). |
| apis/fluentbit/v1alpha2/plugins/output/gelf_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/firehose_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/output/datadog_types_test.go | Updates pointer creation approach in output plugin tests. |
| apis/fluentbit/v1alpha2/plugins/input/syslog_test.go | Updates pointer creation approach in input plugin tests. |
| apis/fluentbit/v1alpha2/plugins/common_types.go | Replaces interface{} maps/returns with any equivalents. |
| apis/fluentbit/v1alpha2/multilineparser_types.go | Updates reflection-based field iteration pattern. |
| apis/fluentbit/v1alpha2/injection_test.go | Updates pointer creation approach in injection-related tests. |
| apis/fluentbit/v1alpha2/filter_types.go | Updates interface{} to any and reflection-based field iteration. |
| apis/fluentbit/v1alpha2/filter_types_test.go | Updates pointer creation approach in filter list tests. |
| apis/fluentbit/v1alpha2/clusteroutput_types_test.go | Updates pointer creation approach across cluster output tests. |
| apis/fluentbit/v1alpha2/clusterinput_types.go | Updates reflection-based field iteration pattern. |
| apis/fluentbit/v1alpha2/clusterinput_types_test.go | Updates pointer creation approach and any map literals in tests. |
| apis/fluentbit/v1alpha2/clusterfluentbitconfig_types_test.go | Updates pointer creation approach and any YAML parse types in tests. |
| apis/fluentbit/v1alpha2/clusterfilter_types.go | Updates reflection-based field iteration pattern. |
| apis/fluentbit/v1alpha2/clusterfilter_types_test.go | Updates pointer creation approach across cluster filter tests. |
Review details
- Files reviewed: 33/33 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
641acd2 to
8bb8779
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Multiple test files now contain invalid new(<literal>) expressions (and one helper uses new(i)), which will not compile.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
tests/e2e/fluentd/deployment_test.go:84
new("0.0.0.0")/new(int32(24224))are invalid (newrequires a type), so this test won't compile. Use an address-of helper (ork8s.io/utils/ptr.To) for these pointer fields.
controllers/collector_config_test.go:96//go:fix inlinelooks like an auto-fix artifact andnew(i)is invalid (newrequires a type), so this helper won't compile. Use&ihere and drop thego:fixcomment.
//go:fix inline
func ptrInt32(i int32) *int32 { return new(i) }
apis/fluentbit/v1alpha2/clusterfluentbitconfig_types_test.go:431
disableInotifyWatcher := new(true)is invalid (newrequires a type), so this test won't compile. Use a pointer helper (ork8s.io/utils/ptr.To).
disableInotifyWatcher := new(true)
- Files reviewed: 33/33 changed files
- Comments generated: 11
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Multiple files now use new(value) as a pointer helper (e.g., new(int32(1)), new(true)), which does not compile with Go’s built-in new and must be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
controllers/collector_config_test.go:76
new(int32(24224))will not compile because the built-innewdoes not take a value argument; it only takes a type (e.g.,new(int32)). Use a pointer helper (e.g.,ptr.To(int32(24224))) or make the value addressable.
apis/fluentbit/v1alpha2/clusterinput_types_test.go:96new(true),new(int64(10)),new(int32(3)), etc. appear throughout this file. The built-innewonly accepts a type, not a value, so these will not compile unless a package-level helper is added. Consider switching back to a pointer helper likek8s.io/utils/ptr.To(...)(or a localtoPtrgeneric) and updating these usages accordingly.
Tail: &input.Tail{
DisableInotifyWatcher: new(true),
Tag: "logs.foo.bar",
Path: "/logs/containers/apps0",
ExcludePath: "/logs/containers/exclude_path",
SkipLongLines: new(true),
IgnoreOlder: "5m",
MemBufLimit: "5MB",
RefreshIntervalSeconds: new(int64(10)),
- Files reviewed: 42/42 changed files
- Comments generated: 5
- Review effort level: Lite
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.
Suppressed comments (3)
tests/e2e/fluentd/deployment_test.go:1
- These
new(...)calls won’t compile in Go:newtakes a type (e.g.,new(int32)) and cannot be called with a value likenew(int32(1))ornew(\"0.0.0.0\"). To keep the desired pointer-to-literal behavior, use a helper likeptr.To(...)(k8s utils), reintroduce a small genericToPtr[T any](v T) *T { return &v }, or assign to a local variable and take its address (e.g.,replicas := int32(1); Replicas: &replicas).
tests/e2e/fluentd/deployment_test.go:1 - These
new(...)calls won’t compile in Go:newtakes a type (e.g.,new(int32)) and cannot be called with a value likenew(int32(1))ornew(\"0.0.0.0\"). To keep the desired pointer-to-literal behavior, use a helper likeptr.To(...)(k8s utils), reintroduce a small genericToPtr[T any](v T) *T { return &v }, or assign to a local variable and take its address (e.g.,replicas := int32(1); Replicas: &replicas).
controllers/fluentdconfig_controller.go:1 - Requeuing after
time.Nanosecondis effectively an immediate hot-loop and can generate excessive API/server load if the NotFound condition persists. Prefer either (a) returningctrl.Result{}and relying on watch events to re-trigger reconciliation, or (b) using a reasonable backoff interval (e.g., seconds) if periodic polling is intended.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 3 comments.
Suppressed comments (4)
tests/e2e/fluentd/deployment_test.go:1
new(...)in Go only accepts a type (e.g.,new(int32)), not a value (e.g.,new(int32(1))/new(\"0.0.0.0\")). As written, this will not compile. Replace these with a real pointer-to-value helper (e.g., re-introduce a small genericToPtr[T any](v T) *Tin a more appropriate package, usek8s.io/utils/ptr.To, or assign to a variable and take its address).
tests/e2e/fluentd/deployment_test.go:1new(...)in Go only accepts a type (e.g.,new(int32)), not a value (e.g.,new(int32(1))/new(\"0.0.0.0\")). As written, this will not compile. Replace these with a real pointer-to-value helper (e.g., re-introduce a small genericToPtr[T any](v T) *Tin a more appropriate package, usek8s.io/utils/ptr.To, or assign to a variable and take its address).
apis/fluentd/v1alpha1/tests/tools.go:1328json.Unmarshalshould receive the destination value (typically a pointer) asobj, not&obj. Passing&objmakes the destination a*any(pointer to the interface variable), so the caller's object will not be populated as intended. Change this to unmarshal intoobj.
func ParseIntoObject(data string, obj any) error {
body, err := yaml.YAMLToJSON([]byte(data))
if err != nil {
return err
}
err = json.Unmarshal(body, &obj)
if err != nil {
return err
}
controllers/fluentdconfig_controller.go:1
- This changes reconcile timing semantics from the previous
RequeueAfter: time.Duration(1)(1ns) +Requeue: trueto a 1 second delay. Even if the old code was likely unintended, this is a behavior change not directly related to 'replace utils with primitives'. Consider either (a) keeping the old duration semantics if that was intentional, or (b) calling out this behavior change explicitly in the PR description (and potentially using a named constant to document the intended delay).
8db7d80 to
9624b81
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 4 comments.
Suppressed comments (3)
tests/e2e/fluentd/deployment_test.go:1
new(...)in Go takes a type, not a value; constructs likenew(int32(1))/new(\"0.0.0.0\")will not compile. If the intent is to get pointers to literal values, use a pointer helper (e.g., keepk8s.io/utils/ptrand useptr.To(...)), or assign to a local variable and take its address (v := int32(1); &v).
tests/e2e/fluentd/deployment_test.go:1new(...)in Go takes a type, not a value; constructs likenew(int32(1))/new(\"0.0.0.0\")will not compile. If the intent is to get pointers to literal values, use a pointer helper (e.g., keepk8s.io/utils/ptrand useptr.To(...)), or assign to a local variable and take its address (v := int32(1); &v).
controllers/collector_config_test.go:1new(int32(24224))is invalid and also removes the prior working behavior of returning a pointer to the provided value. If you want to avoid a local helper likeptrInt32, preferptr.To(int32(24224))(fromk8s.io/utils/ptr) or a tiny local inline variable +&.
| TimeKey: new("test_time_key"), | ||
| TimeKeyFormat: new("%Y-%m-%dT%H:%M:%S.%3N"), |
9624b81 to
712a6bd
Compare
Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
712a6bd to
0df7f73
Compare
Uh oh!
There was an error while loading. Please reload this page.