From 3f442309fc134320d3770af7b90f892336865249 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Fri, 14 Nov 2025 15:22:40 +0100 Subject: [PATCH 1/2] main: embed manifestgen.options into manifestOptions When we generate the osbuild manifest we need to take a bunch of commandline options into account. These are collected and passed via `manifestOptions` in the CLI. There is a (small) overlap with options that are then "converted" to options that need to be passed to `manifestgen` via `manifestgen.Options`. Previously there was manual code for this but its slightly nicer to just embedd the `manifestgen.Options` into the more general `manifestOptions` of the CLI. This avoid some boilerplate code and might serve as a useful pattern in other places. So this commit does that now (even though the wins are not that big). --- cmd/image-builder/main.go | 20 ++++++++++++++------ cmd/image-builder/manifest.go | 21 +++++---------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index 166e67d8..628ba36b 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -21,6 +21,7 @@ import ( "github.com/osbuild/images/pkg/customizations/subscription" "github.com/osbuild/images/pkg/distro/bootc" "github.com/osbuild/images/pkg/imagefilter" + "github.com/osbuild/images/pkg/manifestgen" "github.com/osbuild/images/pkg/osbuild" "github.com/osbuild/images/pkg/ostree" @@ -138,6 +139,9 @@ type cmdManifestWrapperOptions struct { useBootstrapIfNeeded bool } +// used in tests +var manifestgenDepsolver manifestgen.DepsolveFunc + func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []string, w io.Writer, wd io.Writer, wrapperOpts *cmdManifestWrapperOptions) (*imagefilter.Result, error) { if wrapperOpts == nil { wrapperOpts = &cmdManifestWrapperOptions{} @@ -304,27 +308,31 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st } opts := &manifestOptions{ + ManifestgenOptions: manifestgen.Options{ + Cachedir: rpmmdCacheDir, + CustomSeed: customSeed, + RpmDownloader: rpmDownloader, + DepsolveWarningsOutput: wd, + Depsolve: manifestgenDepsolver, + }, OutputDir: outputDir, OutputFilename: outputFilename, BlueprintPath: blueprintPath, Ostree: ostreeImgOpts, BootcRef: bootcRef, BootcInstallerPayloadRef: bootcInstallerPayloadRef, - RpmDownloader: rpmDownloader, WithSBOM: withSBOM, IgnoreWarnings: ignoreWarnings, - CustomSeed: customSeed, Subscription: subscription, - RpmmdCacheDir: rpmmdCacheDir, ForceRepos: forceRepos, } - opts.UseBootstrapContainer = wrapperOpts.useBootstrapIfNeeded && (img.ImgType.Arch().Name() != arch.Current().String()) - if opts.UseBootstrapContainer { + opts.ManifestgenOptions.UseBootstrapContainer = wrapperOpts.useBootstrapIfNeeded && (img.ImgType.Arch().Name() != arch.Current().String()) + if opts.ManifestgenOptions.UseBootstrapContainer { fmt.Fprintf(os.Stderr, "WARNING: using experimental cross-architecture building to build %q\n", img.ImgType.Arch().Name()) } - err = generateManifest(dataDir, extraRepos, img, w, wd, opts) + err = generateManifest(dataDir, extraRepos, img, w, opts) return img, err } diff --git a/cmd/image-builder/manifest.go b/cmd/image-builder/manifest.go index 249ae620..229b44d4 100644 --- a/cmd/image-builder/manifest.go +++ b/cmd/image-builder/manifest.go @@ -20,6 +20,8 @@ import ( ) type manifestOptions struct { + ManifestgenOptions manifestgen.Options + OutputDir string OutputFilename string BlueprintPath string @@ -30,11 +32,8 @@ type manifestOptions struct { RpmDownloader osbuild.RpmDownloader WithSBOM bool IgnoreWarnings bool - CustomSeed *int64 - RpmmdCacheDir string - ForceRepos []string - UseBootstrapContainer bool + ForceRepos []string } func sbomWriter(outputDir, filename string, content io.Reader) error { @@ -55,23 +54,13 @@ func sbomWriter(outputDir, filename string, content io.Reader) error { return f.Sync() } -// used in tests -var manifestgenDepsolver manifestgen.DepsolveFunc - // XXX: just return []byte instead of using output writer -func generateManifest(dataDir string, extraRepos []string, img *imagefilter.Result, output io.Writer, depsolveWarningsOutput io.Writer, opts *manifestOptions) error { +func generateManifest(dataDir string, extraRepos []string, img *imagefilter.Result, output io.Writer, opts *manifestOptions) error { repos, err := newRepoRegistry(dataDir, extraRepos) if err != nil { return err } - manifestGenOpts := &manifestgen.Options{ - DepsolveWarningsOutput: depsolveWarningsOutput, - RpmDownloader: opts.RpmDownloader, - UseBootstrapContainer: opts.UseBootstrapContainer, - CustomSeed: opts.CustomSeed, - Depsolve: manifestgenDepsolver, - Cachedir: opts.RpmmdCacheDir, - } + manifestGenOpts := &opts.ManifestgenOptions if opts.WithSBOM { outputDir := basenameFor(img, opts.OutputDir) manifestGenOpts.SBOMWriter = func(filename string, content io.Reader, docType sbom.StandardType) error { From 283f40711a03f1166ad89f86c26fd21fb72be063 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Wed, 17 Dec 2025 10:42:03 +0100 Subject: [PATCH 2/2] main: always use a bootstrap buildroot (but support disabling) We recently had two issues where creating our buildroot was broken in certain situations because the local rpm would not be compatible with the requirements of the package that got installed in the buildroot: https://issues.redhat.com/browse/RHEL-128741 https://github.com/osbuild/image-builder-cli/issues/413 The issue here is that the local rpm is something we do not control but we need a way to "bootstrap" our buildroot (once we have a buildroot we use that for everything else). This commit enables the "bootstrap" container feature of the images library by default to avoid this dependency. This means that we "podman pull" a minimal container (e.g. ubi for rhel) that contains python3 and rpm to then use it to install our real buildroot. Note that this enables it all the time even if the host distribution and target distribution match. The reason is rebuildability - ie. the same manifest should always produce the same result and in the general case we do not know if e.g. a manifest that was part of `image-builder build --with-manifest` is used somewhere else again. For restricted environment where pulling a container is a problem or for situations where its known that the host rpm is fine we provide: `--without-bootstrap-container` to disable this function. --- cmd/image-builder/main.go | 26 +++++++----------- cmd/image-builder/main_test.go | 48 +++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go index 628ba36b..70b85b02 100644 --- a/cmd/image-builder/main.go +++ b/cmd/image-builder/main.go @@ -135,17 +135,10 @@ func subscriptionImageOptions(cmd *cobra.Command) (*subscription.ImageOptions, e return regs.Redhat.Subscription, nil } -type cmdManifestWrapperOptions struct { - useBootstrapIfNeeded bool -} - // used in tests var manifestgenDepsolver manifestgen.DepsolveFunc -func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []string, w io.Writer, wd io.Writer, wrapperOpts *cmdManifestWrapperOptions) (*imagefilter.Result, error) { - if wrapperOpts == nil { - wrapperOpts = &cmdManifestWrapperOptions{} - } +func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []string, w io.Writer, wd io.Writer) (*imagefilter.Result, error) { dataDir, err := cmd.Flags().GetString("force-data-dir") if err != nil { return nil, err @@ -217,6 +210,10 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st if err != nil { return nil, err } + disableBootstrapContainer, err := cmd.Flags().GetBool("without-bootstrap-container") + if err != nil { + return nil, err + } bootcRef, err := cmd.Flags().GetString("bootc-ref") if err != nil { return nil, err @@ -314,6 +311,7 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st RpmDownloader: rpmDownloader, DepsolveWarningsOutput: wd, Depsolve: manifestgenDepsolver, + UseBootstrapContainer: !disableBootstrapContainer, }, OutputDir: outputDir, OutputFilename: outputFilename, @@ -327,8 +325,7 @@ func cmdManifestWrapper(pbar progress.ProgressBar, cmd *cobra.Command, args []st ForceRepos: forceRepos, } - opts.ManifestgenOptions.UseBootstrapContainer = wrapperOpts.useBootstrapIfNeeded && (img.ImgType.Arch().Name() != arch.Current().String()) - if opts.ManifestgenOptions.UseBootstrapContainer { + if img.ImgType.Arch().Name() != arch.Current().String() { fmt.Fprintf(os.Stderr, "WARNING: using experimental cross-architecture building to build %q\n", img.ImgType.Arch().Name()) } @@ -341,7 +338,7 @@ func cmdManifest(cmd *cobra.Command, args []string) error { if err != nil { return err } - _, err = cmdManifestWrapper(pbar, cmd, args, osStdout, io.Discard, nil) + _, err = cmdManifestWrapper(pbar, cmd, args, osStdout, io.Discard) return err } @@ -409,13 +406,9 @@ func cmdBuild(cmd *cobra.Command, args []string) error { }() var mf bytes.Buffer - opts := &cmdManifestWrapperOptions{ - useBootstrapIfNeeded: true, - } - // We discard any warnings from the depsolver until we figure out a better // idea (likely in manifestgen) - res, err := cmdManifestWrapper(pbar, cmd, args, &mf, io.Discard, opts) + res, err := cmdManifestWrapper(pbar, cmd, args, &mf, io.Discard) if err != nil { return err } @@ -584,6 +577,7 @@ operating systems like Fedora, CentOS and RHEL with easy customizations support. manifestCmd.Flags().Bool("ignore-warnings", false, `ignore warnings during manifest generation`) manifestCmd.Flags().String("registrations", "", `filename of a registrations file with e.g. subscription details`) manifestCmd.Flags().String("rpmmd-cache", "", `osbuild directory to cache rpm metadata`) + manifestCmd.Flags().Bool("without-bootstrap-container", false, `disable using a "bootstrap" container to generate the initial buildroot`) rootCmd.AddCommand(manifestCmd) uploadCmd := &cobra.Command{ diff --git a/cmd/image-builder/main_test.go b/cmd/image-builder/main_test.go index 6ce43916..bab340da 100644 --- a/cmd/image-builder/main_test.go +++ b/cmd/image-builder/main_test.go @@ -899,18 +899,60 @@ func TestBuildCrossArchSmoke(t *testing.T) { assert.NoError(t, err) pipelines, err := manifesttest.PipelineNamesFrom(manifest) assert.NoError(t, err) - crossArchPipeline := "bootstrap-buildroot" + bootstrapPipeline := "bootstrap-buildroot" crossArchWarning := `WARNING: using experimental cross-architecture building to build "aarch64"` + // the bootstrap pipeline is the default for all builds + assert.Contains(t, pipelines, bootstrapPipeline) if withCrossArch { - assert.Contains(t, pipelines, crossArchPipeline) assert.Contains(t, stderr, crossArchWarning) } else { - assert.NotContains(t, pipelines, crossArchPipeline) assert.NotContains(t, stderr, crossArchWarning) } } } +func TestManifestBootstrapContainer(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() + + for _, disableBootstrapContainer := range []bool{false, true} { + cmd := []string{ + "manifest", + "tar", + "--distro", "centos-10", + } + if disableBootstrapContainer { + cmd = append(cmd, "--without-bootstrap-container") + } + restore = main.MockOsArgs(cmd) + defer restore() + + var fakeStdout bytes.Buffer + restore = main.MockOsStdout(&fakeStdout) + defer restore() + + // XXX: capture stderr here too to ensure no cross build warning + err := main.Run() + assert.NoError(t, err) + + pipelines, err := manifesttest.PipelineNamesFrom(fakeStdout.Bytes()) + assert.NoError(t, err) + bootstrapPipeline := "bootstrap-buildroot" + if disableBootstrapContainer { + assert.NotContains(t, pipelines, bootstrapPipeline) + } else { + assert.Contains(t, pipelines, bootstrapPipeline) + } + } +} + func TestBuildIntegrationOutputFilename(t *testing.T) { if testing.Short() { t.Skip("manifest generation takes a while")