ibcli: add new --output-name flag

This commit adds a new `--output-name` flag that will rename
the resulting artifact after it was build. All auxillary artifacts
like buildlog, sbom etc are also name based on the same basename.

See also https://github.com/osbuild/images/pull/1039 for how
this could be simpler (especially the fake osbuild).

Closes: https://github.com/osbuild/image-builder-cli/issues/43
This commit is contained in:
Michael Vogt 2025-03-13 17:30:59 +01:00 committed by Simon de Vlieger
parent 8635a22ad9
commit ccb4269b62
6 changed files with 194 additions and 60 deletions

View file

@ -4,31 +4,34 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"github.com/osbuild/bootc-image-builder/bib/pkg/progress"
"github.com/osbuild/images/pkg/imagefilter"
)
type buildOptions struct {
OutputDir string
StoreDir string
OutputDir string
StoreDir string
OutputBasename string
WriteManifest bool
WriteBuildlog bool
}
func buildImage(pbar progress.ProgressBar, res *imagefilter.Result, osbuildManifest []byte, opts *buildOptions) error {
func buildImage(pbar progress.ProgressBar, res *imagefilter.Result, osbuildManifest []byte, opts *buildOptions) (string, error) {
if opts == nil {
opts = &buildOptions{}
}
basename := basenameFor(res, opts.OutputBasename)
if opts.WriteManifest {
p := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.osbuild-manifest.json", outputNameFor(res)))
p := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.osbuild-manifest.json", basename))
if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil {
return err
return "", err
}
if err := os.WriteFile(p, osbuildManifest, 0644); err != nil {
return err
return "", err
}
}
@ -38,16 +41,32 @@ func buildImage(pbar progress.ProgressBar, res *imagefilter.Result, osbuildManif
}
if opts.WriteBuildlog {
if err := os.MkdirAll(opts.OutputDir, 0755); err != nil {
return fmt.Errorf("cannot create buildlog base directory: %w", err)
return "", fmt.Errorf("cannot create buildlog base directory: %w", err)
}
p := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.buildlog", outputNameFor(res)))
p := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.buildlog", basename))
f, err := os.Create(p)
if err != nil {
return fmt.Errorf("cannot create buildlog: %w", err)
return "", fmt.Errorf("cannot create buildlog: %w", err)
}
defer f.Close()
osbuildOpts.BuildLog = f
}
return progress.RunOSBuild(pbar, osbuildManifest, res.ImgType.Exports(), osbuildOpts)
if err := progress.RunOSBuild(pbar, osbuildManifest, res.ImgType.Exports(), osbuildOpts); err != nil {
return "", err
}
// Rename *sigh*, see https://github.com/osbuild/images/pull/1039
// for my preferred way. Every frontend to images has to duplicate
// similar code like this.
pipelineDir := filepath.Join(opts.OutputDir, res.ImgType.Exports()[0])
srcName := filepath.Join(pipelineDir, res.ImgType.Filename())
imgExt := strings.SplitN(res.ImgType.Filename(), ".", 2)[1]
dstName := filepath.Join(opts.OutputDir, fmt.Sprintf("%s.%v", basename, imgExt))
if err := os.Rename(srcName, dstName); err != nil {
return "", fmt.Errorf("cannot rename artifact to final name: %w", err)
}
// best effort, remove the now empty pipeline export dir from osbuild
_ = os.Remove(pipelineDir)
return dstName, nil
}

View file

@ -7,7 +7,6 @@ import (
"io"
"os"
"os/signal"
"path/filepath"
"syscall"
"github.com/sirupsen/logrus"
@ -28,8 +27,12 @@ var (
osStderr io.Writer = os.Stderr
)
// generate the default output directory name for the given image
func outputNameFor(img *imagefilter.Result) string {
// basenameFor returns the basename for directory and filenames
// for the given imageType. This can be user overriden via userBasename.
func basenameFor(img *imagefilter.Result, userBasename string) string {
if userBasename != "" {
return userBasename
}
return fmt.Sprintf("%s-%s-%s", img.Distro.Name(), img.ImgType.Name(), img.Arch.Name())
}
@ -123,11 +126,14 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st
if useLibrepo {
rpmDownloader = osbuild.RpmDownloaderLibrepo
}
blueprintPath, err := cmd.Flags().GetString("blueprint")
if err != nil {
return nil, err
}
// no error check here as this is (deliberately) not defined on
// "manifest" (if "images" learn to set the output filename in
// manifests we would change this
outputFilename, _ := cmd.Flags().GetString("output-name")
bp, err := blueprintload.Load(blueprintPath)
if err != nil {
@ -158,13 +164,17 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st
return nil, err
}
}
if len(img.ImgType.Exports()) > 1 {
return nil, fmt.Errorf("image %q has multiple exports: this is current unsupport: please report this as a bug", basenameFor(img, ""))
}
opts := &manifestOptions{
OutputDir: outputDir,
BlueprintPath: blueprintPath,
Ostree: ostreeImgOpts,
RpmDownloader: rpmDownloader,
WithSBOM: withSBOM,
OutputDir: outputDir,
OutputFilename: outputFilename,
BlueprintPath: blueprintPath,
Ostree: ostreeImgOpts,
RpmDownloader: rpmDownloader,
WithSBOM: withSBOM,
ForceRepos: forceRepos,
}
@ -206,6 +216,10 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
if err != nil {
return err
}
outputBasename, err := cmd.Flags().GetString("output-name")
if err != nil {
return err
}
withManifest, err := cmd.Flags().GetBool("with-manifest")
if err != nil {
return err
@ -255,20 +269,18 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
return err
}
}
outputDir = basenameFor(res, outputDir)
// XXX: support output filename via commandline (c.f.
// https://github.com/osbuild/images/pull/1039)
if outputDir == "" {
outputDir = outputNameFor(res)
}
buildOpts := &buildOptions{
OutputDir: outputDir,
StoreDir: cacheDir,
WriteManifest: withManifest,
WriteBuildlog: withBuildlog,
OutputDir: outputDir,
OutputBasename: outputBasename,
StoreDir: cacheDir,
WriteManifest: withManifest,
WriteBuildlog: withBuildlog,
}
pbar.SetPulseMsgf("Image building step")
if err := buildImage(pbar, res, mf.Bytes(), buildOpts); err != nil {
imagePath, err := buildImage(pbar, res, mf.Bytes(), buildOpts)
if err != nil {
return err
}
pbar.Stop()
@ -276,8 +288,6 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
if uploader != nil {
// XXX: integrate better into the progress, see bib
imagePath := filepath.Join(outputDir, res.ImgType.Name(), res.ImgType.Filename())
if err := uploadImageWithProgress(uploader, imagePath); err != nil {
return err
}
@ -395,6 +405,7 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support.
// XXX: add "--verbose" here, similar to how bib is doing this
// (see https://github.com/osbuild/bootc-image-builder/pull/790/commits/5cec7ffd8a526e2ca1e8ada0ea18f927695dfe43)
buildCmd.Flags().String("progress", "auto", "type of progress bar to use (e.g. verbose,term)")
buildCmd.Flags().String("output-name", "", "set specific output basename")
rootCmd.AddCommand(buildCmd)
buildCmd.Flags().AddFlagSet(uploadCmd.Flags())
// add after the rest of the uploadCmd flag set is added to avoid

View file

@ -314,6 +314,46 @@ func TestManifestIntegrationOstreeSmokeErrors(t *testing.T) {
}
}
// this is needed because images currently hardcodes the artifact filenames
// so we need to faithfully reproduce this in our tests. see images PR#1039
// for an alternative way that would make this unneeded.
func makeFakeOsbuildScript() string {
return `
cat - > "$0".stdin
output_dir=""
export=""
while [[ $# -gt 0 ]]; do
key="$1"
case $key in
--output-directory)
output_dir="$2"
shift 2
;;
--export)
export="$2"
shift 2
;;
*)
shift 1
esac
done
mkdir -p "$output_dir/$export"
case $export in
qcow2)
echo "fake-img-qcow2" > "$output_dir/$export/disk.qcow2"
;;
image)
echo "fake-img-raw" > "$output_dir/$export/image.raw"
;;
*)
echo "Unknown export: $1 - add to testscript"
exit 1
;;
esac
`
}
func TestBuildIntegrationHappy(t *testing.T) {
if testing.Short() {
t.Skip("manifest generation takes a while")
@ -330,6 +370,14 @@ func TestBuildIntegrationHappy(t *testing.T) {
defer restore()
tmpdir := t.TempDir()
curdir, err := os.Getwd()
require.NoError(t, err)
err = os.Chdir(tmpdir)
require.NoError(t, err)
defer func() {
os.Chdir(curdir)
}()
restore = main.MockOsArgs([]string{
"build",
"qcow2",
@ -339,11 +387,11 @@ func TestBuildIntegrationHappy(t *testing.T) {
})
defer restore()
script := `cat - > "$0".stdin`
script := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", script)
defer fakeOsbuildCmd.Restore()
err := main.Run()
err = main.Run()
assert.NoError(t, err)
assert.Contains(t, fakeStdout.String(), `Image build successful, result in "centos-9-qcow2-x86_64"`+"\n")
@ -420,7 +468,7 @@ func TestBuildIntegrationArgs(t *testing.T) {
restore = main.MockOsArgs(cmd)
defer restore()
script := `cat - > "$0".stdin`
script := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", script)
defer fakeOsbuildCmd.Restore()
@ -436,7 +484,9 @@ func TestBuildIntegrationArgs(t *testing.T) {
// ensure we get exactly the expected files
files, err := filepath.Glob(outputDir + "/*")
assert.NoError(t, err)
assert.Equal(t, len(tc.expectedFiles), len(files), files)
// we always have the qcow2 dir
expectedFiles := append(tc.expectedFiles, "qcow")
assert.Equal(t, len(expectedFiles), len(files), files)
for _, expected := range tc.expectedFiles {
_, err = os.Stat(filepath.Join(outputDir, expected))
assert.NoError(t, err, fmt.Sprintf("file %q missing", expected))
@ -466,11 +516,13 @@ func TestBuildIntegrationErrorsProgressVerbose(t *testing.T) {
restore := main.MockNewRepoRegistry(testrepos.New)
defer restore()
outputDir := t.TempDir()
restore = main.MockOsArgs([]string{
"build",
"qcow2",
"--distro", "centos-9",
"--progress=verbose",
"--output-dir", outputDir,
})
defer restore()
@ -787,10 +839,11 @@ func TestBuildCrossArchCheckSkippedOnExperimentalBuildroot(t *testing.T) {
"--distro", "centos-9",
"--cache", tmpdir,
"--arch=s390x",
"--output-dir", tmpdir,
})
defer restore()
script := `cat - > "$0".stdin`
script := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", script)
defer fakeOsbuildCmd.Restore()
@ -804,3 +857,57 @@ func TestBuildCrossArchCheckSkippedOnExperimentalBuildroot(t *testing.T) {
}
}
}
func TestBuildIntegrationOutputFilename(t *testing.T) {
if testing.Short() {
t.Skip("manifest generation takes a while")
}
if !hasDepsolveDnf() {
t.Skip("no osbuild-depsolve-dnf binary found")
}
restore := main.MockNewRepoRegistry(testrepos.New)
defer restore()
var fakeStdout bytes.Buffer
restore = main.MockOsStdout(&fakeStdout)
defer restore()
tmpdir := t.TempDir()
outputDir := filepath.Join(tmpdir, "output")
restore = main.MockOsArgs([]string{
"build",
"qcow2",
fmt.Sprintf("--blueprint=%s", makeTestBlueprint(t, testBlueprint)),
"--distro", "centos-9",
"--cache", tmpdir,
"--output-dir", outputDir,
"--output-name=foo",
"--with-manifest",
"--with-sbom",
"--with-buildlog",
})
defer restore()
script := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", script)
defer fakeOsbuildCmd.Restore()
err := main.Run()
assert.NoError(t, err)
expectedFiles := []string{
"foo.buildroot-build.spdx.json",
"foo.image-os.spdx.json",
"foo.osbuild-manifest.json",
"foo.buildlog",
"foo.qcow2",
}
files, err := filepath.Glob(outputDir + "/*")
assert.NoError(t, err)
assert.Equal(t, len(expectedFiles), len(files), files)
for _, expected := range expectedFiles {
_, err = os.Stat(filepath.Join(outputDir, expected))
assert.NoError(t, err, fmt.Sprintf("file %q missing from %v", expected, files))
}
}

View file

@ -1,9 +1,11 @@
package main
import (
"fmt"
"io"
"os"
"path/filepath"
"strings"
"github.com/osbuild/images/pkg/distro"
"github.com/osbuild/images/pkg/imagefilter"
@ -16,11 +18,12 @@ import (
)
type manifestOptions struct {
OutputDir string
BlueprintPath string
Ostree *ostree.ImageOptions
RpmDownloader osbuild.RpmDownloader
WithSBOM bool
OutputDir string
OutputFilename string
BlueprintPath string
Ostree *ostree.ImageOptions
RpmDownloader osbuild.RpmDownloader
WithSBOM bool
ForceRepos []string
}
@ -52,11 +55,12 @@ func generateManifest(dataDir string, extraRepos []string, img *imagefilter.Resu
RpmDownloader: opts.RpmDownloader,
}
if opts.WithSBOM {
outputDir := opts.OutputDir
if outputDir == "" {
outputDir = outputNameFor(img)
}
outputDir := basenameFor(img, opts.OutputDir)
overrideSBOMBase := strings.SplitN(opts.OutputFilename, ".", 2)[0]
manifestGenOpts.SBOMWriter = func(filename string, content io.Reader, docType sbom.StandardType) error {
if overrideSBOMBase != "" {
filename = fmt.Sprintf("%s.%s", overrideSBOMBase, strings.SplitN(filename, ".", 2)[1])
}
return sbomWriter(outputDir, filename, content)
}
}

View file

@ -121,12 +121,6 @@ func TestUploadCmdlineErrors(t *testing.T) {
}
}
var fakeOsbuildScriptAmiFmt = `#!/bin/sh -e
cat - > "$0".stdin
mkdir -p %[1]s/ami
echo -n %[2]s > %[1]s/ami/image.raw
`
func TestBuildAndUploadWithAWSMock(t *testing.T) {
if testing.Short() {
t.Skip("manifest generation takes a while")
@ -145,9 +139,8 @@ func TestBuildAndUploadWithAWSMock(t *testing.T) {
})
defer restore()
fakeDiskContent := "fake-raw-img"
outputDir := t.TempDir()
fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
fakeOsbuildScript := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
defer fakeOsbuildCmd.Restore()
@ -174,7 +167,7 @@ func TestBuildAndUploadWithAWSMock(t *testing.T) {
assert.Equal(t, amiName, "aws-ami-3")
assert.Equal(t, 1, fa.checkCalls)
assert.Equal(t, 1, fa.uploadAndRegisterCalls)
assert.Equal(t, fakeDiskContent, fa.uploadAndRegisterRead.String())
assert.Equal(t, "fake-img-raw\n", fa.uploadAndRegisterRead.String())
}
func TestBuildAmiButNotUpload(t *testing.T) {
@ -193,9 +186,8 @@ func TestBuildAmiButNotUpload(t *testing.T) {
})
defer restore()
fakeDiskContent := "fake-raw-img"
outputDir := t.TempDir()
fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
fakeOsbuildScript := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
defer fakeOsbuildCmd.Restore()
@ -225,9 +217,8 @@ func TestBuildAndUploadWithAWSPartialCmdlineErrors(t *testing.T) {
t.Skip("no osbuild-depsolve-dnf binary found")
}
fakeDiskContent := "fake-raw-img"
outputDir := t.TempDir()
fakeOsbuildScript := fmt.Sprintf(fakeOsbuildScriptAmiFmt, outputDir, fakeDiskContent)
fakeOsbuildScript := makeFakeOsbuildScript()
fakeOsbuildCmd := testutil.MockCommand(t, "osbuild", fakeOsbuildScript)
defer fakeOsbuildCmd.Restore()

View file

@ -22,7 +22,8 @@ def test_container_builds_image(tmp_path, build_container, use_librepo):
f"--use-librepo={use_librepo}",
])
arch = "x86_64"
assert (output_dir / f"centos-9-minimal-raw-{arch}/xz/disk.raw.xz").exists()
basename = f"centos-9-minimal-raw-{arch}"
assert (output_dir / basename / f"{basename}.raw.xz").exists()
# XXX: ensure no other leftover dirs
dents = os.listdir(output_dir)
assert len(dents) == 1, f"too many dentries in output dir: {dents}"
@ -88,8 +89,9 @@ def test_container_with_progress(tmp_path, build_fake_container, progress, needl
"-v", f"{output_dir}:/output",
build_fake_container,
"build",
"minimal-raw",
"qcow2",
"--distro", "centos-9",
"--output-dir=.",
f"--progress={progress}",
], text=True)
assert needle in output