Skip to content

config: replace deprecated reflect.Ptr references - #2356

Merged
prestist merged 1 commit into
coreos:mainfrom
cadejacobson:pointer
Nov 3, 2026
Merged

prestist merged 1 commit into
coreos:mainfrom
cadejacobson:pointer

Conversation

@cadejacobson

Copy link
Copy Markdown
Contributor

reflect.Ptr has been deprecated since Golang 1.18, withreflect.Pointer taking its place. Ignition tests for Golang versions 1.25 and 1.26, so the codebase should target the newer Pointer declaration.

These were recommended by the go fix pipeline output for version 1.26, with a link to one example below. There are a large number of these, so I will not link every reference the way I have in other PRs.
Link to most recent Go version 1.26.x CI job on main with reflect.Ptr flag

References:

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 15e68a3 and dfa031e.

Files selected for processing: 10
  • butane/config/util/filter.go
  • butane/translate/translate.go
  • butane/translate/util.go
  • config/config_test.go
  • config/doc/generate.go
  • config/merge/merge.go
  • config/translate/translate.go
  • config/util/reflection.go
  • docs/release-notes.d/reflect-pointer.change.md
  • internal/exec/stages/fetch_offline/fetch-offline.go

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Recent review details
Context from checks skipped due to timeout: 4
  • GitHub Check: CodeRabbit
  • GitHub Check: Test (1.26.x)
  • GitHub Check: Test (1.25.x)
  • GitHub Check: tmt-tests
Additional context used
Code guidelines: 2
AGENTS.md — auto-discovered
.github/ISSUE_TEMPLATE/stabilize-checklist.md — auto-discovered
Path-based instructions: 7
Execution stages are fixed (fetch-offline, fetch, disks, mount, files, umount).

CodeRabbit configuration file

Files:

  • internal/exec/stages/fetch_offline/fetch-offline.go
The config/ directory is the frontend stable library API consumed by external programs (e.g., Butane).

CodeRabbit configuration file

Files:

  • config/doc/generate.go
  • config/config_test.go
  • config/translate/translate.go
  • config/merge/merge.go
  • config/util/reflection.go
Documentation served via GitHub Pages/Jekyll.

CodeRabbit configuration file

Files:

  • docs/release-notes.d/reflect-pointer.change.md
Source excerpt: **Frontend is stable API** -- `config/` is used by external programs.

CodeRabbit inference engine (AGENTS.md)

Files:

  • config/doc/generate.go
  • config/config_test.go
  • config/translate/translate.go
  • config/merge/merge.go
  • config/util/reflection.go
Source excerpt: **Formatting**: `gofmt` enforced (CI and `./test`) Source excerpt: **License header**: Required on all `.go` files (Apache 2.0, 13-line header) Source excerpt: **Imports**: stdlib, blank line, project packages, blank line, e...

CodeRabbit inference engine (AGENTS.md)

Files:

  • butane/translate/util.go
  • butane/translate/translate.go
  • config/doc/generate.go
  • config/config_test.go
  • internal/exec/stages/fetch_offline/fetch-offline.go
  • config/translate/translate.go
  • config/merge/merge.go
  • butane/config/util/filter.go
  • config/util/reflection.go
Applicability reference: AGENTS.md: # Ignition [Spec Stabilization Checklist](.github/ISSUE_TEMPLATE/stabilize-checklist.md) Source excerpt: [ ] Update `config/config_test.go` to add the new experimental version to `TestConfigStructure`.

CodeRabbit inference engine (.github/ISSUE_TEMPLATE/stabilize-checklist.md)

Files:

  • config/config_test.go
Source excerpt: **Pattern**: Table-driven tests with `struct{ in, out }` slices

CodeRabbit inference engine (AGENTS.md)

Files:

  • config/config_test.go
Additional comments: 10
butane/config/util/filter.go: 1

82-82: LGTM!

Also applies to: 121-121, 154-154

config/config_test.go: 1

66-66: LGTM!

Also applies to: 123-123, 167-167, 242-242

config/doc/generate.go: 1

93-93: LGTM!

config/util/reflection.go: 1

51-51: LGTM!

Also applies to: 78-78

config/merge/merge.go: 1

236-236: LGTM!

Also applies to: 243-243, 246-246, 374-374

butane/translate/translate.go: 1

61-61: LGTM!

Also applies to: 124-124, 262-262

butane/translate/util.go: 1

54-54: LGTM!

config/translate/translate.go: 1

49-49: LGTM!

Also applies to: 101-101, 181-181

internal/exec/stages/fetch_offline/fetch-offline.go: 1

120-120: LGTM!

docs/release-notes.d/reflect-pointer.change.md: 1

1-1: LGTM!

Walkthrough

Reflection kind checks across config and translation code now use reflect.Pointer instead of reflect.Ptr. A release note documents the replacement. Pointer handling remains unchanged.

Changes: Reflection pointer-kind updates

Layer / File(s) Summary
Replace pointer-kind checks across reflection paths
butane/config/util/filter.go, butane/translate/*, config/config_test.go, config/doc/generate.go, config/merge/merge.go, config/translate/translate.go, config/util/reflection.go, internal/exec/stages/fetch_offline/fetch-offline.go, docs/release-notes.d/reflect-pointer.change.md
Reflection kind checks use reflect.Pointer in config validation, translation, merging, path traversal, and offline inspection. Existing pointer handling is unchanged. The release note directs users to replace deprecated reflect.Ptr references.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Refactor

Possibly related issues

  • coreos/ignition#2353: This PR adds a release-note fragment using the workflow described by the issue.

Possibly related PRs

  • coreos/ignition#2346: This PR replaces the deprecated reflect.Ptr uses reported by the go fix pipeline.
  • coreos/ignition#2352: This PR adds a release-note fragment under the directory required by this PR.
  • coreos/ignition#2347: This PR adds a release-note fragment using the per-PR format introduced there.

Suggested reviewers: prestist, angelcerveraroldan

Merge Risk: Minimal · up to dfa03

The change is a behavior-preserving replacement compatible with the repository's supported Go versions.

Pre-merge checks: Passed 5 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 5 passed
Check name Status Explanation
Title check Passed The title follows the required format. It uses the subsystem prefix "config:", a lowercase imperative description, and no trailing period. It accurately summarizes the replacement of deprecated `refle…
Description check Passed The description directly explains the replacement of deprecated reflect.Ptr references with reflect.Pointer and provides relevant Go version context and references.
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.
Commit Message Convention Passed The PR has one non-merge commit: config: replace reflect.Ptr with reflect.Pointer. It has a valid subsystem, a lowercase imperative description starting with replace, and no trailing period.
Full details: Title check

Explanation

The title follows the required format. It uses the subsystem prefix "config:", a lowercase imperative description, and no trailing period. It accurately summarizes the replacement of deprecated reflect.Ptr references.

Full details: Docstring Coverage

Explanation

Docstring coverage is 54.55% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 9 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@cadejacobson cadejacobson changed the title config: replace reflect.Ptr with reflect.Pointer config: replace deprecated reflect.Ptr references Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Binary size report (bin/amd64/ignition)

Size
Base (main) 33MiB
PR (#2356) 33MiB
Delta +0B (0.00%)

@prestist prestist left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Locally reviewed: converted all non-vendor reflect.Ptr uses, go.mod targets 1.25 so reflect.Pointer is available, release-note fragment follows docs/release-notes.d convention. No behavior change. LGTM.

@prestist
prestist merged commit 8c825c8 into coreos:main Nov 3, 2026
14 checks passed
@cadejacobson
cadejacobson deleted the pointer branch November 3, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants