Skip to content

Commit

Permalink
distro/rhel84: use a random uuid for XFS partition
Browse files Browse the repository at this point in the history
Imagine this situation: You have a RHEL system booted from an image produced
by osbuild-composer. On this system, you want to use osbuild-composer to
create another image of RHEL.

However, there's currently something funny with partitions:

All RHEL images built by osbuild-composer contain a root xfs partition. The
interesting bit is that they all share the same xfs partition UUID. This might
sound like a good thing for reproducibility but it has a quirk.

The issue appears when osbuild runs the qemu assembler: it needs to mount all
partitions of the future image to copy the OS tree into it.

Imagine that osbuild-composer is running on a system booted from an imaged
produced by osbuild-composer. This means that its root xfs partition has this
uuid:

efe8afea-c0a8-45dc-8e6e-499279f6fa5d

When osbuild-composer builds an image on this system, it runs osbuild that
runs the qemu assembler at some point. As I said previously, it will mount
all partitions of the future image. That means that it will also try to
mount the root xfs partition with this uuid:

efe8afea-c0a8-45dc-8e6e-499279f6fa5d

Do you remember this one? Yeah, it's the same one as before. However, the xfs
kernel driver doesn't like that. It contains a global table[1] of all xfs
partitions that forbids to mount 2 xfs partitions with the same uuid.

I mean... uuids are meant to be unique, right?

This commit changes the way we build RHEL 8.4 images: Each one now has a
unique uuid. It's now literally a unique universally unique identifier. haha

[1]: https://github.com/torvalds/linux/blob/a349e4c659609fd20e4beea89e5c4a4038e33a95/fs/xfs/xfs_mount.c#L51
  • Loading branch information
