From 9af0c95c59d5e12d7732c1d941a0b168b4eae0b0 Mon Sep 17 00:00:00 2001 From: Deepak Bhagat Date: Tue, 18 Aug 2026 02:48:20 +0530 Subject: [PATCH 1/3] butane: use friendly filename in stdin read error The stdin read error used infile.Name(), which is "/dev/stdin" on Linux, instead of the already-computed friendly filename (""). Refactor input reading into readInput() and report the friendly name on read failures. Fixes coreos/ignition#2281 Addresses coreos/butane#726 Signed-off-by: Deepak Bhagat --- butane/internal/main.go | 36 ++++++---- butane/internal/main_test.go | 128 +++++++++++++++++++++++++++++++++++ docs/release-notes.md | 1 + 3 files changed, 151 insertions(+), 14 deletions(-) create mode 100644 butane/internal/main_test.go diff --git a/butane/internal/main.go b/butane/internal/main.go index 98dd97427..f9c24770b 100644 --- a/butane/internal/main.go +++ b/butane/internal/main.go @@ -40,6 +40,26 @@ func isCharDevice(f *os.File) bool { return stat.Mode()&os.ModeCharDevice != 0 } +func readInput(input string) ([]byte, string, error) { + infile := os.Stdin + filename := "" + if input != "" { + f, err := os.Open(input) + if err != nil { + return nil, input, fmt.Errorf("failed to open %s: %w", input, err) + } + defer func() { _ = f.Close() }() + infile = f + filename = input + } + + data, err := io.ReadAll(infile) + if err != nil { + return nil, filename, fmt.Errorf("failed to read %s: %w", filename, err) + } + return data, filename, nil +} + func main() { var ( input string @@ -111,21 +131,9 @@ func main() { os.Exit(0) } - infile := os.Stdin - filename := "" - if input != "" { - var err error - infile, err = os.Open(input) - if err != nil { - fail("failed to open %s: %v\n", input, err) - } - defer func() { _ = infile.Close() }() - filename = input - } - - dataIn, err := io.ReadAll(infile) + dataIn, filename, err := readInput(input) if err != nil { - fail("failed to read %s: %v\n", infile.Name(), err) + fail("%v\n", err) } dataOut, r, err := config.TranslateBytes(dataIn, options) diff --git a/butane/internal/main_test.go b/butane/internal/main_test.go new file mode 100644 index 000000000..1d0d9d9b6 --- /dev/null +++ b/butane/internal/main_test.go @@ -0,0 +1,128 @@ +// Copyright 2019 Red Hat, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "os" + "path/filepath" + "testing" +) + +func TestReadInput(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T) (input string, cleanup func()) + wantData []byte + wantErr bool + }{ + { + name: "stdin", + setup: func(t *testing.T) (string, func()) { + content := []byte("hello from stdin") + orig := os.Stdin + tmp, err := os.CreateTemp("", "butane-stdin") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + if _, err := tmp.Write(content); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } + if _, err := tmp.Seek(0, 0); err != nil { + t.Fatalf("failed to seek temp file: %v", err) + } + os.Stdin = tmp + return "", func() { + os.Stdin = orig + tmp.Close() + os.Remove(tmp.Name()) + } + }, + wantData: []byte("hello from stdin"), + wantErr: false, + }, + { + name: "empty stdin", + setup: func(t *testing.T) (string, func()) { + orig := os.Stdin + tmp, err := os.CreateTemp("", "butane-stdin-empty") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + if _, err := tmp.Seek(0, 0); err != nil { + t.Fatalf("failed to seek temp file: %v", err) + } + os.Stdin = tmp + return "", func() { + os.Stdin = orig + tmp.Close() + os.Remove(tmp.Name()) + } + }, + wantData: []byte{}, + wantErr: false, + }, + { + name: "file", + setup: func(t *testing.T) (string, func()) { + dir := t.TempDir() + path := filepath.Join(dir, "input.bu") + content := []byte("variant: fcos") + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + return path, func() {} + }, + wantData: []byte("variant: fcos"), + wantErr: false, + }, + { + name: "missing file", + setup: func(t *testing.T) (string, func()) { + missing := filepath.Join(t.TempDir(), "does-not-exist") + return missing, func() {} + }, + wantData: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + input, cleanup := tt.setup(t) + defer cleanup() + + data, filename, err := readInput(input) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + } else if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if string(data) != string(tt.wantData) { + t.Errorf("expected data %q, got %q", tt.wantData, data) + } + + wantName := "" + if input != "" { + wantName = input + } + if filename != wantName { + t.Errorf("expected filename %q, got %q", wantName, filename) + } + }) + } +} diff --git a/docs/release-notes.md b/docs/release-notes.md index 138c962ba..c7d18b760 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -28,6 +28,7 @@ nav_order: 9 - Fix the Butane root partition constraint check to only examine subsequent partitions ([#2304](https://github.com/coreos/ignition/pull/2304)) - Close `/proc/mounts` after checking whether block devices are mounted, preventing file descriptor leaks during disk setup ([#2289](https://github.com/coreos/ignition/pull/2289)) - Drop supplementary groups when dropping privileges to write files as a user ([#2242](https://github.com/coreos/ignition/issues/2242)) +- butane: report the friendly input name (`` instead of `/dev/stdin`) in stdin read errors ## Ignition 2.27.0 (2026-08-26) From bb8babc3a6382230bf1e68d7def1540a959e98ea Mon Sep 17 00:00:00 2001 From: Deepak Bhagat Date: Wed, 16 Sep 2026 17:44:26 +0530 Subject: [PATCH 2/3] butane: check errors from temp file cleanup in stdin read test The errcheck linter flags the unchecked Close and Remove calls in the TestReadInput cleanup closures. Surface those errors via the test logger so the cleanup fails loudly instead of being silently dropped. Signed-off-by: Deepak Bhagat --- butane/internal/main_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/butane/internal/main_test.go b/butane/internal/main_test.go index 1d0d9d9b6..2379de0eb 100644 --- a/butane/internal/main_test.go +++ b/butane/internal/main_test.go @@ -45,8 +45,12 @@ func TestReadInput(t *testing.T) { os.Stdin = tmp return "", func() { os.Stdin = orig - tmp.Close() - os.Remove(tmp.Name()) + if err := tmp.Close(); err != nil { + t.Errorf("failed to close temp file: %v", err) + } + if err := os.Remove(tmp.Name()); err != nil { + t.Errorf("failed to remove temp file: %v", err) + } } }, wantData: []byte("hello from stdin"), @@ -66,8 +70,12 @@ func TestReadInput(t *testing.T) { os.Stdin = tmp return "", func() { os.Stdin = orig - tmp.Close() - os.Remove(tmp.Name()) + if err := tmp.Close(); err != nil { + t.Errorf("failed to close temp file: %v", err) + } + if err := os.Remove(tmp.Name()); err != nil { + t.Errorf("failed to remove temp file: %v", err) + } } }, wantData: []byte{}, From 2bab8cc2230151b3ba0c00b38eb95922e88f43bb Mon Sep 17 00:00:00 2001 From: Deepak Bhagat Date: Wed, 16 Sep 2026 21:54:52 +0530 Subject: [PATCH 3/3] butane: add stdin read error test case Add a regression test that assigns a closed *os.File to os.Stdin and calls readInput with an empty input path, asserting the wrapped error contains the friendly name. This exercises the io.ReadAll error path that the existing cases did not cover. Signed-off-by: Deepak Bhagat --- butane/internal/main_test.go | 39 ++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/butane/internal/main_test.go b/butane/internal/main_test.go index 2379de0eb..12e0edf85 100644 --- a/butane/internal/main_test.go +++ b/butane/internal/main_test.go @@ -17,15 +17,17 @@ package main import ( "os" "path/filepath" + "strings" "testing" ) func TestReadInput(t *testing.T) { tests := []struct { - name string - setup func(t *testing.T) (input string, cleanup func()) - wantData []byte - wantErr bool + name string + setup func(t *testing.T) (input string, cleanup func()) + wantData []byte + wantErr bool + wantErrMsg string }{ { name: "stdin", @@ -104,6 +106,30 @@ func TestReadInput(t *testing.T) { wantData: nil, wantErr: true, }, + { + name: "stdin read error", + setup: func(t *testing.T) (string, func()) { + orig := os.Stdin + closed, err := os.CreateTemp("", "butane-closed-stdin") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + name := closed.Name() + if err := closed.Close(); err != nil { + t.Fatalf("failed to close temp file: %v", err) + } + os.Stdin = closed + return "", func() { + os.Stdin = orig + if err := os.Remove(name); err != nil { + t.Errorf("failed to remove temp file: %v", err) + } + } + }, + wantData: nil, + wantErr: true, + wantErrMsg: "failed to read :", + }, } for _, tt := range tests { @@ -116,6 +142,11 @@ func TestReadInput(t *testing.T) { if err == nil { t.Fatalf("expected error, got nil") } + if tt.wantErrMsg != "" { + if !strings.Contains(err.Error(), tt.wantErrMsg) { + t.Errorf("expected error containing %q, got %q", tt.wantErrMsg, err.Error()) + } + } } else if err != nil { t.Fatalf("unexpected error: %v", err) }