Skip to content

ROX-35007: enable modernize/newexpr - #21189

Merged
janisz merged 5 commits into
masterfrom
modernize/newexpr
Aug 5, 2026
Merged

ROX-35007: enable modernize/newexpr #21189
janisz merged 5 commits into
masterfrom
modernize/newexpr

Conversation

@janisz

@janisz janisz commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Replace custom functions that returns a pointer to a value with new. Sometimes it requires a cast e.g. new(int32(1))
Refs:

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

CI

@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Summary by CodeRabbit

  • Refactor
    • Replaced internal pointer-construction helpers with Go’s built-in pointer allocation across application, operator, sensor, CLI, and test code.
    • Removed the shared pointer utility package and updated related lint configuration.
    • Preserved existing runtime behavior, defaults, API contracts, and test expectations.

Walkthrough

The pull request replaces shared pointer helper calls with Go’s built-in new across production code, tests, operator defaults, resolvers, migration logic, sensor code, CLI tooling, and fixtures. It also removes the deleted helper package’s usages and updates the modernize linter configuration.

Changes

Pointer allocation migration

Layer / File(s) Summary
Central services and transition definitions
central/baseimage/..., central/complianceoperator/..., central/detection/..., central/sensor/...
Pointer helpers and local address-taking helpers are replaced with new, including transition state construction.
GraphQL resolver pointer construction
central/graphql/resolvers/...
Resolver query, pagination, filter, sort, and status pointers now use direct allocations.
Operator defaults and translation tests
operator/...
Operator defaults and test fixtures replace ptr.To and shared pointer helpers with new.
Package runtime and test utilities
pkg/..., migrator/..., compliance/...
Runtime pointer construction, PostgreSQL metadata allocation, migration updates, and related tests are updated.
CLI, sensor, and integration test cleanup
roxctl/..., sensor/..., tests/..., tools/...
CLI, sensor, Kubernetes fixture, integration-test, and tooling pointer creation is migrated to new.
Linter configuration
.golangci.yml
The disabled modernize check changes from newexpr to any.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: grimmimeloni

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: enabling the modernize/newexpr check.
Description check ✅ Passed The description explains the pointer-construction change, includes the required sections, and states that CI validated the change.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch modernize/newexpr

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit af61b61. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.12.x-656-gaf61b61351

