bug: Dead validation code: local ignition config files never validated
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 974
- Forks
- 296
- Avg merge
- 6d 14h
- Merged PRs (30d)
- 9
Description
Created originally in Butane by @deepak0x: https://github.com/coreos/butane/issues/724
name: release checklist
about: release checklist template
title: Dead validation code: local ignition config files never validated
labels: jira,kind/bug
warning: |
⚠️ Template generated by https://github.com/coreos/repo-templates; do not edit downstream
Bug Report
File: base/v0_8_exp/translate.go, line 157 (same pattern at base/v0_7/translate.go:153)
Summary:
translateResource contains dead validation code. The strings.HasPrefix guard at line 157 is always false, so ValidateIgnitionConfig is never called for local files — even when those files are used in ignition.config.merge or ignition.config.replace contexts. Invalid or corrupt ignition config data in a local field passes through translation silently with no warning or error.
Buggy code
if from.Local != nil {
c := path.New("yaml", "local")
contents, err := baseutil.ReadLocalFile(*from.Local, options.FilesDir)
if err != nil {
r.AddOnError(c, err)
return
}
// Validating the contents of the local file from here since there is no way to
// get both the filename and filedirectory in the Validate context
if strings.HasPrefix(c.String(), "$.ignition.config") { // ← always false
rp, err := ValidateIgnitionConfig(c, contents)
r.Merge(rp)
if err != nil {
return
}
}
...
}
Root cause
c is constructed as path.New("yaml", "local"). The first argument is the tag; the second is a path element.
path.ContextPath.String() (defined in vendor/github.com/coreos/vcontext/path/path.go:36) builds its output by joining path elements with dots, prefixed with $. The tag is not included in the string representation.
Therefore:
path.New("yaml", "local").String() → "$.local"
strings.HasPrefix("$.local", "$.ignition.config") → false
The condition can never be true regardless of where translateResource is called from. ValidateIgnitionConfig is dead code for all local field inputs.
Impact
Local files referenced via the local field inside ignition.config.merge or ignition.config.replace blocks are never validated as Ignition configs at translation time. Malformed or invalid ignition config JSON/YAML in those files is silently accepted and encoded into the output without any diagnostic.
Reproduction
Confirmed by evaluating the vendored vcontext library directly:
import "github.com/coreos/vcontext/path"
import "strings"
c := path.New("yaml", "local")
fmt.Println(c.String()) // "$.local"
fmt.Println(strings.HasPrefix(c.String(), "$.ignition.config")) // false
path.New("yaml", "local").String() returns "$.local", which does not satisfy strings.HasPrefix(..., "$.ignition.config").
The same dead-code pattern exists independently at base/v0_7/translate.go:153.
Tagging:
- Confirm
path.ContextPath.String()does not include the tag in any vcontext version vendored by this repo - Confirm
ValidateIgnitionConfigis unreachable forlocalfield inputs in bothbase/v0_7andbase/v0_8_exp - Confirm no test currently exercises the
ValidateIgnitionConfigbranch via alocalfield
Fedora packaging:
- No response
GitHub release:
- No response
Quay release:
- No response
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with base/v0_8_exp/translate.go:157 and base/v0_7/translate.go:153, then read vendor/github.com/coreos/vcontext/path/path.go:36 to confirm how the context path is rendered. Check whether tests exercise local fields in ignition.config.merge or ignition.config.replace. Done means malformed local Ignition config data is diagnosed during translation in both versions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100