Skip to content

Commit

Permalink
pkg: make seed in ImageType.Manifest() random by default
Browse files Browse the repository at this point in the history
This commit changes the API of `ImageType.Manifest()` to take
a pointer to the seed value. If no seed value is given it will
automatically generate a random one.

This avoid the pitfall that the consumer needs to know about
properly seeding the manifests. The default use-case is to
generate non-fixed seed manifests so this commit it easier.
  • Loading branch information
mvo5 committed Dec 13, 2024
1 parent 53548a4 commit 7ed5bab
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 65 deletions.
2 changes: 1 addition & 1 deletion cmd/build/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func makeManifest(
return nil, err
}

manifest, warnings, err := imgType.Manifest(&bp, options, repos, seedArg)
manifest, warnings, err := imgType.Manifest(&bp, options, repos, &seedArg)
if err != nil {
return nil, fmt.Errorf("[ERROR] manifest generation failed: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/gen-manifests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func makeManifestJob(
}()
msgq <- fmt.Sprintf("Starting job %s", filename)

manifest, _, err := imgType.Manifest(&bp, options, repos, seedArg)
manifest, _, err := imgType.Manifest(&bp, options, repos, &seedArg)
if err != nil {
err = fmt.Errorf("[%s] failed: %s", filename, err)
return
Expand Down
2 changes: 1 addition & 1 deletion cmd/osbuild-package-sets/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func main() {
URL: "https://example.com", // required by some image types
}
}
manifest, _, err := image.Manifest(&blueprint.Blueprint{}, options, nil, 0)
manifest, _, err := image.Manifest(&blueprint.Blueprint{}, options, nil, nil)
if err != nil {
panic(err)
}
Expand Down
13 changes: 12 additions & 1 deletion pkg/distro/distro.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package distro

import (
"time"

"github.com/osbuild/images/pkg/blueprint"
"github.com/osbuild/images/pkg/customizations/subscription"
"github.com/osbuild/images/pkg/disk"
Expand Down Expand Up @@ -120,7 +122,9 @@ type ImageType interface {
// specified in the given blueprint; it also returns any warnings (e.g.
// deprecation notices) generated by the manifest.
// The packageSpecSets must be labelled in the same way as the originating PackageSets.
Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed int64) (*manifest.Manifest, []string, error)
// A custom seed for the rng can be specified, if nil the seed will
// be random.
Manifest(bp *blueprint.Blueprint, options ImageOptions, repos []rpmmd.RepoConfig, seed *int64) (*manifest.Manifest, []string, error)
}

// The ImageOptions specify options for a specific image build
Expand Down Expand Up @@ -155,3 +159,10 @@ func ExportsFallback() []string {
func PayloadPackageSets() []string {
return []string{}
}

func SeedFrom(p *int64) int64 {
if p == nil {
return time.Now().UnixNano()
}
return *p
}
8 changes: 4 additions & 4 deletions pkg/distro/distro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestImageTypePipelineNames(t *testing.T) {
repoSets[plName] = repos
}

m, _, err := imageType.Manifest(&bp, options, repos, seed)
m, _, err := imageType.Manifest(&bp, options, repos, &seed)
assert.NoError(err)

containers := make(map[string][]container.Spec, 0)
Expand Down Expand Up @@ -416,7 +416,7 @@ func TestPipelineRepositories(t *testing.T) {
}

repos := tCase.repos
manifest, _, err := imageType.Manifest(&bp, options, repos, 0)
manifest, _, err := imageType.Manifest(&bp, options, repos, nil)
require.NoError(err)
packageSets := manifest.GetPackageSetChains()

Expand Down Expand Up @@ -569,7 +569,7 @@ func TestDistro_ManifestFIPSWarning(t *testing.T) {
if strings.HasSuffix(imgTypeName, "simplified-installer") {
bp.Customizations.InstallationDevice = "/dev/dummy"
}
_, warn, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, warn, err := imgType.Manifest(&bp, imgOpts, nil, nil)
if err != nil {
assert.True(t, slices.Contains(noCustomizableImages, imgTypeName))
assert.Equal(t, err, fmt.Errorf(distro.NoCustomizationsAllowedError, imgTypeName))
Expand Down Expand Up @@ -626,7 +626,7 @@ func TestOSTreeOptionsErrorForNonOSTreeImgTypes(t *testing.T) {
},
}

_, _, err = imageType.Manifest(&bp, options, nil, 0)
_, _, err = imageType.Manifest(&bp, options, nil, nil)
if imageType.OSTreeRef() == "" {
assert.Errorf(err,
"OSTree options should not be allowed for non-OSTree image type %s/%s/%s",
Expand Down
14 changes: 7 additions & 7 deletions pkg/distro/distro_test_common/distro_test_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func kernelCount(imgType distro.ImageType, bp blueprint.Blueprint) int {
}
}

manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, 0)
manifest, _, err := imgType.Manifest(&bp, distro.ImageOptions{OSTree: ostreeOptions}, nil, nil)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -220,7 +220,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
}
options := distro.ImageOptions{OSTree: &ostreeOptions}

m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
ostreeOptions.URL = "https://example.com/repo"
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -320,7 +320,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -364,7 +364,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -418,7 +418,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
URL: "https://example.com/repo",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
m, _, err := imgType.Manifest(bp, options, nil, 0)
m, _, err := imgType.Manifest(bp, options, nil, nil)
assert.NoError(err)

nrefs := 0
Expand Down Expand Up @@ -470,7 +470,7 @@ func TestDistro_OSTreeOptions(t *testing.T, d distro.Distro) {
ParentRef: "test/x86_64/02",
}
options := distro.ImageOptions{OSTree: &ostreeOptions}
_, _, err = imgType.Manifest(bp, options, nil, 0)
_, _, err = imgType.Manifest(bp, options, nil, nil)
assert.Error(err)
}
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/distro/fedora/distro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func TestImageType_BuildPackages(t *testing.T) {
if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) {
continue
}
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, 0)
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
buildPkgs := manifest.GetPackageSetChains()["build"]
assert.NotNil(t, buildPkgs)
Expand Down Expand Up @@ -507,7 +507,7 @@ func TestDistro_ManifestError(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "kernel boot parameter customizations are not supported for ostree types")
} else if imgTypeName == "iot-installer" || imgTypeName == "iot-simplified-installer" {
Expand Down Expand Up @@ -738,7 +738,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -772,7 +772,7 @@ func TestDistro_TestRootMountPoint(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -810,7 +810,7 @@ func TestDistro_CustomFileSystemSubDirectories(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -852,7 +852,7 @@ func TestDistro_MountpointsWithArbitraryDepthAllowed(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -890,7 +890,7 @@ func TestDistro_DirtyMountpointsNotAllowed(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "iot-") || strings.HasPrefix(imgTypeName, "image-") {
continue
} else if imgTypeName == "live-installer" {
Expand Down Expand Up @@ -920,7 +920,7 @@ func TestDistro_CustomUsrPartitionNotLargeEnough(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -965,7 +965,7 @@ func TestDistro_PartitioningConflict(t *testing.T) {
arch, _ := fedoraDistro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if imgTypeName == "iot-commit" || imgTypeName == "iot-container" || imgTypeName == "iot-bootable-container" {
assert.EqualError(t, err, "Custom mountpoints and partitioning are not supported for ostree types")
} else if imgTypeName == "iot-raw-image" || imgTypeName == "iot-qcow2-image" {
Expand Down Expand Up @@ -1063,7 +1063,7 @@ func TestDistro_DiskCustomizationRunsValidateLayoutConstraints(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
assert.EqualError(t, err, "multiple LVM volume groups are not yet supported")
})
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/distro/fedora/imagetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ func (t *imageType) PartitionType() disk.PartitionTableType {
func (t *imageType) Manifest(bp *blueprint.Blueprint,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
seed int64) (*manifest.Manifest, []string, error) {
seedp *int64) (*manifest.Manifest, []string, error) {
seed := distro.SeedFrom(seedp)

warnings, err := t.checkOptions(bp, options)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion pkg/distro/rhel/imagetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,8 @@ func (t *ImageType) PartitionType() disk.PartitionTableType {
func (t *ImageType) Manifest(bp *blueprint.Blueprint,
options distro.ImageOptions,
repos []rpmmd.RepoConfig,
seed int64) (*manifest.Manifest, []string, error) {
seedp *int64) (*manifest.Manifest, []string, error) {
seed := distro.SeedFrom(seedp)

if t.Workload != nil {
// For now, if an image type defines its own workload, don't allow any
Expand Down
20 changes: 10 additions & 10 deletions pkg/distro/rhel/rhel10/distro_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func TestImageType_BuildPackages(t *testing.T) {
if assert.NoErrorf(t, err, "d.GetArch(%v) returned err = %v; expected nil", archLabel, err) {
continue
}
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, 0)
manifest, _, err := itStruct.Manifest(&blueprint.Blueprint{}, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
buildPkgs := manifest.GetPackageSetChains()["build"]
assert.NotNil(t, buildPkgs)
Expand Down Expand Up @@ -257,7 +257,7 @@ func TestDistro_ManifestError(t *testing.T) {
imgOpts := distro.ImageOptions{
Size: imgType.Size(0),
}
_, _, err := imgType.Manifest(&bp, imgOpts, nil, 0)
_, _, err := imgType.Manifest(&bp, imgOpts, nil, nil)
assert.NoError(t, err)
}
}
Expand Down Expand Up @@ -427,7 +427,7 @@ func TestDistro_CustomFileSystemManifestError(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.EqualError(t, err, "The following errors occurred while setting up custom mountpoints:\npath \"/etc\" is not allowed")
}
}
Expand All @@ -449,7 +449,7 @@ func TestDistro_TestRootMountPoint(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
}
}
Expand All @@ -475,7 +475,7 @@ func TestDistro_CustomFileSystemSubDirectories(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "edge-") {
continue
} else {
Expand Down Expand Up @@ -513,7 +513,7 @@ func TestDistro_MountpointsWithArbitraryDepthAllowed(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
if strings.HasPrefix(imgTypeName, "edge-") {
continue
} else {
Expand Down Expand Up @@ -547,7 +547,7 @@ func TestDistro_DirtyMountpointsNotAllowed(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.EqualError(t, err, "The following errors occurred while setting up custom mountpoints:\npath \"//\" must be canonical\npath \"/var//\" must be canonical\npath \"/var//log/audit/\" must be canonical")
}
}
Expand All @@ -569,7 +569,7 @@ func TestDistro_CustomUsrPartitionNotLargeEnough(t *testing.T) {
arch, _ := r10distro.GetArch(archName)
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, 0)
_, _, err := imgType.Manifest(&bp, distro.ImageOptions{}, nil, nil)
assert.NoError(t, err)
}
}
Expand Down Expand Up @@ -618,7 +618,7 @@ func TestDiskAndFilesystemCustomizationsError(t *testing.T) {
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
options := distro.ImageOptions{}
_, _, err := imgType.Manifest(&bp, options, nil, 0)
_, _, err := imgType.Manifest(&bp, options, nil, nil)
if skipTest[imgTypeName] {
continue
}
Expand Down Expand Up @@ -664,7 +664,7 @@ func TestNoDiskCustomizationsNoError(t *testing.T) {
for _, imgTypeName := range arch.ListImageTypes() {
imgType, _ := arch.GetImageType(imgTypeName)
options := distro.ImageOptions{}
_, _, err := imgType.Manifest(&bp, options, nil, 0)
_, _, err := imgType.Manifest(&bp, options, nil, nil)
if skipTest[imgTypeName] {
continue
}
Expand Down
Loading

0 comments on commit 7ed5bab

Please sign in to comment.