ondrejbudai committed Dec 15, 2020
1 parent ae0d1b8 commit 973639d
Show file tree
Hide file tree
Showing 23 changed files with 150 additions and 114 deletions.
5 changes: 4 additions & 1 deletion cmd/osbuild-pipeline/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ type rpmMD struct {
func main() {
var rpmmdArg bool
flag.BoolVar(&rpmmdArg, "rpmmd", false, "output rpmmd struct instead of pipeline manifest")
var seedArg int64
flag.Int64Var(&seedArg, "seed", 0, "seed for generating manifests (default: 0)")
flag.Parse()

// Path to composeRequet or '-' for stdin
Expand Down Expand Up @@ -152,7 +154,8 @@ func main() {
},
repos,
packageSpecs,
buildPackageSpecs)
buildPackageSpecs,
seedArg)
if err != nil {
panic(err.Error())
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-store-dump/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func getManifest(bp blueprint.Blueprint, t distro.ImageType, a distro.Arch, d di
if err != nil {
panic(err)
}
manifest, err := t.Manifest(bp.Customizations, distro.ImageOptions{}, repos, pkgs, buildPkgs)
manifest, err := t.Manifest(bp.Customizations, distro.ImageOptions{}, repos, pkgs, buildPkgs, 0)
if err != nil {
panic(err)
}
Expand Down
6 changes: 5 additions & 1 deletion internal/cloudapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package cloudapi
import (
"encoding/json"
"fmt"
"math/rand"
"net/http"

"github.com/go-chi/chi"
Expand Down Expand Up @@ -81,6 +82,9 @@ func (server *Server) Compose(w http.ResponseWriter, r *http.Request) {
imageRequests := make([]imageRequest, len(request.ImageRequests))
var targets []*target.Target

// use the same seed for all images so we get the same IDs
manifestSeed := rand.Int63()

for i, ir := range request.ImageRequests {
arch, err := distribution.GetArch(ir.Architecture)
if err != nil {
Expand Down Expand Up @@ -139,7 +143,7 @@ func (server *Server) Compose(w http.ResponseWriter, r *http.Request) {
}
}

manifest, err := imageType.Manifest(nil, imageOptions, repositories, packages, buildPackages)
manifest, err := imageType.Manifest(nil, imageOptions, repositories, packages, buildPackages, manifestSeed)
if err != nil {
http.Error(w, fmt.Sprintf("Failed to get manifest for for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err), http.StatusBadRequest)
return
Expand Down
2 changes: 1 addition & 1 deletion internal/distro/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type ImageType interface {
// Returns an osbuild manifest, containing the sources and pipeline necessary
// to build an image, given output format with all packages and customizations
// specified in the given blueprint.
Manifest(b *blueprint.Customizations, options ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (Manifest, error)
Manifest(b *blueprint.Customizations, options ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec, seed int64) (Manifest, error)
}

// The ImageOptions specify options for a specific image build
Expand Down
5 changes: 4 additions & 1 deletion internal/distro/distro_test_common/distro_test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"github.com/stretchr/testify/require"
)

const RandomTestSeed = 0

func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, distros ...distro.Distro) {
assert := assert.New(t)
fileNames, err := filepath.Glob(filepath.Join(pipelinePath, prefix))
Expand Down Expand Up @@ -89,7 +91,8 @@ func TestDistro_Manifest(t *testing.T, pipelinePath string, prefix string, distr
},
repos,
tt.RpmMD.Packages,
tt.RpmMD.BuildPackages)
tt.RpmMD.BuildPackages,
RandomTestSeed)

if (err == nil && tt.Manifest == nil) || (err != nil && tt.Manifest != nil) {
t.Errorf("distro.Manifest() error = %v", err)
Expand Down
3 changes: 2 additions & 1 deletion internal/distro/fedora32/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec,
seed int64) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
if err != nil {
return distro.Manifest{}, err
Expand Down
3 changes: 2 additions & 1 deletion internal/distro/fedora33/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec,
seed int64) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
if err != nil {
return distro.Manifest{}, err
Expand Down
3 changes: 2 additions & 1 deletion internal/distro/fedoratest/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec,
seed int64) (distro.Manifest, error) {

return json.Marshal(
osbuild.Manifest{
Expand Down
3 changes: 2 additions & 1 deletion internal/distro/rhel8/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
buildPackageSpecs []rpmmd.PackageSpec,
seed int64) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
if err != nil {
return distro.Manifest{}, err
Expand Down
36 changes: 26 additions & 10 deletions internal/distro/rhel84/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"math/rand"
"sort"

"github.com/osbuild/osbuild-composer/internal/disk"
Expand Down Expand Up @@ -50,7 +52,7 @@ type imageType struct {
bootable bool
rpmOstree bool
defaultSize uint64
partitionTableGenerator func(imageOptions distro.ImageOptions, arch distro.Arch) disk.PartitionTable
partitionTableGenerator func(imageOptions distro.ImageOptions, arch distro.Arch, rng *rand.Rand) disk.PartitionTable
assembler func(pt *disk.PartitionTable, options distro.ImageOptions, arch distro.Arch) *osbuild.Assembler
}

Expand Down Expand Up @@ -188,8 +190,11 @@ func (t *imageType) Manifest(c *blueprint.Customizations,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
packageSpecs,
buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs)
buildPackageSpecs []rpmmd.PackageSpec,
seed int64) (distro.Manifest, error) {
source := rand.NewSource(seed)
rng := rand.New(source)
pipeline, err := t.pipeline(c, options, repos, packageSpecs, buildPackageSpecs, rng)
if err != nil {
return distro.Manifest{}, err
}
Expand Down Expand Up @@ -230,10 +235,10 @@ func sources(packages []rpmmd.PackageSpec) *osbuild.Sources {
}
}

func (t *imageType) pipeline(c *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (*osbuild.Pipeline, error) {
func (t *imageType) pipeline(c *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec, rng *rand.Rand) (*osbuild.Pipeline, error) {
var pt *disk.PartitionTable
if t.partitionTableGenerator != nil {
table := t.partitionTableGenerator(options, t.arch)
table := t.partitionTableGenerator(options, t.arch, rng)
pt = &table
}

Expand Down Expand Up @@ -497,7 +502,7 @@ func (t *imageType) selinuxStageOptions() *osbuild.SELinuxStageOptions {
}
}

func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch) disk.PartitionTable {
func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch, rng *rand.Rand) disk.PartitionTable {
if arch.Name() == "x86_64" {
return disk.PartitionTable{
Size: imageOptions.Size,
Expand Down Expand Up @@ -531,7 +536,7 @@ func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch) d
UUID: "6264D520-3FB9-423F-8AB8-7A0A8E3D3562",
Filesystem: &disk.Filesystem{
Type: "xfs",
UUID: "efe8afea-c0a8-45dc-8e6e-499279f6fa5d",
UUID: uuid.Must(newRandomUUIDFromReader(rng)).String(),
Label: "root",
Mountpoint: "/",
FSTabOptions: "defaults",
Expand Down Expand Up @@ -567,7 +572,7 @@ func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch) d
UUID: "6264D520-3FB9-423F-8AB8-7A0A8E3D3562",
Filesystem: &disk.Filesystem{
Type: "xfs",
UUID: "efe8afea-c0a8-45dc-8e6e-499279f6fa5d",
UUID: uuid.Must(newRandomUUIDFromReader(rng)).String(),
Label: "root",
Mountpoint: "/",
FSTabOptions: "defaults",
Expand All @@ -592,7 +597,7 @@ func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch) d
Start: 10240,
Filesystem: &disk.Filesystem{
Type: "xfs",
UUID: "efe8afea-c0a8-45dc-8e6e-499279f6fa5d",
UUID: uuid.Must(newRandomUUIDFromReader(rng)).String(),
Mountpoint: "/",
FSTabOptions: "defaults",
FSTabFreq: 0,
Expand All @@ -612,7 +617,7 @@ func defaultPartitionTable(imageOptions distro.ImageOptions, arch distro.Arch) d
Bootable: true,
Filesystem: &disk.Filesystem{
Type: "xfs",
UUID: "efe8afea-c0a8-45dc-8e6e-499279f6fa5d",
UUID: uuid.Must(newRandomUUIDFromReader(rng)).String(),
Mountpoint: "/",
FSTabOptions: "defaults",
FSTabFreq: 0,
Expand Down Expand Up @@ -673,6 +678,17 @@ func ostreeCommitAssembler(options distro.ImageOptions, arch distro.Arch) *osbui
)
}

func newRandomUUIDFromReader(r io.Reader) (uuid.UUID, error) {
var id uuid.UUID
_, err := io.ReadFull(r, id[:])
if err != nil {
return uuid.Nil, err
}
id[6] = (id[6] & 0x0f) | 0x40 // Version 4
id[8] = (id[8] & 0x3f) | 0x80 // Variant is 10
return id, nil
}

// New creates a new distro object, defining the supported architectures and image types
func New() distro.Distro {
const GigaByte = 1024 * 1024 * 1024
Expand Down
2 changes: 1 addition & 1 deletion internal/distro/test_distro/distro.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (t *TestImageType) BuildPackages() []string {
return nil
}

func (t *TestImageType) Manifest(b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec) (distro.Manifest, error) {
func (t *TestImageType) Manifest(b *blueprint.Customizations, options distro.ImageOptions, repos []rpmmd.RepoConfig, packageSpecs, buildPackageSpecs []rpmmd.PackageSpec, seed int64) (distro.Manifest, error) {
return json.Marshal(
osbuild.Manifest{
Sources: osbuild.Sources{},
Expand Down
6 changes: 5 additions & 1 deletion internal/kojiapi/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"log"
"math/rand"
"net/http"
"strings"
"time"
Expand Down Expand Up @@ -87,6 +88,9 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error {
kojiFilenames := make([]string, len(request.ImageRequests))
kojiDirectory := "osbuild-composer-koji-" + uuid.New().String()

// use the same seed for all images so we get the same IDs
manifestSeed := rand.Int63()

for i, ir := range request.ImageRequests {
arch, err := d.GetArch(ir.Architecture)
if err != nil {
Expand Down Expand Up @@ -119,7 +123,7 @@ func (h *apiHandlers) PostCompose(ctx echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Failed to depsolve build packages for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err))
}

manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, repositories, packages, buildPackages)
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, repositories, packages, buildPackages, manifestSeed)
if err != nil {
return echo.NewHTTPError(http.StatusBadGateway, fmt.Sprintf("Failed to get manifest for for %s/%s/%s: %s", ir.ImageType, ir.Architecture, request.Distribution, err))
}
Expand Down
4 changes: 2 additions & 2 deletions internal/store/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func FixtureBase() *Store {
if err != nil {
panic("invalid image type qcow2 for x86_64 @ fedoratest")
}
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil, 0)
if err != nil {
panic("could not create manifest")
}
Expand Down Expand Up @@ -160,7 +160,7 @@ func FixtureFinished() *Store {
if err != nil {
panic("invalid image type qcow2 for x86_64 @ fedoratest")
}
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil, 0)
if err != nil {
panic("could not create manifest")
}
Expand Down
2 changes: 1 addition & 1 deletion internal/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (suite *storeTest) SetupSuite() {
suite.myDistro = test_distro.New()
suite.myArch, _ = suite.myDistro.GetArch("test_arch")
suite.myImageType, _ = suite.myArch.GetImageType("test_type")
suite.myManifest, _ = suite.myImageType.Manifest(&suite.myCustomizations, suite.myImageOptions, suite.myRepoConfig, nil, suite.myPackageSpec)
suite.myManifest, _ = suite.myImageType.Manifest(&suite.myCustomizations, suite.myImageOptions, suite.myRepoConfig, nil, suite.myPackageSpec, 0)
suite.mySourceConfig = SourceConfig{
Name: "testSourceConfig",
}
Expand Down
4 changes: 3 additions & 1 deletion internal/weldr/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"log"
"math/rand"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -1846,7 +1847,8 @@ func (api *API) composeHandler(writer http.ResponseWriter, request *http.Request
},
api.allRepositories(),
packages,
buildPackages)
buildPackages,
rand.Int63())
if err != nil {
errors := responseError{
ID: "ManifestCreationFailed",
Expand Down
2 changes: 1 addition & 1 deletion internal/weldr/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ func TestCompose(t *testing.T) {
require.NoError(t, err)
imgType, err := arch.GetImageType("qcow2")
require.NoError(t, err)
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil)
manifest, err := imgType.Manifest(nil, distro.ImageOptions{}, nil, nil, nil, 0)
require.NoError(t, err)
expectedComposeLocal := &store.Compose{
Blueprint: &blueprint.Blueprint{
Expand Down
8 changes: 4 additions & 4 deletions internal/worker/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestCreate(t *testing.T) {
if err != nil {
t.Fatalf("error getting image type from arch")
}
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil, 0)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
Expand All @@ -84,7 +84,7 @@ func TestCancel(t *testing.T) {
if err != nil {
t.Fatalf("error getting image type from arch")
}
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil, 0)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
Expand Down Expand Up @@ -121,7 +121,7 @@ func TestUpdate(t *testing.T) {
if err != nil {
t.Fatalf("error getting image type from arch")
}
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil, 0)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
Expand Down Expand Up @@ -152,7 +152,7 @@ func TestUpload(t *testing.T) {
if err != nil {
t.Fatalf("error getting image type from arch")
}
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil)
manifest, err := imageType.Manifest(nil, distro.ImageOptions{Size: imageType.Size(0)}, nil, nil, nil, 0)
if err != nil {
t.Fatalf("error creating osbuild manifest")
}
Expand Down
Loading

0 comments on commit 973639d

Please sign in to comment.