Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HMS-3141: image: add new ContainerBuildable flag to OSTreeDiskImage #287

Merged
merged 2 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/osbuild-playground/my-container.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (img *MyContainer) InstantiateManifest(m *manifest.Manifest,
// Let's create a simple OCI container!

// configure a build pipeline
build := manifest.NewBuild(m, runner, repos)
build := manifest.NewBuild(m, runner, repos, nil)
build.Checkpoint()

// create a minimal non-bootable OS tree
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-playground/my-image.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (img *MyImage) InstantiateManifest(m *manifest.Manifest,
// Let's create a simple raw image!

// configure a build pipeline
build := manifest.NewBuild(m, runner, repos)
build := manifest.NewBuild(m, runner, repos, nil)
build.Checkpoint()

// create an x86_64 platform with bios boot
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/anaconda_live_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (img *AnacondaLiveInstaller) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

livePipeline := manifest.NewAnacondaInstaller(m,
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/anaconda_ostree_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (img *AnacondaOSTreeInstaller) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

anacondaPipeline := manifest.NewAnacondaInstaller(m,
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/anaconda_tar_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (img *AnacondaTarInstaller) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

anacondaPipeline := manifest.NewAnacondaInstaller(m,
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (img *Archive) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

osPipeline := manifest.NewOS(m, buildPipeline, img.Platform, repos)
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (img *BaseContainer) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

osPipeline := manifest.NewOS(m, buildPipeline, img.Platform, repos)
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (img *DiskImage) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

osPipeline := manifest.NewOS(m, buildPipeline, img.Platform, repos)
Expand Down
15 changes: 15 additions & 0 deletions pkg/image/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package image

import (
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/rpmmd"
"github.com/osbuild/images/pkg/runner"
)

func MockManifestNewBuild(new func(m *manifest.Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) *manifest.Build) (restore func()) {
saved := manifestNewBuild
manifestNewBuild = new
return func() {
manifestNewBuild = saved
}
}
2 changes: 1 addition & 1 deletion pkg/image/ostree_archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (img *OSTreeArchive) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

osPipeline := manifest.NewOS(m, buildPipeline, img.Platform, repos)
Expand Down
2 changes: 1 addition & 1 deletion pkg/image/ostree_container.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (img *OSTreeContainer) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

osPipeline := manifest.NewOS(m, buildPipeline, img.Platform, repos)
Expand Down
9 changes: 8 additions & 1 deletion pkg/image/ostree_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ type OSTreeDiskImage struct {
// Lock the root account in the deployment unless the user defined root
// user options in the build configuration.
LockRoot bool

// Container buildable tweaks the buildroot to be container friendly,
// i.e. to not rely on an installed osbuild-selinux
ContainerBuildable bool
}

func NewOSTreeDiskImageFromCommit(commit ostree.SourceSpec) *OSTreeDiskImage {
Expand Down Expand Up @@ -102,11 +106,14 @@ func baseRawOstreeImage(img *OSTreeDiskImage, m *manifest.Manifest, buildPipelin
return manifest.NewRawOStreeImage(buildPipeline, osPipeline, img.Platform)
}

// replaced in testing
var manifestNewBuild = manifest.NewBuild

func (img *OSTreeDiskImage) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifestNewBuild(m, runner, repos, &manifest.BuildOptions{ContainerBuildable: img.ContainerBuildable})
buildPipeline.Checkpoint()

// don't support compressing non-raw images
Expand Down
61 changes: 61 additions & 0 deletions pkg/image/ostree_disk_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package image_test

import (
"math/rand"
"testing"

"github.com/stretchr/testify/require"

"github.com/osbuild/images/internal/workload"
"github.com/osbuild/images/pkg/container"
"github.com/osbuild/images/pkg/image"
"github.com/osbuild/images/pkg/manifest"
"github.com/osbuild/images/pkg/platform"
"github.com/osbuild/images/pkg/rpmmd"
"github.com/osbuild/images/pkg/runner"
)

func TestOSTreeDiskImageManifestSetsContainerBuildable(t *testing.T) {
rng := rand.New(rand.NewSource(0)) // nolint:gosec

repos := []rpmmd.RepoConfig{}
r := &runner.Fedora{Version: 39}

ref := "ostree/1/1/0"
ondrejbudai marked this conversation as resolved.
Show resolved Hide resolved
containerSource := container.SourceSpec{
Source: "source-spec",
Name: "name",
}

var buildOpts []*manifest.BuildOptions
restore := image.MockManifestNewBuild(func(m *manifest.Manifest, r runner.Runner, repos []rpmmd.RepoConfig, opts *manifest.BuildOptions) *manifest.Build {
buildOpts = append(buildOpts, opts)
return manifest.NewBuild(m, r, repos, opts)
})
defer restore()

for _, containerBuildable := range []bool{true, false} {
buildOpts = nil

mf := manifest.New()
img := image.NewOSTreeDiskImageFromContainer(containerSource, ref)
require.NotNil(t, img)
img.Platform = &platform.X86{
BasePlatform: platform.BasePlatform{
ImageFormat: platform.FORMAT_QCOW2,
},
BIOS: true,
UEFIVendor: "fedora",
}
img.Workload = &workload.BaseWorkload{}
img.OSName = "osname"
img.ContainerBuildable = containerBuildable

_, err := img.InstantiateManifest(&mf, repos, r, rng)
require.Nil(t, err)
require.NotNil(t, img)

require.Equal(t, len(buildOpts), 1)
require.Equal(t, buildOpts[0].ContainerBuildable, containerBuildable)
}
}
2 changes: 1 addition & 1 deletion pkg/image/ostree_simplified_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (img *OSTreeSimplifiedInstaller) InstantiateManifest(m *manifest.Manifest,
repos []rpmmd.RepoConfig,
runner runner.Runner,
rng *rand.Rand) (*artifact.Artifact, error) {
buildPipeline := manifest.NewBuild(m, runner, repos)
buildPipeline := manifest.NewBuild(m, runner, repos, nil)
buildPipeline.Checkpoint()

imageFilename := "image.raw.xz"
Expand Down
20 changes: 19 additions & 1 deletion pkg/manifest/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,31 @@ type Build struct {
dependents []Pipeline
repos []rpmmd.RepoConfig
packageSpecs []rpmmd.PackageSpec

containerBuildable bool
}

type BuildOptions struct {
// ContainerBuildable tweaks the buildroot to be container friendly,
// i.e. to not rely on an installed osbuild-selinux
ContainerBuildable bool
}

// NewBuild creates a new build pipeline from the repositories in repos
// and the specified packages.
func NewBuild(m *Manifest, runner runner.Runner, repos []rpmmd.RepoConfig) *Build {
func NewBuild(m *Manifest, runner runner.Runner, repos []rpmmd.RepoConfig, opts *BuildOptions) *Build {
if opts == nil {
opts = &BuildOptions{}
}

name := "build"
pipeline := &Build{
Base: NewBase(m, name, nil),
runner: runner,
dependents: make([]Pipeline, 0),
repos: filterRepos(repos, name),

containerBuildable: opts.ContainerBuildable,
}
m.addPipeline(pipeline)
return pipeline
Expand Down Expand Up @@ -109,6 +123,10 @@ func (p *Build) getSELinuxLabels() map[string]string {
switch pkg.Name {
case "coreutils":
labels["/usr/bin/cp"] = "system_u:object_r:install_exec_t:s0"
if p.containerBuildable {
labels["/usr/bin/mount"] = "system_u:object_r:install_exec_t:s0"
labels["/usr/bin/umount"] = "system_u:object_r:install_exec_t:s0"
}
case "tar":
labels["/usr/bin/tar"] = "system_u:object_r:install_exec_t:s0"
}
Expand Down
84 changes: 84 additions & 0 deletions pkg/manifest/build_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package manifest

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/osbuild/images/pkg/rpmmd"
"github.com/osbuild/images/pkg/runner"
)

func TestBuildContainerBuildableNo(t *testing.T) {
repos := []rpmmd.RepoConfig{}
mf := New()
runner := &runner.Fedora{Version: 39}

build := NewBuild(&mf, runner, repos, nil)
require.NotNil(t, build)

for _, tc := range []struct {
packageSpec []rpmmd.PackageSpec
containerBuildable bool
expectedSELinuxLabels map[string]string
}{
// no pkgs means no selinux labels (container build or not)
{
[]rpmmd.PackageSpec{},
false,
map[string]string{},
},
{
[]rpmmd.PackageSpec{},
true,
map[string]string{},
},
{
[]rpmmd.PackageSpec{{Name: "coreutils"}},
false,
map[string]string{
"/usr/bin/cp": "system_u:object_r:install_exec_t:s0",
},
},
{
[]rpmmd.PackageSpec{{Name: "tar"}},
false,
map[string]string{
"/usr/bin/tar": "system_u:object_r:install_exec_t:s0",
},
},
{
[]rpmmd.PackageSpec{{Name: "coreutils"}, {Name: "tar"}},
false,
map[string]string{
"/usr/bin/cp": "system_u:object_r:install_exec_t:s0",
"/usr/bin/tar": "system_u:object_r:install_exec_t:s0",
},
},
{
[]rpmmd.PackageSpec{{Name: "coreutils"}},
true,
map[string]string{
"/usr/bin/cp": "system_u:object_r:install_exec_t:s0",
"/usr/bin/mount": "system_u:object_r:install_exec_t:s0",
"/usr/bin/umount": "system_u:object_r:install_exec_t:s0",
},
},
{
[]rpmmd.PackageSpec{{Name: "coreutils"}, {Name: "tar"}},
true,
map[string]string{
"/usr/bin/cp": "system_u:object_r:install_exec_t:s0",
"/usr/bin/mount": "system_u:object_r:install_exec_t:s0",
"/usr/bin/umount": "system_u:object_r:install_exec_t:s0",
"/usr/bin/tar": "system_u:object_r:install_exec_t:s0",
},
},
} {
build.packageSpecs = tc.packageSpec
build.containerBuildable = tc.containerBuildable

labels := build.getSELinuxLabels()
require.Equal(t, labels, tc.expectedSELinuxLabels)
}
}
2 changes: 1 addition & 1 deletion pkg/manifest/os_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func NewTestOS() *OS {
repos := []rpmmd.RepoConfig{}
manifest := New()
runner := &runner.Fedora{Version: 37}
build := NewBuild(&manifest, runner, repos)
build := NewBuild(&manifest, runner, repos, nil)
build.Checkpoint()

// create an x86_64 platform with bios boot
Expand Down
Loading