@janisz
janisz force-pushed the modernize/rangeint branch from f5028b2 to 0c0eb42 Compare June 16, 2026 16:26
@janisz
janisz force-pushed the modernize/newexpr branch from 976a4fa to 300bb02 Compare June 16, 2026 16:27
Comment thread central/backgroundmigrations/runner/rollout_test.go Outdated
@janisz
janisz force-pushed the modernize/rangeint branch from 0c0eb42 to cd2ebea Compare June 17, 2026 07:18
@janisz
janisz force-pushed the modernize/newexpr branch from 300bb02 to 347a815 Compare June 17, 2026 07:18
@janisz
janisz force-pushed the modernize/rangeint branch from cd2ebea to 9f5379a Compare June 17, 2026 15:18
@janisz
janisz force-pushed the modernize/newexpr branch from 347a815 to 555024a Compare June 17, 2026 15:18
@janisz
janisz force-pushed the modernize/rangeint branch from 9f5379a to b92b69a Compare June 18, 2026 04:54
@janisz
janisz force-pushed the modernize/newexpr branch from 555024a to 87e73f2 Compare June 18, 2026 04:54
@janisz
janisz force-pushed the modernize/rangeint branch from b92b69a to b8b1ce7 Compare June 18, 2026 15:04
@janisz
janisz force-pushed the modernize/newexpr branch from 78fc490 to 10dea27 Compare July 14, 2026 16:16
@janisz
janisz force-pushed the modernize/rangeint branch from 19e6ed4 to eb8cc27 Compare July 15, 2026 10:16
@janisz
janisz force-pushed the modernize/newexpr branch from 10dea27 to bc199a8 Compare July 15, 2026 10:16
@janisz
janisz force-pushed the modernize/rangeint branch from eb8cc27 to 138870e Compare July 17, 2026 09:57
@janisz
janisz force-pushed the modernize/newexpr branch from bc199a8 to 61c23d4 Compare July 17, 2026 09:57
Base automatically changed from modernize/rangeint to master July 27, 2026 17:19
@janisz
janisz force-pushed the modernize/newexpr branch 2 times, most recently from 759b6ec to 075e1b8 Compare July 27, 2026 17:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@central/graphql/resolvers/image_vulnerabilities_benchmark_test.go`:
- Line 57: Update the Limit initialization in the benchmark test to use a typed
int32 value, such as a local int32 variable initialized to 20000 and passed by
address, or new(int32) with assignment; do not call new with the numeric
literal.

In `@central/graphql/resolvers/search_test.go`:
- Line 195: Update every test occurrence of Pagination.Limit using new(10) to
use new(int32(10)), ensuring the pointer type matches the field’s expected
*int32 type.

In `@tests/redhat_signing_key_test.go`:
- Around line 373-374: Update the DeploymentSpec.Replicas assignment in the test
to use an int32 value and pass its address, replacing new(1) with a
*int32-compatible expression.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: dd1ea72d-d1b5-484c-9ce3-5763288f5903

📥 Commits

Reviewing files that changed from the base of the PR and between e76b3f3 and 759b6ec.

📒 Files selected for processing (66)
  • .golangci.yml
  • central/backgroundmigrations/runner/rollout_test.go
  • central/baseimage/datastore/repository/datastore_impl_test.go
  • central/baseimage/service/service_impl.go
  • central/baseimage/service/service_impl_test.go
  • central/complianceoperator/v2/integration/datastore/datastore_impl_test.go
  • central/complianceoperator/v2/integration/service/convert_test.go
  • central/complianceoperator/v2/integration/service/service_impl_test.go
  • central/detection/alertmanager/alert_manager_impl_test.go
  • central/graphql/resolvers/deployments_test.go
  • central/graphql/resolvers/image_cve_core.go
  • central/graphql/resolvers/image_cve_core_test.go
  • central/graphql/resolvers/image_cve_v2_core_test.go
  • central/graphql/resolvers/image_vulnerabilities_benchmark_test.go
  • central/graphql/resolvers/image_vulnerabilities_v2_test.go
  • central/graphql/resolvers/images_test.go
  • central/graphql/resolvers/inputtypes/vuln_req_test.go
  • central/graphql/resolvers/node_cve_core.go
  • central/graphql/resolvers/node_cve_core_test.go
  • central/graphql/resolvers/platform_cve_core_test.go
  • central/graphql/resolvers/search_test.go
  • central/graphql/resolvers/utils.go
  • central/sensor/service/connection/upgradecontroller/stateutils/all_transitions.go
  • central/views/imagecve/view_test.go
  • central/views/imagecveflat/view_test.go
  • compliance/collection/kubernetes/kubelet.go
  • migrator/migrations/policymigrationhelper/policy_migrator.go
  • migrator/migrations/policymigrationhelper/postgres_policy_migrator_test.go
  • operator/api/v1alpha1/central_defaults_test.go
  • operator/api/v1alpha1/common_types_test.go
  • operator/internal/central/defaults/central_db.go
  • operator/internal/central/defaults/central_db_test.go
  • operator/internal/central/defaults/static.go
  • operator/internal/central/extensions/reconcile_central_db_password_test.go
  • operator/internal/central/extensions/reconcile_pvc_test.go
  • operator/internal/central/values/translation/translation_test.go
  • operator/internal/common/status/predicate_test.go
  • operator/internal/securedcluster/defaults/admission_controller.go
  • operator/internal/securedcluster/defaults/static.go
  • operator/internal/securedcluster/extensions/reconcile_defaulting_test.go
  • operator/internal/securedcluster/values/translation/translation_test.go
  • operator/internal/tlsprofile/convert_test.go
  • operator/internal/values/translation/translation_test.go
  • pkg/booleanpolicy/evaluator/evaluator_test.go
  • pkg/cluster/filtering.go
  • pkg/cluster/filtering_test.go
  • pkg/migratetooperator/central.go
  • pkg/migratetooperator/securedcluster.go
  • pkg/pointers/pointer.go
  • pkg/protoconv/resources/resources_test.go
  • pkg/search/postgres/common.go
  • pkg/search/postgres/query_metadata.go
  • roxctl/central/central.go
  • roxctl/common/flags/endpoint_test.go
  • roxctl/netpol/generate/generate_test.go
  • roxctl/sensor/generate/openshift.go
  • sensor/common/clusterentities/store_ips_test.go
  • sensor/kubernetes/complianceoperator/utils.go
  • sensor/kubernetes/fake/deployment.go
  • sensor/kubernetes/fake/virtualmachines_test.go
  • sensor/kubernetes/listener/resources/virtualmachine/store/virtual_machine_store_test.go
  • tests/common.go
  • tests/init_container_test.go
  • tests/ocp_dynamic_plugin_test.go
  • tests/redhat_signing_key_test.go
  • tools/retest/github.go
💤 Files with no reviewable changes (2)
  • pkg/pointers/pointer.go
  • .golangci.yml

Comment thread central/graphql/resolvers/image_vulnerabilities_benchmark_test.go Outdated
Comment thread central/graphql/resolvers/search_test.go Outdated
Comment thread tests/redhat_signing_key_test.go Outdated
@janisz
janisz force-pushed the modernize/newexpr branch 5 times, most recently from a376ccd to e698c20 Compare July 30, 2026 16:28
@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.22%. Comparing base (07596ed) to head (af61b61).
⚠️ Report is 17 commits behind head on master.

Files with missing lines Patch % Lines
central/graphql/resolvers/utils.go 0.00% 8 Missing ⚠️
...igrations/policymigrationhelper/policy_migrator.go 0.00% 5 Missing ⚠️
...al/securedcluster/defaults/admission_controller.go 0.00% 3 Missing ⚠️
sensor/kubernetes/fake/deployment.go 0.00% 3 Missing ⚠️
central/graphql/resolvers/image_cve_core.go 0.00% 2 Missing ⚠️
pkg/migratetooperator/central.go 85.71% 1 Missing ⚠️
pkg/search/postgres/query_metadata.go 83.33% 1 Missing ⚠️
roxctl/sensor/generate/openshift.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21189      +/-   ##
==========================================
- Coverage   51.26%   51.22%   -0.05%     
==========================================
  Files        2869     2868       -1     
  Lines      179230   179222       -8     
==========================================
- Hits        91883    91800      -83     
- Misses      79289    79347      +58     
- Partials     8058     8075      +17     
Flag Coverage Δ
go-unit-tests 51.22% <50.00%> (-0.05%) ⬇️

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

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@janisz
janisz force-pushed the modernize/newexpr branch from 267a40c to b676194 Compare July 31, 2026 09:44
@janisz
janisz marked this pull request as ready for review August 3, 2026 10:49
@janisz
janisz requested review from a team as code owners August 3, 2026 10:49
janisz and others added 5 commits August 3, 2026 13:03
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Enable modernize/newexpr linter rule and replace pointer helper
functions with Go 1.26's new(value) syntax across 29 files.

Replaces pointers.String/Bool/Int32/Int64/Float32/Pointer calls
and deletes 6 now-unused local pointer helper functions.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ptr/ptr32 pointer helpers with new(value) and delete
the now-unused functions. Missed in previous commit because
these files were added to master after the branch diverged.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>

@vikin91 vikin91 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked by machinery: "No correctness issues found in the scoped PR changes."

Manually checked the pkg & sensor folders - no issues.

@janisz

janisz commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

/retest

2 similar comments
@rhacs-bot

Copy link
Copy Markdown
Contributor

/retest

@rhacs-bot

Copy link
Copy Markdown
Contributor

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants